From: Karthik Nayak <karthik.188@gmail.com>
Date: Fri, 18 Sep 2015 15:10:35 GMT
Subject: Re: [PATCH v17 14/14] tag.c: implement '--merged' and '--no-merged' options
Message-ID: <CAOLa=ZQmGD2vo1B2K21-f_2RZRXduFt4VNKq2Pp9B_5bO=8=+w@mail.gmail.com>
In-Reply-To: <20150918084208.GJ17201@serenity.lan>

On Fri, Sep 18, 2015 at 2:12 PM, John Keeping <john@keeping.me.uk> wrote:
> On Fri, Sep 18, 2015 at 09:10:08AM +0200, Matthieu Moy wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>> > John Keeping <john@keeping.me.uk> writes:
>> >
>> >>> +--[no-]merged [<commit>]::
>> >>
>> >> We prefer to write --[no-]* as:
>> >>
>> >>    --option::
>> >>    --no-option::
>> >>
>> >> although this may be the first instance that we see this combination
>> >> with an argument.
>> >>
>> >> I also found the "[<commit>]" syntax confusing and had to go and figure
>> >> out what PARSE_OPT_LASTARG_DEFAULT does; I wonder if it's worth
>> >> appending something like the following to the help for this option:
>> >>
>> >>    The `commit` may be omitted if this is the final argument.
>> >
>> > "may be omitted" must be followed by a description of what happens
>> > when omitted (i.e. "defaults to ...").
>>
>> Then:
>>
>> The `commit` may be omitted and defaults to HEAD if this is the final
>> argument.
>
> I find that slightly confusing, although that might just be me.  It's
> slightly longer, but I would write:
>
>         The `commit` may be omitted if this is the final argument, in
>         which case it defaults to `HEAD`.
>

This seems good to be included.

> I also had a look at git-branch(1), which has similar `--merged` and
> `--no-merged` options and says:
>
>         Only list branches whose tips are reachable from the specified
>         commit (HEAD if not specified).  Implies `--list`.
>
> The two options are listed separately in that case.

Not sure this is much of a problem with regards to "--[no-]merged"
I mean isn't the square brackets self-explanatory?

-- 
Regards,
Karthik Nayak