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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Johannes Schindelin <Johannes.Schindelin@xxxxxx> 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.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]