checkout: add advice for ambiguous "checkout <branch>"

As the "checkout" documentation describes:

If <branch> is not found but there does exist a tracking branch in
exactly one remote (call it <remote>) with a matching name, treat
as equivalent to [...] <remote>/<branch.

This is a really useful feature. The problem is that when you add
another remote (e.g. a fork), git won't find a unique branch name
anymore, and will instead print this unhelpful message:

$ git checkout master
error: pathspec 'master' did not match any file(s) known to git

Now it will, on my git.git checkout, print:

$ ./git --exec-path=$PWD checkout master
error: pathspec 'master' did not match any file(s) known to git.
hint: 'master' matched more than one remote tracking branch.
hint: We found 26 remotes with a reference that matched. So we fell back
hint: on trying to resolve the argument as a path, but failed there too!
hint:
hint: If you meant to check out a remote tracking branch on, e.g. 'origin',
hint: you can do so by fully qualifying the name with the --track option:
hint:
hint:     git checkout --track origin/<name>

Note that the "error: pathspec[...]" message is still printed. This is
because whatever else checkout may have tried earlier, its final
fallback is to try to resolve the argument as a path. E.g. in this
case:

$ ./git --exec-path=$PWD checkout master pu
error: pathspec 'master' did not match any file(s) known to git.
error: pathspec 'pu' did not match any file(s) known to git.

There we don't print the "hint:" implicitly due to earlier logic
around the DWIM fallback. That fallback is only used if it looks like
we have one argument that might be a branch.

I can't think of an intrinsic reason for why we couldn't in some
future change skip printing the "error: pathspec[...]" error. However,
to do so we'd need to pass something down to checkout_paths() to make
it suppress printing an error on its own, and for us to be confident
that we're not silencing cases where those errors are meaningful.

I don't think that's worth it since determining whether that's the
case could easily change due to future changes in the checkout logic.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ab641bf5a9984b7ab2dfea787a0db5704f79a448..dfc0413a842692370aa26d151d36d6cd94a90a07 100644 (file)
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -344,6 +344,13 @@ advice.*::
                Advice shown when you used linkgit:git-checkout[1] to
                move to the detach HEAD state, to instruct how to create
                a local branch after the fact.
+       checkoutAmbiguousRemoteBranchName::
+               Advice shown when the argument to
+               linkgit:git-checkout[1] ambiguously resolves to a
+               remote tracking branch on more than one remote in
+               situations where an unambiguous argument would have
+               otherwise caused a remote-tracking branch to be
+               checked out.
        amWorkDir::
                Advice that shows the location of the patch file when
                linkgit:git-am[1] fails to apply it.

diff --git a/advice.c b/advice.c
index 370a56d0546bb3fc7aa4a62203c600e9a8af2d7a..75e7dede90a6fe0ae201217f9cc6d2b619480296 100644 (file)
--- a/advice.c
+++ b/advice.c
@@ -21,6 +21,7 @@ int advice_add_embedded_repo = 1;
 int advice_ignored_hook = 1;
 int advice_waiting_for_editor = 1;
 int advice_graft_file_deprecated = 1;
+int advice_checkout_ambiguous_remote_branch_name = 1;
 
 static int advice_use_color = -1;
 static char advice_colors[][COLOR_MAXLEN] = {
@@ -72,6 +73,7 @@ static struct {
        { "ignoredhook", &advice_ignored_hook },
        { "waitingforeditor", &advice_waiting_for_editor },
        { "graftfiledeprecated", &advice_graft_file_deprecated },
+       { "checkoutambiguousremotebranchname", &advice_checkout_ambiguous_remote_branch_name },
 
        /* make this an alias for backward compatibility */
        { "pushnonfastforward", &advice_push_update_rejected }

diff --git a/advice.h b/advice.h
index 9f5064e82a862e6daaabe3f23ea9ea896aad87e6..4d11d51d433da3a95134e6b374ae08f91a9dae7b 100644 (file)
--- a/advice.h
+++ b/advice.h
@@ -22,6 +22,7 @@ extern int advice_add_embedded_repo;
 extern int advice_ignored_hook;
 extern int advice_waiting_for_editor;
 extern int advice_graft_file_deprecated;
+extern int advice_checkout_ambiguous_remote_branch_name;
 
 int git_default_advice_config(const char *var, const char *value);
 __attribute__((format (printf, 1, 2)))

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 8c93c55cbc2a2f09954f3574712a85749c3ec61b..baa027455ad74df72700b90893291ae4dc545254 100644 (file)
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -22,6 +22,7 @@
 #include "resolve-undo.h"
 #include "submodule-config.h"
 #include "submodule.h"
+#include "advice.h"
 
 static const char * const checkout_usage[] = {
        N_("git checkout [<options>] <branch>"),
@@ -1267,6 +1268,18 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
        UNLEAK(opts);
        if (opts.patch_mode || opts.pathspec.nr) {
                int ret = checkout_paths(&opts, new_branch_info.name);
+               if (ret && dwim_remotes_matched > 1 &&
+                   advice_checkout_ambiguous_remote_branch_name)
+                       advise(_("'%s' matched more than one remote tracking branch.\n"
+                                "We found %d remotes with a reference that matched. So we fell back\n"
+                                "on trying to resolve the argument as a path, but failed there too!\n"
+                                "\n"
+                                "If you meant to check out a remote tracking branch on, e.g. 'origin',\n"
+                                "you can do so by fully qualifying the name with the --track option:\n"
+                                "\n"
+                                "    git checkout --track origin/<name>"),
+                              argv[0],
+                              dwim_remotes_matched);
                return ret;
        } else {
                return checkout_branch(&opts, &new_branch_info);

diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
index d2fb4e2c0c7e9bdceaddb346198065e2fb30ce33..082147a875c2999210fd1870b12167c6f6fda909 100755 (executable)
--- a/t/t2024-checkout-dwim.sh
+++ b/t/t2024-checkout-dwim.sh
@@ -76,6 +76,20 @@ test_expect_success 'checkout of branch from multiple remotes fails #1' '
        test_branch master
 '
 
+test_expect_success 'checkout of branch from multiple remotes fails with advice' '
+       git checkout -B master &&
+       test_might_fail git branch -D foo &&
+       test_must_fail git checkout foo 2>stderr &&
+       test_branch master &&
+       status_uno_is_clean &&
+       test_i18ngrep "^hint: " stderr &&
+       test_must_fail git -c advice.checkoutAmbiguousRemoteBranchName=false \
+               checkout foo 2>stderr &&
+       test_branch master &&
+       status_uno_is_clean &&
+       test_i18ngrep ! "^hint: " stderr
+'
+
 test_expect_success 'checkout of branch from a single remote succeeds #1' '
        git checkout -B master &&
        test_might_fail git branch -D bar &&