On Sat, Feb 8, 2014 at 11:45 PM, Jeff King <peff@xxxxxxxx> wrote: > 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. Hm... > >> 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. No that's what I had in mind. But yeah my patches are flawed. > > I guess by quoting you meant: > > echo '"trailing "' >.gitignore This makes " special. If we follow shell convention then things between ".." should be literal (e.g. "*" is no longer a wildcard). We don't support it yet. So I rather go with backslash as it adds less code. > > 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 -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html