{"thread":{"id":"47077","subject":"[PATCH 0/2] New send-email option --quote-email","startedAt":"2017-10-30T22:35:38Z","lastAt":"2017-11-09T08:50:03Z","messageCount":9,"participants":["Payre Nathan","Junio C Hamano","Matthieu Moy","Stefan Beller","Nathan PAYRE"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"331421","messageId":"20171030223444.5052-1-nathan.payre@etu.univ-lyon1.fr","threadId":"47077","inReplyTo":null,"subject":"[PATCH 0/2] New send-email option --quote-email","fromName":"Payre Nathan","fromEmail":"second.payre@gmail.com","sentAt":"2017-10-30T22:34:42Z","receivedAt":"2017-10-30T22:35:38Z","isPatch":true,"sender":{"key":"second.payre@gmail.com","avatar":null},"body":"Those patches implements a new --quote-email= option.\n\nTypical use case: the user receives a bug report by email and replies with a patch.\nBefore this patch, to make a proper reply, the user had to perform\nseveral steps manually using \"git send-email\":\n\n* Add --in-reply-to= to the command-line for proper\n threading.\n\n* Include the original recipients of the message using --to and --cc.\n\n* Copy and prefix the original message with '> ' in the \"below triple\n dash\" part of the patch.\n\nThis patch allows send-email to do most of the job for the user, who can\nnow save the email to a file and use:\n\n git send-email --quote-email=\n\n\"To\" and \"Cc\" will be added automaticaly and the email quoted.\nIt's possible to edit the email before sending with --compose.\n\nBased-on-patch-by: Tom Russello \nSigned-off: Nathan Payre \nSigned-off: Matthieu Moy \n\nTom Russello (2):\n quote-email populates the fields\n send-email: quote-email quotes the message body\n\n Documentation/git-send-email.txt | 5 ++\n git-send-email.perl | 146 +++++++++++++++++++++++++++++++++++++--\n t/t9001-send-email.sh | 134 ++++++++++++++++++++++++-----------\n 3 files changed, 240 insertions(+), 45 deletions(-)\n\n-- \n2.14.2\n\n"},{"id":"331422","messageId":"20171030223444.5052-2-nathan.payre@etu.univ-lyon1.fr","threadId":"47077","inReplyTo":"20171030223444.5052-1-nathan.payre@etu.univ-lyon1.fr","subject":"[PATCH 1/2] quote-email populates the fields","fromName":"Payre Nathan","fromEmail":"second.payre@gmail.com","sentAt":"2017-10-30T22:34:43Z","receivedAt":"2017-10-30T22:35:48Z","isPatch":true,"sender":{"key":"second.payre@gmail.com","avatar":null},"body":"From: Tom Russello \n\n---\n Documentation/git-send-email.txt | 3 +\n git-send-email.perl | 70 ++++++++++++++++++++++-\n t/t9001-send-email.sh | 117 +++++++++++++++++++++++++--------------\n 3 files changed, 147 insertions(+), 43 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex bac9014ac..710b5ff32 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -106,6 +106,9 @@ illustration below where `[PATCH v2 0/3]` is in reply to `[PATCH 0/2]`:\n Only necessary if --compose is also set. If --compose\n is not set, this will be prompted for.\n \n+--quote-email=::\n+\tFill appropriately header fields for the reply to the given email.\n+\n --subject=::\n \tSpecify the initial subject of the email thread.\n \tOnly necessary if --compose is also set. If --compose\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 2208dcc21..665c47d15 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -57,6 +57,7 @@ git send-email --dump-aliases\n --[no-]bcc * Email Bcc:\n --subject * Email \"Subject:\"\n --in-reply-to * Email \"In-Reply-To:\"\n+ --quote-email * Populate header fields appropriately.\n --[no-]xmailer * Add \"X-Mailer:\" header (default).\n --[no-]annotate * Review each patch that will be sent in an editor.\n --compose * Open an editor for introduction.\n@@ -166,7 +167,7 @@ my $re_encoded_word = qr/=\\?($re_token)\\?($re_token)\\?($re_encoded_text)\\?=/;\n \n # Variables we fill in automatically, or via prompting:\n my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@bcclist,$no_bcc,@xh,\n-\t$initial_reply_to,$initial_subject,@files,\n+\t$initial_reply_to,$initial_references,$quote_email,$initial_subject,@files,\n \t$author,$sender,$smtp_authpass,$annotate,$use_xmailer,$compose,$time);\n \n my $envelope_sender;\n@@ -316,6 +317,7 @@ $rc = GetOptions(\n \t\t \"sender|from=s\" => \\$sender,\n \"in-reply-to=s\" => \\$initial_reply_to,\n \t\t \"subject=s\" => \\$initial_subject,\n+\t\t \"quote-email=s\" => \\$quote_email,\n \t\t \"to=s\" => \\@initial_to,\n \t\t \"to-cmd=s\" => \\$to_cmd,\n \t\t \"no-to\" => \\$no_to,\n@@ -652,6 +654,70 @@ if (@files) {\n \tusage();\n }\n \n+if ($quote_email) {\n+\tmy $error = validate_patch($quote_email);\n+\tdie \"fatal: $quote_email: $error\\nwarning: no patches were sent\\n\"\n+\t\tif $error;\n+\n+\tmy @header = ();\n+\n+\topen my $fh, \"<\", $quote_email or die \"can't open file $quote_email\";\n+\n+\t# Get the email header\n+\twhile (<$fh>) {\n+\t\t# Turn crlf line endings into lf-only\n+\t\ts/\\r//g;\n+\t\tlast if /^\\s*$/;\n+\t\tif (/^\\s+\\S/ and @header) {\n+\t\t\tchomp($header[$#header]);\n+\t\t\ts/^\\s+/ /;\n+\t\t\t$header[$#header] .= $_;\n+\t\t} else {\n+\t\t\tpush(@header, $_);\n+\t\t}\n+\t}\n+\n+\t# Parse the header\n+\tforeach (@header) {\n+\t\tmy $initial_sender = $sender || $repoauthor || $repocommitter || '';\n+\n+\t\tchomp;\n+\n+\t\tif (/^Subject:\\s+(.*)$/i) {\n+\t\t\tmy $prefix_re = \"\";\n+\t\t\tmy $subject_re = $1;\n+\t\t\tif ($1 =~ /^[^Re:]/) {\n+\t\t\t\t$prefix_re = \"Re: \";\n+\t\t\t}\n+\t\t\t$initial_subject = $prefix_re . $subject_re;\n+\t\t} elsif (/^From:\\s+(.*)$/i) {\n+\t\t\tpush @initial_to, $1;\n+\t\t} elsif (/^To:\\s+(.*)$/i) {\n+\t\t\tforeach my $addr (parse_address_line($1)) {\n+\t\t\t\tif (!($addr eq $initial_sender)) {\n+\t\t\t\t\tpush @initial_cc, $addr;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t} elsif (/^Cc:\\s+(.*)$/i) {\n+\t\t\tforeach my $addr (parse_address_line($1)) {\n+\t\t\t\tmy $qaddr = unquote_rfc2047($addr);\n+\t\t\t\tmy $saddr = sanitize_address($qaddr);\n+\t\t\t\tif ($saddr eq $initial_sender) {\n+\t\t\t\t\tnext if ($suppress_cc{'self'});\n+\t\t\t\t} else {\n+\t\t\t\t\tnext if ($suppress_cc{'cc'});\n+\t\t\t\t}\n+\t\t\t\tpush @initial_cc, $addr;\n+\t\t\t}\n+\t\t} elsif (/^Message-Id: (.*)/i) {\n+\t\t\t$initial_reply_to = $1;\n+\t\t} elsif (/^References:\\s+(.*)/i) {\n+\t\t\t$initial_references = $1;\n+\t\t}\n+\t}\n+\t$initial_references = $initial_references . $initial_reply_to;\n+}\n+\n sub get_patch_subject {\n \tmy $fn = shift;\n \topen (my $fh, '<', $fn);\n@@ -1488,7 +1554,7 @@ EOF\n }\n \n $reply_to = $initial_reply_to;\n-$references = $initial_reply_to || '';\n+$references = $initial_references || $initial_reply_to || '';\n $subject = $initial_subject;\n $message_num = 0;\n \ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex f30980895..ce12a1164 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1917,52 +1917,87 @@ test_expect_success $PREREQ 'leading and trailing whitespaces are removed' '\n \ttest_cmp expected-list actual-list\n '\n \n-test_expect_success $PREREQ 'invoke hook' '\n-\tmkdir -p .git/hooks &&\n-\n-\twrite_script .git/hooks/sendemail-validate <<-\\EOF &&\n-\t# test that we have the correct environment variable, pwd, and\n-\t# argument\n-\tcase \"$GIT_DIR\" in\n-\t*.git)\n-\t\ttrue\n-\t\t;;\n-\t*)\n-\t\tfalse\n-\t\t;;\n-\tesac &&\n-\ttest -f 0001-add-master.patch &&\n-\tgrep \"add master\" \"$1\"\n-\tEOF\n-\n-\tmkdir subdir &&\n-\t(\n-\t\t# Test that it works even if we are not at the root of the\n-\t\t# working tree\n-\t\tcd subdir &&\n-\t\tgit send-email \\\n-\t\t\t--from=\"Example \" \\\n-\t\t\t--to=nobody@example.com \\\n-\t\t\t--smtp-server=\"$(pwd)/../fake.sendmail\" \\\n-\t\t\t../0001-add-master.patch &&\n+test_expect_success $PREREQ 'setup expect' '\n+\tcat >email <<-\\EOF\n+\tSubject: subject goes here\n+\tFrom: author@example.com\n+\tTo: to1@example.com\n+\tCc: cc1@example.com, cc2@example.com,\n+ cc3@example.com\n+\tDate: Sat, 12 Jun 2010 15:53:58 +0200\n+\tMessage-Id: \n+\tReferences: \n+ \n+ \n \n-\t\t# Verify error message when a patch is rejected by the hook\n-\t\tsed -e \"s/add master/x/\" ../0001-add-master.patch >../another.patch &&\n-\t\tgit send-email \\\n-\t\t\t--from=\"Example \" \\\n-\t\t\t--to=nobody@example.com \\\n-\t\t\t--smtp-server=\"$(pwd)/../fake.sendmail\" \\\n-\t\t\t../another.patch 2>err\n-\t\ttest_i18ngrep \"rejected by sendemail-validate hook\" err\n-\t)\n+\tHave you seen my previous email?\n+\t> Previous content\n+\tEOF\n '\n \n-test_expect_success $PREREQ 'test that send-email works outside a repo' '\n-\tnongit git send-email \\\n+test_expect_success $PREREQ 'Fields with --quote-email are correct' '\n+\tclean_fake_sendmail &&\n+\tgit send-email \\\n+\t\t--quote-email=email \\\n \t\t--from=\"Example \" \\\n-\t\t--to=nobody@example.com \\\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n-\t\t\"$(pwd)/0001-add-master.patch\"\n+\t\t-1 \\\n+\t\t2>errors &&\n+\tgrep \"From: Example \" msgtxt1 &&\n+\tgrep \"In-Reply-To: \" msgtxt1 &&\n+\tto_adr=$(awk \"/^To: /{flag=1}/^Cc: /{flag=0} flag {print}\" msgtxt1) &&\n+\tcc_adr=$(awk \"/^Cc: /{flag=1}/^Subject: /{flag=0} flag {print}\" msgtxt1) &&\n+\tref_adr=$(awk \"/^References: /{flag=1}/^MIME-Version: /{flag=0} flag {print}\" \\\n+\t\tmsgtxt1) &&\n+\techo \"$to_adr\" | grep author@example.com &&\n+\techo \"$cc_adr\" | grep to1@example.com &&\n+\techo \"$cc_adr\" | grep cc1@example.com &&\n+\techo \"$cc_adr\" | grep cc2@example.com &&\n+\techo \"$cc_adr\" | grep cc3@example.com &&\n+\techo \"$ref_adr\" | grep \"\" &&\n+\techo \"$ref_adr\" | grep \"\" &&\n+\techo \"$ref_adr\" | grep \"\" &&\n+\techo \"$ref_adr\" | grep \"\" &&\n+\techo \"$ref_adr\" | grep -v \"References: \"\n+'\n+\n+test_expect_success $PREREQ 'Fields with --quote-email and --compose are correct' '\n+\tclean_fake_sendmail &&\n+\tgit send-email \\\n+\t\t--quote-email=email \\\n+\t\t--compose \\\n+\t\t--from=\"Example \" \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t-1 \\\n+\t\t2>errors &&\n+\tgrep \"From: Example \" msgtxt1 &&\n+\tgrep \"In-Reply-To: \" msgtxt1 &&\n+\tgrep \"Subject: Re: subject goes here\" msgtxt1 &&\n+\tto_adr=$(awk \"/^To: /{flag=1}/^Cc: /{flag=0} flag {print}\" msgtxt1) &&\n+\tcc_adr=$(awk \"/^Cc: /{flag=1}/^Subject: /{flag=0} flag {print}\" msgtxt1) &&\n+\tref_adr=$(awk \"/^References: /{flag=1}/^MIME-Version: /{flag=0} flag {print}\" \\\n+\t\tmsgtxt1) &&\n+\techo \"$to_adr\" | grep author@example.com &&\n+\techo \"$cc_adr\" | grep to1@example.com &&\n+\techo \"$cc_adr\" | grep cc1@example.com &&\n+\techo \"$cc_adr\" | grep cc2@example.com &&\n+\techo \"$cc_adr\" | grep cc3@example.com &&\n+\techo \"$ref_adr\" | grep \"\" &&\n+\techo \"$ref_adr\" | grep \"\" &&\n+\techo \"$ref_adr\" | grep \"\" &&\n+\techo \"$ref_adr\" | grep \"\" &&\n+\techo \"$ref_adr\" | grep -v \"References: \"\n+'\n+\n+test_expect_success $PREREQ 'Re: written only once with --quote-email and --compose ' '\n+\tgit send-email \\\n+\t\t--quote-email=msgtxt1 \\\n+\t\t--compose \\\n+\t\t--from=\"Example \" \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t-1 \\\n+\t\t2>errors &&\n+\tgrep \"Subject: Re: subject goes here\" msgtxt3\n '\n \n test_done\n-- \n2.14.2\n\n"},{"id":"331423","messageId":"20171030223444.5052-3-nathan.payre@etu.univ-lyon1.fr","threadId":"47077","inReplyTo":"20171030223444.5052-1-nathan.payre@etu.univ-lyon1.fr","subject":"[PATCH 2/2] send-email: quote-email quotes the message body","fromName":"Payre Nathan","fromEmail":"second.payre@gmail.com","sentAt":"2017-10-30T22:34:44Z","receivedAt":"2017-10-30T22:35:52Z","isPatch":true,"sender":{"key":"second.payre@gmail.com","avatar":null},"body":"From: Tom Russello \n\n---\n Documentation/git-send-email.txt | 4 +-\n git-send-email.perl | 80 ++++++++++++++++++++++++++++++++++++++--\n t/t9001-send-email.sh | 19 +++++++++-\n 3 files changed, 97 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex 710b5ff32..329af66af 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -107,7 +107,9 @@ Only necessary if --compose is also set. If --compose\n is not set, this will be prompted for.\n \n --quote-email=::\n-\tFill appropriately header fields for the reply to the given email.\n+\tFill appropriately header fields for the reply to the given email and quote\n+\tthe message body in the cover letter if `--compose` is set or otherwise\n+\tafter the triple-dash in the first patch given.\n \n --subject=::\n \tSpecify the initial subject of the email thread.\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 665c47d15..6f6995c9d 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -26,6 +26,7 @@ use Text::ParseWords;\n use Term::ANSIColor;\n use File::Temp qw/ tempdir tempfile /;\n use File::Spec::Functions qw(catdir catfile);\n+use File::Copy;\n use Error qw(:try);\n use Cwd qw(abs_path cwd);\n use Git;\n@@ -57,7 +58,8 @@ git send-email --dump-aliases\n --[no-]bcc * Email Bcc:\n --subject * Email \"Subject:\"\n --in-reply-to * Email \"In-Reply-To:\"\n- --quote-email * Populate header fields appropriately.\n+ --quote-email * Populate header fields appropriately and\n+ quote the message body.\n --[no-]xmailer * Add \"X-Mailer:\" header (default).\n --[no-]annotate * Review each patch that will be sent in an editor.\n --compose * Open an editor for introduction.\n@@ -654,12 +656,15 @@ if (@files) {\n \tusage();\n }\n \n+my $message_quoted;\n if ($quote_email) {\n \tmy $error = validate_patch($quote_email);\n \tdie \"fatal: $quote_email: $error\\nwarning: no patches were sent\\n\"\n \t\tif $error;\n \n \tmy @header = ();\n+\tmy $date;\n+\tmy $recipient;\n \n \topen my $fh, \"<\", $quote_email or die \"can't open file $quote_email\";\n \n@@ -691,7 +696,8 @@ if ($quote_email) {\n \t\t\t}\n \t\t\t$initial_subject = $prefix_re . $subject_re;\n \t\t} elsif (/^From:\\s+(.*)$/i) {\n-\t\t\tpush @initial_to, $1;\n+\t\t\t$recipient = $1;\n+\t\t\tpush @initial_to, $recipient;\n \t\t} elsif (/^To:\\s+(.*)$/i) {\n \t\t\tforeach my $addr (parse_address_line($1)) {\n \t\t\t\tif (!($addr eq $initial_sender)) {\n@@ -713,9 +719,28 @@ if ($quote_email) {\n \t\t\t$initial_reply_to = $1;\n \t\t} elsif (/^References:\\s+(.*)/i) {\n \t\t\t$initial_references = $1;\n+\t\t} elsif (/^Date: (.*)/i) {\n+\t\t\t$date = $1;\n \t\t}\n \t}\n \t$initial_references = $initial_references . $initial_reply_to;\n+\n+\tmy $tpl_date = $date && \"On $date, \" || '';\n+\t$message_quoted = $tpl_date . $recipient . \" wrote:\\n\";\n+\n+\t# Quote the message body\n+\twhile (<$fh>) {\n+\t\t# Turn crlf line endings into lf-only\n+\t\ts/\\r//g;\n+\t\tmy $space = \"\";\n+\t\tif (/^[^>]/) {\n+\t\t\t$space = \" \";\n+\t\t}\n+\t\t$message_quoted .= \">\" . $space . $_;\n+\t}\n+\tif (!$compose) {\n+\t\t$annotate = 1;\n+\t}\n }\n \n sub get_patch_subject {\n@@ -743,6 +768,9 @@ if ($compose) {\n \tmy $tpl_sender = $sender || $repoauthor || $repocommitter || '';\n \tmy $tpl_subject = $initial_subject || '';\n \tmy $tpl_reply_to = $initial_reply_to || '';\n+\tmy $tpl_quote = $message_quoted &&\n+\t\t\"\\nGIT: Please, trim down irrelevant sections in the quoted message\\n\".\n+\t\t\"GIT: to keep your email concise.\\n\" . $message_quoted || '';\n \n \tprint $c < $repo->repo_path()) :\n+\t\t\ttempfile(\".gitsendemail.msg.XXXXXX\",\n+\t\t\t\tDIR => \".\"))[1];\n+\n+\t\t# Insertion in a temporary file to keep the original file clean\n+\t\t# in case of cancellation/error.\n+\t\tdo_insert_quoted_message($quote_email_filename, $files[0]);\n+\n+\t\tmy $tmp = $files[0];\n+\t\t$files[0] = $quote_email_filename;\n+\n+\t\tdo_edit(@files);\n+\n+\t\t# Erase the original patch if the edition went well\n+\t\tmove($quote_email_filename, $tmp);\n+\t\t$files[0] = $tmp;\n+\t} else {\n+\t\tdo_edit(@files);\n+\t}\n }\n \n+sub do_insert_quoted_message {\n+\tmy $tmp_file = shift;\n+\tmy $original_file = shift;\n+\n+\topen my $c, \"<\", $original_file\n+\tor die \"Failed to open $original_file: \" . $!;\n+\n+\topen my $c2, \">\", $tmp_file\n+\t\tor die \"Failed to open $tmp_file: \" . $!;\n+\n+\t# Insertion after the triple-dash\n+\twhile (<$c>) {\n+\t\tprint $c2 $_;\n+\t\tlast if (/^---$/);\n+\t}\n+\tprint $c2 $message_quoted;\n+\twhile (<$c>) {\n+\t\tprint $c2 $_;\n+\t}\n+\n+\tclose $c;\n+\tclose $c2;\n+}\n sub ask {\n \tmy ($prompt, %arg) = @_;\n \tmy $valid_re = $arg{valid_re};\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex ce12a1164..7c29c829d 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1941,7 +1941,7 @@ test_expect_success $PREREQ 'Fields with --quote-email are correct' '\n \t\t--quote-email=email \\\n \t\t--from=\"Example \" \\\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n-\t\t-1 \\\n+\t\t-2 \\\n \t\t2>errors &&\n \tgrep \"From: Example \" msgtxt1 &&\n \tgrep \"In-Reply-To: \" msgtxt1 &&\n@@ -1961,6 +1961,17 @@ test_expect_success $PREREQ 'Fields with --quote-email are correct' '\n \techo \"$ref_adr\" | grep -v \"References: \"\n '\n \n+test_expect_success $PREREQ 'correct quoted message with --quote-email' '\n+\tmsg_quoted=$(grep -A 3 \"^---$\" msgtxt1) &&\n+\techo \"$msg_quoted\" | grep \"On Sat, 12 Jun 2010 15:53:58 +0200, author@example.com wrote:\" &&\n+\techo \"$msg_quoted\" | grep \"> Have you seen my previous email?\" &&\n+\techo \"$msg_quoted\" | grep \">> Previous content\"\n+'\n+\n+test_expect_success $PREREQ 'second patch body is not modified by --quote-email' '\n+\t! grep \"Have you seen my previous email?\" msgtxt2\n+'\n+\n test_expect_success $PREREQ 'Fields with --quote-email and --compose are correct' '\n \tclean_fake_sendmail &&\n \tgit send-email \\\n@@ -2000,4 +2011,10 @@ test_expect_success $PREREQ 'Re: written only once with --quote-email and --comp\n \tgrep \"Subject: Re: subject goes here\" msgtxt3\n '\n \n+test_expect_success $PREREQ 'correct quoted message with --quote-email and --compose' '\n+\tgrep \"> On Sat, 12 Jun 2010 15:53:58 +0200, author@example.com wrote:\" msgtxt3 &&\n+\tgrep \">> Have you seen my previous email?\" msgtxt3 &&\n+\tgrep \">>> Previous content\" msgtxt3\n+'\n+\n test_done\n-- \n2.14.2\n\n"},{"id":"331533","messageId":"xmqqk1zawwd3.fsf@gitster.mtv.corp.google.com","threadId":"47077","inReplyTo":"20171030223444.5052-2-nathan.payre@etu.univ-lyon1.fr","subject":"Re: [PATCH 1/2] quote-email populates the fields","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-01T02:44:56Z","receivedAt":"2017-11-01T02:45:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Payre Nathan writes:\n\n> From: Tom Russello \n>\n> ---\n\nMissing something here???\n\n> Documentation/git-send-email.txt | 3 +\n> git-send-email.perl | 70 ++++++++++++++++++++++-\n> t/t9001-send-email.sh | 117 +++++++++++++++++++++++++--------------\n> 3 files changed, 147 insertions(+), 43 deletions(-)\n>\n> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\n> index bac9014ac..710b5ff32 100644\n> --- a/Documentation/git-send-email.txt\n> +++ b/Documentation/git-send-email.txt\n> @@ -106,6 +106,9 @@ illustration below where `[PATCH v2 0/3]` is in reply to `[PATCH 0/2]`:\n> Only necessary if --compose is also set. If --compose\n> is not set, this will be prompted for.\n> \n> +--quote-email=::\n> +\tFill appropriately header fields for the reply to the given email.\n> +\n\nThe cover letter said:\n\n This patch allows send-email to do most of the job for the user, who can\n now save the email to a file and use:\n\n git send-email --quote-email=\n\n \"To\" and \"Cc\" will be added automaticaly and the email quoted.\n It's possible to edit the email before sending with --compose.\n\nand I somehow expected to see the body of the e-mail this option is\n\"quoting\" to be also inserted in the text. After all, that is what\n\"quote\" means.\n\nBut the description above (and the code below, judging from the way\nthe reading from $fh that was opened form $quote_email stops at the\nfirst blank line, aka end of header) says what is happening is quite\ndifferent. The contents of the file is used to extract what the\nuser would have given to --cc/--to/--in-reply-to from the command\nline by looking at it, if this option were not available.\n\nI personally prefer the \"pick up the header information so that the\nuser do not have to formulate the command line options\" behaviour\nthat does *NOT* quote the body of the message into the outgoing\nmessage. So:\n\n * Do not call this option \"quote\" anything; you are not quoting,\n just using some info from the given file. \n\n I wonder if we can simply reuse \"--in-reply-to\" option for this\n purpose. If it is a message id and not a file on the filesystem,\n we behave just as before. Otherwise we try to open it as a file\n and grab the \"Message-ID:\" header from it and use it.\n\n * The description \"Fill *appropriately* header fileds\" is useless,\n as what looks \"appropriate\" to you is not clear/known to\n readers. Instead, say what header is filled with what\n information (e.g. \"find Message-Id: and place its value on\n In-Reply-To: header\").\n\n For that matter, \"To and CC will be added automatically\" in the\n coer letter is still vague; are you reading To/CC in the given\n file and placing their values on some (unnamed) header of the\n outgoing message? Or are you reading some (unnamed) header in\n the given file and placing their values on To/CC header of the\n outging message?\n\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 2208dcc21..665c47d15 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -57,6 +57,7 @@ git send-email --dump-aliases\n> --[no-]bcc * Email Bcc:\n> --subject * Email \"Subject:\"\n> --in-reply-to * Email \"In-Reply-To:\"\n> + --quote-email * Populate header fields appropriately.\n\nLikewise. If what's \"appropriate\" is clear to the readers, the word\nin this description adds no value because everybody would know how\nfields are populated. Otherwise, it does not add any value because\neverybody would have no clue how fields are populated.\n\n> @@ -652,6 +654,70 @@ if (@files) {\n> \tusage();\n> }\n> \n> +if ($quote_email) {\n> +\tmy $error = validate_patch($quote_email);\n> +\tdie \"fatal: $quote_email: $error\\nwarning: no patches were sent\\n\"\n> +\t\tif $error;\n\nvalidate_patch() calls sendemail-validate hook that is expecting to\nbe fed a patch email you are going to send out that might have\nerrors so that it can catch it and save you from embarrassment. The\nfile you are feeding it is *NOT* what you are going to send out, but\nis what you are responding to with your patch. Even if it had an\nembarassing error as a patch, that is not something you care about\n(and it is something you received, so catching this late won't save\nthe sender from embarrassment anyway).\n\n> +\n> +\tmy @header = ();\n> +\n> +\topen my $fh, \"<\", $quote_email or die \"can't open file $quote_email\";\n> +\n> +\t# Get the email header\n> +\twhile (<$fh>) {\n> +\t\t# Turn crlf line endings into lf-only\n> +\t\ts/\\r//g;\n> +\t\tlast if /^\\s*$/;\n> +\t\tif (/^\\s+\\S/ and @header) {\n\nI wonder how significant this requirement to have at least one \"\\S\"\non the line is. I know you copied&pasted this from the main sending\nloop, so this is not a new issue and not something we may want to\nfix in this patch.\n\n> +\t\t\tchomp($header[$#header]);\n> +\t\t\ts/^\\s+/ /;\n> +\t\t\t$header[$#header] .= $_;\n> +\t\t} else {\n> +\t\t\tpush(@header, $_);\n> +\t\t}\n> +\t}\n\nYou do not use $fh after this point. Do not force readers to\nrealize that fact by scanning to the end of the function--instead,\nclose it here.\n\n> +\t# Parse the header\n> +\tforeach (@header) {\n> +\t\tmy $initial_sender = $sender || $repoauthor || $repocommitter || '';\n> +\n> +\t\tchomp;\n> +\n> +\t\tif (/^Subject:\\s+(.*)$/i) {\n> +\t\t\tmy $prefix_re = \"\";\n> +\t\t\tmy $subject_re = $1;\n\nWhat does \"_re\" mean in the variable name $subject_re?\n\n> +\t\t\tif ($1 =~ /^[^Re:]/) {\n> +\t\t\t\t$prefix_re = \"Re: \";\n> +\t\t\t}\n> +\t\t\t$initial_subject = $prefix_re . $subject_re;\n> +\t\t} elsif (/^From:\\s+(.*)$/i) {\n> +\t\t\tpush @initial_to, $1;\n> +\t\t} elsif (/^To:\\s+(.*)$/i) {\n> +\t\t\tforeach my $addr (parse_address_line($1)) {\n> +\t\t\t\tif (!($addr eq $initial_sender)) {\n\nThis if() condition makes a policy decision; shouldn't it honor the\nsetting of \"--[no-]suppress-from\", \"--suppress-cc\" and friends?\n\n> +\t\t\t\t\tpush @initial_cc, $addr;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\t\t} elsif (/^Cc:\\s+(.*)$/i) {\n> +\t\t\tforeach my $addr (parse_address_line($1)) {\n> +\t\t\t\tmy $qaddr = unquote_rfc2047($addr);\n> +\t\t\t\tmy $saddr = sanitize_address($qaddr);\n> +\t\t\t\tif ($saddr eq $initial_sender) {\n> +\t\t\t\t\tnext if ($suppress_cc{'self'});\n> +\t\t\t\t} else {\n> +\t\t\t\t\tnext if ($suppress_cc{'cc'});\n> +\t\t\t\t}\n> +\t\t\t\tpush @initial_cc, $addr;\n> +\t\t\t}\n> +\t\t} elsif (/^Message-Id: (.*)/i) {\n> +\t\t\t$initial_reply_to = $1;\n> +\t\t} elsif (/^References:\\s+(.*)/i) {\n> +\t\t\t$initial_references = $1;\n> +\t\t}\n> +\t}\n> +\t$initial_references = $initial_references . $initial_reply_to;\n\nI cannot see how this can produce correct result by simply\nconcatenating them with nothing in between. Shouldn't you make sure\nthere is a SP in between, at least?\n\nBy the way, if you are adding a new variable $initial_references,\nmake sure it is initialized to either an empty string or an undef\n(and if you choose to do the latter, the right hand side of this\nassignment cannot blindly reference $initial_references that could\nstill be undef); the way the existing code handles $initial_reply_to\nmay serve as an example.\n"},{"id":"331548","messageId":"xmqqa806tscf.fsf@gitster.mtv.corp.google.com","threadId":"47077","inReplyTo":"20171030223444.5052-3-nathan.payre@etu.univ-lyon1.fr","subject":"Re: [PATCH 2/2] send-email: quote-email quotes the message body","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-01T06:40:00Z","receivedAt":"2017-11-01T06:40:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Payre Nathan writes:\n\n> From: Tom Russello \n>\n> ---\n> Documentation/git-send-email.txt | 4 +-\n> git-send-email.perl | 80 ++++++++++++++++++++++++++++++++++++++--\n> t/t9001-send-email.sh | 19 +++++++++-\n> 3 files changed, 97 insertions(+), 6 deletions(-)\n>\n> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\n> index 710b5ff32..329af66af 100644\n> --- a/Documentation/git-send-email.txt\n> +++ b/Documentation/git-send-email.txt\n> @@ -107,7 +107,9 @@ Only necessary if --compose is also set. If --compose\n> is not set, this will be prompted for.\n> \n> --quote-email=::\n> -\tFill appropriately header fields for the reply to the given email.\n> +\tFill appropriately header fields for the reply to the given email and quote\n> +\tthe message body in the cover letter if `--compose` is set or otherwise\n> +\tafter the triple-dash in the first patch given.\n\nHmmm. I have a strong suspicion that people want an option to\ntrigger the feature from just 1/2 but not 2/2 some of the time.\nSure, removing the unwanted lines in the compose editor may be easy,\nbut it feels wasteful use of user's time to include the lines of\ntext from the original only to have them removed.\n\nAlso, if you are not offering these two as separate features, there\nisn't much point splitting them into two patches.\n\n> @@ -743,6 +768,9 @@ if ($compose) {\n> \tmy $tpl_sender = $sender || $repoauthor || $repocommitter || '';\n> \tmy $tpl_subject = $initial_subject || '';\n> \tmy $tpl_reply_to = $initial_reply_to || '';\n> +\tmy $tpl_quote = $message_quoted &&\n> +\t\t\"\\nGIT: Please, trim down irrelevant sections in the quoted message\\n\".\n> +\t\t\"GIT: to keep your email concise.\\n\" . $message_quoted || '';\n> \n> \tprint $c < From $tpl_sender # This line is ignored.\n> @@ -756,7 +784,7 @@ EOT2\n> From: $tpl_sender\n> Subject: $tpl_subject\n> In-Reply-To: $tpl_reply_to\n> -\n> +$tpl_quote\n> EOT3\n\nOK, by emitting it into $compose_filename as part of the front\nmatter, you get the \"do we have to do mime?\" etc. for free, which\nsort-of makes sense.\n\n> @@ -821,9 +849,53 @@ EOT3\n> \t\t$compose = -1;\n> \t}\n> } elsif ($annotate) {\n> -\tdo_edit(@files);\n> +\tif ($quote_email) {\n> +\t\tmy $quote_email_filename = ($repo ?\n> +\t\t\ttempfile(\".gitsendemail.msg.XXXXXX\",\n> +\t\t\t\tDIR => $repo->repo_path()) :\n> +\t\t\ttempfile(\".gitsendemail.msg.XXXXXX\",\n> +\t\t\t\tDIR => \".\"))[1];\n> +\n> +\t\t# Insertion in a temporary file to keep the original file clean\n> +\t\t# in case of cancellation/error.\n> +\t\tdo_insert_quoted_message($quote_email_filename, $files[0]);\n> +\n> +\t\tmy $tmp = $files[0];\n> +\t\t$files[0] = $quote_email_filename;\n> +\n> +\t\tdo_edit(@files);\n> +\n> +\t\t# Erase the original patch if the edition went well\n> +\t\tmove($quote_email_filename, $tmp);\n> +\t\t$files[0] = $tmp;\n> +\t} else {\n> +\t\tdo_edit(@files);\n> +\t}\n> }\n"},{"id":"331557","messageId":"q7h97evaxntq.fsf@orange.lip.ens-lyon.fr","threadId":"47077","inReplyTo":"607ed87207454d1098484b0ffbc6916f@BPMBX2013-01.univ-lyon1.fr","subject":"Re: [PATCH 1/2] quote-email populates the fields","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@univ-lyon1.fr","sentAt":"2017-11-01T11:04:01Z","receivedAt":"2017-11-01T11:04:16Z","isPatch":true,"sender":{"key":"matthieu.moy@univ-lyon1.fr","avatar":"https://gravatar.com/avatar/8ab83b763226bd297b59ddd1463a8bfc852924577191233f73a953b86888fe0c?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Payre Nathan writes:\n>\n>> From: Tom Russello \n>>\n>> ---\n>\n> Missing something here???\n\nTo clarify for Nathan, Thimothee and Danial: the cover-letter is an\nintroduction send before the patch series. It can be needed to explain\nthe overall approach followed by the series. But in general, it does not\nend up in the Git history, i.e. after the review is finished, the\ncover-letter is forgotten.\n\nOTOH, the commit messages for each patch is what ends up in the Git\nhistory. This is what people will find later when running e.g. \"git\nblame\", \"git bisect\" or so. Clearly the future user examining history\nexpects more than \"quote-email populates the fields\" (which was a good\nreminder during development, but is actually a terrible subject line for\na final version).\n\nA quick advice: if in doubt, prefer writing explanations in commit\nmessage rather than the cover letter. If still in doubt, write the\nexplanations twice: once quickly in the cover letter and once more\ndetailed in the commit message.\n\n> * Do not call this option \"quote\" anything; you are not quoting,\n> just using some info from the given file. \n>\n> I wonder if we can simply reuse \"--in-reply-to\" option for this\n> purpose. If it is a message id and not a file on the filesystem,\n> we behave just as before. Otherwise we try to open it as a file\n> and grab the \"Message-ID:\" header from it and use it.\n\nThere's a possible ambiguity since user may in theory want to run\n\"--in-reply-to=msgid\" with a file named msgid and still want the old\nbehavior. But: a real message-id is typically something rather cryptic\nand it is safe to assume that users won't have a file named exactly like\nan actual message id containing something which isn't the message in\nquestion.\n\nThe main drawback I see in re-using \"--in-reply-to\" is that typos are\nhard to miss. For example, running\n\n git send-email --in-reply-to=msgi\n\nwhen the user actually wanted msgid would trigger a different behavior\ninstead of raising an error (no such file or directory). I think it's\nacceptable: in the current form of send-email, we're already not\nuser-friendly to users when they write a typo in the --in-reply-to\nargument (and we can't really detect typos anyway).\n\n>> +if ($quote_email) {\n>> +\tmy $error = validate_patch($quote_email);\n>> +\tdie \"fatal: $quote_email: $error\\nwarning: no patches were sent\\n\"\n>> +\t\tif $error;\n>\n> validate_patch() calls sendemail-validate hook that is expecting to\n> be fed a patch email you are going to send out that might have\n> errors so that it can catch it and save you from embarrassment. The\n> file you are feeding it is *NOT* what you are going to send out, but\n> is what you are responding to with your patch. Even if it had an\n> embarassing error as a patch, that is not something you care about\n> (and it is something you received, so catching this late won't save\n> the sender from embarrassment anyway).\n\nI think the intention was to detect cases when $quote_email is not a\npatch at all (and give a proper error message instead of trying to\ncontinue with probably absurd behavior).\n\nBut I agree that there's no point in being too strict here, and if that\nwas the intension then it should be documented with a comment.\n\n-- \nMatthieu Moy\nhttps://matthieu-moy.fr/\n"},{"id":"331559","messageId":"q7h9shdyw8vu.fsf@orange.lip.ens-lyon.fr","threadId":"47077","inReplyTo":"0db6387ef95b4fafbd70068be9e4f7c5@BPMBX2013-01.univ-lyon1.fr","subject":"Re: [PATCH 2/2] send-email: quote-email quotes the message body","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@univ-lyon1.fr","sentAt":"2017-11-01T11:12:05Z","receivedAt":"2017-11-01T11:12:11Z","isPatch":true,"sender":{"key":"matthieu.moy@univ-lyon1.fr","avatar":"https://gravatar.com/avatar/8ab83b763226bd297b59ddd1463a8bfc852924577191233f73a953b86888fe0c?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Hmmm. I have a strong suspicion that people want an option to\n> trigger the feature from just 1/2 but not 2/2 some of the time.\n> Sure, removing the unwanted lines in the compose editor may be easy,\n> but it feels wasteful use of user's time to include the lines of\n> text from the original only to have them removed.\n\nSo, that could be\n\n git send-email --in-reply-to=message-id # message-id is not a file\n => existing behavior\n\n git send-email --in-reply-to=file\n => populate To:, Cc:, In-Reply-To: and References:\n\n git send-email --in-reply-to=file --quote\n => in addition to the above, include the quoted message in the body\n\n(perhaps --quote should be --cite, I'm not sure which one looks best for\na native speaker)\n\nThis also leaves room for\n\n git send-email --in-reply-to=message-id --fetch [--quote]\n => download the message body from e.g. public-inbox and do the same as\n for --in-reply-to=file\n\n(which doesn't have to be implemented now, but would be a nice-to-have\nin the future)\n\n-- \nMatthieu Moy\nhttps://matthieu-moy.fr/\n"},{"id":"331587","messageId":"CAGZ79kbCkj+=tUPL6W48cedSzETsydpHSMkZX=Xn7XnVijpjZQ@mail.gmail.com","threadId":"47077","inReplyTo":"q7h97evaxntq.fsf@orange.lip.ens-lyon.fr","subject":"Re: [PATCH 1/2] quote-email populates the fields","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-01T18:12:04Z","receivedAt":"2017-11-01T18:12:10Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Nov 1, 2017 at 4:04 AM, Matthieu Moy wrote:\n> Junio C Hamano writes:\n>\n>> Payre Nathan writes:\n>>\n>>> From: Tom Russello \n>>>\n>>> ---\n>>\n>> Missing something here???\n>\n> To clarify for Nathan, Thimothee and Danial: the cover-letter is an\n> introduction send before the patch series. It can be needed to explain\n> the overall approach followed by the series. But in general, it does not\n> end up in the Git history, i.e. after the review is finished, the\n> cover-letter is forgotten.\n>\n> OTOH, the commit messages for each patch is what ends up in the Git\n> history. This is what people will find later when running e.g. \"git\n> blame\", \"git bisect\" or so. Clearly the future user examining history\n> expects more than \"quote-email populates the fields\" (which was a good\n> reminder during development, but is actually a terrible subject line for\n> a final version).\n>\n> A quick advice: if in doubt, prefer writing explanations in commit\n> message rather than the cover letter. If still in doubt, write the\n> explanations twice: once quickly in the cover letter and once more\n> detailed in the commit message.\n\nOh, and I thought the sign offs.\n"},{"id":"332111","messageId":"CAGb4CBVjjh7VgY0OQJJOjU9Q2+eCH9Z3wkLBV_JhcaSpCHpLag@mail.gmail.com","threadId":"47077","inReplyTo":"xmqqk1zawwd3.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] quote-email populates the fields","fromName":"Nathan PAYRE","fromEmail":"second.payre@gmail.com","sentAt":"2017-11-09T08:49:55Z","receivedAt":"2017-11-09T08:50:03Z","isPatch":true,"sender":{"key":"second.payre@gmail.com","avatar":null},"body":"I Will send the modification in the next patch, I prefer to refractor\na part of the code before.\n\n>> diff --git a/git-send-email.perl b/git-send-email.perl\n>> index 2208dcc21..665c47d15 100755\n>> --- a/git-send-email.perl\n>> +++ b/git-send-email.perl\n>> @@ -57,6 +57,7 @@ git send-email --dump-aliases\n>> --[no-]bcc * Email Bcc:\n>> --subject * Email \"Subject:\"\n>> --in-reply-to * Email \"In-Reply-To:\"\n>> + --quote-email * Populate header fields appropriately.\n\n> Likewise. If what's \"appropriate\" is clear to the readers, the word\n> in this description adds no value because everybody would know how\n> fields are populated. Otherwise, it does not add any value because\n> everybody would have no clue how fields are populated.\n\nRemove \"approprietly\" done.\n\n\n>> @@ -652,6 +654,70 @@ if (@files) {\n>> usage();\n>> }\n>>\n>> +if ($quote_email) {\n>> + my $error = validate_patch($quote_email);\n>> + die \"fatal: $quote_email: $error\\nwarning: no patches were sent\\n\"\n>> + if $error;\n\n> validate_patch() calls sendemail-validate hook that is expecting to\n> be fed a patch email you are going to send out that might have\n> errors so that it can catch it and save you from embarrassment. The\n> file you are feeding it is *NOT* what you are going to send out, but\n> is what you are responding to with your patch. Even if it had an\n> embarassing error as a patch, that is not something you care about\n> (and it is something you received, so catching this late won't save\n> the sender from embarrassment anyway).\n\nI will remove lines which use validate_patch().\n\n\n>> + chomp($header[$#header]);\n>> + s/^\\s+/ /;\n>> + $header[$#header] .= $_;\n>> + } else {\n>> + push(@header, $_);\n>> + }\n>> + }\n\n> You do not use $fh after this point. Do not force readers to\n> realize that fact by scanning to the end of the function--instead,\n> close it here.\n\nIn fact $fh is reuse at the end of the if($quote_email) {} but if you\ndon't see it maybe it's because it's anormal to reuse it after a\nlong block of code, that's why I think to create a subroutine\nfor the following code which is similar to the part of if($compose).\n\nforeach (@header) {\n my $initial_sender = $sender || $repoauthor || $repocommitter || '';\n\n chomp;\n\n if (/^Subject:\\s+(.*)$/i) {\n my $prefix_re = \"\";\n my $subject_re = $1;\n if ($1 =~ /^[^Re:]/) {\n $prefix_re = \"Re: \";\n }\n $initial_subject = $prefix_re . $subject_re;\n } elsif (/^From:\\s+(.*)$/i) {\n $recipient = $1;\n push @initial_to, $recipient;\n } elsif (/^To:\\s+(.*)$/i) {\n foreach my $addr (parse_address_line($1)) {\n if (!($addr eq $initial_sender)) {\n push @initial_cc, $addr;\n }\n }\n } elsif (/^Cc:\\s+(.*)$/i) {\n foreach my $addr (parse_address_line($1)) {\n my $qaddr = unquote_rfc2047($addr);\n my $saddr = sanitize_address($qaddr);\n if ($saddr eq $initial_sender) {\n next if ($suppress_cc{'self'});\n } else {\n next if ($suppress_cc{'cc'});\n }\n push @initial_cc, $addr;\n }\n } elsif (/^Message-Id: (.*)/i) {\n $initial_reply_to = $1;\n } elsif (/^References:\\s+(.*)/i) {\n $initial_references = $1;\n } elsif (/^Date: (.*)/i) {\n $date = $1;\n }\n}\n\n\nI close $fh after the second call then.\n\n\n>> + # Parse the header\n>> + foreach (@header) {\n>> + my $initial_sender = $sender || $repoauthor || $repocommitter || '';\n>> +\n>> + chomp;\n>> +\n>> + if (/^Subject:\\s+(.*)$/i) {\n>> + my $prefix_re = \"\";\n>> + my $subject_re = $1;\n\n> What does \"_re\" mean in the variable name $subject_re?\n\n\"_re\" mean regular expression but maybe it's clumsy because\nit contain the result of a regular expression. What do you think\nabout rename it into \"$prefix\" and \"$subject\" ?\n\n\n2017-11-01 3:44 GMT+01:00 Junio C Hamano :\n> Payre Nathan writes:\n>\n>> From: Tom Russello \n>>\n>> ---\n>\n> Missing something here???\n>\n>> Documentation/git-send-email.txt | 3 +\n>> git-send-email.perl | 70 ++++++++++++++++++++++-\n>> t/t9001-send-email.sh | 117 +++++++++++++++++++++++++--------------\n>> 3 files changed, 147 insertions(+), 43 deletions(-)\n>>\n>> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\n>> index bac9014ac..710b5ff32 100644\n>> --- a/Documentation/git-send-email.txt\n>> +++ b/Documentation/git-send-email.txt\n>> @@ -106,6 +106,9 @@ illustration below where `[PATCH v2 0/3]` is in reply to `[PATCH 0/2]`:\n>> Only necessary if --compose is also set. If --compose\n>> is not set, this will be prompted for.\n>>\n>> +--quote-email=::\n>> + Fill appropriately header fields for the reply to the given email.\n>> +\n>\n> The cover letter said:\n>\n> This patch allows send-email to do most of the job for the user, who can\n> now save the email to a file and use:\n>\n> git send-email --quote-email=\n>\n> \"To\" and \"Cc\" will be added automaticaly and the email quoted.\n> It's possible to edit the email before sending with --compose.\n>\n> and I somehow expected to see the body of the e-mail this option is\n> \"quoting\" to be also inserted in the text. After all, that is what\n> \"quote\" means.\n>\n> But the description above (and the code below, judging from the way\n> the reading from $fh that was opened form $quote_email stops at the\n> first blank line, aka end of header) says what is happening is quite\n> different. The contents of the file is used to extract what the\n> user would have given to --cc/--to/--in-reply-to from the command\n> line by looking at it, if this option were not available.\n>\n> I personally prefer the \"pick up the header information so that the\n> user do not have to formulate the command line options\" behaviour\n> that does *NOT* quote the body of the message into the outgoing\n> message. So:\n>\n> * Do not call this option \"quote\" anything; you are not quoting,\n> just using some info from the given file.\n>\n> I wonder if we can simply reuse \"--in-reply-to\" option for this\n> purpose. If it is a message id and not a file on the filesystem,\n> we behave just as before. Otherwise we try to open it as a file\n> and grab the \"Message-ID:\" header from it and use it.\n>\n> * The description \"Fill *appropriately* header fileds\" is useless,\n> as what looks \"appropriate\" to you is not clear/known to\n> readers. Instead, say what header is filled with what\n> information (e.g. \"find Message-Id: and place its value on\n> In-Reply-To: header\").\n>\n> For that matter, \"To and CC will be added automatically\" in the\n> coer letter is still vague; are you reading To/CC in the given\n> file and placing their values on some (unnamed) header of the\n> outgoing message? Or are you reading some (unnamed) header in\n> the given file and placing their values on To/CC header of the\n> outging message?\n>\n>> diff --git a/git-send-email.perl b/git-send-email.perl\n>> index 2208dcc21..665c47d15 100755\n>> --- a/git-send-email.perl\n>> +++ b/git-send-email.perl\n>> @@ -57,6 +57,7 @@ git send-email --dump-aliases\n>> --[no-]bcc * Email Bcc:\n>> --subject * Email \"Subject:\"\n>> --in-reply-to * Email \"In-Reply-To:\"\n>> + --quote-email * Populate header fields appropriately.\n>\n> Likewise. If what's \"appropriate\" is clear to the readers, the word\n> in this description adds no value because everybody would know how\n> fields are populated. Otherwise, it does not add any value because\n> everybody would have no clue how fields are populated.\n>\n>> @@ -652,6 +654,70 @@ if (@files) {\n>> usage();\n>> }\n>>\n>> +if ($quote_email) {\n>> + my $error = validate_patch($quote_email);\n>> + die \"fatal: $quote_email: $error\\nwarning: no patches were sent\\n\"\n>> + if $error;\n>\n> validate_patch() calls sendemail-validate hook that is expecting to\n> be fed a patch email you are going to send out that might have\n> errors so that it can catch it and save you from embarrassment. The\n> file you are feeding it is *NOT* what you are going to send out, but\n> is what you are responding to with your patch. Even if it had an\n> embarassing error as a patch, that is not something you care about\n> (and it is something you received, so catching this late won't save\n> the sender from embarrassment anyway).\n>\n>> +\n>> + my @header = ();\n>> +\n>> + open my $fh, \"<\", $quote_email or die \"can't open file $quote_email\";\n>> +\n>> + # Get the email header\n>> + while (<$fh>) {\n>> + # Turn crlf line endings into lf-only\n>> + s/\\r//g;\n>> + last if /^\\s*$/;\n>> + if (/^\\s+\\S/ and @header) {\n>\n> I wonder how significant this requirement to have at least one \"\\S\"\n> on the line is. I know you copied&pasted this from the main sending\n> loop, so this is not a new issue and not something we may want to\n> fix in this patch.\n>\n>> + chomp($header[$#header]);\n>> + s/^\\s+/ /;\n>> + $header[$#header] .= $_;\n>> + } else {\n>> + push(@header, $_);\n>> + }\n>> + }\n>\n> You do not use $fh after this point. Do not force readers to\n> realize that fact by scanning to the end of the function--instead,\n> close it here.\n>\n>> + # Parse the header\n>> + foreach (@header) {\n>> + my $initial_sender = $sender || $repoauthor || $repocommitter || '';\n>> +\n>> + chomp;\n>> +\n>> + if (/^Subject:\\s+(.*)$/i) {\n>> + my $prefix_re = \"\";\n>> + my $subject_re = $1;\n>\n> What does \"_re\" mean in the variable name $subject_re?\n>\n>> + if ($1 =~ /^[^Re:]/) {\n>> + $prefix_re = \"Re: \";\n>> + }\n>> + $initial_subject = $prefix_re . $subject_re;\n>> + } elsif (/^From:\\s+(.*)$/i) {\n>> + push @initial_to, $1;\n>> + } elsif (/^To:\\s+(.*)$/i) {\n>> + foreach my $addr (parse_address_line($1)) {\n>> + if (!($addr eq $initial_sender)) {\n>\n> This if() condition makes a policy decision; shouldn't it honor the\n> setting of \"--[no-]suppress-from\", \"--suppress-cc\" and friends?\n>\n>> + push @initial_cc, $addr;\n>> + }\n>> + }\n>> + } elsif (/^Cc:\\s+(.*)$/i) {\n>> + foreach my $addr (parse_address_line($1)) {\n>> + my $qaddr = unquote_rfc2047($addr);\n>> + my $saddr = sanitize_address($qaddr);\n>> + if ($saddr eq $initial_sender) {\n>> + next if ($suppress_cc{'self'});\n>> + } else {\n>> + next if ($suppress_cc{'cc'});\n>> + }\n>> + push @initial_cc, $addr;\n>> + }\n>> + } elsif (/^Message-Id: (.*)/i) {\n>> + $initial_reply_to = $1;\n>> + } elsif (/^References:\\s+(.*)/i) {\n>> + $initial_references = $1;\n>> + }\n>> + }\n>> + $initial_references = $initial_references . $initial_reply_to;\n>\n> I cannot see how this can produce correct result by simply\n> concatenating them with nothing in between. Shouldn't you make sure\n> there is a SP in between, at least?\n>\n> By the way, if you are adding a new variable $initial_references,\n> make sure it is initialized to either an empty string or an undef\n> (and if you choose to do the latter, the right hand side of this\n> assignment cannot blindly reference $initial_references that could\n> still be undef); the way the existing code handles $initial_reply_to\n> may serve as an example.\n"}]}