[PATCH] builtin/checkout: track the command use (checkout/switch) for advice

[PATCH] builtin/checkout: track the command use (checkout/switch) for advice

[Carlo Marcelo Arenas Belón]

While using `git switch` to create a local branch to track a remote
one, and more than one remote has it, a helpful message will be
printed with instructions on how to resolve the ambiguity, but the
command used in the example will be confusingly `git checkout`.

Modify parse_remote_branch() so that a bit could be passed to it
to identify the source and print the same command used in the
advice.

Add a test for this new behaviour and while at it, modify the
old one to ensure backward compatibility and update it so no longer
uses the deprecated test_i18ngrep calls.

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
The overloading of cb_option to track which command was invoked seems
fragile, but it was convenient to make this the minimal change required
for a bug fix.

I am hoping that part will be reverted (including moving it back to
where it was and that seemed more obvious for its current use), once
a better facility to differenciate is build (if there isn't one that
I obviously missed).

I originally though about sending it as an RFC because of that, but
since it is working code and with tests, assumed it was better use
of the scarse reviewer time this way; either way feedback welcomed.

 builtin/checkout.c       | 29 ++++++++++++++++++-----------
 t/t2024-checkout-dwim.sh | 20 ++++++++++++++++++--
 2 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index b5d477919a..607ed21c36 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -29,6 +29,11 @@
 #include "entry.h"
 #include "parallel-checkout.h"
 
+enum {
+	AMBIGUOS = (1<<0),
+	SWITCH = (1<<1),
+} checkout_resolution_flags;
+
 static const char * const checkout_usage[] = {
 	N_("git checkout [<options>] <branch>"),
 	N_("git checkout [<options>] [<branch>] -- <file>..."),
@@ -1170,12 +1175,12 @@ static void setup_new_branch_info_and_source_tree(
 
 static const char *parse_remote_branch(const char *arg,
 				       struct object_id *rev,
-				       int could_be_checkout_paths)
+				       unsigned flags)
 {
 	int num_matches = 0;
 	const char *remote = unique_tracking_name(arg, rev, &num_matches);
 
-	if (remote && could_be_checkout_paths) {
+	if (remote && (flags & AMBIGUOS)) {
 		die(_("'%s' could be both a local file and a tracking branch.\n"
 			"Please use -- (and optionally --no-guess) to disambiguate"),
 		    arg);
@@ -1186,11 +1191,12 @@ static const char *parse_remote_branch(const char *arg,
 		    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 checkout --track origin/<name>\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."));
+			     "checkout.defaultRemote=origin in your config."),
+				flags & SWITCH ? "switch" : "checkout");
 	    }
 
 	    die(_("'%s' matched multiple (%d) remote tracking branches"),
@@ -1200,6 +1206,9 @@ static const char *parse_remote_branch(const char *arg,
 	return remote;
 }
 
+/* create-branch option (either b or c) */
+static char cb_option = 'b';
+
 static int parse_branchname_arg(int argc, const char **argv,
 				int dwim_new_local_branch_ok,
 				struct branch_info *new_branch_info,
@@ -1293,8 +1302,10 @@ static int parse_branchname_arg(int argc, const char **argv,
 		 */
 		int recover_with_dwim = dwim_new_local_branch_ok;
 
-		int could_be_checkout_paths = !has_dash_dash &&
-			check_filename(opts->prefix, arg);
+		unsigned flags = (cb_option == 'c') ? SWITCH : 0;
+
+		if (!has_dash_dash && check_filename(opts->prefix, arg))
+			flags |= AMBIGUOS;
 
 		if (!has_dash_dash && !no_wildcard(arg))
 			recover_with_dwim = 0;
@@ -1309,8 +1320,7 @@ static int parse_branchname_arg(int argc, const char **argv,
 			recover_with_dwim = 0;
 
 		if (recover_with_dwim) {
-			const char *remote = parse_remote_branch(arg, rev,
-								 could_be_checkout_paths);
+			const char *remote = parse_remote_branch(arg, rev, flags);
 			if (remote) {
 				*new_branch = arg;
 				arg = remote;
@@ -1571,9 +1581,6 @@ static struct option *add_checkout_path_options(struct checkout_opts *opts,
 	return newopts;
 }
 
-/* create-branch option (either b or c) */
-static char cb_option = 'b';
-
 static int checkout_main(int argc, const char **argv, const char *prefix,
 			 struct checkout_opts *opts, struct option *options,
 			 const char * const usagestr[])
diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
index 4a1c901456..71656d6545 100755
--- a/t/t2024-checkout-dwim.sh
+++ b/t/t2024-checkout-dwim.sh
@@ -105,12 +105,28 @@ test_expect_success 'checkout of branch from multiple remotes fails with advice'
 	test_must_fail git checkout foo 2>stderr &&
 	test_branch main &&
 	status_uno_is_clean &&
-	test_i18ngrep "^hint: " stderr &&
+	grep "^hint: " stderr &&
+	grep "git checkout" stderr &&
 	test_must_fail git -c advice.checkoutAmbiguousRemoteBranchName=false \
 		checkout foo 2>stderr &&
 	test_branch main &&
 	status_uno_is_clean &&
-	test_i18ngrep ! "^hint: " stderr
+	! grep "^hint: " stderr
+'
+
+test_expect_success 'switch of branch from multiple remotes fails with advice' '
+	git checkout -B main &&
+	test_might_fail git branch -D foo &&
+	test_must_fail git switch foo 2>stderr &&
+	test_branch main &&
+	status_uno_is_clean &&
+	grep "^hint: " stderr &&
+	grep "git switch" stderr &&
+	test_must_fail git -c advice.checkoutAmbiguousRemoteBranchName=false \
+		switch foo 2>stderr &&
+	test_branch main &&
+	status_uno_is_clean &&
+	! grep "^hint: " stderr
 '
 
 test_expect_success PERL 'checkout -p with multiple remotes does not print advice' '
-- 
2.33.0.481.g26d3bed244

Re: [PATCH] builtin/checkout: track the command use (checkout/switch) for advice

[Bagas Sanjaya]

On 03/09/21 13.11, Carlo Marcelo Arenas Belón wrote:
> +enum {
> +	AMBIGUOS = (1<<0),
> +	SWITCH = (1<<1),
> +} checkout_resolution_flags;
> +

s/AMBIGUOS/AMBIGUOUS/g (did you mean ambiguous)?

-- 
An old man doll... just what I always wanted! - Clara