push: suggest <remote> <branch> for a slash slip

When pushing the 'main' branch to the remote 'origin', i.e.,

    $ git push origin main

it is easy to mistakenly write

    $ git push origin/main

That is parsed as the repository to push to, and since 'origin/main'
is neither a configured remote nor a path it dies with:

    fatal: 'origin/main' does not appear to be a git repository

Often 'origin/main' does not exist as a repository, so the command
fails without doing any harm, but it gives no hint that a space was
meant instead of a slash and can leave the user puzzled.

When the argument is not an existing path or configured remote but
its part before the first slash names one, suggest the intended
'<remote> <branch>' form:

    $ git push origin main

The suggestion is shown as advice so it can be silenced with
advice.pushRepoLooksLikeRef.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc
index 257db58918179a931ba42f223de30025cb7b9dca..fa77a5110eb10d056f90e2bcfe11cd40ef167e7d 100644 (file)
--- a/Documentation/config/advice.adoc
+++ b/Documentation/config/advice.adoc
@@ -90,6 +90,11 @@ all advice messages.
                Shown when linkgit:git-push[1] rejects a forced update of
                a branch when its remote-tracking ref has updates that we
                do not have locally.
+       pushRepoLooksLikeRef::
+               Shown when the repository given to linkgit:git-push[1] is not
+               a configured remote but looks like a `<remote>/<branch>` ref,
+               suggesting that the remote and branch be given as separate
+               arguments.
        pushUnqualifiedRefname::
                Shown when linkgit:git-push[1] gives up trying to
                guess based on the source and destination refs what

