[PATCH] Support \ in non-wildcard .gitignore entries

If you had an exclude-pattern with a backslash in it, e.g. "\#foo",
this would not work, since git would do a strcmp of the exclude pattern
and the filename. Only wildcard patterns were matched with fnmatch,
which does the right thing with backslashes. We now also treat all patterns
containing backslashes as wildcards.

De-escaping the pattern while reading the .gitignore file is error prone,
since that would break patterns with both backslashes and wildcards.
E.g. "\\*.c" would be translated to "\*.c" before fnmatch got it,
and would change the meaning of the rule dramatically.

Signed-off-by: Finn Arne Gangstad <redacted>
---
 dir.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/dir.c b/dir.c
index cfd1ea5..2245749 100644
--- a/dir.c
+++ b/dir.c

@@ -137,7 +137,7 @@ int match_pathspec(const char **pathspec, const char *name, int namelen,
 
 static int no_wildcard(const char *string)
 {
-	return string[strcspn(string, "*?[{")] == '\0';
+	return string[strcspn(string, "*?[{\\")] == '\0';
 }
 
 void add_exclude(const char *string, const char *base,

-- 
1.6.2.rc0.11.g68cbb.dirty

Hi,

On Tue, 10 Feb 2009, Finn Arne Gangstad wrote:

If you had an exclude-pattern with a backslash in it, e.g. "\#foo",
this would not work, since git would do a strcmp of the exclude pattern
and the filename. Only wildcard patterns were matched with fnmatch,
which does the right thing with backslashes. We now also treat all patterns
containing backslashes as wildcards.

De-escaping the pattern while reading the .gitignore file is error prone,
since that would break patterns with both backslashes and wildcards.
E.g. "\\*.c" would be translated to "\*.c" before fnmatch got it,
and would change the meaning of the rule dramatically.

I am not sure I understand (maybe a test case would help, but that test 
case would have to be disabled on Windows, I guess):

You mean that '\#abc' would match '\#abc', but '\#abc*' would not?

Ciao,
Dscho

On Tue, Feb 10, 2009 at 01:56:36PM +0100, Johannes Schindelin wrote:

Hi,

On Tue, 10 Feb 2009, Finn Arne Gangstad wrote:

quoted

If you had an exclude-pattern with a backslash in it, e.g. "\#foo",
this would not work, since git would do a strcmp of the exclude pattern
and the filename. Only wildcard patterns were matched with fnmatch,
which does the right thing with backslashes. We now also treat all patterns
containing backslashes as wildcards.

De-escaping the pattern while reading the .gitignore file is error prone,
since that would break patterns with both backslashes and wildcards.
E.g. "\\*.c" would be translated to "\*.c" before fnmatch got it,
and would change the meaning of the rule dramatically.

I am not sure I understand (maybe a test case would help, but that test 
case would have to be disabled on Windows, I guess):

You mean that '\#abc' would match '\#abc', but '\#abc*' would not?

Currently, \#abc does not match a file named #abc, but \#abc* does.
With the patch, both will match.

- Finn Arne

Hi,

On Tue, 10 Feb 2009, Finn Arne Gangstad wrote:

On Tue, Feb 10, 2009 at 01:56:36PM +0100, Johannes Schindelin wrote:

quoted

Hi,

On Tue, 10 Feb 2009, Finn Arne Gangstad wrote:

quoted

If you had an exclude-pattern with a backslash in it, e.g. "\#foo",
this would not work, since git would do a strcmp of the exclude pattern
and the filename. Only wildcard patterns were matched with fnmatch,
which does the right thing with backslashes. We now also treat all patterns
containing backslashes as wildcards.

De-escaping the pattern while reading the .gitignore file is error prone,
since that would break patterns with both backslashes and wildcards.
E.g. "\\*.c" would be translated to "\*.c" before fnmatch got it,
and would change the meaning of the rule dramatically.

I am not sure I understand (maybe a test case would help, but that test 
case would have to be disabled on Windows, I guess):

You mean that '\#abc' would match '\#abc', but '\#abc*' would not?

Currently, \#abc does not match a file named #abc, but \#abc* does.
With the patch, both will match.

Ah, so I was wrong, and the test case would not have to be disabled on 
Windows.

Thanks,
Dscho

"\" was treated differently in exclude rules depending on whether a
wildcard match was done. For wildcard rules, "\" was de-escaped in
fnmatch, but this was not done for other rules since they used strcmp
instead.  A file named "#foo" would not be excluded by "\#foo", but would
be excluded by "\#foo*".

We now treat all rules with "\" as wildcard rules.

Another solution could be to de-escape all non-wildcard rules as we
read them, but we would have to do the de-escaping exactly as fnmatch
does it to avoid inconsistencies.

Signed-off-by: Finn Arne Gangstad <redacted>
---
 dir.c                                       |    2 +-
 t/t3003-ls-files-others-escaped-excludes.sh |   37 +++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletions(-)
 create mode 100755 t/t3003-ls-files-others-escaped-excludes.sh

diff --git a/dir.c b/dir.c
index cfd1ea5..2245749 100644
--- a/dir.c
+++ b/dir.c

@@ -137,7 +137,7 @@ int match_pathspec(const char **pathspec, const char *name, int namelen,
 
 static int no_wildcard(const char *string)
 {
-	return string[strcspn(string, "*?[{")] == '\0';
+	return string[strcspn(string, "*?[{\\")] == '\0';
 }
 
 void add_exclude(const char *string, const char *base,

diff --git a/t/t3003-ls-files-others-escaped-excludes.sh b/t/t3003-ls-files-others-escaped-excludes.sh
new file mode 100755
index 0000000..bce8741
--- /dev/null
+++ b/t/t3003-ls-files-others-escaped-excludes.sh

@@ -0,0 +1,37 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Finn Arne Gangstad
+#
+
+test_description='git ls-files --others with escaped excludes
+
+This test tests exclusion patterns with \ in them and makes sure they
+are treated correctly and identically both for normal and wildcard rules.
+'
+
+. ./test-lib.sh
+
+touch \#ignore1 &&
+touch \#ignore2 &&
+touch \#hidden &&
+touch keep
+
+echo keep > expect
+
+cat >.gitignore <<EOF
+.gitignore
+expect
+output
+\#ignore1
+\#ignore2*
+\#hid*n
+EOF
+
+test_expect_success \
+    'git ls-files --others with escaped excludes.' \
+    'git ls-files --others \
+       --exclude-per-directory=.gitignore \
+       >output &&
+     test_cmp expect output'
+
+test_done

-- 
1.6.2.rc0.11.g665ed

Hi,

On Tue, 10 Feb 2009, Finn Arne Gangstad wrote:

"\" was treated differently in exclude rules depending on whether a
wildcard match was done. For wildcard rules, "\" was de-escaped in
fnmatch, but this was not done for other rules since they used strcmp
instead.  A file named "#foo" would not be excluded by "\#foo", but would
be excluded by "\#foo*".

We now treat all rules with "\" as wildcard rules.

Another solution could be to de-escape all non-wildcard rules as we
read them, but we would have to do the de-escaping exactly as fnmatch
does it to avoid inconsistencies.

Signed-off-by: Finn Arne Gangstad <redacted>
---
 dir.c                                       |    2 +-
 t/t3003-ls-files-others-escaped-excludes.sh |   37 +++++++++++++++++++++++++++

Thanks, much appreciated!

In other tests, we avoid 'touch' (IIRC it is not available everywhere or 
some such), and we write ': > \#ignore1' instead.

BTW we do not need the # in the name, it could be any letter, right?  
(Just for my understanding, not as a request to change it.)

+touch \#ignore2 &&
+touch \#hidden &&
+touch keep
+
+echo keep > expect
+
+cat >.gitignore <<EOF

You probably want to use \EOF here.

+.gitignore
+expect
+output
+\#ignore1
+\#ignore2*
+\#hid*n
+EOF
+
+test_expect_success \
+    'git ls-files --others with escaped excludes.' \
+    'git ls-files --others \
+       --exclude-per-directory=.gitignore \
+       >output &&
+     test_cmp expect output'
+
+test_done

Thanks!
Dscho

On Tue, Feb 10, 2009 at 03:27:43PM +0100, Johannes Schindelin wrote:

quoted

+
+. ./test-lib.sh
+
+touch \#ignore1 &&

In other tests, we avoid 'touch' (IIRC it is not available everywhere or 
some such), and we write ': > \#ignore1' instead.

Will fix it up after seeing if there are some other comments.

BTW we do not need the # in the name, it could be any letter, right?  
(Just for my understanding, not as a request to change it.)

So far I have only had to use \ in exclusion rules starting with #,
since they would otherwise be interpreted as comments, but it could be
any character that \ would not change the meaning of. I am not sure
what fnmatch would do with \t, \r and \n for example.

quoted

+cat >.gitignore <<EOF

You probably want to use \EOF here.

I am curious, does it matter? Most of the tests use EOF and not \EOF.

- Finn Arne