diff mbox series

send-email: add --compose-cover option

Message ID 20231012112743.2756259-1-ben.dooks@codethink.co.uk (mailing list archive)
State New, archived
Headers show
Series send-email: add --compose-cover option | expand

Commit Message

Ben Dooks Oct. 12, 2023, 11:27 a.m. UTC
Adding an option to automatically compose a cover letter would be
helpful to put the whole process of sending an series with a cover
into one command.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
 Documentation/git-send-email.txt |  5 +++++
 git-send-email.perl              | 11 ++++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

Comments

Ben Dooks Oct. 12, 2023, 1:07 p.m. UTC | #1
On 12/10/2023 12:27, Ben Dooks wrote:
> Adding an option to automatically compose a cover letter would be
> helpful to put the whole process of sending an series with a cover
> into one command.
> 
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
>   Documentation/git-send-email.txt |  5 +++++
>   git-send-email.perl              | 11 ++++++++++-
>   2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 41cd8cb424..f299732867 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -78,6 +78,11 @@ Missing From or In-Reply-To headers will be prompted for.
>   +
>   See the CONFIGURATION section for `sendemail.multiEdit`.
>   
> +--compose-cover::
> +	Invoke a text editor (see GIT_EDITOR in linkgit:git-var[1])
> +	to edit a cover letter generated by passing --cover-letter to
> +	git-send-email before invoking the editor.
> +

OOPS, clearly meant git-format-patch here, not git-send-email.


