[PATCH] Trailing whitespace and no newline fix

[PATCH] Trailing whitespace and no newline fix

[SZEDER Gábor]

If a patch adds a new line to the end of a file and this line ends with
one trailing whitespace character and has no newline, then
'--whitespace=fix' currently does not remove that trailing whitespace.

This patch fixes this by removing the check for trailing whitespace at
the end of the line at a hardcoded offset which does not take the
eventual absence of newline into account.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 t/t4124-apply-ws-rule.sh |   18 ++++++++++++++++++
 ws.c                     |    5 ++---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh
index f83322e..5698a9a 100755
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@ -148,4 +148,22 @@ do
 	done
 done
 
+create_patch () {
+	sed -e "s/_/ /" <<-\EOF
+		diff --git a/target b/target
+		index e69de29..8bd6648 100644
+		--- a/target
+		+++ b/target
+		@@ -0,0 +1 @@
+		+A line with trailing whitespace and no newline_
+		\ No newline at end of file
+	EOF
+}
+
+test_expect_success 'trailing whitespace & no newline at the end of file' '
+	>target &&
+	create_patch | git apply --whitespace=fix - &&
+	grep "newline$" target
+'
+
 test_done
diff --git a/ws.c b/ws.c
index b1efcd9..9c20acd 100644
--- a/ws.c
+++ b/ws.c
@@ -259,9 +259,8 @@ 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) &&
-	    (2 <= len && isspace(src[len-2]))) {
-		if (src[len - 1] == '\n') {
+	if (ws_rule & WS_TRAILING_SPACE) {
+		if (1 < len && src[len - 1] == '\n') {
 			add_nl_to_tail = 1;
 			len--;
 			if (1 < len && src[len - 1] == '\r') {
-- 
1.6.4.rc1.73.ga0daf

Re: [PATCH] Trailing whitespace and no newline fix

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

> If a patch adds a new line to the end of a file and this line ends with
> one trailing whitespace character and has no newline, then
> '--whitespace=fix' currently does not remove that trailing whitespace.

Looks obvious and correct.  Thanks.

Re: [PATCH] Trailing whitespace and no newline fix

[Junio C Hamano]

SZEDER Gábor <szeder@ira.uka.de> writes:

> If a patch adds a new line to the end of a file and this line ends with
> one trailing whitespace character and has no newline, then
> '--whitespace=fix' currently does not remove that trailing whitespace.
>
> This patch fixes this by removing the check for trailing whitespace at
> the end of the line at a hardcoded offset which does not take the
> eventual absence of newline into account.

This patch does a lot more than what it claims it does.

Try to apply Jakub's 10 patch series with this, with --whitespace=fix, and
notice that the 9th patch gets all the blank lines removed, claiming that
114 lines had whitespace errors, even though there is only one trailing
whitespace. 

Here is a fix.  I am pissed at myself not spotting this earlier.  The
worst part is that it is part of 'maint' and all integration branches
needs fixing.

-- >8 --
From: Junio C Hamano <gitster@pobox.com>
Date: Sat, 25 Jul 2009 01:29:20 -0700
Subject: [PATCH] Fix severe breakage in "git-apply --whitespace=fix"

735c674 (Trailing whitespace and no newline fix, 2009-07-22) completely
broke --whitespace=fix, causing it to lose all the empty lines in a patch.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t4124-apply-ws-rule.sh |   10 +++++++---
 ws.c                     |    4 ++--
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh
index 5698a9a..fac2093 100755
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@ -154,7 +154,9 @@ create_patch () {
 		index e69de29..8bd6648 100644
 		--- a/target
 		+++ b/target
-		@@ -0,0 +1 @@
+		@@ -0,0 +1,3 @@
+		+An empty line follows
+		+
 		+A line with trailing whitespace and no newline_
 		\ No newline at end of file
 	EOF
@@ -162,8 +164,10 @@ create_patch () {
 
 test_expect_success 'trailing whitespace & no newline at the end of file' '
 	>target &&
-	create_patch | git apply --whitespace=fix - &&
-	grep "newline$" target
+	create_patch >patch-file &&
+	git apply --whitespace=fix patch-file &&
+	grep "newline$" target &&
+	grep "^$" target
 '
 
 test_done
diff --git a/ws.c b/ws.c
index 8d855b7..59d0883 100644
--- a/ws.c
+++ b/ws.c
@@ -262,10 +262,10 @@ 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 (1 < len && src[len - 1] == '\n') {
+		if (0 < len && src[len - 1] == '\n') {
 			add_nl_to_tail = 1;
 			len--;
-			if (1 < len && src[len - 1] == '\r') {
+			if (0 < len && src[len - 1] == '\r') {
 				add_cr_to_tail = !!(ws_rule & WS_CR_AT_EOL);
 				len--;
 			}
-- 
1.6.4.rc2.19.gfa5c0

warning: "git am --whitespace=fix" broken in 1.6.4-rc2

[Junio C Hamano]

I am very sorry that I did not spot this earlier, but 1.6.4-rc2 has a
quite serious bug in "git apply --whitespace=fix" (hence "git am" with the
same option) in that it can lose all the added empty lines when it detects
any trailing spaces.  I've sent a fix out already, and am planning to
issue 1.6.4-rc3 this weekend, but in the meantime, please do not use it,
nor any version that contains commit 735c674 (Trailing whitespace and no
newline fix, 2009-07-22).

Unfortunately the commit is included in all recent integration branches
down to 'maint'.  I'll push out the fixes for them soonish.

Sorry for the trouble.