Skip to contentSkip to conversation

branch/push: suggest intended form when remote/branch slip given

PATCH Superseded Harald Nordgren via GitGitGadget 5 participants 26 messages Jun 12 – Jun 28 Documentation

lore
Share & tools
lore
#0 lore raw

cover branch/push: suggest intended form when remote/branch slip given

When the repository or upstream argument is a slip like "origin/main" or "origin main", suggest the intended "git push origin main" or "git branch --set-upstream-to=origin/main" form instead of failing with an unrelated error.

Harald Nordgren (2):
branch: suggest <remote>/<branch> on upstream slip
push: suggest <remote> <branch> for a slash slip

 Documentation/config/advice.adoc |  5 +++++
 advice.c                         |  1 +
 advice.h                         |  1 +
 builtin/branch.c                 | 17 ++++++++++++++
 builtin/push.c                   | 26 +++++++++++++++++++++-
 t/t3200-branch.sh                | 38 ++++++++++++++++++++++++++++++++
 t/t5529-push-errors.sh           | 31 ++++++++++++++++++++++++++
 7 files changed, 118 insertions(+), 1 deletion(-)

base-commit: 3e65291872de10c3f0bf05ea8c24187e7a71ebf0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2331%2FHaraldNordgren%2Fsuggest-remote-branch-slips-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2331/HaraldNordgren/suggest-remote-branch-slips-v1
Pull-Request: https://github.com/git/git/pull/2331

Signature · 1 line
gitgitgadget
#1 lore raw

PATCH 1/2 branch: suggest <remote>/<branch> on upstream slip

From: Harald Nordgren <[email protected]>

"git branch --set-upstream-to origin main" reads the trailing word as the local branch to operate on and dies with "branch 'main' does not exist", pointing at the wrong problem.

When that branch is missing and "<remote>/<branch>" names a real remote-tracking ref, suggest the intended
"git branch --set-upstream-to=<remote>/<branch>" form.

---
 builtin/branch.c  | 17 +++++++++++++++++
 t/t3200-branch.sh | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)
builtin/branch.c +17 -0
index 1572a4f9ef..7ad3efb908 100644
@@ -957,6 +957,23 @@ int cmd_branch(int argc,
 		if (!refs_ref_exists(get_main_ref_store(the_repository), branch->refname)) {
 			if (!argc || branch_checked_out(branch->refname))
 				die(_("no commit on branch '%s' yet"), branch->name);
+			if (argc == 1 && !strchr(new_upstream, '/') &&
+			    remote_is_configured(remote_get(new_upstream), 0)) {
+				struct strbuf remote_ref = STRBUF_INIT;
+
+				strbuf_addf(&remote_ref, "refs/remotes/%s/%s",
+					    new_upstream, argv[0]);
+				if (refs_ref_exists(get_main_ref_store(the_repository),
+						    remote_ref.buf)) {
+					int code = die_message(_("--set-upstream-to takes a single <remote>/<branch> argument"));
+					advise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,
+							  _("Did you mean to use: git branch --set-upstream-to=%s/%s?"),
+							  new_upstream, argv[0]);
+					strbuf_release(&remote_ref);
+					exit(code);
+				}
+				strbuf_release(&remote_ref);
+			}
 			die(_("branch '%s' does not exist"), branch->name);
 		}
 
t/t3200-branch.sh +38 -0
index e7829c2c4b..e2682a83a0 100755
@@ -1022,6 +1022,44 @@ test_expect_success '--set-upstream-to fails on a missing dst branch' '
 	test_cmp expect err
 '
 
+test_expect_success '--set-upstream-to suggests <remote>/<branch> on slip' '
+	test_when_finished "git remote remove slip-remote" &&
+	git remote add slip-remote . &&
+	git update-ref refs/remotes/slip-remote/slip-feature HEAD &&
+	test_must_fail git branch --set-upstream-to slip-remote slip-feature 2>err &&
+	test_grep "takes a single <remote>/<branch> argument" err &&
+	test_grep "hint: Did you mean to use: git branch --set-upstream-to=slip-remote/slip-feature?" err &&
+	test_must_fail git -c advice.setUpstreamFailure=false \
+		branch --set-upstream-to slip-remote slip-feature 2>err &&
+	test_grep ! "Did you mean" err
+'
+
+test_expect_success '--set-upstream-to does not suggest when no matching remote ref' '
+	test_when_finished "git remote remove slip-remote" &&
+	git remote add slip-remote . &&
+	test_must_fail git branch --set-upstream-to slip-remote no-such-branch 2>err &&
+	test_grep "branch ${SQ}no-such-branch${SQ} does not exist" err &&
+	test_grep ! "Did you mean" err
+'
+
+test_expect_success '--set-upstream-to to a local branch is not mistaken for a slip' '
+	git branch slip-local-upstream &&
+	git branch slip-local-target &&
+	git branch --set-upstream-to=slip-local-upstream slip-local-target 2>err &&
+	test_grep ! "Did you mean" err &&
+	echo refs/heads/slip-local-upstream >expect &&
+	git config branch.slip-local-target.merge >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '--set-upstream-to slip suggestion keeps a slashed branch name' '
+	test_when_finished "git remote remove slip-remote" &&
+	git remote add slip-remote . &&
+	git update-ref refs/remotes/slip-remote/slip/feature HEAD &&
+	test_must_fail git branch --set-upstream-to slip-remote slip/feature 2>err &&
+	test_grep "hint: Did you mean to use: git branch --set-upstream-to=slip-remote/slip/feature?" err
+'
+
 test_expect_success '--set-upstream-to fails on a missing src branch' '
 	test_must_fail git branch --set-upstream-to does-not-exist main 2>err &&
 	test_grep "the requested upstream branch '"'"'does-not-exist'"'"' does not exist" err
Signature · 1 line
gitgitgadget
#2 lore raw

Re: [PATCH 1/2] branch: suggest <remote>/<branch> on upstream slip

Quoted 5 lines · Harald Nordgren via GitGitGadget

From: Harald Nordgren <[email protected]>

"git branch --set-upstream-to origin main" reads the trailing word as the local branch to operate on and dies with "branch 'main' does not exist", pointing at the wrong problem.

When 'main' does not exist locally,

    $ git branch --set-upstream-to "$anything" main

would fail before even looking at the "$anything" (which is supposed to specify the new_upstream for the named local branch 'main'). The operation is to set the upstream for 'main', and if 'main' does not exist, doesn't the user deserve the error that says 'main' does not exist, no matter what "$anything" is, whether it is a well-formed or ill-formed remote tracking branch name?

So it is unclear, at least to me, why "branch 'main' does not exist" is an inappropriate message, mostly because these three lines does not clearly tell me what the user _expected_ the command line to do.

When 'main' does exist, but named upstream "$anything" does not, we get

    $ git branch sample master ;# make sure the thing exists
    $ git branch --set-upstream-to origin sample
    fatal: the requested upstream branch 'origin' does not exist
    hint:
    hint: If you are planning on basing your work on an upstream
    hint: branch that already exists at the remote, you may need to
    hint: run "git fetch" to retrieve it.
    hint:
    hint: If you are planning to push out a new local branch that
    hint: will track its remote counterpart, you may want to use
    hint: "git push -u" to set the upstream config as you push.
    hint: Disable this message with "git config set advice.setUpstreamFailure false"

which does sound clear enough to me, even though it does not exactly say "Even though upstream branch 'origin' does not exist, 'origin' is a nickname for a remote, perhaps you meant to say origin/something?"

