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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to