git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Denton Liu <liu.denton@gmail.com>
To: Git Mailing List <git@vger.kernel.org>
Cc: Robert Simpson <no-reply@MailScreen.com>,
	Taylor Blau <me@ttaylorr.com>,
	Eric Sunshine <sunshine@sunshineco.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2] switch: fix errors and comments related to -c and -C
Date: Thu, 30 Apr 2020 07:54:57 -0400	[thread overview]
Message-ID: <75c9cf6ce95e753f50ce9b8b86b2c3ded5d8d38d.1588247662.git.liu.denton@gmail.com> (raw)
In-Reply-To: <0f7f9eefc056dd4d9b11dab737e00235b3243a2f.1588150804.git.liu.denton@gmail.com>

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


  parent reply	other threads:[~2020-04-30 11:55 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-28 23:12 Git error message suggests an invalid switch Robert Simpson
2020-04-29  9:00 ` [PATCH] switch: fix errors and comments related to -c and -C Denton Liu
2020-04-29 16:09   ` Taylor Blau
2020-04-29 16:31     ` Eric Sunshine
2020-04-29 16:35   ` Junio C Hamano
2020-04-30 11:54   ` Denton Liu [this message]
2020-04-30 16:21     ` [PATCH v2] " Taylor Blau
2020-04-30 20:45       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=75c9cf6ce95e753f50ce9b8b86b2c3ded5d8d38d.1588247662.git.liu.denton@gmail.com \
    --to=liu.denton@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    --cc=no-reply@MailScreen.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).