From: Carlo Arenas <carenas@gmail.com>
Date: Fri, 24 Sep 2021 17:34:19 GMT
Subject: Re: [PATCH v5 3/3] send-email docs: add format-patch options
Message-ID: <CAPUEspiSh91a8MvhqhKm=DAn5_u-HoRBuL6-gBRMUtSshqpf=g@mail.gmail.com>
In-Reply-To: <xmqqzgs2vvim.fsf@gitster.g>

On Fri, Sep 24, 2021 at 8:33 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Carlo Arenas <carenas@gmail.com> writes:
>
> > On Thu, Sep 23, 2021 at 9:36 PM Bagas Sanjaya <bagasdotme@gmail.com> wrote:
> >>
> >> On 24/09/21 09.46, Thiago Perrotta wrote:
> >> >   SYNOPSIS
> >> >   --------
> >> >   [verse]
> >> > -'git send-email' [<options>] <file|directory|rev-list options>...
> >> > +'git send-email' [<options>] <file|directory>...
> >> > +'git send-email' [<options>] <format-patch options>
> >> >   'git send-email' --dump-aliases
> >>
> >> Is <format-patch options> optional? If so, we can say [<format-patch
> >> options>].
> >
> > no; as Junio explained [1] this omission is intentional while the
> > rev-list options that
> > got cut to make space are not and are more relevant.
> >
> > IMHO leaving [<options>] to imply ALL options (that also include diff
> > options, for example) is better
>
> Could you claify this idea a bit more?  Do you mean that the second form
> can just be:
>
>         git send-email <format-patch options>
>
> That will exclude the send-email specific ones (like
> "--in-reply-to=<msg>"), so it may not be a great idea.
>
> Or do you mean
>
>         git send-email <options>
>
> and have the <options> placeholder to stand for both send-email
> options and options meant for format-patch?

the later AND including a non optional part that explains that you
need to indicate some sort of revision (which hopefully will also
imply to users that all the related options are also expected),
but also won't be specific, not to promote the use of this type of
mode or the need to update it further to include the whole universe of
options through format-patch (ex: log and diff)

Granted, I could have worded it better, but Bagas' proposal[1] (based
on this discussion) does and clarifies both; why this is not optional
in a way that is less confusing than what Thiago posted and is IMHO[2]
a good candidate for replacing this patch in the series

Carlo

[1] https://lore.kernel.org/git/20210924121352.42138-1-bagasdotme@gmail.com
[2] https://lore.kernel.org/git/CAPUEsphn15H9HbHKHRk+GFMvjq5O=8NL0Vo4L8NoUwiRrQUJJA@mail.gmail.com/T/#m6f0600b597fbe0b59aa767603a7f1d24d60e8fe9