verify_filename(): treat ":(magic)" as a pathspec

For commands that take revisions and pathspecs, magic
pathspecs like ":(exclude)foo" require the user to specify
a disambiguating "--", since they do not match a file in the
filesystem, like:

  git grep foo -- :(exclude)bar

This makes them more annoying to use than they need to be.
We loosened the rules for wildcards in 28fcc0b71 (pathspec:
avoid the need of "--" when wildcard is used, 2015-05-02).
Let's do the same for pathspecs with long-form magic.

We already handle the short-forms ":/" and ":^" specially in
check_filename(), so we don't need to handle them here. And
in fact, we could do the same with long-form magic, parsing
out the actual filename and making sure it exists. But there
are a few reasons not to do it that way:

  - the parsing gets much more complicated, and we'd want to
    hand it off to the pathspec code. But that code isn't
    ready to do this kind of speculative parsing (it's happy
    to die() when it sees a syntactically invalid pathspec).

  - not all pathspec magic maps to a filesystem path. E.g.,
    :(attr) should be treated as a pathspec regardless of
    what is in the filesystem

  - we can be a bit looser with ":(" than with the
    short-form ":/", because it is much less likely to have
    a false positive. Whereas ":/" also means "search for a
    commit with this regex".

Note that because the change is in verify_filename() and not
in its helper check_filename(), this doesn't affect the
verify_non_filename() case. I.e., if an item that matches
our new rule doesn't resolve as an object, we may fallback
to treating it as a pathspec (rather than complaining it
doesn't exist). But if it does resolve (e.g., as a file in
the index that starts with an open-paren), we won't then
complain that it's also a valid pathspec. This matches the
wildcard-exception behavior.

And of course in either case, one can always insert the "--"
to get more precise results.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

1 files changed, 13 insertions, 0 deletions

diff --git a/t/t4208-log-magic-pathspec.sh b/t/t4208-log-magic-pathspec.sh
index 627890d7d8..935df6a65c 100755
--- a/t/t4208-log-magic-pathspec.sh
+++ b/t/t4208-log-magic-pathspec.sh
@@ -65,6 +65,19 @@ test_expect_success  '"git log :!" behaves the same as :^' '
 	test_must_fail git log :!does-not-exist
 '
 
+test_expect_success '"git log :(exclude)sub" is not ambiguous' '
+	git log ":(exclude)sub"
+'
+
+test_expect_success '"git log :(exclude)sub --" must resolve as an object' '
+	test_must_fail git log ":(exclude)sub" --
+'
+
+test_expect_success '"git log :(unknown-magic) complains of bogus magic' '
+	test_must_fail git log ":(unknown-magic)" 2>error &&
+	test_i18ngrep pathspec.magic error
+'
+
 test_expect_success 'command line pathspec parsing for "git log"' '
 	git reset --hard &&
 	>a &&