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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 14/03/2024 17:05, Phillip Wood via GitGitGadget wrote:

Here is the cover letter - I don't know why GGG keeps omitting it

Passing an invalid conflict style name such as "--conflict=bad" to "git checkout" 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 series refactors the implementation to pass the conflict style down the call chain to the merge machinery rather than abusing the config setting.

Thanks to Junio for his comments on v1, the changes in v2 are:
 - renamed parse_conflict_style() to parse_conflict_style_name()
 - parse --conflict using OPT_CALLBACK to avoid storing the string argument
 - added a new patch to fix the interaction of --conflict with --no-merge


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@xxxxxxxxxxxxx> @@ 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





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux