threads / patch / 35830

patch, 2 partsIgnore trailing spaces in .gitignore

Subject: [PATCH 0/2] Ignore trailing spaces in .gitignore

## tl;dr

13 messages between Feb 8, 2014 and Feb 10, 2014. Diffs are folded; open one to read it.

replies: 12people: 4as markdown or json

Nguyễn Thái Ngọc Duy· Feb 8, 2014, 08:10 UTC · lore

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.

So let's ignore them. Nobody sane would put a trailing space in file names. But we could be careful and do it in two steps: warn first, then ignore trailing spaces. Another option is merge two patches in one and be done with it.

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.

(*) either that or my coworkers keep pissing me off on purpose when
    they never clean trailing spaces.
Nguyễn Thái Ngọc Duy (2):
  dir: warn about trailing spaces in exclude pattern
  dir: ignore trailing spaces in exclude patterns
 Documentation/gitignore.txt | 3 +++
 dir.c                       | 9 +++++++++
 2 files changed, 12 insertions(+)
-- 
1.8.5.2.240.g8478abd
Nguyễn Thái Ngọc Duy· Feb 8, 2014, 08:10 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 1/2] dir: warn about trailing spaces in exclude pattern

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 dir.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
Show changes to dir.c +11 −0
diff --git a/dir.c b/dir.c
index b35b633..9edde44 100644
--- a/dir.c
+++ b/dir.c
@@ -491,6 +491,16 @@ void clear_exclude_list(struct exclude_list *el)
 	el->filebuf = NULL;
 }
 
+static void check_trailing_spaces(const char *fname, char *buf)
+{
+	int len = strlen(buf);
+	while (len && buf[len - 1] == ' ')
+		len--;
+	if (buf[len] != '\0')
+		warning(_("%s: trailing spaces in '%s'. Please quote them."),
+			fname, buf);
+}
+
 int add_excludes_from_file_to_list(const char *fname,
 				   const char *base,
 				   int baselen,
@@ -542,6 +552,7 @@ int add_excludes_from_file_to_list(const char *fname,
 		if (buf[i] == '\n') {
 			if (entry != buf + i && entry[0] != '#') {
 				buf[i - (i && buf[i-1] == '\r')] = 0;
+				check_trailing_spaces(fname, entry);
 				add_exclude(entry, base, baselen, el, lineno);
 			}
 			lineno++;

-- 
1.8.5.2.240.g8478abd
Torsten Bögershausen· Feb 8, 2014, 14:33 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 1/2] dir: warn about trailing spaces in exclude pattern

On 2014-02-08 09.10, Nguyễn Thái Ngọc Duy wrote:
Show 19 quoted lines
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  dir.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/dir.c b/dir.c
> index b35b633..9edde44 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -491,6 +491,16 @@ void clear_exclude_list(struct exclude_list *el)
>  	el->filebuf = NULL;
>  }
>  
> +static void check_trailing_spaces(const char *fname, char *buf)
> +{
> +	int len = strlen(buf);
> +	while (len && buf[len - 1] == ' ')
> +		len--;
> +	if (buf[len] != '\0')
Do we need the while loop here, (when we only warn) ?
> +		warning(_("%s: trailing spaces in '%s'. Please quote them."),
> +			fname, buf);
> +}
This is nice. However we can hint the user that there are 2 choices: 
		warning(_("%s: trailing spaces in '%s'. Please remove them or quote them."),
Show 13 quoted lines
> +
>  int add_excludes_from_file_to_list(const char *fname,
>  				   const char *base,
>  				   int baselen,
> @@ -542,6 +552,7 @@ int add_excludes_from_file_to_list(const char *fname,
>  		if (buf[i] == '\n') {
>  			if (entry != buf + i && entry[0] != '#') {
>  				buf[i - (i && buf[i-1] == '\r')] = 0;
> +				check_trailing_spaces(fname, entry);
>  				add_exclude(entry, base, baselen, el, lineno);
>  			}
>  			lineno++;
>
Nguyễn Thái Ngọc Duy· Feb 8, 2014, 08:10 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH 2/2] dir: ignore trailing spaces in exclude patterns

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/gitignore.txt | 3 +++
 dir.c                       | 4 +---
 2 files changed, 4 insertions(+), 3 deletions(-)
Show changes to 2 files +4 −3

Documentation/gitignore.txt, dir.c

diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index b08d34d..8734c15 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -77,6 +77,9 @@ PATTERN FORMAT
    Put a backslash ("`\`") in front of the first hash for patterns
    that begin with a hash.
 
+ - Trailing spaces are ignored unless they are quoted with backlash
+   ("`\`").
+
  - An optional prefix "`!`" which negates the pattern; any
    matching file excluded by a previous pattern will become
    included again. It is not possible to re-include a file if a parent
diff --git a/dir.c b/dir.c
index 9edde44..7dee5ca 100644
--- a/dir.c
+++ b/dir.c
@@ -496,9 +496,7 @@ static void check_trailing_spaces(const char *fname, char *buf)
 	int len = strlen(buf);
 	while (len && buf[len - 1] == ' ')
 		len--;
-	if (buf[len] != '\0')
-		warning(_("%s: trailing spaces in '%s'. Please quote them."),
-			fname, buf);
+	buf[len] = '\0';
 }
 
 int add_excludes_from_file_to_list(const char *fname,

-- 
1.8.5.2.240.g8478abd
Jeff King· Feb 8, 2014, 16:45 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 0/2] Ignore trailing spaces in .gitignore

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).

Show changes to t/t0008-ignores.sh +29 −0
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 Nguyen· Feb 8, 2014, 23:48 UTC · re: Jeff King · lore

Re: [PATCH 0/2] Ignore trailing spaces in .gitignore

On Sat, Feb 8, 2014 at 11:45 PM, Jeff King <peff@peff.net> wrote:
Show 12 quoted lines
> 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...
Show 14 quoted lines
>
>> 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.

Show 44 quoted lines
>
> 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
Jeff King· Feb 10, 2014, 01:19 UTC · re: Duy Nguyen · lore

Re: [PATCH 0/2] Ignore trailing spaces in .gitignore

On Sun, Feb 09, 2014 at 06:48:18AM +0700, Duy Nguyen wrote:
Show 8 quoted lines
> > 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.

For some reason I was thinking that we already handled double-quotes here (as we do in other places where quoting is optional). But it looks like we don't currently, so yeah, I don't think it is worth adding due to the potential confusion.

Backslash-escaping was what I had originally assumed you meant, and it was, so we are all on the same page (the patch was just broken. ;) ).

-Peff
Nguyễn Thái Ngọc Duy· Feb 9, 2014, 00:26 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v2 0/2] Ignore trailing spaces in .gitignore

