Pranit Bauva [off-list ref] writes:
quoted
Also this version fails to catch "bisect reset a b c" as an error, I
suspect.
It didn't when I tried it right now. Could you please elaborate on why
you think it can fail? There might be a thing which I haven't tested.
My bad. I just compared your bisect_reset() implementation that had
if (no specific commit) {
reset to the branch
} else {
reset to the commit
}
with the original
case $# in
0) reset to the branch ;;
1) reset to the commit ;;
*) give usage and die ;;
esac
and took the difference and reacted "ah, excess parameters are not
diagnosed in this function".
Your caller does complain about excess parameters without giving
usage, and that is what I missed.
I am not sure if you intended to change the behaviour in this case
to avoid giving the usage string; I tend to think it is a good
change, but I didn't see it mentioned in the proposed commit log,
which also contributed to my not noticing the test in the caller.
Hey Junio,
On Fri, Aug 26, 2016 at 9:59 PM, Junio C Hamano [off-list ref] wrote:
Pranit Bauva [off-list ref] writes:
quoted
quoted
Also this version fails to catch "bisect reset a b c" as an error, I
suspect.
It didn't when I tried it right now. Could you please elaborate on why
you think it can fail? There might be a thing which I haven't tested.
My bad. I just compared your bisect_reset() implementation that had
if (no specific commit) {
reset to the branch
} else {
reset to the commit
}
with the original
case $# in
0) reset to the branch ;;
1) reset to the commit ;;
*) give usage and die ;;
esac
and took the difference and reacted "ah, excess parameters are not
diagnosed in this function".
Your caller does complain about excess parameters without giving
usage, and that is what I missed.
I am not sure if you intended to change the behaviour in this case
to avoid giving the usage string; I tend to think it is a good
change, but I didn't see it mentioned in the proposed commit log,
which also contributed to my not noticing the test in the caller.
I could include this in the commit message. Its not really something
which we would want to test in the function because to the function,
we are not passing the raw arguments. Since we are removing that check
from the function but including it in cmd_bisect__helper(), I will
talk about it in the commit message.
Regards,
Pranit Bauva