threads / patch / 20849

patch, 9 partsWar on blank-at-eof

Subject: [PATCH 0/9] War on blank-at-eof

## tl;dr

14 messages between Sep 4, 2009 and Sep 6, 2009. Diffs are folded; open one to read it.

replies: 13people: 3as markdown or json

Junio C Hamano· Sep 4, 2009, 10:55 UTC · lore

We had quite inconsistent handing of patches that add new blank lines at the end of file, and this miniseries is about fixing it.

Patch 1 is a fix to an ancient bug introduced by v1.5.5-rc0~156^2~11.

Patch 2 fixes a bug that is even older---I suspect it dates back to the very first change that introduced the feature, but I did not bother to dig.

Patch 4 (Patch 3 is a preliminary refactoring used by it) is about the discrepancy between "--whitespace=fix" and "--whitespace=warn". The blank-at-eof error was silently fixed but never diagnosed, which has been one of the long-standing itch of mine to fix.

Patch 5 corrects the definition of blank-at-eof. If a patch adds an non-empty line that consists solely of whitespaces at the end of file, we should diagnose and strip it just line a new empty line. After all, both are blank lines.

Patch 6 is a simple code reduction I noticed while preparing this series; it can be a standalone patch, but it is obvious enough to be here.

Patches 7 and 8 address "git diff --check", which had roughly the same logic as the --whitespace=fix. It shared the same problems the earlier parts of the series fixed for "git apply".

Patch 9 is about "diff --color" to paint blank-at-eof as error, which we did not do so far because it was too cumbersome. This has been another one of the long-standing itch of mine to fix.

The series applies to v1.6.0.6-87-g82d97da; merging the result to 'master' needs some conflict resolution.

 1 apply --whitespace=fix: fix handling of blank lines at the eof
 2 apply --whitespace=fix: detect new blank lines at eof correctly
 3 apply.c: split check_whitespace() into two
 4 apply --whitespace=warn/error: diagnose blank at EOF
 5 apply --whitespace: warn blank but not necessarily empty lines at EOF
 6 diff.c: the builtin_diff() deals with only two-file comparison
 7 diff --whitespace=warn/error: obey blank-at-eof
 8 diff --whitespace=warn/error: fix blank-at-eof check
 9 diff --color: color blank-at-eof
 Documentation/config.txt   |    2 +
 builtin-apply.c            |   61 +++++++++++++++-------
 cache.h                    |    3 +-
 diff.c                     |  119 +++++++++++++++++++++++++++++---------------
 t/t4015-diff-whitespace.sh |   11 +++-
 t/t4019-diff-wserror.sh    |   11 ++++-
 t/t4124-apply-ws-rule.sh   |   80 +++++++++++++++++++++++++++++
 ws.c                       |    6 ++
 8 files changed, 230 insertions(+), 63 deletions(-)
Junio C Hamano· Sep 4, 2009, 10:55 UTC · re: Junio C Hamano · lore

[PATCH 1/9] apply --whitespace=fix: fix handling of blank lines at the eof

b94f2ed (builtin-apply.c: make it more line oriented, 2008-01-26) broke the logic used to detect if a hunk adds blank lines at the end of the file. With the new code after that commit:

 - img holds the contents of the file that the hunk is being applied to;
 - preimage has the lines the hunk expects to be in img; and
 - postimage has the lines the hunk wants to update the part in img that
   corresponds to preimage with.

and we need to compare if the last line of preimage (not postimage) matches the last line of img to see if the hunk applies at the end of the file.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-apply.c          |    2 +-
 t/t4124-apply-ws-rule.sh |   29 +++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletions(-)
Show changes to 2 files +30 −1

builtin-apply.c, t/t4124-apply-ws-rule.sh

