[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