>   --from=<address>::
>   	Specify the sender of the emails.  If not specified on the command line,
>   	the value of the `sendemail.from` configuration option is used.  If
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 5861e99a6e..debec088f6 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -54,6 +54,7 @@ sub usage {
>       --in-reply-to           <str>  * Email "In-Reply-To:"
>       --[no-]xmailer                 * Add "X-Mailer:" header (default).
>       --[no-]annotate                * Review each patch that will be sent in an editor.
> +    --compose-cover		   * Open an editor on format-patch --cover-letter
>       --compose                      * Open an editor for introduction.
>       --compose-encoding      <str>  * Encoding to assume for introduction.
>       --8bit-encoding         <str>  * Encoding to assume 8bit mails if undeclared
> @@ -199,7 +200,7 @@ sub format_2822_time {
>   # Variables we fill in automatically, or via prompting:
>   my (@to,@cc,@xh,$envelope_sender,
>   	$initial_in_reply_to,$reply_to,$initial_subject,@files,
> -	$author,$sender,$smtp_authpass,$annotate,$compose,$time);
> +	$author,$sender,$smtp_authpass,$annotate,$compose_cover,$compose,$time);
>   # Things we either get from config, *or* are overridden on the
>   # command-line.
>   my ($no_cc, $no_to, $no_bcc, $no_identity);
> @@ -512,6 +513,7 @@ sub config_regexp {
>   		    "no-smtp-auth" => sub {$smtp_auth = 'none'},
>   		    "annotate!" => \$annotate,
>   		    "no-annotate" => sub {$annotate = 0},
> +		    "compose-cover" => \$compose_cover,
>   		    "compose" => \$compose,
>   		    "quiet" => \$quiet,
>   		    "cc-cmd=s" => \$cc_cmd,
> @@ -782,6 +784,9 @@ sub is_format_patch_arg {
>   	die __("Cannot run git format-patch from outside a repository\n")
>   		unless $repo;
>   	require File::Temp;
> +	if ($compose_cover) {
> +	    push @rev_list_opts, "--cover-letter";
> +	}
>   	push @files, $repo->command('format-patch', '-o', File::Temp::tempdir(CLEANUP => 1), @rev_list_opts);
>   }
>   
> @@ -854,6 +859,8 @@ sub get_patch_subject {
>   
>   	if ($annotate) {
>   		do_edit($compose_filename, @files);
> +	} elsif ($compose_cover) {
> +		do_edit($files[0]);
>   	} else {
>   		do_edit($compose_filename);
>   	}
> @@ -927,6 +934,8 @@ sub get_patch_subject {
>   
>   } elsif ($annotate) {
>   	do_edit(@files);
> +} elsif ($compose_cover) {
> +	do_edit($files[0]);
>   }
>   
>   sub term {
Kristoffer Haugsbakk Oct. 12, 2023, 8:03 p.m. UTC | #2
Hi Ben

On Thu, Oct 12, 2023, at 13:27, Ben Dooks wrote:
> Adding an option to automatically compose a cover letter would be
> helpful to put the whole process of sending an series with a cover
> into one command.

I didn't manage to apply the patch so I haven't tried it out. Some
questions:

1. There is already `--compose` which is clearly different. What's the
   difference between a cover letter an an introduction? That's a more
   immediate question now with two similar-looking options.
2. This would be really useful in `format-patch`. One could write the
   cover letter immediately instead of either (1) editing the generated
   one (placeholders) or (2) providing a file.
   - Personally I don't use `send-email` immediately—I use `format-patch`
     to generate everything so that I can review it before any
     sending. But there are many different workflows.
3. Does this editor only deal with the subject and the message part before
   the diffstat and all the auto-generated stuff? And then it applies it
   after the user closes the editor (to make the complete cover letter)?
Junio C Hamano Oct. 12, 2023, 8:31 p.m. UTC | #3
"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:

> 2. This would be really useful in `format-patch`. One could write the
>    cover letter immediately instead of either (1) editing the generated
>    one (placeholders) or (2) providing a file.
>    - Personally I don't use `send-email` immediately—I use `format-patch`
>      to generate everything so that I can review it before any
>      sending. But there are many different workflows.

Hmph, I am not sure why a format-patch user would bother with the
"--compose-cover" option for the cover letter, to be honest.  I am a
format-patch user and never drive it from send-email, just like the
workflow you use, and the norm to me is to review everything, not
just the cover letter but also the patches, in my editor session.
You apparently do not need "--edit-patches" option to review and
adjust as needed before sending, even though that is what you do
already.  Why do you need a "--compose-cover" option?
Kristoffer Haugsbakk Oct. 12, 2023, 8:48 p.m. UTC | #4
On Thu, Oct 12, 2023, at 22:31, Junio C Hamano wrote:
> Hmph, I am not sure why a format-patch user would bother with the
> "--compose-cover" option for the cover letter, to be honest.  I am a
> format-patch user and never drive it from send-email, just like the
> workflow you use, and the norm to me is to review everything, not
> just the cover letter but also the patches, in my editor session.
> You apparently do not need "--edit-patches" option to review and
> adjust as needed before sending, even though that is what you do
> already.  Why do you need a "--compose-cover" option?

The thought was to get an interactive editor session for the subject and
the message and let `format-patch` fill in the range-diff etc., but on
second thought I have to do so many edits (fix mistakes and use the cover
letter in later versions) that an interactive session would probably not
be useful for me, after all. If so I would probably need to use some tool
that makes cover-letter a first-class entity that you can edit multiple
times similar to a commit message, like `git-series` (I'm guessing that's
how it works?).

The upcoming `--description-file`[1] is probably what I'm gonna end up
using.

[1] https://lore.kernel.org/git/20230821170720.577820-1-oswald.buddenhagen@gmx.de/
diff mbox series

Patch

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 41cd8cb424..f299732867 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -78,6 +78,11 @@  Missing From or In-Reply-To headers will be prompted for.
 +
 See the CONFIGURATION section for `sendemail.multiEdit`.
 
+--compose-cover::
+	Invoke a text editor (see GIT_EDITOR in linkgit:git-var[1])
+	to edit a cover letter generated by passing --cover-letter to
+	git-send-email before invoking the editor.
+
 --from=<address>::
 	Specify the sender of the emails.  If not specified on the command line,
 	the value of the `sendemail.from` configuration option is used.  If
diff --git a/git-send-email.perl b/git-send-email.perl
index 5861e99a6e..debec088f6 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -54,6 +54,7 @@  sub usage {
     --in-reply-to           <str>  * Email "In-Reply-To:"
     --[no-]xmailer                 * Add "X-Mailer:" header (default).
     --[no-]annotate                * Review each patch that will be sent in an editor.
+    --compose-cover		   * Open an editor on format-patch --cover-letter
     --compose                      * Open an editor for introduction.
     --compose-encoding      <str>  * Encoding to assume for introduction.
     --8bit-encoding         <str>  * Encoding to assume 8bit mails if undeclared
@@ -199,7 +200,7 @@  sub format_2822_time {
 # Variables we fill in automatically, or via prompting:
 my (@to,@cc,@xh,$envelope_sender,
 	$initial_in_reply_to,$reply_to,$initial_subject,@files,
-	$author,$sender,$smtp_authpass,$annotate,$compose,$time);
+	$author,$sender,$smtp_authpass,$annotate,$compose_cover,$compose,$time);
 # Things we either get from config, *or* are overridden on the
 # command-line.
 my ($no_cc, $no_to, $no_bcc, $no_identity);
@@ -512,6 +513,7 @@  sub config_regexp {
 		    "no-smtp-auth" => sub {$smtp_auth = 'none'},
 		    "annotate!" => \$annotate,
 		    "no-annotate" => sub {$annotate = 0},
+		    "compose-cover" => \$compose_cover,
 		    "compose" => \$compose,
 		    "quiet" => \$quiet,
 		    "cc-cmd=s" => \$cc_cmd,
@@ -782,6 +784,9 @@  sub is_format_patch_arg {
 	die __("Cannot run git format-patch from outside a repository\n")
 		unless $repo;
 	require File::Temp;
+	if ($compose_cover) {
+	    push @rev_list_opts, "--cover-letter";
+	}
 	push @files, $repo->command('format-patch', '-o', File::Temp::tempdir(CLEANUP => 1), @rev_list_opts);
 }
 
@@ -854,6 +859,8 @@  sub get_patch_subject {
 
 	if ($annotate) {
 		do_edit($compose_filename, @files);
+	} elsif ($compose_cover) {
+		do_edit($files[0]);
 	} else {
 		do_edit($compose_filename);
 	}
@@ -927,6 +934,8 @@  sub get_patch_subject {
 
 } elsif ($annotate) {
 	do_edit(@files);
+} elsif ($compose_cover) {
+	do_edit($files[0]);
 }
 
 sub term {