emit_line_0 grew complicated again, so here is an attempt to make it
a bit simpler. emit_line_0 is called for all lines that are added,
removed or context lines, and it follows the format:

 <sign color> <sign> <main color> <content of length 'len'> <reset> \
    <CR> <LF>

with each of the components optional.

Another follow up cleanup (that also touches the tests) could be
a stricter check to consolidate with ws_check_emit (and not emit the
color/reset twice).

Signed-off-by: Stefan Beller <sbel...@google.com>
---
 
 oops wrong patch, this one should do.
 Now this passes all tests. 

 diff.c                     | 84 +++++++++++++++++++-------------------
 t/t4015-diff-whitespace.sh | 10 ++---
 2 files changed, 48 insertions(+), 46 deletions(-)

diff --git a/diff.c b/diff.c
index 028d7d9a59c..0b00df7b3c8 100644
--- a/diff.c
+++ b/diff.c
@@ -563,42 +563,48 @@ 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,
-                       int first, const char *line, int len)
+                       const char *maincolor, const char *signcolor,
+                       const char *reset, const char *sign,
+                       const char *line, int len)
 {
        int has_trailing_newline, has_trailing_carriage_return;
-       int nofirst;
        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--;
+
+       /*
+        * Color the sign differently if requested, otherwise use the main
+        * color.
+        */
+       if (signcolor)
+               fputs(signcolor, file);
+       else if (maincolor)
+               fputs(maincolor, file);
+
+       if (sign)
+               fputs(sign, file);
+
+       /*
+        * Only put the main color here if it we did not color the sign the
+        * same way already
+        */
+       if (signcolor && maincolor && strcmp(signcolor, maincolor))
+               fputs(maincolor, file);
 
-       if (len || !nofirst) {
-               if (reverse && want_color(o->use_color))
-                       fputs(GIT_COLOR_REVERSE, file);
-               fputs(set, file);
-               if (first && !nofirst)
-                       fputc(first, file);
+       if (len)
                fwrite(line, len, 1, file);
+
+       if (((maincolor && *maincolor) || (signcolor && *signcolor) || len > 0)
+           && reset)
                fputs(reset, file);
-       }
+
        if (has_trailing_carriage_return)
                fputc('\r', file);
        if (has_trailing_newline)
@@ -608,7 +614,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, reset, NULL, line, len);
 }
 
 enum diff_symbol {
@@ -980,20 +986,16 @@ static void emit_line_ws_markup(struct diff_options *o,
                        ws = NULL;
        }
 
-       if (!ws && !set_sign)
-               emit_line_0(o, set, 0, reset, sign[0], 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], "", 0);
-               emit_line_0(o, set, 0, reset, 0, line, len);
-       } else if (blank_at_eof)
+       if (!ws)
+               emit_line_0(o, 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[0], line, len);
+               emit_line_0(o, ws, set_sign, reset, sign, line, len);
        else {
                /* Emit just the prefix, then the rest. */
-               emit_line_0(o, set_sign ? set_sign : set, !!set_sign, reset,
-                           sign[0], "", 0);
+               emit_line_0(o, set, set_sign, reset,
+                           sign, "", 0);
                ws_check_emit(line, len, ws_rule,
                              o->file, set, reset, ws);
        }
@@ -1016,7 +1018,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, 0, reset, "\\",
                            nneof, strlen(nneof));
                break;
        case DIFF_SYMBOL_SUBMODULE_HEADER:
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 17df491a3ab..95baf237a83 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -945,7 +945,7 @@ test_expect_success 'ws-error-highlight test setup' '
        <BOLD>--- a/x<RESET>
        <BOLD>+++ b/x<RESET>
        <CYAN>@@ -1,2 +1,3 @@<RESET>
-        <RESET>0. blank-at-eol<RESET><BLUE> <RESET>
+        0. blank-at-eol<RESET><BLUE> <RESET>
        <RED>-<RESET><RED>1. blank-at-eol<RESET><BLUE> <RESET>
        <GREEN>+<RESET><GREEN>1. still-blank-at-eol<RESET><BLUE> <RESET>
        <GREEN>+<RESET><GREEN>2. and a new line<RESET><BLUE> <RESET>
@@ -1140,7 +1140,7 @@ test_expect_success 'detect malicious moved code, inside 
file' '
        <CYAN>@@ -5,13 +5,6 @@<RESET> <RESET>printf("Hello ");<RESET>
         printf("World\n");<RESET>
         }<RESET>
-        <RESET>
+        
        <BRED>-int secure_foo(struct user *u)<RESET>
        <BRED>-{<RESET>
        <BLUE>-if (!u->is_allowed_foo)<RESET>
@@ -1158,7 +1158,7 @@ test_expect_success 'detect malicious moved code, inside 
file' '
        <CYAN>@@ -4,6 +4,13 @@<RESET> <RESET>int bar()<RESET>
         printf("Hello World, but different\n");<RESET>
         }<RESET>
-        <RESET>
+        
        <BGREEN>+<RESET><BGREEN>int secure_foo(struct user *u)<RESET>
        <BGREEN>+<RESET><BGREEN>{<RESET>
        <GREEN>+<RESET><GREEN>foo(u);<RESET>
@@ -1189,7 +1189,7 @@ test_expect_success 'plain moved code, inside file' '
        <CYAN>@@ -5,13 +5,6 @@<RESET> <RESET>printf("Hello ");<RESET>
         printf("World\n");<RESET>
         }<RESET>
-        <RESET>
+        
        <BRED>-int secure_foo(struct user *u)<RESET>
        <BRED>-{<RESET>
        <BRED>-if (!u->is_allowed_foo)<RESET>
@@ -1207,7 +1207,7 @@ test_expect_success 'plain moved code, inside file' '
        <CYAN>@@ -4,6 +4,13 @@<RESET> <RESET>int bar()<RESET>
         printf("Hello World, but different\n");<RESET>
         }<RESET>
-        <RESET>
+        
        <BGREEN>+<RESET><BGREEN>int secure_foo(struct user *u)<RESET>
        <BGREEN>+<RESET><BGREEN>{<RESET>
        <BGREEN>+<RESET><BGREEN>foo(u);<RESET>
-- 
2.18.0.203.gfac676dfb9-goog

Reply via email to