Thread (1 message) 1 message, 1 author, 2016-06-15

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:57:52

Arnaud Fontaine [off-list ref] 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.

quoted hunk
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.
quoted hunk
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?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help