diff mbox series

send-email: clarify dual-mode behavior

Message ID 20210924121352.42138-1-bagasdotme@gmail.com (mailing list archive)
State New, archived
Headers show
Series send-email: clarify dual-mode behavior | expand

Commit Message

Bagas Sanjaya Sept. 24, 2021, 12:13 p.m. UTC
git send-email can be operated in two modes: one that sends
already-prepared patches and one that generates patches from
revision range on-the-fly for sending. Clarify it in the documentation
and usage help.

Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
---
 This patch is based on [PATCH v5 2/3] send-email: programmatically
 generate bash completions [1]. PATCH v5 3/3 can be replaced with
 this patch, or be integrated as stand-alone patch.

 Questions:

   1. Do all supported revision range syntaxes from git rev-list also be
      accepted by git send-email? I only test `A..B` and `B ^A` syntaxes
      and assumed that all are supported.
   2. Does git send-email also accepts options understood by git
      rev-list? I interpreted that git send-email accepts file, directory
      or rev-list ranges but not rev-list options (I interpreted from
      previous synopsis before this patch but it said `rev-list options`).

 [1]:
https://lore.kernel.org/git/20210924024606.20542-3-tbperrotta@gmail.com/

 Documentation/git-send-email.txt | 14 ++++++++------
 git-send-email.perl              |  3 ++-
 2 files changed, 10 insertions(+), 7 deletions(-)

Comments

Junio C Hamano Sept. 24, 2021, 5:53 p.m. UTC | #1
Bagas Sanjaya <bagasdotme@gmail.com> writes:

> git send-email can be operated in two modes: one that sends
> already-prepared patches and one that generates patches from
> revision range on-the-fly for sending. Clarify it in the documentation
> and usage help.
>
> Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
> ---
>  This patch is based on [PATCH v5 2/3] send-email: programmatically
>  generate bash completions [1]. PATCH v5 3/3 can be replaced with
>  this patch, or be integrated as stand-alone patch.

Hmph.  I appreciate your enthusiasm, but I am not sure about this
change.

>
>  Questions:
>
>    1. Do all supported revision range syntaxes from git rev-list also be
>       accepted by git send-email? I only test `A..B` and `B ^A` syntaxes
>       and assumed that all are supported.

We do not have to ask that question if we said "format-patch
options" instead of ""revision range".

>    2. Does git send-email also accepts options understood by git
>       rev-list?

This becomes an irrelevant question if we used "format-patch
options" instead of "revision range".  Does git format-patch accept
options understood by git rev-list?  Very likely, given that it
shares the underlying option parser.  Do all options understood by
git rev-list make sense in that context?  Absolutely not.  What does
"git format-patch --left-right --boundary" even mean, for example?

But "git format-patch -U5 master" would make sense (show the commits
not yet in 'master' in patch form, but using 5-line context instead
of the usual 3).  So it is not "revision range", but more like "what
format-patch takes".

So from that point of view ...

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

... this is not improving Thiago's [3/3], I suspect.

Thanks.
Carlo Marcelo Arenas Belón Sept. 24, 2021, 6:55 p.m. UTC | #2
On Fri, Sep 24, 2021 at 10:53 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> > +'git send-email' [<options>] <file|directory>...
> > +'git send-email' [<options>] <revision range>
>
> ... this is not improving Thiago's [3/3], I suspect.

I think it does; after all, the original (which I know you proposed)
did confuse Bagas who thought it might be missing the tagging of being
optional.

The other reasons why this might be better are :
* because of the points and examples you provided, a vague [<options>]
is better than something that might imply all options taken from
format-patch make sense here, which IMHO might be difficult to
understand as logical to you as an experienced user.
* avoids "promoting" this mode, which is known to be problematic as it
could lead to patch bombs without the proper review.  it might even be
worth adding mentions to --compose or --annotate to avoid that.

Carlo
diff mbox series

Patch

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 3db4eab4ba..b1d239d74c 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -9,17 +9,19 @@  git-send-email - Send a collection of patches as emails
 SYNOPSIS
 --------
 [verse]
-'git send-email' [<options>] <file|directory|rev-list options>...
+'git send-email' [<options>] <file|directory>...
+'git send-email' [<options>] <revision range>
 'git send-email' --dump-aliases
 
 
 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.
+In the first form, take the patches in the <file> or <directory> and
+email them out. In the second form, generate patches from the specified
+<revision range> and email them out. <revision range> can be syntaxes
+that are accepted by linkgit:git-rev-list[1]. Options that are
+understood by linkgit:git-format-patch[1] can also be specified in
+<options> if the second form is used.
 
 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
diff --git a/git-send-email.perl b/git-send-email.perl
index 0214b55b4f..5d623f2fa8 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -39,7 +39,8 @@  package main;
 
 
 my $USAGE = <<EOT
-git send-email [options] <file | directory | rev-list options>
+git send-email [options] <file | directory>...
+git send-email [options] <revision range>
 git send-email --dump-aliases
 
   Composing: