[PATCH/RFC] Restructure some of the checkout opts.

Subsystems: the rest

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH/RFC] Restructure some of the checkout opts.

From: Jared Hance <hidden>
Date: 2016-06-15 22:49:29

Previously, most opts in `builtin.checkout.c' had been an instance of
`struct checkout_opts'. It appears that some of the opts that were
perhaps added later were not in the struct. Move them into the struct
in order to maintain consistency.

Signed-off-by: Jared Hance <redacted>
---

 builtin/checkout.c |   27 ++++++++++++++-------------
 1 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 7250e5c..3a35cde 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -31,6 +31,9 @@ struct checkout_opts {
 	int force;
 	int writeout_stage;
 	int writeout_error;
+	int patch_mode;
+	int dwim_new_local_branch;
+	char *conflict_style;
 
 	/* not set by parse_options */
 	int branch_exists;
@@ -660,9 +663,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	const char *arg;
 	struct branch_info new;
 	struct tree *source_tree = NULL;
-	char *conflict_style = NULL;
-	int patch_mode = 0;
-	int dwim_new_local_branch = 1;
 	struct option options[] = {
 		OPT__QUIET(&opts.quiet),
 		OPT_STRING('b', NULL, &opts.new_branch, "branch",
@@ -679,10 +679,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 			    3),
 		OPT_BOOLEAN('f', "force", &opts.force, "force"),
 		OPT_BOOLEAN('m', "merge", &opts.merge, "merge"),
-		OPT_STRING(0, "conflict", &conflict_style, "style",
+		OPT_STRING(0, "conflict", &opts.conflict_style, "style",
 			   "conflict style (merge or diff3)"),
-		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
-		{ OPTION_BOOLEAN, 0, "guess", &dwim_new_local_branch, NULL,
+		OPT_BOOLEAN('p', "patch", &opts.patch_mode, "select hunks interactively"),
+		{ OPTION_BOOLEAN, 0, "guess", &opts.dwim_new_local_branch, NULL,
 		  "second guess 'git checkout no-such-branch'",
 		  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
 		OPT_END(),
@@ -695,6 +695,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	git_config(git_checkout_config, NULL);
 
 	opts.track = BRANCH_TRACK_UNSPECIFIED;
+	opts.dwim_new_local_branch = 1;
 
 	argc = parse_options(argc, argv, prefix, options, checkout_usage,
 			     PARSE_OPT_KEEP_DASHDASH);
@@ -707,7 +708,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	if (opts.new_branch_force)
 		opts.new_branch = opts.new_branch_force;
 
-	if (patch_mode && (opts.track > 0 || opts.new_branch
+	if (opts.patch_mode && (opts.track > 0 || opts.new_branch
 			   || opts.new_branch_log || opts.merge || opts.force))
 		die ("--patch is incompatible with all other options");
 
@@ -734,9 +735,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		opts.new_branch = opts.new_orphan_branch;
 	}
 
-	if (conflict_style) {
+	if (opts.conflict_style) {
 		opts.merge = 1; /* implied */
-		git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
+		git_xmerge_config("merge.conflictstyle", opts.conflict_style, NULL);
 	}
 
 	if (opts.force && opts.merge)
@@ -787,8 +788,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		if (get_sha1_mb(arg, rev)) {
 			if (has_dash_dash)          /* case (1) */
 				die("invalid reference: %s", arg);
-			if (!patch_mode &&
-			    dwim_new_local_branch &&
+			if (!opts.patch_mode &&
+			    opts.dwim_new_local_branch &&
 			    opts.track == BRANCH_TRACK_UNSPECIFIED &&
 			    !opts.new_branch &&
 			    !check_filename(NULL, arg) &&
@@ -851,7 +852,7 @@ no_reference:
 		if (!pathspec)
 			die("invalid path specification");
 
-		if (patch_mode)
+		if (opts.patch_mode)
 			return interactive_checkout(new.name, pathspec, &opts);
 
 		/* Checkout paths */
@@ -869,7 +870,7 @@ no_reference:
 		return checkout_paths(source_tree, pathspec, &opts);
 	}
 
-	if (patch_mode)
+	if (opts.patch_mode)
 		return interactive_checkout(new.name, NULL, &opts);
 
 	if (opts.new_branch) {
-- 
1.7.2.2

Re: [PATCH/RFC] Restructure some of the checkout opts.

From: Jens Lehmann <hidden>
Date: 2016-06-15 22:49:29

Am 05.09.2010 14:09, schrieb Jared Hance:
Previously, most opts in `builtin.checkout.c' had been an instance of
`struct checkout_opts'. It appears that some of the opts that were
perhaps added later were not in the struct. Move them into the struct
in order to maintain consistency.
Hm, they aren't used outside of cmd_checkout() (as the other members
are), so maybe it is ok that they aren't in the struct?

Re: [PATCH/RFC] Restructure some of the checkout opts.

From: Jared Hance <hidden>
Date: 2016-06-15 22:49:29

On Sun, Sep 05, 2010 at 02:24:09PM +0200, Jens Lehmann wrote:
Am 05.09.2010 14:09, schrieb Jared Hance:
quoted
Previously, most opts in `builtin.checkout.c' had been an instance of
`struct checkout_opts'. It appears that some of the opts that were
perhaps added later were not in the struct. Move them into the struct
in order to maintain consistency.
Hm, they aren't used outside of cmd_checkout() (as the other members
are), so maybe it is ok that they aren't in the struct?
I guessed that this was the reason that they were overlooked, but it
seemed a little odd that every other opt was stored in the struct and
it just seemed to be more consistent to have _everything_ in the
struct.

It also makes it more clear that they opts, whereas before, you might
think they weren't at a first glance because they weren't in the
struct like the others.

I'll leave this to you guys to decide what to do with it.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help