Re: [PATCH 4/4] git-rebase -i: New option to support rebase with merges

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 23, 2008, 22:41 UTC
Message-ID: <alpine.LSU.1.00.0803232334130.4353@racer.site>
In-Reply-To: <1206308562-31489-4-git-send-email-joerg@alea.gnuu.de>

Hi,

On Sun, 23 Mar 2008, Jörg Sommer wrote:

> The option --preserve-merges does not allow to change the order of 
> commits or squash them. The new option --linear-history does support 
> this, but doing so it can only look at the commits reachable with 
> through the first parent of each merge.

Why do you call it "linear-history"? That name is pretty ambiguous. Why not calling it "--first-parents"?

> Joining merge commits with other commits leads to problems, because git
> merge fails with a dirty index (the case “COMMIT squash MERGE”) and
> squashing a merge leads to the lost of the parents (case “MERGE squash
> COMMIT”).

Please use the term "to squash" not "to join". And say "to the loss" instead of "to the lost".

And I still think that it would be better to fix the bug that squashing merges fails.

Show 6 quoted lines

> @@ -247,6 +248,13 @@ OPTIONS
>  	Instead of ignoring merges, try to recreate them.  This option
>  	only works in interactive mode.
>  
> +-l, \--linear-history::
> +	Use only commits of the branch they are not merged in, i.e.

s/they are/that are/

> +	follow only the first parent of a merge. Merges are part of this

s/first parent of a merge/first parents of the encountered merge commits/

Show 17 quoted lines

> @@ -150,7 +152,18 @@ pick_one () {
>  		sha1=$(git rev-parse --short $sha1)
>  		output warn Fast forward to $sha1
>  	else
> -		output git cherry-pick "$@"
> +		if test t = "$LINEAR_HISTORY" &&
> +			other_parents="$(parents_of_commit $sha1 | cut -s -d' ' -f2-)" &&
> +			test -n "$other_parents"
> +		then
> +			if test a"$1" = a-n
> +			then
> +				merge_opt=--no-commit
> +			fi
> +			redo_merge $sha1 $no_commit $other_parents
> +		else
> +			output git cherry-pick "$@"
> +		fi

Now, that is funny. In case of --preserve-merges, I would have expected you to touch pick_one_preserving_merges(), not pick_one().

I would find it highly illogical to try to redo merges _without_ -p.

And again, I have to stress that fixing -p for the cases you mentioned should be a higher priority than to introduce new options to work around the bugs. Seems like I am repeating myself, but hopefully I don't have to do that many more times.

Ciao, Dscho