Git error message suggests an invalid switch

Git error message suggests an invalid switch

[Robert Simpson]

The command below shows an error message that says "try -b". However "-b"
is not a valid switch for the command, as indicated by "unknown swtich
'b'" when that has been added to the command.

--------------------------------------------------------------------
git switch "master" --track "origin/master"

fatal: missing branch name; try -b

git switch -b "master" --track "origin/master"

error: unknown switch `b'
usage: git switch [<options>] [<branch>]

    -c, --create <branch>
                          create and switch to a new branch
    -C, --force-create <branch>
                          create/reset and switch to a branch
    --guess               second guess 'git switch <no-such-branch>'
    --discard-changes     throw away local modifications
    -q, --quiet           suppress progress reporting
    --recurse-submodules[=<checkout>]
                          control recursive updating of submodules
    --progress            force progress reporting
    -m, --merge           perform a 3-way merge with the new branch
    --conflict <style>    conflict style (merge or diff3)
    -d, --detach          detach HEAD at named commit
    -t, --track           set upstream info for new branch
    -f, --force           force checkout (throw away local modifications)
    --orphan <new-branch>
                          new unparented branch
    --overwrite-ignore    update ignored files (default)
    --ignore-other-worktrees
                          do not check if another worktree is holding the
given ref

[PATCH] switch: fix errors and comments related to -c and -C

[Denton Liu]

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`.

For comments that refer to `-b` and `-B`, add `-c` and `-C` to the
comment.

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@MailScreen.com>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---

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.)

 builtin/checkout.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 8bc94d392b..0ca74cde08 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1544,9 +1544,16 @@ static struct option *add_checkout_path_options(struct checkout_opts *opts,
 	return newopts;
 }
 
+enum cmd_variant {
+	CMD_CHECKOUT,
+	CMD_SWITCH,
+	CMD_RESTORE
+};
+
 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;
@@ -1586,7 +1593,9 @@ 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"));
 
 	if (opts->overlay_mode == 1 && opts->patch_mode)
 		die(_("-p and --overlay are mutually exclusive"));
@@ -1614,7 +1623,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 +1631,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 -b/-B/--orphan for checkout or -c/-C/--orphan for switch should DWIM */
 	if (opts->track != BRANCH_TRACK_UNSPECIFIED && !opts->new_branch) {
 		const char *argv0 = argv[0];
 		if (!argc || !strcmp(argv0, "--"))
@@ -1631,7 +1640,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"),
+					variant == CMD_CHECKOUT ? 'b' : 'c');
 		opts->new_branch = argv0 + 1;
 	}
 
@@ -1785,7 +1795,7 @@ 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;
 }
@@ -1823,7 +1833,7 @@ int cmd_switch(int argc, const char **argv, const char *prefix)
 	options = add_common_switch_branch_options(&opts, options);
 
 	ret = checkout_main(argc, argv, prefix, &opts,
-			    options, switch_branch_usage);
+			    options, switch_branch_usage, CMD_SWITCH);
 	FREE_AND_NULL(options);
 	return ret;
 }
@@ -1860,7 +1870,7 @@ 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;
 }
-- 
2.26.2.548.gbb00c8a0a9

Re: [PATCH] switch: fix errors and comments related to -c and -C

[Taylor Blau]

Hi Denton,

