@matheusbsbs / matheus-bagliano / commits / dbeaf8e8c0

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>

Phillip Wood committed Mar 14, 2024 at 17:05 UTC dbeaf8e8c0ada5ba6a8c379e2ad7bab7e815ba79

2 files changed +38 -19

builtin/checkout.c

+32 -19

@@ -91,7 +91,7 @@ struct checkout_opts {

91 int new_branch_log;

92 enum branch_track track;

93 struct diff_options diff_options;

94 - char *conflict_style;

94 + int conflict_style;

96 int branch_exists;

97 const char *prefix;

@@ -100,6 +100,8 @@ struct checkout_opts {

100 struct tree *source_tree;

101 };

102

103 +#define CHECKOUT_OPTS_INIT { .conflict_style = -1 }

104 +

105 struct branch_info {

106 char *name; /* The short name used */

107 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,

253 }

254

255 static int checkout_merged(int pos, const struct checkout *state,

254 - int *nr_checkouts, struct mem_pool *ce_mem_pool)

256 + int *nr_checkouts, struct mem_pool *ce_mem_pool,

257 + int conflict_style)

258 {

259 struct cache_entry *ce = the_index.cache[pos];

260 const char *path = ce->name;

@@ -286,6 +289,7 @@ static int checkout_merged(int pos, const struct checkout *state,

289

290 git_config_get_bool("merge.renormalize", &renormalize);

291 ll_opts.renormalize = renormalize;

292 + ll_opts.conflict_style = conflict_style;

293 merge_status = ll_merge(&result_buf, path, &ancestor, "base",

294 &ours, "ours", &theirs, "theirs",

295 state->istate, &ll_opts);

@@ -416,7 +420,8 @@ static int checkout_worktree(const struct checkout_opts *opts,

420 else if (opts->merge)

421 errs |= checkout_merged(pos, &state,

422 &nr_unmerged,

419 - &ce_mem_pool);

423 + &ce_mem_pool,

424 + opts->conflict_style);

425 pos = skip_same_name(ce, pos) - 1;

426 }

427 }

@@ -886,6 +891,7 @@ static int merge_working_tree(const struct checkout_opts *opts,

891 }

892 o.branch1 = new_branch_info->name;

893 o.branch2 = "local";

894 + o.conflict_style = opts->conflict_style;

