Re: [PATCH v5 3/3] send-email docs: add format-patch options

From: Carlo Arenas <carenas@gmail.com>
Date: Sep 24, 2021, 17:34 UTC
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:

Show 38 quoted lines

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