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

From: "Phillip Wood via GitGitGadget" <[email-masked]>
Date: Thu, 14 Mar 2024 17:05:02 +0000

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 <[email-masked]>
      
     @@ 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

-- 
gitgitgadget