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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 29, 2021, 21:20 UTC
Message-ID: <xmqqpmsrcc4g.fsf@gitster.g>
In-Reply-To: <YU6+BWC+xvGJP3b0@carlos-mbp.lan>

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

Show 7 quoted lines

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

Show 11 quoted lines

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