From: Junio C Hamano Date: Wed, 24 Jun 2026 22:33:03 GMT Subject: Re: [PATCH v2 1/2] branch: suggest / on upstream slip Message-ID: In-Reply-To: <11bcecebf43797a889f08e79401370f43b2917a8.1782338114.git.gitgitgadget@gmail.com> "Harald Nordgren via GitGitGadget" writes: > From: Harald Nordgren > > 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. 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. > When the operated-on branch is missing and '/' names > a real remote-tracking ref, suggest the intended form: > > $ git branch --set-upstream-to=origin/main > > The suggestion is gated on '/' 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 '' is not a name suitable for an upstream, but '/' is? > diff --git a/builtin/branch.c b/builtin/branch.c > index 1572a4f9ef..cefc4519a7 100644 > --- a/builtin/branch.c > +++ b/builtin/branch.c > @@ -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 / 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? > + 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); > } 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.