Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to
- From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
- Date
- May 23, 2016, 20:00 UTC
- Message-ID
- <vpq60u4bl4e.fsf@anie.imag.fr>
- In-Reply-To
- <1464031829-6107-2-git-send-email-tom.russello@grenoble-inp.org>
Tom Russello <tom.russello@grenoble-inp.org> writes:
> This option involves the `--compose` mode to edit the cover letter quoting the
s/involves/implies/
?
I don't think this is right: I often reply to an email with a single patch, for which it would clearly be overkill to have a cover-letter.
Your --quote-mail does two things:
1) Populate the To and Cc field
2) Include the original message body with quotation prefix.
When not using --compose, 1) clearly makes sense already, and there's no reason to prevent this use-case. When sending a single patch, 2) also makes sense as "below-tripple-dash comment", like
This is the commit message for feature A. --- John Smith wrote: > You should implement feature A.
Indeed, here's a patch.
modified-file.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
When sending multiple patches without --compose, 2) may not make sense, but I think a sane behavior would be:
* If --compose is given, cite the message there.
* If --compose is not given, don't send a cover-letter but cite the body as comment in the first patch.
As a first step, the second point can be changed to "if --compose is not given, don't cite the message, just populate the To: and Cc: fields".
> --- > > diff --git a/git-send-email.perl b/git-send-email.perl
No diffstat?
Show 6 quoted lines
> @@ -638,6 +640,98 @@ if (@files) {
> print STDERR "\nNo patch files specified!\n\n";
> usage();
> }
> +my $message_quoted;
> +if ($quote_mail) {Style: The code you're adding doesn't look related to the code right before => separate them with a blank line.
> + while(<$fh>) {Style: space before (.
> + push(@header, $_);
I think the code would be clearer if @header was a list of pairs (header-name, header-content). Then you'd need much less regex magic when going through it.
> + #for files containing crlf line endings
Sytle: space after #.
> + foreach(@header) {Space before (.
Show 9 quoted lines
> + elsif (/^From:\s+(.*)$/i) {
> + push @initial_to, $1;
> + }
> + elsif (/^To:\s+(.*)$/i) {
> + foreach my $addr (parse_address_line($1)) {
> + if (!($addr eq $initial_sender)) {
> + push @initial_to, $addr;
> + }
> + }This adds the content of the To: field in the original email to the Cc: field in the new message, right? If so, this is a weird behavior: when following up to an email, one usually addresses to the person s/he's replying, keeping the others Cc-ed, hence the original From: becomes the To header, and the original To: and Cc: become Cc:.
> + } elsif (/^Cc:\s+(.*)$/i) {Style: IIRC, there's no consensus on whether "elsif" should be on the same line as the closing }, but please follow the same convention inside a single if/elsif/ chain.
> + #Message body
Style: space after # (more below). And while you're there, the comment could be "Quote the message body" or so, to give a hint to the user about what's going on.
> + while (<$fh>) {
> + #for files containing crlf line endings
> + $_=~ s/\r//g;
> + my $space="";Style: spaces around =.
Show 7 quoted lines
> @@ -676,6 +771,8 @@ From: $tpl_sender > Subject: $tpl_subject > In-Reply-To: $tpl_reply_to > > +$tpl_quote > + > EOT
Doesn't this add two extra useless blank lines if $tpl_quote is empty?
-- Matthieu Moy http://www-verimag.imag.fr/~moy/