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 <sbel...@google.com>
Signed-off-by: Junio C Hamano <gits...@pobox.com>
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.schinde...@gmx.de>
Signed-off-by: Stefan Beller <sbel...@google.com>
Signed-off-by: Junio C Hamano <gits...@pobox.com>
@@ -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 <sbel...@google.com>
- Signed-off-by: Junio C Hamano <gits...@pobox.com>
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 {
