Merge branch 'sb/range-diff-colors'
authorJunio C Hamano <gitster@pobox.com>
Mon, 17 Sep 2018 20:53:54 +0000 (13:53 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 17 Sep 2018 20:53:54 +0000 (13:53 -0700)
The color output support for recently introduced "range-diff"
command got tweaked a bit.

* sb/range-diff-colors:
range-diff: indent special lines as context
range-diff: make use of different output indicators
diff.c: add --output-indicator-{new, old, context}
diff.c: rewrite emit_line_0 more understandably
diff.c: omit check for line prefix in emit_line_0
diff: use emit_line_0 once per line
diff.c: add set_sign to emit_line_0
diff.c: reorder arguments for emit_line_ws_markup
diff.c: simplify caller of emit_line_0
t3206: add color test for range-diff --dual-color
test_decode_color: understand FAINT and ITALIC

1  2 
diff.c
diff.h
t/test-lib-functions.sh
diff --combined diff.c
index 145cfbae5929c69224f9f9e5bc473f2a221603de,03486c35b756ecb098373a4925e8989e2f65146f..0549fc6e2cf3e0ea2610921be5ee0bdf990f2670
--- 1/diff.c
--- 2/diff.c
+++ b/diff.c
@@@ -283,12 -283,10 +283,12 @@@ static int parse_color_moved(const cha
                return COLOR_MOVED_ZEBRA;
        else if (!strcmp(arg, "default"))
                return COLOR_MOVED_DEFAULT;
 +      else if (!strcmp(arg, "dimmed-zebra"))
 +              return COLOR_MOVED_ZEBRA_DIM;
        else if (!strcmp(arg, "dimmed_zebra"))
                return COLOR_MOVED_ZEBRA_DIM;
        else
 -              return error(_("color moved setting must be one of 'no', 'default', 'blocks', 'zebra', 'dimmed_zebra', 'plain'"));
 +              return error(_("color moved setting must be one of 'no', 'default', 'blocks', 'zebra', 'dimmed-zebra', 'plain'"));
  }
  
  static int parse_color_moved_ws(const char *arg)
@@@ -624,42 -622,54 +624,54 @@@ static void check_blank_at_eof(mmfile_
  }
  
  static void emit_line_0(struct diff_options *o,
-                       const char *set, unsigned reverse, const char *reset,
+                       const char *set_sign, const char *set, unsigned reverse, const char *reset,
                        int first, const char *line, int len)
  {
        int has_trailing_newline, has_trailing_carriage_return;
-       int nofirst;
+       int needs_reset = 0; /* at the end of the line */
        FILE *file = o->file;
  
-       if (first)
-               fputs(diff_line_prefix(o), file);
-       else if (!len)
-               return;
+       fputs(diff_line_prefix(o), file);
  
-       if (len == 0) {
-               has_trailing_newline = (first == '\n');
-               has_trailing_carriage_return = (!has_trailing_newline &&
-                                               (first == '\r'));
-               nofirst = has_trailing_newline || has_trailing_carriage_return;
-       } else {
-               has_trailing_newline = (len > 0 && line[len-1] == '\n');
-               if (has_trailing_newline)
-                       len--;
-               has_trailing_carriage_return = (len > 0 && line[len-1] == '\r');
-               if (has_trailing_carriage_return)
-                       len--;
-               nofirst = 0;
+       has_trailing_newline = (len > 0 && line[len-1] == '\n');
+       if (has_trailing_newline)
+               len--;
+       has_trailing_carriage_return = (len > 0 && line[len-1] == '\r');
+       if (has_trailing_carriage_return)
+               len--;
+       if (!len && !first)
+               goto end_of_line;
+       if (reverse && want_color(o->use_color)) {
+               fputs(GIT_COLOR_REVERSE, file);
+               needs_reset = 1;
        }
  
-       if (len || !nofirst) {
-               if (reverse && want_color(o->use_color))
-                       fputs(GIT_COLOR_REVERSE, file);
+       if (set_sign) {
+               fputs(set_sign, file);
+               needs_reset = 1;
+       }
+       if (first)
+               fputc(first, file);
+       if (!len)
+               goto end_of_line;
+       if (set) {
+               if (set_sign && set != set_sign)
+                       fputs(reset, file);
                fputs(set, file);
-               if (first && !nofirst)
-                       fputc(first, file);
-               fwrite(line, len, 1, file);
-               fputs(reset, file);
+               needs_reset = 1;
        }
+       fwrite(line, len, 1, file);
+       needs_reset = 1; /* 'line' may contain color codes. */
+ end_of_line:
+       if (needs_reset)
+               fputs(reset, file);
        if (has_trailing_carriage_return)
                fputc('\r', file);
        if (has_trailing_newline)
  static void emit_line(struct diff_options *o, const char *set, const char *reset,
                      const char *line, int len)
  {
-       emit_line_0(o, set, 0, reset, line[0], line+1, len-1);
+       emit_line_0(o, set, NULL, 0, reset, 0, line, len);
  }
  
  enum diff_symbol {
@@@ -1187,9 -1197,9 +1199,9 @@@ static void dim_moved_lines(struct diff
  }
  
  static void emit_line_ws_markup(struct diff_options *o,
-                               const char *set, const char *reset,
-                               const char *line, int len,
-                               const char *set_sign, char sign,
+                               const char *set_sign, const char *set,
+                               const char *reset,
+                               char sign, const char *line, int len,
                                unsigned ws_rule, int blank_at_eof)
  {
        const char *ws = NULL;
        }
  
        if (!ws && !set_sign)
-               emit_line_0(o, set, 0, reset, sign, line, len);
+               emit_line_0(o, set, NULL, 0, reset, sign, line, len);
        else if (!ws) {
-               /* Emit just the prefix, then the rest. */
-               emit_line_0(o, set_sign ? set_sign : set, !!set_sign, reset,
-                           sign, "", 0);
-               emit_line_0(o, set, 0, reset, 0, line, len);
+               emit_line_0(o, set_sign, set, !!set_sign, reset, sign, line, len);
        } else if (blank_at_eof)
                /* Blank line at EOF - paint '+' as well */
-               emit_line_0(o, ws, 0, reset, sign, line, len);
+               emit_line_0(o, ws, NULL, 0, reset, sign, line, len);
        else {
                /* Emit just the prefix, then the rest. */
-               emit_line_0(o, set_sign ? set_sign : set, !!set_sign, reset,
+               emit_line_0(o, set_sign ? set_sign : set, NULL, !!set_sign, reset,
                            sign, "", 0);
                ws_check_emit(line, len, ws_rule,
                              o->file, set, reset, ws);
@@@ -1236,7 -1243,7 +1245,7 @@@ static void emit_diff_symbol_from_struc
                context = diff_get_color_opt(o, DIFF_CONTEXT);
                reset = diff_get_color_opt(o, DIFF_RESET);
                putc('\n', o->file);
-               emit_line_0(o, context, 0, reset, '\\',
+               emit_line_0(o, context, NULL, 0, reset, '\\',
                            nneof, strlen(nneof));
                break;
        case DIFF_SYMBOL_SUBMODULE_HEADER:
                        else if (c == '-')
                                set = diff_get_color_opt(o, DIFF_FILE_OLD);
                }
-               emit_line_ws_markup(o, set, reset, line, len, set_sign, ' ',
+               emit_line_ws_markup(o, set_sign, set, reset,
+                                   o->output_indicators[OUTPUT_INDICATOR_CONTEXT],
+                                   line, len,
                                    flags & (DIFF_SYMBOL_CONTENT_WS_MASK), 0);
                break;
        case DIFF_SYMBOL_PLUS:
                                set = diff_get_color_opt(o, DIFF_CONTEXT_BOLD);
                        flags &= ~DIFF_SYMBOL_CONTENT_WS_MASK;
                }
-               emit_line_ws_markup(o, set, reset, line, len, set_sign, '+',
+               emit_line_ws_markup(o, set_sign, set, reset,
+                                   o->output_indicators[OUTPUT_INDICATOR_NEW],
+                                   line, len,
                                    flags & DIFF_SYMBOL_CONTENT_WS_MASK,
                                    flags & DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF);
                break;
                        else
                                set = diff_get_color_opt(o, DIFF_CONTEXT_DIM);
                }
-               emit_line_ws_markup(o, set, reset, line, len, set_sign, '-',
+               emit_line_ws_markup(o, set_sign, set, reset,
+                                   o->output_indicators[OUTPUT_INDICATOR_OLD],
+                                   line, len,
                                    flags & DIFF_SYMBOL_CONTENT_WS_MASK, 0);
                break;
        case DIFF_SYMBOL_WORDS_PORCELAIN:
@@@ -2141,8 -2154,8 +2156,8 @@@ static void init_diff_words_data(struc
                if (regcomp(ecbdata->diff_words->word_regex,
                            o->word_regex,
                            REG_EXTENDED | REG_NEWLINE))
 -                      die ("Invalid regular expression: %s",
 -                           o->word_regex);
 +                      die("invalid regular expression: %s",
 +                          o->word_regex);
        }
        for (i = 0; i < ARRAY_SIZE(diff_words_styles); i++) {
                if (o->word_diff == diff_words_styles[i].type) {
@@@ -3968,7 -3981,7 +3983,7 @@@ static void prep_temp_blob(const char *
        temp->tempfile = mks_tempfile_ts(tempfile.buf, strlen(base) + 1);
        if (!temp->tempfile)
                die_errno("unable to create temp-file");
 -      if (convert_to_working_tree(path,
 +      if (convert_to_working_tree(&the_index, path,
                        (const char *)blob, (size_t)size, &buf)) {
                blob = buf.buf;
                size = buf.len;
@@@ -4375,6 -4388,9 +4390,9 @@@ void diff_setup(struct diff_options *op
  
        options->file = stdout;
  
+       options->output_indicators[OUTPUT_INDICATOR_NEW] = '+';
+       options->output_indicators[OUTPUT_INDICATOR_OLD] = '-';
+       options->output_indicators[OUTPUT_INDICATOR_CONTEXT] = ' ';
        options->abbrev = DEFAULT_ABBREV;
        options->line_termination = '\n';
        options->break_opt = -1;
@@@ -4489,6 -4505,16 +4507,6 @@@ void diff_setup_done(struct diff_option
  
        if (options->detect_rename && options->rename_limit < 0)
                options->rename_limit = diff_rename_limit_default;
 -      if (options->setup & DIFF_SETUP_USE_CACHE) {
 -              if (!active_cache)
 -                      /* read-cache does not die even when it fails
 -                       * so it is safe for us to do this here.  Also
 -                       * it does not smudge active_cache or active_nr
 -                       * when it fails, so we do not have to worry about
 -                       * cleaning it up ourselves either.
 -                       */
 -                      read_cache();
 -      }
        if (hexsz < options->abbrev)
                options->abbrev = hexsz; /* full */
  
@@@ -4852,6 -4878,12 +4870,12 @@@ int diff_opt_parse(struct diff_options 
                 options->output_format |= DIFF_FORMAT_DIFFSTAT;
        } else if (!strcmp(arg, "--no-compact-summary"))
                 options->flags.stat_with_summary = 0;
+       else if (skip_prefix(arg, "--output-indicator-new=", &arg))
+               options->output_indicators[OUTPUT_INDICATOR_NEW] = arg[0];
+       else if (skip_prefix(arg, "--output-indicator-old=", &arg))
+               options->output_indicators[OUTPUT_INDICATOR_OLD] = arg[0];
+       else if (skip_prefix(arg, "--output-indicator-context=", &arg))
+               options->output_indicators[OUTPUT_INDICATOR_CONTEXT] = arg[0];
  
        /* renames options */
        else if (starts_with(arg, "-B") ||
diff --combined diff.h
index 89544e64bc5797fe40f069e84657d85589f0252a,d7edc64454a443331176e926097a4600d3648dae..a30cc35ec3b4cb525340691a39ca88e89e379e4c
--- 1/diff.h
--- 2/diff.h
+++ b/diff.h
@@@ -194,6 -194,11 +194,11 @@@ struct diff_options 
        FILE *file;
        int close_file;
  
+ #define OUTPUT_INDICATOR_NEW 0
+ #define OUTPUT_INDICATOR_OLD 1
+ #define OUTPUT_INDICATOR_CONTEXT 2
+       char output_indicators[3];
        struct pathspec pathspec;
        pathchange_fn_t pathchange;
        change_fn_t change;
@@@ -266,15 -271,15 +271,15 @@@ const char *diff_line_prefix(struct dif
  
  extern const char mime_boundary_leader[];
  
 -extern struct combine_diff_path *diff_tree_paths(
 +struct combine_diff_path *diff_tree_paths(
        struct combine_diff_path *p, const struct object_id *oid,
        const struct object_id **parents_oid, int nparent,
        struct strbuf *base, struct diff_options *opt);
 -extern int diff_tree_oid(const struct object_id *old_oid,
 -                       const struct object_id *new_oid,
 -                       const char *base, struct diff_options *opt);
 -extern int diff_root_tree_oid(const struct object_id *new_oid, const char *base,
 -                            struct diff_options *opt);
 +int diff_tree_oid(const struct object_id *old_oid,
 +                const struct object_id *new_oid,
 +                const char *base, struct diff_options *opt);
 +int diff_root_tree_oid(const struct object_id *new_oid, const char *base,
 +                     struct diff_options *opt);
  
  struct combine_diff_path {
        struct combine_diff_path *next;
        st_add4(sizeof(struct combine_diff_path), (l), 1, \
                st_mult(sizeof(struct combine_diff_parent), (n)))
  
 -extern void show_combined_diff(struct combine_diff_path *elem, int num_parent,
 -                            int dense, struct rev_info *);
 +void show_combined_diff(struct combine_diff_path *elem, int num_parent,
 +                      int dense, struct rev_info *);
  
 -extern void diff_tree_combined(const struct object_id *oid, const struct oid_array *parents, int dense, struct rev_info *rev);
 +void diff_tree_combined(const struct object_id *oid, const struct oid_array *parents, int dense, struct rev_info *rev);
  
 -extern void diff_tree_combined_merge(const struct commit *commit, int dense, struct rev_info *rev);
 +void diff_tree_combined_merge(const struct commit *commit, int dense, struct rev_info *rev);
  
  void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const char *b);
  
 -extern int diff_can_quit_early(struct diff_options *);
 +int diff_can_quit_early(struct diff_options *);
  
 -extern void diff_addremove(struct diff_options *,
 -                         int addremove,
 -                         unsigned mode,
 -                         const struct object_id *oid,
 -                         int oid_valid,
 -                         const char *fullpath, unsigned dirty_submodule);
 +void diff_addremove(struct diff_options *,
 +                  int addremove,
 +                  unsigned mode,
 +                  const struct object_id *oid,
 +                  int oid_valid,
 +                  const char *fullpath, unsigned dirty_submodule);
  
 -extern void diff_change(struct diff_options *,
 -                      unsigned mode1, unsigned mode2,
 -                      const struct object_id *old_oid,
 -                      const struct object_id *new_oid,
 -                      int old_oid_valid, int new_oid_valid,
 -                      const char *fullpath,
 -                      unsigned dirty_submodule1, unsigned dirty_submodule2);
 +void diff_change(struct diff_options *,
 +               unsigned mode1, unsigned mode2,
 +               const struct object_id *old_oid,
 +               const struct object_id *new_oid,
 +               int old_oid_valid, int new_oid_valid,
 +               const char *fullpath,
 +               unsigned dirty_submodule1, unsigned dirty_submodule2);
  
 -extern struct diff_filepair *diff_unmerge(struct diff_options *, const char *path);
 +struct diff_filepair *diff_unmerge(struct diff_options *, const char *path);
  
  #define DIFF_SETUP_REVERSE            1
 -#define DIFF_SETUP_USE_CACHE          2
  #define DIFF_SETUP_USE_SIZE_CACHE     4
  
  /*
   * Poor man's alternative to parse-option, to allow both stuck form
   * (--option=value) and separate form (--option value).
   */
 -extern int parse_long_opt(const char *opt, const char **argv,
 -                       const char **optarg);
 -
 -extern int git_diff_basic_config(const char *var, const char *value, void *cb);
 -extern int git_diff_heuristic_config(const char *var, const char *value, void *cb);
 -extern void init_diff_ui_defaults(void);
 -extern int git_diff_ui_config(const char *var, const char *value, void *cb);
 -extern void diff_setup(struct diff_options *);
 -extern int diff_opt_parse(struct diff_options *, const char **, int, const char *);
 -extern void diff_setup_done(struct diff_options *);
 -extern int git_config_rename(const char *var, const char *value);
 +int parse_long_opt(const char *opt, const char **argv,
 +                 const char **optarg);
 +
 +int git_diff_basic_config(const char *var, const char *value, void *cb);
 +int git_diff_heuristic_config(const char *var, const char *value, void *cb);
 +void init_diff_ui_defaults(void);
 +int git_diff_ui_config(const char *var, const char *value, void *cb);
 +void diff_setup(struct diff_options *);
 +int diff_opt_parse(struct diff_options *, const char **, int, const char *);
 +void diff_setup_done(struct diff_options *);
 +int git_config_rename(const char *var, const char *value);
  
  #define DIFF_DETECT_RENAME    1
  #define DIFF_DETECT_COPY      2
  
  #define DIFF_PICKAXE_IGNORE_CASE      32
  
 -extern void diffcore_std(struct diff_options *);
 -extern void diffcore_fix_diff_index(struct diff_options *);
 +void diffcore_std(struct diff_options *);
 +void diffcore_fix_diff_index(struct diff_options *);
  
  #define COMMON_DIFF_OPTIONS_HELP \
  "\ncommon diff options:\n" \
  "                show all files diff when -S is used and hit is found.\n" \
  "  -a  --text    treat all files as text.\n"
  
 -extern int diff_queue_is_empty(void);
 -extern void diff_flush(struct diff_options*);
 -extern void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc);
 +int diff_queue_is_empty(void);
 +void diff_flush(struct diff_options*);
 +void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc);
  
  /* diff-raw status letters */
  #define DIFF_STATUS_ADDED             'A'
   * This is different from find_unique_abbrev() in that
   * it stuffs the result with dots for alignment.
   */
 -extern const char *diff_aligned_abbrev(const struct object_id *sha1, int);
 +const char *diff_aligned_abbrev(const struct object_id *sha1, int);
  
  /* do not report anything on removed paths */
  #define DIFF_SILENT_ON_REMOVED 01
  /* report racily-clean paths as modified */
  #define DIFF_RACY_IS_MODIFIED 02
 -extern int run_diff_files(struct rev_info *revs, unsigned int option);
 -extern int run_diff_index(struct rev_info *revs, int cached);
 +int run_diff_files(struct rev_info *revs, unsigned int option);
 +int run_diff_index(struct rev_info *revs, int cached);
  
 -extern int do_diff_cache(const struct object_id *, struct diff_options *);
 -extern int diff_flush_patch_id(struct diff_options *, struct object_id *, int);
 +int do_diff_cache(const struct object_id *, struct diff_options *);
 +int diff_flush_patch_id(struct diff_options *, struct object_id *, int);
  
 -extern int diff_result_code(struct diff_options *, int);
 +int diff_result_code(struct diff_options *, int);
  
 -extern void diff_no_index(struct rev_info *, int, const char **);
 +void diff_no_index(struct rev_info *, int, const char **);
  
 -extern int index_differs_from(const char *def, const struct diff_flags *flags,
 -                            int ita_invisible_in_index);
 +int index_differs_from(const char *def, const struct diff_flags *flags,
 +                     int ita_invisible_in_index);
  
  /*
   * Fill the contents of the filespec "df", respecting any textconv defined by
   * struct. If it is non-NULL, then "outbuf" points to a newly allocated buffer
   * that should be freed by the caller.
   */
 -extern size_t fill_textconv(struct userdiff_driver *driver,
 -                          struct diff_filespec *df,
 -                          char **outbuf);
 +size_t fill_textconv(struct userdiff_driver *driver,
 +                   struct diff_filespec *df,
 +                   char **outbuf);
  
  /*
   * Look up the userdiff driver for the given filespec, and return it if
   * and only if it has textconv enabled (otherwise return NULL). The result
   * can be passed to fill_textconv().
   */
 -extern struct userdiff_driver *get_textconv(struct diff_filespec *one);
 +struct userdiff_driver *get_textconv(struct diff_filespec *one);
  
  /*
   * Prepare diff_filespec and convert it using diff textconv API
   * if the textconv driver exists.
   * Return 1 if the conversion succeeds, 0 otherwise.
   */
 -extern int textconv_object(const char *path, unsigned mode, const struct object_id *oid, int oid_valid, char **buf, unsigned long *buf_size);
 +int textconv_object(const char *path, unsigned mode, const struct object_id *oid, int oid_valid, char **buf, unsigned long *buf_size);
  
 -extern int parse_rename_score(const char **cp_p);
 +int parse_rename_score(const char **cp_p);
  
 -extern long parse_algorithm_value(const char *value);
 +long parse_algorithm_value(const char *value);
  
 -extern void print_stat_summary(FILE *fp, int files,
 -                             int insertions, int deletions);
 -extern void setup_diff_pager(struct diff_options *);
 +void print_stat_summary(FILE *fp, int files,
 +                      int insertions, int deletions);
 +void setup_diff_pager(struct diff_options *);
  
  #endif /* DIFF_H */
diff --combined t/test-lib-functions.sh
index 4207af40777c69365dc395e800a7e4214beab076,be8244c47b56611fe9abdddd2b539cf0cd95f1d0..d82fac9d790b9349df908f684e61a03780ac58c8
@@@ -42,6 -42,8 +42,8 @@@ test_decode_color () 
                function name(n) {
                        if (n == 0) return "RESET";
                        if (n == 1) return "BOLD";
+                       if (n == 2) return "FAINT";
+                       if (n == 3) return "ITALIC";
                        if (n == 7) return "REVERSE";
                        if (n == 30) return "BLACK";
                        if (n == 31) return "RED";
@@@ -565,14 -567,6 +567,14 @@@ test_path_is_dir () 
        fi
  }
  
 +test_path_exists () {
 +      if ! test -e "$1"
 +      then
 +              echo "Path $1 doesn't exist. $2"
 +              false
 +      fi
 +}
 +
  # Check if the directory exists and is empty as expected, barf otherwise.
  test_dir_is_empty () {
        test_path_is_dir "$1" &&