diff --git a/advice.c b/advice.c
index 0018501b7bc103267ac32c94f8869944a921d3de..63bf8b0c5f048122f9577d6417b0c935c49c603a 100644 (file)
--- a/advice.c
+++ b/advice.c
@@ -69,6 +69,7 @@ static struct {
        [ADVICE_PUSH_NON_FF_CURRENT]                    = { "pushNonFFCurrent" },
        [ADVICE_PUSH_NON_FF_MATCHING]                   = { "pushNonFFMatching" },
        [ADVICE_PUSH_REF_NEEDS_UPDATE]                  = { "pushRefNeedsUpdate" },
+       [ADVICE_PUSH_REPO_LOOKS_LIKE_REF]               = { "pushRepoLooksLikeRef" },
        [ADVICE_PUSH_UNQUALIFIED_REF_NAME]              = { "pushUnqualifiedRefName" },
        [ADVICE_PUSH_UPDATE_REJECTED]                   = { "pushUpdateRejected" },
        [ADVICE_PUSH_UPDATE_REJECTED_ALIAS]             = { "pushNonFastForward" }, /* backwards compatibility */

diff --git a/advice.h b/advice.h
index 8def28068861df05851c70025f819e3a390dd71c..66f6cd6a772d8cd74f0bfa372afad9108a4f3e25 100644 (file)
--- a/advice.h
+++ b/advice.h
@@ -36,6 +36,7 @@ enum advice_type {
        ADVICE_PUSH_NON_FF_CURRENT,
        ADVICE_PUSH_NON_FF_MATCHING,
        ADVICE_PUSH_REF_NEEDS_UPDATE,
+       ADVICE_PUSH_REPO_LOOKS_LIKE_REF,
        ADVICE_PUSH_UNQUALIFIED_REF_NAME,
        ADVICE_PUSH_UPDATE_REJECTED,
        ADVICE_PUSH_UPDATE_REJECTED_ALIAS,

diff --git a/builtin/push.c b/builtin/push.c
index 6021b71d66845567a155ddd2242ca6fb4fba4fb9..1b2ad3b8df7c550e0e66c493cb6b042bd349aeef 100644 (file)
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -8,6 +8,7 @@
 #include "advice.h"
 #include "branch.h"
 #include "config.h"
+#include "dir.h"
 #include "environment.h"
 #include "gettext.h"
 #include "hex.h"
@@ -662,6 +663,29 @@ static int push_multiple(struct string_list *list,
        return result;
 }
 
+static void die_if_repo_looks_like_ref(const char *repo)
+{
+       const char *slash = strchr(repo, '/');
+       struct strbuf name = STRBUF_INIT;
+       int code;
+
+       if (!slash || !slash[1] || file_exists(repo))
+               return;
+
+       strbuf_add(&name, repo, slash - repo);
+       if (!remote_is_configured(remote_get(name.buf), 0)) {
+               strbuf_release(&name);
+               return;
+       }
+
+       code = die_message(_("'%s' is not a valid push target"), repo);
+       advise_if_enabled(ADVICE_PUSH_REPO_LOOKS_LIKE_REF,
+                         _("Did you mean to use: git push %s %s?"),
+                         name.buf, slash + 1);
+       strbuf_release(&name);
+       exit(code);
+}
+
 int cmd_push(int argc,
             const char **argv,
             const char *prefix,
@@ -744,6 +768,17 @@ int cmd_push(int argc,
 
        if (repo) {
                if (!add_remote_or_group(repo, &remote_group)) {
+                       struct remote *r;
+
+                       /*
+                        * Check the advice up front to avoid the remote
+                        * lookup when the hint is off. The helper still
+                        * calls advise_if_enabled() so the hint carries the
+                        * standard "disable this message" instructions.
+                        */
+                       if (advice_enabled(ADVICE_PUSH_REPO_LOOKS_LIKE_REF))
+                               die_if_repo_looks_like_ref(repo);
+
                        /*
                         * Not a configured remote name or group name.
                         * Try treating it as a direct URL or path, e.g.
@@ -753,7 +788,7 @@ int cmd_push(int argc,
                         * from the URL so the loop below can handle it
                         * identically to a named remote.
                         */
-                       struct remote *r = pushremote_get(repo);
+                       r = pushremote_get(repo);
                        if (!r)
                                die(_("bad repository '%s'"), repo);
                        string_list_append(&remote_group, r->name);

diff --git a/t/t5529-push-errors.sh b/t/t5529-push-errors.sh
index 80b06a0cd2886d235fc59d67177b43985472fd4b..2294645902c90e1e76fcaee06b2291d2fcd93854 100755 (executable)
--- a/t/t5529-push-errors.sh
+++ b/t/t5529-push-errors.sh
@@ -54,6 +54,37 @@ test_expect_success 'detect empty remote with targeted refspec' '
        grep "fatal: bad repository ${SQ}${SQ}" stderr
 '
 
+test_expect_success 'suggest <remote> <branch> for a <remote>/<branch> slip' '
+       test_must_fail git push origin/main 2>stderr &&
+       test_grep "${SQ}origin/main${SQ} is not a valid push target" stderr &&
+       test_grep "hint: Did you mean to use: git push origin main?" stderr &&
+       test_must_fail git -c advice.pushRepoLooksLikeRef=false push origin/main 2>stderr &&
+       test_grep ! "Did you mean" stderr
+'
+
+test_expect_success 'suggest <remote> <branch> when the branch has slashes' '
+       test_must_fail git push origin/feature/x 2>stderr &&
+       test_grep "hint: Did you mean to use: git push origin feature/x?" stderr
+'
+
+test_expect_success 'no suggestion when prefix is not a configured remote' '
+       test_must_fail git push not-a-remote/main 2>stderr &&
+       test_grep ! "Did you mean" stderr
+'
+
+test_expect_success 'no suggestion for a trailing slash with no branch' '
+       test_must_fail git push origin/ 2>stderr &&
+       test_grep ! "Did you mean" stderr
+'
+
+test_expect_success 'no suggestion when the argument is an existing path' '
+       test_when_finished "rm -rf origin" &&
+       git init --bare origin/main &&
+       git push origin/main HEAD:refs/heads/pushed 2>stderr &&
+       test_grep ! "Did you mean" stderr &&
+       git -C origin/main rev-parse --verify refs/heads/pushed
+'
+
 test_expect_success 'detect ambiguous refs early' '
        git branch foo &&
        git tag foo &&