Re: [PATCH] Do not ignore merge options in interactive rebase

[Junio C Hamano <gitster@pobox.com>]

Arnaud Fontaine <arnau@debian.org> writes:

> Merge strategy and its options can be specified in `git rebase`,
> but with `--interactive`, they were completely ignored.

And why is it a bad thing?  If you meant s/--interactive/-m/ in the
above, then I can sort of understand the justification, though.


> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> old mode 100644
> new mode 100755

I see an unjustifiable mode change here.

> index f953d8d..c157fdf
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -239,7 +239,16 @@ pick_one () {
>  
>  	test -d "$rewritten" &&
>  		pick_one_preserving_merges "$@" && return
> -	output git cherry-pick $empty_args $ff "$@"
> +
> +	if test -n "$do_merge"
> +	then

So you _did_ mean "rebase -m"?

> +		test -z "$strategy" && strategy=recursive
> +		output git cherry-pick --strategy=$strategy \

This is a bad change.

I would understand if the above were:

	git cherry-pick ${strategy+--strategy=$strategy} ...

in other words, "if there is no strategy specified, do not override
the configured default that might be different from recursive"
(pull.twohead may be set to resolve).

> +			$(echo $strategy_opts | sed "s/'--\([^']*\)'/-X\1/g") \

Is it guaranteed $startegy_opts do not have a space in it?

There is a call to "git merge" that uses "${strategy+-s $strategy}",
but it does not seem to propagate the strategy option.  Does it need
a similar change?  It seems that the first step might be to factor
out these calls to the "git cherry-pick" and "git merge" to helper
functions to make it easier to call them with -s/-X options in a
consistent way.

> +			$empty_args $ff "$@"
> +	else
> +		output git cherry-pick $empty_args $ff "$@"
> +	fi

It seems that there is another call to "git cherry-pick" in the
script ("git grep" for it).  Does it need a similar change?