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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

 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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux