* Re: [PATCH] git-completion.bash: add support for path completion
From: Junio C Hamano @ 2012-12-19 22:49 UTC (permalink / raw)
To: Manlio Perillo; +Cc: git
In-Reply-To: <50D23960.4070108@gmail.com>
Manlio Perillo <manlio.perillo@gmail.com> writes:
>> git mv COPYING README X
>
> Assuming X is a new untracked directory, do you think it is an usability
> problem if an user try to do:
>
> git mv COPYING README <TAB>
>
> and X does not appear in the completion list?
It is hard to say. Will it show "Documentation/" in the list? Will
it show "git.c" in the list?
Your "git mv git.<TAB>" completing to "git mv git.c" would be an
improvement compared to the stock "less git.<TAB>" that offers
"git.c" and "git.o" as choices. For things like "mv" and "rm", this
may not make too much of a difference, "git add <TAB>" would be a
vast improvement from the stock one. The users will notice that the
completion knows what it is doing, and will come to depend on it.
But at that point, if "git mv COPYING README <TAB>" offers only
directories that contain tracked files, the users may get irritated,
because it is likely that the destination directory was created
immediately before the user started typing "git mv". You will hear
"Surely I created X, it is there, why aren't you showing it to me?"
The updated completion knows what it is doing everywhere else, and
it sets the user-expectation at that level. Uneven cleverness will
stand out like a sore thumb and hurts the user perception, which is
arguably unfair, but nothing in life is fair X-<.
I think over-showing the choices is much better than hiding some
choices, if we cannot do a perfect completion in some cases. "You
should know that I won't be moving these files in Y, as I already
marked it to be ignored in the .gitignore file!" is less grave a
gripe than "You know I created X just a minute ago, and it is there,
why aren't you showing it to me?" because you can simply say "but Y
is there as a directory." admitting that you are less clever than
the user expects you to be, but at least you are giving the choice
to the user of not picking it.
In the ideal world (read: I am *not* saying "you should implement it
this way, or we won't look at your patch"), I think you would want
to show tracked files (because it may be the third path that the
user wants to move with the command, not the destination directory)
and any directory on the filesystem (it could be the third path that
is being moved if it has tracked files, or it could be the final
destination directory argument).
^ permalink raw reply
* Re: [PATCH] add GIT_PATHSPEC_GLOB environment variable
From: Jeff King @ 2012-12-19 22:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20121219222035.GA22998@sigill.intra.peff.net>
On Wed, Dec 19, 2012 at 05:20:35PM -0500, Jeff King wrote:
> > > Do we want to change the variable name and invert the logic?
> >
> > That would be my preference.
> > [...]
> That's fine. I'll send out a revised version, and you can pick it up
> later.
Here it is.
-- >8 --
Subject: [PATCH] add global --literal-pathspecs option
Git takes pathspec arguments in many places to limit the
scope of an operation. These pathspecs are treated not as
literal paths, but as glob patterns that can be fed to
fnmatch. When a user is giving a specific pattern, this is a
nice feature.
However, when programatically providing pathspecs, it can be
a nuisance. For example, to find the latest revision which
modified "$foo", one can use "git rev-list -- $foo". But if
"$foo" contains glob characters (e.g., "f*"), it will
erroneously match more entries than desired. The caller
needs to quote the characters in $foo, and even then, the
results may not be exactly the same as with a literal
pathspec. For instance, the depth checks in
match_pathspec_depth do not kick in if we match via fnmatch.
This patch introduces a global command-line option (i.e.,
one for "git" itself, not for specific commands) to turn
this behavior off. It also has a matching environment
variable, which can make it easier if you are a script or
porcelain interface that is going to issue many such
commands.
This option cannot turn off globbing for particular
pathspecs. That could eventually be done with a ":(noglob)"
magic pathspec prefix. However, that level of granularity is
more cumbersome to use for many cases, and doing ":(noglob)"
right would mean converting the whole codebase to use
"struct pathspec", as the usual "const char **pathspec"
cannot represent extra per-item flags.
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/git.txt | 15 +++++++++++
cache.h | 3 +++
dir.c | 25 ++++++++++++++-----
git.c | 8 ++++++
t/t6130-pathspec-noglob.sh | 62 ++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 107 insertions(+), 6 deletions(-)
create mode 100755 t/t6130-pathspec-noglob.sh
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 1d797f2..8d869db 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -422,6 +422,11 @@ help ...`.
Do not use replacement refs to replace git objects. See
linkgit:git-replace[1] for more information.
+--literal-pathspecs::
+ Treat pathspecs literally, rather than as glob patterns. This is
+ equivalent to setting the `GIT_LITERAL_PATHSPECS` environment
+ variable to `1`.
+
GIT COMMANDS
------------
@@ -790,6 +795,16 @@ for further details.
as a file path and will try to write the trace messages
into it.
+GIT_LITERAL_PATHSPECS::
+ Setting this variable to `1` will cause git to treat all
+ pathspecs literally, rather than as glob patterns. For example,
+ running `GIT_LITERAL_PATHSPECS=1 git log -- '*.c'` will search
+ for commits that touch the path `*.c`, not any paths that the
+ glob `*.c` matches. You might want this if you are feeding
+ literal paths to git (e.g., paths previously given to you by
+ `git ls-tree`, `--raw` diff output, etc).
+
+
Discussion[[Discussion]]
------------------------
diff --git a/cache.h b/cache.h
index 18fdd18..95608d9 100644
--- a/cache.h
+++ b/cache.h
@@ -362,6 +362,7 @@ static inline enum object_type object_type(unsigned int mode)
#define GIT_NOTES_DISPLAY_REF_ENVIRONMENT "GIT_NOTES_DISPLAY_REF"
#define GIT_NOTES_REWRITE_REF_ENVIRONMENT "GIT_NOTES_REWRITE_REF"
#define GIT_NOTES_REWRITE_MODE_ENVIRONMENT "GIT_NOTES_REWRITE_MODE"
+#define GIT_LITERAL_PATHSPECS_ENVIRONMENT "GIT_LITERAL_PATHSPECS"
/*
* Repository-local GIT_* environment variables
@@ -490,6 +491,8 @@ extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pa
extern void free_pathspec(struct pathspec *);
extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec);
+extern int limit_pathspec_to_literal(void);
+
#define HASH_WRITE_OBJECT 1
#define HASH_FORMAT_CHECK 2
extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum object_type type, const char *path, unsigned flags);
diff --git a/dir.c b/dir.c
index 5a83aa7..03ff36b 100644
--- a/dir.c
+++ b/dir.c
@@ -38,6 +38,7 @@ static size_t common_prefix_len(const char **pathspec)
{
const char *n, *first;
size_t max = 0;
+ int literal = limit_pathspec_to_literal();
if (!pathspec)
return max;
@@ -47,7 +48,7 @@ static size_t common_prefix_len(const char **pathspec)
size_t i, len = 0;
for (i = 0; first == n || i < max; i++) {
char c = n[i];
- if (!c || c != first[i] || is_glob_special(c))
+ if (!c || c != first[i] || (!literal && is_glob_special(c)))
break;
if (c == '/')
len = i + 1;
@@ -117,6 +118,7 @@ static int match_one(const char *match, const char *name, int namelen)
static int match_one(const char *match, const char *name, int namelen)
{
int matchlen;
+ int literal = limit_pathspec_to_literal();
/* If the match was just the prefix, we matched */
if (!*match)
@@ -126,7 +128,7 @@ static int match_one(const char *match, const char *name, int namelen)
for (;;) {
unsigned char c1 = tolower(*match);
unsigned char c2 = tolower(*name);
- if (c1 == '\0' || is_glob_special(c1))
+ if (c1 == '\0' || (!literal && is_glob_special(c1)))
break;
if (c1 != c2)
return 0;
@@ -138,7 +140,7 @@ static int match_one(const char *match, const char *name, int namelen)
for (;;) {
unsigned char c1 = *match;
unsigned char c2 = *name;
- if (c1 == '\0' || is_glob_special(c1))
+ if (c1 == '\0' || (!literal && is_glob_special(c1)))
break;
if (c1 != c2)
return 0;
@@ -148,14 +150,16 @@ static int match_one(const char *match, const char *name, int namelen)
}
}
-
/*
* If we don't match the matchstring exactly,
* we need to match by fnmatch
*/
matchlen = strlen(match);
- if (strncmp_icase(match, name, matchlen))
+ if (strncmp_icase(match, name, matchlen)) {
+ if (literal)
+ return 0;
return !fnmatch_icase(match, name, 0) ? MATCHED_FNMATCH : 0;
+ }
if (namelen == matchlen)
return MATCHED_EXACTLY;
@@ -1429,7 +1433,8 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
item->match = path;
item->len = strlen(path);
- item->use_wildcard = !no_wildcard(path);
+ item->use_wildcard = !limit_pathspec_to_literal() &&
+ !no_wildcard(path);
if (item->use_wildcard)
pathspec->has_wildcard = 1;
}
@@ -1445,3 +1450,11 @@ void free_pathspec(struct pathspec *pathspec)
free(pathspec->items);
pathspec->items = NULL;
}
+
+int limit_pathspec_to_literal(void)
+{
+ static int flag = -1;
+ if (flag < 0)
+ flag = git_env_bool(GIT_LITERAL_PATHSPECS_ENVIRONMENT, 0);
+ return flag;
+}
diff --git a/git.c b/git.c
index d33f9b3..ed66c66 100644
--- a/git.c
+++ b/git.c
@@ -135,6 +135,14 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
git_config_push_parameter((*argv)[1]);
(*argv)++;
(*argc)--;
+ } else if (!strcmp(cmd, "--literal-pathspecs")) {
+ setenv(GIT_LITERAL_PATHSPECS_ENVIRONMENT, "1", 1);
+ if (envchanged)
+ *envchanged = 1;
+ } else if (!strcmp(cmd, "--no-literal-pathspecs")) {
+ setenv(GIT_LITERAL_PATHSPECS_ENVIRONMENT, "0", 1);
+ if (envchanged)
+ *envchanged = 1;
} else {
fprintf(stderr, "Unknown option: %s\n", cmd);
usage(git_usage_string);
diff --git a/t/t6130-pathspec-noglob.sh b/t/t6130-pathspec-noglob.sh
new file mode 100755
index 0000000..bb5e710
--- /dev/null
+++ b/t/t6130-pathspec-noglob.sh
@@ -0,0 +1,62 @@
+#!/bin/sh
+
+test_description='test globbing (and noglob) of pathspec limiting'
+. ./test-lib.sh
+
+test_expect_success 'create commits with glob characters' '
+ test_commit unrelated bar &&
+ test_commit vanilla foo &&
+ test_commit star "f*" &&
+ test_commit bracket "f[o][o]"
+'
+
+test_expect_success 'vanilla pathspec matches literally' '
+ echo vanilla >expect &&
+ git log --format=%s -- foo >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'star pathspec globs' '
+ cat >expect <<-\EOF &&
+ bracket
+ star
+ vanilla
+ EOF
+ git log --format=%s -- "f*" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'bracket pathspec globs and matches literal brackets' '
+ cat >expect <<-\EOF &&
+ bracket
+ vanilla
+ EOF
+ git log --format=%s -- "f[o][o]" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'no-glob option matches literally (vanilla)' '
+ echo vanilla >expect &&
+ git --literal-pathspecs log --format=%s -- foo >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'no-glob option matches literally (star)' '
+ echo star >expect &&
+ git --literal-pathspecs log --format=%s -- "f*" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'no-glob option matches literally (bracket)' '
+ echo bracket >expect &&
+ git --literal-pathspecs log --format=%s -- "f[o][o]" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'no-glob environment variable works' '
+ echo star >expect &&
+ GIT_LITERAL_PATHSPECS=1 git log --format=%s -- "f*" >actual &&
+ test_cmp expect actual
+'
+
+test_done
--
1.8.1.rc2.24.gf3bad77
^ permalink raw reply related
* Re: [PATCH v2] Add directory pattern matching to attributes
From: Jean-Noël AVILA @ 2012-12-19 22:34 UTC (permalink / raw)
To: git
In-Reply-To: <7vbodp90o4.fsf@alter.siamese.dyndns.org>
Le mercredi 19 décembre 2012 22:44:59, vous avez écrit :
> "Jean-Noël AVILA" <avila.jn@gmail.com> writes:
> > This patch was not reviewed when I submitted it for the second time.
>
> Did you miss this?
>
>
Grml, I did. Sorry for the noise.
^ permalink raw reply
* Re: [PATCH] add GIT_PATHSPEC_GLOB environment variable
From: Jeff King @ 2012-12-19 22:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v38z18z6z.fsf@alter.siamese.dyndns.org>
On Wed, Dec 19, 2012 at 02:16:52PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> >> Will queue; thanks.
> >
> > Do we want to change the variable name and invert the logic?
>
> That would be my preference.
>
> I am deep into today's integration cycle, and this PATHSPEC_GLOB
> version is sitting at the tip of 'pu', so today's pushout will
> contain that version, though.
That's fine. I'll send out a revised version, and you can pick it up
later.
-Peff
^ permalink raw reply
* Re: [PATCH] add GIT_PATHSPEC_GLOB environment variable
From: Junio C Hamano @ 2012-12-19 22:16 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20121219221250.GA22823@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> Will queue; thanks.
>
> Do we want to change the variable name and invert the logic?
That would be my preference.
I am deep into today's integration cycle, and this PATHSPEC_GLOB
version is sitting at the tip of 'pu', so today's pushout will
contain that version, though.
^ permalink raw reply
* Re: [PATCH] add GIT_PATHSPEC_GLOB environment variable
From: Jeff King @ 2012-12-19 22:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7god8zz0.fsf@alter.siamese.dyndns.org>
On Wed, Dec 19, 2012 at 02:00:03PM -0800, Junio C Hamano wrote:
> > Subject: add GIT_PATHSPEC_GLOB environment variable
>
> Seems cleanly done from a quick look.
>
> Given that the normal mode of operation is to use globbing, I
> suspect that the names would have been more natural if the toggle
> were GIT_PATHSPEC_LITERAL and the boolean function were
> limit_pathspec_to_literal(), instead of "allow_pathspec_glob()",
> sounding as if using glob is done only upon request.
>
> But that is a minor issue.
Yeah, I was trying to avoid the double-negation of "noglob=false" for
the default behavior. I guess calling it literal is another way of
accomplishing that, and it keeps the default at "false". I don't have a
strong preference.
> > This patch introduces an environment variable to turn all
> > pathspecs into literal strings. This makes it easy to turn
> > off the globbing behavior for a whole environment (e.g., if
> > you are serving repos via a web interface that is only going
> > to use literal programmatic pathspecs), or for a particular
> > run.
>
> I am not sure if "web interface" is a particularly good example,
> though. Is it unusual to imagine a Web UI that takes pathspecs from
> the user to limit its output (e.g. "diff" or "ls-tree") to those
> paths that match them? In such a case, the user would expect their
> pathspecs to work the same way as the Git installed on their
> desktop, I would think.
Yes. If you want to provide for user-provided globbing pathspecs, then
you'd have to annotate each invocation with whether you want globbing or
not. What I was trying to illustrate was more how "gitweb" will let you
click on the "history" link for "foo" in a tree listing, resulting in a
page that is generated by calling "git rev-list foo". You would probably
not want pathspec globbing there.
> Will queue; thanks.
Do we want to change the variable name and invert the logic? Now is
probably the best time to do it.
-Peff
^ permalink raw reply
* Re: [PATCH] git-completion.bash: add support for path completion
From: Manlio Perillo @ 2012-12-19 22:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzk1clb3n.fsf@alter.siamese.dyndns.org>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 17/12/2012 20:42, Junio C Hamano ha scritto:
> [...]
>>> I am not sure how you would handle the last parameter to "git mv",
>>> though. That is by definition a path that does not exist,
>>> i.e. cannot be completed.
>>
>> Right, the code should be changed.
>> No completion should be done for the second parameter.
>
> I deliberately wrote "the last" not "the second", as you can do
>
> $ mkdir X
> $ git mv COPYING README X/.
>
> You do need to expand the second parameter to README when the user
> types
>
> git mv COPYING REAMDE X
>
> then goes back with \C-b to "M", types \C-d three times to remove
> "MDE", and then finally says <TAB>, to result in
>
> git mv COPYING README X
>
Assuming X is a new untracked directory, do you think it is an usability
problem if an user try to do:
git mv COPYING README <TAB>
and X does not appear in the completion list?
As far as I know, the solution is to only support custom expansion the
first parameter, unless the user will do something like:
git mv COPYING README -- <TAB>
Regards Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDSOWAACgkQscQJ24LbaUSOnACfds93RtX1CDOeGbwCGM5/N8HI
yVwAn0AZEO6rE083gKgFimGIbiRTyN5Z
=z7K5
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [PATCH] add GIT_PATHSPEC_GLOB environment variable
From: Junio C Hamano @ 2012-12-19 22:00 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20121219215008.GA17908@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I included the common_prefix fix you mentioned (I do not think it
> produced incorrect results as it was, but it did not take full advantage
> of an optimization).
I do not think it would have affected the outcome; you would only
have worked with more cycles.
> Subject: add GIT_PATHSPEC_GLOB environment variable
Seems cleanly done from a quick look.
Given that the normal mode of operation is to use globbing, I
suspect that the names would have been more natural if the toggle
were GIT_PATHSPEC_LITERAL and the boolean function were
limit_pathspec_to_literal(), instead of "allow_pathspec_glob()",
sounding as if using glob is done only upon request.
But that is a minor issue.
> This patch introduces an environment variable to turn all
> pathspecs into literal strings. This makes it easy to turn
> off the globbing behavior for a whole environment (e.g., if
> you are serving repos via a web interface that is only going
> to use literal programmatic pathspecs), or for a particular
> run.
I am not sure if "web interface" is a particularly good example,
though. Is it unusual to imagine a Web UI that takes pathspecs from
the user to limit its output (e.g. "diff" or "ls-tree") to those
paths that match them? In such a case, the user would expect their
pathspecs to work the same way as the Git installed on their
desktop, I would think.
Will queue; thanks.
^ permalink raw reply
* Re: [PATCH v3] git-completion.bash: add support for path completion
From: Manlio Perillo @ 2012-12-19 21:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, SZEDER Gábor, Felipe Contreras
In-Reply-To: <7vzk1995mx.fsf@alter.siamese.dyndns.org>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Il 19/12/2012 20:57, Junio C Hamano ha scritto:
> [jch: again, adding area experts to Cc]
>
> Manlio Perillo <manlio.perillo@gmail.com> writes:
>
>> Changes from version 2:
>>
>> * Perl is no more used.
>> * Fixed some coding style issues.
>> * Refactorized code, to improve future path completion support for
>> the "git reset" command.
>
> Thanks. Will replace what was queued on 'pu'.
>
>> +# Process path list returned by "ls-files" and "diff-index --name-only"
>> +# commands, in order to list only file names relative to a specified
>> +# directory, and append a slash to directory names.
>> +# It accepts 1 optional argument: a directory path. The path must have
>> +# a trailing slash.
>
> The callsites that call this function, and the way the argument is
> used, do not make it look like it is an optional argument to me.
>
> After reading later parts of the patch, I think the callers are
> wrong (see below) and you did intend to make "$1" optional.
>
Thanks for the corrections.
As you can see, I'm not very expert in bash programming.
I will review the code to use proper escaping and correct optional
parameters handling, based on your review.
In this case, I incorrectly assumed that bash expands ${1} to an empty
string, in case no arguments are passed to the function.
>> +__git_index_file_list_filter ()
>> +{
>> + local path
>> +
>> + while read -r path; do
>> + path=${path#$1}
>
> This will work correctly only if $1 does not have any shell
> metacharacter that removes prefix of $path that matches $1 as a
> pathname expansion pattern. As this file is for bash completion,
> using string-oriented Bash-isms like ${#1} (to get the length of the
> prefix) and ${path:$offset} (to get substring) are perfectly fine
> way to correct it.
>
Ok.
> Also, as $1 is optional, won't this barf if it is run under "set -u"?
>
Ok.
Here I should use ${1-}.
>> +# __git_index_files accepts 1 or 2 arguments:
>> +# 1: A string for file index status mode ("c", "m", "d", "o"), as
>> +# supported by git ls-files command.
>> +# 2: A directory path (optional).
>> +# If provided, only files within the specified directory are listed.
>> +# Sub directories are never recursed. Path must have a trailing
>> +# slash.
>> +__git_index_files ()
>> +{
>> + local dir="$(__gitdir)"
>> +
>> + if [ -d "$dir" ]; then
>> + git --git-dir="$dir" ls-files --exclude-standard "-${1}" "${2}" |
>> + __git_index_file_list_filter ${2} | uniq
>
> Even though everywhere else you seem to quote the variables with dq,
> but this ${2} is not written as "${2}", which looks odd. Deliberate?
>
No, I simply missed it.
> If you wanted to call __git_index_file_list_filter without parameter
> when the caller did not give you the optional $2, then the above is
> not the way to write it. It would be ${2+"$2"}.
Yes, this seems the better solution.
> [...]
>> +# __git_diff_index_files accepts 1 or 2 arguments:
>> +# 1) The id of a tree object.
>> +# 2) A directory path (optional).
>> +# If provided, only files within the specified directory are listed.
>> +# Sub directories are never recursed. Path must have a trailing
>> +# slash.
>> +__git_diff_index_files ()
>> +{
>> + local dir="$(__gitdir)"
>> +
>> + if [ -d "$dir" ]; then
>> + git --git-dir="$dir" diff-index --name-only "${1}" |
>> + __git_index_file_list_filter "${2}" | uniq
>
> This one, when the optional $2 is absent, will call __git_index_file_list_filter
> with an empty string in its "$1". Intended, or is it also ${2+"$2"}?
No, it was not intended. But here it should probably be ${2-}.
One possible rule is:
* ${n+"$n"} should be used by the _git_complete_xxx_file function;
* ${n-} should be used by the _git_xxx_file functions
The alternative is for each function to use ${n-}, or {n+"$n"}.
But I'm not sure. What is the best practice in bash for optional
parameters "propagation"?
>
>> +# __git_complete_index_file requires 1 argument: the file index
>> +# status mode
>> +__git_complete_index_file ()
>> +{
>> + local pfx cur_="$cur"
>> +
>> + case "$cur_" in
>> + ?*/*)
>> + pfx="${cur_%/*}"
>> + cur_="${cur_##*/}"
>> + pfx="${pfx}/"
>> +
>> + __gitcomp_nl "$(__git_index_files ${1} ${pfx})" "$pfx" "$cur_" ""
>> + ;;
>> + *)
>> + __gitcomp_nl "$(__git_index_files ${1})" "" "$cur_" ""
>> + ;;
>> + esac
>
> Please dedent the case arms by one level, i.e.
>
I missed it.
Vim do not intent correctly this, and I forgot to dedent.
> [...]
>> +# __git_complete_diff_index_file requires 1 argument: the id of a tree
>> +# object
>> +__git_complete_diff_index_file ()
>> +{
>> + local pfx cur_="$cur"
>> +
>> + case "$cur_" in
>> + ?*/*)
>> + pfx="${cur_%/*}"
>> + cur_="${cur_##*/}"
>> + pfx="${pfx}/"
>> +
>> + __gitcomp_nl "$(__git_diff_index_files ${1} ${pfx})" "$pfx" "$cur_" ""
>> + ;;
>> + *)
>> + __gitcomp_nl "$(__git_diff_index_files ${1})" "" "$cur_" ""
>> + ;;
>
> Unquoted $1 looks fishy, even if the only caller of this function
> always gives "HEAD" to it (in which case you can do without making
> it a parameter in the first place).
>
Currently it is always "HEAD", but in future it may contain an arbitrary
tree-ish (for my planned "git reset" path completion support).
This is the reason why in version 3 of the patch I added this new
__git_complete_diff_index_file function.
Better to quote it.
> Unquoted ${pfx} given to __git_diff_index_files also looks fishy.
I missed it.
Thanks Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlDSN30ACgkQscQJ24LbaUQPRQCgkQaDyBeXjk5gMJsufGoe9FCe
yDkAn2M4d1xRYSkF6P0lQQmENlbYiCb8
=iml7
-----END PGP SIGNATURE-----
^ permalink raw reply
* [PATCH] add GIT_PATHSPEC_GLOB environment variable
From: Jeff King @ 2012-12-19 21:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20121219210919.GA11894@sigill.intra.peff.net>
On Wed, Dec 19, 2012 at 04:09:19PM -0500, Jeff King wrote:
> It's perhaps a better match to make it an environment variable. Then it
> is tied to a particular flow of execution, rather than having it be a
> property of a system, user, or repo (which is what config does). So for
> my restricted environment, it would be sufficient for me to set the
> environment variable for the user who runs the scripts. And people who
> want a command-line option can set it via the shell, or we can provide
> "git --no-pathspec-glob" to set it.
Here it is as an environment variable. I think this probably makes more
sense in the general case (it's a little more work for my use case, but
I think the intended use is much more obvious).
I included the common_prefix fix you mentioned (I do not think it
produced incorrect results as it was, but it did not take full advantage
of an optimization).
-- >8 --
Subject: add GIT_PATHSPEC_GLOB environment variable
Git takes pathspec arguments in many places to limit the
scope of an operation. These pathspecs are treated not as
literal paths, but as glob patterns that can be fed to
fnmatch. When a user is giving a specific pattern, this is a
nice feature.
However, when programatically providing pathspecs, it can be
a nuisance. For example, to find the latest revision which
modified "$foo", one can use "git rev-list -- $foo". But if
"$foo" contains glob characters (e.g., "f*"), it will
erroneously match more entries than desired. The caller
needs to quote the characters in $foo, and even then, the
results may not be exactly the same as with a literal
pathspec. For instance, the depth checks in
match_pathspec_depth do not kick in if we match via fnmatch.
This patch introduces an environment variable to turn all
pathspecs into literal strings. This makes it easy to turn
off the globbing behavior for a whole environment (e.g., if
you are serving repos via a web interface that is only going
to use literal programmatic pathspecs), or for a particular
run.
It cannot turn off globbing for particular pathspecs. That
could eventually be done with a ":(noglob)" magic pathspec
prefix. However, that level of granularity is not often
needed, and doing ":(noglob)" right would mean converting
the whole codebase to use "struct pathspec", as the usual
"const char **pathspec" cannot represent extra per-item
flags.
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/git.txt | 15 +++++++++++
cache.h | 3 +++
dir.c | 24 +++++++++++++-----
git.c | 8 ++++++
t/t6130-pathspec-noglob.sh | 62 ++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 106 insertions(+), 6 deletions(-)
create mode 100755 t/t6130-pathspec-noglob.sh
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 1d797f2..7008b4d 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -422,6 +422,10 @@ help ...`.
Do not use replacement refs to replace git objects. See
linkgit:git-replace[1] for more information.
+--no-pathspec-glob::
+ Do not treat pathspecs as glob patterns. See the section on
+ the `GIT_PATHSPEC_GLOB` environment variable below.
+
GIT COMMANDS
------------
@@ -790,6 +794,17 @@ for further details.
as a file path and will try to write the trace messages
into it.
+GIT_PATHSPEC_GLOB::
+ Setting this variable to `0` will turn off the globbing features
+ of git's pathspecs. For example, running `GIT_PATHSPEC_GLOB=0 git
+ log -- '*.c'` will search for commits literally touching the
+ path `*.c`, not any paths that the glob `*.c` matches. You might
+ want this if you are feeding literal paths to git (e.g., paths
+ previously given to you by `git ls-tree`, `--raw` diff output,
+ etc). Setting it to `1` enables pathspec globbing (which is also
+ the default).
+
+
Discussion[[Discussion]]
------------------------
diff --git a/cache.h b/cache.h
index 18fdd18..98af77c 100644
--- a/cache.h
+++ b/cache.h
@@ -362,6 +362,7 @@ static inline enum object_type object_type(unsigned int mode)
#define GIT_NOTES_DISPLAY_REF_ENVIRONMENT "GIT_NOTES_DISPLAY_REF"
#define GIT_NOTES_REWRITE_REF_ENVIRONMENT "GIT_NOTES_REWRITE_REF"
#define GIT_NOTES_REWRITE_MODE_ENVIRONMENT "GIT_NOTES_REWRITE_MODE"
+#define GIT_PATHSPEC_GLOB_ENVIRONMENT "GIT_PATHSPEC_GLOB"
/*
* Repository-local GIT_* environment variables
@@ -490,6 +491,8 @@ extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pa
extern void free_pathspec(struct pathspec *);
extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec);
+extern int allow_pathspec_glob(void);
+
#define HASH_WRITE_OBJECT 1
#define HASH_FORMAT_CHECK 2
extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum object_type type, const char *path, unsigned flags);
diff --git a/dir.c b/dir.c
index 5a83aa7..a4d4321 100644
--- a/dir.c
+++ b/dir.c
@@ -38,6 +38,7 @@ static size_t common_prefix_len(const char **pathspec)
{
const char *n, *first;
size_t max = 0;
+ int glob = allow_pathspec_glob();
if (!pathspec)
return max;
@@ -47,7 +48,7 @@ static size_t common_prefix_len(const char **pathspec)
size_t i, len = 0;
for (i = 0; first == n || i < max; i++) {
char c = n[i];
- if (!c || c != first[i] || is_glob_special(c))
+ if (!c || c != first[i] || (glob && is_glob_special(c)))
break;
if (c == '/')
len = i + 1;
@@ -117,6 +118,7 @@ static int match_one(const char *match, const char *name, int namelen)
static int match_one(const char *match, const char *name, int namelen)
{
int matchlen;
+ int glob = allow_pathspec_glob();
/* If the match was just the prefix, we matched */
if (!*match)
@@ -126,7 +128,7 @@ static int match_one(const char *match, const char *name, int namelen)
for (;;) {
unsigned char c1 = tolower(*match);
unsigned char c2 = tolower(*name);
- if (c1 == '\0' || is_glob_special(c1))
+ if (c1 == '\0' || (glob && is_glob_special(c1)))
break;
if (c1 != c2)
return 0;
@@ -138,7 +140,7 @@ static int match_one(const char *match, const char *name, int namelen)
for (;;) {
unsigned char c1 = *match;
unsigned char c2 = *name;
- if (c1 == '\0' || is_glob_special(c1))
+ if (c1 == '\0' || (glob && is_glob_special(c1)))
break;
if (c1 != c2)
return 0;
@@ -148,14 +150,16 @@ static int match_one(const char *match, const char *name, int namelen)
}
}
-
/*
* If we don't match the matchstring exactly,
* we need to match by fnmatch
*/
matchlen = strlen(match);
- if (strncmp_icase(match, name, matchlen))
+ if (strncmp_icase(match, name, matchlen)) {
+ if (!glob)
+ return 0;
return !fnmatch_icase(match, name, 0) ? MATCHED_FNMATCH : 0;
+ }
if (namelen == matchlen)
return MATCHED_EXACTLY;
@@ -1429,7 +1433,7 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
item->match = path;
item->len = strlen(path);
- item->use_wildcard = !no_wildcard(path);
+ item->use_wildcard = allow_pathspec_glob() && !no_wildcard(path);
if (item->use_wildcard)
pathspec->has_wildcard = 1;
}
@@ -1445,3 +1449,11 @@ void free_pathspec(struct pathspec *pathspec)
free(pathspec->items);
pathspec->items = NULL;
}
+
+int allow_pathspec_glob(void)
+{
+ static int flag = -1;
+ if (flag < 0)
+ flag = git_env_bool(GIT_PATHSPEC_GLOB_ENVIRONMENT, 1);
+ return flag;
+}
diff --git a/git.c b/git.c
index d33f9b3..db0496f 100644
--- a/git.c
+++ b/git.c
@@ -135,6 +135,14 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
git_config_push_parameter((*argv)[1]);
(*argv)++;
(*argc)--;
+ } else if (!strcmp(cmd, "--pathspec-glob")) {
+ setenv(GIT_PATHSPEC_GLOB_ENVIRONMENT, "1", 1);
+ if (envchanged)
+ *envchanged = 1;
+ } else if (!strcmp(cmd, "--no-pathspec-glob")) {
+ setenv(GIT_PATHSPEC_GLOB_ENVIRONMENT, "0", 1);
+ if (envchanged)
+ *envchanged = 1;
} else {
fprintf(stderr, "Unknown option: %s\n", cmd);
usage(git_usage_string);
diff --git a/t/t6130-pathspec-noglob.sh b/t/t6130-pathspec-noglob.sh
new file mode 100755
index 0000000..835e730
--- /dev/null
+++ b/t/t6130-pathspec-noglob.sh
@@ -0,0 +1,62 @@
+#!/bin/sh
+
+test_description='test globbing (and noglob) of pathspec limiting'
+. ./test-lib.sh
+
+test_expect_success 'create commits with glob characters' '
+ test_commit unrelated bar &&
+ test_commit vanilla foo &&
+ test_commit star "f*" &&
+ test_commit bracket "f[o][o]"
+'
+
+test_expect_success 'vanilla pathspec matches literally' '
+ echo vanilla >expect &&
+ git log --format=%s -- foo >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'star pathspec globs' '
+ cat >expect <<-\EOF &&
+ bracket
+ star
+ vanilla
+ EOF
+ git log --format=%s -- "f*" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'bracket pathspec globs and matches literal brackets' '
+ cat >expect <<-\EOF &&
+ bracket
+ vanilla
+ EOF
+ git log --format=%s -- "f[o][o]" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'no-glob option matches literally (vanilla)' '
+ echo vanilla >expect &&
+ git --no-pathspec-glob log --format=%s -- foo >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'no-glob option matches literally (star)' '
+ echo star >expect &&
+ git --no-pathspec-glob log --format=%s -- "f*" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'no-glob option matches literally (bracket)' '
+ echo bracket >expect &&
+ git --no-pathspec-glob log --format=%s -- "f[o][o]" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'no-glob environment variable works' '
+ echo star >expect &&
+ GIT_PATHSPEC_GLOB=0 git log --format=%s -- "f*" >actual &&
+ test_cmp expect actual
+'
+
+test_done
--
1.8.1.rc2.24.gf3bad77
^ permalink raw reply related
* Re: [PATCH v2] Add directory pattern matching to attributes
From: Junio C Hamano @ 2012-12-19 21:44 UTC (permalink / raw)
To: Jean-Noël AVILA; +Cc: git
In-Reply-To: <201212192233.53002.avila.jn@gmail.com>
"Jean-Noël AVILA" <avila.jn@gmail.com> writes:
> This patch was not reviewed when I submitted it for the second time.
Did you miss this?
http://thread.gmane.org/gmane.comp.version-control.git/211214/focus=211470
^ permalink raw reply
* Re: [PATCH] add core.pathspecGlob config option
From: Jeff King @ 2012-12-19 21:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfw3191bi.fsf@alter.siamese.dyndns.org>
On Wed, Dec 19, 2012 at 01:30:57PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > diff --git a/dir.c b/dir.c
> > index 5a83aa7..6e81d4f 100644
> > --- a/dir.c
> > +++ b/dir.c
> > @@ -126,7 +126,7 @@ static int match_one(const char *match, const char *name, int namelen)
> > for (;;) {
> > unsigned char c1 = tolower(*match);
> > unsigned char c2 = tolower(*name);
> > - if (c1 == '\0' || is_glob_special(c1))
> > + if (c1 == '\0' || (core_pathspec_glob && is_glob_special(c1)))
> > break;
> > if (c1 != c2)
> > return 0;
>
> I think you can also do the same to the common_prefix(); we check
> for common leading directory prefix but punt upon seeing a directory
> component that has a glob character, and under the "literal" mode,
> it is not a special character.
Yeah, I think you're right. Will add to my re-roll.
-Peff
^ permalink raw reply
* [PATCH v2] Add directory pattern matching to attributes
From: Jean-Noël AVILA @ 2012-12-19 21:33 UTC (permalink / raw)
To: git
The manpage of gitattributes says: "The rules how the pattern
matches paths are the same as in .gitignore files" and the gitignore
pattern matching has a pattern ending with / for directory matching.
This rule is specifically relevant for the 'export-ignore' rule used
for git archive.
Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
---
This patch was not reviewed when I submitted it for the second time. Is there
any pending work which prevents it from being merged?
archive.c | 3 ++-
attr.c | 32 ++++++++++++++++------
t/t5002-archive-attr-pattern.sh | 57 +++++++++++++++++++++++++++++++++++++++
3 files changed, 83 insertions(+), 9 deletions(-)
create mode 100644 t/t5002-archive-attr-pattern.sh
diff --git a/archive.c b/archive.c
index 4666404..93e00bb 100644
--- a/archive.c
+++ b/archive.c
@@ -120,6 +120,8 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
strbuf_add(&path, args->base, args->baselen);
strbuf_add(&path, base, baselen);
strbuf_addstr(&path, filename);
+ if (S_ISDIR(mode) || S_ISGITLINK(mode))
+ strbuf_addch(&path, '/');
path_without_prefix = path.buf + args->baselen;
setup_archive_check(check);
@@ -130,7 +132,6 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
}
if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
- strbuf_addch(&path, '/');
if (args->verbose)
fprintf(stderr, "%.*s\n", (int)path.len, path.buf);
err = write_entry(args, sha1, path.buf, path.len, mode);
diff --git a/attr.c b/attr.c
index 097ae87..cdba88a 100644
--- a/attr.c
+++ b/attr.c
@@ -564,17 +564,31 @@ static void bootstrap_attr_stack(void)
attr_stack = elem;
}
+static const char *find_basename(const char *path)
+{
+ char pathbuf[PATH_MAX];
+ int pathlen;
+ const char *cp;
+
+ pathlen =strlen(path);
+ if (path[pathlen-1] != '/') {
+ cp =strrchr(path, '/');
+ return cp ? cp + 1: path;
+ } else {
+ strncpy(pathbuf, path, pathlen);
+ pathbuf[pathlen-1] = '\0';
+ cp =strrchr(pathbuf, '/');
+ return cp ? path + (cp - pathbuf) + 1 : path;
+ }
+}
+
static void prepare_attr_stack(const char *path)
{
struct attr_stack *elem, *info;
int dirlen, len;
const char *cp;
- cp = strrchr(path, '/');
- if (!cp)
- dirlen = 0;
- else
- dirlen = cp - path;
+ dirlen = find_basename(path) - path;
/*
* At the bottom of the attribute stack is the built-in
@@ -668,6 +682,10 @@ static int path_matches(const char *pathname, int pathlen,
const char *pattern = pat->pattern;
int prefix = pat->nowildcardlen;
+ if ((pat->flags & EXC_FLAG_MUSTBEDIR) &&
+ ((!pathlen) || (pathname[pathlen-1] != '/')))
+ return 0;
+
if (pat->flags & EXC_FLAG_NODIR) {
return match_basename(basename,
pathlen - (basename - pathname),
@@ -758,9 +776,7 @@ static void collect_all_attrs(const char *path)
for (i = 0; i < attr_nr; i++)
check_all_attr[i].value = ATTR__UNKNOWN;
- basename = strrchr(path, '/');
- basename = basename ? basename + 1 : path;
-
+ basename = find_basename(path);
pathlen = strlen(path);
rem = attr_nr;
for (stk = attr_stack; 0 < rem && stk; stk = stk->prev)
diff --git a/t/t5002-archive-attr-pattern.sh b/t/t5002-archive-attr-pattern.sh
new file mode 100644
index 0000000..0c847fb
--- /dev/null
+++ b/t/t5002-archive-attr-pattern.sh
@@ -0,0 +1,57 @@
+#!/bin/sh
+
+test_description='git archive attribute pattern tests'
+
+. ./test-lib.sh
+
+test_expect_exists() {
+ test_expect_success " $1 exists" "test -e $1"
+}
+
+test_expect_missing() {
+ test_expect_success " $1 does not exist" "test ! -e $1"
+}
+
+test_expect_success 'setup' '
+ echo ignored >ignored &&
+ echo ignored export-ignore >>.git/info/attributes &&
+ git add ignored &&
+
+ mkdir not-ignored-dir &&
+ echo ignored-in-tree >not-ignored-dir/ignored &&
+ echo not-ignored-in-tree >not-ignored-dir/ignored-only-if-dir &&
+ git add not-ignored-dir &&
+
+ mkdir ignored-only-if-dir &&
+ echo ignored by ignored dir >ignored-only-if-dir/ignored-by-ignored-dir &&
+ echo ignored-only-if-dir/ export-ignore >>.git/info/attributes &&
+ git add ignored-only-if-dir &&
+
+
+ mkdir -p one-level-lower/two-levels-lower/ignored-only-if-dir &&
+ echo ignored by ignored dir >one-level-lower/two-levels-lower/ignored-only-if-dir/ignored-by-ignored-dir &&
+ git add one-level-lower &&
+
+ git commit -m. &&
+
+ git clone --bare . bare &&
+ cp .git/info/attributes bare/info/attributes
+'
+
+test_expect_success 'git archive' '
+ git archive HEAD >archive.tar &&
+ (mkdir archive && cd archive && "$TAR" xf -) <archive.tar
+'
+
+test_expect_missing archive/ignored
+test_expect_missing archive/not-ignored-dir/ignored
+test_expect_exists archive/not-ignored-dir/ignored-only-if-dir
+test_expect_exists archive/not-ignored-dir/
+test_expect_missing archive/ignored-only-if-dir/
+test_expect_missing archive/ignored-ony-if-dir/ignored-by-ignored-dir
+test_expect_exists archive/one-level-lower/
+test_expect_missing archive/one-level-lower/two-levels-lower/ignored-only-if-dir/
+test_expect_missing archive/one-level-lower/two-levels-lower/ignored-ony-if-dir/ignored-by-ignored-dir
+
+
+test_done
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] add core.pathspecGlob config option
From: Junio C Hamano @ 2012-12-19 21:30 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20121219203449.GA10001@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> diff --git a/dir.c b/dir.c
> index 5a83aa7..6e81d4f 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -126,7 +126,7 @@ static int match_one(const char *match, const char *name, int namelen)
> for (;;) {
> unsigned char c1 = tolower(*match);
> unsigned char c2 = tolower(*name);
> - if (c1 == '\0' || is_glob_special(c1))
> + if (c1 == '\0' || (core_pathspec_glob && is_glob_special(c1)))
> break;
> if (c1 != c2)
> return 0;
I think you can also do the same to the common_prefix(); we check
for common leading directory prefix but punt upon seeing a directory
component that has a glob character, and under the "literal" mode,
it is not a special character.
^ permalink raw reply
* Re: [PATCH] add core.pathspecGlob config option
From: Jeff King @ 2012-12-19 21:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk3sd930z.fsf@alter.siamese.dyndns.org>
On Wed, Dec 19, 2012 at 12:54:04PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > ... doing ":(noglob)" right would mean converting
> > the whole codebase to use "struct pathspec", as the usual
> > "const char **pathspec" cannot represent extra per-item
> > flags.
>
> As that is the longer-term direction we would want to go, I'd rather
> not to take the approach in this patch for introducing user-facing
> support of literal pathspecs.
>
> Having said that, even when we have the ':(noglob)' magic pathspec
> support, it would make sense to introduce a command line option to
> make it easier for scripted Porcelains that call plumbing commands
> to pass literal pathspecs (i.e. they know exactly what paths they
> want to operate, not what patterns the paths they want to operate
> would match).
Right, that is my use case. Even once ":(noglob)" exists, it will still
be much simpler for me to use a single option, since I would never have
mixed patterns and literal paths (and I suspect most use cases that
would care about the distinction would be in the same boat). And that is
what led me to submit this at all, as it is not just "well, it is a hack
until :(noglob) works", but "this is not as fine a granularity as
:(noglob), but it better matches what callers want".
> I do not think configuration variable makes much sense (unless you are
> thinking "git -c core.literalpathspec=true" as that command line
> option).
The configuration variable is much more convenient for me, because
otherwise I have to instrument every call to git with a "--no-glob"
option. Because I have a very restricted environment, and happen to know
that globs will _never_ be useful on my repos (or, say, on commands run
by a particular user who has this in their ~/.gitconfig), it makes sense
to just turn off the feature with one switch.
And my thinking was that for people who are not in such a restricted
environment, "git -c core.pathspecglob=false" could serve as that
command-line option, as you mentioned.
It's perhaps a better match to make it an environment variable. Then it
is tied to a particular flow of execution, rather than having it be a
property of a system, user, or repo (which is what config does). So for
my restricted environment, it would be sufficient for me to set the
environment variable for the user who runs the scripts. And people who
want a command-line option can set it via the shell, or we can provide
"git --no-pathspec-glob" to set it.
-Peff
^ permalink raw reply
* Re: [PATCH] add core.pathspecGlob config option
From: Junio C Hamano @ 2012-12-19 20:54 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20121219203449.GA10001@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> ... doing ":(noglob)" right would mean converting
> the whole codebase to use "struct pathspec", as the usual
> "const char **pathspec" cannot represent extra per-item
> flags.
As that is the longer-term direction we would want to go, I'd rather
not to take the approach in this patch for introducing user-facing
support of literal pathspecs.
Having said that, even when we have the ':(noglob)' magic pathspec
support, it would make sense to introduce a command line option to
make it easier for scripted Porcelains that call plumbing commands
to pass literal pathspecs (i.e. they know exactly what paths they
want to operate, not what patterns the paths they want to operate
would match). I do not think configuration variable makes much
sense (unless you are thinking "git -c core.literalpathspec=true"
as that command line option).
Thanks
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2012, #05; Tue, 18)
From: Junio C Hamano @ 2012-12-19 20:47 UTC (permalink / raw)
To: git
In-Reply-To: <7v4njjf6fk.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Here are the topics that have been cooking. Commits prefixed with
> '-' are only in 'pu' (proposed updates) while commits prefixed with
> '+' are in 'next'.
>
> The tip of the 'master' branch is a bit past 1.8.1-rc2; hopefully we
> can go final around the end of the week.
>
> Many topics are getting into good shape in 'next', hopefully ready
> to be merged soon after the 1.8.1 final.
As we seem to be getting more minor documentation updates that are
safe for the upcoming release, and also because I've updated the
AsciiDoc toolchain used to prepare the preformatted manpage and HTML
tarballs, I changed my mind to delay the final til the end of the
year, and tag an rc3 toward the end of this week instead.
Both repositories http://code.google.com/p/git-htmldocs/ and
http://code.google.com/p/git-manpages/, and the browser-reachable
pages at http://git-htmldocs.googlecode.com/git/git.html should
start getting the output formatted with AsciiDoc 8.6.8.
The following documentation updates topics are likely to be in the
rc3:
as/doc-for-devs
cr/doc-checkout-branch
jc/doc-diff-blobs
jc/fetch-tags-doc
jk/avoid-mailto-invalid-in-doc
nd/index-format-doc
sl/git-svn-docs
sl/readme-gplv2
ta/api-index-doc
Also I am planning to have the .mailmap cleanup topic in the rc3:
jk/mailmap-cleanup
Thanks.
^ permalink raw reply
* [PATCH] add core.pathspecGlob config option
From: Jeff King @ 2012-12-19 20:34 UTC (permalink / raw)
To: git
Git takes pathspec arguments in many places to limit the
scope of an operation. These pathspecs are treated not as
literal paths, but as glob patterns that can be fed to
fnmatch. When a user is giving a specific pattern, this is a
nice feature.
However, when programatically providing pathspecs, it can be
a nuisance. For example, to find the latest revision which
modified "$foo", one can use "git rev-list -- $foo". But if
"$foo" contains glob characters (e.g., "f*"), it will
erroneously match more entries than desired. The caller
needs to quote the characters in $foo, and even then, the
results may not be exactly the same as with a literal
pathspec. For instance, the depth checks in
match_pathspec_depth do not kick in if we match via fnmatch.
This patch introduces a config option to turn all pathspecs
into literal strings. This makes it easy to turn off the
globbing behavior for a whole environment (e.g., if you are
serving repos via a web interface that is only going to
use literal programmatic pathspecs), or for a particular run
(by using "git -c" to set the option for this run).
It cannot turn off globbing for particular pathspecs. That
could eventually be done with a ":(noglob)" magic pathspec
prefix. However, that level of granularity is not often
needed, and doing ":(noglob)" right would mean converting
the whole codebase to use "struct pathspec", as the usual
"const char **pathspec" cannot represent extra per-item
flags.
Signed-off-by: Jeff King <peff@peff.net>
---
Part of me thinks this is just gross, because ":(noglob)" is the right
solution. But after spending a few hours trying it this morning, there
is a ton of refactoring required to make it work correctly everywhere
(although we could die() if we see it in places that are not using
init_pathspec, so at least we could say "not supported yet" and not just
silently ignore it).
Still, this is so easy to do, and the lack of granularity does not hurt
my use cases at all (which, if you have not guessed, are improving
corner cases in scripted calls on GitHub). And I do not think it is just
me, or just GitHub. Any server-side porcelain would be better off with
such an option (for example, I think gitweb has the same issues; it is
just that they are pretty rare, because most people do not put glob
metacharacters into their filenames).
So I think this is a nice, simple approach for sites that want it, and
noglob magic can come later (and will not be any harder to implement as
a result of this patch).
cache.h | 1 +
config.c | 5 +++++
dir.c | 12 +++++-----
environment.c | 1 +
t/t6130-pathspec-noglob.sh | 56 ++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 70 insertions(+), 5 deletions(-)
create mode 100755 t/t6130-pathspec-noglob.sh
diff --git a/cache.h b/cache.h
index 18fdd18..0725a33 100644
--- a/cache.h
+++ b/cache.h
@@ -555,6 +555,7 @@ extern int precomposed_unicode;
extern int core_preload_index;
extern int core_apply_sparse_checkout;
extern int precomposed_unicode;
+extern int core_pathspec_glob;
enum branch_track {
BRANCH_TRACK_UNSPECIFIED = -1,
diff --git a/config.c b/config.c
index fb3f868..8a96ba7 100644
--- a/config.c
+++ b/config.c
@@ -760,6 +760,11 @@ static int git_default_core_config(const char *var, const char *value)
return 0;
}
+ if (!strcmp(var, "core.pathspecglob")) {
+ core_pathspec_glob = git_config_bool(var, value);
+ return 0;
+ }
+
/* Add other config variables here and to Documentation/config.txt. */
return 0;
}
diff --git a/dir.c b/dir.c
index 5a83aa7..6e81d4f 100644
--- a/dir.c
+++ b/dir.c
@@ -126,7 +126,7 @@ static int match_one(const char *match, const char *name, int namelen)
for (;;) {
unsigned char c1 = tolower(*match);
unsigned char c2 = tolower(*name);
- if (c1 == '\0' || is_glob_special(c1))
+ if (c1 == '\0' || (core_pathspec_glob && is_glob_special(c1)))
break;
if (c1 != c2)
return 0;
@@ -138,7 +138,7 @@ static int match_one(const char *match, const char *name, int namelen)
for (;;) {
unsigned char c1 = *match;
unsigned char c2 = *name;
- if (c1 == '\0' || is_glob_special(c1))
+ if (c1 == '\0' || (core_pathspec_glob && is_glob_special(c1)))
break;
if (c1 != c2)
return 0;
@@ -148,14 +148,16 @@ static int match_one(const char *match, const char *name, int namelen)
}
}
-
/*
* If we don't match the matchstring exactly,
* we need to match by fnmatch
*/
matchlen = strlen(match);
- if (strncmp_icase(match, name, matchlen))
+ if (strncmp_icase(match, name, matchlen)) {
+ if (!core_pathspec_glob)
+ return 0;
return !fnmatch_icase(match, name, 0) ? MATCHED_FNMATCH : 0;
+ }
if (namelen == matchlen)
return MATCHED_EXACTLY;
@@ -1429,7 +1431,7 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
item->match = path;
item->len = strlen(path);
- item->use_wildcard = !no_wildcard(path);
+ item->use_wildcard = core_pathspec_glob && !no_wildcard(path);
if (item->use_wildcard)
pathspec->has_wildcard = 1;
}
diff --git a/environment.c b/environment.c
index 85edd7f..d0d30a0 100644
--- a/environment.c
+++ b/environment.c
@@ -59,6 +59,7 @@ int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
int core_apply_sparse_checkout;
int merge_log_config = -1;
int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
+int core_pathspec_glob = 1;
struct startup_info *startup_info;
unsigned long pack_size_limit_cfg;
diff --git a/t/t6130-pathspec-noglob.sh b/t/t6130-pathspec-noglob.sh
new file mode 100755
index 0000000..bb962ac
--- /dev/null
+++ b/t/t6130-pathspec-noglob.sh
@@ -0,0 +1,56 @@
+#!/bin/sh
+
+test_description='test globbing (and noglob) of pathspec limiting'
+. ./test-lib.sh
+
+test_expect_success 'create commits with glob characters' '
+ test_commit unrelated bar &&
+ test_commit vanilla foo &&
+ test_commit star "f*" &&
+ test_commit bracket "f[o][o]"
+'
+
+test_expect_success 'vanilla pathspec matches literally' '
+ echo vanilla >expect &&
+ git log --format=%s -- foo >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'star pathspec globs' '
+ cat >expect <<-\EOF &&
+ bracket
+ star
+ vanilla
+ EOF
+ git log --format=%s -- "f*" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'bracket pathspec globs and matches literal brackets' '
+ cat >expect <<-\EOF &&
+ bracket
+ vanilla
+ EOF
+ git log --format=%s -- "f[o][o]" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'no-glob config matches literally (vanilla)' '
+ echo vanilla >expect &&
+ git -c core.pathspecglob=false log --format=%s -- foo >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'no-glob config matches literally (star)' '
+ echo star >expect &&
+ git -c core.pathspecglob=false log --format=%s -- "f*" >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'no-glob config matches literally (bracket)' '
+ echo bracket >expect &&
+ git -c core.pathspecglob=false log --format=%s -- "f[o][o]" >actual &&
+ test_cmp expect actual
+'
+
+test_done
--
1.8.1.rc2.24.gf3bad77
^ permalink raw reply related
* Re: [BUG] Cannot push some grafted branches
From: Junio C Hamano @ 2012-12-19 20:07 UTC (permalink / raw)
To: Thomas Rast
Cc: Yann Dirson, Andreas Schwab, Christian Couder, git list,
Jeff King
In-Reply-To: <87ip7yp4mf.fsf@pctrast.inf.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> I still wouldn't recommend this approach in git-replace(1) for several
> reasons:
>
> * It does not generalize in any direction. For each field you may want
> to change, you have to know a _specific_ way of getting just the
> commit you want.
>
> * More to the point of replacing the parent lists, while the above might
> be expected of a slightly advanced git user, you get into deep magic
> the second you want to fake a merge commit with an arbitrary
> combination of parents. (No, you don't need to tell me how. I'm just
> saying that fooling with either MERGE_HEAD or read-tree is not for
> mere mortals.)
I do not buy either of the above. When you are replacing one with
something else, you ought to know what that something else is and
how to create it. Editing a text file with an editor to replace
40-hex object names with another is not a more intuitive way for end
users, either (in other words, you are seeing this from the point of
view of somebody who *knows* the internal representation of Git
objects too much).
> * The above potentially introduces clock skew into the repository, which
> can trigger bugs (like rev-list accidentally missing out on some side
> arm!) until we get around to implementing and using generation
> numbers.
That is an irrelevant point when comparing the "go down to bare
metal replacing the object representation" vs "use the usual Git
tools the end users are already familiar with" approaches. You will
encounter the issue you are raising if you make a newer commit a
parent of an existing child with an older commit timestamp, no
matter how you do the grafting.
^ permalink raw reply
* Re: [PATCH v3] git-completion.bash: add support for path completion
From: Junio C Hamano @ 2012-12-19 19:57 UTC (permalink / raw)
To: Manlio Perillo; +Cc: git, SZEDER Gábor, Felipe Contreras
In-Reply-To: <1355943496-5533-1-git-send-email-manlio.perillo@gmail.com>
[jch: again, adding area experts to Cc]
Manlio Perillo <manlio.perillo@gmail.com> writes:
> Changes from version 2:
>
> * Perl is no more used.
> * Fixed some coding style issues.
> * Refactorized code, to improve future path completion support for
> the "git reset" command.
Thanks. Will replace what was queued on 'pu'.
> +# Process path list returned by "ls-files" and "diff-index --name-only"
> +# commands, in order to list only file names relative to a specified
> +# directory, and append a slash to directory names.
> +# It accepts 1 optional argument: a directory path. The path must have
> +# a trailing slash.
The callsites that call this function, and the way the argument is
used, do not make it look like it is an optional argument to me.
After reading later parts of the patch, I think the callers are
wrong (see below) and you did intend to make "$1" optional.
> +__git_index_file_list_filter ()
> +{
> + local path
> +
> + while read -r path; do
> + path=${path#$1}
This will work correctly only if $1 does not have any shell
metacharacter that removes prefix of $path that matches $1 as a
pathname expansion pattern. As this file is for bash completion,
using string-oriented Bash-isms like ${#1} (to get the length of the
prefix) and ${path:$offset} (to get substring) are perfectly fine
way to correct it.
Also, as $1 is optional, won't this barf if it is run under "set -u"?
> +# __git_index_files accepts 1 or 2 arguments:
> +# 1: A string for file index status mode ("c", "m", "d", "o"), as
> +# supported by git ls-files command.
> +# 2: A directory path (optional).
> +# If provided, only files within the specified directory are listed.
> +# Sub directories are never recursed. Path must have a trailing
> +# slash.
> +__git_index_files ()
> +{
> + local dir="$(__gitdir)"
> +
> + if [ -d "$dir" ]; then
> + git --git-dir="$dir" ls-files --exclude-standard "-${1}" "${2}" |
> + __git_index_file_list_filter ${2} | uniq
Even though everywhere else you seem to quote the variables with dq,
but this ${2} is not written as "${2}", which looks odd. Deliberate?
If you wanted to call __git_index_file_list_filter without parameter
when the caller did not give you the optional $2, then the above is
not the way to write it. It would be ${2+"$2"}. The upstream of
the pipe (ls-files) also is getting an empty string as the pathspec
when $2 is omitted, and the call will break if this is run under
"set -u".
> +# __git_diff_index_files accepts 1 or 2 arguments:
> +# 1) The id of a tree object.
> +# 2) A directory path (optional).
> +# If provided, only files within the specified directory are listed.
> +# Sub directories are never recursed. Path must have a trailing
> +# slash.
> +__git_diff_index_files ()
> +{
> + local dir="$(__gitdir)"
> +
> + if [ -d "$dir" ]; then
> + git --git-dir="$dir" diff-index --name-only "${1}" |
> + __git_index_file_list_filter "${2}" | uniq
This one, when the optional $2 is absent, will call __git_index_file_list_filter
with an empty string in its "$1". Intended, or is it also ${2+"$2"}?
> +# __git_complete_index_file requires 1 argument: the file index
> +# status mode
> +__git_complete_index_file ()
> +{
> + local pfx cur_="$cur"
> +
> + case "$cur_" in
> + ?*/*)
> + pfx="${cur_%/*}"
> + cur_="${cur_##*/}"
> + pfx="${pfx}/"
> +
> + __gitcomp_nl "$(__git_index_files ${1} ${pfx})" "$pfx" "$cur_" ""
> + ;;
> + *)
> + __gitcomp_nl "$(__git_index_files ${1})" "" "$cur_" ""
> + ;;
> + esac
Please dedent the case arms by one level, i.e.
case $value in
$pattern1)
action1
;;
...
;;
esac
> +# __git_complete_diff_index_file requires 1 argument: the id of a tree
> +# object
> +__git_complete_diff_index_file ()
> +{
> + local pfx cur_="$cur"
> +
> + case "$cur_" in
> + ?*/*)
> + pfx="${cur_%/*}"
> + cur_="${cur_##*/}"
> + pfx="${pfx}/"
> +
> + __gitcomp_nl "$(__git_diff_index_files ${1} ${pfx})" "$pfx" "$cur_" ""
> + ;;
> + *)
> + __gitcomp_nl "$(__git_diff_index_files ${1})" "" "$cur_" ""
> + ;;
Unquoted $1 looks fishy, even if the only caller of this function
always gives "HEAD" to it (in which case you can do without making
it a parameter in the first place).
Unquoted ${pfx} given to __git_diff_index_files also looks fishy.
^ permalink raw reply
* [PATCH v3] git-completion.bash: add support for path completion
From: Manlio Perillo @ 2012-12-19 18:58 UTC (permalink / raw)
To: git; +Cc: Manlio Perillo
The git-completion.bash script did not implemented full, git aware,
support to complete paths, for git commands that operate on files within
the current working directory or the index.
As an example:
git add <TAB>
will suggest all files in the current working directory, including
ignored files and files that have not been modified.
Support path completion, for git commands where the non-option arguments
always refer to paths within the current working directory or the index,
as the follow:
* the path completion for the "git mv", "git rm" and "git ls-tree"
commands will suggest all cached files.
* the path completion for the "git add" command will suggest all
untracked and modified files. Ignored files are excluded.
* the path completion for the "git clean" command will suggest all
untracked files. Ignored files are excluded.
* the path completion for the "git commit" command will suggest all
files that have been modified from the HEAD.
For all affected commands, completion will always stop at directory
boundary. Only standard ignored files are excluded, using the
--exclude-standard option of the ls-files command.
Signed-off-by: Manlio Perillo <manlio.perillo@gmail.com>
---
Changes from version 2:
* Perl is no more used.
* Fixed some coding style issues.
* Refactorized code, to improve future path completion support for
the "git reset" command.
contrib/completion/git-completion.bash | 127 ++++++++++++++++++++++++++++-----
1 file changed, 111 insertions(+), 16 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0b77eb1..fc12d0f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -13,6 +13,7 @@
# *) .git/remotes file names
# *) git 'subcommands'
# *) tree paths within 'ref:path/to/file' expressions
+# *) file paths within current working directory and index
# *) common --long-options
#
# To use these routines:
@@ -233,6 +234,58 @@ __gitcomp_nl ()
COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
}
+# Process path list returned by "ls-files" and "diff-index --name-only"
+# commands, in order to list only file names relative to a specified
+# directory, and append a slash to directory names.
+# It accepts 1 optional argument: a directory path. The path must have
+# a trailing slash.
+__git_index_file_list_filter ()
+{
+ local path
+
+ while read -r path; do
+ path=${path#$1}
+
+ case "$path" in
+ ?*/*) echo "${path%%/*}/" ;;
+ *) echo $path ;;
+ esac
+ done
+}
+
+# __git_index_files accepts 1 or 2 arguments:
+# 1: A string for file index status mode ("c", "m", "d", "o"), as
+# supported by git ls-files command.
+# 2: A directory path (optional).
+# If provided, only files within the specified directory are listed.
+# Sub directories are never recursed. Path must have a trailing
+# slash.
+__git_index_files ()
+{
+ local dir="$(__gitdir)"
+
+ if [ -d "$dir" ]; then
+ git --git-dir="$dir" ls-files --exclude-standard "-${1}" "${2}" |
+ __git_index_file_list_filter ${2} | uniq
+ fi
+}
+
+# __git_diff_index_files accepts 1 or 2 arguments:
+# 1) The id of a tree object.
+# 2) A directory path (optional).
+# If provided, only files within the specified directory are listed.
+# Sub directories are never recursed. Path must have a trailing
+# slash.
+__git_diff_index_files ()
+{
+ local dir="$(__gitdir)"
+
+ if [ -d "$dir" ]; then
+ git --git-dir="$dir" diff-index --name-only "${1}" |
+ __git_index_file_list_filter "${2}" | uniq
+ fi
+}
+
__git_heads ()
{
local dir="$(__gitdir)"
@@ -430,6 +483,46 @@ __git_complete_revlist_file ()
}
+# __git_complete_index_file requires 1 argument: the file index
+# status mode
+__git_complete_index_file ()
+{
+ local pfx cur_="$cur"
+
+ case "$cur_" in
+ ?*/*)
+ pfx="${cur_%/*}"
+ cur_="${cur_##*/}"
+ pfx="${pfx}/"
+
+ __gitcomp_nl "$(__git_index_files ${1} ${pfx})" "$pfx" "$cur_" ""
+ ;;
+ *)
+ __gitcomp_nl "$(__git_index_files ${1})" "" "$cur_" ""
+ ;;
+ esac
+}
+
+# __git_complete_diff_index_file requires 1 argument: the id of a tree
+# object
+__git_complete_diff_index_file ()
+{
+ local pfx cur_="$cur"
+
+ case "$cur_" in
+ ?*/*)
+ pfx="${cur_%/*}"
+ cur_="${cur_##*/}"
+ pfx="${pfx}/"
+
+ __gitcomp_nl "$(__git_diff_index_files ${1} ${pfx})" "$pfx" "$cur_" ""
+ ;;
+ *)
+ __gitcomp_nl "$(__git_diff_index_files ${1})" "" "$cur_" ""
+ ;;
+ esac
+}
+
__git_complete_file ()
{
__git_complete_revlist_file
@@ -770,8 +863,6 @@ _git_apply ()
_git_add ()
{
- __git_has_doubledash && return
-
case "$cur" in
--*)
__gitcomp "
@@ -780,7 +871,9 @@ _git_add ()
"
return
esac
- COMPREPLY=()
+
+ # XXX should we check for --update and --all options ?
+ __git_complete_index_file "om"
}
_git_archive ()
@@ -930,15 +1023,15 @@ _git_cherry_pick ()
_git_clean ()
{
- __git_has_doubledash && return
-
case "$cur" in
--*)
__gitcomp "--dry-run --quiet"
return
;;
esac
- COMPREPLY=()
+
+ # XXX should we check for -x option ?
+ __git_complete_index_file "o"
}
_git_clone ()
@@ -969,8 +1062,6 @@ _git_clone ()
_git_commit ()
{
- __git_has_doubledash && return
-
case "$cur" in
--cleanup=*)
__gitcomp "default strip verbatim whitespace
@@ -997,8 +1088,10 @@ _git_commit ()
--verbose --quiet --fixup= --squash=
"
return
+ ;;
esac
- COMPREPLY=()
+
+ __git_complete_diff_index_file "HEAD"
}
_git_describe ()
@@ -1216,8 +1309,6 @@ _git_init ()
_git_ls_files ()
{
- __git_has_doubledash && return
-
case "$cur" in
--*)
__gitcomp "--cached --deleted --modified --others --ignored
@@ -1230,7 +1321,10 @@ _git_ls_files ()
return
;;
esac
- COMPREPLY=()
+
+ # XXX ignore options like --modified and always suggest all cached
+ # files.
+ __git_complete_index_file "c"
}
_git_ls_remote ()
@@ -1362,7 +1456,9 @@ _git_mv ()
return
;;
esac
- COMPREPLY=()
+
+ # XXX needs more work
+ __git_complete_index_file "c"
}
_git_name_rev ()
@@ -2068,15 +2164,14 @@ _git_revert ()
_git_rm ()
{
- __git_has_doubledash && return
-
case "$cur" in
--*)
__gitcomp "--cached --dry-run --ignore-unmatch --quiet"
return
;;
esac
- COMPREPLY=()
+
+ __git_complete_index_file "c"
}
_git_shortlog ()
--
1.8.1.rc1.18.g9db0d25
^ permalink raw reply related
* Re: [PATCH] Remove duplicate entry in ./Documentation/Makefile
From: Matt Kraai @ 2012-12-19 18:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Thomas Ackermann
Hi,
Junio C Hamano wrote:
> If not by eyeballing but with some mechanical process, did you spot
> any others?
I found one other unnecessarily duplicated line in the top-level
Makefile:
LIB_H += xdiff/xdiff.h
by running
find -name Makefile | xargs grep += | sort | uniq -d
and inspecting the results by hand, but this only checked lines
containing "+=".
--
Matt Kraai
https://ftbfs.org/kraai
^ permalink raw reply
* Re: [PATCH 3/3] Convert all fnmatch() calls to wildmatch()
From: Junio C Hamano @ 2012-12-19 18:36 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1355922488-20976-4-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> diff --git a/tree-walk.c b/tree-walk.c
> index 492c7cd..c729e89 100644
> --- a/tree-walk.c
> +++ b/tree-walk.c
> @@ -3,6 +3,7 @@
> #include "unpack-trees.h"
> #include "dir.h"
> #include "tree.h"
> +#include "wildmatch.h"
>
> static const char *get_mode(const char *str, unsigned int *modep)
> {
> @@ -627,7 +628,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
> return entry_interesting;
>
> if (item->use_wildcard) {
> - if (!fnmatch(match + baselen, entry->path, 0))
> + if (!wildmatch(match + baselen, entry->path, 0))
> return entry_interesting;
This and the change to dir.c obviously have interactions with
8c6abbc (pathspec: apply "*.c" optimization from exclude,
2012-11-24).
I've already alluded to it in my response to 2/3, I guess.
^ permalink raw reply
* Re: [PATCH] Remove duplicate entry in ./Documentation/Makefile
From: Junio C Hamano @ 2012-12-19 18:22 UTC (permalink / raw)
To: Thomas Ackermann; +Cc: git
In-Reply-To: <1674037566.22743.1355940925216.JavaMail.ngmail@webmail07.arcor-online.net>
Thomas Ackermann <th.acker@arcor.de> writes:
> Signed-off-by: Thomas Ackermann <th.acker@arcor.de>
> ---
> Documentation/Makefile | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 3615504..7df75d0 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -31,7 +31,6 @@ SP_ARTICLES += howto/separating-topic-branches
> SP_ARTICLES += howto/revert-a-faulty-merge
> SP_ARTICLES += howto/recover-corrupted-blob-object
> SP_ARTICLES += howto/rebuild-from-update-hook
> -SP_ARTICLES += howto/rebuild-from-update-hook
Heh, good eyes. How did you spot it?
If not by eyeballing but with some mechanical process, did you spot
any others?
> SP_ARTICLES += howto/rebase-from-internal-branch
> SP_ARTICLES += howto/maintain-git
> API_DOCS = $(patsubst %.txt,%,$(filter-out technical/api-index-skel.txt technical/api-index.txt, $(wildcard technical/api-*.txt)))
^ permalink raw reply
* [PATCH] Remove duplicate entry in ./Documentation/Makefile
From: Thomas Ackermann @ 2012-12-19 18:15 UTC (permalink / raw)
To: git; +Cc: th.acker
Signed-off-by: Thomas Ackermann <th.acker@arcor.de>
---
Documentation/Makefile | 1 -
1 file changed, 1 deletion(-)
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 3615504..7df75d0 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -31,7 +31,6 @@ SP_ARTICLES += howto/separating-topic-branches
SP_ARTICLES += howto/revert-a-faulty-merge
SP_ARTICLES += howto/recover-corrupted-blob-object
SP_ARTICLES += howto/rebuild-from-update-hook
-SP_ARTICLES += howto/rebuild-from-update-hook
SP_ARTICLES += howto/rebase-from-internal-branch
SP_ARTICLES += howto/maintain-git
API_DOCS = $(patsubst %.txt,%,$(filter-out technical/api-index-skel.txt technical/api-index.txt, $(wildcard technical/api-*.txt)))
--
1.8.0.msysgit.0
---
Thomas
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox