git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 4/5] checkout: cleanup --conflict=<style> parsing

From
PGPhillip Wood via GitGitGadget <gitgitgadget@gmail.com>
Date
Mar 14, 2024, 17:05 UTC
Message-ID
<511e03d3db26b174fa6585310b9866a06ac2dff0.1710435907.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1684.v2.git.1710435907.gitgitgadget@gmail.com>
From: Phillip Wood <phillip.wood@dunelm.org.uk>

Passing an invalid conflict style name such as "--conflict=bad" gives the error message

    error: unknown style 'bad' given for 'merge.conflictstyle'

which is unfortunate as it talks about a config setting rather than the option given on the command line. This happens because the implementation calls git_xmerge_config() to set the conflict style using the value given on the command line. Use the newly added parse_conflict_style_name() instead and pass the value down the call chain to override the config setting. This also means we can avoid setting up a struct config_context required for calling git_xmerge_config().

The option is now parsed in a callback to avoid having to store the option name. This is a change in behavior as now

    git checkout --conflict=bad --conflict=diff3

will error out when parsing "--conflict=bad" whereas before this change it would succeed because it would only try to parse the value of the last "--conflict" option given on the command line.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 builtin/checkout.c | 51 +++++++++++++++++++++++++++++-----------------
 t/t7201-co.sh      |  6 ++++++
 2 files changed, 38 insertions(+), 19 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 6ded58bd95c..d6ab3b1d665 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -91,7 +91,7 @@ struct checkout_opts {
 	int new_branch_log;
 	enum branch_track track;
 	struct diff_options diff_options;
-	char *conflict_style;
+	int conflict_style;
 
 	int branch_exists;
 	const char *prefix;
@@ -100,6 +100,8 @@ struct checkout_opts {
 	struct tree *source_tree;
 };
 
+#define CHECKOUT_OPTS_INIT { .conflict_style = -1 }
+
 struct branch_info {
 	char *name; /* The short name used */
 	char *path; /* The full name of a real branch */
@@ -251,7 +253,8 @@ static int checkout_stage(int stage, const struct cache_entry *ce, int pos,
 }
 
 static int checkout_merged(int pos, const struct checkout *state,
-			   int *nr_checkouts, struct mem_pool *ce_mem_pool)
+			   int *nr_checkouts, struct mem_pool *ce_mem_pool,
+			   int conflict_style)
 {
 	struct cache_entry *ce = the_index.cache[pos];
 	const char *path = ce->name;
@@ -286,6 +289,7 @@ static int checkout_merged(int pos, const struct checkout *state,
 
 	git_config_get_bool("merge.renormalize", &renormalize);
 	ll_opts.renormalize = renormalize;
+	ll_opts.conflict_style = conflict_style;
 	merge_status = ll_merge(&result_buf, path, &ancestor, "base",
 				&ours, "ours", &theirs, "theirs",
 				state->istate, &ll_opts);
@@ -416,7 +420,8 @@ static int checkout_worktree(const struct checkout_opts *opts,
 			else if (opts->merge)
 				errs |= checkout_merged(pos, &state,
 							&nr_unmerged,
-							&ce_mem_pool);
+							&ce_mem_pool,
+							opts->conflict_style);
 			pos = skip_same_name(ce, pos) - 1;
 		}
 	}
@@ -886,6 +891,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
 			}
 			o.branch1 = new_branch_info->name;
 			o.branch2 = "local";
+			o.conflict_style = opts->conflict_style;
 			ret = merge_trees(&o,
 					  new_tree,
 					  work,
@@ -1618,6 +1624,21 @@ static int checkout_branch(struct checkout_opts *opts,
 	return switch_branches(opts, new_branch_info);
 }
 
+static int parse_opt_conflict(const struct option *o, const char *arg, int unset)
+{
+	struct checkout_opts *opts = o->value;
+
+	if (unset) {
+		opts->conflict_style = -1;
+		return 0;
+	}
+	opts->conflict_style = parse_conflict_style_name(arg);
+	if (opts->conflict_style < 0)
+		return error(_("unknown conflict style '%s'"), arg);
+
+	return 0;
+}
+
 static struct option *add_common_options(struct checkout_opts *opts,
 					 struct option *prevopts)
 {
@@ -1628,8 +1649,9 @@ static struct option *add_common_options(struct checkout_opts *opts,
 			    PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater),
 		OPT_BOOL(0, "progress", &opts->show_progress, N_("force progress reporting")),
 		OPT_BOOL('m', "merge", &opts->merge, N_("perform a 3-way merge with the new branch")),
-		OPT_STRING(0, "conflict", &opts->conflict_style, N_("style"),
-			   N_("conflict style (merge, diff3, or zdiff3)")),
+		OPT_CALLBACK(0, "conflict", opts, N_("style"),
+			     N_("conflict style (merge, diff3, or zdiff3)"),
+			     parse_opt_conflict),
 		OPT_END()
 	};
 	struct option *newopts = parse_options_concat(prevopts, options);
@@ -1720,15 +1742,9 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
 			opts->show_progress = isatty(2);
 	}
 
