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

On Mon, Feb 08, 2016 at 11:35:10AM -0800, Junio C Hamano wrote:

To be honest, I think I am more concerned with ":/reg.*ex" than "tree:path" using meta-characters. I agree that actual metacharacters in file names are relatively rare.

Good point. I forgot about the short-hands, and I agree that there is not much point in doing a sloppy match of the long-hands if we do not cover the short-hands.

In retrospect, it is not worth trying to match magic pathspecs here at all, as it generally requires "--" anyway. It is only ones with wildcard which came along for the ride in 28fcc0b, and that was not the primary focus of that patch. I.e., without my patch, we already have:

  $ git rev-list --count HEAD Makefile
  2028

  $ git rev-list --count HEAD ':(top)Makefile'
  fatal: ambiguous argument ':(top)Makefile': unknown revision or path
  not in the working tree.
  Use '--' to separate paths from revisions, like this:
  'git <command> [<revision>...] -- [<file>...]'

  $ git rev-list --count HEAD ':(top)M[a]kefile'
  2028

which is slightly ridiculous. With my patch, the final one behaves the same as the second.

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).

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

Note that we drop the tests in t2019 in favor of a more complete set in t6133. t2019 was not the right place for them (it's about refname ambiguity, not dwim parsing ambiguity), and the second test explicitly checked for the opposite result of the case we are fixing here (which didn't really make any sense; as show by the test_must_fail in the test, it would only serve to annoy people).

Signed-off-by: Jeff King <peff@peff.net>
---
 setup.c                           | 15 ++++++++++++++-
 t/t2019-checkout-ambiguous-ref.sh | 26 --------------------------
 t/t6133-pathspec-rev-dwim.sh      | 38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 27 deletions(-)
 create mode 100755 t/t6133-pathspec-rev-dwim.sh