Re: [PATCH v2 2/6] doc: align 'diff --no-index' in text with synopsis

From: Martin Ågren <martin.agren@gmail.com>
Date: Apr 27, 2018, 18:45 UTC
Message-ID: <CAN0heSquZ9WGQE=SCZ0DkgsbKPc80r5TmYhNXOGKFJ6MaJ_sOQ@mail.gmail.com>
In-Reply-To: <a3253c99-63a4-22bc-53a0-4d94c01a564c@gmail.com>

On 27 April 2018 at 20:28, Andreas Heiduk <asheiduk@gmail.com> wrote:

Show 28 quoted lines

> Am 27.04.2018 um 19:18 schrieb Martin Ågren:
>> On 27 April 2018 at 19:04, Andreas Heiduk <asheiduk@gmail.com> wrote:
>>> The two '<path>' parameters are not optional but the option
>>> '--no-index' is. Also move the `--options` part to the same
>>> place where the other variants show them.
>>
>> Should this commit message be updated after the changes you did to
>> address Junio's comment? This text suggests you want to place --no-index
>> in [] (and you did in v1) but you do not do that below.
>>
>>> All three items are already correct in the synopsis.
>>
>> Same here, now you actually do change things there.
>>
>>> Signed-off-by: Andreas Heiduk <asheiduk@gmail.com>
>>> Reviewed-by: Martin Ågren <martin.agren@gmail.com>
>>
>> Strictly speaking, my Reviewed-by was on another patch. I do find this
>
> Sorry, I've added that trailer after reading "The diff LGTM.", then
> applied Junio's changes and forgot to remove the trailer.
>
>> one better though thanks to Junio's suggestion (except the mismatch with
>> the commit message).
>
> I'll fix that with this:
>
>         doc: align 'diff --no-index' in text with synopsis

s/with/and/ since they both change? It's not that the first changes to match the second, but they actually both change to match each other (and to be correct, obviously).

>         Make the two '<path>' parameters in DESCRIPTION mandatory and
>         move the `--options` part to the same place where the other
>         variants show them. And finally make `--no-index` in SYNOPSIS
>         as mandatory as in DESCRIPTION.

Great! Junio had some good reasoning about how --no-index is sometimes optional, but not always. Not sure if it's worth spelling that out. (Although one could argue that it already did trip us up once. :-))

Eric's point about "--options" vs "options" seemed right to me. If you address that, note that this message says "--options".