In d787d311db (checkout: split part of it to new command 'switch', 2019-03-29), the `git switch` command was created by extracting the common functionality of cmd_checkout() in checkout_main(). However, in b7b5fce270 (switch: better names for -b and -B, 2019-03-29), the branch creation and force creation options for 'switch' were changed to -c and -C, respectively. As a result of this, error messages and comments that previously referred to `-b` and `-B` became invalid for `git switch`. For error messages that refer to `-b` and `-B`, use a format string instead so that `-c` and `-C` can be printed when `git switch` is invoked. Reported-by: Robert Simpson Signed-off-by: Denton Liu <liu.denton@xxxxxxxxx> --- Notes: Robert, is the email listed above correct? If not, please let me know which email to use. (I hope that this reaches you somehow.) Range-diff against v1: 1: 0f7f9eefc0 ! 1: 75c9cf6ce9 switch: fix errors and comments related to -c and -C @@ Commit message In d787d311db (checkout: split part of it to new command 'switch', 2019-03-29), the `git switch` command was created by extracting the common functionality of cmd_checkout() in checkout_main(). However, in - b7b5fce270 (switch: better names for -b and -B, 2019-03-29), these - the branch creation and force creation options for 'switch' were changed - to -c and -C, respectively. As a result of this, error messages and - comments that previously referred to `-b` and `-B` became invalid for - `git switch`. + b7b5fce270 (switch: better names for -b and -B, 2019-03-29), the branch + creation and force creation options for 'switch' were changed to -c and + -C, respectively. As a result of this, error messages and comments that + previously referred to `-b` and `-B` became invalid for `git switch`. - For comments that refer to `-b` and `-B`, add `-c` and `-C` to the - comment. + For error messages that refer to `-b` and `-B`, use a format string + instead so that `-c` and `-C` can be printed when `git switch` is + invoked. - For error messages that refer to `-b`, introduce `enum cmd_variant` and - use it to differentiate between `checkout` and `switch` when printing - out error messages. - - An alternative implementation which was considered involved inserting - option name variants into a struct which is passed in by each command - variant. Even though this approach is more general and could be - applicable for future differing option names, it seemed like an - over-engineered solution when the current pair of options are the only - differing ones. We should probably avoid adding options which have - different names anyway. - - Reported-by: Robert Simpson <no-reply@xxxxxxxxxxxxxx> + Reported-by: Robert Simpson ## Notes ## @@ builtin/checkout.c: static struct option *add_checkout_path_options(struct check return newopts; } -+enum cmd_variant { -+ CMD_CHECKOUT, -+ CMD_SWITCH, -+ CMD_RESTORE -+}; ++/* 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[]) -+ const char * const usagestr[], -+ enum cmd_variant variant) - { - struct branch_info new_branch_info; - int parseopt_flags = 0; + const char * const usagestr[]) @@ builtin/checkout.c: static int checkout_main(int argc, const char **argv, const char *prefix, } if ((!!opts->new_branch + !!opts->new_branch_force + !!opts->new_orphan_branch) > 1) - die(_("-b, -B and --orphan are mutually exclusive")); -+ die(variant == CMD_CHECKOUT ? -+ _("-b, -B and --orphan are mutually exclusive") : -+ _("-c, -C and --orphan are mutually exclusive")); ++ die(_("-%c, -%c and --orphan are mutually exclusive"), ++ cb_option, toupper(cb_option)); if (opts->overlay_mode == 1 && opts->patch_mode) die(_("-p and --overlay are mutually exclusive")); @@ builtin/checkout.c: static int checkout_main(int argc, const char **argv, const opts->new_branch = opts->new_orphan_branch; - /* --track without -b/-B/--orphan should DWIM */ -+ /* --track without -b/-B/--orphan for checkout or -c/-C/--orphan for switch should DWIM */ ++ /* --track without -c/-C/-b/-B/--orphan should DWIM */ if (opts->track != BRANCH_TRACK_UNSPECIFIED && !opts->new_branch) { const char *argv0 = argv[0]; if (!argc || !strcmp(argv0, "--")) @@ builtin/checkout.c: static int checkout_main(int argc, const char **argv, const if (!argv0 || !argv0[1]) - die(_("missing branch name; try -b")); + die(_("missing branch name; try -%c"), -+ variant == CMD_CHECKOUT ? 'b' : 'c'); ++ cb_option); opts->new_branch = argv0 + 1; } -@@ builtin/checkout.c: int cmd_checkout(int argc, const char **argv, const char *prefix) - options = add_checkout_path_options(&opts, options); - - ret = checkout_main(argc, argv, prefix, &opts, -- options, checkout_usage); -+ options, checkout_usage, CMD_CHECKOUT); - FREE_AND_NULL(options); - return ret; - } @@ builtin/checkout.c: int cmd_switch(int argc, const char **argv, const char *prefix) + options = add_common_options(&opts, options); options = add_common_switch_branch_options(&opts, options); ++ cb_option = 'c'; ++ ret = checkout_main(argc, argv, prefix, &opts, -- options, switch_branch_usage); -+ options, switch_branch_usage, CMD_SWITCH); + options, switch_branch_usage); FREE_AND_NULL(options); - return ret; - } -@@ builtin/checkout.c: int cmd_restore(int argc, const char **argv, const char *prefix) - options = add_checkout_path_options(&opts, options); - - ret = checkout_main(argc, argv, prefix, &opts, -- options, restore_usage); -+ options, restore_usage, CMD_RESTORE); - FREE_AND_NULL(options); - return ret; - } builtin/checkout.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 8bc94d392b..a45519a2b4 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1544,6 +1544,9 @@ 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[]) @@ -1586,7 +1589,8 @@ static int checkout_main(int argc, const char **argv, const char *prefix, } if ((!!opts->new_branch + !!opts->new_branch_force + !!opts->new_orphan_branch) > 1) - die(_("-b, -B and --orphan are mutually exclusive")); + die(_("-%c, -%c and --orphan are mutually exclusive"), + cb_option, toupper(cb_option)); if (opts->overlay_mode == 1 && opts->patch_mode) die(_("-p and --overlay are mutually exclusive")); @@ -1614,7 +1618,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix, /* * From here on, new_branch will contain the branch to be checked out, * and new_branch_force and new_orphan_branch will tell us which one of - * -b/-B/--orphan is being used. + * -b/-B/-c/-C/--orphan is being used. */ if (opts->new_branch_force) opts->new_branch = opts->new_branch_force; @@ -1622,7 +1626,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix, if (opts->new_orphan_branch) opts->new_branch = opts->new_orphan_branch; - /* --track without -b/-B/--orphan should DWIM */ + /* --track without -c/-C/-b/-B/--orphan should DWIM */ if (opts->track != BRANCH_TRACK_UNSPECIFIED && !opts->new_branch) { const char *argv0 = argv[0]; if (!argc || !strcmp(argv0, "--")) @@ -1631,7 +1635,8 @@ static int checkout_main(int argc, const char **argv, const char *prefix, skip_prefix(argv0, "remotes/", &argv0); argv0 = strchr(argv0, '/'); if (!argv0 || !argv0[1]) - die(_("missing branch name; try -b")); + die(_("missing branch name; try -%c"), + cb_option); opts->new_branch = argv0 + 1; } @@ -1822,6 +1827,8 @@ int cmd_switch(int argc, const char **argv, const char *prefix) options = add_common_options(&opts, options); options = add_common_switch_branch_options(&opts, options); + cb_option = 'c'; + ret = checkout_main(argc, argv, prefix, &opts, options, switch_branch_usage); FREE_AND_NULL(options); -- 2.26.2.548.gbb00c8a0a9