Re: [PATCH] Allow "-" as a short-hand for "@{-1}" in "git branch -d @{-1}"

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 6, 2016, 21:09 UTC
Message-ID: <xmqqa8mbtit2.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAPig+cTHBLGjn_tnAKE2i_zv9TRxdVbNfONpxg=ZvRSz9-4t=Q@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 5 quoted lines

>> +       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.