Re: [PATCH alt-v3] Improve function dir.c:trim_trailing_spaces()

Subsystems: the rest

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH alt-v3] Improve function dir.c:trim_trailing_spaces()

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:01:37

Pasha Bolokhov [off-list ref] writes:

quoted hunk

diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 63beb99..5ef5ad3 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh

@@ -806,4 +806,27 @@ test_expect_success !MINGW 'quoting allows trailing whitespace' '
 	test_cmp err.expect err
 '
 
+test_expect_success NOT_MINGW,NOT_CYGWIN 'correct handling of backslashes' '
+	rm -rf whitespace &&
+	mkdir whitespace &&
+	>"whitespace/trailing 1  " &&
+	>"whitespace/trailing 2 \\\\" &&
+	>"whitespace/trailing 3 \\\\" &&
+	>"whitespace/trailing 4   \\ " &&
+	>"whitespace/trailing 5 \\ \\ " &&
+	>"whitespace/trailing 6 \\a\\" &&
+	>whitespace/untracked &&
+	echo "whitespace/trailing 1 \\    " >ignore  &&
+	echo "whitespace/trailing 2 \\\\\\\\\\\\\\\\" >>ignore &&
+	echo "whitespace/trailing 3 \\\\\\\\\\\\\\\\ " >>ignore &&
+	echo "whitespace/trailing 4   \\\\\\\\\\\\    " >>ignore &&
+	echo "whitespace/trailing 5 \\\\\\\\ \\\\\\\\\\\\   " >>ignore &&
+	echo "whitespace/trailing 6 \\\\\\\\a\\\\\\\\" >>ignore &&
+	echo whitespace/untracked >expect &&
+	>err.expect &&
+	git ls-files -o -X ignore whitespace >actual 2>err &&
+	test_cmp expect actual &&
+	test_cmp err.expect err
+'

This passes with your shell set to dash but fails with bash.

Let's fix it up like so.

 t/t0008-ignores.sh | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 5ef5ad3..39e55a1 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh

@@ -816,12 +816,14 @@ test_expect_success NOT_MINGW,NOT_CYGWIN 'correct handling of backslashes' '
 	>"whitespace/trailing 5 \\ \\ " &&
 	>"whitespace/trailing 6 \\a\\" &&
 	>whitespace/untracked &&
-	echo "whitespace/trailing 1 \\    " >ignore  &&
-	echo "whitespace/trailing 2 \\\\\\\\\\\\\\\\" >>ignore &&
-	echo "whitespace/trailing 3 \\\\\\\\\\\\\\\\ " >>ignore &&
-	echo "whitespace/trailing 4   \\\\\\\\\\\\    " >>ignore &&
-	echo "whitespace/trailing 5 \\\\\\\\ \\\\\\\\\\\\   " >>ignore &&
-	echo "whitespace/trailing 6 \\\\\\\\a\\\\\\\\" >>ignore &&
+	sed -e "s/Z$//" >ignore <<-\EOF &&
+	whitespace/trailing 1 \    Z
+	whitespace/trailing 2 \\\\Z
+	whitespace/trailing 3 \\\\ Z
+	whitespace/trailing 4   \\\    Z
+	whitespace/trailing 5 \\ \\\   Z
+	whitespace/trailing 6 \\a\\Z
+	EOF
 	echo whitespace/untracked >expect &&
 	>err.expect &&
 	git ls-files -o -X ignore whitespace >actual 2>err &&

Re: [PATCH alt-v3] Improve function dir.c:trim_trailing_spaces()

From: Jeff King <hidden>
Date: 2016-06-15 23:01:37

On Fri, Jun 13, 2014 at 01:23:47PM -0700, Junio C Hamano wrote:

This passes with your shell set to dash but fails with bash.

Let's fix it up like so.

Thanks, I should have been more suspicious of the combination of backslashes
and echo during review. :)

quoted hunk

diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 5ef5ad3..39e55a1 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh

@@ -816,12 +816,14 @@ test_expect_success NOT_MINGW,NOT_CYGWIN 'correct handling of backslashes' '
 	>"whitespace/trailing 5 \\ \\ " &&
 	>"whitespace/trailing 6 \\a\\" &&
 	>whitespace/untracked &&
-	echo "whitespace/trailing 1 \\    " >ignore  &&
-	echo "whitespace/trailing 2 \\\\\\\\\\\\\\\\" >>ignore &&
-	echo "whitespace/trailing 3 \\\\\\\\\\\\\\\\ " >>ignore &&
-	echo "whitespace/trailing 4   \\\\\\\\\\\\    " >>ignore &&
-	echo "whitespace/trailing 5 \\\\\\\\ \\\\\\\\\\\\   " >>ignore &&
-	echo "whitespace/trailing 6 \\\\\\\\a\\\\\\\\" >>ignore &&
+	sed -e "s/Z$//" >ignore <<-\EOF &&
+	whitespace/trailing 1 \    Z
+	whitespace/trailing 2 \\\\Z
+	whitespace/trailing 3 \\\\ Z
+	whitespace/trailing 4   \\\    Z
+	whitespace/trailing 5 \\ \\\   Z
+	whitespace/trailing 6 \\a\\Z
+	EOF

The end result is much more readable, too, IMHO.

-Peff

Re: [PATCH alt-v3] Improve function dir.c:trim_trailing_spaces()

From: Pasha Bolokhov <hidden>
Date: 2016-06-15 23:01:37

Thanks for the fix. I knew these backslashes could cause trouble. No
doubt the fixed version reads better than a hundred backslashes


On Fri, Jun 13, 2014 at 4:25 PM, Jeff King [off-list ref] wrote:

On Fri, Jun 13, 2014 at 01:23:47PM -0700, Junio C Hamano wrote:

quoted

This passes with your shell set to dash but fails with bash.

Let's fix it up like so.

Thanks, I should have been more suspicious of the combination of backslashes
and echo during review. :)

quoted

diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 5ef5ad3..39e55a1 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh

@@ -816,12 +816,14 @@ test_expect_success NOT_MINGW,NOT_CYGWIN 'correct handling of backslashes' '
      >"whitespace/trailing 5 \\ \\ " &&
      >"whitespace/trailing 6 \\a\\" &&
      >whitespace/untracked &&
-     echo "whitespace/trailing 1 \\    " >ignore  &&
-     echo "whitespace/trailing 2 \\\\\\\\\\\\\\\\" >>ignore &&
-     echo "whitespace/trailing 3 \\\\\\\\\\\\\\\\ " >>ignore &&
-     echo "whitespace/trailing 4   \\\\\\\\\\\\    " >>ignore &&
-     echo "whitespace/trailing 5 \\\\\\\\ \\\\\\\\\\\\   " >>ignore &&
-     echo "whitespace/trailing 6 \\\\\\\\a\\\\\\\\" >>ignore &&
+     sed -e "s/Z$//" >ignore <<-\EOF &&
+     whitespace/trailing 1 \    Z
+     whitespace/trailing 2 \\\\Z
+     whitespace/trailing 3 \\\\ Z
+     whitespace/trailing 4   \\\    Z
+     whitespace/trailing 5 \\ \\\   Z
+     whitespace/trailing 6 \\a\\Z
+     EOF

The end result is much more readable, too, IMHO.

-Peff

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help