Tom Russello <[email protected]> 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?
> @@ -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 (.
> + 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 =.
> @@ -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/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
