On Sat, Feb 8, 2014 at 11:45 PM, Jeff King <[email protected]> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
