[PATCH v2] worktree add: improve message for ambiguous remote branch name

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

From: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>

Display a hint and a descriptive error message when DWIM fails.

Signed-off-by: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>
---
    worktree add: improve message for ambiguous remote branch name

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

Range-diff vs v1:

 1:  00b814fe09 ! 1:  1bc57ce497 worktree add: improve message for ambiguous remote branch name
     @@ Metadata
       ## Commit message ##
          worktree add: improve message for ambiguous remote branch name
      
     -    Display a descriptive message when DWIM fails.
     -
     -    Add advice on how to work around this by specifying the fully
     -    qualified name or by setting checkout.defaultRemote.
     +    Display a hint and a descriptive error message when DWIM fails.
      
          Signed-off-by: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>
      
       ## builtin/worktree.c ##
     -@@ builtin/worktree.c: static const char * const git_worktree_unlock_usage[] = {
     - 	NULL
     - };
     +@@
     + 	"\n" \
     + 	"    git worktree add --orphan %s\n")
       
     -+static const char message_advice_ambiguous_remote_tracking_branch[] =
     -+	N_("If you meant to create a worktree from a remote tracking branch on,\n"
     -+	   "e.g. 'origin', you can do so by fully qualifying the name:\n"
     -+	   "\n"
     -+	   "    git worktree add <path> 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.");
     ++#define WORKTREE_ADD_AMBIGUOUS_REMOTE_BRANCH_NAME_HINT_TEXT \
     ++	_("Matched multiple remote tracking branches, you can list them by:\n" \
     ++	"\n" \
     ++	"    git branch -r --list \"*/%s\"\n" \
     ++	"\n" \
     ++	"If you meant to create a worktree from a remote tracking branch on,\n" \
     ++	"e.g. 'origin', you can do so by:\n" \
     ++	"\n" \
     ++	"    git worktree add -b %s %s origin/%s\n" \
     ++	"\n" \
     ++	"If you'd like to always prefer some remote, e.g. 'origin',\n" \
     ++	"consider setting checkout.defaultRemote=origin in your config.")
      +
     - struct add_opts {
     - 	int force;
     - 	int detach;
     -@@ builtin/worktree.c: static int dwim_orphan(const struct add_opts *opts, int opt_track, int remote)
     - 	return 1;
     - }
     - 
     --static char *dwim_branch(const char *path, char **new_branch)
     -+static char *dwim_branch(const struct add_opts *opts, const char *path, char **new_branch)
     - {
     - 	int n;
     - 	int branch_exists;
     -@@ builtin/worktree.c: static char *dwim_branch(const char *path, char **new_branch)
     - 
     - 	*new_branch = branchname;
     - 	if (guess_remote) {
     -+		int num_matches = 0;
     - 		struct object_id oid;
     --		char *remote = unique_tracking_name(*new_branch, &oid, NULL);
     -+		char *remote = unique_tracking_name(*new_branch, &oid, &num_matches);
     -+		if (!opts->quiet && !remote && num_matches > 1) {
     -+			if (advice_enabled(ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME))
     -+				advise(_(message_advice_ambiguous_remote_tracking_branch));
     -+			warning(_("'%s' matched multiple (%d) remote tracking branches\n"), branchname, num_matches);
     -+		}
     - 		return remote;
     - 	}
     - 	return NULL;
     -@@ builtin/worktree.c: static int add(int ac, const char **av, const char *prefix,
     - 		opts.orphan = dwim_orphan(&opts, !!opt_track, 0);
     - 	} else if (ac < 2) {
     - 		/* DWIM: Guess branch name from path. */
     --		char *s = dwim_branch(path, &new_branch_to_free);
     -+		char *s = dwim_branch(&opts, path, &new_branch_to_free);
     - 		if (s)
     - 			branch = branch_to_free = s;
     - 		new_branch = new_branch_to_free;
     + static const char * const git_worktree_usage[] = {
     + 	BUILTIN_WORKTREE_ADD_USAGE,
     + 	BUILTIN_WORKTREE_LIST_USAGE,
      @@ builtin/worktree.c: static int add(int ac, const char **av, const char *prefix,
       
       		commit = lookup_commit_reference_by_name(branch);
     @@ builtin/worktree.c: static int add(int ac, const char **av, const char *prefix,
       				new_branch = branch;
       				branch = new_branch_to_free = remote;
      +			} else if (num_matches > 1) {
     -+				if (!opts.quiet && advice_enabled(ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME)) {
     -+					advise(_(message_advice_ambiguous_remote_tracking_branch));
     -+				}
     -+				die(_("'%s' matched multiple (%d) remote tracking branches"), branch, num_matches);
     ++				if (!opts.quiet)
     ++					advise_if_enabled(ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME,
     ++							  WORKTREE_ADD_AMBIGUOUS_REMOTE_BRANCH_NAME_HINT_TEXT,
     ++							  branch, branch, path, branch);
     ++				die(_("'%s' matched multiple (%d) remote tracking branches"),
     ++				    branch, num_matches);
       			}
       		}
       
     @@ t/t2400-worktree-add.sh: test_expect_success '"add" <path> <branch> dwims' '
       		git -c checkout.defaultRemote=repo_upstream worktree add ../foo foo &&
       		git status -uno --porcelain >status.actual &&
       		test_must_be_empty status.actual
     -@@ t/t2400-worktree-add.sh: test_expect_success 'git worktree add --guess-remote sets up tracking' '
     - 		test_cmp_rev refs/remotes/repo_a/foo refs/heads/foo
     - 	)
     - '
     -+test_expect_success 'git worktree add --guess-remote with ambiguous name' '
     -+	test_when_finished rm -rf repo_a repo_b foo &&
     -+	setup_remote_repo repo_a repo_b &&
     -+	(
     -+		cd repo_b &&
     -+		git remote add upstream2 ../repo_a &&
     -+		git fetch upstream2 &&
     -+		git worktree add --guess-remote ../foo 2>actual &&
     -+		test_grep "matched multiple (2) remote tracking branches" actual
     -+	) &&
     -+	(
     -+		cd foo &&
     -+		test_must_fail git config "branch.foo.remote" &&
     -+		test_must_fail git config "branch.foo.merge" &&
     -+		test_cmp_rev refs/heads/main refs/heads/foo
     -+	)
     -+'
     - test_expect_success 'git worktree add --guess-remote sets up tracking (quiet)' '
     - 	test_when_finished rm -rf repo_a repo_b foo &&
     - 	setup_remote_repo repo_a repo_b &&


 builtin/worktree.c      | 23 ++++++++++++++++++++++-
 t/t2400-worktree-add.sh |  4 ++--
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 654d27c3e1..b29c3a3755 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -64,6 +64,19 @@
 	"\n" \
 	"    git worktree add --orphan %s\n")
 
+#define WORKTREE_ADD_AMBIGUOUS_REMOTE_BRANCH_NAME_HINT_TEXT \
+	_("Matched multiple remote tracking branches, you can list them by:\n" \
+	"\n" \
+	"    git branch -r --list \"*/%s\"\n" \
+	"\n" \
+	"If you meant to create a worktree from a remote tracking branch on,\n" \
+	"e.g. 'origin', you can do so by:\n" \
+	"\n" \
+	"    git worktree add -b %s %s origin/%s\n" \
+	"\n" \
+	"If you'd like to always prefer some remote, e.g. 'origin',\n" \
+	"consider setting checkout.defaultRemote=origin in your config.")
+
 static const char * const git_worktree_usage[] = {
 	BUILTIN_WORKTREE_ADD_USAGE,
 	BUILTIN_WORKTREE_LIST_USAGE,
@@ -904,10 +917,18 @@ static int add(int ac, const char **av, const char *prefix,
 
 		commit = lookup_commit_reference_by_name(branch);
 		if (!commit) {
-			remote = unique_tracking_name(branch, &oid, NULL);
+			int num_matches = 0;
+			remote = unique_tracking_name(branch, &oid, &num_matches);
 			if (remote) {
 				new_branch = branch;
 				branch = new_branch_to_free = remote;
+			} else if (num_matches > 1) {
+				if (!opts.quiet)
+					advise_if_enabled(ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME,
+							  WORKTREE_ADD_AMBIGUOUS_REMOTE_BRANCH_NAME_HINT_TEXT,
+							  branch, branch, path, branch);
+				die(_("'%s' matched multiple (%d) remote tracking branches"),
+				    branch, num_matches);
 			}
 		}
 
diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index 87b926728a..5c105cf252 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -624,12 +624,12 @@ test_expect_success '"add" <path> <branch> dwims' '
 test_expect_success '"add" <path> <branch> dwims with checkout.defaultRemote' '
 	test_when_finished rm -rf repo_upstream repo_dwim foo &&
 	setup_remote_repo repo_upstream repo_dwim &&
-	git init repo_dwim &&
 	(
 		cd repo_dwim &&
 		git remote add repo_upstream2 ../repo_upstream &&
 		git fetch repo_upstream2 &&
-		test_must_fail git worktree add ../foo foo &&
+		test_must_fail git worktree add ../foo foo 2>error.actual &&
+		test_grep "matched multiple (2) remote tracking branches" error.actual &&
 		git -c checkout.defaultRemote=repo_upstream worktree add ../foo foo &&
 		git status -uno --porcelain >status.actual &&
 		test_must_be_empty status.actual

base-commit: 010afd3166ddc64c9863b1506f12cbcdda0d4ea1
-- 
gitgitgadget