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 <finnag@pvv.org>
---
 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

--

[ message continues ]

Hi,


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

[ message continues ]

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

- Finn Arne
--

[ message continues ]

Hi,


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

Thanks,
Dscho

--

[ message continues ]

"\" 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 <finnag@pvv.org>
---
 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 ...
[ message continues ]

Hi,



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?  


Thanks!
Dscho

--

[ message continues ]

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

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

- Finn Arne
--

[ message continues ]

If you want the same shell variable expansion and quoting rules as you get
inside double-quote pair, you would say <<EOF without any quotes.  If you
quote the EOF, no such substitutions happen.

In this particular case, you want what you typed there literally in the
file, so <<\EOF would be more correct, even though \# expands to \#
itself.

IOW, your current list of patterns does not happen to have anything like
$var nor \\ that would make a difference, but to protect future breakages
by people adding more patterns there, it is better to say <<\EOF when you
know you are not asking for any funny expansion to be explicit.

--

[ message continues ]

Oh, by the way, do we really want to add a new test script?  I am
wondering why the test is not an update to an existing test for the
exclusion feature, such as t/t3001-ls-files-others-exclude.sh

--

[ message continues ]

I really did not want to add a new test script, but extending 3001
with this test seemed to make the test slightly more opaque. I can try
adding it to 3001 and see what it ends up looking like.

- Finn Arne
--

[ message continues ]

Ok, here is the final version with your suggested test-modification,
which seems to to the trick!

- Finn Arne

--8<--
Support "\" in non-wildcard exclusion entries

"\" 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 <finnag@pvv.org>
---
 dir.c                              |    2 +-
 t/t3001-ls-files-others-exclude.sh |    7 +++++++
 2 files changed, 8 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,
diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
index 85aef12..9be9557 100755
--- a/t/t3001-ls-files-others-exclude.sh
+++ b/t/t3001-ls-files-others-exclude.sh
@@ -19,6 +19,9 @@ do
     >$dir/a.$i
   done
 done
+>"#ignore1"
+>"#ignore2"
+>"#hidden"
 
 cat >expect <<EOF
 a.2
@@ -42,6 +45,9 @@ three/a.8
 EOF
 
 echo '.gitignore
+\#ignore1
+\#ignore2*
+\#hid*n
 output
 expect
 .gitignore
@@ -82,6 +88,7 @@ test_expect_success \
 cat > excludes-file << EOF
 *.[1-8]
 e*
+\#*
 EOF
 
 git config core.excludesFile excludes-file
-- 
1.6.2.rc0.11.g665ed

--

[ message continues ]

Hi,


You addressed all comments, except the \EOF comment, I guess.

Thanks,
Dscho

--

[ message continues ]