Re: [PATCH 8/8] Support ** wildcard in .gitignore and .gitattributes

2012-10-09 Thread Nguyen Thai Ngoc Duy
On Tue, Oct 9, 2012 at 2:57 PM, Michael Haggerty mhag...@alum.mit.edu wrote:
 + - A trailing /** matches everything inside. For example,
 +   abc/** is equivalent to `/abc/`.

 It seems odd that you add a leading slash in this example.  I assume
 that is because of the rule that a pattern containing a slash is
 considered anchored at the current directory. But I find it confusing
 because the addition of the leading slash is not part of the rule you
 are trying to illustrate here, and is therefore a distraction.  I
 suggest that you write either

 - A trailing /** matches everything inside. For example,
   /abc/** is equivalent to `/abc/`.

 or

 - A trailing /** matches everything inside. For example,
   abc/** is equivalent to `abc/` (which is also equivalent
   to `/abc/`).

The tricky thing in .gitignore is that the last '/' alone does not
imply anchor. So abc/ means match _directory_ abc anywhere in
worktree. So the former is probably better. I should also add a note
here (or in gitattributes.txt) about the difference between /abc/*
and /abc/**. The former assigns attributes to all files directly
under abc (e.g. depth 1), the latter infinite depth.

 + - A slash followed by two consecutive asterisks then a slash
 +   matches zero or more directories. For example, `a/**/b`
 +   matches `a/b`, `a/x/b`, `a/x/y/b` and so on.
 +
 + - Consecutive asterisks otherwise are treated like normal
 +   asterisk wildcards.
 +

 I don't like the last rule.  (1) This construct is superfluous; why
 wouldn't the user just use a single asterisk?  (2) Allowing this
 construct means that it could appear in .gitignore files, creating
 unnecessary confusion: extrapolating from the other meanings of **
 users would expect that it would somehow match slashes.  (3) It is
 conceivable (though admittedly unlikely) that we might want to assign a
 distinct meaning to this construct in the future, and accepting it now
 as a different way to spell * would prevent such a change.

 Perhaps this rule was intended for backwards compatibility?

We break backwards compatibility already. Existing **/ or /**
patterns now behave differently.

 I think it would be preferable to say that other uses of consecutive
 asterisks are undefined, and probably make them trigger a warning.

Instead of undefined, we can reject the pattern as broken. I have to
check how fnmatch/wildmatch deals with broken patterns (it must do).
If it returns a different code for broken patterns, then we can warn
users, which is not limited in just ** breakage.
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 8/8] Support ** wildcard in .gitignore and .gitattributes

2012-10-08 Thread Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/gitignore.txt| 19 +++
 attr.c |  4 +++-
 dir.c  |  4 +++-
 t/t0003-attributes.sh  | 38 ++
 t/t3001-ls-files-others-exclude.sh | 19 +++
 5 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index 96639e0..5a9c9f7 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -104,6 +104,25 @@ PATTERN FORMAT
For example, /{asterisk}.c matches cat-file.c but not
mozilla-sha1/sha1.c.
 
+Two consecutive asterisks (`**`) in patterns matched against
+full pathname may have special meaning:
+
+ - A leading `**` followed by a slash means match in all
+   directories. For example, `**/foo` matches file or directory
+   `foo` anywhere, the same as pattern `foo`. **/foo/bar
+   matches file or directory `bar` anywhere that is directly
+   under directory `foo`.
+
+ - A trailing /** matches everything inside. For example,
+   abc/** is equivalent to `/abc/`.
+
+ - A slash followed by two consecutive asterisks then a slash
+   matches zero or more directories. For example, `a/**/b`
+   matches `a/b`, `a/x/b`, `a/x/y/b` and so on.
+
+ - Consecutive asterisks otherwise are treated like normal
+   asterisk wildcards.
+
 NOTES
 -
 
diff --git a/attr.c b/attr.c
index 887a9ae..e85e5ed 100644
--- a/attr.c
+++ b/attr.c
@@ -12,6 +12,7 @@
 #include exec_cmd.h
 #include attr.h
 #include dir.h
+#include wildmatch.h
 
 const char git_attr__true[] = (builtin)true;
 const char git_attr__false[] = \0(builtin)false;
@@ -666,7 +667,8 @@ static int path_matches(const char *pathname, int pathlen,
return 0;
if (baselen != 0)
baselen++;
-   return fnmatch_icase(pattern, pathname + baselen, FNM_PATHNAME) == 0;
+   return wildmatch(pattern, pathname + baselen,
+ignore_case ? FNM_CASEFOLD : 0);
 }
 
 static int macroexpand_one(int attr_nr, int rem);
diff --git a/dir.c b/dir.c
index 4868339..dc721c0 100644
--- a/dir.c
+++ b/dir.c
@@ -8,6 +8,7 @@
 #include cache.h
 #include dir.h
 #include refs.h
+#include wildmatch.h
 
 struct path_simplify {
int len;
@@ -575,7 +576,8 @@ int excluded_from_list(const char *pathname,
namelen -= prefix;
}
 
-   if (!namelen || !fnmatch_icase(exclude, name, FNM_PATHNAME))
+   if (!namelen ||
+   wildmatch(exclude, name, ignore_case ? FNM_CASEFOLD : 0))
return to_exclude;
}
return -1; /* undecided */
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index febc45c..67a5694 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -232,4 +232,42 @@ test_expect_success 'bare repository: test 
info/attributes' '
attr_check subdir/a/i unspecified
 '
 
+test_expect_success '** test' '
+   cd .. 
+   echo **/f foo=bar .gitattributes 
+   cat \EOF expect 
+f: foo: bar
+a/f: foo: bar
+a/b/f: foo: bar
+a/b/c/f: foo: bar
+EOF
+   git check-attr foo -- f actual 2err 
+   git check-attr foo -- a/f actual 2err 
+   git check-attr foo -- a/b/f actual 2err 
+   git check-attr foo -- a/b/c/f actual 2err 
+   test_cmp expect actual 
+   test_line_count = 0 err
+'
+
+test_expect_success '** with no slashes test' '
+   echo a**f foo=bar .gitattributes 
+   git check-attr foo -- f actual 
+   cat \EOF expect 
+f: foo: unspecified
+af: foo: bar
+axf: foo: bar
+a/f: foo: unspecified
+a/b/f: foo: unspecified
+a/b/c/f: foo: unspecified
+EOF
+   git check-attr foo -- f actual 2err 
+   git check-attr foo -- af actual 2err 
+   git check-attr foo -- axf actual 2err 
+   git check-attr foo -- a/f actual 2err 
+   git check-attr foo -- a/b/f actual 2err 
+   git check-attr foo -- a/b/c/f actual 2err 
+   test_cmp expect actual 
+   test_line_count = 0 err
+'
+
 test_done
diff --git a/t/t3001-ls-files-others-exclude.sh 
b/t/t3001-ls-files-others-exclude.sh
index c8fe978..278315d 100755
--- a/t/t3001-ls-files-others-exclude.sh
+++ b/t/t3001-ls-files-others-exclude.sh
@@ -214,4 +214,23 @@ test_expect_success 'subdirectory ignore (l1)' '
test_cmp expect actual
 '
 
+
+test_expect_success 'ls-files with ** patterns' '
+   cat \EOF expect 
+a.1
+one/a.1
+one/two/a.1
+three/a.1
+EOF
+   git ls-files -o -i --exclude **/a.1 actual
+   test_cmp expect actual
+'
+
+
+test_expect_success 'ls-files with ** patterns and no slashes' '
+   : expect 
+   git ls-files -o -i --exclude one**a.1 actual 
+   test_cmp expect actual
+'
+
 test_done
-- 
1.8.0.rc0.29.g1fdd78f

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at