checkout: cleanup --conflict=<style> parsing

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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/checkout.c b/builtin/checkout.c
index e9859a1980330535deb6c89824da43f1663f09b4..0e5d1af75bf84cb00aeba10f76ba3c3c31d82ab2 100644 (file)
--- 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,
@@ -1616,6 +1622,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)
 {
@@ -1626,8 +1647,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);
@@ -1718,15 +1740,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";
@@ -1891,7 +1907,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"),
@@ -1907,7 +1923,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;
@@ -1946,7 +1961,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"),
@@ -1962,7 +1977,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;
@@ -1988,7 +2002,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>",
@@ -2005,7 +2019,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 10cc6c46051e95e8e8d52fae23396574f319b802..e1f85a91565760f1395e13456a89bfec2765fea5 100755 (executable)
--- 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 &&