This is also avaliable as git fetch https://github.com/stefanbeller/git sb/range-diff-colors This applies on top of js/range-diff (a7be92acd9600), this version * resolves a semantic conflict in the test (Did range-diff change its output slightly?) * addressed Johannes feedback, such as -> keeping the reverse computation out of emit_line_0 -> better commit messages. -> Split "use emit_line_0 once per line" to have an additional "diff.c: omit check for line prefix in emit_line_0" as having these two things in the same patch is confusing The interdiff seems more useful than the range-diff here, both attached below. Thanks, Stefan Stefan Beller (8): test_decode_color: understand FAINT and ITALIC t3206: add color test for range-diff --dual-color diff.c: simplify caller of emit_line_0 diff.c: reorder arguments for emit_line_ws_markup diff.c: add set_sign to emit_line_0 diff: use emit_line_0 once per line diff.c: omit check for line prefix in emit_line_0 diff.c: rewrite emit_line_0 more understandably diff.c | 91 ++++++++++++++++++++++------------------- t/t3206-range-diff.sh | 39 ++++++++++++++++++ t/test-lib-functions.sh | 2 + 3 files changed, 91 insertions(+), 41 deletions(-) (interdiff seems to be more useful) git diff 4dc97b54a35..058e03a1601 diff --git a/builtin/range-diff.c b/builtin/range-diff.c index ef3ba22e297..f52d45d9d61 100644 --- a/builtin/range-diff.c +++ b/builtin/range-diff.c @@ -53,6 +53,8 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix) else i += c; } + while (i < argc) + argv[j++] = argv[i++]; argc = j; diff_setup_done(&diffopt); diff --git a/diff.c b/diff.c index af9316c8f91..c5c7739ce34 100644 --- a/diff.c +++ b/diff.c @@ -622,13 +622,11 @@ static void check_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2, } static void emit_line_0(struct diff_options *o, - const char *set_sign, const char *set, 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 reverse = !!set && !!set_sign; - int needs_reset = 0; - + int needs_reset = 0; /* at the end of the line */ FILE *file = o->file; fputs(diff_line_prefix(o), file); @@ -649,8 +647,8 @@ static void emit_line_0(struct diff_options *o, needs_reset = 1; } - if (set_sign || set) { - fputs(set_sign ? set_sign : set, file); + if (set_sign) { + fputs(set_sign, file); needs_reset = 1; } @@ -667,7 +665,7 @@ static void emit_line_0(struct diff_options *o, needs_reset = 1; } fwrite(line, len, 1, file); - needs_reset |= len > 0; + needs_reset = 1; /* 'line' may contain color codes. */ end_of_line: if (needs_reset) @@ -681,7 +679,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, NULL, reset, 0, line, len); + emit_line_0(o, set, NULL, 0, reset, 0, line, len); } enum diff_symbol { @@ -1213,15 +1211,16 @@ static void emit_line_ws_markup(struct diff_options *o, } if (!ws && !set_sign) - emit_line_0(o, set, NULL, reset, sign, line, len); + emit_line_0(o, set, NULL, 0, reset, sign, line, len); else if (!ws) { - emit_line_0(o, set_sign, set, reset, sign, 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, NULL, 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, reset, sign, "", 0); + 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); } @@ -1244,7 +1243,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, NULL, reset, '\\', + emit_line_0(o, context, NULL, 0, reset, '\\', nneof, strlen(nneof)); break; case DIFF_SYMBOL_SUBMODULE_HEADER: ./git-range-diff github/sb/range-diff-colors... below: 22: 0fedd4c0a20 = 22: dd772035ac9 test_decode_color: understand FAINT and ITALIC 23: 6a1c7698c68 ! 23: 5701745379b t3206: add color test for range-diff --dual-color @@ -23,7 +23,7 @@ + : s/4/A/<RESET> + : <RESET> + : <REVERSE><GREEN>+<RESET><BOLD> Also a silly comment here!<RESET> -+ : <REVERSE><GREEN>+<RESET> ++ : <REVERSE><GREEN>+<RESET><BOLD><RESET> + : diff --git a/file b/file<RESET> + : <RED> --- a/file<RESET> + : <GREEN> +++ b/file<RESET> 24: 7e12ece1d34 = 24: 4e56f63a5f5 diff.c: simplify caller of emit_line_0 25: 74dabd6d36f ! 25: cf126556940 diff.c: reorder arguments for emit_line_ws_markup @@ -3,7 +3,8 @@ diff.c: reorder arguments for emit_line_ws_markup The order shall be all colors first, then the content, flags at the end. - The colors are in order. + The colors are in the order of occurrence, i.e. first the color for the + sign and then the color for the rest of the line. Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> 26: e304e15aa6b ! 26: 69008364cb8 diff.c: add set_sign to emit_line_0 @@ -2,14 +2,17 @@ diff.c: add set_sign to emit_line_0 - For now just change the signature, we'll reason about the actual - change in a follow up patch. + Split the meaning of the `set` parameter that is passed to + emit_line_0()` to separate between the color of the "sign" (i.e. + the diff marker '+', '-' or ' ' that is passed in as the `first` + parameter) and the color of the rest of the line. - Pass 'set_sign' (which is output before the sign) and 'set' which - controls the color after the first character. Hence, promote any - 'set's to 'set_sign' as we want to have color before the sign - for now. + This changes the meaning of the `set` parameter to no longer refer + to the color of the diff marker, but instead to refer to the color + of the rest of the line. A value of `NULL` indicates that the rest + of the line wants to be colored the same as the diff marker. + Helped-by: Johannes Schindelin <Johannes.Schindelin@xxxxxx> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> @@ -30,12 +33,15 @@ if (reverse && want_color(o->use_color)) fputs(GIT_COLOR_REVERSE, file); - fputs(set, file); -+ if (set_sign && set_sign[0]) ++ if (set_sign) + fputs(set_sign, file); if (first && !nofirst) fputc(first, file); -+ if (set) ++ if (set && set != set_sign) { ++ if (set_sign) ++ fputs(reset, file); + fputs(set, file); ++ } fwrite(line, len, 1, file); fputs(reset, file); } 27: 8d935d5212c < -: ----------- diff: use emit_line_0 once per line 28: 2344aac787a < -: ----------- diff.c: compute reverse locally in emit_line_0 -: ----------- > 27: 5d2629281a1 diff: use emit_line_0 once per line -: ----------- > 28: 5240e94a69a diff.c: omit check for line prefix in emit_line_0 29: 4dc97b54a35 ! 29: 058e03a1601 diff.c: rewrite emit_line_0 more understandably @@ -2,21 +2,15 @@ diff.c: rewrite emit_line_0 more understandably - emit_line_0 has no nested conditions, but puts out all its arguments - (if set) in order. There is still a slight confusion with set - and set_sign, but let's defer that to a later patch. + Rewrite emit_line_0 to have fewer (nested) conditions. - 'first' used be output always no matter if it was 0, but that got lost - at "diff: add an internal option to dual-color diffs of diffs", - 2018-07-21), as there we broadened the meaning of 'first' to also - signal an early return. - - The change in 'emit_line' makes sure that 'first' is never content, but - always under our control, a sign or special character in the beginning - of the line (or 0, in which case we ignore it). + The change in 'emit_line' makes sure that 'first' is never user data, + but always under our control, a sign or special character in the + beginning of the line (or 0, in which case we ignore it). + So from now on, let's pass only a diff marker or 0 as the 'first' + character of the line. Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx> - Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> diff --git a/diff.c b/diff.c --- a/diff.c @@ -26,9 +20,7 @@ { int has_trailing_newline, has_trailing_carriage_return; - int nofirst; - int reverse = !!set && !!set_sign; -+ int needs_reset = 0; -+ ++ int needs_reset = 0; /* at the end of the line */ FILE *file = o->file; fputs(diff_line_prefix(o), file); @@ -51,15 +43,16 @@ - if (len || !nofirst) { - if (reverse && want_color(o->use_color)) - fputs(GIT_COLOR_REVERSE, file); -- if (set_sign || set) -- fputs(set_sign ? set_sign : set, file); +- if (set_sign) +- fputs(set_sign, file); - if (first && !nofirst) - fputc(first, file); - if (len) { -- if (set_sign && set && set != set_sign) -- fputs(reset, file); -- if (set) +- if (set && set != set_sign) { +- if (set_sign) +- fputs(reset, file); - fputs(set, file); +- } - fwrite(line, len, 1, file); - } - fputs(reset, file); @@ -79,8 +72,8 @@ + needs_reset = 1; + } + -+ if (set_sign || set) { -+ fputs(set_sign ? set_sign : set, file); ++ if (set_sign) { ++ fputs(set_sign, file); + needs_reset = 1; } + @@ -97,7 +90,7 @@ + needs_reset = 1; + } + fwrite(line, len, 1, file); -+ needs_reset |= len > 0; ++ needs_reset = 1; /* 'line' may contain color codes. */ + +end_of_line: + if (needs_reset) @@ -109,8 +102,8 @@ static void emit_line(struct diff_options *o, const char *set, const char *reset, const char *line, int len) { -- emit_line_0(o, set, NULL, reset, line[0], line+1, len-1); -+ emit_line_0(o, set, NULL, reset, 0, line, len); +- emit_line_0(o, set, NULL, 0, reset, line[0], line+1, len-1); ++ emit_line_0(o, set, NULL, 0, reset, 0, line, len); } enum diff_symbol {
