[RFC-PATCH v2 1/2] send-email: quote-email populates the fields
Take an email message file, parse it and fill the "To", "Cc" and "In-Reply-To" fields appropriately. If `--compose` option is set, it will also fill the subject field with "Re: ['s subject]". Signed-off-by: Tom Russello Signed-off-by: Samuel Groot Signed-off-by: Matthieu Moy --- As it is said in the cover letter, the file git-send-email.perl is being refactored therefore the parsing section with nested if's is ought to change. changes since v1: - option's name changed and is now --quote-email - original From: becomes To:, original To:'s become Cc: and original Cc:'s stay Cc: - coding style improved - documentation for the option - more tests Documentation/git-send-email.txt | 5 +++ git-send-email.perl | 87 +++- t/t9001-send-email.sh| 61 3 files changed, 152 insertions(+), 1 deletion(-) diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index 771a7b5..2334d69 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -106,6 +106,11 @@ illustration below where `[PATCH v2 0/3]` is in reply to `[PATCH 0/2]`: Only necessary if --compose is also set. If --compose is not set, this will be prompted for. +--quote-email=:: + Reply to the given email and automatically populate the "To:", "Cc:" and + "In-Reply-To:" fields. If `--compose` is set, this will also fill the + subject field with "Re: ['s subject]". + --subject=:: Specify the initial subject of the email thread. Only necessary if --compose is also set. If --compose diff --git a/git-send-email.perl b/git-send-email.perl index 6958785..9df3dee 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -55,6 +55,8 @@ git send-email --dump-aliases --[no-]bcc* Email Bcc: --subject * Email "Subject:" --in-reply-to * Email "In-Reply-To:" +--quote-email* Fill the fields "To:", "Cc:", "Subject:", + "In-Reply-To" appropriately. --[no-]xmailer * Add "X-Mailer:" header (default). --[no-]annotate* Review each patch that will be sent in an editor. --compose * Open an editor for introduction. @@ -160,7 +162,7 @@ my $re_encoded_word = qr/=\?($re_token)\?($re_token)\?($re_encoded_text)\?=/; # Variables we fill in automatically, or via prompting: my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@bcclist,$no_bcc,@xh, - $initial_reply_to,$initial_subject,@files, + $initial_reply_to,$quote_email,$initial_subject,@files, $author,$sender,$smtp_authpass,$annotate,$use_xmailer,$compose,$time); my $envelope_sender; @@ -304,6 +306,7 @@ $rc = GetOptions( "sender|from=s" => \$sender, "in-reply-to=s" => \$initial_reply_to, "subject=s" => \$initial_subject, + "quote-email=s" => \$quote_email, "to=s" => \@initial_to, "to-cmd=s" => \$to_cmd, "no-to" => \$no_to, @@ -639,6 +642,88 @@ if (@files) { usage(); } +if ($quote_email) { + my $error = validate_patch($quote_email); + $error and die "fatal: $quote_email: $error\nwarning: no patches were sent\n"; + + my @header = (); + + open my $fh, "<", $quote_email or die "can't open file $quote_email"; + + # Get the email header + while (<$fh>) { + # For files containing crlf line endings + s/\r//g; + last if /^\s*$/; + if (/^\s+\S/ and @header) { + chomp($header[$#header]); + s/^\s+/ /; + $header[$#header] .= $_; + } else { + push(@header, $_); + } + } + + # Parse the header + foreach (@header) { + my $input_format; + my $initial_sender = $sender || $repoauthor || $repocommitter || ''; + + if (/^From /) { + $input_format = 'mbox'; + next; + } + chomp; + if (!defined $input_format && /^[-A-Za-z]+:\s/) { + $input_format = 'mbox'; + } + + if (defined $input_format && $input_format eq 'mbox') { + if (/^Subject:\s+(.*)$/i) { + my $prefix_re = ""; + my $subject_re = $1; + if ($1 =~ /^[^Re:]/) { + $prefix_re = "Re: "; + } + $initial_subject = $prefix_re . $subject_re; + } elsif (/^From:\s+(.*)$/i) { +
Re: [RFC-PATCH v2 1/2] send-email: quote-email populates the fields
Tom Russello writes: > --- a/Documentation/git-send-email.txt > +++ b/Documentation/git-send-email.txt > @@ -106,6 +106,11 @@ illustration below where `[PATCH v2 0/3]` is in reply to > `[PATCH 0/2]`: > Only necessary if --compose is also set. If --compose > is not set, this will be prompted for. > > +--quote-email=:: > + Reply to the given email and automatically populate the "To:", "Cc:" and > + "In-Reply-To:" fields. I think this is a bit too technical for a user documentation. To: and Cc: is OK, but people need not know about "In-Reply-To:" to understand this. See what the doc of --in-reply-to says. If you want to be technical, you'd need to mention the References: field too. Talking about Reference: field, something your patch could do is to add all references in to the references of the new email (see what a mailer is doing when replying). This way, the recipient can still get threading if the last message being replied-to is missing. > +"Re: ['s subject]". Perhaps `Re: ...` instead of double-quotes. > +if ($quote_email) { > + my $error = validate_patch($quote_email); > + $error and die "fatal: $quote_email: $error\nwarning: no patches were > sent\n"; I know it's done this way elsewhere, but I don't like this "$error and die", I'd rather see a proper if here. > + if (defined $input_format && $input_format eq 'mbox') { To me, the input format refers to patch files, not the . I'm not sure anyone still use the "lots of email" format, and you are not testing it. So, this is claiming that we have a feature without being sure we have it nor that anyone's ever going to use it. I'd just drop this "if" and the "else" branch, and just assume the email file is a normal email file. -- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC-PATCH v2 1/2] send-email: quote-email populates the fields
On 05/28/16 16:35, Matthieu Moy wrote: >> +--quote-email=:: >> +Reply to the given email and automatically populate the "To:", "Cc:" and >> +"In-Reply-To:" fields. > > I think this is a bit too technical for a user documentation. To: and > Cc: is OK, but people need not know about "In-Reply-To:" to understand > this. See what the doc of --in-reply-to says. If you want to be > technical, you'd need to mention the References: field too. You have a point here. Maybe, we can explain that the `--quote-email` option behaves like a mailer when replying to someone without getting into details. > Talking about Reference: field, something your patch could do is to add > all references in to the references of the new email (see > what a mailer is doing when replying). This way, the recipient can still > get threading if the last message being replied-to is missing. I didn't know about this field, it looks like it appends all the parent message ID's. >> +"Re: ['s subject]". > > Perhaps `Re: ...` instead of double-quotes. Agreed. >> +if ($quote_email) { >> +my $error = validate_patch($quote_email); >> +$error and die "fatal: $quote_email: $error\nwarning: no patches were sent\n"; > > I know it's done this way elsewhere, but I don't like this "$error and > die", I'd rather see a proper if here. You're right, I'll change that in the next version. >> +if (defined $input_format && $input_format eq 'mbox') { > > To me, the input format refers to patch files, not the . > > I'm not sure anyone still use the "lots of email" format, and you are > not testing it. So, this is claiming that we have a feature without > being sure we have it nor that anyone's ever going to use it. You summed up the situation well. > I'd just drop this "if" and the "else" branch, and just assume the email > file is a normal email file. I'll do that. Thank you for the review. -- 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