From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 26 Nov 2009 06:16:07 GMT
Subject: Re: [PATCH 4/8] Teach git-merge to pass -X<option> to the backend strategy module
Message-ID: <7vk4xderq0.fsf@alter.siamese.dyndns.org>
In-Reply-To: <73a42e99b4a083c74b017caf2970d1bbf5886b96.1259201377.git.apenwarr@gmail.com>

Avery Pennarun <apenwarr@gmail.com> writes:

> Distinguishing slight variation of modes of operation between the vanilla
> merge-recursive and merge-recursive-ours by the command name may have been
> an easy way to experiment, but we should bite the bullet and allow backend
> specific options to be given by the end user.
>
> (Patch originally by Junio Hamano <gitster@pobox.com>.)
>
> Signed-off-by: Avery Pennarun <apenwarr@gmail.com>
> ---
>  Makefile                     |    3 ---
>  builtin-merge-recursive.c    |   21 +++++++++++++++------
>  builtin-merge.c              |   40 ++++++++++++++++++++++++++++++++++++----
>  t/t6034-merge-ours-theirs.sh |    4 ++--
>  4 files changed, 53 insertions(+), 15 deletions(-)

You added .gitignore entries for the two programs previously, and are
removing them in this patch, but forgot to remove them from .gitignore in
this patch.

As I already suggested you to, if you squash this to the previous one, it
is not a big deal, though.

> diff --git a/builtin-merge.c b/builtin-merge.c
> index df089bb..9a95bc8 100644
> --- a/builtin-merge.c
> +++ b/builtin-merge.c
> @@ -148,6 +148,17 @@ static int option_parse_strategy(const struct option *opt,
>  	return 0;
>  }
>  
> +static int option_parse_x(const struct option *opt,
> +			  const char *arg, int unset)
> +{
> +	if (unset)
> +		return 0;

Should "merge --no-extended" silently be ignored, or be diagnosed as an
error?

> @@ -174,6 +185,8 @@ static struct option builtin_merge_options[] = {
>  		"abort if fast-forward is not possible"),
>  	OPT_CALLBACK('s', "strategy", &use_strategies, "strategy",
>  		"merge strategy to use", option_parse_strategy),
> +	OPT_CALLBACK('X', "extended", &xopts, "option=value",
> +		"option for selected merge strategy", option_parse_x),

I actually didn't name X for "extended" but more for "external" (to the
merge program proper).  "--strategy-option" perhaps?