On Mon, Feb 08, 2016 at 09:15:52AM -0500, Jeff King wrote:
> 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@xxxxxxxx>
---
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html