[PATCH v2 0/2] branch/push: suggest intended form when remote/branch slip given

["Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com>]

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 <haraldnordgren@gmail.com>
      
       ## 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 <haraldnordgren@gmail.com>
      
     @@ 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.

-- 
gitgitgadget