Merge branch 'sb/range-diff-colors'

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

diff --git a/diff.c b/diff.c
index 145cfbae5929c69224f9f9e5bc473f2a221603de..0549fc6e2cf3e0ea2610921be5ee0bdf990f2670 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -624,42 +624,54 @@ static void check_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2,
 }
 
 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)
@@ -669,7 +681,7 @@ static void emit_line_0(struct diff_options *o,
 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 +1199,9 @@ static void dim_moved_lines(struct diff_options *o)
 }
 
 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;
@@ -1201,18 +1213,15 @@ static void emit_line_ws_markup(struct diff_options *o,
        }
 
        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 +1245,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,
                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:
@@ -1274,7 +1283,9 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,
                        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:
@@ -1317,7 +1328,9 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,
                                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;
@@ -1360,7 +1373,9 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,
                        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:
@@ -4375,6 +4390,9 @@ void diff_setup(struct diff_options *options)
 
        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;
@@ -4852,6 +4870,12 @@ int diff_opt_parse(struct diff_options *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 --git a/diff.h b/diff.h
index 89544e64bc5797fe40f069e84657d85589f0252a..a30cc35ec3b4cb525340691a39ca88e89e379e4c 100644 (file)
--- a/diff.h
+++ b/diff.h
@@ -194,6 +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;

diff --git a/range-diff.c b/range-diff.c
index b6b9abac266f3a6a2abdfcdd815f80b7da930854..3e9b9844012dac7fb4a0e80451e0d459da9a820e 100644 (file)
--- a/range-diff.c
+++ b/range-diff.c
@@ -38,6 +38,14 @@ static int read_patches(const char *range, struct string_list *list)
 
        argv_array_pushl(&cp.args, "log", "--no-color", "-p", "--no-merges",
                        "--reverse", "--date-order", "--decorate=no",
+                       /*
+                        * Choose indicators that are not used anywhere
+                        * else in diffs, but still look reasonable
+                        * (e.g. will not be confusing when debugging)
+                        */
+                       "--output-indicator-new=>",
+                       "--output-indicator-old=<",
+                       "--output-indicator-context=#",
                        "--no-abbrev-commit", range,
                        NULL);
        cp.out = -1;
@@ -82,6 +90,7 @@ static int read_patches(const char *range, struct string_list *list)
                        strbuf_addch(&buf, '\n');
                        if (!util->diff_offset)
                                util->diff_offset = buf.len;
+                       strbuf_addch(&buf, ' ');
                        strbuf_addbuf(&buf, &line);
                } else if (in_header) {
                        if (starts_with(line.buf, "Author: ")) {
@@ -108,8 +117,19 @@ static int read_patches(const char *range, struct string_list *list)
                         * we are not interested.
                         */
                        continue;
-               else
+               else if (line.buf[0] == '>') {
+                       strbuf_addch(&buf, '+');
+                       strbuf_add(&buf, line.buf + 1, line.len - 1);
+               } else if (line.buf[0] == '<') {
+                       strbuf_addch(&buf, '-');
+                       strbuf_add(&buf, line.buf + 1, line.len - 1);
+               } else if (line.buf[0] == '#') {
+                       strbuf_addch(&buf, ' ');
+                       strbuf_add(&buf, line.buf + 1, line.len - 1);
+               } else {
+                       strbuf_addch(&buf, ' ');
                        strbuf_addbuf(&buf, &line);
+               }
 
                strbuf_addch(&buf, '\n');
                util->diffsize++;

diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 2237c7f4af9464cbee087696ec3a86aeaa86e033..c94f9bf5ee12c1db38764ac6e51bbc011d3a2619 100755 (executable)
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -133,13 +133,52 @@ test_expect_success 'changed message' '
            Z
            +    Also a silly comment here!
            +
-           Zdiff --git a/file b/file
-           Z--- a/file
-           Z+++ b/file
+           Z diff --git a/file b/file
+           Z --- a/file
+           Z +++ b/file
        3:  147e64e = 3:  b9cb956 s/11/B/
        4:  a63e992 = 4:  8add5f1 s/12/B/
        EOF
        test_cmp expected actual
 '
 
+test_expect_success 'dual-coloring' '
+       sed -e "s|^:||" >expect <<-\EOF &&
+       :<YELLOW>1:  a4b3333 = 1:  f686024 s/5/A/<RESET>
+       :<RED>2:  f51d370 <RESET><YELLOW>!<RESET><GREEN> 2:  4ab067d<RESET><YELLOW> s/4/A/<RESET>
+       :    <REVERSE><CYAN>@@ -2,6 +2,8 @@<RESET>
+       :     <RESET>
+       :         s/4/A/<RESET>
+       :     <RESET>
+       :    <REVERSE><GREEN>+<RESET><BOLD>    Also a silly comment here!<RESET>
+       :    <REVERSE><GREEN>+<RESET>
+       :      diff --git a/file b/file<RESET>
+       :      --- a/file<RESET>
+       :      +++ b/file<RESET>
+       :<RED>3:  0559556 <RESET><YELLOW>!<RESET><GREEN> 3:  b9cb956<RESET><YELLOW> s/11/B/<RESET>
+       :    <REVERSE><CYAN>@@ -10,7 +10,7 @@<RESET>
+       :      9<RESET>
+       :      10<RESET>
+       :    <RED> -11<RESET>
+       :    <REVERSE><RED>-<RESET><FAINT;GREEN>+BB<RESET>
+       :    <REVERSE><GREEN>+<RESET><BOLD;GREEN>+B<RESET>
+       :      12<RESET>
+       :      13<RESET>
+       :      14<RESET>
+       :<RED>4:  d966c5c <RESET><YELLOW>!<RESET><GREEN> 4:  8add5f1<RESET><YELLOW> s/12/B/<RESET>
+       :    <REVERSE><CYAN>@@ -8,7 +8,7 @@<RESET>
+       :    <CYAN> @@<RESET>
+       :      9<RESET>
+       :      10<RESET>
+       :    <REVERSE><RED>-<RESET><FAINT> BB<RESET>
+       :    <REVERSE><GREEN>+<RESET><BOLD> B<RESET>
+       :    <RED> -12<RESET>
+       :    <GREEN> +B<RESET>
+       :      13<RESET>
+       EOF
+       git range-diff changed...changed-message --color --dual-color >actual.raw &&
+       test_decode_color >actual <actual.raw &&
+       test_cmp expect actual
+'
+
 test_done

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 4207af40777c69365dc395e800a7e4214beab076..d82fac9d790b9349df908f684e61a03780ac58c8 100644 (file)
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -42,6 +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";