Re: What's cooking in git.git (Jan 2010, #01; Mon, 04)

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> It might be easier to understand like this:
>
> 	case "$1" in
> 	*...*)
> 		left=${1%...*} &&
> 		right=${1#*...} &&
> 		onto="$(git merge-base "${left:-HEAD}" "${right:-HEAD}")" &&
> 		test ! -z "$onto" &&
> 		echo "$onto"
> 	;;
> 	*)
> 		git rev-parse --verify "$1^0"
> 	;;
> 	esac

Double-semicolons should be indented one level deeper.

I think your version may be slightly better (avoids one "expr"), but it
actually was much harder to read your cascade of && that implicitly exits
with non-zero status in the first case arm than the explicit exit status
given by the original patch.

As far as I can tell, both versions inherit the same bug from me when the
user gave us A...B pair that has more than one merge bases.  I think you
need to give --all to merge-base and resurrect the "did we get more than
one" test from her patch.

> Besides, why do you change the "$1" to "$1^0"?

Isn't it a bugfix?

Earlier code wouldn't have caught "--onto $blob_id" as an error, but this
will do so---I actually think it is a good change.

>> diff --git a/git-rebase.sh b/git-rebase.sh
>> index 6503113..43c62c0 100755
>> --- a/git-rebase.sh
>> +++ b/git-rebase.sh
>
> I would separate the patches.  rebase.sh and rebase--interactive.sh are 
> fundamentally different.

I too think splitting into two patches would make sense in this case.  The
patch to git-rebase.sh seems to be a bugfix in the left/right computation;
I am kind of surprised that I haven't triggered it myself so far.

Thanks.