[RFC] Documentation: better document format-patch options in send-email

Commit Message

Carlo Marcelo Arenas Belón Oct. 9, 2021, 8:31 a.m. UTC

From: Thiago Perrotta <[e-mail]>

The use of <rev-list options> in format-patch is confusing because
it is meant to represent instead a subset of options from format-patch
and a revision range.

Change the documentation from both git send-email and format-patch to
better describe the list of options that are expected.

Signed-off-by: Carlo Marcelo Arenas Belón <[e-mail]>
---
This is an alternative to patch 3 which include most of the suggestions
by Junio and Bagas, while also trying to minimize the change or the need
to link to even more pages (which is why the suggestion[1] to link/use
rev-list options was avoided).

I expect it to be consistent and clear, but might had gotten some of the
terminology wrong, and probably could have better grammar, so hoping for
some feedback to improve on that.

[1] https://lore.kernel.org/git/CABOtWuqXS_kJk2md=[e-mail]/T/#m396f2a42936ec9a3191658d4b40d9043dfbc59e2

 Documentation/git-format-patch.txt | 20 +++++++++++---------
 Documentation/git-send-email.txt   |  7 ++++---
 2 files changed, 15 insertions(+), 12 deletions(-)

Comments

Bagas Sanjaya Oct. 9, 2021, 8:57 a.m. UTC | #1

On 09/10/21 15.31, Carlo Marcelo Arenas Belón wrote:
> -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>`.
> +2. A Generic <revision range> expression that describes with
> +   options and revisions a range of commits.
> +
> +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 the `--root` option:
> +`git format-patch --root <commit>`.  If you want to format only the
> +<commit> itself, use instead `git format-patch -1 <commit>`.
> +
> +If you want to affect a set of commits, provide a range (see
> +"SPECIFYING RANGES" section in linkgit:gitrevisions[7]).
>   

Supposed that we have following commit graph:

a --- b --- c --- d --- e --- f (main)
             \
              --- g --- h --- i (mywork, HEAD)

According to your edit, `git format-patch <c>` and `git format-patch 
--root <i>` are equivalent, right?

I think for the third case, s/affect/format/.

> @@ -18,8 +19,8 @@ DESCRIPTION
>   Takes the patches given on the command line and emails them out.
>   Patches can be specified as files, directories (which will send all
>   files in the directory), or directly as a revision list.  In the
> -last case, any format accepted by linkgit:git-format-patch[1] can
> -be passed to git send-email.
> +last case, a revision in a format accepted by linkgit:git-format-patch[1]
> +as well as relevant options must be provided to git send-email.
>   
>   The header of the email is configurable via command-line options.  If not
>   specified on the command line, the user will be prompted with a ReadLine
> 

Did you mean that in the second form of git send-email, only 
format-patch options are accepted?

Carlo Marcelo Arenas Belón Oct. 9, 2021, 9:32 a.m. UTC | #2

On Sat, Oct 9, 2021 at 1:57 AM Bagas Sanjaya <[e-mail]> wrote:
>
> On 09/10/21 15.31, Carlo Marcelo Arenas Belón wrote:
> > -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>`.
> > +2. A Generic <revision range> expression that describes with
> > +   options and revisions a range of commits.
> > +
> > +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 from the
> > +beginning of history up until <commit>, use the `--root` option:
> > +`git format-patch --root <commit>`.  If you want to format only the
> > +<commit> itself, use instead `git format-patch -1 <commit>`.
> > +
> > +If you want to affect a set of commits, provide a range (see
> > +"SPECIFYING RANGES" section in linkgit:gitrevisions[7]).
>
> Supposed that we have following commit graph:
>
> a --- b --- c --- d --- e --- f (main)
>              \
>               --- g --- h --- i (mywork, HEAD)
>
> According to your edit, `git format-patch <c>` and `git format-patch
> --root <i>` are equivalent, right?

I didn't change the definition of --root with my edit, but I guess it
is still confusing.

the beginning of history from your tree is a, c would be a "fork
point" AFAIK, but if you use --root will get 6 patches (a, b, c, g, h,
i)

> > @@ -18,8 +19,8 @@ DESCRIPTION
> >   Takes the patches given on the command line and emails them out.
> >   Patches can be specified as files, directories (which will send all
> >   files in the directory), or directly as a revision list.  In the
> > -last case, any format accepted by linkgit:git-format-patch[1] can
> > -be passed to git send-email.
> > +last case, a revision in a format accepted by linkgit:git-format-patch[1]
> > +as well as relevant options must be provided to git send-email.
> >
> >   The header of the email is configurable via command-line options.  If not
> >   specified on the command line, the user will be prompted with a ReadLine
>
> Did you mean that in the second form of git send-email, only
> format-patch options are accepted?

