Re: [PATCH 02/18] Add a new builtin: branch-diff
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- May 4, 2018, 07:27 UTC
- Message-ID
- <CAPig+cSVve5vK6F6CgOoTahKZtzU5Pvv8c6DJzFSMeeqcB1fug@mail.gmail.com>
- In-Reply-To
- <nycvar.QRO.7.76.6.1805040843050.77@tvgsbejvaqbjf.bet>
On Fri, May 4, 2018 at 2:52 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 12 quoted lines
> On Thu, 3 May 2018, Eric Sunshine wrote:
>> On Thu, May 3, 2018 at 11:30 AM, Johannes Schindelin
>> <johannes.schindelin@gmx.de> wrote:
>> > +static const char * const builtin_branch_diff_usage[] = {
>> > + N_("git rebase--helper [<options>] ( A..B C..D | A...B | base A B )"),
>>
>> The formatting of "<options>" vs. "base" confused me into thinking
>> that the latter was a literal keyword, but I see from reading patch
>> 3/18 that it is not a literal at all, thus probably ought to be
>> specified as "<base>".
>
> Good point. Or maybe BASE?Indeed, that's probably more consistent with 'A', 'B', etc. than <base>.
Show 11 quoted lines
> Or I should just use the same convention as in the man page. Or not, as
> the usage should be conciser.
>
> This is what I have currently:
>
> static const char * const builtin_branch_diff_usage[] = {
> N_("git branch-diff [<options>] <old-base>..<old-tip> <new-base>..<new-tip>"),
> N_("git branch-diff [<options>] <old-tip>...<new-tip>"),
> N_("git branch-diff [<options>] <base> <old-tip> <new-tip>"),
> NULL
> };I can live with this. It's more verbose but more self-explanatory, thus likely a good choice.