Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
>> + if (argc == 3 && !strcmp(argv[2], "-"))
> ...
> To address these issues, it seems like a more correct place to
> recognize "-" as an alias would be somewhere within
> builtin/branch.d:delete_branches().
I agree with all your review comments, and the above would also
cover another issue you did not mention above: "git branch -d/-D"
actually takes one or more branches to delete.
Another issue to consider is if we get a sensible error message.
What should the error message say if a user did these in a freshly
created empty repository?
$ git commit --allow-empty -m initial
$ git commit --allow-empty -m second
$ git branch side master^
$ git branch -d -
Currently, you would get
$ git branch -d -
error: branch '-' not found.
Is it OK to say
error: branch '@{-1}' not found.
when the user never said '@{-1}' from the command line?
I am not saying it _is_ a problem--it may very well be acceptable,
as long as the users understand that '-' is merely a short-hand for
'@{-1}'. Either way, the log message needs to have some evidence
that the patch author thought about the issue, the resolution the
patch author chose for the issue, and the reason why a particular
resolution was chosen.
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