-	if (opts->conflict_style) {
-		struct key_value_info kvi = KVI_INIT;
-		struct config_context ctx = {
-			.kvi = &kvi,
-		};
+	if (opts->conflict_style >= 0)
 		opts->merge = 1; /* implied */
-		git_xmerge_config("merge.conflictstyle", opts->conflict_style,
-				  &ctx, NULL);
-	}
+
 	if (opts->force) {
 		opts->discard_changes = 1;
 		opts->ignore_unmerged_opt = "--force";
@@ -1893,7 +1909,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
 
 int cmd_checkout(int argc, const char **argv, const char *prefix)
 {
-	struct checkout_opts opts;
+	struct checkout_opts opts = CHECKOUT_OPTS_INIT;
 	struct option *options;
 	struct option checkout_options[] = {
 		OPT_STRING('b', NULL, &opts.new_branch, N_("branch"),
@@ -1909,7 +1925,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	int ret;
 	struct branch_info new_branch_info = { 0 };
 
-	memset(&opts, 0, sizeof(opts));
 	opts.dwim_new_local_branch = 1;
 	opts.switch_branch_doing_nothing_is_ok = 1;
 	opts.only_merge_on_switching_branches = 0;
@@ -1948,7 +1963,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 
 int cmd_switch(int argc, const char **argv, const char *prefix)
 {
-	struct checkout_opts opts;
+	struct checkout_opts opts = CHECKOUT_OPTS_INIT;
 	struct option *options = NULL;
 	struct option switch_options[] = {
 		OPT_STRING('c', "create", &opts.new_branch, N_("branch"),
@@ -1964,7 +1979,6 @@ int cmd_switch(int argc, const char **argv, const char *prefix)
 	int ret;
 	struct branch_info new_branch_info = { 0 };
 
-	memset(&opts, 0, sizeof(opts));
 	opts.dwim_new_local_branch = 1;
 	opts.accept_ref = 1;
 	opts.accept_pathspec = 0;
@@ -1990,7 +2004,7 @@ int cmd_switch(int argc, const char **argv, const char *prefix)
 
 int cmd_restore(int argc, const char **argv, const char *prefix)
 {
-	struct checkout_opts opts;
+	struct checkout_opts opts = CHECKOUT_OPTS_INIT;
 	struct option *options;
 	struct option restore_options[] = {
 		OPT_STRING('s', "source", &opts.from_treeish, "<tree-ish>",
@@ -2007,7 +2021,6 @@ int cmd_restore(int argc, const char **argv, const char *prefix)
 	int ret;
 	struct branch_info new_branch_info = { 0 };
 
-	memset(&opts, 0, sizeof(opts));
 	opts.accept_ref = 0;
 	opts.accept_pathspec = 1;
 	opts.empty_pathspec_ok = 0;
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 10cc6c46051..e1f85a91565 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -631,6 +631,12 @@ test_expect_success 'checkout --conflict=diff3' '
 	test_cmp merged file
 '
 
+test_expect_success 'checkout with invalid conflict style' '
+	test_must_fail git checkout --conflict=bad 2>actual -- file &&
+	echo "error: unknown conflict style ${SQ}bad${SQ}" >expect &&
+	test_cmp expect actual
+'
+
 test_expect_success 'failing checkout -b should not break working tree' '
 	git clean -fd &&  # Remove untracked files in the way
 	git reset --hard main &&

-- 
gitgitgadget
Previous: Junio C HamanoNext: Junio C Hamano
Message 25 of 29 in “checkout: cleanup --conflict=”
  1. 0/4 checkout: cleanup --conflict=Phillip Wood via GitGitGadget, Mar 8, 2024
  2. 1/4 xdiff-interface: refactor parsing of merge.conflictstylePhillip Wood via GitGitGadget, Mar 8, 2024
  3. 2/4 merge-ll: introduce LL_MERGE_OPTIONS_INITPhillip Wood via GitGitGadget, Mar 8, 2024
  4. 3/4 merge options: add a conflict style memberPhillip Wood via GitGitGadget, Mar 8, 2024
  5. Junio C HamanoMar 8, 2024
  6. phillip.wood123@gmail.comMar 8, 2024
  7. Junio C HamanoMar 8, 2024
  8. Elijah NewrenMar 9, 2024
  9. Junio C HamanoMar 9, 2024
  10. 4/4 checkout: cleanup --conflict=<style> parsingPhillip Wood via GitGitGadget, Mar 8, 2024
  11. Junio C HamanoMar 8, 2024
  12. phillip.wood123@gmail.comMar 8, 2024
  13. Junio C HamanoMar 8, 2024
  14. phillip.wood123@gmail.comMar 11, 2024
  15. Junio C HamanoMar 11, 2024
  16. Phillip WoodMar 12, 2024
  17. Junio C HamanoMar 8, 2024
  18. phillip.wood123@gmail.comMar 8, 2024
  19. 0/5 checkout: cleanup --conflict=Phillip Wood via GitGitGadget, Mar 14, 2024
  20. 1/5 xdiff-interface: refactor parsing of merge.conflictstylePhillip Wood via GitGitGadget, Mar 14, 2024
  21. Junio C HamanoMar 14, 2024
  22. 2/5 merge-ll: introduce LL_MERGE_OPTIONS_INITPhillip Wood via GitGitGadget, Mar 14, 2024
  23. 3/5 merge options: add a conflict style memberPhillip Wood via GitGitGadget, Mar 14, 2024
  24. Junio C HamanoMar 14, 2024
  25. 4/5 checkout: cleanup --conflict=<style> parsingPhillip Wood via GitGitGadget, Mar 14, 2024
  26. Junio C HamanoMar 14, 2024
  27. 5/5 checkout: fix interaction between --conflict and --mergePhillip Wood via GitGitGadget, Mar 14, 2024
  28. Junio C HamanoMar 14, 2024
  29. Phillip WoodMar 14, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.