895 ret = merge_trees(&o,

896 new_tree,

897 work,

@@ -1616,6 +1622,21 @@ static int checkout_branch(struct checkout_opts *opts,

1622 return switch_branches(opts, new_branch_info);

1623 }

1624

1625 +static int parse_opt_conflict(const struct option *o, const char *arg, int unset)

1626 +{

1627 + struct checkout_opts *opts = o->value;

1628 +

1629 + if (unset) {

1630 + opts->conflict_style = -1;

1631 + return 0;

1632 + }

1633 + opts->conflict_style = parse_conflict_style_name(arg);

1634 + if (opts->conflict_style < 0)

1635 + return error(_("unknown conflict style '%s'"), arg);

1636 +

1637 + return 0;

1638 +}

1639 +

1640 static struct option *add_common_options(struct checkout_opts *opts,

1641 struct option *prevopts)

1642 {

@@ -1626,8 +1647,9 @@ static struct option *add_common_options(struct checkout_opts *opts,

1647 PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater),

1648 OPT_BOOL(0, "progress", &opts->show_progress, N_("force progress reporting")),

1649 OPT_BOOL('m', "merge", &opts->merge, N_("perform a 3-way merge with the new branch")),

1629 - OPT_STRING(0, "conflict", &opts->conflict_style, N_("style"),

1630 - N_("conflict style (merge, diff3, or zdiff3)")),

1650 + OPT_CALLBACK(0, "conflict", opts, N_("style"),

1651 + N_("conflict style (merge, diff3, or zdiff3)"),

1652 + parse_opt_conflict),

1653 OPT_END()

1654 };

1655 struct option *newopts = parse_options_concat(prevopts, options);

@@ -1718,15 +1740,9 @@ static int checkout_main(int argc, const char **argv, const char *prefix,

1740 opts->show_progress = isatty(2);

1741 }

1742

1721 - if (opts->conflict_style) {

1722 - struct key_value_info kvi = KVI_INIT;

1723 - struct config_context ctx = {

1724 - .kvi = &kvi,

1725 - };

1743 + if (opts->conflict_style >= 0)

1744 opts->merge = 1; /* implied */

1727 - git_xmerge_config("merge.conflictstyle", opts->conflict_style,

1728 - &ctx, NULL);

1729 - }

1745 +

1746 if (opts->force) {

1747 opts->discard_changes = 1;

1748 opts->ignore_unmerged_opt = "--force";

@@ -1891,7 +1907,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,

1907

1908 int cmd_checkout(int argc, const char **argv, const char *prefix)

1909 {

1894 - struct checkout_opts opts;

1910 + struct checkout_opts opts = CHECKOUT_OPTS_INIT;

1911 struct option *options;

1912 struct option checkout_options[] = {

1913 OPT_STRING('b', NULL, &opts.new_branch, N_("branch"),

@@ -1907,7 +1923,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)

1923 int ret;

1924 struct branch_info new_branch_info = { 0 };

1925

1910 - memset(&opts, 0, sizeof(opts));

1926 opts.dwim_new_local_branch = 1;

1927 opts.switch_branch_doing_nothing_is_ok = 1;

1928 opts.only_merge_on_switching_branches = 0;

@@ -1946,7 +1961,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)

1961

1962 int cmd_switch(int argc, const char **argv, const char *prefix)

1963 {

1949 - struct checkout_opts opts;

1964 + struct checkout_opts opts = CHECKOUT_OPTS_INIT;

1965 struct option *options = NULL;

1966 struct option switch_options[] = {

1967 OPT_STRING('c', "create", &opts.new_branch, N_("branch"),

@@ -1962,7 +1977,6 @@ int cmd_switch(int argc, const char **argv, const char *prefix)

1977 int ret;

1978 struct branch_info new_branch_info = { 0 };

1979

1965 - memset(&opts, 0, sizeof(opts));

1980 opts.dwim_new_local_branch = 1;

1981 opts.accept_ref = 1;

1982 opts.accept_pathspec = 0;

@@ -1988,7 +2002,7 @@ int cmd_switch(int argc, const char **argv, const char *prefix)

2002

2003 int cmd_restore(int argc, const char **argv, const char *prefix)

2004 {

1991 - struct checkout_opts opts;

2005 + struct checkout_opts opts = CHECKOUT_OPTS_INIT;

2006 struct option *options;

2007 struct option restore_options[] = {

2008 OPT_STRING('s', "source", &opts.from_treeish, "<tree-ish>",

@@ -2005,7 +2019,6 @@ int cmd_restore(int argc, const char **argv, const char *prefix)

2019 int ret;

2020 struct branch_info new_branch_info = { 0 };

2021

2008 - memset(&opts, 0, sizeof(opts));

2022 opts.accept_ref = 0;

2023 opts.accept_pathspec = 1;

2024 opts.empty_pathspec_ok = 0;

t/t7201-co.sh

@@ -631,6 +631,12 @@ test_expect_success 'checkout --conflict=diff3' '

631 test_cmp merged file

632 '

633

634 +test_expect_success 'checkout with invalid conflict style' '

635 + test_must_fail git checkout --conflict=bad 2>actual -- file &&

636 + echo "error: unknown conflict style ${SQ}bad${SQ}" >expect &&

637 + test_cmp expect actual

638 +'

639 +

640 test_expect_success 'failing checkout -b should not break working tree' '

641 git clean -fd && # Remove untracked files in the way

642 git reset --hard main &&