Re: [PATCH 0/2] Ignore trailing spaces in .gitignore
From: Duy Nguyen <hidden>
Date: 2016-06-15 22:59:52
On Sat, Feb 8, 2014 at 11:45 PM, Jeff King [off-list ref] wrote:
On Sat, Feb 08, 2014 at 03:10:02PM +0700, Nguyễn Thái Ngọc Duy wrote:quoted
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...
quoted
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.
quoted hunk ↗ jump to hunk
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