{"thread":{"id":"56434","subject":"[PATCH] builtin/checkout: track the command use (checkout/switch) for advice","startedAt":"2021-09-03T06:11:31Z","lastAt":"2021-09-03T10:22:26Z","messageCount":2,"participants":["Carlo Marcelo Arenas Belón","Bagas Sanjaya"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"434625","messageId":"20210903061120.31897-1-carenas@gmail.com","threadId":"56434","inReplyTo":null,"subject":"[PATCH] builtin/checkout: track the command use (checkout/switch) for advice","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2021-09-03T06:11:20Z","receivedAt":"2021-09-03T06:11:31Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"While using `git switch` to create a local branch to track a remote\none, and more than one remote has it, a helpful message will be\nprinted with instructions on how to resolve the ambiguity, but the\ncommand used in the example will be confusingly `git checkout`.\n\nModify parse_remote_branch() so that a bit could be passed to it\nto identify the source and print the same command used in the\nadvice.\n\nAdd a test for this new behaviour and while at it, modify the\nold one to ensure backward compatibility and update it so no longer\nuses the deprecated test_i18ngrep calls.\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\nThe overloading of cb_option to track which command was invoked seems\nfragile, but it was convenient to make this the minimal change required\nfor a bug fix.\n\nI am hoping that part will be reverted (including moving it back to\nwhere it was and that seemed more obvious for its current use), once\na better facility to differenciate is build (if there isn't one that\nI obviously missed).\n\nI originally though about sending it as an RFC because of that, but\nsince it is working code and with tests, assumed it was better use\nof the scarse reviewer time this way; either way feedback welcomed.\n\n builtin/checkout.c       | 29 ++++++++++++++++++-----------\n t/t2024-checkout-dwim.sh | 20 ++++++++++++++++++--\n 2 files changed, 36 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex b5d477919a..607ed21c36 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -29,6 +29,11 @@\n #include \"entry.h\"\n #include \"parallel-checkout.h\"\n \n+enum {\n+\tAMBIGUOS = (1<<0),\n+\tSWITCH = (1<<1),\n+} checkout_resolution_flags;\n+\n static const char * const checkout_usage[] = {\n \tN_(\"git checkout [<options>] <branch>\"),\n \tN_(\"git checkout [<options>] [<branch>] -- <file>...\"),\n@@ -1170,12 +1175,12 @@ static void setup_new_branch_info_and_source_tree(\n \n static const char *parse_remote_branch(const char *arg,\n \t\t\t\t       struct object_id *rev,\n-\t\t\t\t       int could_be_checkout_paths)\n+\t\t\t\t       unsigned flags)\n {\n \tint num_matches = 0;\n \tconst char *remote = unique_tracking_name(arg, rev, &num_matches);\n \n-\tif (remote && could_be_checkout_paths) {\n+\tif (remote && (flags & AMBIGUOS)) {\n \t\tdie(_(\"'%s' could be both a local file and a tracking branch.\\n\"\n \t\t\t\"Please use -- (and optionally --no-guess) to disambiguate\"),\n \t\t    arg);\n@@ -1186,11 +1191,12 @@ static const char *parse_remote_branch(const char *arg,\n \t\t    advise(_(\"If you meant to check out a remote tracking branch on, e.g. 'origin',\\n\"\n \t\t\t     \"you can do so by fully qualifying the name with the --track option:\\n\"\n \t\t\t     \"\\n\"\n-\t\t\t     \"    git checkout --track origin/<name>\\n\"\n+\t\t\t     \"    git %s --track origin/<name>\\n\"\n \t\t\t     \"\\n\"\n \t\t\t     \"If you'd like to always have checkouts of an ambiguous <name> prefer\\n\"\n \t\t\t     \"one remote, e.g. the 'origin' remote, consider setting\\n\"\n-\t\t\t     \"checkout.defaultRemote=origin in your config.\"));\n+\t\t\t     \"checkout.defaultRemote=origin in your config.\"),\n+\t\t\t\tflags & SWITCH ? \"switch\" : \"checkout\");\n \t    }\n \n \t    die(_(\"'%s' matched multiple (%d) remote tracking branches\"),\n@@ -1200,6 +1206,9 @@ static const char *parse_remote_branch(const char *arg,\n \treturn remote;\n }\n \n+/* create-branch option (either b or c) */\n+static char cb_option = 'b';\n+\n static int parse_branchname_arg(int argc, const char **argv,\n \t\t\t\tint dwim_new_local_branch_ok,\n \t\t\t\tstruct branch_info *new_branch_info,\n@@ -1293,8 +1302,10 @@ static int parse_branchname_arg(int argc, const char **argv,\n \t\t */\n \t\tint recover_with_dwim = dwim_new_local_branch_ok;\n \n-\t\tint could_be_checkout_paths = !has_dash_dash &&\n-\t\t\tcheck_filename(opts->prefix, arg);\n+\t\tunsigned flags = (cb_option == 'c') ? SWITCH : 0;\n+\n+\t\tif (!has_dash_dash && check_filename(opts->prefix, arg))\n+\t\t\tflags |= AMBIGUOS;\n \n \t\tif (!has_dash_dash && !no_wildcard(arg))\n \t\t\trecover_with_dwim = 0;\n@@ -1309,8 +1320,7 @@ static int parse_branchname_arg(int argc, const char **argv,\n \t\t\trecover_with_dwim = 0;\n \n \t\tif (recover_with_dwim) {\n-\t\t\tconst char *remote = parse_remote_branch(arg, rev,\n-\t\t\t\t\t\t\t\t could_be_checkout_paths);\n+\t\t\tconst char *remote = parse_remote_branch(arg, rev, flags);\n \t\t\tif (remote) {\n \t\t\t\t*new_branch = arg;\n \t\t\t\targ = remote;\n@@ -1571,9 +1581,6 @@ static struct option *add_checkout_path_options(struct checkout_opts *opts,\n \treturn newopts;\n }\n \n-/* create-branch option (either b or c) */\n-static char cb_option = 'b';\n-\n static int checkout_main(int argc, const char **argv, const char *prefix,\n \t\t\t struct checkout_opts *opts, struct option *options,\n \t\t\t const char * const usagestr[])\ndiff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh\nindex 4a1c901456..71656d6545 100755\n--- a/t/t2024-checkout-dwim.sh\n+++ b/t/t2024-checkout-dwim.sh\n@@ -105,12 +105,28 @@ test_expect_success 'checkout of branch from multiple remotes fails with advice'\n \ttest_must_fail git checkout foo 2>stderr &&\n \ttest_branch main &&\n \tstatus_uno_is_clean &&\n-\ttest_i18ngrep \"^hint: \" stderr &&\n+\tgrep \"^hint: \" stderr &&\n+\tgrep \"git checkout\" stderr &&\n \ttest_must_fail git -c advice.checkoutAmbiguousRemoteBranchName=false \\\n \t\tcheckout foo 2>stderr &&\n \ttest_branch main &&\n \tstatus_uno_is_clean &&\n-\ttest_i18ngrep ! \"^hint: \" stderr\n+\t! grep \"^hint: \" stderr\n+'\n+\n+test_expect_success 'switch of branch from multiple remotes fails with advice' '\n+\tgit checkout -B main &&\n+\ttest_might_fail git branch -D foo &&\n+\ttest_must_fail git switch foo 2>stderr &&\n+\ttest_branch main &&\n+\tstatus_uno_is_clean &&\n+\tgrep \"^hint: \" stderr &&\n+\tgrep \"git switch\" stderr &&\n+\ttest_must_fail git -c advice.checkoutAmbiguousRemoteBranchName=false \\\n+\t\tswitch foo 2>stderr &&\n+\ttest_branch main &&\n+\tstatus_uno_is_clean &&\n+\t! grep \"^hint: \" stderr\n '\n \n test_expect_success PERL 'checkout -p with multiple remotes does not print advice' '\n-- \n2.33.0.481.g26d3bed244\n\n"},{"id":"434627","messageId":"ee6ce58e-8cd6-edc3-164c-651883f3332f@gmail.com","threadId":"56434","inReplyTo":"20210903061120.31897-1-carenas@gmail.com","subject":"Re: [PATCH] builtin/checkout: track the command use (checkout/switch) for advice","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-09-03T10:22:15Z","receivedAt":"2021-09-03T10:22:26Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 03/09/21 13.11, Carlo Marcelo Arenas Belón wrote:\n> +enum {\n> +\tAMBIGUOS = (1<<0),\n> +\tSWITCH = (1<<1),\n> +} checkout_resolution_flags;\n> +\n\ns/AMBIGUOS/AMBIGUOUS/g (did you mean ambiguous)?\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"}]}