Re: [PATCH v2 0/5] checkout: cleanup --conflict=

> Phillip Wood (5):
>    xdiff-interface: refactor parsing of merge.conflictstyle
>    merge-ll: introduce LL_MERGE_OPTIONS_INIT
>    merge options: add a conflict style member
>    checkout: cleanup --conflict=<style> parsing
>    checkout: fix interaction between --conflict and --merge
> 
>   builtin/checkout.c | 60 +++++++++++++++++++++++++----------------
>   merge-ll.c         |  6 +++--
>   merge-ll.h         |  5 ++++
>   merge-ort.c        |  3 ++-
>   merge-recursive.c  |  5 +++-
>   merge-recursive.h  |  1 +
>   t/t7201-co.sh      | 66 ++++++++++++++++++++++++++++++++++++++++++++++
>   xdiff-interface.c  | 29 ++++++++++++--------
>   xdiff-interface.h  |  1 +
>   9 files changed, 139 insertions(+), 37 deletions(-)
> 
> 
> base-commit: b387623c12f3f4a376e4d35a610fd3e55d7ea907
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1684%2Fphillipwood%2Frefactor-conflict-style-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1684/phillipwood/refactor-conflict-style-v2
> Pull-Request: https://github.com/gitgitgadget/git/pull/1684
> 
> Range-diff vs v1:
> 
>   1:  0263d001634 ! 1:  629e577879c xdiff-interface: refactor parsing of merge.conflictstyle
>       @@ xdiff-interface.c: int xdiff_compare_lines(const char *l1, long s1,
>         	return xdl_recmatch(l1, s1, l2, s2, flags);
>         }
>         
>       -+int parse_conflict_style(const char *value)
>       ++int parse_conflict_style_name(const char *value)
>        +{
>        +	if (!strcmp(value, "diff3"))
>        +		return XDL_MERGE_DIFF3;
>       @@ xdiff-interface.c: int git_xmerge_config(const char *var, const char *value,
>        -		 * git-completion.bash when you add new merge config
>        -		 */
>        -		else
>       -+		git_xmerge_style = parse_conflict_style(value);
>       ++		git_xmerge_style = parse_conflict_style_name(value);
>        +		if (git_xmerge_style == -1)
>         			return error(_("unknown style '%s' given for '%s'"),
>         				     value, var);
>       @@ xdiff-interface.h: int buffer_is_binary(const char *ptr, unsigned long size);
>         void xdiff_set_find_func(xdemitconf_t *xecfg, const char *line, int cflags);
>         void xdiff_clear_find_func(xdemitconf_t *xecfg);
>         struct config_context;
>       -+int parse_conflict_style(const char *value);
>       ++int parse_conflict_style_name(const char *value);
>         int git_xmerge_config(const char *var, const char *value,
>         		      const struct config_context *ctx, void *cb);
>         extern int git_xmerge_style;
>   2:  4e05bc156bc = 2:  46e0f5a0af7 merge-ll: introduce LL_MERGE_OPTIONS_INIT
>   3:  c0d7bafd438 = 3:  47d0ec28a55 merge options: add a conflict style member
>   4:  317bb7a70d0 ! 4:  511e03d3db2 checkout: cleanup --conflict=<style> parsing
>       @@ Commit message
>            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() 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().
>       +    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: struct checkout_opts {
>         	enum branch_track track;
>         	struct diff_options diff_options;
>        -	char *conflict_style;
>       -+	char *conflict_style_name;
>        +	int conflict_style;
>         
>         	int branch_exists;
>       @@ builtin/checkout.c: static int merge_working_tree(const struct checkout_opts *op
>         			ret = merge_trees(&o,
>         					  new_tree,
>         					  work,
>       +@@ builtin/checkout.c: 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)
>       + {
>        @@ builtin/checkout.c: 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"),
>       -+		OPT_STRING(0, "conflict", &opts->conflict_style_name, N_("style"),
>       - 			   N_("conflict style (merge, diff3, or zdiff3)")),
>       +-			   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);
>        @@ builtin/checkout.c: static int checkout_main(int argc, const char **argv, const char *prefix,
>         			opts->show_progress = isatty(2);
>         	}
>       @@ builtin/checkout.c: static int checkout_main(int argc, const char **argv, const
>        -		struct config_context ctx = {
>        -			.kvi = &kvi,
>        -		};
>       -+	if (opts->conflict_style_name) {
>       ++	if (opts->conflict_style >= 0)
>         		opts->merge = 1; /* implied */
>        -		git_xmerge_config("merge.conflictstyle", opts->conflict_style,
>        -				  &ctx, NULL);
>       -+		opts->conflict_style =
>       -+			parse_conflict_style(opts->conflict_style_name);
>       -+		if (opts->conflict_style < 0)
>       -+			die(_("unknown conflict style '%s'"),
>       -+			    opts->conflict_style_name);
>       - 	}
>       +-	}
>       ++
>         	if (opts->force) {
>         		opts->discard_changes = 1;
>       + 		opts->ignore_unmerged_opt = "--force";
>        @@ builtin/checkout.c: static int checkout_main(int argc, const char **argv, const char *prefix,
>         
>         int cmd_checkout(int argc, const char **argv, const char *prefix)
>       @@ t/t7201-co.sh: test_expect_success 'checkout --conflict=diff3' '
>         
>        +test_expect_success 'checkout with invalid conflict style' '
>        +	test_must_fail git checkout --conflict=bad 2>actual -- file &&
>       -+	echo "fatal: unknown conflict style ${SQ}bad${SQ}" >expect &&
>       ++	echo "error: unknown conflict style ${SQ}bad${SQ}" >expect &&
>        +	test_cmp expect actual
>        +'
>        +
>   -:  ----------- > 5:  b771b29e45a checkout: fix interaction between --conflict and --merge
>