diff --git a/builtin-apply.c b/builtin-apply.c
index 7a1ff04..5b5bde4 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2069,7 +2069,7 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,
 	if (applied_pos >= 0) {
 		if (ws_error_action == correct_ws_error &&
 		    new_blank_lines_at_end &&
-		    postimage.nr + applied_pos == img->nr) {
+		    preimage.nr + applied_pos == img->nr) {
 			/*
 			 * If the patch application adds blank lines
 			 * at the end, and if the patch applies at the
diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh
index f83322e..6898722 100755
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@ -148,4 +148,33 @@ do
 	done
 done
 
+
+test_expect_success 'blank at EOF with --whitespace=fix (1)' '
+	: these can fail depending on what we did before
+	git config --unset core.whitespace
+	rm -f .gitattributes
+
+	{ echo a; echo b; echo c; } >one &&
+	git add one &&
+	{ echo a; echo b; echo c; } >expect &&
+	{ cat expect; echo; } >one &&
+	git diff -- one >patch &&
+
+	git checkout one &&
+	git apply --whitespace=fix patch &&
+	test_cmp expect one
+'
+
+test_expect_success 'blank at EOF with --whitespace=fix (2)' '
+	{ echo a; echo b; echo c; } >one &&
+	git add one &&
+	{ echo a; echo c; } >expect &&
+	{ cat expect; echo; echo; } >one &&
+	git diff -- one >patch &&
+
+	git checkout one &&
+	git apply --whitespace=fix patch &&
+	test_cmp expect one
+'
+
 test_done

-- 
1.6.4.2.313.g0425f
Junio C Hamano· Sep 4, 2009, 10:55 UTC · re: Junio C Hamano · lore

[PATCH 2/9] apply --whitespace=fix: detect new blank lines at eof correctly

The command tries to strip blank lines at the end of the file added by a patch. However, if the original ends with blank lines, often the patch hunk ends like this:

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

where _ stands for SP and $ shows a end-of-line. This example patch adds three trailing blank lines, but the code fails to notice it, because it only pays attention to added blank lines at the very end of the hunk. In this example, the three added blank lines do not appear textually at the end in the patch, even though you can see that they are indeed added at the end, if you rearrange the diff like this:

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

Fix this by not resetting the number of (candidate) added blank lines at the end when the loop sees a context line that is empty.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-apply.c          |    6 ++++++
 t/t4124-apply-ws-rule.sh |   12 ++++++++++++
 2 files changed, 18 insertions(+), 0 deletions(-)
Show changes to 2 files +18 −0

builtin-apply.c, t/t4124-apply-ws-rule.sh

diff --git a/builtin-apply.c b/builtin-apply.c
index 5b5bde4..c5e4048 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1913,6 +1913,7 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,
 		int len = linelen(patch, size);
 		int plen, added;
 		int added_blank_line = 0;
+		int is_blank_context = 0;
 
 		if (!len)
 			break;
@@ -1945,8 +1946,11 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,
 			*new++ = '\n';
 			add_line_info(&preimage, "\n", 1, LINE_COMMON);
 			add_line_info(&postimage, "\n", 1, LINE_COMMON);
+			is_blank_context = 1;
 			break;
 		case ' ':
+			if (plen && patch[1] == '\n')
+				is_blank_context = 1;
 		case '-':
 			memcpy(old, patch + 1, plen);
 			add_line_info(&preimage, old, plen,
@@ -1986,6 +1990,8 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,
 		}
 		if (added_blank_line)
 			new_blank_lines_at_end++;
+		else if (is_blank_context)
+			;
 		else
 			new_blank_lines_at_end = 0;
 		patch += len;
diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh
index 6898722..ba2b7f9 100755
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@ -177,4 +177,16 @@ test_expect_success 'blank at EOF with --whitespace=fix (2)' '
 	test_cmp expect one
 '
 
+test_expect_success 'blank at EOF with --whitespace=fix (3)' '
+	{ echo a; echo b; echo; } >one &&
+	git add one &&
+	{ echo a; echo c; echo; } >expect &&
+	{ cat expect; echo; echo; } >one &&
+	git diff -- one >patch &&
+
+	git checkout one &&
+	git apply --whitespace=fix patch &&
+	test_cmp expect one
+'
+
 test_done

-- 
1.6.4.2.313.g0425f
Johannes Sixt· Sep 4, 2009, 12:02 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/9] apply --whitespace=fix: detect new blank lines at eof correctly

Junio C Hamano schrieb:
Show 33 quoted lines
> The command tries to strip blank lines at the end of the file added by a
> patch.  However, if the original ends with blank lines, often the patch
> hunk ends like this:
> 
>     @@ -l,5 +m,7 @@$
>     _context$
>     _context$
>     -deleted$
>     +$
>     +$
>     +$
>     _$
>     _$
> 
> where _ stands for SP and $ shows a end-of-line.  This example patch adds
> three trailing blank lines, but the code fails to notice it, because it
> only pays attention to added blank lines at the very end of the hunk.  In
> this example, the three added blank lines do not appear textually at the
> end in the patch, even though you can see that they are indeed added at
> the end, if you rearrange the diff like this:
> 
>     @@ -l,5 +m,7 @@$
>     _context$
>     _context$
>     -deleted$
>     _$
>     _$
>     +$
>     +$
>     +$
> 
> Fix this by not resetting the number of (candidate) added blank lines at
> the end when the loop sees a context line that is empty.

After reading this explanation, I was worried that added blank lines that are at the end of a patch but apply in the middle of a file would be mis-attributed as blank lines at EOF. But appearently, they are not, i.e. such added blank lines are not removed. Could you squash in this test case that checks for this condition.

-- Hannes
Show changes to t/t4124-apply-ws-rule.sh +12 −0
diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh
index ba2b7f9..fedc8b9 100755
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@ -189,4 +189,16 @@ test_expect_success 'blank at EOF with --whitespace=fix (3)' '
 	test_cmp expect one
 '

+test_expect_success 'blank at end of hunk, not at EOF with --whitespace=fix' '
+	{ echo a; echo b; echo; echo; echo; echo; echo; echo d; } >one &&
+	git add one &&
+	{ echo a; echo c; echo; echo; echo; echo; echo; echo; echo d; } >expect &&
+	cp expect one &&
+	git diff -- one >patch &&
+
+	git checkout one &&
+	git apply --whitespace=fix patch &&
+	test_cmp expect one
+'
+
 test_done
Junio C Hamano· Sep 4, 2009, 16:26 UTC · re: Johannes Sixt · lore

Re: [PATCH 2/9] apply --whitespace=fix: detect new blank lines at eof correctly

Johannes Sixt <j.sixt@viscovery.net> writes:
> After reading this explanation, I was worried that added blank lines that
> are at the end of a patch but apply in the middle of a file would be
> mis-attributed as blank lines at EOF.

The codepath this patch is about checks "does the hunk result in more blank at the end of the place it applies to?". There is a separate logic that checks "does the hunk applies at the end of the file", which is the topic of the codepath that is fixed by Patch #1.

Junio C Hamano· Sep 4, 2009, 10:55 UTC · re: Junio C Hamano · lore

[PATCH 3/9] apply.c: split check_whitespace() into two

This splits the logic to record the presence of whitespace errors out of the check_whitespace() function, which checks and then records. The new function, record_ws_error(), can be used by the blank-at-eof check that does not use ws_check() logic to report its findings in the same output format.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-apply.c |   24 +++++++++++++++---------
 1 files changed, 15 insertions(+), 9 deletions(-)
Show changes to builtin-apply.c +15 −9
diff --git a/builtin-apply.c b/builtin-apply.c
index c5e4048..80ddf55 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1055,23 +1055,29 @@ static int find_header(char *line, unsigned long size, int *hdrsize, struct patc
 	return -1;
 }
 
-static void check_whitespace(const char *line, int len, unsigned ws_rule)
+static void record_ws_error(unsigned result, const char *line, int len, int linenr)
 {
 	char *err;
-	unsigned result = ws_check(line + 1, len - 1, ws_rule);
+
 	if (!result)
 		return;
 
 	whitespace_error++;
 	if (squelch_whitespace_errors &&
 	    squelch_whitespace_errors < whitespace_error)
-		;
-	else {
-		err = whitespace_error_string(result);
-		fprintf(stderr, "%s:%d: %s.\n%.*s\n",
-			patch_input_file, linenr, err, len - 2, line + 1);
-		free(err);
-	}
+		return;
+
+	err = whitespace_error_string(result);
+	fprintf(stderr, "%s:%d: %s.\n%.*s\n",
+		patch_input_file, linenr, err, len, line);
+	free(err);
+}
+
+static void check_whitespace(const char *line, int len, unsigned ws_rule)
+{
+	unsigned result = ws_check(line + 1, len - 1, ws_rule);
+
+	record_ws_error(result, line + 1, len - 2, linenr);
 }
 
 /*

-- 
1.6.4.2.313.g0425f
Junio C Hamano· Sep 4, 2009, 10:55 UTC · re: Junio C Hamano · lore

[PATCH 4/9] apply --whitespace=warn/error: diagnose blank at EOF

"git apply" strips new blank lines at EOF under --whitespace=fix option, but neigher --whitespace=warn nor --whitespace=error paid any attention to these errors.

Introduce a new whitespace error class, blank-at-eof, to make the whitespace error handling more consistent.

The patch adds a new "linenr" field to the struct fragment in order to record which line the hunk started in the input file, but this is needed solely for reporting purposes. The detection of this class of whitespace errors cannot be done while parsing a patch like we do for all the other classes of whitespace errors. It instead has to wait until we find where to apply the hunk, but at that point, we do not have an access to the original line number in the input file anymore, hence the new field.

Depending on your point of view, this may be a bugfix that makes warn and error in line with fix. Or you could call it a new feature. The line between them is somewhat fuzzy in this case.

Strictly speaking, triggering more errors than before is a change in behaviour that is not backward compatible, even though the reason for the change is because the code was not checking for an error that it should have. People who do not want added blank lines at EOF to trigger an error can disable the new error class.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt |    2 ++
 builtin-apply.c          |   27 ++++++++++++++++++---------
 cache.h                  |    3 ++-
 t/t4124-apply-ws-rule.sh |   26 ++++++++++++++++++++++++++
 ws.c                     |    6 ++++++
 5 files changed, 54 insertions(+), 10 deletions(-)
Show changes to 5 files +54 −10

Documentation/config.txt, builtin-apply.c, cache.h, t/t4124-apply-ws-rule.sh, ws.c

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 113d9d1..871384e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -389,6 +389,8 @@ core.whitespace::
   error (enabled by default).
 * `indent-with-non-tab` treats a line that is indented with 8 or more
   space characters as an error (not enabled by default).
+* `blank-at-eof` treats blank lines added at the end of file as an error
+  (enabled by default).
 * `cr-at-eol` treats a carriage-return at the end of line as
   part of the line terminator, i.e. with it, `trailing-space`
   does not trigger if the character before such a carriage-return
diff --git a/builtin-apply.c b/builtin-apply.c
index 80ddf55..37d3bc0 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -126,6 +126,7 @@ struct fragment {
 	const char *patch;
 	int size;
 	int rejected;
+	int linenr;
 	struct fragment *next;
 };
 
@@ -1193,6 +1194,7 @@ static int parse_single_patch(char *line, unsigned long size, struct patch *patc
 		int len;
 
 		fragment = xcalloc(1, sizeof(*fragment));
+		fragment->linenr = linenr;
 		len = parse_fragment(line, size, patch, fragment);
 		if (len <= 0)
 			die("corrupt patch at line %d", linenr);
@@ -2079,17 +2081,24 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,
 	}
 
 	if (applied_pos >= 0) {
-		if (ws_error_action == correct_ws_error &&
-		    new_blank_lines_at_end &&
-		    preimage.nr + applied_pos == img->nr) {
+		if (new_blank_lines_at_end &&
+		    preimage.nr + applied_pos == img->nr &&
+		    (ws_rule & WS_BLANK_AT_EOF) &&
+		    ws_error_action != nowarn_ws_error) {
+			record_ws_error(WS_BLANK_AT_EOF, "+", 1, frag->linenr);
+			if (ws_error_action == correct_ws_error) {
+				while (new_blank_lines_at_end--)
+					remove_last_line(&postimage);
+			}
 			/*
-			 * If the patch application adds blank lines
-			 * at the end, and if the patch applies at the
-			 * end of the image, remove those added blank
-			 * lines.
+			 * We would want to prevent write_out_results()
+			 * from taking place in apply_patch() that follows
+			 * the callchain led us here, which is:
+			 * apply_patch->check_patch_list->check_patch->
+			 * apply_data->apply_fragments->apply_one_fragment
 			 */
-			while (new_blank_lines_at_end--)
-				remove_last_line(&postimage);
+			if (ws_error_action == die_on_ws_error)
+				apply = 0;
 		}
 
 		/*
diff --git a/cache.h b/cache.h
index 099a32e..7152fea 100644
--- a/cache.h
+++ b/cache.h
@@ -845,7 +845,8 @@ void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, i
 #define WS_SPACE_BEFORE_TAB	02
 #define WS_INDENT_WITH_NON_TAB	04
 #define WS_CR_AT_EOL           010
-#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB)
+#define WS_BLANK_AT_EOF        020
+#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB|WS_BLANK_AT_EOF)
 extern unsigned whitespace_rule_cfg;
 extern unsigned whitespace_rule(const char *);
 extern unsigned parse_whitespace_rule(const char *);
diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh
index ba2b7f9..89b71e1 100755
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@ -189,4 +189,30 @@ test_expect_success 'blank at EOF with --whitespace=fix (3)' '
 	test_cmp expect one
 '
 
+test_expect_success 'blank at EOF with --whitespace=warn' '
+	{ echo a; echo b; echo c; } >one &&
+	git add one &&
+	echo >>one &&
+	cat one >expect &&
+	git diff -- one >patch &&
+
+	git checkout one &&
+	git apply --whitespace=warn patch 2>error &&
+	test_cmp expect one &&
+	grep "new blank line at EOF" error
+'
+
+test_expect_success 'blank at EOF with --whitespace=error' '
+	{ echo a; echo b; echo c; } >one &&
+	git add one &&
+	cat one >expect &&
+	echo >>one &&
+	git diff -- one >patch &&
+
+	git checkout one &&
+	test_must_fail git apply --whitespace=error patch 2>error &&
+	test_cmp expect one &&
+	grep "new blank line at EOF" error
+'
+
 test_done
diff --git a/ws.c b/ws.c
index 7a7ff13..d56636b 100644
--- a/ws.c
+++ b/ws.c
@@ -15,6 +15,7 @@ static struct whitespace_rule {
 	{ "space-before-tab", WS_SPACE_BEFORE_TAB },
 	{ "indent-with-non-tab", WS_INDENT_WITH_NON_TAB },
 	{ "cr-at-eol", WS_CR_AT_EOL },
+	{ "blank-at-eof", WS_BLANK_AT_EOF },
 };
 
 unsigned parse_whitespace_rule(const char *string)
@@ -113,6 +114,11 @@ char *whitespace_error_string(unsigned ws)
 			strbuf_addstr(&err, ", ");
 		strbuf_addstr(&err, "indent with spaces");
 	}
+	if (ws & WS_BLANK_AT_EOF) {
+		if (err.len)
+			strbuf_addstr(&err, ", ");
+		strbuf_addstr(&err, "new blank line at EOF");
+	}
 	return strbuf_detach(&err, NULL);
 }
 

-- 
1.6.4.2.313.g0425f
Junio C Hamano· Sep 4, 2009, 10:55 UTC · re: Junio C Hamano · lore

[PATCH 5/9] apply --whitespace: warn blank but not necessarily empty lines at EOF

The whitespace error of adding blank lines at the end of file should trigger if you added a non-empty line at the end, if the contents of the line is full of whitespaces.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-apply.c          |    6 ++++--
 t/t4124-apply-ws-rule.sh |   13 +++++++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)
Show changes to 2 files +17 −2

builtin-apply.c, t/t4124-apply-ws-rule.sh

diff --git a/builtin-apply.c b/builtin-apply.c
index 37d3bc0..6662cc4 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1957,7 +1957,8 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,
 			is_blank_context = 1;
 			break;
 		case ' ':
-			if (plen && patch[1] == '\n')
+			if (plen && (ws_rule & WS_BLANK_AT_EOF) &&
+			    ws_blank_line(patch + 1, plen, ws_rule))
 				is_blank_context = 1;
 		case '-':
 			memcpy(old, patch + 1, plen);
@@ -1985,7 +1986,8 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,
 				      (first == '+' ? 0 : LINE_COMMON));
 			new += added;
 			if (first == '+' &&
-			    added == 1 && new[-1] == '\n')
+			    (ws_rule & WS_BLANK_AT_EOF) &&
+			    ws_blank_line(patch + 1, plen, ws_rule))
 				added_blank_line = 1;
 			break;
 		case '@': case '\\':
diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh
index 89b71e1..b3c3b2c 100755
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@ -215,4 +215,17 @@ test_expect_success 'blank at EOF with --whitespace=error' '
 	grep "new blank line at EOF" error
 '
 
+test_expect_success 'blank but not empty at EOF' '
+	{ echo a; echo b; echo c; } >one &&
+	git add one &&
+	echo "   " >>one &&
+	cat one >expect &&
+	git diff -- one >patch &&
+
+	git checkout one &&
+	git apply --whitespace=warn patch 2>error &&
+	test_cmp expect one &&
+	grep "new blank line at EOF" error
+'
+
 test_done

-- 
1.6.4.2.313.g0425f
Junio C Hamano· Sep 4, 2009, 10:55 UTC · re: Junio C Hamano · lore

[PATCH 6/9] diff.c: the builtin_diff() deals with only two-file comparison

The combined diff is implemented in combine_diff() and fn_out_consume() codepath never has to deal with anything but two-file comparison.

Drop nparents from the emit_callback structure and simplify the code.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.c |   32 +++++++++-----------------------
 1 files changed, 9 insertions(+), 23 deletions(-)
Show changes to diff.c +9 −23
diff --git a/diff.c b/diff.c
index 6fea3c0..1eddd59 100644
--- a/diff.c
+++ b/diff.c
@@ -489,7 +489,7 @@ typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len);
 
 struct emit_callback {
 	struct xdiff_emit_state xm;
-	int nparents, color_diff;
+	int color_diff;
 	unsigned ws_rule;
 	sane_truncate_fn truncate;
 	const char **label_path;
@@ -549,9 +549,8 @@ static void emit_add_line(const char *reset, struct emit_callback *ecbdata, cons
 		emit_line(ecbdata->file, set, reset, line, len);
 	else {
 		/* Emit just the prefix, then the rest. */
-		emit_line(ecbdata->file, set, reset, line, ecbdata->nparents);
-		ws_check_emit(line + ecbdata->nparents,
-			      len - ecbdata->nparents, ecbdata->ws_rule,
+		emit_line(ecbdata->file, set, reset, line, 1);
+		ws_check_emit(line + 1, len - 1, ecbdata->ws_rule,
 			      ecbdata->file, set, reset, ws);
 	}
 }