The synopsis shows "send-email options" are also allowed (as
optional), the mention of "relevant options" here was to indicate
additional options from git-format-patch which make sense (ex: common
diff options, --root, or the options from the range section of
references).

The truth is that you can actually do files, directories and revisions
now, but that is a bug.

Carlo

Bagas Sanjaya Oct. 9, 2021, 11:04 a.m. UTC | #3

On 09/10/21 16.32, Carlo Arenas wrote:
> The synopsis shows "send-email options" are also allowed (as
> optional), the mention of "relevant options" here was to indicate
> additional options from git-format-patch which make sense (ex: common
> diff options, --root, or the options from the range section of
> references).
> 
> The truth is that you can actually do files, directories and revisions
> now, but that is a bug.
> 
> Carlo
> 

git send-email behaves similar to ln(1), depending on what options and 
arguments that are passed. See my attempted patch [1] for the wording.

[1]: 
https://lore.kernel.org/git/[e-mail]/

For SYNOPSIS for file/directory and revision range, I suggest:

'git send-email' [<options>] <file|directory>...
'git send-email' [<send-email options>] [<format-patch options>] 
<revision range>

where <revision range> must be any forms that are accepted by 
git-format-patch(1).

Junio C Hamano Oct. 10, 2021, 9:33 p.m. UTC | #4

Carlo Arenas <[e-mail]> writes:

>> > +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 from the
>> > +beginning of history up until <commit>, use the `--root` option:
>> > +`git format-patch --root <commit>`.
>>
>> Supposed that we have following commit graph:
>>
>> a --- b --- c --- d --- e --- f (main)
>>              \
>>               --- g --- h --- i (mywork, HEAD)
>>
>> According to your edit, `git format-patch <c>` and `git format-patch
>> --root <i>` are equivalent, right?
>
> I didn't change the definition of --root with my edit, but I guess it
> is still confusing.

The explanation you gave Bagas describes what these two variants,
i.e. "<c> alone" and "--root <c>", mean and how they behave very
clearly.  In fact, the 4-line description is designed to encourage
readers to compare a pair of cases like these, using the same commit
in the middle of the history you get two quite different set of
commits with and without "--root".

You can ask for "--root <i>" to get a-b-c-g-h-i but it is not
a good example as there is no useful contrasting example that does
not use "--root", as "git format-patch i" would be showing an empty
set.

The fact that somebody wanted to compare between "<c>" and "--root
<i>" might be a sign that the 4-line description is still somehow
confusing, but I cannot quite figure out what needs to be improved
in the description to avoid such a confusion (IOW, I fail to see how
the description can lead to such a confusion).

Thanks.

Patch

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index fe2f69d36e..fa2377d5e9 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -64,15 +64,17 @@  There are two ways to specify which commits to operate on.
    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>`.
+2. A Generic <revision range> expression that describes with
+   options and revisions a range of commits.
+
+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 the `--root` option:
+`git format-patch --root <commit>`.  If you want to format only the
+<commit> itself, use instead `git format-patch -1 <commit>`.
+
+If you want to affect a set of commits, provide a range (see
+"SPECIFYING RANGES" section in linkgit:gitrevisions[7]).
 
 By default, each output file is numbered sequentially from 1, and uses the
 first line of the commit message (massaged for pathname safety) as
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 3db4eab4ba..16d8d19cf5 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -9,7 +9,8 @@  git-send-email - Send a collection of patches as emails
 SYNOPSIS
 --------
 [verse]
-'git send-email' [<options>] <file|directory|rev-list options>...
+'git send-email' [<send-email options>] <file|directory>...
+'git send-email' [<send-email options>] <format-patch options>
 'git send-email' --dump-aliases
 
 
@@ -18,8 +19,8 @@  DESCRIPTION
 Takes the patches given on the command line and emails them out.
 Patches can be specified as files, directories (which will send all
 files in the directory), or directly as a revision list.  In the
-last case, any format accepted by linkgit:git-format-patch[1] can
-be passed to git send-email.
+last case, a revision in a format accepted by linkgit:git-format-patch[1]
+as well as relevant options must be provided to git send-email.
 
 The header of the email is configurable via command-line options.  If not
 specified on the command line, the user will be prompted with a ReadLine