Re: diff --check is stupid about blank lines

Junio C Hamano [off-list ref] writes:

Jeff King [off-list ref] writes:

quoted

... The problem seems to be the conditional at
diff.c:1622:

        if ((data.ws_rule & WS_TRAILING_SPACE) &&
            data.trailing_blanks_start) {
                fprintf(o->file, "%s:%d: ends with blank lines.\n",
                        data.filename, data.trailing_blanks_start);
                data.status = 1; /* report errors */
        }

that should probably be "if we care about trailing space, and the last
thing we saw was a trailing blank, _and_ the last hunk adds to
end-of-file, then...".

Instead, data.trailing_blanks_start is supposed to be reset to 0 every
time we see non-blank newline, a copied context line, or new hunk.

So if this triggers with -U0 I'd understand, but otherwise I do not see
how.

Ahhh, what idiot wrote the logic for checking trailing blank lines in
checkdiff_consume().

It does not ask for any context lines.  Sheesh.

This should fix it.

diff --git i/diff.c w/diff.c
index 10d5440..5923fe2 100644
--- i/diff.c
+++ w/diff.c

@@ -1628,6 +1628,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
 		xdemitcb_t ecb;
 
 		memset(&xecfg, 0, sizeof(xecfg));
+		xecfg.ctxlen = 1; /* at least one context line */
 		xpp.flags = XDF_NEED_MINIMAL;
 		xdi_diff_outf(&mf1, &mf2, checkdiff_consume, &data,
 			      &xpp, &xecfg, &ecb);

On 2008.08.20 11:42:34 -0700, Junio C Hamano wrote:

Junio C Hamano [off-list ref] writes:

quoted

Instead, data.trailing_blanks_start is supposed to be reset to 0 every
time we see non-blank newline, a copied context line, or new hunk.

So if this triggers with -U0 I'd understand, but otherwise I do not see
how.

Ahhh, what idiot wrote the logic for checking trailing blank lines in
checkdiff_consume().

It does not ask for any context lines.  Sheesh.

This should fix it.

Thanks, works for me :-)

Björn

On Wed, Aug 20, 2008 at 11:42:34AM -0700, Junio C Hamano wrote:

Ahhh, what idiot wrote the logic for checking trailing blank lines in
checkdiff_consume().

It does not ask for any context lines.  Sheesh.

This should fix it.

Ah, that was much easier than I expected. ;)

Maybe this should be squashed in?

---

diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 6110566..974efd0 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh

@@ -341,4 +341,13 @@ test_expect_success 'checkdiff detects trailing blank lines' '
 	git diff --check | grep "ends with blank"
 '
 
+test_expect_success 'checkdiff does not mistake trailing blank lines in hunk' '
+	echo content >x &&
+	git add x &&
+	echo "" >x &&
+	echo content >>x &&
+	git diff --check >out &&
+	! grep "ends with blank" out
+'
+
 test_done