Merge branch 'jk/pathspec-magic-disambiguation'

The convention for a command line is to follow "git cmdname
--options" with revisions followed by an optional "--"
disambiguator and then finally pathspecs.  When "--" is not there,
we make sure early ones are all interpretable as revs (and do not
look like paths) and later ones are the other way around.  A
pathspec with "magic" (e.g. ":/p/a/t/h" that matches p/a/t/h from
the top-level of the working tree, no matter what subdirectory you
are working from) are conservatively judged as "not a path", which
required disambiguation more often.  The command line parser
learned to say "it's a pathspec" a bit more often when the syntax
looks like so.

* jk/pathspec-magic-disambiguation:
verify_filename(): flip order of checks
verify_filename(): treat ":(magic)" as a pathspec
check_filename(): handle ":^" path magic
check_filename(): use skip_prefix
check_filename(): refactor ":/" handling
t4208: add check for ":/" without matching file

diff --git a/setup.c b/setup.c
index ba6e855178847a37044eacceae0ebcb17d8e990b..751d02b9be627176009cdda4f1b20a3674b0dc95 100644 (file)
--- a/setup.c
+++ b/setup.c
@@ -134,19 +134,23 @@ int path_inside_repo(const char *prefix, const char *path)
 
 int check_filename(const char *prefix, const char *arg)
 {
-       const char *name;
        char *to_free = NULL;
        struct stat st;
 
-       if (starts_with(arg, ":/")) {
-               if (arg[2] == '\0') /* ":/" is root dir, always exists */
+       if (skip_prefix(arg, ":/", &arg)) {
+               if (!*arg) /* ":/" is root dir, always exists */
                        return 1;
-               name = arg + 2;
-       } else if (prefix)
-               name = to_free = prefix_filename(prefix, arg);
-       else
-               name = arg;
-       if (!lstat(name, &st)) {
+               prefix = NULL;
+       } else if (skip_prefix(arg, ":!", &arg) ||
+                  skip_prefix(arg, ":^", &arg)) {
+               if (!*arg) /* excluding everything is silly, but allowed */
+                       return 1;
+       }
+
+       if (prefix)
+               arg = to_free = prefix_filename(prefix, arg);
+
+       if (!lstat(arg, &st)) {
                free(to_free);
                return 1; /* file exists */
        }
@@ -181,6 +185,24 @@ static void NORETURN die_verify_filename(const char *prefix,
 
 }
 
+/*
+ * Check for arguments that don't resolve as actual files,
+ * but which look sufficiently like pathspecs that we'll consider
+ * them such for the purposes of rev/pathspec DWIM parsing.
+ */
+static int looks_like_pathspec(const char *arg)
+{
+       /* anything with a wildcard character */
+       if (!no_wildcard(arg))
+               return 1;
+
+       /* long-form pathspec magic */
+       if (starts_with(arg, ":("))
+               return 1;
+
+       return 0;
+}
+
 /*
  * Verify a filename that we got as an argument for a pathspec
  * entry. Note that a filename that begins with "-" never verifies
@@ -207,7 +229,7 @@ void verify_filename(const char *prefix,
 {
        if (*arg == '-')
                die("bad flag '%s' used after filename", arg);
-       if (check_filename(prefix, arg) || !no_wildcard(arg))
+       if (looks_like_pathspec(arg) || check_filename(prefix, arg))
                return;
        die_verify_filename(prefix, arg, diagnose_misspelt_rev);
 }

diff --git a/t/t4208-log-magic-pathspec.sh b/t/t4208-log-magic-pathspec.sh
index 001343e2fc2722506172e702266f5bab6278d1e8..935df6a65cab2ba901856633baad7b2e38ebecfa 100755 (executable)
--- a/t/t4208-log-magic-pathspec.sh
+++ b/t/t4208-log-magic-pathspec.sh
@@ -29,6 +29,12 @@ test_expect_success '"git log -- :/a" should not be ambiguous' '
        git log -- :/a
 '
 
+# This differs from the ":/a" check above in that :/in looks like a pathspec,
+# but doesn't match an actual file.
+test_expect_success '"git log :/in" should not be ambiguous' '
+       git log :/in
+'
+
 test_expect_success '"git log :" should be ambiguous' '
        test_must_fail git log : 2>error &&
        test_i18ngrep ambiguous error
@@ -46,6 +52,32 @@ test_expect_success 'git log HEAD -- :/' '
        test_cmp expected actual
 '
 
+test_expect_success '"git log :^sub" is not ambiguous' '
+       git log :^sub
+'
+
+test_expect_success '"git log :^does-not-exist" does not match anything' '
+       test_must_fail git log :^does-not-exist
+'
+
+test_expect_success  '"git log :!" behaves the same as :^' '
+       git log :!sub &&
+       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 &&