From: Jeff King <peff@peff.net>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 0/2] Ignore trailing spaces in .gitignore
Date: Sat, 8 Feb 2014 11:45:49 -0500 [thread overview]
Message-ID: <20140208164548.GA24600@sigill.intra.peff.net> (raw)
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
next prev parent reply other threads:[~2014-02-08 16:45 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-08 8:10 [PATCH 0/2] Ignore trailing spaces in .gitignore Nguyễn Thái Ngọc Duy
2014-02-08 8:10 ` [PATCH 1/2] dir: warn about trailing spaces in exclude pattern Nguyễn Thái Ngọc Duy
2014-02-08 14:33 ` Torsten Bögershausen
2014-02-08 8:10 ` [PATCH 2/2] dir: ignore trailing spaces in exclude patterns Nguyễn Thái Ngọc Duy
2014-02-08 16:45 ` Jeff King [this message]
2014-02-08 23:48 ` [PATCH 0/2] Ignore trailing spaces in .gitignore Duy Nguyen
2014-02-10 1:19 ` Jeff King
2014-02-09 0:26 ` [PATCH v2 " Nguyễn Thái Ngọc Duy
2014-02-09 0:26 ` [PATCH v2 1/2] dir: warn about trailing spaces in exclude patterns Nguyễn Thái Ngọc Duy
2014-02-09 0:26 ` [PATCH v2 2/2] dir: ignore " Nguyễn Thái Ngọc Duy
2014-02-10 4:07 ` [PATCH 0/2] Ignore trailing spaces in .gitignore Junio C Hamano
2014-02-10 4:29 ` Duy Nguyen
2014-02-10 5:04 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140208164548.GA24600@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=pclouds@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).