Re: git show doesn't work on file names with square brackets
- From
Jeff King <peff@peff.net>
- Date
- Feb 8, 2016, 15:07 UTC
- Message-ID
- <20160208150709.GA13664@sigill.intra.peff.net>
- In-Reply-To
- <20160208141552.GC27054@sigill.intra.peff.net>
On Mon, Feb 08, 2016 at 09:15:52AM -0500, Jeff King wrote:
Show 15 quoted lines
> I wonder if we could fix this pretty simply, though, by skipping the
> "does it have a wildcard" check when we see a colon in the path. That is
> a good indication that we are using one of git's special rev syntaxes
> (either "tree:path", or ":path", or ":/search string". That breaks
> anybody who really wanted to look for "path:with:colons.*", but that
> seems a lot less likely to me.
>
> It doesn't cover:
>
> git log 'HEAD^{/Merge.*}'
>
> which is similarly affected by 28fcc0b. Perhaps "^{" should be such a
> magic string, as well. We can be liberal with such strings as they are
> really just limiting the impact of 28fcc0b; we would fall back in those
> cases to the usual "can it be resolved, or is it a path?" rule.The patch for that might look like this. I like it for its relative simplicity, though it does make the rules even harder to explain to a user (whereas if we actually tried to glob each pathspec, that would keep the rule simple and work well in practice; I'm not sure how easy that it is to do, though, if we are dealing with things like :(magic) pathspecs, but maybe we should simply be dealing with them syntactically much earlier).
This breaks the second test in t2019 added by ae454f6, but I am not sure that test is doing the right thing (I'm also not sure t2019 is the best place for these tests; I added new ones here in a separate script).
-- >8 -- Subject: [PATCH] check_filename: tighten requirements for dwim-wildcards
Commit 28fcc0b (pathspec: avoid the need of "--" when wildcard is used, 2015-05-02) introduced a convenience to our dwim-parsing: when "--" is not present, we guess that items with wildcard characters are probably pathspecs.
This makes a lot of cases simpler (e.g., "git log '*.c'"), but makes others harder. While revision expressions do not typically have wildcard characters in them (because they are not valid in refnames), there are a few constructs where we take more arbitrary strings, such as:
- pathnames in tree:path syntax (or :0:path) for the
index) - :/foo and ^{/foo} for searching commit messages;
likewise "^{}" is extensible and may learn new formats
in the future - @{foo}, which can take arbitrary approxidate text (which
is not itself that likely to have wildcards, but @{} is
also a potential generic extension mechanism).When we see these constructs, they are almost certainly an attempt at a revision, and not a pathspec; we should not give them the magic "wildcard characters mean a pathspec" treatment.
We can afford to be fairly slack in our parsing here. We are not making a real decision on "this is or is not definitely a revision" here, but rather just deciding whether or not the extra "wildcards mean pathspecs" magic kicks in.
Signed-off-by: Jeff King <peff@peff.net> --- setup.c | 21 ++++++++++++++++++++- t/t6133-pathspec-rev-dwim.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) create mode 100755 t/t6133-pathspec-rev-dwim.sh
diff --git a/setup.c b/setup.c index 2c4b22c..03ee4eb 100644 --- a/setup.c +++ b/setup.c @@ -130,6 +130,25 @@ int path_inside_repo(const char *prefix, const char *path) return 0; } +static int dwim_as_wildcard(const char *arg) +{ + const char *p; + + if (no_wildcard(arg)) + return 0; + if (strstr(arg, "^{")) + return 0; /* probably "^{something}" */ + if (strstr(arg, "@{")) + return 0; /* probably "ref@{something}" */ + + /* catch "tree:path", but not ":(magic)" */ + p = strchr(arg, ':'); + if (p && p[1] != '(') + return 0; + + return 1; +} + int check_filename(const char *prefix, const char *arg) { const char *name; @@ -139,7 +158,7 @@ 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)) + } else if (dwim_as_wildcard(arg)) return 1; else if (prefix) name = prefix_filename(prefix, strlen(prefix), arg); diff --git a/t/t6133-pathspec-rev-dwim.sh b/t/t6133-pathspec-rev-dwim.sh new file mode 100755 index 0000000..8f68937 --- /dev/null +++ b/t/t6133-pathspec-rev-dwim.sh @@ -0,0 +1,44 @@ +#!/bin/sh + +test_description='test dwim of revs versus pathspecs in revision parser' +. ./test-lib.sh + +test_expect_success 'setup' ' + test_commit base && + echo content >"br[ack]ets" && + git add . && + test_tick && + git commit -m brackets +' + +test_expect_success 'wildcard dwims to pathspec' ' + git log -- "*.t" >expect && + git log "*.t" >actual && + test_cmp expect actual +' + +test_expect_success ':(magic) dwims to pathspec' ' + git log -- ":(literal)br[ack]ets" >expect && + git log ":(literal)br[ack]ets" >actual && + test_cmp expect actual +' + +test_expect_success 'tree:path dwims to rev' ' + git show "HEAD:br[ack]ets" -- >expect && + git show "HEAD:br[ack]ets" >actual && + test_cmp expect actual +' + +test_expect_success '^{foo} dwims to rev' ' + git log "HEAD^{/b.*}" -- >expect && + git log "HEAD^{/b.*}" >actual && + test_cmp expect actual +' + +test_expect_success '@{foo} dwims to rev' ' + git log "HEAD@{now [or thereabouts]}" -- >expect && + git log "HEAD@{now [or thereabouts]}" >actual && + test_cmp expect actual +' + +test_done
-- 2.7.1.526.gd04f550