Re: git show doesn't work on file names with square brackets
- From
Jeff King <peff@peff.net>
- Date
- Feb 8, 2016, 20:56 UTC
- Message-ID
- <20160208205637.GA13732@sigill.intra.peff.net>
- In-Reply-To
- <20160208202043.GA6002@sigill.intra.peff.net>
On Mon, Feb 08, 2016 at 03:20:43PM -0500, Jeff King wrote:
Show 19 quoted lines
> On Mon, Feb 08, 2016 at 02:52:30PM -0500, Jeff King wrote: > > > Here is my patch again, with that part removed, and the tests fixed up. > > Though on reflection, I do think it would be better if we could simply > > expand the wildcard globs to say "does this match anything in the file > > system". That makes a nice, simple rule that follows the spirit of the > > original. I'm not sure if it would be easy to apply magic like ":(top)" > > there, but even if we don't, we're not worse off than we are today > > (where that requires "--" unless it happens to have a wildcard, as > > above). > > So here is a hacky attempt at that. It uses glob(), which is not quite > right for the reasons below, though I suspect works OK in practice. > > I think doing it correctly would require actually calling our > read_directory() function. That feels kind of heavy-weight for this > case, but I guess in theory the pathspec limits it (and it's not like > glob() does not have to walk the filesystem, too). So maybe it's not so > bad.
And here that is. It does end up traversing quite a bit for something as simple as "*.foo", because that doesn't let fill_directory() limit us at all.
But having looked at this, I can't help but wonder if the rule should not be "does the file exist" in the first place, but "is the file in the index". This dwimmery is about commands like "log" that are reading existing commits. I cannot think of a case where we would want to include something that exists in the filesystem but not in the index.
---
diff --git a/setup.c b/setup.c index 2c4b22c..1a40516 100644 --- a/setup.c +++ b/setup.c @@ -1,5 +1,6 @@ #include "cache.h" #include "dir.h" +#include "pathspec.h" #include "string-list.h" static int inside_git_dir = -1; @@ -130,6 +131,33 @@ int path_inside_repo(const char *prefix, const char *path) return 0; } +/* + * Return true if a file exists that matches the pattern + * glob. + */ +static int pathspec_exists(const char *one_pathspec) +{ + struct dir_struct dir; + const char *pathspec_v[] = { one_pathspec, NULL }; + struct pathspec pathspec; + int ret = 0; + int i; + + memset(&dir, 0, sizeof(dir)); + parse_pathspec(&pathspec, 0, 0, "", pathspec_v); + + fill_directory(&dir, &pathspec); + for (i = 0; i < dir.nr; i++) { + if (dir_path_match(dir.entries[i], &pathspec, 0, NULL)) { + ret = 1; + break; + } + } + + free(dir.entries); + return ret; +} + int check_filename(const char *prefix, const char *arg) { const char *name; @@ -139,12 +167,14 @@ int check_filename(const char *prefix, const char *arg) if (arg[2] == '\0') /* ":/" is root dir, always exists */ return 1; name = arg + 2; - } else if (!no_wildcard(arg)) - return 1; - else if (prefix) + } else if (prefix) name = prefix_filename(prefix, strlen(prefix), arg); else name = arg; + + if (!no_wildcard(arg)) + return pathspec_exists(name); + if (!lstat(name, &st)) return 1; /* file exists */ if (errno == ENOENT || errno == ENOTDIR)