Re: [PATCH 0/2] Ignore trailing spaces in .gitignore

From: Jeff King <peff@peff.net>
Date: Feb 8, 2014, 16:45 UTC
Message-ID: <20140208164548.GA24600@sigill.intra.peff.net>
In-Reply-To: <1391847004-22810-1-git-send-email-pclouds@gmail.com>

On Sat, Feb 08, 2014 at 03:10:02PM +0700, Nguyễn Thái Ngọc Duy wrote:

> Trailing spaces are invisible in most standard editors (*). "git diff"
> does show trailing spaces by default. But that does not help newly
> written .gitignore files. And trailing spaces are the source of
> frustration when writing .gitignore.

I guess leading spaces are not as frustrating, but I wonder if it would be more consistent to say that we soak up all whitespace. That is also a regression, but I agree that these are exceptional cases, and as long as we provide an "out" for people to cover them via quoting, ignoring the whitespace seems like a sane default.

> People can still quote trailing spaces, which will not be ignored (and
> much more visible). Quoting comes with a cost of doing fnmatch(). But
> I won't optimize it until I see someone shows me they have a use case
> for it.

I naively expected that:

  echo 'trailing\ \ ' >.gitignore

would count as quoting, but your patch doesn't handle that. It _does_ seem to work currently (that is, the backslashes go away and we treat it literally), but I am not sure if that is planned, or simply happens to work.

I guess by quoting you meant:

  echo '"trailing  "' >.gitignore

Anyway, here are some tests I wrote up while playing with this. They do not pass with your current patch for the reasons above, but maybe they will be useful to squash in (either after tweaking the tests, or tweaking the patch).

diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index b4d98e6..4dde314 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -775,4 +775,33 @@ test_expect_success PIPE 'streaming support for --stdin' '
 	echo "$response" | grep "^::	two"
 '
 
+############################################################################
+#
+# test whitespace handling
+
+test_expect_success 'leading/trailing whitespace is ignored' '
+	mkdir whitespace &&
+	>whitespace/leading &&
+	>whitespace/trailing &&
+	>whitespace/untracked &&
+	{
+		echo "    whitespace/leading" &&
+		echo "whitespace/trailing   "
+	} >ignore &&
+	echo whitespace/untracked >expect &&
+	git ls-files -o -X ignore whitespace >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'quoting allows trailing whitespace' '
+	rm -rf whitespace &&
+	mkdir whitespace &&
+	>"whitespace/trailing  " &&
+	>whitespace/untracked &&
+	echo "whitespace/trailing\\ \\ " >ignore &&
+	echo whitespace/untracked >expect &&
+	git ls-files -o -X ignore whitespace >actual &&
+	test_cmp expect actual
+'
+
 test_done