I do not doubt you are trying to address a real issue, but the above three-line description does not tell me what that problem is.

Now I do not regularly use --set-upstream-to, so I may be missing an obvious common mistake modes, but a couple of my attempts to make bad command invocations seem to give me reasonable responses:

    $ git branch --set-upstream-to ko/master sample
    branch 'sample' set up to track 'ko/master'.

OK, both are well formed so no problem.

    $ git branch --set-upstream-to ko/mastre sample
    fatal: the requested upstream branch 'ko/mastre' does not exist
    hint:
    hint: If you are planning on basing your work on an upstream
    hint: branch that already exists at the remote, you may need to
    hint: run "git fetch" to retrieve it.
    hint:
    hint: If you are planning to push out a new local branch that
    hint: will track its remote counterpart, you may want to use
    hint: "git push -u" to set the upstream config as you push.
    hint: Disable this message with "git config set advice.setUpstreamFailure false"

Misspelt upstream branch name diagnosed correctly, just like the case where I gave 'origin', which does not exist, either.

#3 lore raw

Re: [PATCH 1/2] branch: suggest <remote>/<branch> on upstream slip

Quoted 19 lines · Junio C Hamano
Quoted 5 lines · Harald Nordgren via GitGitGadget

From: Harald Nordgren <[email protected]>

"git branch --set-upstream-to origin main" reads the trailing word as the local branch to operate on and dies with "branch 'main' does not exist", pointing at the wrong problem.

When 'main' does not exist locally,

    $ git branch --set-upstream-to "$anything" main

would fail before even looking at the "$anything" (which is supposed to specify the new_upstream for the named local branch 'main'). The operation is to set the upstream for 'main', and if 'main' does not exist, doesn't the user deserve the error that says 'main' does not exist, no matter what "$anything" is, whether it is a well-formed or ill-formed remote tracking branch name?

Quoted 3 lines

So it is unclear, at least to me, why "branch 'main' does not exist" is an inappropriate message, mostly because these three lines does not clearly tell me what the user _expected_ the command line to do.

After pondering on this a bit, I _think_ (but I am guessing, and your job as an author of proposed commit log message is to make sure your readers do not have to guess) what the user expected was to set the upstream for the currrent branch.

When trying to set the upstream for the current branch to "main" branch of the remote "origin", i.e.,

$ git branch --set-upstream-to origin/main

it is easy for some users to mistakenly say

$ git branch --set-upstream-to origin main

But it is a request to set the upstream for the local branch "main" to "origin", which is not expected to work as the upstream most likely would look like <remote>/<branch> (e.g., "origin/main"). The user would get either one of these errors:

fatal: branch 'main' does not exist
fatal: the requested upstream branch 'origin' does not exist

Give a hint that we _suspect_ the user may have meant to set the upstream of the current branch to 'origin/main' (but do so only when 'origin/main' does exist), and tell them the right way to spell that request.

or something perhaps?

#4 lore raw

Re: [PATCH 1/2] branch: suggest <remote>/<branch> on upstream slip

In reply to Junio C Hamano
Quoted 26 lines · Junio C Hamano
Quoted 5 lines · "Harald Nordgren via GitGitGadget

From: Harald Nordgren <[email protected]>

"git branch --set-upstream-to origin main" reads the trailing word as the local branch to operate on and dies with "branch 'main' does not exist", pointing at the wrong problem.

When 'main' does not exist locally,

   $ git branch --set-upstream-to "$anything" main

would fail before even looking at the "$anything" (which is supposed to specify the new_upstream for the named local branch 'main'). The operation is to set the upstream for 'main', and if 'main' does not exist, doesn't the user deserve the error that says 'main' does not exist, no matter what "$anything" is, whether it is a well-formed or ill-formed remote tracking branch name?

So it is unclear, at least to me, why "branch 'main' does not exist" is an inappropriate message, mostly because these three lines does not clearly tell me what the user _expected_ the command line to do.

Quoted 6 lines

When 'main' does exist, but named upstream "$anything" does not, we get

   $ git branch sample master ;# make sure the thing exists
   $ git branch --set-upstream-to origin sample
   fatal: the requested upstream branch 'origin' does not exist

Relatedly, if memory serves: when origin/HEAD is available locally, this works to set sample’s upstream branch to whatever origin/HEAD refers to, right? So it may not even be a mistake. (I’m pretty sure I’ve used something like « git branch -u origin » when I didn’t give an upstream of « origin » to « git switch -c branch » for various reasons.)

#5 lore raw

PATCH 2/2 push: suggest <remote> <branch> for a slash slip

In reply to Harald Nordgren via GitGitGadget

From: Harald Nordgren <[email protected]>

"git push origin/main" is treated as a repository and dies with "'origin/main' does not appear to be a git repository", with no hint that a space was meant instead of a slash.

When the argument is not an existing path or configured remote but its part before the first slash names one, suggest the intended "git push <remote> <branch>" form. The suggestion is shown as advice so it can be silenced with advice.pushRepoLooksLikeRef.

---
 Documentation/config/advice.adoc |  5 +++++
 advice.c                         |  1 +
 advice.h                         |  1 +
 builtin/push.c                   | 26 +++++++++++++++++++++++++-
 t/t5529-push-errors.sh           | 31 +++++++++++++++++++++++++++++++
 5 files changed, 63 insertions(+), 1 deletion(-)