@@ -576,7 +575,6 @@ static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, u
 
 static void fn_out_consume(void *priv, char *line, unsigned long len)
 {
-	int i;
 	int color;
 	struct emit_callback *ecbdata = priv;
 	const char *meta = diff_get_color(ecbdata->color_diff, DIFF_METAINFO);
@@ -598,13 +596,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 		ecbdata->label_path[0] = ecbdata->label_path[1] = NULL;
 	}
 
-	/* This is not really necessary for now because
-	 * this codepath only deals with two-way diffs.
-	 */
-	for (i = 0; i < len && line[i] == '@'; i++)
-		;
-	if (2 <= i && i < len && line[i] == ' ') {
-		ecbdata->nparents = i - 1;
+	if (line[0] == '@') {
 		len = sane_truncate_line(ecbdata, line, len);
 		emit_line(ecbdata->file,
 			  diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO),
@@ -614,15 +606,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 		return;
 	}
 
-	if (len < ecbdata->nparents) {
+	if (len < 1) {
 		emit_line(ecbdata->file, reset, reset, line, len);
 		return;
 	}
 
 	color = DIFF_PLAIN;
-	if (ecbdata->diff_words && ecbdata->nparents != 1)
-		/* fall back to normal diff */
-		free_diff_words_data(ecbdata);
 	if (ecbdata->diff_words) {
 		if (line[0] == '-') {
 			diff_words_append(line, len,
@@ -641,13 +630,10 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 		emit_line(ecbdata->file, plain, reset, line, len);
 		return;
 	}
-	for (i = 0; i < ecbdata->nparents && len; i++) {
-		if (line[i] == '-')
-			color = DIFF_FILE_OLD;
-		else if (line[i] == '+')
-			color = DIFF_FILE_NEW;
-	}
-
+	if (line[0] == '-')
+		color = DIFF_FILE_OLD;
+	else if (line[0] == '+')
+		color = DIFF_FILE_NEW;
 	if (color != DIFF_FILE_NEW) {
 		emit_line(ecbdata->file,
 			  diff_get_color(ecbdata->color_diff, color),

-- 
1.6.4.2.313.g0425f
Junio C Hamano· Sep 4, 2009, 10:55 UTC · re: Junio C Hamano · lore

[PATCH 7/9] diff --whitespace=warn/error: obey blank-at-eof

The "diff --check" code used to conflate trailing-space whitespace error class with this, but now we have a proper separate error class, we should check it under blank-at-eof, not trailing-space.

The whitespace error is not about _having_ blank lines at end, but about adding _new_ blank lines. To keep the message consistent with what is given by "git apply", call whitespace_error_string() to generate it, instead of using a hardcoded custom message.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.c                     |   10 +++++++---
 t/t4015-diff-whitespace.sh |    4 ++--
 t/t4019-diff-wserror.sh    |    2 +-
 3 files changed, 10 insertions(+), 6 deletions(-)
Show changes to 3 files +10 −6

diff.c, t/t4015-diff-whitespace.sh, t/t4019-diff-wserror.sh

diff --git a/diff.c b/diff.c
index 1eddd59..a693d18 100644
--- a/diff.c
+++ b/diff.c
@@ -1650,10 +1650,14 @@ 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_TRAILING_SPACE) &&
+		if ((data.ws_rule & WS_BLANK_AT_EOF) &&
 		    data.trailing_blanks_start) {
-			fprintf(o->file, "%s:%d: ends with blank lines.\n",
-				data.filename, 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 */
 		}
 	}
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index b1cbd36..a5d4461 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -335,10 +335,10 @@ test_expect_success 'line numbers in --check output are correct' '
 
 '
 
-test_expect_success 'checkdiff detects trailing blank lines' '
+test_expect_success 'checkdiff detects new trailing blank lines (1)' '
 	echo "foo();" >x &&
 	echo "" >>x &&
-	git diff --check | grep "ends with blank"
+	git diff --check | grep "new blank line"
 '
 
 test_expect_success 'checkdiff allows new blank lines' '
diff --git a/t/t4019-diff-wserror.sh b/t/t4019-diff-wserror.sh
index 84a1fe3..1517fff 100755
--- a/t/t4019-diff-wserror.sh
+++ b/t/t4019-diff-wserror.sh
@@ -165,7 +165,7 @@ test_expect_success 'trailing empty lines (1)' '
 
 	rm -f .gitattributes &&
 	test_must_fail git diff --check >output &&
-	grep "ends with blank lines." output &&
+	grep "new blank line at" output &&
 	grep "trailing whitespace" output
 
 '

-- 
1.6.4.2.313.g0425f
Junio C Hamano· Sep 4, 2009, 10:55 UTC · re: Junio C Hamano · lore

[PATCH 8/9] 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.c                     |   64 +++++++++++++++++++++++++++++++++-----------
 t/t4015-diff-whitespace.sh |    7 +++++
 2 files changed, 55 insertions(+), 16 deletions(-)
Show changes to 2 files +55 −16

diff.c, t/t4015-diff-whitespace.sh

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
Junio C Hamano· Sep 4, 2009, 10:55 UTC · re: Junio C Hamano · lore

[PATCH 9/9] diff --color: color blank-at-eof

Since the coloring logic processed the patch output one line at a time, we couldn't easily color code the new blank lines at the end of file.

Reuse the adds_blank_at_eof() function to find where the runs of such blank lines start, keep track of the line number in the preimage while processing the patch output one line at a time, and paint the new blank lines that appear after that line to implement this.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 diff.c                  |   37 +++++++++++++++++++++++++++----------
 t/t4019-diff-wserror.sh |    9 +++++++++
 2 files changed, 36 insertions(+), 10 deletions(-)
Show changes to 2 files +36 −10

diff.c, t/t4019-diff-wserror.sh

diff --git a/diff.c b/diff.c
index c19c476..2b285b8 100644
--- a/diff.c
+++ b/diff.c
@@ -491,6 +491,8 @@ struct emit_callback {
 	struct xdiff_emit_state xm;
 	int color_diff;
 	unsigned ws_rule;
+	int blank_at_eof;
+	int lno_in_preimage;
 	sane_truncate_fn truncate;
 	const char **label_path;
 	struct diff_words_data *diff_words;
@@ -547,6 +549,12 @@ static void emit_add_line(const char *reset, struct emit_callback *ecbdata, cons
 
 	if (!*ws)
 		emit_line(ecbdata->file, set, reset, line, len);
+	else if ((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&
+		 ecbdata->blank_at_eof &&
+		 (ecbdata->blank_at_eof <= ecbdata->lno_in_preimage) &&
+		 ws_blank_line(line + 1, len - 1, ecbdata->ws_rule))
+		/* Blank line at EOF */
+		emit_line(ecbdata->file, ws, reset, line, len);
 	else {
 		/* Emit just the prefix, then the rest. */
 		emit_line(ecbdata->file, set, reset, line, 1);
@@ -573,9 +581,16 @@ static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, u
 	return allot - l;
 }
 
+static int find_preimage_lno(const char *line)
+{
+	char *p = strchr(line, '-');
+	if (!p)
+		return 0; /* should not happen */
+	return strtol(p+1, NULL, 10);
+}
+
 static void fn_out_consume(void *priv, char *line, unsigned long len)
 {
-	int color;
 	struct emit_callback *ecbdata = priv;
 	const char *meta = diff_get_color(ecbdata->color_diff, DIFF_METAINFO);
 	const char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);
@@ -598,6 +613,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 
 	if (line[0] == '@') {
 		len = sane_truncate_line(ecbdata, line, len);
+		ecbdata->lno_in_preimage = find_preimage_lno(line);
 		emit_line(ecbdata->file,
 			  diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO),
 			  reset, line, len);
@@ -611,7 +627,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 		return;
 	}
 
-	color = DIFF_PLAIN;
 	if (ecbdata->diff_words) {
 		if (line[0] == '-') {
 			diff_words_append(line, len,
@@ -630,14 +645,13 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 		emit_line(ecbdata->file, plain, reset, line, len);
 		return;
 	}
-	if (line[0] == '-')
-		color = DIFF_FILE_OLD;
-	else if (line[0] == '+')
-		color = DIFF_FILE_NEW;
-	if (color != DIFF_FILE_NEW) {
-		emit_line(ecbdata->file,
-			  diff_get_color(ecbdata->color_diff, color),
-			  reset, line, len);
+
+	if (line[0] != '+') {
+		const char *color =
+			diff_get_color(ecbdata->color_diff,
+				       line[0] == '-' ? DIFF_FILE_OLD : DIFF_PLAIN);
+		ecbdata->lno_in_preimage++;
+		emit_line(ecbdata->file, color, reset, line, len);
 		return;
 	}
 	emit_add_line(reset, ecbdata, line, len);
@@ -1557,6 +1571,9 @@ static void builtin_diff(const char *name_a,
 		ecbdata.color_diff = DIFF_OPT_TST(o, COLOR_DIFF);
 		ecbdata.found_changesp = &o->found_changes;
 		ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
+		if (ecbdata.ws_rule & WS_BLANK_AT_EOF)
+			ecbdata.blank_at_eof =
+				adds_blank_at_eof(&mf1, &mf2, ecbdata.ws_rule);
 		ecbdata.file = o->file;
 		xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;
 		xecfg.ctxlen = o->context;
diff --git a/t/t4019-diff-wserror.sh b/t/t4019-diff-wserror.sh
index 1517fff..1e75f1a 100755
--- a/t/t4019-diff-wserror.sh
+++ b/t/t4019-diff-wserror.sh
@@ -190,4 +190,13 @@ test_expect_success 'do not color trailing cr in context' '
 
 '
 
+test_expect_success 'color new trailing blank lines' '
+	{ echo a; echo b; echo; echo; } >x &&
+	git add x &&
+	{ echo a; echo; echo; echo; echo; } >x &&
+	git diff --color x >output &&
+	cnt=$(grep "${blue_grep}" output | wc -l) &&
+	test $cnt = 2
+'
+
 test_done

-- 
1.6.4.2.313.g0425f
Thell Fowler· Sep 5, 2009, 21:28 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/9] War on blank-at-eof

On Fri, 4 Sep 2009, Junio C Hamano wrote:
Show 5 quoted lines
> Patch 5 corrects the definition of blank-at-eof.  If a patch adds an
> non-empty line that consists solely of whitespaces at the end of file, we
> should diagnose and strip it just line a new empty line.  After all, both
> are blank lines.
>
Thank you. Thank you, thank you. Thank you!  And did I mention thank you?

Tested this out after cherry-picking: 3b5ef0e xutils: Fix xdl_recmatch() on incomplete lines 78ed710 xutils: Fix hashing an incomplete line with whitespaces at the end

It worked as nicely!  I'm throwing away the --allow-whitelines-at-eof 
patch! :D  Converting a _real_ dirty whitespace branch into an 'almost' 
whitespace policy compliant branch with validation of the diffs was 
able to be done like so:
	git diff -b DIRTY CLEAN
	git diff DIRTY^ CLEAN > diff1
	git diff CLEAN^ DIRTY > diff2
	git diff -b diff1 diff2

I mention 'almost' above because unfortunately this type of conversion leaves extra line-spaces at the end of some files that you might not want to have in a whitespace policy.

While thinking about what appeared in:

http://article.gmane.org/gmane.comp.version-control.git/124138 Junio C Hamano <gitster <at> pobox.com> writes:

Show 6 quoted lines
>Bruno Haible <bruno <at> clisp.org> writes:
>> In some GNU projects, there are file types for which trailing spaces in a line
>> ...
>> Currently the user has to turn off the 'trailing-space' whitespace attribute
>> in order for 'git diff --check' to not complain about such files. This has
>> the drawback that trailing spaces are not detected.
>Very good problem description.  Thanks.
I thought it might be interesting to throw this out there...  What do you 
think of an additional attribute value like
	core.whitespace blank-at-eof-min-<some 0 to N #>
	core.whitespace blank-at-eof-max-<some 0 to N #>
that could be read in when core.whitespace blank-at-eof is set.

If neither are present then use current. (No new eof blanks). If min but not max is set then allow new blanks and ensure at least min. If max but not min is set then only allow max blanks at eof. If both then treat it as a boundary.

This could ensure a whitespace policy without the repository maintainer having to correct this type of minutia and without having to nit-pick contributors into submission.

Then perhaps diff could also recognize an in range blank-at-eof so a diff using one of the ignore whitespace options would ignore eof whitelines that are in range?

> The series applies to v1.6.0.6-87-g82d97da; merging the result to 'master'
> needs some conflict resolution.
>
-- 
Thell
Junio C Hamano· Sep 6, 2009, 06:13 UTC · re: Thell Fowler · lore

Re: [PATCH 0/9] War on blank-at-eof

Thell Fowler <git@tbfowler.name> writes:
> While thinking about what appeared in:
>
> http://article.gmane.org/gmane.comp.version-control.git/124138

Oh, I forgot all about that one. The suggestion does include two very good points, one being "git apply" which I did, and the other being what I completely forgot. Introduction of blank-at-eol and blank-at-eof, and make trailing-space a convenience synonym that triggers both.

Thanks for a reminder. The following patch can come on top of the series.

-- >8 --
Subject: core.whitespace: split trailing-space into blank-at-{eol,eof}

People who configured trailing-space depended on it to catch both extra white space at the end of line, and extra blank lines at the end of file. Earlier attempt to introduce only blank-at-eof gave them an escape hatch to keep the old behaviour, but it is a regression until they explicitly specify the new error class.

This introduces a blank-at-eol that only catches extra white space at the end of line, and makes the traditional trailing-space a convenient synonym to catch both blank-at-eol and blank-at-eof. This way, people who used trailing-space continue to catch both classes of errors.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt |    5 ++++-
 cache.h                  |    5 +++--
 ws.c                     |   24 +++++++++++++++---------
 3 files changed, 22 insertions(+), 12 deletions(-)
Show changes to 3 files +22 −12

Documentation/config.txt, cache.h, ws.c

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 871384e..0e245a7 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -382,7 +382,7 @@ core.whitespace::
 	consider them as errors.  You can prefix `-` to disable
 	any of them (e.g. `-trailing-space`):
 +
-* `trailing-space` treats trailing whitespaces at the end of the line
+* `blank-at-eol` treats trailing whitespaces at the end of the line
   as an error (enabled by default).
 * `space-before-tab` treats a space character that appears immediately
   before a tab character in the initial indent part of the line as an
@@ -391,11 +391,14 @@ core.whitespace::
   space characters as an error (not enabled by default).
 * `blank-at-eof` treats blank lines added at the end of file as an error
   (enabled by default).
+* `trailing-space` is a short-hand to cover both `blank-at-eol` and
+  `blank-at-eof`.
 * `cr-at-eol` treats a carriage-return at the end of line as
   part of the line terminator, i.e. with it, `trailing-space`
   does not trigger if the character before such a carriage-return
   is not a whitespace (not enabled by default).
 
+
 core.fsyncobjectfiles::
 	This boolean will enable 'fsync()' when writing object files.
 +
diff --git a/cache.h b/cache.h
index 7152fea..ee12e74 100644
--- a/cache.h
+++ b/cache.h
@@ -841,12 +841,13 @@ void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, i
  * whitespace rules.
  * used by both diff and apply
  */
-#define WS_TRAILING_SPACE	01
+#define WS_BLANK_AT_EOL         01
 #define WS_SPACE_BEFORE_TAB	02
 #define WS_INDENT_WITH_NON_TAB	04
 #define WS_CR_AT_EOL           010
 #define WS_BLANK_AT_EOF        020
-#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB|WS_BLANK_AT_EOF)
+#define WS_TRAILING_SPACE      (WS_BLANK_AT_EOL|WS_BLANK_AT_EOF)
+#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB)
 extern unsigned whitespace_rule_cfg;
 extern unsigned whitespace_rule(const char *);
 extern unsigned parse_whitespace_rule(const char *);
diff --git a/ws.c b/ws.c
index d56636b..cd03bc0 100644
--- a/ws.c
+++ b/ws.c
@@ -15,6 +15,7 @@ static struct whitespace_rule {
 	{ "space-before-tab", WS_SPACE_BEFORE_TAB },
 	{ "indent-with-non-tab", WS_INDENT_WITH_NON_TAB },
 	{ "cr-at-eol", WS_CR_AT_EOL },
+	{ "blank-at-eol", WS_BLANK_AT_EOL },
 	{ "blank-at-eof", WS_BLANK_AT_EOF },
 };
 
@@ -101,9 +102,19 @@ unsigned whitespace_rule(const char *pathname)
 char *whitespace_error_string(unsigned ws)
 {
 	struct strbuf err;
+
 	strbuf_init(&err, 0);
-	if (ws & WS_TRAILING_SPACE)
+	if ((ws & WS_TRAILING_SPACE) == WS_TRAILING_SPACE)
 		strbuf_addstr(&err, "trailing whitespace");
+	else {
+		if (ws & WS_BLANK_AT_EOL)
+			strbuf_addstr(&err, "trailing whitespace");
+		if (ws & WS_BLANK_AT_EOF) {
+			if (err.len)
+				strbuf_addstr(&err, ", ");
+			strbuf_addstr(&err, "new blank line at EOF");
+		}
+	}
 	if (ws & WS_SPACE_BEFORE_TAB) {
 		if (err.len)
 			strbuf_addstr(&err, ", ");
@@ -114,11 +125,6 @@ char *whitespace_error_string(unsigned ws)
 			strbuf_addstr(&err, ", ");
 		strbuf_addstr(&err, "indent with spaces");
 	}
-	if (ws & WS_BLANK_AT_EOF) {
-		if (err.len)
-			strbuf_addstr(&err, ", ");
-		strbuf_addstr(&err, "new blank line at EOF");
-	}
 	return strbuf_detach(&err, NULL);
 }
 
@@ -146,11 +152,11 @@ static unsigned ws_check_emit_1(const char *line, int len, unsigned ws_rule,
 	}
 
 	/* Check for trailing whitespace. */
-	if (ws_rule & WS_TRAILING_SPACE) {
+	if (ws_rule & WS_BLANK_AT_EOL) {
 		for (i = len - 1; i >= 0; i--) {
 			if (isspace(line[i])) {
 				trailing_whitespace = i;
-				result |= WS_TRAILING_SPACE;
+				result |= WS_BLANK_AT_EOL;
 			}
 			else
 				break;
@@ -266,7 +272,7 @@ int ws_fix_copy(char *dst, const char *src, int len, unsigned ws_rule, int *erro
 	/*
 	 * Strip trailing whitespace
 	 */
-	if ((ws_rule & WS_TRAILING_SPACE) &&
+	if ((ws_rule & WS_BLANK_AT_EOL) &&
 	    (2 <= len && isspace(src[len-2]))) {
 		if (src[len - 1] == '\n') {
 			add_nl_to_tail = 1;

-- 
1.6.4.2.313.g0425f

← back to recent threads