This reroll now respects backslash quoting. Thanks Jeff and Torsten for the comments.

Nguyễn Thái Ngọc Duy (2):
  dir: warn about trailing spaces in exclude patterns
  dir: ignore trailing spaces in exclude patterns
 Documentation/gitignore.txt |  3 +++
 dir.c                       | 20 ++++++++++++++++++++
 t/t0008-ignores.sh          | 31 +++++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+)
-- 
1.8.5.2.240.g8478abd
Nguyễn Thái Ngọc Duy· Feb 9, 2014, 00:26 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v2 1/2] dir: warn about trailing spaces in exclude patterns

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 dir.c              | 17 +++++++++++++++++
 t/t0008-ignores.sh | 31 +++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)
Show changes to 2 files +48 −0

dir.c, t/t0008-ignores.sh

diff --git a/dir.c b/dir.c
index b35b633..6162209 100644
--- a/dir.c
+++ b/dir.c
@@ -491,6 +491,22 @@ void clear_exclude_list(struct exclude_list *el)
 	el->filebuf = NULL;
 }
 
+static void check_trailing_spaces(const char *fname, char *buf)
+{
+	int i, last_space = -1, len = strlen(buf);
+	for (i = 0; i < len; i++)
+		if (buf[i] == '\\')
+			i++;
+		else if (buf[i] == ' ')
+			last_space = i;
+		else
+			last_space = -1;
+
+	if (last_space == len - 1)
+		warning(_("%s: trailing spaces in '%s'. Please quote or remove them."),
+			fname, buf);
+}
+
 int add_excludes_from_file_to_list(const char *fname,
 				   const char *base,
 				   int baselen,
@@ -542,6 +558,7 @@ int add_excludes_from_file_to_list(const char *fname,
 		if (buf[i] == '\n') {
 			if (entry != buf + i && entry[0] != '#') {
 				buf[i - (i && buf[i-1] == '\r')] = 0;
+				check_trailing_spaces(fname, entry);
 				add_exclude(entry, base, baselen, el, lineno);
 			}
 			lineno++;
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index b4d98e6..9e1d64c 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -775,4 +775,35 @@ test_expect_success PIPE 'streaming support for --stdin' '
 	echo "$response" | grep "^::	two"
 '
 
+############################################################################
+#
+# test whitespace handling
+
+test_expect_success 'trailing whitespace is warned' '
+	mkdir whitespace &&
+	>whitespace/trailing &&
+	>whitespace/untracked &&
+	echo "whitespace/trailing   " >ignore &&
+	cat >expect <<EOF &&
+whitespace/trailing
+whitespace/untracked
+EOF
+	git ls-files -o -X ignore whitespace >actual 2>err &&
+	grep "ignore:.*'\''whitespace/trailing   '\''" err &&
+	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 &&
+	: >err.expect &&
+	git ls-files -o -X ignore whitespace >actual 2>err &&
+	test_cmp expect actual &&
+	test_cmp err.expect err
+'
+
 test_done

-- 
1.8.5.2.240.g8478abd
Nguyễn Thái Ngọc Duy· Feb 9, 2014, 00:26 UTC · re: Nguyễn Thái Ngọc Duy · lore

[PATCH v2 2/2] dir: ignore trailing spaces in exclude patterns

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/gitignore.txt |  3 +++
 dir.c                       | 21 ++++++++++++---------
 t/t0008-ignores.sh          |  8 ++++----
 3 files changed, 19 insertions(+), 13 deletions(-)
Show changes to 3 files +19 −13

Documentation/gitignore.txt, dir.c, t/t0008-ignores.sh

diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index b08d34d..8734c15 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -77,6 +77,9 @@ PATTERN FORMAT
    Put a backslash ("`\`") in front of the first hash for patterns
    that begin with a hash.
 
+ - Trailing spaces are ignored unless they are quoted with backlash
+   ("`\`").
+
  - An optional prefix "`!`" which negates the pattern; any
    matching file excluded by a previous pattern will become
    included again. It is not possible to re-include a file if a parent
diff --git a/dir.c b/dir.c
index 6162209..70076b5 100644
--- a/dir.c
+++ b/dir.c
@@ -491,20 +491,23 @@ void clear_exclude_list(struct exclude_list *el)
 	el->filebuf = NULL;
 }
 
-static void check_trailing_spaces(const char *fname, char *buf)
+static void trim_trailing_spaces(char *buf)
 {
-	int i, last_space = -1, len = strlen(buf);
+	int i, last_space = -1, nr_spaces, len = strlen(buf);
 	for (i = 0; i < len; i++)
 		if (buf[i] == '\\')
 			i++;
-		else if (buf[i] == ' ')
-			last_space = i;
-		else
+		else if (buf[i] == ' ') {
+			if (last_space == -1) {
+				last_space = i;
+				nr_spaces = 1;
+			} else
+				nr_spaces++;
+		} else
 			last_space = -1;
 
-	if (last_space == len - 1)
-		warning(_("%s: trailing spaces in '%s'. Please quote or remove them."),
-			fname, buf);
+	if (last_space != -1 && last_space + nr_spaces == len)
+		buf[last_space] = '\0';
 }
 
 int add_excludes_from_file_to_list(const char *fname,
@@ -558,7 +561,7 @@ int add_excludes_from_file_to_list(const char *fname,
 		if (buf[i] == '\n') {
 			if (entry != buf + i && entry[0] != '#') {
 				buf[i - (i && buf[i-1] == '\r')] = 0;
-				check_trailing_spaces(fname, entry);
+				trim_trailing_spaces(entry);
 				add_exclude(entry, base, baselen, el, lineno);
 			}
 			lineno++;
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 9e1d64c..bbaf6b5 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -779,18 +779,18 @@ test_expect_success PIPE 'streaming support for --stdin' '
 #
 # test whitespace handling
 
-test_expect_success 'trailing whitespace is warned' '
+test_expect_success 'trailing whitespace is ignored' '
 	mkdir whitespace &&
 	>whitespace/trailing &&
 	>whitespace/untracked &&
 	echo "whitespace/trailing   " >ignore &&
 	cat >expect <<EOF &&
-whitespace/trailing
 whitespace/untracked
 EOF
+	: >err.expect &&
 	git ls-files -o -X ignore whitespace >actual 2>err &&
-	grep "ignore:.*'\''whitespace/trailing   '\''" err &&
-	test_cmp expect actual
+	test_cmp expect actual &&
+	test_cmp err.expect err
 '
 
 test_expect_success 'quoting allows trailing whitespace' '

-- 
1.8.5.2.240.g8478abd
Junio C Hamano· Feb 10, 2014, 04:07 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH 0/2] Ignore trailing spaces in .gitignore

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
Show 12 quoted lines
> 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.
>
> So let's ignore them. Nobody sane would put a trailing space in file
> names. But we could be careful and do it in two steps: warn first,
> then ignore trailing spaces. Another option is merge two patches in
> one and be done with it.
>
> People can still quote trailing spaces, which will not be ignored (and
> much more visible). Quoting comes with a cost of doing fnmatch(). But

Hmph, sorry but I fail to see why we need to incur cost for fnmatch(). We read and parse the file and keep them as internal strings, so your unquoting (and complaining the unquoted trailng spaces) can be done at the parse time, while keeping the trailing spaces the user explicitly told us to keep by quoting in the internal string that we eventually feed fnmatch() with _after_ unquoting, no?

Puzzled...
Duy Nguyen· Feb 10, 2014, 04:29 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/2] Ignore trailing spaces in .gitignore

On Mon, Feb 10, 2014 at 11:07 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 22 quoted lines
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> 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.
>>
>> So let's ignore them. Nobody sane would put a trailing space in file
>> names. But we could be careful and do it in two steps: warn first,
>> then ignore trailing spaces. Another option is merge two patches in
>> one and be done with it.
>>
>> People can still quote trailing spaces, which will not be ignored (and
>> much more visible). Quoting comes with a cost of doing fnmatch(). But
>
> Hmph, sorry but I fail to see why we need to incur cost for
> fnmatch().  We read and parse the file and keep them as internal
> strings, so your unquoting (and complaining the unquoted trailng
> spaces) can be done at the parse time, while keeping the trailing
> spaces the user explicitly told us to keep by quoting in the
> internal string that we eventually feed fnmatch() with _after_
> unquoting, no?

That's the optimization in the "but" sentence. Another (off topic) opt. we could do is make "*.[ch]" behave more like "*.c", where we just try to match the tail part.

-- 
Duy
Junio C Hamano· Feb 10, 2014, 05:04 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/2] Ignore trailing spaces in .gitignore

Junio C Hamano <gitster@pobox.com> writes:
Show 24 quoted lines
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> 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.
>>
>> So let's ignore them. Nobody sane would put a trailing space in file
>> names. But we could be careful and do it in two steps: warn first,
>> then ignore trailing spaces. Another option is merge two patches in
>> one and be done with it.
>>
>> People can still quote trailing spaces, which will not be ignored (and
>> much more visible). Quoting comes with a cost of doing fnmatch(). But
>
> Hmph, sorry but I fail to see why we need to incur cost for
> fnmatch().  We read and parse the file and keep them as internal
> strings, so your unquoting (and complaining the unquoted trailng
> spaces) can be done at the parse time, while keeping the trailing
> spaces the user explicitly told us to keep by quoting in the
> internal string that we eventually feed fnmatch() with _after_
> unquoting, no?
>
> Puzzled...

Heh, silly me. Yes, we _could_ parse and strip "\ " into " " if we wanted to, but we can just give "\ " and let fnmatch(3) do its thing, and that is the right thing to do for a rare corner case like this (i.e. deliberate trailing spaces).

So I think I understand your reasoning, and I agree with it.

← back to recent threads