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