branch/push: suggest intended form when remote/branch slip given
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
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
@@ -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
@@ -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
PATCH 2/2 push: suggest <remote> <branch> for a slash slip
In reply to Harald Nordgren via GitGitGadgetFrom: 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
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
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.
Re: [PATCH 1/2] branch: suggest <remote>/<branch> on upstream slip
In reply to Harald Nordgren via GitGitGadgetQuoted 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.
Re: [PATCH 2/2] push: suggest <remote> <branch> for a slash slip
In reply to Harald Nordgren via GitGitGadgetQuoted 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
-0if (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.
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.
Re: [PATCH 1/2] branch: suggest <remote>/<branch> on upstream slip
In reply to Junio C HamanoQuoted 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" mainwould fail before even looking at the "$anything" (which is supposed to specify the
new_upstreamfor 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?
By slip it meant mistake, so you can call it 'hn/branch-push-mistake-advise'
Harald
Re: [PATCH 1/2] branch: suggest <remote>/<branch> on upstream slip
In reply to Junio C HamanoQuoted 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" mainwould fail before even looking at the "$anything" (which is supposed to specify the
new_upstreamfor 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.)
v2 cover branch/push: suggest intended form when remote/branch slip given
In reply to Harald Nordgren via GitGitGadgetWhen 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
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
@@ -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
@@ -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
PATCH v2 2/2 push: suggest <remote> <branch> for a slash slip
In reply to Harald Nordgren via GitGitGadgetFrom: 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
Re: [PATCH v2 1/2] branch: suggest <remote>/<branch> on upstream slip
In reply to Harald Nordgren via GitGitGadgetQuoted 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/mainit 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/mainThe 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@@ -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.
Re: [PATCH v2 2/2] push: suggest <remote> <branch> for a slash slip
In reply to Harald Nordgren via GitGitGadgetQuoted 20 lines · Harald Nordgren via GitGitGadget
t/t5529-push-errors.sh +12
-0@@ -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)
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@@ -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 (usetest_grep) t5529-push-errors.sh:62: error: bare grep outside pipeline (usetest_grep) t5529-push-errors.sh:67: error: bare grep outside pipeline (usetest_grep) t5529-push-errors.sh:72: error: bare grep outside pipeline (usetest_grep) t5529-push-errors.sh:77: error: bare grep outside pipeline (usetest_grep) t5529-push-errors.sh:84: error: bare grep outside pipeline (usetest_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
@@ -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
Re: [PATCH v2 1/2] branch: suggest <remote>/<branch> on upstream slip
In reply to Junio C HamanoQuoted 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
Re: [PATCH v2 2/2] push: suggest <remote> <branch> for a slash slip
In reply to Junio C HamanoQuoted 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@@ -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 (usetest_grep) t5529-push-errors.sh:62: error: bare grep outside pipeline (usetest_grep) t5529-push-errors.sh:67: error: bare grep outside pipeline (usetest_grep) t5529-push-errors.sh:72: error: bare grep outside pipeline (usetest_grep) t5529-push-errors.sh:77: error: bare grep outside pipeline (usetest_grep) t5529-push-errors.sh:84: error: bare grep outside pipeline (usetest_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
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.
Re: [PATCH v2 1/2] branch: suggest <remote>/<branch> on upstream slip
In reply to Harald NordgrenQuoted 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.
v3 cover branch/push: suggest intended form when remote/branch slip given
In reply to Harald Nordgren via GitGitGadgetWhen 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
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
@@ -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
@@ -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
PATCH v3 2/2 push: suggest <remote> <branch> for a slash slip
In reply to Harald Nordgren via GitGitGadgetFrom: 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
Re: [PATCH v3 1/2] branch: suggest <remote>/<branch> on upstream slip
In reply to Harald Nordgren via GitGitGadgetQuoted 39 lines · Harald Nordgren via GitGitGadget
builtin/branch.c +24
-0@@ -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'?
Re: [PATCH v3 1/2] branch: suggest <remote>/<branch> on upstream slip
Let's do it!
Harald