On 05/23/2016 10:00 PM, Matthieu Moy wrote:
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.

Yes indeed, we are working on inserting the quoted message body in the patch's "below-triple-dash" section.

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

It seems a good behavior to me too.

* If --compose is not given, don't send a cover-letter but cite the body
  as comment in the first patch.

Then should the option imply `--annotate`, to let the user edit the patch containing the quoted email?

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

The next series of patches may include (if the code is clean enough by then) a cleaner subroutine to parse the email, probably returning something like:

  return (
    "from" => $from,
    "subject" => $subject,
    "date" => $date,
    "message_id" => $message_id,
    "to" => [@to],
    "cc" => [@cc],
    "xh" => [@xh],
    "config" => {%config}
  );

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

We made the option behave like Thunderbird does, but indeed RFC 2822 [1] sees it the same you do, it will be fixed in next iteration.

@@ -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?

Yes it does, it will be fixed in the next series of patches.

Thank you for your time!


[1] https://www.ietf.org/rfc/rfc2822.txt
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to