diff: correct suppress_blank_empty hack

The suppress-blank-empty feature abused the CONTEXT_INCOMPLETE
symbol that was meant to be used only for "\ No newline at the end
of file" code path.

The intent of the feature was to turn a context line we receive from
xdiff machinery (which always uses ' ' for context lines, even an
empty one) and spit it out as a truly empty line.

Perform such a conversion very locally at where a line from xdiff
that begins with ' ' is handled for output; there are many checks
before the control reaches such place that checks the first letter
of the diff output line to see if it is a context line, and having
to check for '\n' and treat it as a special case is error prone.

In order to catch similar hacks in the future, make sure the code
path that is meant for "\ No newline" case checks the first byte is
indeed a backslash.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/diff.c b/diff.c
index 9a24a0791ceacad8719071d079b78ef5c142a071..b9ef8550cc859a8f0504f5f07fd43e592fa7307d 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -1321,6 +1321,11 @@ static void emit_line_ws_markup(struct diff_options *o,
        const char *ws = NULL;
        int sign = o->output_indicators[sign_index];
 
+       if (diff_suppress_blank_empty &&
+           sign_index == OUTPUT_INDICATOR_CONTEXT &&
+           len == 1 && line[0] == '\n')
+               sign = 0;
+
        if (o->ws_error_highlight & ws_rule) {
                ws = diff_get_color_opt(o, DIFF_WHITESPACE);
                if (!*ws)
@@ -1498,15 +1503,9 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,
        case DIFF_SYMBOL_WORDS:
                context = diff_get_color_opt(o, DIFF_CONTEXT);
                reset = diff_get_color_opt(o, DIFF_RESET);
-               /*
-                * Skip the prefix character, if any.  With
-                * diff_suppress_blank_empty, there may be
-                * none.
-                */
-               if (line[0] != '\n') {
-                       line++;
-                       len--;
-               }
+
+               /* Skip the prefix character */
+               line++; len--;
                emit_line(o, context, reset, line, len);
                break;
        case DIFF_SYMBOL_FILEPAIR_PLUS:
@@ -2375,12 +2374,6 @@ static int fn_out_consume(void *priv, char *line, unsigned long len)
                ecbdata->label_path[0] = ecbdata->label_path[1] = NULL;
        }
 
-       if (diff_suppress_blank_empty
-           && len == 2 && line[0] == ' ' && line[1] == '\n') {
-               line[0] = '\n';
-               len = 1;
-       }
-
        if (line[0] == '@') {
                if (ecbdata->diff_words)
                        diff_words_flush(ecbdata);
@@ -2431,12 +2424,14 @@ static int fn_out_consume(void *priv, char *line, unsigned long len)
                ecbdata->lno_in_preimage++;
                emit_context_line(ecbdata, line + 1, len - 1);
                break;
-       default:
+       case '\\':
                /* incomplete line at the end */
                ecbdata->lno_in_preimage++;
                emit_diff_symbol(o, DIFF_SYMBOL_CONTEXT_INCOMPLETE,
                                 line, len, 0);
                break;
+       default:
+               BUG("fn_out_consume: unknown line '%s'", line);
        }
        return 0;
 }