[PATCH v6 0/3] worktree add: improve message for ambiguous remote branch name

["Yoichi NAKAYAMA via GitGitGadget" <gitgitgadget@gmail.com>]

'git worktree add ../foo-dir bar-topic' fails to dwim when there are
multiple remote branches with name `bar-topic'. But it doesn't display
meaningful message as 'git checkout bar-topic' does under the same
situation.

We improve this by adding advice and modify the error message for worktree
add.

By Junio's suggestion, we include matched remote names in the advice. It is
applied to checkout, too.

The changes to 'checkout' are almost identical to what Junio proposed; I
have made minor adjustments to use the specified branch name. I'm not sure
how to handle the "Author" field in this case, so I've set it to myself for
now, but I'll correct it if that's not appropriate.

Yoichi NAKAYAMA (3):
  checkout: extract function to display advice for ambiguous remotes
  checkout: improve message for ambiguous remote branch name
  worktree add: improve message for ambiguous remote branch name

 builtin/checkout.c      | 76 +++++++++++++++++++++++++----------------
 builtin/worktree.c      | 37 ++++++++++++++++++--
 checkout.c              | 14 ++++++--
 checkout.h              |  5 ++-
 t/t2400-worktree-add.sh |  4 +--
 5 files changed, 98 insertions(+), 38 deletions(-)


base-commit: dea0ea3582e6980ddbc1173cc8e3e9f9db91cde0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2197%2Fyoichi%2Fimprove-worktree-add-error-message-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2197/yoichi/improve-worktree-add-error-message-v6
Pull-Request: https://github.com/gitgitgadget/git/pull/2197

Range-diff vs v5:

 -:  ---------- > 1:  e3f7d88520 checkout: extract function to display advice for ambiguous remotes
 1:  b838fdabb7 ! 2:  97e99ae31e checkout: improve message for ambiguous remote branch name
     @@ Commit message
          To make the advice more feasible, we will provide matched remote names
          for the specified branch name.
      
     -    To achive that, we add an optional feature to the
     +    To achieve that, we add an optional feature to the
          `unique_tracking_name()` function that allows the matched remote name
          to be exposed to the caller.
      
     @@ builtin/checkout.c: enum checkout_command {
       	CHECKOUT_RESTORE = 3,
       };
       
     -+static void be_explicit(const char *branch,
     -+			enum checkout_command which_command,
     -+			const struct string_list *matched_remote_names)
     -+{
     -+	const char *cmdname;
     +-static void advice_disambiguating_remotes(enum checkout_command which_command)
     ++static void advise_disambiguating_remotes(enum checkout_command which_command,
     ++					  const char *branch,
     ++					  const struct string_list *matched_remote_names)
     + {
     + 	const char *cmdname;
      +	struct string_list_item *item;
     -+
     -+	switch (which_command) {
     -+	case CHECKOUT_CHECKOUT:
     -+		cmdname = "checkout";
     -+		break;
     -+	case CHECKOUT_SWITCH:
     -+		cmdname = "switch";
     -+		break;
     -+	default:
     -+		BUG("command <%d> should not reach parse_remote_branch",
     -+		     which_command);
     -+		break;
     -+	}
     -+
     + 
     + 	switch (which_command) {
     + 	case CHECKOUT_CHECKOUT:
     +@@ builtin/checkout.c: static void advice_disambiguating_remotes(enum checkout_command which_command)
     + 		break;
     + 	}
     + 
     +-	advise(_("If you meant to check out a remote tracking branch on, e.g. 'origin',\n"
      +	advise(_("Branches with the same name appears in multiple remotes:"));
      +	for_each_string_list_item(item, matched_remote_names) {
      +		advise(_("  %s"), item->string);
      +	}
      +	advise(_("If you meant to check out a remote tracking branch on <remote>,\n"
     -+		 "you can do so by fully qualifying the name with the --track option:\n"
     -+		 "\n"
     + 		 "you can do so by fully qualifying the name with the --track option:\n"
     + 		 "\n"
     +-		 "    git %s --track origin/<name>\n"
      +		 "    git %s --track <remote>/%s\n"
     -+		 "\n"
     + 		 "\n"
     +-		 "If you'd like to always have checkouts of an ambiguous <name> prefer\n"
      +		 "If you'd like to always have checkouts of an ambiguous name prefer\n"
     -+		 "one remote, e.g. the 'origin' remote, consider setting\n"
     -+		 "checkout.defaultRemote=origin in your config."),
     + 		 "one remote, e.g. the 'origin' remote, consider setting\n"
     + 		 "checkout.defaultRemote=origin in your config."),
     +-	       cmdname);
      +	       cmdname, branch);
     -+}
     -+
     + }
     + 
       static char *parse_remote_branch(const char *arg,
     - 				 struct object_id *rev,
     - 				 int could_be_checkout_paths,
     +@@ builtin/checkout.c: static char *parse_remote_branch(const char *arg,
       				 enum checkout_command which_command)
       {
       	int num_matches = 0;
     @@ builtin/checkout.c: enum checkout_command {
       	if (remote && could_be_checkout_paths) {
       		die(_("'%s' could be both a local file and a tracking branch.\n"
      @@ builtin/checkout.c: static char *parse_remote_branch(const char *arg,
     - 	}
       
       	if (!remote && num_matches > 1) {
     --	    if (advice_enabled(ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME)) {
     --		    const char *cmdname;
     --
     --		    switch (which_command) {
     --		    case CHECKOUT_CHECKOUT:
     --			    cmdname = "checkout";
     --			    break;
     --		    case CHECKOUT_SWITCH:
     --			    cmdname = "switch";
     --			    break;
     --		    default:
     --			    BUG("command <%d> should not reach parse_remote_branch",
     --				which_command);
     --			    break;
     --		    }
     --
     --		    advise(_("If you meant to check out a remote tracking branch on, e.g. 'origin',\n"
     --			     "you can do so by fully qualifying the name with the --track option:\n"
     --			     "\n"
     --			     "    git %s --track origin/<name>\n"
     --			     "\n"
     --			     "If you'd like to always have checkouts of an ambiguous <name> prefer\n"
     --			     "one remote, e.g. the 'origin' remote, consider setting\n"
     --			     "checkout.defaultRemote=origin in your config."),
     --			   cmdname);
     --	    }
     + 		if (advice_enabled(ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME))
     +-			advice_disambiguating_remotes(which_command);
      -
     --	    die(_("'%s' matched multiple (%d) remote tracking branches"),
     --		arg, num_matches);
     -+		if (advice_enabled(ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME))
     -+			be_explicit(arg, which_command, &matched_remote_names);
     -+		die(_("'%s' matched multiple (%d) remote tracking branches"),
     -+		    arg, num_matches);
     ++			advise_disambiguating_remotes(which_command, arg,
     ++						      &matched_remote_names);
     + 		die(_("'%s' matched multiple (%d) remote tracking branches"),
     + 		    arg, num_matches);
       	}
       
      +	string_list_clear(&matched_remote_names, 0);
 2:  777862235e ! 3:  dcb84a69a6 worktree add: improve message for ambiguous remote branch name
     @@ builtin/worktree.c: static char *dwim_branch(const char *path, char **new_branch
       	return NULL;
       }
       
     -+static void advise_ambiguous_remote(const char *path, const char *branch,
     -+				    const struct string_list *matched_remote_names)
     ++static void advise_disambiguating_remotes(const char *path, const char *branch,
     ++					  const struct string_list *matched_remote_names)
      +{
      +	struct string_list_item *item;
      +
     @@ builtin/worktree.c: static int add(int ac, const char **av, const char *prefix,
      +			} else if (num_matches > 1) {
      +				if (!opts.quiet &&
      +				    advice_enabled(ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME))
     -+					advise_ambiguous_remote(path, branch, &matched_remote_names);
     ++					advise_disambiguating_remotes(path, branch,
     ++								      &matched_remote_names);
      +				die(_("'%s' matched multiple (%d) remote tracking branches"),
      +				    branch, num_matches);
       			}

-- 
gitgitgadget