diff --whitespace=warn/error: fix blank-at-eof check

The "diff --check" logic used to share the same issue as the one fixed for
"git apply" earlier in this series, in that a patch that adds new blank
lines at end could appear as

    @@ -l,5 +m,7 @@$
    _context$
    _context$
    -deleted$
    +$
    +$
    +$
    _$
    _$

where _ stands for SP and $ shows a end-of-line.  Instead of looking at
each line in the patch in the callback, simply count the blank lines from
the end in two versions, and notice the presence of new ones.

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

diff --git a/diff.c b/diff.c
index a693d184c95d3aaf8f2ab592bd806bbc64441a39..c19c4760fe90a6ead26bbd240b0b9f64e2d60397 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -1149,7 +1149,6 @@ struct checkdiff_t {
        struct diff_options *o;
        unsigned ws_rule;
        unsigned status;
-       int trailing_blanks_start;
 };
 
 static int is_conflict_marker(const char *line, unsigned long len)
@@ -1193,10 +1192,6 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
        if (line[0] == '+') {
                unsigned bad;
                data->lineno++;
-               if (!ws_blank_line(line + 1, len - 1, data->ws_rule))
-                       data->trailing_blanks_start = 0;
-               else if (!data->trailing_blanks_start)
-                       data->trailing_blanks_start = data->lineno;
                if (is_conflict_marker(line + 1, len - 1)) {
                        data->status |= 1;
                        fprintf(data->o->file,
@@ -1216,14 +1211,12 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
                              data->o->file, set, reset, ws);
        } else if (line[0] == ' ') {
                data->lineno++;
-               data->trailing_blanks_start = 0;
        } else if (line[0] == '@') {
                char *plus = strchr(line, '+');
                if (plus)
                        data->lineno = strtol(plus, NULL, 10) - 1;
                else
                        die("invalid diff");
-               data->trailing_blanks_start = 0;
        }
 }
 
@@ -1437,6 +1430,44 @@ static const struct funcname_pattern_entry *diff_funcname_pattern(struct diff_fi
        return NULL;
 }
 
+static int count_trailing_blank(mmfile_t *mf, unsigned ws_rule)
+{
+       char *ptr = mf->ptr;
+       long size = mf->size;
+       int cnt = 0;
+
+       if (!size)
+               return cnt;
+       ptr += size - 1; /* pointing at the very end */
+       if (*ptr != '\n')
+               ; /* incomplete line */
+       else
+               ptr--; /* skip the last LF */
+       while (mf->ptr < ptr) {
+               char *prev_eol;
+               for (prev_eol = ptr; mf->ptr <= prev_eol; prev_eol--)
+                       if (*prev_eol == '\n')
+                               break;
+               if (!ws_blank_line(prev_eol + 1, ptr - prev_eol, ws_rule))
+                       break;
+               cnt++;
+               ptr = prev_eol - 1;
+       }
+       return cnt;
+}
+
+static int adds_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2, unsigned ws_rule)
+{
+       int l1, l2, at;
+       l1 = count_trailing_blank(mf1, ws_rule);
+       l2 = count_trailing_blank(mf2, ws_rule);
+       if (l2 <= l1)
+               return 0;
+       /* starting where? */
+       at = count_lines(mf1->ptr, mf1->size);
+       return (at - l1) + 1; /* the line number counts from 1 */
+}
+
 static void builtin_diff(const char *name_a,
                         const char *name_b,
                         struct diff_filespec *one,
@@ -1650,15 +1681,16 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
                ecb.priv = &data;
                xdi_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
 
-               if ((data.ws_rule & WS_BLANK_AT_EOF) &&
-                   data.trailing_blanks_start) {
-                       static char *err;
-
-                       if (!err)
-                               err = whitespace_error_string(WS_BLANK_AT_EOF);
-                       fprintf(o->file, "%s:%d: %s\n",
-                               data.filename, data.trailing_blanks_start, err);
-                       data.status = 1; /* report errors */
+               if (data.ws_rule & WS_BLANK_AT_EOF) {
+                       int blank_at_eof = adds_blank_at_eof(&mf1, &mf2, data.ws_rule);
+                       if (blank_at_eof) {
+                               static char *err;
+                               if (!err)
+                                       err = whitespace_error_string(WS_BLANK_AT_EOF);
+                               fprintf(o->file, "%s:%d: %s.\n",
+                                       data.filename, blank_at_eof, err);
+                               data.status = 1; /* report errors */
+                       }
                }
        }
  free_and_return:

diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index a5d446157455de909d47abfd32a363091ed41e42..e0b481d42b1b5a21cef3a8fae77d7eedcb8aeb2a 100755 (executable)
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -341,6 +341,13 @@ test_expect_success 'checkdiff detects new trailing blank lines (1)' '
        git diff --check | grep "new blank line"
 '
 
+test_expect_success 'checkdiff detects new trailing blank lines (2)' '
+       { echo a; echo b; echo; echo; } >x &&
+       git add x &&
+       { echo a; echo; echo; echo; echo; } >x &&
+       git diff --check | grep "new blank line"
+'
+
 test_expect_success 'checkdiff allows new blank lines' '
        git checkout x &&
        mv x y &&