From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 29 Sep 2021 21:20:31 GMT
Subject: Re: [PATCH v5 3/3] send-email docs: add format-patch options
Message-ID: <xmqqpmsrcc4g.fsf@gitster.g>
In-Reply-To: <YU6+BWC+xvGJP3b0@carlos-mbp.lan>

Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:

>> Everything after "git format-patch", i.e. -2, master, master..HEAD,
>> are usable, and there isn't much point in singling out revision
>> ranges.  FWIW, I think you can even give "-- <pathspec>" at the end,
>> which are not options or revision ranges.
>
> <format-patch options> then it is; would the following be worth adding
> in top so the recursive reference can be followed?

I am not sure what "the recursive reference" is an issue here, but
I agree that we may want to improve upon <revision range> in the
part you are touching, which currently we say:

    There are two ways to specify which commits to operate on.

    1. A single commit, <since>, specifies that the commits leading
       to the tip of the current branch that are not in the history
       that leads to the <since> to be output.

    2. Generic <revision range> expression (see "SPECIFYING
       REVISIONS" section in linkgit:gitrevisions[7]) means the
       commits in the specified range.

    The first rule takes precedence in the case of a single <commit>.  To
    apply the second rule, i.e., format everything since the beginning of
    history up until <commit>, use the `--root` option: `git format-patch
    --root <commit>`.  If you want to format only <commit> itself, you
    can do this with `git format-patch -1 <commit>`.

What we refer to in the prose, e.g. "--root" and " -1", do not
appear in the SYNOPSIS section.

> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> index fe2f69d36e..806ff93259 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -30,7 +30,7 @@ SYNOPSIS
>  		   [--range-diff=<previous> [--creation-factor=<percent>]]
>  		   [--filename-max-length=<n>]
>  		   [--progress]
> -		   [<common diff options>]
> +		   [<common diff options>] [<rev-list options>]
>  		   [ <since> | <revision range> ]

I think the "<rev-list options>" you are adding here is to enhance
what <revision range> in the original wants to convey.  In addition
to things like @{u}..HEAD~2 (i.e. "the branch is mostly good, but
the tip 2 commits are not yet ready so do not send them out"), you
can do "-2" (i.e. "the topmost 2 commits"), which is not exactly
what "SPECIFYING REVISIONS" part of gitrevisions(7) describes.

So, yes, I like the spirit of the change, but no, I do not think it
goes there; rather, it would replace or extend <revision range>, I
would think.

In addition, "Generic <revision range> expression (see "SPECIFYING
REVISIONS" section...) may need to be updated.  First, what we'd
want to refer to is not ways to specify revisions, but ways to
specify a range.  IOW, it should be referring to "SPECIFYING RANGES"
section instead.

If we replace <revision range> with your <rev-list options> in the
SYNOPSIS, that will fall out as a natural consequence.  Perhaps, the
second description and an earlier part of the explanation can be
rewritten like so:

    2. <rev-list options> that specifies a range of commits (see
       linkgit:git-rev-list[1]) to be shown.

    If you give a single <commit> and nothing else, it is taken as
    the <since> of the first form.  If you want to format everything 
    since the beginning of history up until <commit>, use ...

Thanks.