Re: git show doesn't work on file names with square brackets

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 <redacted>
---
 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