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