Documentation/config/advice.adoc +5 -0
index 257db58918..fa77a5110e 100644
@@ -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
advice.c +1 -0
index 0018501b7b..63bf8b0c5f 100644
@@ -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 */
advice.h +1 -0
index 8def280688..66f6cd6a77 100644
@@ -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,
builtin/push.c +25 -1
index 6021b71d66..c21febadbe 100644
@@ -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"
@@ -744,6 +745,29 @@ int cmd_push(int argc,
 
 	if (repo) {
 		if (!add_remote_or_group(repo, &remote_group)) {
+			const char *slash = strchr(repo, '/');
+			struct remote *r;
+
+			/*
+			 * A "<remote>/<branch>" argument that does not name
+			 * a path is likely a slip for the separate
+			 * "<remote> <branch>" form, so suggest that instead.
+			 */
+			if (slash && slash[1] && !file_exists(repo)) {
+				struct strbuf name = STRBUF_INIT;
+
+				strbuf_add(&name, repo, slash - repo);
+				if (remote_is_configured(remote_get(name.buf), 0)) {
+					int 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);
+				}
+				strbuf_release(&name);
+			}
+
 			/*
 			 * Not a configured remote name or group name.
 			 * Try treating it as a direct URL or path, e.g.
@@ -753,7 +777,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);
t/t5529-push-errors.sh +31 -0
index 80b06a0cd2..cfb294305d 100755
@@ -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 &&
+	grep "${SQ}origin/main${SQ} is not a valid push target" stderr &&
+	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 &&
+	! 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 &&
+	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 &&
+	! 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 &&
+	! 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 &&
+	! 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 &&
Signature · 1 line
gitgitgadget
#6 lore raw

Re: [PATCH 2/2] push: suggest <remote> <branch> for a slash slip

Quoted 5 lines · Harald Nordgren via GitGitGadget

From: Harald Nordgren <[email protected]>

"git push origin/main" is treated as a repository and dies with "'origin/main' does not appear to be a git repository", with no hint that a space was meant instead of a slash.

This is easier for me to guess than what the user may have wanted to do in the decription of [1/2]. But it still will be easier on readers to say

When pusing out up update the "main" branch to the remote "origin", i.e.,

$ git push origin main

it is easy for some users to mistakenly say

$ git push origin/main

instead. This however instructs to push to remote "origin/main" with configured refspecs, which means a completely different thing. Lucikly, often origin/main does not exist as a remote and the command fails without doing any harm, but still may leave the user puzzled what happened. Give hint to ...

or something like that.

Quoted 4 lines

When the argument is not an existing path or configured remote but its part before the first slash names one, suggest the intended "git push <remote> <branch>" form. The suggestion is shown as advice so it can be silenced with advice.pushRepoLooksLikeRef.

Sounds sensible.

Quoted 14 lines
builtin/push.c +12 -0
 	if (repo) {
 		if (!add_remote_or_group(repo, &remote_group)) {
+			const char *slash = strchr(repo, '/');
+			struct remote *r;
+
+			/*
+			 * A "<remote>/<branch>" argument that does not name
+			 * a path is likely a slip for the separate
+			 * "<remote> <branch>" form, so suggest that instead.
+			 */
+			if (slash && slash[1] && !file_exists(repo)) {
+				struct strbuf name = STRBUF_INIT;
+
+				strbuf_add(&name, repo, slash - repo);

builtin/push.c int cmd_push(int argc, · line 769

Quoted 10 lines
builtin/push.c +10 -0
+				if (remote_is_configured(remote_get(name.buf), 0)) {
+					int 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);
+				}
+				strbuf_release(&name);
+			}

Hmph, if this class of hint is not enabled, do we still have to spend cycles on these "is this a remote? is the first token a remote?" computation? I would have expected that a change here would be a two-liner:

    if (!add_remote_or_group(...)) {
+	if (advise_enabled(ADVICE_PUSH_REPO_LOOKS_LIKE_REF))
+		die_if_plausible_typo(...);
	... do the "try treating it as a direct URL or path" thing ...
    }

with the bulk of the "if it has slash, it is not a file, then advise and die" logic inside the new helper function.

What I find especially troubling is that even when advise for this class of hint is not enabled, the new code will hit the new exit(), without falling back to the "try treating it as a direct URL or path" thing. Or am I missing something?

Thanks.

#7 lore raw
In reply to Harald Nordgren via GitGitGadget

Would be nice to get some feedback here.

This is maybe not the most pressing issue, but I remember being quite dumbfounded when I was a new Git user; not understanding why some commands required one format "origin/main" and other required "origin main". It would be better if we could actually solve it, but I suspect it's hard without breaking backward compatibility, so maybe this is a good middle road?

Harald

#8 lore raw
Quoted 4 lines · Harald Nordgren

This is maybe not the most pressing issue, but I remember being quite dumbfounded when I was a new Git user; not understanding why some commands required one format "origin/main" and other required "origin main".

Can't agree more. It's quite confusing for new users like me.

#9 lore raw
In reply to Harald Nordgren via GitGitGadget
Quoted 4 lines · Harald Nordgren via GitGitGadget

When the repository or upstream argument is a slip like "origin/main" or "origin main", suggest the intended "git push origin main" or "git branch --set-upstream-to=origin/main" form instead of failing with an unrelated error.

Sorry for asking a question that may be stupid, but what does the word "slip" mean in the context of the above sentence? I am having a hard time coming up with a topic name while queuing these two patches (an obvious candidate is hn/branch-push-slip-advise but I do not know how well the word sits there).

Thanks.

#10 lore raw

By slip it meant mistake, so you can call it 'hn/branch-push-mistake-advise'

Harald

#11 lore raw

v2 cover branch/push: suggest intended form when remote/branch slip given

In reply to Harald Nordgren via GitGitGadget

When the repository or upstream argument is a slip like "origin/main" or "origin main", suggest the intended "git push origin main" or "git branch --set-upstream-to=origin/main" form instead of failing with an unrelated error.

Changes in v2:

* Rewrote both commit messages to lead with the intended command, the easy slip, and the resulting error, instead of the terse original.
* Gated each suggestion on advice_enabled() up front, so a user who silenced the hint pays no remote/ref lookups and falls through to the original error. Extracted the detection logic into helpers (die_if_repo_looks_like_ref, die_if_upstream_looks_like_remote) so each call site reads as a single guarded line.

Harald Nordgren (2):
branch: suggest <remote>/<branch> on upstream slip
push: suggest <remote> <branch> for a slash slip

 Documentation/config/advice.adoc |  5 +++++
 advice.c                         |  1 +
 advice.h                         |  1 +
 builtin/branch.c                 | 26 ++++++++++++++++++++++
 builtin/push.c                   | 31 +++++++++++++++++++++++++-
 t/t3200-branch.sh                | 38 ++++++++++++++++++++++++++++++++
 t/t5529-push-errors.sh           | 31 ++++++++++++++++++++++++++
 7 files changed, 132 insertions(+), 1 deletion(-)

base-commit: ab776a62a78576513ee121424adb19597fbb7613
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2331%2FHaraldNordgren%2Fsuggest-remote-branch-slips-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2331/HaraldNordgren/suggest-remote-branch-slips-v2
Pull-Request: https://github.com/git/git/pull/2331

Range-diff vs v1:

1: 21684539de ! 1: 11bcecebf4 branch: suggest <remote>/<branch> on upstream slip

     @@ Metadata
       ## Commit message ##
          branch: suggest <remote>/<branch> on upstream slip
      
     -    "git branch --set-upstream-to origin main" reads the trailing word as
     -    the local branch to operate on and dies with "branch 'main' does not
     -    exist", pointing at the wrong problem.
     +    When setting the upstream of the current branch to the 'main' branch
     +    of the remote 'origin', i.e.,
      
     -    When that branch is missing and "<remote>/<branch>" names a real
     -    remote-tracking ref, suggest the intended
     -    "git branch --set-upstream-to=<remote>/<branch>" form.
     +        $ git branch --set-upstream-to origin/main
     +
     +    it is easy to mistakenly write
     +
     +        $ git branch --set-upstream-to origin main
     +
     +    That is parsed as a request to set the upstream of the local branch
     +    'main' to 'origin'. When 'main' does not exist, the command dies
     +    with:
     +
     +        fatal: branch 'main' does not exist
     +
     +    pointing at a branch the user never meant to name.
     +
     +    When the operated-on branch is missing and '<remote>/<branch>' names
     +    a real remote-tracking ref, suggest the intended form:
     +
     +        $ git branch --set-upstream-to=origin/main
     +
     +    The suggestion is gated on '<remote>/<branch>' existing so it only
     +    appears when a slipped slash is the likely explanation.
      
          Signed-off-by: Harald Nordgren <[email protected]>
      
       ## builtin/branch.c ##
     +@@ builtin/branch.c: static int edit_branch_description(const char *branch_name)
     + 	return 0;
     + }
     + 
     ++static void die_if_upstream_looks_like_remote(const char *new_upstream, const char *branch_name)
     ++{
     ++	struct strbuf remote_ref = STRBUF_INIT;
     ++	int code;
     ++
     ++	if (strchr(new_upstream, '/') ||
     ++	    !remote_is_configured(remote_get(new_upstream), 0))
     ++		return;
     ++
     ++	strbuf_addf(&remote_ref, "refs/remotes/%s/%s", new_upstream, branch_name);
     ++	if (!refs_ref_exists(get_main_ref_store(the_repository), remote_ref.buf)) {
     ++		strbuf_release(&remote_ref);
     ++		return;
     ++	}
     ++
     ++	code = die_message(_("--set-upstream-to takes a single <remote>/<branch> argument"));
     ++	advise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,
     ++			  _("Did you mean to use: git branch --set-upstream-to=%s/%s?"),
     ++			  new_upstream, branch_name);
     ++	strbuf_release(&remote_ref);
     ++	exit(code);
     ++}
     ++
     + int cmd_branch(int argc,
     + 	       const char **argv,
     + 	       const char *prefix,
      @@ builtin/branch.c: int cmd_branch(int argc,
       		if (!refs_ref_exists(get_main_ref_store(the_repository), branch->refname)) {
       			if (!argc || branch_checked_out(branch->refname))
       				die(_("no commit on branch '%s' yet"), branch->name);
     -+			if (argc == 1 && !strchr(new_upstream, '/') &&
     -+			    remote_is_configured(remote_get(new_upstream), 0)) {
     -+				struct strbuf remote_ref = STRBUF_INIT;
     -+
     -+				strbuf_addf(&remote_ref, "refs/remotes/%s/%s",
     -+					    new_upstream, argv[0]);
     -+				if (refs_ref_exists(get_main_ref_store(the_repository),
     -+						    remote_ref.buf)) {
     -+					int code = die_message(_("--set-upstream-to takes a single <remote>/<branch> argument"));
     -+					advise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,
     -+							  _("Did you mean to use: git branch --set-upstream-to=%s/%s?"),
     -+							  new_upstream, argv[0]);
     -+					strbuf_release(&remote_ref);
     -+					exit(code);
     -+				}
     -+				strbuf_release(&remote_ref);
     -+			}
     ++			if (argc == 1 &&
     ++			    advice_enabled(ADVICE_SET_UPSTREAM_FAILURE))
     ++				die_if_upstream_looks_like_remote(new_upstream, argv[0]);
       			die(_("branch '%s' does not exist"), branch->name);
       		}

2: ea1412b110 ! 2: 49de5a925d push: suggest <remote> <branch> for a slash slip

     @@ Metadata
       ## Commit message ##
          push: suggest <remote> <branch> for a slash slip
      
     -    "git push origin/main" is treated as a repository and dies with
     -    "'origin/main' does not appear to be a git repository", with no hint
     -    that a space was meant instead of a slash.
     +    When pushing the 'main' branch to the remote 'origin', i.e.,
      
     -    When the argument is not an existing path or configured remote but its
     -    part before the first slash names one, suggest the intended
     -    "git push <remote> <branch>" form. The suggestion is shown as advice so
     -    it can be silenced with advice.pushRepoLooksLikeRef.
     +        $ 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 <[email protected]>
      
     @@ builtin/push.c
       #include "environment.h"
       #include "gettext.h"
       #include "hex.h"
     +@@ builtin/push.c: 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,
      @@ builtin/push.c: int cmd_push(int argc,
       
       	if (repo) {
       		if (!add_remote_or_group(repo, &remote_group)) {
     -+			const char *slash = strchr(repo, '/');
      +			struct remote *r;
      +
     -+			/*
     -+			 * A "<remote>/<branch>" argument that does not name
     -+			 * a path is likely a slip for the separate
     -+			 * "<remote> <branch>" form, so suggest that instead.
     -+			 */
     -+			if (slash && slash[1] && !file_exists(repo)) {
     -+				struct strbuf name = STRBUF_INIT;
     -+
     -+				strbuf_add(&name, repo, slash - repo);
     -+				if (remote_is_configured(remote_get(name.buf), 0)) {
     -+					int 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);
     -+				}
     -+				strbuf_release(&name);
     -+			}
     ++			if (advice_enabled(ADVICE_PUSH_REPO_LOOKS_LIKE_REF))
     ++				die_if_repo_looks_like_ref(repo);
      +
       			/*
       			 * Not a configured remote name or group name.
Signature · 1 line
gitgitgadget
#12 lore raw

PATCH v2 1/2 branch: suggest <remote>/<branch> on upstream slip

From: Harald Nordgren <[email protected]>

When setting the upstream of the current branch to the 'main' branch of the remote 'origin', i.e.,

    $ git branch --set-upstream-to origin/main

it is easy to mistakenly write

    $ git branch --set-upstream-to origin main

That is parsed as a request to set the upstream of the local branch 'main' to 'origin'. When 'main' does not exist, the command dies with:

fatal: branch 'main' does not exist

pointing at a branch the user never meant to name.

When the operated-on branch is missing and '<remote>/<branch>' names a real remote-tracking ref, suggest the intended form:

    $ git branch --set-upstream-to=origin/main

The suggestion is gated on '<remote>/<branch>' existing so it only appears when a slipped slash is the likely explanation.

---
 builtin/branch.c  | 26 ++++++++++++++++++++++++++
 t/t3200-branch.sh | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)
builtin/branch.c +26 -0
index 1572a4f9ef..cefc4519a7 100644
@@ -706,6 +706,29 @@ static int edit_branch_description(const char *branch_name)
 	return 0;
 }
 
+static void die_if_upstream_looks_like_remote(const char *new_upstream, const char *branch_name)
+{
+	struct strbuf remote_ref = STRBUF_INIT;
+	int code;
+
+	if (strchr(new_upstream, '/') ||
+	    !remote_is_configured(remote_get(new_upstream), 0))
+		return;
+
+	strbuf_addf(&remote_ref, "refs/remotes/%s/%s", new_upstream, branch_name);
+	if (!refs_ref_exists(get_main_ref_store(the_repository), remote_ref.buf)) {
+		strbuf_release(&remote_ref);
+		return;
+	}
+
+	code = die_message(_("--set-upstream-to takes a single <remote>/<branch> argument"));
+	advise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,
+			  _("Did you mean to use: git branch --set-upstream-to=%s/%s?"),
+			  new_upstream, branch_name);
+	strbuf_release(&remote_ref);
+	exit(code);
+}
+
 int cmd_branch(int argc,
 	       const char **argv,
 	       const char *prefix,
@@ -957,6 +980,9 @@ int cmd_branch(int argc,
 		if (!refs_ref_exists(get_main_ref_store(the_repository), branch->refname)) {
 			if (!argc || branch_checked_out(branch->refname))
 				die(_("no commit on branch '%s' yet"), branch->name);
+			if (argc == 1 &&
+			    advice_enabled(ADVICE_SET_UPSTREAM_FAILURE))
+				die_if_upstream_looks_like_remote(new_upstream, argv[0]);
 			die(_("branch '%s' does not exist"), branch->name);
 		}
 
t/t3200-branch.sh +38 -0
index e7829c2c4b..e2682a83a0 100755
@@ -1022,6 +1022,44 @@ test_expect_success '--set-upstream-to fails on a missing dst branch' '
 	test_cmp expect err
 '
 
+test_expect_success '--set-upstream-to suggests <remote>/<branch> on slip' '
+	test_when_finished "git remote remove slip-remote" &&
+	git remote add slip-remote . &&
+	git update-ref refs/remotes/slip-remote/slip-feature HEAD &&
+	test_must_fail git branch --set-upstream-to slip-remote slip-feature 2>err &&
+	test_grep "takes a single <remote>/<branch> argument" err &&
+	test_grep "hint: Did you mean to use: git branch --set-upstream-to=slip-remote/slip-feature?" err &&
+	test_must_fail git -c advice.setUpstreamFailure=false \
+		branch --set-upstream-to slip-remote slip-feature 2>err &&
+	test_grep ! "Did you mean" err
+'
+
+test_expect_success '--set-upstream-to does not suggest when no matching remote ref' '
+	test_when_finished "git remote remove slip-remote" &&
+	git remote add slip-remote . &&
+	test_must_fail git branch --set-upstream-to slip-remote no-such-branch 2>err &&
+	test_grep "branch ${SQ}no-such-branch${SQ} does not exist" err &&
+	test_grep ! "Did you mean" err
+'
+
+test_expect_success '--set-upstream-to to a local branch is not mistaken for a slip' '
+	git branch slip-local-upstream &&
+	git branch slip-local-target &&
+	git branch --set-upstream-to=slip-local-upstream slip-local-target 2>err &&
+	test_grep ! "Did you mean" err &&
+	echo refs/heads/slip-local-upstream >expect &&
+	git config branch.slip-local-target.merge >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '--set-upstream-to slip suggestion keeps a slashed branch name' '
+	test_when_finished "git remote remove slip-remote" &&
+	git remote add slip-remote . &&
+	git update-ref refs/remotes/slip-remote/slip/feature HEAD &&
+	test_must_fail git branch --set-upstream-to slip-remote slip/feature 2>err &&
+	test_grep "hint: Did you mean to use: git branch --set-upstream-to=slip-remote/slip/feature?" err
+'
+
 test_expect_success '--set-upstream-to fails on a missing src branch' '
 	test_must_fail git branch --set-upstream-to does-not-exist main 2>err &&
 	test_grep "the requested upstream branch '"'"'does-not-exist'"'"' does not exist" err
Signature · 1 line
gitgitgadget
#13 lore raw

Re: [PATCH v2 1/2] branch: suggest <remote>/<branch> on upstream slip

Quoted 11 lines · Harald Nordgren via GitGitGadget

From: Harald Nordgren <[email protected]>

When setting the upstream of the current branch to the 'main' branch of the remote 'origin', i.e.,

    $ git branch --set-upstream-to origin/main

it is easy to mistakenly write

    $ git branch --set-upstream-to origin main
Quoted 7 lines

That is parsed as a request to set the upstream of the local branch 'main' to 'origin'. When 'main' does not exist, the command dies with:

fatal: branch 'main' does not exist

pointing at a branch the user never meant to name.

It is more complete to add the other case here, something along the lines of ...

And then when 'main' does exist, the command would die with

fatal: the requested upstream branch 'origin' does not exist

leaving the user equally confused.

... no? In any case, this is much more nicely described than the previous round. I see no room for confusion.

Quoted 7 lines

When the operated-on branch is missing and '<remote>/<branch>' names a real remote-tracking ref, suggest the intended form:

    $ git branch --set-upstream-to=origin/main

The suggestion is gated on '<remote>/<branch>' existing so it only appears when a slipped slash is the likely explanation.

Makes sense.

Do we want to do anything on a case where the operated-on branch does exist but '<remote>' is not a name suitable for an upstream, but '<remote>/<branch>' is?

Quoted 17 lines
builtin/branch.c +9 -0
index 1572a4f9ef..cefc4519a7 100644
@@ -706,6 +706,29 @@ static int edit_branch_description(const char *branch_name)
 	return 0;
 }
 
+static void die_if_upstream_looks_like_remote(const char *new_upstream, const char *branch_name)
+{
+	struct strbuf remote_ref = STRBUF_INIT;
+	int code;
+
+	if (strchr(new_upstream, '/') ||
+	    !remote_is_configured(remote_get(new_upstream), 0))
+		return;
+

builtin/branch.c static int edit_branch_description(const char *branch_name) · line 727

Quoted 10 lines
builtin/branch.c +10 -0
+	strbuf_addf(&remote_ref, "refs/remotes/%s/%s", new_upstream, branch_name);
+	if (!refs_ref_exists(get_main_ref_store(the_repository), remote_ref.buf)) {
+		strbuf_release(&remote_ref);
+		return;
+	}
+
+	code = die_message(_("--set-upstream-to takes a single <remote>/<branch> argument"));
+	advise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,
+			  _("Did you mean to use: git branch --set-upstream-to=%s/%s?"),
+			  new_upstream, branch_name);

Do we still need the _if_enabled() thing here? Isn't the caller gated with the same condition in this version?

Quoted 8 lines
builtin/branch.c +4 -0
+	strbuf_release(&remote_ref);
+	exit(code);
+}
+
 int cmd_branch(int argc,
 	       const char **argv,
 	       const char *prefix,

builtin/branch.c int cmd_branch(int argc, · line 987

Quoted 8 lines
builtin/branch.c +3 -0
@@ -957,6 +980,9 @@ int cmd_branch(int argc,
 		if (!refs_ref_exists(get_main_ref_store(the_repository), branch->refname)) {
 			if (!argc || branch_checked_out(branch->refname))
 				die(_("no commit on branch '%s' yet"), branch->name);
+			if (argc == 1 &&
+			    advice_enabled(ADVICE_SET_UPSTREAM_FAILURE))
+				die_if_upstream_looks_like_remote(new_upstream, argv[0]);
 			die(_("branch '%s' does not exist"), branch->name);
 		}

This is totally a tangent, but has anybody noticed that the web interface to the lore archive seems to be constipated? I am reading over nntp and subscribers are reading from their inbox, so no real harm done, but from time to time we get reminded how heavily our development process relies on the services like kernel.org and feel grateful to have them.

#14 lore raw

Re: [PATCH v2 1/2] branch: suggest <remote>/<branch> on upstream slip

Quoted 19 lines

Do we still need the _if_enabled() thing here? Isn't the caller gated with the same condition in this version?

Quoted 8 lines
builtin/branch.c +4 -0
+     strbuf_release(&remote_ref);
+     exit(code);
+}
+
 int cmd_branch(int argc,
             const char **argv,
             const char *prefix,

builtin/branch.c int cmd_branch(int argc, · line 987

Quoted 8 lines
builtin/branch.c +3 -0
@@ -957,6 +980,9 @@ int cmd_branch(int argc,
              if (!refs_ref_exists(get_main_ref_store(the_repository), branch->refname)) {
                      if (!argc || branch_checked_out(branch->refname))
                              die(_("no commit on branch '%s' yet"), branch->name);
+                     if (argc == 1 &&
+                         advice_enabled(ADVICE_SET_UPSTREAM_FAILURE))
+                             die_if_upstream_looks_like_remote(new_upstream, argv[0]);
                      die(_("branch '%s' does not exist"), branch->name);
              }

I think we do, so it will give the advice and tell the user that it can be disabled in the standard format.

Harald

#15 lore raw

Re: [PATCH v2 1/2] branch: suggest <remote>/<branch> on upstream slip

Quoted 14 lines · Harald Nordgren
Quoted 14 lines

Do we still need the _if_enabled() thing here? Isn't the caller gated with the same condition in this version?

Quoted 11 lines
builtin/branch.c +4 -0
+     strbuf_release(&remote_ref);
+     exit(code);
+}
+
 int cmd_branch(int argc,
             const char **argv,
             const char *prefix,
@@ -957,6 +980,9 @@ int cmd_branch(int argc,
              if (!refs_ref_exists(get_main_ref_store(the_repository), branch->refname)) {
                      if (!argc || branch_checked_out(branch->refname))
                              die(_("no commit on branch '%s' yet"), branch->name);
Quoted 8 lines
Quoted 5 lines

builtin/branch.c int cmd_branch(int argc, · line 987

Quoted 5 lines
builtin/branch.c +3 -0
+                     if (argc == 1 &&
+                         advice_enabled(ADVICE_SET_UPSTREAM_FAILURE))
+                             die_if_upstream_looks_like_remote(new_upstream, argv[0]);
                      die(_("branch '%s' does not exist"), branch->name);
              }

I think we do, so it will give the advice and tell the user that it can be disabled in the standard format.

I was hoping that unconditional advise() should be sufficient, but the caller there needs to say if_enabled, even though it _knows_ that it is enabled, only to give the turn-off instructions.

I wonder if future readers would be confused just like I was, without a comment on the callsite of _if_enabled() added by this patch?

Thanks.

#16 lore raw

PATCH v2 2/2 push: suggest <remote> <branch> for a slash slip

In reply to Harald Nordgren via GitGitGadget

From: Harald Nordgren <[email protected]>

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.

---
 Documentation/config/advice.adoc |  5 +++++
 advice.c                         |  1 +
 advice.h                         |  1 +
 builtin/push.c                   | 31 ++++++++++++++++++++++++++++++-
 t/t5529-push-errors.sh           | 31 +++++++++++++++++++++++++++++++
 5 files changed, 68 insertions(+), 1 deletion(-)
Documentation/config/advice.adoc +5 -0
index 257db58918..fa77a5110e 100644
@@ -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
advice.c +1 -0
index 0018501b7b..63bf8b0c5f 100644
@@ -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 */
advice.h +1 -0
index 8def280688..66f6cd6a77 100644
@@ -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,
builtin/push.c +30 -1
index 6021b71d66..255556b44d 100644
@@ -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,11 @@ int cmd_push(int argc,
 
 	if (repo) {
 		if (!add_remote_or_group(repo, &remote_group)) {
+			struct remote *r;
+
+			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 +782,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);
t/t5529-push-errors.sh +31 -0
index 80b06a0cd2..cfb294305d 100755
@@ -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 &&
+	grep "${SQ}origin/main${SQ} is not a valid push target" stderr &&
+	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 &&
+	! 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 &&
+	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 &&
+	! 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 &&
+	! 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 &&
+	! 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 &&
Signature · 1 line
gitgitgadget
#17 lore raw

Re: [PATCH v2 2/2] push: suggest <remote> <branch> for a slash slip

Quoted 20 lines · Harald Nordgren via GitGitGadget
t/t5529-push-errors.sh +12 -0
index 80b06a0cd2..cfb294305d 100755
@@ -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 &&
+	grep "${SQ}origin/main${SQ} is not a valid push target" stderr &&
+	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 &&
+	! 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 &&
+	grep "hint: Did you mean to use: git push origin feature/x?" stderr
+'

t/t5529-push-errors.sh test_expect_success 'detect empty remote with targeted refspec' ' · line 78

Quoted 10 lines
t/t5529-push-errors.sh +10 -0
+
+test_expect_success 'no suggestion when prefix is not a configured remote' '
+	test_must_fail git push not-a-remote/main 2>stderr &&
+	! 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 &&
+	! grep "Did you mean" stderr
+'

t5529-push-errors.sh:59: error: bare grep outside pipeline (use test_grep) t5529-push-errors.sh:60: error: bare grep outside pipeline (use test_grep) t5529-push-errors.sh:62: error: bare grep outside pipeline (use test_grep) t5529-push-errors.sh:67: error: bare grep outside pipeline (use test_grep) t5529-push-errors.sh:72: error: bare grep outside pipeline (use test_grep) t5529-push-errors.sh:77: error: bare grep outside pipeline (use test_grep) t5529-push-errors.sh:84: error: bare grep outside pipeline (use test_grep)

#18 lore raw

Re: [PATCH v2 2/2] push: suggest <remote> <branch> for a slash slip

Quoted 16 lines · Junio C Hamano

t/t5529-push-errors.sh test_expect_success 'detect empty remote with targeted refspec' ' · line 55

Quoted 7 lines · Harald Nordgren via GitGitGadget
t/t5529-push-errors.sh +0 -0
index 80b06a0cd2..cfb294305d 100755
@@ -54,6 +54,37 @@ test_expect_success 'detect empty remote with targeted refspec' '
 	grep "fatal: bad repository ${SQ}${SQ}" stderr
 '

t5529-push-errors.sh:59: error: bare grep outside pipeline (use test_grep) t5529-push-errors.sh:60: error: bare grep outside pipeline (use test_grep) t5529-push-errors.sh:62: error: bare grep outside pipeline (use test_grep) t5529-push-errors.sh:67: error: bare grep outside pipeline (use test_grep) t5529-push-errors.sh:72: error: bare grep outside pipeline (use test_grep) t5529-push-errors.sh:77: error: bare grep outside pipeline (use test_grep) t5529-push-errors.sh:84: error: bare grep outside pipeline (use test_grep)

I've queued this squashable? fix on top of the branch before merging the result to 'seen' for tonight's push-out.

Thanks.

--- >8 ---
Subject: [PATCH] SQUASH??? use test_grep

---
 t/t5529-push-errors.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
t/t5529-push-errors.sh +7 -7
index cfb294305d..2294645902 100755
@@ -56,32 +56,32 @@ test_expect_success 'detect empty remote with targeted refspec' '
 
 test_expect_success 'suggest <remote> <branch> for a <remote>/<branch> slip' '
 	test_must_fail git push origin/main 2>stderr &&
-	grep "${SQ}origin/main${SQ} is not a valid push target" stderr &&
-	grep "hint: Did you mean to use: git push origin main?" 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 &&
-	! grep "Did you mean" 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 &&
-	grep "hint: Did you mean to use: git push origin feature/x?" 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 &&
-	! grep "Did you mean" 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 &&
-	! grep "Did you mean" 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 &&
-	! grep "Did you mean" stderr &&
+	test_grep ! "Did you mean" stderr &&
 	git -C origin/main rev-parse --verify refs/heads/pushed
 '
 
Signature · 1 line
2.55.0-rc2-165-g3249676ba5
#19 lore raw

Re: [PATCH v2 2/2] push: suggest <remote> <branch> for a slash slip

Quoted 19 lines
Quoted 16 lines · Junio C Hamano

t/t5529-push-errors.sh test_expect_success 'detect empty remote with targeted refspec' ' · line 55

Quoted 7 lines · Harald Nordgren via GitGitGadget
t/t5529-push-errors.sh +0 -0
index 80b06a0cd2..cfb294305d 100755
@@ -54,6 +54,37 @@ test_expect_success 'detect empty remote with targeted refspec' '
     grep "fatal: bad repository ${SQ}${SQ}" stderr
 '

t5529-push-errors.sh:59: error: bare grep outside pipeline (use test_grep) t5529-push-errors.sh:60: error: bare grep outside pipeline (use test_grep) t5529-push-errors.sh:62: error: bare grep outside pipeline (use test_grep) t5529-push-errors.sh:67: error: bare grep outside pipeline (use test_grep) t5529-push-errors.sh:72: error: bare grep outside pipeline (use test_grep) t5529-push-errors.sh:77: error: bare grep outside pipeline (use test_grep) t5529-push-errors.sh:84: error: bare grep outside pipeline (use test_grep)

Quoted 4 lines

I've queued this squashable? fix on top of the branch before merging the result to 'seen' for tonight's push-out.

Thanks.

Thank you!

Is someone working on fixing the GitHub CI? I used to rely on it before, it would have caught this, and now I'm relying on "if less than five GitHub CI tests are failing (with the 4GB warning) and tests are passing locally, then I can submit" which is admittedly not a good heuristic.

Harald

#20 lore raw

Re: [PATCH v2 2/2] push: suggest <remote> <branch> for a slash slip

Quoted 3 lines · Harald Nordgren

than five GitHub CI tests are failing (with the 4GB warning) and tests are passing locally, then I can submit" which is admittedly not a good heuristic.

This particular one you would have caught if you try-merged your topic to 'seen' and ran test locally. That is how I found it before pushing the integration result out.

#21 lore raw

v3 cover branch/push: suggest intended form when remote/branch slip given

In reply to Harald Nordgren via GitGitGadget

When the repository or upstream argument is a slip like "origin/main" or "origin main", suggest the intended "git push origin main" or "git branch --set-upstream-to=origin/main" form instead of failing with an unrelated error.

Changes in v3:

* Reworded the branch commit message to also show the second confusing error (the requested upstream branch 'origin' does not exist) for when the operated-on branch does exist.
* Converted the new t5529 tests from bare grep/! grep to test_grep to fix the test-lint failures.
* Added a brief call-site comment explaining the up-front advice_enabled() gate (skips the lookups when the hint is off, while advise_if_enabled() still prints the disable-message footer).

Changes in v2:

* Rewrote both commit messages to lead with the intended command, the easy slip, and the resulting error, instead of the terse original.
* Gated each suggestion on advice_enabled() up front, so a user who silenced the hint pays no remote/ref lookups and falls through to the original error. Extracted the detection logic into helpers (die_if_repo_looks_like_ref, die_if_upstream_looks_like_remote) so each call site reads as a single guarded line.

Harald Nordgren (2):
branch: suggest <remote>/<branch> on upstream slip
push: suggest <remote> <branch> for a slash slip

 Documentation/config/advice.adoc |  5 +++++
 advice.c                         |  1 +
 advice.h                         |  1 +
 builtin/branch.c                 | 32 +++++++++++++++++++++++++++
 builtin/push.c                   | 37 ++++++++++++++++++++++++++++++-
 t/t3200-branch.sh                | 38 ++++++++++++++++++++++++++++++++
 t/t5529-push-errors.sh           | 31 ++++++++++++++++++++++++++
 7 files changed, 144 insertions(+), 1 deletion(-)

base-commit: ab776a62a78576513ee121424adb19597fbb7613
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2331%2FHaraldNordgren%2Fsuggest-remote-branch-slips-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2331/HaraldNordgren/suggest-remote-branch-slips-v3
Pull-Request: https://github.com/git/git/pull/2331

Range-diff vs v2:

1: 11bcecebf4 ! 1: 9883c28482 branch: suggest <remote>/<branch> on upstream slip
@@ Commit message

fatal: branch 'main' does not exist

- pointing at a branch the user never meant to name.
+ pointing at a branch the user never meant to name. When 'main' does
+ exist, it instead dies with:
+
+ fatal: the requested upstream branch 'origin' does not exist
+
+ leaving the user equally puzzled.

When the operated-on branch is missing and '<remote>/<branch>' names a real remote-tracking ref, suggest the intended form:
@@ builtin/branch.c: int cmd_branch(int argc,
if (!refs_ref_exists(get_main_ref_store(the_repository), branch->refname)) {
if (!argc || branch_checked_out(branch->refname))
die(_("no commit on branch '%s' yet"), branch->name);
++ /*
++ * Check the advice up front to avoid the ref
++ * lookups when the hint is off. The helper still
++ * calls advise_if_enabled() so the hint carries the
++ * standard "disable this message" instructions.
++ */
+ if (argc == 1 &&
+ advice_enabled(ADVICE_SET_UPSTREAM_FAILURE))
+ die_if_upstream_looks_like_remote(new_upstream, argv[0]);
2: 49de5a925d ! 2: dbe4dbc346 push: suggest <remote> <branch> for a slash slip

     @@ builtin/push.c: int cmd_push(int argc,
       		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);
      +
     @@ t/t5529-push-errors.sh: test_expect_success 'detect empty remote with targeted r
       
      +test_expect_success 'suggest <remote> <branch> for a <remote>/<branch> slip' '
      +	test_must_fail git push origin/main 2>stderr &&
     -+	grep "${SQ}origin/main${SQ} is not a valid push target" stderr &&
     -+	grep "hint: Did you mean to use: git push origin main?" 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 &&
     -+	! grep "Did you mean" 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 &&
     -+	grep "hint: Did you mean to use: git push origin feature/x?" 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 &&
     -+	! grep "Did you mean" 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 &&
     -+	! grep "Did you mean" 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 &&
     -+	! grep "Did you mean" stderr &&
     ++	test_grep ! "Did you mean" stderr &&
      +	git -C origin/main rev-parse --verify refs/heads/pushed
      +'
      +
Signature · 1 line
gitgitgadget
#22 lore raw

PATCH v3 1/2 branch: suggest <remote>/<branch> on upstream slip

From: Harald Nordgren <[email protected]>

When setting the upstream of the current branch to the 'main' branch of the remote 'origin', i.e.,

    $ git branch --set-upstream-to origin/main

it is easy to mistakenly write

    $ git branch --set-upstream-to origin main

That is parsed as a request to set the upstream of the local branch 'main' to 'origin'. When 'main' does not exist, the command dies with:

fatal: branch 'main' does not exist

pointing at a branch the user never meant to name. When 'main' does exist, it instead dies with:

fatal: the requested upstream branch 'origin' does not exist

leaving the user equally puzzled.

When the operated-on branch is missing and '<remote>/<branch>' names a real remote-tracking ref, suggest the intended form:

    $ git branch --set-upstream-to=origin/main

The suggestion is gated on '<remote>/<branch>' existing so it only appears when a slipped slash is the likely explanation.

---
 builtin/branch.c  | 32 ++++++++++++++++++++++++++++++++
 t/t3200-branch.sh | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+)
builtin/branch.c +32 -0
index 1572a4f9ef..dede60d27b 100644
@@ -706,6 +706,29 @@ static int edit_branch_description(const char *branch_name)
 	return 0;
 }
 
+static void die_if_upstream_looks_like_remote(const char *new_upstream, const char *branch_name)
+{
+	struct strbuf remote_ref = STRBUF_INIT;
+	int code;
+
+	if (strchr(new_upstream, '/') ||
+	    !remote_is_configured(remote_get(new_upstream), 0))
+		return;
+
+	strbuf_addf(&remote_ref, "refs/remotes/%s/%s", new_upstream, branch_name);
+	if (!refs_ref_exists(get_main_ref_store(the_repository), remote_ref.buf)) {
+		strbuf_release(&remote_ref);
+		return;
+	}
+
+	code = die_message(_("--set-upstream-to takes a single <remote>/<branch> argument"));
+	advise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,
+			  _("Did you mean to use: git branch --set-upstream-to=%s/%s?"),
+			  new_upstream, branch_name);
+	strbuf_release(&remote_ref);
+	exit(code);
+}
+
 int cmd_branch(int argc,
 	       const char **argv,
 	       const char *prefix,
@@ -957,6 +980,15 @@ int cmd_branch(int argc,
 		if (!refs_ref_exists(get_main_ref_store(the_repository), branch->refname)) {
 			if (!argc || branch_checked_out(branch->refname))
 				die(_("no commit on branch '%s' yet"), branch->name);
+			/*
+			 * Check the advice up front to avoid the ref
+			 * lookups when the hint is off. The helper still
+			 * calls advise_if_enabled() so the hint carries the
+			 * standard "disable this message" instructions.
+			 */
+			if (argc == 1 &&
+			    advice_enabled(ADVICE_SET_UPSTREAM_FAILURE))
+				die_if_upstream_looks_like_remote(new_upstream, argv[0]);
 			die(_("branch '%s' does not exist"), branch->name);
 		}
 
t/t3200-branch.sh +38 -0
index e7829c2c4b..e2682a83a0 100755
@@ -1022,6 +1022,44 @@ test_expect_success '--set-upstream-to fails on a missing dst branch' '
 	test_cmp expect err
 '
 
+test_expect_success '--set-upstream-to suggests <remote>/<branch> on slip' '
+	test_when_finished "git remote remove slip-remote" &&
+	git remote add slip-remote . &&
+	git update-ref refs/remotes/slip-remote/slip-feature HEAD &&
+	test_must_fail git branch --set-upstream-to slip-remote slip-feature 2>err &&
+	test_grep "takes a single <remote>/<branch> argument" err &&
+	test_grep "hint: Did you mean to use: git branch --set-upstream-to=slip-remote/slip-feature?" err &&
+	test_must_fail git -c advice.setUpstreamFailure=false \
+		branch --set-upstream-to slip-remote slip-feature 2>err &&
+	test_grep ! "Did you mean" err
+'
+
+test_expect_success '--set-upstream-to does not suggest when no matching remote ref' '
+	test_when_finished "git remote remove slip-remote" &&
+	git remote add slip-remote . &&
+	test_must_fail git branch --set-upstream-to slip-remote no-such-branch 2>err &&
+	test_grep "branch ${SQ}no-such-branch${SQ} does not exist" err &&
+	test_grep ! "Did you mean" err
+'
+
+test_expect_success '--set-upstream-to to a local branch is not mistaken for a slip' '
+	git branch slip-local-upstream &&
+	git branch slip-local-target &&
+	git branch --set-upstream-to=slip-local-upstream slip-local-target 2>err &&
+	test_grep ! "Did you mean" err &&
+	echo refs/heads/slip-local-upstream >expect &&
+	git config branch.slip-local-target.merge >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '--set-upstream-to slip suggestion keeps a slashed branch name' '
+	test_when_finished "git remote remove slip-remote" &&
+	git remote add slip-remote . &&
+	git update-ref refs/remotes/slip-remote/slip/feature HEAD &&
+	test_must_fail git branch --set-upstream-to slip-remote slip/feature 2>err &&
+	test_grep "hint: Did you mean to use: git branch --set-upstream-to=slip-remote/slip/feature?" err
+'
+
 test_expect_success '--set-upstream-to fails on a missing src branch' '
 	test_must_fail git branch --set-upstream-to does-not-exist main 2>err &&
 	test_grep "the requested upstream branch '"'"'does-not-exist'"'"' does not exist" err
Signature · 1 line
gitgitgadget
#23 lore raw

Re: [PATCH v3 1/2] branch: suggest <remote>/<branch> on upstream slip

Quoted 39 lines · Harald Nordgren via GitGitGadget
builtin/branch.c +24 -0
index 1572a4f9ef..dede60d27b 100644
@@ -706,6 +706,29 @@ static int edit_branch_description(const char *branch_name)
 	return 0;
 }
 
+static void die_if_upstream_looks_like_remote(const char *new_upstream, const char *branch_name)
+{
+	struct strbuf remote_ref = STRBUF_INIT;
+	int code;
+
+	if (strchr(new_upstream, '/') ||
+	    !remote_is_configured(remote_get(new_upstream), 0))
+		return;
+
+	strbuf_addf(&remote_ref, "refs/remotes/%s/%s", new_upstream, branch_name);
+	if (!refs_ref_exists(get_main_ref_store(the_repository), remote_ref.buf)) {
+		strbuf_release(&remote_ref);
+		return;
+	}
+
+	code = die_message(_("--set-upstream-to takes a single <remote>/<branch> argument"));
+	advise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,
+			  _("Did you mean to use: git branch --set-upstream-to=%s/%s?"),
+			  new_upstream, branch_name);
+	strbuf_release(&remote_ref);
+	exit(code);
+}
+
 int cmd_branch(int argc,
 	       const char **argv,
 	       const char *prefix,
@@ -957,6 +980,15 @@ int cmd_branch(int argc,
 		if (!refs_ref_exists(get_main_ref_store(the_repository), branch->refname)) {
 			if (!argc || branch_checked_out(branch->refname))
 				die(_("no commit on branch '%s' yet"), branch->name);
+			/*

builtin/branch.c int cmd_branch(int argc, · line 993

Quoted 10 lines
builtin/branch.c +8 -0
+			 * Check the advice up front to avoid the ref
+			 * lookups when the hint is off. The helper still
+			 * calls advise_if_enabled() so the hint carries the
+			 * standard "disable this message" instructions.
+			 */
+			if (argc == 1 &&
+			    advice_enabled(ADVICE_SET_UPSTREAM_FAILURE))
+				die_if_upstream_looks_like_remote(new_upstream, argv[0]);
 			die(_("branch '%s' does not exist"), branch->name);
 		}

Hmph, something like adding a single liner in the caller, like this. ...

	code = die_message(_("--set-upstream-to takes a single <remote>/<branch> argument"));
+	/* use _if_enabled here to show the hint on how to disable */
	advise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,
			  _("Did you mean to use: git branch --set-upstream-to=%s/%s?"),
			  new_upstream, branch_name);
	strbuf_release(&remote_ref);
	exit(code);

... was what I meant, because the most puzzling piece is that the function calls _if_enabled form there, when the caller is presumably already checked _enabled() and leaves the reader wondering if there are other callers of this function that does not check before calling it.

But this is so tiny a thing that once the code is written, it is probably not worth the churn to redo it. Let's declare victory and mark the topic ready for 'next'?

#24 lore raw

Re: [PATCH v3 1/2] branch: suggest <remote>/<branch> on upstream slip

Let's do it!

Harald

#25 lore raw

PATCH v3 2/2 push: suggest <remote> <branch> for a slash slip

In reply to Harald Nordgren via GitGitGadget

From: Harald Nordgren <[email protected]>

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.

---
 Documentation/config/advice.adoc |  5 +++++
 advice.c                         |  1 +
 advice.h                         |  1 +
 builtin/push.c                   | 37 +++++++++++++++++++++++++++++++-
 t/t5529-push-errors.sh           | 31 ++++++++++++++++++++++++++
 5 files changed, 74 insertions(+), 1 deletion(-)
Documentation/config/advice.adoc +5 -0
index 257db58918..fa77a5110e 100644
@@ -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
advice.c +1 -0
index 0018501b7b..63bf8b0c5f 100644
@@ -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 */
advice.h +1 -0
index 8def280688..66f6cd6a77 100644
@@ -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,
builtin/push.c +36 -1
index 6021b71d66..1b2ad3b8df 100644
@@ -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);
t/t5529-push-errors.sh +31 -0
index 80b06a0cd2..2294645902 100755
@@ -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 &&
Signature · 1 line
gitgitgadget