git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 8/9] diff --whitespace=warn/error: fix blank-at-eof check

From
Junio C Hamano <gitster@pobox.com>
Date
Sep 4, 2009, 10:55 UTC
Message-ID
<1252061718-11579-9-git-send-email-gitster@pobox.com>
In-Reply-To
<1252061718-11579-1-git-send-email-gitster@pobox.com>

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.c                     |   64 +++++++++++++++++++++++++++++++++-----------
 t/t4015-diff-whitespace.sh |    7 +++++
 2 files changed, 55 insertions(+), 16 deletions(-)
diff --git a/diff.c b/diff.c
index a693d18..c19c476 100644
--- 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 a5d4461..e0b481d 100755
--- 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 &&

-- 
1.6.4.2.313.g0425f
Previous: Junio C HamanoNext: Junio C Hamano
Message 11 of 14 in “War on blank-at-eof”
  1. 0/9 War on blank-at-eofJunio C Hamano, Sep 4, 2009
  2. 1/9 apply --whitespace=fix: fix handling of blank lines at the eofJunio C Hamano, Sep 4, 2009
  3. 2/9 apply --whitespace=fix: detect new blank lines at eof correctlyJunio C Hamano, Sep 4, 2009
  4. Johannes SixtSep 4, 2009
  5. Junio C HamanoSep 4, 2009
  6. 3/9 apply.c: split check_whitespace() into twoJunio C Hamano, Sep 4, 2009
  7. 4/9 apply --whitespace=warn/error: diagnose blank at EOFJunio C Hamano, Sep 4, 2009
  8. 5/9 apply --whitespace: warn blank but not necessarily empty lines at EOFJunio C Hamano, Sep 4, 2009
  9. 6/9 diff.c: the builtin_diff() deals with only two-file comparisonJunio C Hamano, Sep 4, 2009
  10. 7/9 diff --whitespace=warn/error: obey blank-at-eofJunio C Hamano, Sep 4, 2009
  11. 8/9 diff --whitespace=warn/error: fix blank-at-eof checkJunio C Hamano, Sep 4, 2009
  12. 9/9 diff --color: color blank-at-eofJunio C Hamano, Sep 4, 2009
  13. Thell FowlerSep 5, 2009
  14. Junio C HamanoSep 6, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.