On Wed, Apr 29, 2020 at 05:00:19AM -0400, Denton Liu wrote:
> 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`.
>
> For comments that refer to `-b` and `-B`, add `-c` and `-C` to the
> comment.

I had to read this sentence a couple of times more than I would have
liked to in order to grok it fully. Would it be perhaps clearer as:

  Update comments in 'cmd_checkout()' that mention `-b` or `-B` to
  instead refer to `-c` or `-C` when invoked from 'git switch'.

?

> 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.

Yeah, I don't think we should spend much time trying to figure out a
general solution here when these are the only differing pair.

> Reported-by: Robert Simpson <no-reply@MailScreen.com>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>
> 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.)

I'll be shocked if this is his real email address ;).

>  builtin/checkout.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 8bc94d392b..0ca74cde08 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -1544,9 +1544,16 @@ static struct option *add_checkout_path_options(struct checkout_opts *opts,
>  	return newopts;
>  }
>
> +enum cmd_variant {
> +	CMD_CHECKOUT,
> +	CMD_SWITCH,
> +	CMD_RESTORE
> +};
> +
>  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;
> @@ -1586,7 +1593,9 @@ 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"));

Hmm. Do we need to generate an extra string for translation here? If the
string was instead:

  _("%s and --orphan are mutually exclusive")

where the first format string is filled in something like:

  die(_("%s and --orphan are mutually exclusive"),
      variant == CMD_CHECKOUT ? "-b, -B" : "-c, -C");

may save translators some work.

>  	if (opts->overlay_mode == 1 && opts->patch_mode)
>  		die(_("-p and --overlay are mutually exclusive"));
> @@ -1614,7 +1623,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 +1631,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 -b/-B/--orphan for checkout or -c/-C/--orphan for switch should DWIM */

This line is getting a little long. Would you mind wrapping this as a
multi-line comment instead?

>  	if (opts->track != BRANCH_TRACK_UNSPECIFIED && !opts->new_branch) {
>  		const char *argv0 = argv[0];
>  		if (!argc || !strcmp(argv0, "--"))
> @@ -1631,7 +1640,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"),
> +					variant == CMD_CHECKOUT ? 'b' : 'c');
>  		opts->new_branch = argv0 + 1;
>  	}
>
> @@ -1785,7 +1795,7 @@ 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;
>  }
> @@ -1823,7 +1833,7 @@ int cmd_switch(int argc, const char **argv, const char *prefix)
>  	options = add_common_switch_branch_options(&opts, options);
>
>  	ret = checkout_main(argc, argv, prefix, &opts,
> -			    options, switch_branch_usage);
> +			    options, switch_branch_usage, CMD_SWITCH);
>  	FREE_AND_NULL(options);
>  	return ret;
>  }
> @@ -1860,7 +1870,7 @@ 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;
>  }
> --
> 2.26.2.548.gbb00c8a0a9

All of the rest makes sense, thanks.

Thanks,
Taylor

Re: [PATCH] switch: fix errors and comments related to -c and -C

[Eric Sunshine]

On Wed, Apr 29, 2020 at 12:10 PM Taylor Blau <me@ttaylorr.com> wrote:
> On Wed, Apr 29, 2020 at 05:00:19AM -0400, Denton Liu wrote:
> > 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

s/these the/the/

> > 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.
>
> I had to read this sentence a couple of times more than I would have
> liked to in order to grok it fully. Would it be perhaps clearer as:
>
>   Update comments in 'cmd_checkout()' that mention `-b` or `-B` to
>   instead refer to `-c` or `-C` when invoked from 'git switch'.

I had no problem groking Denton's wording but had to re-read this
proposal several times before understanding it. I could try providing
yet another proposal, however, I think the entire sentence can simply
be dropped (after all, it's just stating the obvious).

> > +             die(variant == CMD_CHECKOUT ?
> > +                             _("-b, -B and --orphan are mutually exclusive") :
> > +                             _("-c, -C and --orphan are mutually exclusive"));
>
> Hmm. Do we need to generate an extra string for translation here? If the
> string was instead:
>
>   _("%s and --orphan are mutually exclusive")
>
> where the first format string is filled in something like:
>
>   die(_("%s and --orphan are mutually exclusive"),
>       variant == CMD_CHECKOUT ? "-b, -B" : "-c, -C");
>
> may save translators some work.

We don't know the grammatical or syntactic rules of each language, so
hard-coding untranslatable "-b, -B" is contraindicated. Since the
option letters ought not be translated (just as "--orphan" shouldn't
be), the letters themselves could be interpolated into the string.
However, that's probably less helpful for translators since it
eliminates contextual clues. Therefore, it seems like a good idea to
leave it as two separate translatable strings (as the patch already
does).

Re: [PATCH] switch: fix errors and comments related to -c and -C

[Junio C Hamano]

Denton Liu <liu.denton@gmail.com> writes:

> 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.

Sure.  Or another alternative is to take "-B/-b" silently without
advertising.  It's not like the reason why we introduced "-C/-c" was
because we wanted to reuse them in "switch" for other purposes.

> Reported-by: Robert Simpson <no-reply@MailScreen.com>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>
> 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.)

If we do not get any response, it is OK to remove the fake e-mail
address and recording only the namee (this is only OK for trailers
that are not SoB; the Signed-off-by: trailers want to be more
strict).

> +enum cmd_variant {
> +	CMD_CHECKOUT,
> +	CMD_SWITCH,
> +	CMD_RESTORE
> +};

Yuck, but OK.  Does "git restore" even take -b/-c/--orphan option?
I somehow doubt it.

This is too invasive for what it achieves.

How about having a file-scope global

/* create-branch option (either b or c) */
static char cb_option = 'b';

and then ...

> +
>  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;
> @@ -1586,7 +1593,9 @@ 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"));

... use it here like

		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 +1623,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 +1631,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 -b/-B/--orphan for checkout or -c/-C/--orphan for switch should DWIM */

Way overlong comment.  Just 

	/* --track without -b/-B/-c/-C/--orphan should DWIM */

is sufficient, no?

>  	if (opts->track != BRANCH_TRACK_UNSPECIFIED && !opts->new_branch) {
>  		const char *argv0 = argv[0];
>  		if (!argc || !strcmp(argv0, "--"))
> @@ -1631,7 +1640,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"),
> +					variant == CMD_CHECKOUT ? 'b' : 'c');

Likewise,

			die(_("missing branch name; try -%c"), cb_option);

>  		opts->new_branch = argv0 + 1;
>  	}

And override cb_option in one of these helpers, perhaps like ...

> @@ -1785,7 +1795,7 @@ 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;
>  }
> @@ -1823,7 +1833,7 @@ int cmd_switch(int argc, const char **argv, const char *prefix)
>  	options = add_common_switch_branch_options(&opts, options);
>  

... here (the other one uses 'b')

	cb_option = 'c';

>  	ret = checkout_main(argc, argv, prefix, &opts,
> -			    options, switch_branch_usage);
> +			    options, switch_branch_usage, CMD_SWITCH);
>  	FREE_AND_NULL(options);
>  	return ret;
>  }

... and you do not have to change function signature of
checkout_main() at all.

> @@ -1860,7 +1870,7 @@ 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;
>  }

[PATCH v2] switch: fix errors and comments related to -c and -C

[Denton Liu]

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@gmail.com>
---

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@MailScreen.com>
    +    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

Re: [PATCH v2] switch: fix errors and comments related to -c and -C

[Taylor Blau]

On Thu, Apr 30, 2020 at 07:54:57AM -0400, Denton Liu wrote:
> 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@gmail.com>
> ---
>
> 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.)

Nicely done. This revision looks great to me, thanks.

  Reviewed-by: Taylor Blau <me@ttaylorr.com>

Thanks,
Taylor

Re: [PATCH v2] switch: fix errors and comments related to -c and -C

[Junio C Hamano]

Taylor Blau <me@ttaylorr.com> writes:

> Nicely done. This revision looks great to me, thanks.
>
>   Reviewed-by: Taylor Blau <me@ttaylorr.com>

I'd squeeze this into a single line, though.

> -			die(_("missing branch name; try -b"));
> +			die(_("missing branch name; try -%c"),
> +					cb_option);

Will queue with a local fix-up for the above.

Thanks.