{"thread":{"id":"42426","subject":"[RFC-PATCH 0/2] send-email: new --quote-mail option","startedAt":"2016-05-23T19:30:27Z","lastAt":"2016-06-16T06:55:53Z","messageCount":96,"participants":["Tom Russello","Matthieu Moy","Eric Wong","Samuel GROOT","Aaron Schrab","Junio C Hamano","Remi Galan Alfonso","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"287298","messageId":"1464031829-6107-1-git-send-email-tom.russello@grenoble-inp.org","threadId":"42426","inReplyTo":null,"subject":"[RFC-PATCH 0/2] send-email: new --quote-mail option","fromName":"Tom Russello","fromEmail":"tom.russello@grenoble-inp.org","sentAt":"2016-05-23T19:30:27Z","receivedAt":"2016-05-23T19:30:27Z","isPatch":true,"sender":{"key":"tom.russello@grenoble-inp.org","avatar":null},"body":"Hello,\n\nWith the current send-email command, you can send a series of patches \"in reply\nto\" an email.\nThis patch adds a new option to `git send-email`, `--quote-mail=`, to\nquote an email in the cover letter in your series of patches.\n\nThe \"To\", \"Cc\" and \"Subject\" fields will be filled appropriately and the message\ngiven quoted in the cover letter for the series of patches.\n\nIn this first patch, the new option `--quote-mail=` needs an\nemail file and does not manage accents.\n\nThere is still work in progress, including:\n\n 1. An option `--quote-mail-id=` to download the message\n from any source, e.g. http://mid.gmane.org//raw.\n The server's address could be set in the repo's config file.\n\n 2. The proper documentation for `--quote-mail=` and\n `--quote-mail-id=` as soon as their definitive\n behavior is approved by the community.\n\n 3. The code to parse the email headers is currently duplicated several\n times, we should refactor it to help maintaining the code.\n\n 4. More tests on the features described above.\n\n\ngit-send-email.perl | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\nt/t9001-send-email.sh | 43 +++++++++++++++++++++++++++++++++++++++++++\n2 files changed, 141 insertions(+), 1 deletion(-)\n"},{"id":"287300","messageId":"1464031829-6107-2-git-send-email-tom.russello@grenoble-inp.org","threadId":"42426","inReplyTo":"1464031829-6107-1-git-send-email-tom.russello@grenoble-inp.org","subject":"[RFC-PATCH 1/2] send-email: new option to quote an email and reply to","fromName":"Tom Russello","fromEmail":"tom.russello@grenoble-inp.org","sentAt":"2016-05-23T19:30:28Z","receivedAt":"2016-05-23T19:30:28Z","isPatch":true,"sender":{"key":"tom.russello@grenoble-inp.org","avatar":null},"body":"From: Tom Russello \n\nThis new option takes an email message file, parses it, fills the \"To\",\n\"Subject\" and \"Cc\" fields appropriately and quote the message.\nThis option involves the `--compose` mode to edit the cover letter quoting the\ngiven email.\n\nSigned-off-by: Tom Russello \nSigned-off-by: Samuel Groot \nSigned-off-by: Matthieu Moy \n\n---\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 6958785..784b8a6 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -55,6 +55,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-mail * Email To, Cc and quote the message.\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@@ -160,7 +161,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,$quote_mail,$initial_subject,@files,\n \t$author,$sender,$smtp_authpass,$annotate,$use_xmailer,$compose,$time);\n \n my $envelope_sender;\n@@ -304,6 +305,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-mail=s\" => \\$quote_mail,\n \t\t \"to=s\" => \\@initial_to,\n \t\t \"to-cmd=s\" => \\$to_cmd,\n \t\t \"no-to\" => \\$no_to,\n@@ -638,6 +640,98 @@ if (@files) {\n \tprint STDERR \"\\nNo patch files specified!\\n\\n\";\n \tusage();\n }\n+my $message_quoted;\n+if ($quote_mail) {\n+\t$message_quoted = \"\";\n+\tmy $error = validate_patch($quote_mail);\n+\t$error and die \"fatal: $quote_mail: $error\\nwarning: no patches were sent\\n\";\n+\t$compose = 1;\n+\tmy @header = ();\n+\topen my $fh, \"<\", $quote_mail or die \"can't open file $quote_mail\";\n+\twhile(<$fh>) {\n+\t\t#for files containing crlf line endings\n+\t\t$_=~ s/\\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+\tforeach(@header) {\n+\t\tmy $input_format;\n+\t\tmy $initial_sender = $sender || $repoauthor || $repocommitter || '';\n+\t\tif (/^From /) {\n+\t\t\t$input_format = 'mbox';\n+\t\t\tnext;\n+\t\t}\n+\t\tchomp;\n+\t\tif (!defined $input_format && /^[-A-Za-z]+:\\s/) {\n+\t\t\t$input_format = 'mbox';\n+\t\t}\n+\n+\t\tif (defined $input_format && $input_format eq 'mbox') {\n+\t\t\tif (/^Subject:\\s+(.*)$/i) {\n+\t\t\t\tmy $prefix_re = \"\";\n+\t\t\t\tmy $subject_re = $1;\n+\t\t\t\tif ($1=~/^[^Re:]/) {\n+\t\t\t\t\t$prefix_re = \"Re: \";\n+\t\t\t\t}\n+\t\t\t\t$initial_subject = $prefix_re.$subject_re;\n+\t\t\t}\n+\t\t\telsif (/^From:\\s+(.*)$/i) {\n+\t\t\t\tpush @initial_to, $1;\n+\t\t\t}\n+\t\t\telsif (/^To:\\s+(.*)$/i) {\n+\t\t\t\tforeach my $addr (parse_address_line($1)) {\n+\t\t\t\t\tif (!($addr eq $initial_sender)) {\n+\t\t\t\t\t\tpush @initial_to, $addr;\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t} elsif (/^Cc:\\s+(.*)$/i) {\n+\t\t\t\tforeach my $addr (parse_address_line($1)) {\n+\t\t\t\t\tmy $qaddr = unquote_rfc2047($addr);\n+\t\t\t\t\tmy $saddr = sanitize_address($qaddr);\n+\t\t\t\t\tif ($saddr eq $initial_sender) {\n+\t\t\t\t\t\tnext if ($suppress_cc{'self'});\n+\t\t\t\t\t} else {\n+\t\t\t\t\t\tnext if ($suppress_cc{'cc'});\n+\t\t\t\t\t}\n+\t\t\t\t\tpush @initial_cc, $addr;\n+\t\t\t\t}\n+\t\t\t} elsif (/^Message-Id: (.*)/i) {\n+\t\t\t\t$initial_reply_to = $1;\n+\t\t\t}\n+\t\t} else {\n+\t\t\t# In the traditional\n+\t\t\t# \"send lots of email\" format,\n+\t\t\t# line 1 = cc\n+\t\t\t# line 2 = subject\n+\t\t\t# So let's support that, too.\n+\t\t\t$input_format = 'lots';\n+\t\t\tif (@cc == 0 && !$suppress_cc{'cc'}) {\n+\t\t\t\tpush @cc, $_;\n+\t\t\t} elsif (!defined $initial_subject) {\n+\t\t\t\t$initial_subject = $_;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\t#Message body\n+\twhile (<$fh>) {\n+\t\t#for files containing crlf line endings\n+\t\t$_=~ s/\\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+\n+}\n \n sub get_patch_subject {\n \tmy $fn = shift;\n@@ -664,6 +758,7 @@ 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 \n \tprint $c <\n\nTests if the \"To\", \"Cc\" and \"Subject\" fields are adequately filled and if the\nmessage is correctly quoted.\n\nSigned-off-by: Tom Russello \nSigned-off-by: Samuel Groot \nSigned-off-by: Matthieu Moy \n\n---\n\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex b3355d2..bda4018 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1885,4 +1885,47 @@ test_expect_success $PREREQ 'leading and trailing whitespaces are removed' '\n \ttest_cmp expected-list actual-list\n '\n \n+test_expect_success $PREREQ 'setup expect' '\n+\tcat >email <<-\\EOF\n+\tMessage-Id: \n+\tFrom: author@example.com\n+\tTo: to1@example.com\n+\tCc: cc1@example.com\n+\tDate: Sat, 12 Jun 2010 15:53:58 +0200\n+\tSubject: subject goes here\n+\n+\tHave you seen my previous email?\n+\t> Previous content\n+\tEOF\n+'\n+\n+test_expect_success $PREREQ 'From, To, Cc, Subject with --quote-mail are correct' '\n+\tclean_fake_sendmail &&\n+\tgit send-email \\\n+\t\t--quote-mail=email \\\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+\tto_adr=$(awk \"/^To: /,/^Cc: /\" msgtxt1) &&\n+\techo \"$to_adr\" | grep author@example.com &&\n+\techo \"$to_adr\" | grep to1@example.com &&\n+\tgrep \"Cc: cc1@example.com\" msgtxt1\n+'\n+test_expect_success $PREREQ 'the message given is quoted with --quote-mail' '\n+\tgrep \"> Have you seen my previous email?\" msgtxt1 &&\n+\tgrep \">> Previous content\" msgtxt1\n+'\n+test_expect_success $PREREQ 'Check if Re is written, only once with --quote-mail' '\n+\tgrep \"Subject: Re: subject goes here\" msgtxt1 &&\n+\tgit send-email \\\n+\t\t--quote-mail=msgtxt1 \\\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.8.2\n"},{"id":"287302","messageId":"vpqlh30d0p1.fsf@anie.imag.fr","threadId":"42426","inReplyTo":"1464031829-6107-1-git-send-email-tom.russello@grenoble-inp.org","subject":"Re: [RFC-PATCH 0/2] send-email: new --quote-mail option","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-05-23T19:38:34Z","receivedAt":"2016-05-23T19:38:34Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Tom Russello writes:\n\n> Hello,\n>\n> With the current send-email command, you can send a series of patches \"in reply\n> to\" an email.\n> This patch adds a new option to `git send-email`, `--quote-mail=`, to\n\nI think the option name should be --quote-email. Even though \"mail\"\nusually means \"email\" for French people, there's still non-electronic\nmail for english-speaking ones.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"287305","messageId":"20160523195504.GA8957@dcvr.yhbt.net","threadId":"42426","inReplyTo":"1464031829-6107-2-git-send-email-tom.russello@grenoble-inp.org","subject":"Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-05-23T19:55:04Z","receivedAt":"2016-05-23T19:55:04Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Tom Russello wrote:\n> This new option takes an email message file, parses it, fills the \"To\",\n> \"Subject\" and \"Cc\" fields appropriately and quote the message.\n> This option involves the `--compose` mode to edit the cover letter quoting the\n> given email.\n\nCool! There should probably be some help text to encourage\ntrimming down the quoted text to only relevant portions.\n\n> +\t#Message body\n> +\twhile (<$fh>) {\n> +\t\t#for files containing crlf line endings\n> +\t\t$_=~ s/\\r//g;\n> +\t\tmy $space=\"\";\n> +\t\tif (/^[^>]/) {\n> +\t\t\t$space = \" \";\n> +\t\t}\n> +\t\t$message_quoted .= \">\".$space.$_;\n\nIs this really necessary to switch between \"> \" and \">\" prefix?\nAFAIK, MUAs prefix unconditionally with \"> \".\nAt least that's how mutt does it...\n"},{"id":"287307","messageId":"211e812b-e61b-09c5-2030-38cd4f73136c@grenoble-inp.org","threadId":"42426","inReplyTo":"vpqlh30d0p1.fsf@anie.imag.fr","subject":"Re: [RFC-PATCH 0/2] send-email: new --quote-mail option","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-05-23T19:56:30Z","receivedAt":"2016-05-23T19:56:30Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"\n\nOn 05/23/2016 09:38 PM, Matthieu Moy wrote:\n> Tom Russello writes:\n>\n>> Hello,\n>>\n>> With the current send-email command, you can send a series of patches \"in reply\n>> to\" an email.\n>> This patch adds a new option to `git send-email`, `--quote-mail=`, to\n>\n> I think the option name should be --quote-email. Even though \"mail\"\n> usually means \"email\" for French people, there's still non-electronic\n> mail for english-speaking ones.\n\nThat makes sense, all occurrences of \"mail\" will be changed into \"email\" \nfor v2.\n\nThanks for your feedback !\n"},{"id":"287306","messageId":"vpq60u4bl4e.fsf@anie.imag.fr","threadId":"42426","inReplyTo":"1464031829-6107-2-git-send-email-tom.russello@grenoble-inp.org","subject":"Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-05-23T20:00:17Z","receivedAt":"2016-05-23T20:00:17Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Tom Russello writes:\n\n> This option involves the `--compose` mode to edit the cover letter quoting the\n\ns/involves/implies/\n\n?\n\nI don't think this is right: I often reply to an email with a single\npatch, for which it would clearly be overkill to have a cover-letter.\n\nYour --quote-mail does two things:\n\n1) Populate the To and Cc field\n\n2) Include the original message body with quotation prefix.\n\nWhen not using --compose, 1) clearly makes sense already, and there's no\nreason to prevent this use-case. When sending a single patch, 2) also\nmakes sense as \"below-tripple-dash comment\", like\n\n This is the commit message for feature A.\n ---\n John Smith wrote:\n > You should implement feature A.\n\n Indeed, here's a patch.\n\n modified-file.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n\nWhen sending multiple patches without --compose, 2) may not make sense,\nbut I think a sane behavior would be:\n\n* If --compose is given, cite the message there.\n\n* If --compose is not given, don't send a cover-letter but cite the body\n as comment in the first patch.\n\nAs a first step, the second point can be changed to \"if --compose is not\ngiven, don't cite the message, just populate the To: and Cc: fields\".\n\n> ---\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n\nNo diffstat?\n\n> @@ -638,6 +640,98 @@ if (@files) {\n> \tprint STDERR \"\\nNo patch files specified!\\n\\n\";\n> \tusage();\n> }\n> +my $message_quoted;\n> +if ($quote_mail) {\n\nStyle: The code you're adding doesn't look related to the code right\nbefore => separate them with a blank line.\n\n> +\twhile(<$fh>) {\n\nStyle: space before (.\n\n> +\t\t\tpush(@header, $_);\n\nI think the code would be clearer if @header was a list of pairs\n(header-name, header-content). Then you'd need much less regex magic\nwhen going through it.\n\n> +\t\t#for files containing crlf line endings\n\nSytle: space after #.\n\n> +\tforeach(@header) {\n\nSpace before (.\n\n> +\t\t\telsif (/^From:\\s+(.*)$/i) {\n> +\t\t\t\tpush @initial_to, $1;\n> +\t\t\t}\n> +\t\t\telsif (/^To:\\s+(.*)$/i) {\n> +\t\t\t\tforeach my $addr (parse_address_line($1)) {\n> +\t\t\t\t\tif (!($addr eq $initial_sender)) {\n> +\t\t\t\t\t\tpush @initial_to, $addr;\n> +\t\t\t\t\t}\n> +\t\t\t\t}\n\nThis adds the content of the To: field in the original email to the Cc:\nfield in the new message, right? If so, this is a weird behavior: when\nfollowing up to an email, one usually addresses to the person s/he's\nreplying, keeping the others Cc-ed, hence the original From: becomes the\nTo header, and the original To: and Cc: become Cc:.\n\n> +\t\t\t} elsif (/^Cc:\\s+(.*)$/i) {\n\nStyle: IIRC, there's no consensus on whether \"elsif\" should be on the\nsame line as the closing }, but please follow the same convention inside\na single if/elsif/ chain.\n\n> +\t#Message body\n\nStyle: space after # (more below). And while you're there, the comment\ncould be \"Quote the message body\" or so, to give a hint to the user\nabout what's going on.\n\n> +\twhile (<$fh>) {\n> +\t\t#for files containing crlf line endings\n> +\t\t$_=~ s/\\r//g;\n> +\t\tmy $space=\"\";\n\nStyle: spaces around =.\n\n> @@ -676,6 +771,8 @@ From: $tpl_sender\n> Subject: $tpl_subject\n> In-Reply-To: $tpl_reply_to\n> \n> +$tpl_quote\n> +\n> EOT\n\nDoesn't this add two extra useless blank lines if $tpl_quote is empty?\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"287308","messageId":"vpqshx8a6ak.fsf@anie.imag.fr","threadId":"42426","inReplyTo":"1464031829-6107-3-git-send-email-tom.russello@grenoble-inp.org","subject":"Re: [RFC-PATCH 2/2] t9001: adding --quote-mail option test","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-05-23T20:05:55Z","receivedAt":"2016-05-23T20:05:55Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"> Subject: [RFC-PATCH 2/2] t9001: adding --quote-mail option test\n\nWe write messages at imperative tone, hence s/adding/add/\n\nTom Russello writes:\n\n> From: Tom Russello \n\nPlease use the same identity for email and commit to avoid this line.\n\n> ---\n>\n>\n> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n\nNo diffstat again?\n\nSplitting a patch series as \"code first; tests after\" is not a good idea\nIMHO. When questionning the behavior of To: Vs Cc: in the previous\npatch, I would have appreciated having tests in the same message, to\ncheck that the tested behavior was indeed the one I was reading in the\ncode.\n\nOTOH, having one patch to introduce \"--quote-email populates To: and Cc:\nheaders\", and then another one for \"--quote-email quotes the message\nbody\" would make the review much easier.\n\nOh, BTW, this obviously lacks documentation\n(Documentation/git-send-email.txt).\n\nAnd that's one reason why the diffstat is useful: one can reply \"this\nlacks tests and doc\" before even reviewing the patch.\n\nRegards,\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"287309","messageId":"vpqmvng8rnt.fsf@anie.imag.fr","threadId":"42426","inReplyTo":"20160523195504.GA8957@dcvr.yhbt.net","subject":"Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-05-23T20:07:18Z","receivedAt":"2016-05-23T20:07:18Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Eric Wong writes:\n\n> Tom Russello wrote:\n>\n>> +\t#Message body\n>> +\twhile (<$fh>) {\n>> +\t\t#for files containing crlf line endings\n>> +\t\t$_=~ s/\\r//g;\n>> +\t\tmy $space=\"\";\n>> +\t\tif (/^[^>]/) {\n>> +\t\t\t$space = \" \";\n>> +\t\t}\n>> +\t\t$message_quoted .= \">\".$space.$_;\n>\n> Is this really necessary to switch between \"> \" and \">\" prefix?\n> AFAIK, MUAs prefix unconditionally with \"> \".\n\nI had the same question, but at least my mailer (Gnus) has the same\nspecial-case it seems.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"287323","messageId":"6e44c8dd-56cd-37c2-81bb-cfaa001bb549@grenoble-inp.org","threadId":"42426","inReplyTo":"vpqmvng8rnt.fsf@anie.imag.fr","subject":"Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-05-23T22:10:20Z","receivedAt":"2016-05-23T22:10:20Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"\n\nOn 05/23/2016 10:07 PM, Matthieu Moy wrote:\n> Eric Wong writes:\n>\n>> Tom Russello wrote:\n>>\n>>> +\t#Message body\n>>> +\twhile (<$fh>) {\n>>> +\t\t#for files containing crlf line endings\n>>> +\t\t$_=~ s/\\r//g;\n>>> +\t\tmy $space=\"\";\n>>> +\t\tif (/^[^>]/) {\n>>> +\t\t\t$space = \" \";\n>>> +\t\t}\n>>> +\t\t$message_quoted .= \">\".$space.$_;\n>>\n>> Is this really necessary to switch between \"> \" and \">\" prefix?\n>> AFAIK, MUAs prefix unconditionally with \"> \".\n>\n> I had the same question, but at least my mailer (Gnus) has the same\n> special-case it seems.\n\n\nThunderbird behaves the same way, so we decided to mimic that behavior.\n\nIt is specified neither in RFC 2822 [1] nor in RFC 5322 [2].\n\nWhen we write an email, we write it with a maximum width of 72 columns. \nIf we insert \"> \" with each reply, the 80-columns limit will be reached \nwith only 4 replies.\n\nSo IMHO we should trim the extra space to allow up to 7 replies before \nreaching the 80-columns limit.\n\n\n[1] https://www.ietf.org/rfc/rfc2822.txt\n[2] https://www.ietf.org/rfc/rfc5322.txt\n"},{"id":"287412","messageId":"0285f901-a1d1-2715-4207-68c7d103da5e@grenoble-inp.org","threadId":"42426","inReplyTo":"20160523195504.GA8957@dcvr.yhbt.net","subject":"Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-05-24T12:43:47Z","receivedAt":"2016-05-24T12:43:47Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"\n\nOn 05/23/2016 09:55 PM, Eric Wong wrote:\n> Tom Russello wrote:\n>> This new option takes an email message file, parses it, fills the \"To\",\n>> \"Subject\" and \"Cc\" fields appropriately and quote the message.\n>> This option involves the `--compose` mode to edit the cover letter quoting the\n>> given email.\n>\n> Cool! There should probably be some help text to encourage\n> trimming down the quoted text to only relevant portions.\n\nWhat kind of help text would you want to see?\n\nMaybe something like this:\n\n GIT: Quoted message body below.\n GIT: Feel free to trim down the quoted text\n GIT: to only relevant portions.\n\nAs \"GIT:\" portions are ignored when parsed by `git send-email`.\n"},{"id":"287413","messageId":"vpqshx77h9o.fsf@anie.imag.fr","threadId":"42426","inReplyTo":"0285f901-a1d1-2715-4207-68c7d103da5e@grenoble-inp.org","subject":"Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-05-24T12:49:23Z","receivedAt":"2016-05-24T12:49:23Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Samuel GROOT writes:\n\n> On 05/23/2016 09:55 PM, Eric Wong wrote:\n>> Tom Russello wrote:\n>>> This new option takes an email message file, parses it, fills the \"To\",\n>>> \"Subject\" and \"Cc\" fields appropriately and quote the message.\n>>> This option involves the `--compose` mode to edit the cover letter quoting the\n>>> given email.\n>>\n>> Cool! There should probably be some help text to encourage\n>> trimming down the quoted text to only relevant portions.\n>\n> What kind of help text would you want to see?\n>\n> Maybe something like this:\n>\n> GIT: Quoted message body below.\n> GIT: Feel free to trim down the quoted text\n> GIT: to only relevant portions.\n>\n> As \"GIT:\" portions are ignored when parsed by `git send-email`.\n\nThat's an option, but in the context of email, I think these\ninstructions are not necessary.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"287456","messageId":"20160524212356.GA2649@dcvr.yhbt.net","threadId":"42426","inReplyTo":"0285f901-a1d1-2715-4207-68c7d103da5e@grenoble-inp.org","subject":"Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-05-24T21:23:56Z","receivedAt":"2016-05-24T21:23:56Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Samuel GROOT wrote:\n> On 05/23/2016 09:55 PM, Eric Wong wrote:\n> >Cool! There should probably be some help text to encourage\n> >trimming down the quoted text to only relevant portions.\n> \n> What kind of help text would you want to see?\n> \n> Maybe something like this:\n> \n> GIT: Quoted message body below.\n> GIT: Feel free to trim down the quoted text\n> GIT: to only relevant portions.\n> \n> As \"GIT:\" portions are ignored when parsed by `git send-email`.\n\nYes, given we have instructions for diffstat and table of contents;\nI think it'd be useful to discourage quoting irrelevant parts of\nthe message (especially signatures and like).\n"},{"id":"287460","messageId":"20160524223005.GB23162@pug","threadId":"42426","inReplyTo":"vpqshx77h9o.fsf@anie.imag.fr","subject":"Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2016-05-24T22:30:05Z","receivedAt":"2016-05-24T22:30:05Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"At 14:49 +0200 24 May 2016, Matthieu Moy wrote:\n>Samuel GROOT writes:\n>\n>> What kind of help text would you want to see?\n>>\n>> Maybe something like this:\n>>\n>> GIT: Quoted message body below.\n>> GIT: Feel free to trim down the quoted text\n>> GIT: to only relevant portions.\n>>\n>> As \"GIT:\" portions are ignored when parsed by `git send-email`.\n>\n>That's an option, but in the context of email, I think these\n>instructions are not necessary.\n\nIn an ideal world that would be true. But in the real world I think \nevidence of many messages to this mailing list containing full quotes \nsuggests it might be helpful. I'd actually argue that the message be \nmore forceful, making it a suggestion/request to trim rather than simply \ntelling the user that it's allowed.\n"},{"id":"287466","messageId":"b1752a59-af2b-6e18-fc69-0650440939e3@grenoble-inp.org","threadId":"42426","inReplyTo":"vpq60u4bl4e.fsf@anie.imag.fr","subject":"Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-05-24T23:31:28Z","receivedAt":"2016-05-24T23:31:28Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 05/23/2016 10:00 PM, Matthieu Moy wrote:\n> I don't think this is right: I often reply to an email with a single\n> patch, for which it would clearly be overkill to have a cover-letter.\n\nYes indeed, we are working on inserting the quoted message body in the \npatch's \"below-triple-dash\" section.\n\n> Your --quote-mail does two things:\n>\n> 1) Populate the To and Cc field\n>\n> 2) Include the original message body with quotation prefix.\n>\n> When not using --compose, 1) clearly makes sense already, and there's no\n> reason to prevent this use-case. When sending a single patch, 2) also\n> makes sense as \"below-tripple-dash comment\", like\n>\n> This is the commit message for feature A.\n> ---\n> John Smith wrote:\n> > You should implement feature A.\n>\n> Indeed, here's a patch.\n>\n> modified-file.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n>\n> When sending multiple patches without --compose, 2) may not make sense,\n> but I think a sane behavior would be:\n>\n> * If --compose is given, cite the message there.\n>\n> * If --compose is not given, don't send a cover-letter but cite the body\n> as comment in the first patch.\n>\n> As a first step, the second point can be changed to \"if --compose is not\n> given, don't cite the message, just populate the To: and Cc: fields\".\n\nIt seems a good behavior to me too.\n\n> * If --compose is not given, don't send a cover-letter but cite the body\n> as comment in the first patch.\n\nThen should the option imply `--annotate`, to let the user edit the \npatch containing the quoted email?\n\n>> +\t\t\tpush(@header, $_);\n>\n> I think the code would be clearer if @header was a list of pairs\n> (header-name, header-content). Then you'd need much less regex magic\n> when going through it.\n\nThe next series of patches may include (if the code is clean enough by \nthen) a cleaner subroutine to parse the email, probably returning \nsomething like:\n\n return (\n \"from\" => $from,\n \"subject\" => $subject,\n \"date\" => $date,\n \"message_id\" => $message_id,\n \"to\" => [@to],\n \"cc\" => [@cc],\n \"xh\" => [@xh],\n \"config\" => {%config}\n );\n\n>> +\t\t\telsif (/^From:\\s+(.*)$/i) {\n>> +\t\t\t\tpush @initial_to, $1;\n>> +\t\t\t}\n>> +\t\t\telsif (/^To:\\s+(.*)$/i) {\n>> +\t\t\t\tforeach my $addr (parse_address_line($1)) {\n>> +\t\t\t\t\tif (!($addr eq $initial_sender)) {\n>> +\t\t\t\t\t\tpush @initial_to, $addr;\n>> +\t\t\t\t\t}\n>> +\t\t\t\t}\n>\n> This adds the content of the To: field in the original email to the Cc:\n> field in the new message, right? If so, this is a weird behavior: when\n> following up to an email, one usually addresses to the person s/he's\n> replying, keeping the others Cc-ed, hence the original From: becomes the\n> To header, and the original To: and Cc: become Cc:.\n\nWe made the option behave like Thunderbird does, but indeed RFC 2822 [1] \nsees it the same you do, it will be fixed in next iteration.\n\n>> @@ -676,6 +771,8 @@ From: $tpl_sender\n>> Subject: $tpl_subject\n>> In-Reply-To: $tpl_reply_to\n>>\n>> +$tpl_quote\n>> +\n>> EOT\n>\n> Doesn't this add two extra useless blank lines if $tpl_quote is empty?\n\nYes it does, it will be fixed in the next series of patches.\n\nThank you for your time!\n\n\n[1] https://www.ietf.org/rfc/rfc2822.txt\n"},{"id":"287467","messageId":"9afb4e1e-9e60-79eb-c90f-77b2eb55d1aa@grenoble-inp.org","threadId":"42426","inReplyTo":"20160524223005.GB23162@pug","subject":"Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to","fromName":"Tom Russello","fromEmail":"tom.russello@grenoble-inp.org","sentAt":"2016-05-25T00:04:46Z","receivedAt":"2016-05-25T00:04:46Z","isPatch":true,"sender":{"key":"tom.russello@grenoble-inp.org","avatar":null},"body":"On 05/25/16 00:30, Aaron Schrab wrote:\n> At 14:49 +0200 24 May 2016, Matthieu Moy \n> wrote:\n>> Samuel GROOT writes:\n>>\n>>> What kind of help text would you want to see?\n>>>\n>>> Maybe something like this:\n>>>\n>>> GIT: Quoted message body below.\n>>> GIT: Feel free to trim down the quoted text\n>>> GIT: to only relevant portions.\n>>>\n>>> As \"GIT:\" portions are ignored when parsed by `git send-email`.\n>>\n>> That's an option, but in the context of email, I think these\n>> instructions are not necessary.\n> \n> In an ideal world that would be true. But in the real world I think\n> evidence of many messages to this mailing list containing full quotes\n> suggests it might be helpful. I'd actually argue that the message be\n> more forceful, making it a suggestion/request to trim rather than simply\n> telling the user that it's allowed.\n\nFurthermore, it is a good way to avoid very long messages due to\nunnecessary parts quoted.\n\nTherefore, we thought about a request like \"Please, trim down irrelevant\nsections in the quoted message to keep your email concise\"\n"},{"id":"287483","messageId":"vpqh9dmfy5k.fsf@anie.imag.fr","threadId":"42426","inReplyTo":"b1752a59-af2b-6e18-fc69-0650440939e3@grenoble-inp.org","subject":"Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-05-25T06:29:43Z","receivedAt":"2016-05-25T06:29:43Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Samuel GROOT writes:\n\n> On 05/23/2016 10:00 PM, Matthieu Moy wrote:\n>\n>> Your --quote-mail does two things:\n>>\n>> 1) Populate the To and Cc field\n>>\n>> 2) Include the original message body with quotation prefix.\n>>\n\n[...]\n\n>> * If --compose is not given, don't send a cover-letter but cite the body\n>> as comment in the first patch.\n>\n> Then should the option imply `--annotate`, to let the user edit the\n> patch containing the quoted email?\n\nActually, I'm not sure what the ideal behavior should be. Perhaps it's\nbetter to distinguish 1) and 2) above, and have two options\n--reply-to-email= doing 1), and --quote doing 2), implying\n--compose and requiring --reply-to-email.\n\nThat would be more flexible, but that would require two new options, and\nI also like to keep things simple.\n\nIn any case, quoting the message without replying to it does not make\nsense (especially if you add instructions to trim it: the user would not\neven see it). So it its current form, I'd say --quote-email should imply\n--annotate.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"287496","messageId":"xmqqwpmi16zt.fsf@gitster.mtv.corp.google.com","threadId":"42426","inReplyTo":"vpqh9dmfy5k.fsf@anie.imag.fr","subject":"Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-25T15:40:06Z","receivedAt":"2016-05-25T15:40:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Actually, I'm not sure what the ideal behavior should be. Perhaps it's\n> better to distinguish 1) and 2) above, and have two options\n> --reply-to-email= doing 1), and --quote doing 2), implying\n> --compose and requiring --reply-to-email.\n\nI tend to agree that sounds like a better way to structure these\nfeatures.\n\nI wonder if we can safely repurpose existing --in-reply-to option?\nThat is, if the value of --in-reply-to can be reliably determined as\na filename that has the message (as opposed to a message-id), we\nread the \"Message-Id:\" from that file to figuire out what message-id\nto use, and figure out To/Cc: to use for the purpose of your (1) at\nthe same time. In the future, you might even teach send-email,\nperhaps via a user configurable hook, a way to get to the message\nheader and text given a message-id, and when it happens, the same\nlogic can be used when --in-reply-to is given a message-id (i.e. you\ngo from the id to the message and find the addresses you would\nTo/Cc: your message).\n\n> In any case, quoting the message without replying to it does not make\n> sense (especially if you add instructions to trim it: the user would not\n> even see it). So it its current form, I'd say --quote-email should imply\n> --annotate.\n"},{"id":"287502","messageId":"vpqeg8q5b6f.fsf@anie.imag.fr","threadId":"42426","inReplyTo":"xmqqwpmi16zt.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-05-25T16:56:08Z","receivedAt":"2016-05-25T16:56:08Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> I wonder if we can safely repurpose existing --in-reply-to option?\n\nIn theory, obviously no as there can be a file with this name _and_ it\ncan be a valid message-id. In practice, it is clearly unlikely. The only\nuse-case I can think of where both would be valid is if the user happens\nto have saved the message using the message-id as filename. But then,\nthe ambiguity would not harm, as the message-id contained in the file\nwould be the same as the filename.\n\n> That is, if the value of --in-reply-to can be reliably determined as\n> a filename that has the message (as opposed to a message-id), we\n> read the \"Message-Id:\" from that file to figuire out what message-id\n> to use, and figure out To/Cc: to use for the purpose of your (1) at\n> the same time.\n\nThis should work, but sounds like too much of overloading of\n--in-reply-to IMHO: if given a message id, it would only add a reference\nto this message-id, but if given a file, it would also modify the To:\nand Cc: list.\n\nNot a strong objection, though.\n\n> In the future, you might even teach send-email, perhaps via a user\n> configurable hook, a way to get to the message header and text given a\n> message-id, and when it happens, the same logic can be used when\n> --in-reply-to is given a message-id (i.e. you go from the id to the\n> message and find the addresses you would To/Cc: your message).\n\nThat is the plan indeed. Fetching from gmane for example should be\nrather easy in perl, and would be really convenient!\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"287504","messageId":"xmqqr3cqypg6.fsf@gitster.mtv.corp.google.com","threadId":"42426","inReplyTo":"vpqeg8q5b6f.fsf@anie.imag.fr","subject":"Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-25T18:15:05Z","receivedAt":"2016-05-25T18:15:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> This should work, but sounds like too much of overloading of\n> --in-reply-to IMHO: if given a message id, it would only add a reference\n> to this message-id, but if given a file, it would also modify the To:\n> and Cc: list.\n>\n> Not a strong objection, though.\n\nWell, with your \"that is the plan indeed\", the option would behave\nthe same whether given a message ID or a filename, no?\n\nBut I do agree that those who have accustomed to the behaviour of\n--in-reply-to that does not mess with To/Cc:, such a behaviour\nchange is not desirable.\n\nIf we are adding a new --reply-to-email=, it should behave\nas a superset of --in-reply-to (i.e. it should set In-Reply-to:\nusing the message ID of the e-mail we are replying to), though.\n\n>> In the future, you might even teach send-email, perhaps via a user\n>> configurable hook, a way to get to the message header and text given a\n>> message-id, and when it happens, the same logic can be used when\n>> --in-reply-to is given a message-id (i.e. you go from the id to the\n>> message and find the addresses you would To/Cc: your message).\n>\n> That is the plan indeed. Fetching from gmane for example should be\n> rather easy in perl, and would be really convenient!\n"},{"id":"287506","messageId":"vpq8tyyt2fv.fsf@anie.imag.fr","threadId":"42426","inReplyTo":"xmqqr3cqypg6.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-05-25T18:31:00Z","receivedAt":"2016-05-25T18:31:00Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Matthieu Moy writes:\n>\n>> This should work, but sounds like too much of overloading of\n>> --in-reply-to IMHO: if given a message id, it would only add a reference\n>> to this message-id, but if given a file, it would also modify the To:\n>> and Cc: list.\n>>\n>> Not a strong objection, though.\n>\n> Well, with your \"that is the plan indeed\", the option would behave\n> the same whether given a message ID or a filename, no?\n\nThe \"fetch message from ID\" feature should not be unconditional IMHO. So\nit would probably be stg like:\n\n git send-email --in-reply-to= --fetch\n\nWhat's a bit counter-intuitive is that --fetch would not only trigger\nfetching the complete message, but also populate To/Cc. But thinking\nabout it, it's not _that_ counter-intuitive, as fetching the message\nshould be done for a reason, so the user can guess that the message is\ngoing to be used for something.\n\nSo, a possible UI would be:\n\n git send-email --in-reply-to= => just set In-Reply-To: field.\n\n git send-email --in-reply-to= => set In-Reply-To, To and Cc.\n\n git send-email --in-reply-to= --cite => in addition, add the\n body of the message quoted with '> '.\n\n git send-email --in-reply-to= --fetch => fetch and do like \n using the default configuration for fetch.\n\nThis leaves room for:\n\n git send-email --in-reply-to= --fetch=gmane => fetch from gmane\n (details on how to fetch would be in the config file)\n\nThis UI wouldn't allow using a file to get only the message-id. But I'm\nnot sure this is an interesting use-case.\n\nSo, I guess you convinced me.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"287544","messageId":"e678afcb-b378-9829-fae4-1f1ed38ee593@grenoble-inp.org","threadId":"42426","inReplyTo":"vpq8tyyt2fv.fsf@anie.imag.fr","subject":"Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-05-26T00:08:32Z","receivedAt":"2016-05-26T00:08:32Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 05/25/2016 08:31 PM, Matthieu Moy wrote:\n> So, a possible UI would be:\n>\n> git send-email --in-reply-to= => just set In-Reply-To: field.\n>\n> git send-email --in-reply-to= => set In-Reply-To, To and Cc.\n>\n> git send-email --in-reply-to= --cite => in addition, add the\n> body of the message quoted with '> '.\n>\n> git send-email --in-reply-to= --fetch => fetch and do like \n> using the default configuration for fetch.\n\nWe designed a similar UI, except for the --fetch option:\n\nWe wanted to try to fetch the email from a distant server (e.g. gmane)\nif that server address was set in the config file, and populate the\nTo:/Cc: fields.\n\nIf the file cannot be downloaded, or server address not set, just fill \nthe Reply-to header.\n\nEither way, display what was done with the message-id given (unless\n--quiet is set, of course).\n\n> This leaves room for:\n>\n> git send-email --in-reply-to= --fetch=gmane => fetch from gmane\n> (details on how to fetch would be in the config file)\n>\n> This UI wouldn't allow using a file to get only the message-id. But I'm\n> not sure this is an interesting use-case.\n\nIMHO when you reply to a thread with a patch, it seems\ncounter-productive to reply without notifying (putting in To:/Cc:) the\noriginal author and people involved in the thread.\n"},{"id":"287664","messageId":"vpq4m9juaxd.fsf@anie.imag.fr","threadId":"42426","inReplyTo":"e678afcb-b378-9829-fae4-1f1ed38ee593@grenoble-inp.org","subject":"Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-05-27T09:06:54Z","receivedAt":"2016-05-27T09:06:54Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Samuel GROOT writes:\n\n> On 05/25/2016 08:31 PM, Matthieu Moy wrote:\n>> So, a possible UI would be:\n>>\n>> git send-email --in-reply-to= => just set In-Reply-To: field.\n>>\n>> git send-email --in-reply-to= => set In-Reply-To, To and Cc.\n>>\n>> git send-email --in-reply-to= --cite => in addition, add the\n>> body of the message quoted with '> '.\n>>\n>> git send-email --in-reply-to= --fetch => fetch and do like \n>> using the default configuration for fetch.\n>\n> We designed a similar UI, except for the --fetch option:\n>\n> We wanted to try to fetch the email from a distant server (e.g. gmane)\n> if that server address was set in the config file, and populate the\n> To:/Cc: fields.\n>\n> If the file cannot be downloaded, or server address not set, just fill\n> the Reply-to header.\n\nI'm not sure this is right. I'd typically configure gmane in my\nuser-wide config file, so I'd have it configured all the time, but I may\nnot always want to fetch from it (e.g. replying to a message which is\nnot on a mailing-list, or if gmane is down, or ...).\n\nFetching by default would clearly work in most cases, but for the few\nremaning cases it may be painful for the user if the only way to disable\nfetching is to remove the configuration from the config file.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"287693","messageId":"1464369102-7551-1-git-send-email-tom.russello@grenoble-inp.org","threadId":"42426","inReplyTo":"1464031829-6107-1-git-send-email-tom.russello@grenoble-inp.org","subject":"[RFC-PATCH v2 0/2] send-email: new --quote-email option","fromName":"Tom Russello","fromEmail":"tom.russello@grenoble-inp.org","sentAt":"2016-05-27T17:11:40Z","receivedAt":"2016-05-27T17:11:40Z","isPatch":true,"sender":{"key":"tom.russello@grenoble-inp.org","avatar":null},"body":"Hello,\n\nWith the current send-email command, you can send a series of patches\n\"in reply to\" an email.\n\nThis patch adds a new option to `git send-email`,\n`--quote-email=` which does two things:\n\t- set fields appropriately (To, Cc, Subject, In-Reply-To)\n\t- quote the given message\n\nIn this second patch, the new option `--quote-email=` needs an\nemail file and does not manage non ascii characters.\n\nThere is still work in progress, including:\n\n 1. An option `--quote-email-id=` to download the message\n from any source, e.g. http://mid.gmane.org//raw.\n The server's address could be set in the repo's config file.\n\n 2. There's also a discussion about whether this option should be\n integrated in the current `--in-reply-to` option or not\n\n * http://news.gmane.org/find-root.php?message_id=vpqh9dmfy5k.fsf@anie.imag.fr\n\n\n 3. The code to parse the email headers is currently duplicated\n several times, we are refactoring it to help maintaining the\n code.\n\n 4. Manage non ascii characters\n\nDocumentation/git-send-email.txt | 8 +++++++\ngit-send-email.perl | 160 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---\nt/t9001-send-email.sh | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n3 files changed, 232 insertions(+), 3 deletions(-)\n"},{"id":"287695","messageId":"1464369102-7551-2-git-send-email-tom.russello@grenoble-inp.org","threadId":"42426","inReplyTo":"1464369102-7551-1-git-send-email-tom.russello@grenoble-inp.org","subject":"[RFC-PATCH v2 1/2] send-email: quote-email populates the fields","fromName":"Tom Russello","fromEmail":"tom.russello@grenoble-inp.org","sentAt":"2016-05-27T17:11:41Z","receivedAt":"2016-05-27T17:11:41Z","isPatch":true,"sender":{"key":"tom.russello@grenoble-inp.org","avatar":null},"body":"Take an email message file, parse it and fill the \"To\", \"Cc\" and\n\"In-Reply-To\" fields appropriately.\n\nIf `--compose` option is set, it will also fill the subject field with\n\"Re: ['s subject]\".\n\nSigned-off-by: Tom Russello \nSigned-off-by: Samuel Groot \nSigned-off-by: Matthieu Moy \n---\nAs it is said in the cover letter, the file git-send-email.perl is being\nrefactored therefore the parsing section with nested if's is ought to\nchange.\n\nchanges since v1:\n\t- option's name changed and is now --quote-email\n\t- original From: becomes To:, original To:'s become Cc: and original\n\t Cc:'s stay Cc:\n\t- coding style improved\n\t- documentation for the option\n\t- more tests\n\n Documentation/git-send-email.txt | 5 +++\n git-send-email.perl | 87 +++++++++++++++++++++++++++++++++++++++-\n t/t9001-send-email.sh | 61 ++++++++++++++++++++++++++++\n 3 files changed, 152 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex 771a7b5..2334d69 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -106,6 +106,11 @@ 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+\tReply to the given email and automatically populate the \"To:\", \"Cc:\" and\n+\t\"In-Reply-To:\" fields. If `--compose` is set, this will also fill the\n+\tsubject field with \"Re: ['s subject]\".\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 6958785..9df3dee 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -55,6 +55,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 * Fill the fields \"To:\", \"Cc:\", \"Subject:\",\n+ \"In-Reply-To\" 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@@ -160,7 +162,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,$quote_email,$initial_subject,@files,\n \t$author,$sender,$smtp_authpass,$annotate,$use_xmailer,$compose,$time);\n \n my $envelope_sender;\n@@ -304,6 +306,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@@ -639,6 +642,88 @@ if (@files) {\n \tusage();\n }\n \n+if ($quote_email) {\n+\tmy $error = validate_patch($quote_email);\n+\t$error and die \"fatal: $quote_email: $error\\nwarning: no patches were sent\\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# For files containing crlf line endings\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 $input_format;\n+\t\tmy $initial_sender = $sender || $repoauthor || $repocommitter || '';\n+\n+\t\tif (/^From /) {\n+\t\t\t$input_format = 'mbox';\n+\t\t\tnext;\n+\t\t}\n+\t\tchomp;\n+\t\tif (!defined $input_format && /^[-A-Za-z]+:\\s/) {\n+\t\t\t$input_format = 'mbox';\n+\t\t}\n+\n+\t\tif (defined $input_format && $input_format eq 'mbox') {\n+\t\t\tif (/^Subject:\\s+(.*)$/i) {\n+\t\t\t\tmy $prefix_re = \"\";\n+\t\t\t\tmy $subject_re = $1;\n+\t\t\t\tif ($1 =~ /^[^Re:]/) {\n+\t\t\t\t\t$prefix_re = \"Re: \";\n+\t\t\t\t}\n+\t\t\t\t$initial_subject = $prefix_re . $subject_re;\n+\t\t\t} elsif (/^From:\\s+(.*)$/i) {\n+\t\t\t\tpush @initial_to, $1;\n+\t\t\t} elsif (/^To:\\s+(.*)$/i) {\n+\t\t\t\tforeach my $addr (parse_address_line($1)) {\n+\t\t\t\t\tif (!($addr eq $initial_sender)) {\n+\t\t\t\t\t\tpush @initial_cc, $addr;\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t} elsif (/^Cc:\\s+(.*)$/i) {\n+\t\t\t\tforeach my $addr (parse_address_line($1)) {\n+\t\t\t\t\tmy $qaddr = unquote_rfc2047($addr);\n+\t\t\t\t\tmy $saddr = sanitize_address($qaddr);\n+\t\t\t\t\tif ($saddr eq $initial_sender) {\n+\t\t\t\t\t\tnext if ($suppress_cc{'self'});\n+\t\t\t\t\t} else {\n+\t\t\t\t\t\tnext if ($suppress_cc{'cc'});\n+\t\t\t\t\t}\n+\t\t\t\t\tpush @initial_cc, $addr;\n+\t\t\t\t}\n+\t\t\t} elsif (/^Message-Id: (.*)/i) {\n+\t\t\t\t$initial_reply_to = $1;\n+\t\t\t}\n+\t\t} else {\n+\t\t\t# In the traditional\n+\t\t\t# \"send lots of email\" format,\n+\t\t\t# line 1 = cc\n+\t\t\t# line 2 = subject\n+\t\t\t# So let's support that, too.\n+\t\t\t$input_format = 'lots';\n+\t\t\tif (@cc == 0 && !$suppress_cc{'cc'}) {\n+\t\t\t\tpush @cc, $_;\n+\t\t\t} elsif (!defined $initial_subject) {\n+\t\t\t\t$initial_subject = $_;\n+\t\t\t}\n+\t\t}\n+\t}\n+}\n+\n sub get_patch_subject {\n \tmy $fn = shift;\n \topen (my $fh, '<', $fn);\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex b3355d2..389a54c 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1885,4 +1885,65 @@ test_expect_success $PREREQ 'leading and trailing whitespaces are removed' '\n \ttest_cmp expected-list actual-list\n '\n \n+test_expect_success $PREREQ 'setup expect' '\n+\tcat >email <<-\\EOF\n+\tMessage-Id: \n+\tFrom: author@example.com\n+\tTo: to1@example.com\n+\tCc: cc1@example.com\n+\tDate: Sat, 12 Jun 2010 15:53:58 +0200\n+\tSubject: subject goes here\n+\n+\tHave you seen my previous email?\n+\t> Previous content\n+\tEOF\n+'\n+\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--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+\tto_adr=$(awk \"/^To: /,/^Cc: /\" msgtxt1) &&\n+\tcc_adr=$(awk \"/^Cc: /,/^Date /\" msgtxt1) &&\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+'\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: /,/^Cc: /\" msgtxt1) &&\n+\tcc_adr=$(awk \"/^Cc: /,/^Date /\" msgtxt1) &&\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+'\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.8.2\n"},{"id":"287694","messageId":"1464369102-7551-3-git-send-email-tom.russello@grenoble-inp.org","threadId":"42426","inReplyTo":"1464369102-7551-1-git-send-email-tom.russello@grenoble-inp.org","subject":"[RFC-PATCH v2 2/2] send-email: quote-email quotes the message body","fromName":"Tom Russello","fromEmail":"tom.russello@grenoble-inp.org","sentAt":"2016-05-27T17:11:42Z","receivedAt":"2016-05-27T17:11:42Z","isPatch":true,"sender":{"key":"tom.russello@grenoble-inp.org","avatar":null},"body":"Take an email message file, parse it and quote the message body.\n\nIf `--compose` is set, then it will quote the message in the cover letter.\nOtherwise, imply `--annotate` option and put the quoted message in the first\npatch after the triple-dash.\n\nSigned-off-by: Tom Russello \nSigned-off-by: Samuel Groot \nSigned-off-by: Matthieu Moy \n---\nCurrently, `send-email` without `--compose` implies `--annotate`. The\ncurrent behavior of `--annotate` is that changes made during edition are saved\nin patch files.\n\nKeeping that behavior when using `--quote-email` populates the patch file with\nthe quoted message body, and the patch is saved no matter what. If the user\ncloses his editor and then exits `send-email`, changes will be saved.\n\nShould we keep the current behavior for the user, keeping the changes (including\nthe quoted message body) in the patch, or should we discard them?\n\n\nchanges since v1:\n\t- default behaviour of the option: it is now --annotate as one may not want\n\t to send a cover letter for a single patch\n\t- \"On [date], [original author] wrote:\" is added before the quoted message\n\t- request to trim useless parts of the cited message when `--compose` is set\n\t- extra blank removed when the message quoted is empty\n\n Documentation/git-send-email.txt | 5 ++-\n git-send-email.perl | 79 +++++++++++++++++++++++++++++++++++++---\n t/t9001-send-email.sh | 6 +++\n 3 files changed, 84 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex 2334d69..68bbb6c 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -109,7 +109,10 @@ is not set, this will be prompted for.\n --quote-email=::\n \tReply to the given email and automatically populate the \"To:\", \"Cc:\" and\n \t\"In-Reply-To:\" fields. If `--compose` is set, this will also fill the\n-\tsubject field with \"Re: ['s subject]\".\n+\tsubject field with \"Re: ['s subject]\" and quote the message body\n+\tof . If `--compose` is not set, this will imply `--annotate`\n+\toption and will quote the message body of after the triple-dash\n+\tin the first patch.\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 9df3dee..e73aa25 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(catfile);\n+use File::Copy;\n use Error qw(:try);\n use Git;\n \n@@ -55,8 +56,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 * Fill the fields \"To:\", \"Cc:\", \"Subject:\",\n- \"In-Reply-To\" appropriately.\n+ --quote-email * Fill To:, Cc:, Subject:, In-Reply-To:\n+ appropriately and 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@@ -642,11 +643,14 @@ if (@files) {\n \tusage();\n }\n \n+my $message_quoted;\n if ($quote_email) {\n \tmy $error = validate_patch($quote_email);\n \t$error and die \"fatal: $quote_email: $error\\nwarning: no patches were sent\\n\";\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@@ -687,7 +691,8 @@ if ($quote_email) {\n \t\t\t\t}\n \t\t\t\t$initial_subject = $prefix_re . $subject_re;\n \t\t\t} elsif (/^From:\\s+(.*)$/i) {\n-\t\t\t\tpush @initial_to, $1;\n+\t\t\t\t$recipient = $1;\n+\t\t\t\tpush @initial_to, $recipient;\n \t\t\t} elsif (/^To:\\s+(.*)$/i) {\n \t\t\t\tforeach my $addr (parse_address_line($1)) {\n \t\t\t\t\tif (!($addr eq $initial_sender)) {\n@@ -707,6 +712,8 @@ if ($quote_email) {\n \t\t\t\t}\n \t\t\t} elsif (/^Message-Id: (.*)/i) {\n \t\t\t\t$initial_reply_to = $1;\n+\t\t\t} elsif (/^Date: (.*)/i) {\n+\t\t\t\t$date = $1;\n \t\t\t}\n \t\t} else {\n \t\t\t# In the traditional\n@@ -722,6 +729,23 @@ if ($quote_email) {\n \t\t\t}\n \t\t}\n \t}\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# Only for files containing crlf line endings\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@@ -749,6 +773,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\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\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 389a54c..5ab7533 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1916,6 +1916,12 @@ test_expect_success $PREREQ 'Fields with --quote-email are correct' '\n \techo \"$cc_adr\" | grep cc1@example.com\n '\n \n+test_expect_success $PREREQ 'correct quoted message with --quote-email' '\n+\tgrep \"On Sat, 12 Jun 2010 15:53:58 +0200, author@example.com wrote:\" msgtxt1 &&\n+\tgrep \"> Have you seen my previous email?\" msgtxt1 &&\n+\tgrep \">> Previous content\" msgtxt1\n+'\n+\n test_expect_success $PREREQ 'Fields with --quote-email and --compose are correct' '\n \tclean_fake_sendmail &&\n \tgit send-email \\\n-- \n2.8.2\n"},{"id":"287736","messageId":"vpqk2iel081.fsf@anie.imag.fr","threadId":"42426","inReplyTo":"1464369102-7551-2-git-send-email-tom.russello@grenoble-inp.org","subject":"Re: [RFC-PATCH v2 1/2] send-email: quote-email populates the fields","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-05-28T14:35:10Z","receivedAt":"2016-05-28T14:35:10Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Tom Russello writes:\n\n> --- a/Documentation/git-send-email.txt\n> +++ b/Documentation/git-send-email.txt\n> @@ -106,6 +106,11 @@ 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> +\tReply to the given email and automatically populate the \"To:\", \"Cc:\" and\n> +\t\"In-Reply-To:\" fields.\n\nI think this is a bit too technical for a user documentation. To: and\nCc: is OK, but people need not know about \"In-Reply-To:\" to understand\nthis. See what the doc of --in-reply-to says. If you want to be\ntechnical, you'd need to mention the References: field too.\n\nTalking about Reference: field, something your patch could do is to add\nall references in to the references of the new email (see\nwhat a mailer is doing when replying). This way, the recipient can still\nget threading if the last message being replied-to is missing.\n\n> +\"Re: ['s subject]\".\n\nPerhaps `Re: ...` instead of double-quotes.\n\n> +if ($quote_email) {\n> +\tmy $error = validate_patch($quote_email);\n> +\t$error and die \"fatal: $quote_email: $error\\nwarning: no patches were sent\\n\";\n\nI know it's done this way elsewhere, but I don't like this \"$error and\ndie\", I'd rather see a proper if here.\n\n> +\t\tif (defined $input_format && $input_format eq 'mbox') {\n\nTo me, the input format refers to patch files, not the .\n\nI'm not sure anyone still use the \"lots of email\" format, and you are\nnot testing it. So, this is claiming that we have a feature without\nbeing sure we have it nor that anyone's ever going to use it.\n\nI'd just drop this \"if\" and the \"else\" branch, and just assume the email\nfile is a normal email file.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"287741","messageId":"vpqlh2ujkg8.fsf@anie.imag.fr","threadId":"42426","inReplyTo":"1464369102-7551-3-git-send-email-tom.russello@grenoble-inp.org","subject":"Re: [RFC-PATCH v2 2/2] send-email: quote-email quotes the message body","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-05-28T15:01:11Z","receivedAt":"2016-05-28T15:01:11Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Tom Russello writes:\n\n> Currently, `send-email` without `--compose` implies `--annotate`.\n\nI don't get it. Did you mean s/without/with/? Even if so, this is not\nexactly true: \"git send-email --compose -1\" will open the editor only\nfor the cover-letter, while adding --annotate will also open it for the\npatch.\n\n> Keeping that behavior when using `--quote-email` populates the patch file with\n> the quoted message body, and the patch is saved no matter what. If the user\n> closes his editor and then exits `send-email`, changes will be saved.\n>\n> Should we keep the current behavior for the user, keeping the changes (including\n> the quoted message body) in the patch, or should we discard them?\n\n(Note: we discussed this off-list already, but I'll try to summarize my\nthoughts here)\n\nI don't have strong opinion on this, but I think there's a difference\nbetween launching the editor directly on the input patch files\n(resulting in _user_'s edit being done directly on them) and having the\nscript modify it in-place (resulting in automatic changes done directly\non the user's files).\n\nI usually use \"git send-email\" directly without using \"git\nformat-patch\", so I'm not the best juge. But I can imagine a flow like\n\n1) run \"git send-email *.patch\"\n\n2) start editting\n\n3) notice there's something wrong, give up for now (answer 'q' when git\n send-email prompts for confirmation, or kill it via Control-C in a\n terminal)\n\n4) run \"git send-email *.patch\" again\n\n5) be happy that changes done at 2) are still there.\n\nWith --quote-email, it's different. The scenario above would result in\n\n5') WTF, why is the email quoted twice?\n\nUnfortunately, I don't really have a solution for this. My first thought\nwas that we should copy the files to a temporary location before\nstarting the editor (that what I'm used to when using \"git send-email\"\nwithout \"git format-patch\"), but that would prevent 5) above.\n\n> @@ -109,7 +109,10 @@ is not set, this will be prompted for.\n> --quote-email=::\n> \tReply to the given email and automatically populate the \"To:\", \"Cc:\" and\n> \t\"In-Reply-To:\" fields. If `--compose` is set, this will also fill the\n> -\tsubject field with \"Re: ['s subject]\".\n> +\tsubject field with \"Re: ['s subject]\" and quote the message body\n> +\tof .\n\nI'd add \"in the introductory message\".\n\n> +\twhile (<$fh>) {\n> +\t\t# Only for files containing crlf line endings\n> +\t\ts/\\r//g;\n\nThe comment doesn't really say what it does.\n\nWhat about \"turn crlf line endings into lf-only\"?\n\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\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\n> +\t\tmove($quote_email_filename, $tmp);\n> +\t\t$files[0] = $tmp;\n\nWhen writing comment, always try to ask the question \"why?\" more than\n\"what?\". This part is possibly controversial, think about a contributor\nfinding this piece of code later without having followed the current\nconversation. He'd probably expect an explanation about why you need a\ntemp file here and not elsewhere.\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\nNo space before :.\n\n> --- a/t/t9001-send-email.sh\n> +++ b/t/t9001-send-email.sh\n> @@ -1916,6 +1916,12 @@ test_expect_success $PREREQ 'Fields with --quote-email are correct' '\n> \techo \"$cc_adr\" | grep cc1@example.com\n> '\n> \n> +test_expect_success $PREREQ 'correct quoted message with --quote-email' '\n> +\tgrep \"On Sat, 12 Jun 2010 15:53:58 +0200, author@example.com wrote:\" msgtxt1 &&\n> +\tgrep \"> Have you seen my previous email?\" msgtxt1 &&\n> +\tgrep \">> Previous content\" msgtxt1\n> +'\n\nWhen the spec says \"if --compose ... then ...\", \"after the triple-dash\",\nand \"in the first patch\", one would expect at least one test with\n--compose and one without, something to check that the insertion was\ndone below the triple-dash, and one test with two patches, checking that\nthe second patch is not altered by --quote-email.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"287753","messageId":"6f76c691-c822-a9bc-4568-819e4ff31491@grenoble-inp.org","threadId":"42426","inReplyTo":"vpqlh2ujkg8.fsf@anie.imag.fr","subject":"Re: [RFC-PATCH v2 2/2] send-email: quote-email quotes the message body","fromName":"Tom Russello","fromEmail":"tom.russello@grenoble-inp.org","sentAt":"2016-05-29T11:41:14Z","receivedAt":"2016-05-29T11:41:14Z","isPatch":true,"sender":{"key":"tom.russello@grenoble-inp.org","avatar":null},"body":"On 05/28/16 17:01, Matthieu Moy wrote:\n>> Currently, `send-email` without `--compose` implies `--annotate`.\n>\n> I don't get it. Did you mean s/without/with/? Even if so, this is not\n> exactly true: \"git send-email --compose -1\" will open the editor only\n> for the cover-letter, while adding --annotate will also open it for the\n> patch.\n\nWe meant that the default behavior of `--quote-email` (i.e. without\n--compose enabled) will open the editor with the given patches in\nargument and will quote the message body in the first one.\n\n> (Note: we discussed this off-list already, but I'll try to summarize my\n> thoughts here)\n>\n> I don't have strong opinion on this, but I think there's a difference\n> between launching the editor directly on the input patch files\n> (resulting in _user_'s edit being done directly on them) and having the\n> script modify it in-place (resulting in automatic changes done directly\n> on the user's files).\n>\n> I usually use \"git send-email\" directly without using \"git\n> format-patch\", so I'm not the best juge. But I can imagine a flow like\n>\n> 1) run \"git send-email *.patch\"\n>\n> 2) start editting\n>\n> 3) notice there's something wrong, give up for now (answer 'q' when git\n> send-email prompts for confirmation, or kill it via Control-C in a\n> terminal)\n>\n> 4) run \"git send-email *.patch\" again\n>\n> 5) be happy that changes done at 2) are still there.\n>\n> With --quote-email, it's different. The scenario above would result in\n>\n> 5') WTF, why is the email quoted twice?\n\nActually the Control-C during the edition will cancel all the\nannotations written (including the cited email).\n\n> Unfortunately, I don't really have a solution for this. My first thought\n> was that we should copy the files to a temporary location before\n> starting the editor (that what I'm used to when using \"git send-email\"\n> without \"git format-patch\"), but that would prevent 5) above.\n\nIt's already what we did: the first original patch is copied in a\ntemporary file. However, if the edition went well (i.e. the editor\nclosed by the user), the temporary file will erase the original one.\n\n>> @@ -109,7 +109,10 @@ is not set, this will be prompted for.\n>> --quote-email=::\n>> \tReply to the given email and automatically populate the \"To:\",\n\"Cc:\" and\n>> \t\"In-Reply-To:\" fields. If `--compose` is set, this will also fill the\n>> -\tsubject field with \"Re: ['s subject]\".\n>> +\tsubject field with \"Re: ['s subject]\" and quote the\nmessage body\n>> +\tof .\n>\n> I'd add \"in the introductory message\".\n\nAgreed.\n\n>> +\twhile (<$fh>) {\n>> +\t\t# Only for files containing crlf line endings\n>> +\t\ts/\\r//g;\n>\n> The comment doesn't really say what it does.\n>\n> What about \"turn crlf line endings into lf-only\"?\n\nYes, I completely agree this suggestion.\n\n> When writing comment, always try to ask the question \"why?\" more than\n> \"what?\". This part is possibly controversial, think about a contributor\n> finding this piece of code later without having followed the current\n> conversation. He'd probably expect an explanation about why you need a\n> temp file here and not elsewhere.\n\nThank you for the advice, I'll keep it in mind.\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> No space before :.\n\nSorry, I copied the previous error messages.\n\n> When the spec says \"if --compose ... then ...\", \"after the triple-dash\",\n> and \"in the first patch\", one would expect at least one test with\n> --compose and one without, something to check that the insertion was\n> done below the triple-dash, and one test with two patches, checking that\n> the second patch is not altered by --quote-email.\n\nYes, indeed. I'll add these tests in the next version.\n\nThank you for the review.\n"},{"id":"287766","messageId":"16f3b6f8-0a4c-3102-8723-e4dfacc51906@grenoble-inp.org","threadId":"42426","inReplyTo":"vpqk2iel081.fsf@anie.imag.fr","subject":"Re: [RFC-PATCH v2 1/2] send-email: quote-email populates the fields","fromName":"Tom Russello","fromEmail":"tom.russello@grenoble-inp.org","sentAt":"2016-05-29T23:38:02Z","receivedAt":"2016-05-29T23:38:02Z","isPatch":true,"sender":{"key":"tom.russello@grenoble-inp.org","avatar":null},"body":"On 05/28/16 16:35, Matthieu Moy wrote:\n>> +--quote-email=::\n>> +\tReply to the given email and automatically populate the \"To:\",\n\"Cc:\" and\n>> +\t\"In-Reply-To:\" fields.\n>\n> I think this is a bit too technical for a user documentation. To: and\n> Cc: is OK, but people need not know about \"In-Reply-To:\" to understand\n> this. See what the doc of --in-reply-to says. If you want to be\n> technical, you'd need to mention the References: field too.\n\nYou have a point here. Maybe, we can explain that the `--quote-email`\noption behaves like a mailer when replying to someone without getting\ninto details.\n\n> Talking about Reference: field, something your patch could do is to add\n> all references in to the references of the new email (see\n> what a mailer is doing when replying). This way, the recipient can still\n> get threading if the last message being replied-to is missing.\n\nI didn't know about this field, it looks like it appends all the\nparent message ID's.\n\n>> +\"Re: ['s subject]\".\n>\n> Perhaps `Re: ...` instead of double-quotes.\n\nAgreed.\n\n>> +if ($quote_email) {\n>> +\tmy $error = validate_patch($quote_email);\n>> +\t$error and die \"fatal: $quote_email: $error\\nwarning: no patches\nwere sent\\n\";\n>\n> I know it's done this way elsewhere, but I don't like this \"$error and\n> die\", I'd rather see a proper if here.\n\nYou're right, I'll change that in the next version.\n\n>> +\t\tif (defined $input_format && $input_format eq 'mbox') {\n>\n> To me, the input format refers to patch files, not the .\n>\n> I'm not sure anyone still use the \"lots of email\" format, and you are\n> not testing it. So, this is claiming that we have a feature without\n> being sure we have it nor that anyone's ever going to use it.\n\nYou summed up the situation well.\n\n> I'd just drop this \"if\" and the \"else\" branch, and just assume the email\n> file is a normal email file.\n\nI'll do that.\n\n\nThank you for the review.\n"},{"id":"288601","messageId":"20160607140148.23242-1-tom.russello@grenoble-inp.org","threadId":"42426","inReplyTo":"1464369102-7551-1-git-send-email-tom.russello@grenoble-inp.org","subject":"[PATCH v3 0/6] send-email: cleaner tests and quote email","fromName":"Tom Russello","fromEmail":"tom.russello@grenoble-inp.org","sentAt":"2016-06-07T14:01:42Z","receivedAt":"2016-06-07T14:01:42Z","isPatch":true,"sender":{"key":"tom.russello@grenoble-inp.org","avatar":null},"body":"The purpose of this series of patches is to implement a new\n\"quote-email\" feature integrated in the current `--in-reply-to` option.\n\n\n * The first 2 patches make the tests less dependent to\n `git send-email`'s exact output.\n\n * Third patch makes `git send-email` a bit less verbose.\n\n * Fourth patch introduces our email parser subroutine.\n\n * Fifth patch makes the `--in-reply-to` open a email file (if it\n exists) and populates From:, To:, Cc:, In-reply-to and\n References: fields.\n\n * Sixth patch quotes the message body in the cover letter if\n `--compose` is set. Else, imply `--annotate` and insert quoted\n message body below triple-dash in the first patch.\n\n\nGeneral changes since v2:\n - Modify tests to be less dependent on `git send-email`'s exact\n output.\n - New email parser subroutine.\n - `--quote-email` option is now merged with `--in-reply-to`.\n - Add `--cite` option to quote the message body.\n - `git send-email` is less verbose.\n\n Documentation/git-send-email.txt | 17 +++++--\n git-send-email.perl | 178 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------\n t/t9001-send-email.sh | 238 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------\n 3 files changed, 356 insertions(+), 77 deletions(-)\n"},{"id":"288602","messageId":"20160607140148.23242-2-tom.russello@grenoble-inp.org","threadId":"42426","inReplyTo":"20160607140148.23242-1-tom.russello@grenoble-inp.org","subject":"[PATCH v3 1/6] t9001: non order-sensitive file comparison","fromName":"Tom Russello","fromEmail":"tom.russello@grenoble-inp.org","sentAt":"2016-06-07T14:01:43Z","receivedAt":"2016-06-07T14:01:43Z","isPatch":true,"sender":{"key":"tom.russello@grenoble-inp.org","avatar":null},"body":"Tests might fail if lines compared in text files don't have the same order.\n\nSigned-off-by: Samuel GROOT \nSigned-off-by: Tom RUSSELLO \nSigned-off-by: Matthieu MOY \n---\n t/t9001-send-email.sh | 61 ++++++++++++++++++++++++++++-----------------------\n 1 file changed, 34 insertions(+), 27 deletions(-)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex b3355d2..4558e0f 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -54,6 +54,13 @@ test_no_confirm () {\n \t\t>no_confirm_okay\n }\n \n+# Check if two files have the same content, non-order sensitive\n+test_cmp_noorder () {\n+\tsort $1 >$1;\n+\tsort $2 >$2;\n+\treturn $(test_cmp $1 $2)\n+}\n+\n # Exit immediately to prevent hang if a no-confirm test fails\n check_no_confirm () {\n \tif ! test -f no_confirm_okay\n@@ -97,7 +104,7 @@ test_expect_success $PREREQ 'setup expect' '\n '\n \n test_expect_success $PREREQ 'Verify commandline' '\n-\ttest_cmp expected commandline1\n+\ttest_cmp_noorder expected commandline1\n '\n \n test_expect_success $PREREQ 'Send patches with --envelope-sender' '\n@@ -117,7 +124,7 @@ test_expect_success $PREREQ 'setup expect' '\n '\n \n test_expect_success $PREREQ 'Verify commandline' '\n-\ttest_cmp expected commandline1\n+\ttest_cmp_noorder expected commandline1\n '\n \n test_expect_success $PREREQ 'Send patches with --envelope-sender=auto' '\n@@ -137,7 +144,7 @@ test_expect_success $PREREQ 'setup expect' '\n '\n \n test_expect_success $PREREQ 'Verify commandline' '\n-\ttest_cmp expected commandline1\n+\ttest_cmp_noorder expected commandline1\n '\n \n test_expect_success $PREREQ 'setup expect' \"\n@@ -196,7 +203,7 @@ test_suppress_self () {\n \t>\"expected-no-cc-$3\" &&\n \n \t(grep '^Cc:' msghdr1-$3 >\"actual-no-cc-$3\";\n-\t test_cmp expected-no-cc-$3 actual-no-cc-$3)\n+\t test_cmp_noorder expected-no-cc-$3 actual-no-cc-$3)\n }\n \n test_suppress_self_unquoted () {\n@@ -269,7 +276,7 @@ test_expect_success $PREREQ 'Show all headers' '\n \t\t-e \"s/^\\(Message-Id:\\).*/\\1 MESSAGE-ID-STRING/\" \\\n \t\t-e \"s/^\\(X-Mailer:\\).*/\\1 X-MAILER-STRING/\" \\\n \t\t>actual-show-all-headers &&\n-\ttest_cmp expected-show-all-headers actual-show-all-headers\n+\ttest_cmp_noorder expected-show-all-headers actual-show-all-headers\n '\n \n test_expect_success $PREREQ 'Prompting works' '\n@@ -436,13 +443,13 @@ test_expect_success $PREREQ 'In-Reply-To without --chain-reply-to' '\n \t\t2>errors &&\n \t# The first message is a reply to --in-reply-to\n \tsed -n -e \"s/^In-Reply-To: *\\(.*\\)/\\1/p\" msgtxt1 >actual &&\n-\ttest_cmp expect actual &&\n+\ttest_cmp_noorder expect actual &&\n \t# Second and subsequent messages are replies to the first one\n \tsed -n -e \"s/^Message-Id: *\\(.*\\)/\\1/p\" msgtxt1 >expect &&\n \tsed -n -e \"s/^In-Reply-To: *\\(.*\\)/\\1/p\" msgtxt2 >actual &&\n-\ttest_cmp expect actual &&\n+\ttest_cmp_noorder expect actual &&\n \tsed -n -e \"s/^In-Reply-To: *\\(.*\\)/\\1/p\" msgtxt3 >actual &&\n-\ttest_cmp expect actual\n+\ttest_cmp_noorder expect actual\n '\n \n test_expect_success $PREREQ 'In-Reply-To with --chain-reply-to' '\n@@ -457,13 +464,13 @@ test_expect_success $PREREQ 'In-Reply-To with --chain-reply-to' '\n \t\t$patches $patches $patches \\\n \t\t2>errors &&\n \tsed -n -e \"s/^In-Reply-To: *\\(.*\\)/\\1/p\" msgtxt1 >actual &&\n-\ttest_cmp expect actual &&\n+\ttest_cmp_noorder expect actual &&\n \tsed -n -e \"s/^Message-Id: *\\(.*\\)/\\1/p\" msgtxt1 >expect &&\n \tsed -n -e \"s/^In-Reply-To: *\\(.*\\)/\\1/p\" msgtxt2 >actual &&\n-\ttest_cmp expect actual &&\n+\ttest_cmp_noorder expect actual &&\n \tsed -n -e \"s/^Message-Id: *\\(.*\\)/\\1/p\" msgtxt2 >expect &&\n \tsed -n -e \"s/^In-Reply-To: *\\(.*\\)/\\1/p\" msgtxt3 >actual &&\n-\ttest_cmp expect actual\n+\ttest_cmp_noorder expect actual\n '\n \n test_expect_success $PREREQ 'setup fake editor' '\n@@ -537,7 +544,7 @@ test_suppression () {\n \t\t--smtp-server relay.example.com \\\n \t\t$patches | replace_variable_fields \\\n \t\t>actual-suppress-$1${2+\"-$2\"} &&\n-\ttest_cmp expected-suppress-$1${2+\"-$2\"} actual-suppress-$1${2+\"-$2\"}\n+\ttest_cmp_noorder expected-suppress-$1${2+\"-$2\"} actual-suppress-$1${2+\"-$2\"}\n }\n \n test_expect_success $PREREQ 'sendemail.cc set' '\n@@ -1213,7 +1220,7 @@ test_expect_success $PREREQ 'ASCII subject is not RFC2047 quoted' '\n \t\t\t--8bit-encoding=UTF-8 \\\n \t\t\temail-using-8bit >stdout &&\n \tgrep \"Subject\" msgtxt1 >actual &&\n-\ttest_cmp expected actual\n+\ttest_cmp_noorder expected actual\n '\n \n test_expect_success $PREREQ 'setup expect' '\n@@ -1234,7 +1241,7 @@ test_expect_success $PREREQ 'asks about and fixes 8bit encodings' '\n \tgrep email-using-8bit stdout &&\n \tgrep \"Which 8bit encoding\" stdout &&\n \tegrep \"Content|MIME\" msgtxt1 >actual &&\n-\ttest_cmp actual content-type-decl\n+\ttest_cmp_noorder actual content-type-decl\n '\n \n test_expect_success $PREREQ 'sendemail.8bitEncoding works' '\n@@ -1245,7 +1252,7 @@ test_expect_success $PREREQ 'sendemail.8bitEncoding works' '\n \t\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n \t\t\temail-using-8bit >stdout &&\n \tegrep \"Content|MIME\" msgtxt1 >actual &&\n-\ttest_cmp actual content-type-decl\n+\ttest_cmp_noorder actual content-type-decl\n '\n \n test_expect_success $PREREQ '--8bit-encoding overrides sendemail.8bitEncoding' '\n@@ -1257,7 +1264,7 @@ test_expect_success $PREREQ '--8bit-encoding overrides sendemail.8bitEncoding' '\n \t\t\t--8bit-encoding=UTF-8 \\\n \t\t\temail-using-8bit >stdout &&\n \tegrep \"Content|MIME\" msgtxt1 >actual &&\n-\ttest_cmp actual content-type-decl\n+\ttest_cmp_noorder actual content-type-decl\n '\n \n test_expect_success $PREREQ 'setup expect' '\n@@ -1286,7 +1293,7 @@ test_expect_success $PREREQ '--8bit-encoding also treats subject' '\n \t\t\t--8bit-encoding=UTF-8 \\\n \t\t\temail-using-8bit >stdout &&\n \tgrep \"Subject\" msgtxt1 >actual &&\n-\ttest_cmp expected actual\n+\ttest_cmp_noorder expected actual\n '\n \n test_expect_success $PREREQ 'setup expect' '\n@@ -1335,7 +1342,7 @@ test_expect_success $PREREQ 'sendemail.transferencoding=8bit' '\n \t\t2>errors >out &&\n \tsed '1,/^$/d' msgtxt1 >actual &&\n \tsed '1,/^$/d' email-using-8bit >expected &&\n-\ttest_cmp expected actual\n+\ttest_cmp_noorder expected actual\n '\n \n test_expect_success $PREREQ 'setup expect' '\n@@ -1352,7 +1359,7 @@ test_expect_success $PREREQ '8-bit and sendemail.transferencoding=quoted-printab\n \t\temail-using-8bit \\\n \t\t2>errors >out &&\n \tsed '1,/^$/d' msgtxt1 >actual &&\n-\ttest_cmp expected actual\n+\ttest_cmp_noorder expected actual\n '\n \n test_expect_success $PREREQ 'setup expect' '\n@@ -1369,7 +1376,7 @@ test_expect_success $PREREQ '8-bit and sendemail.transferencoding=base64' '\n \t\temail-using-8bit \\\n \t\t2>errors >out &&\n \tsed '1,/^$/d' msgtxt1 >actual &&\n-\ttest_cmp expected actual\n+\ttest_cmp_noorder expected actual\n '\n \n test_expect_success $PREREQ 'setup expect' '\n@@ -1395,7 +1402,7 @@ test_expect_success $PREREQ 'convert from quoted-printable to base64' '\n \t\temail-using-qp \\\n \t\t2>errors >out &&\n \tsed '1,/^$/d' msgtxt1 >actual &&\n-\ttest_cmp expected actual\n+\ttest_cmp_noorder expected actual\n '\n \n test_expect_success $PREREQ 'setup expect' \"\n@@ -1425,7 +1432,7 @@ test_expect_success $PREREQ 'CRLF and sendemail.transferencoding=quoted-printabl\n \t\temail-using-crlf \\\n \t\t2>errors >out &&\n \tsed '1,/^$/d' msgtxt1 >actual &&\n-\ttest_cmp expected actual\n+\ttest_cmp_noorder expected actual\n '\n \n test_expect_success $PREREQ 'setup expect' '\n@@ -1442,7 +1449,7 @@ test_expect_success $PREREQ 'CRLF and sendemail.transferencoding=base64' '\n \t\temail-using-crlf \\\n \t\t2>errors >out &&\n \tsed '1,/^$/d' msgtxt1 >actual &&\n-\ttest_cmp expected actual\n+\ttest_cmp_noorder expected actual\n '\n \n \n@@ -1582,7 +1589,7 @@ test_dump_aliases () {\n \t\t\t\"$(pwd)/.tmp-email-aliases\" &&\n \t\tgit config sendemail.aliasfiletype \"$filetype\" &&\n \t\tgit send-email --dump-aliases 2>errors >actual &&\n-\t\ttest_cmp expect actual\n+\t\ttest_cmp_noorder expect actual\n \t'\n }\n \n@@ -1842,7 +1849,7 @@ test_expect_success $PREREQ 'use email list in --cc --to and --bcc' '\n \t--bcc=\"bcc1@example.com, bcc2@example.com\" \\\n \t0001-add-master.patch | replace_variable_fields \\\n \t>actual-list &&\n-\ttest_cmp expected-list actual-list\n+\ttest_cmp_noorder expected-list actual-list\n '\n \n test_expect_success $PREREQ 'aliases work with email list' '\n@@ -1858,7 +1865,7 @@ test_expect_success $PREREQ 'aliases work with email list' '\n \t--bcc=\"bcc1@example.com, bcc2@example.com\" \\\n \t0001-add-master.patch | replace_variable_fields \\\n \t>actual-list &&\n-\ttest_cmp expected-list actual-list\n+\ttest_cmp_noorder expected-list actual-list\n '\n \n test_expect_success $PREREQ 'leading and trailing whitespaces are removed' '\n@@ -1882,7 +1889,7 @@ test_expect_success $PREREQ 'leading and trailing whitespaces are removed' '\n \t--bcc=\"bcc2@example.com\" \\\n \t0001-add-master.patch | replace_variable_fields \\\n \t>actual-list &&\n-\ttest_cmp expected-list actual-list\n+\ttest_cmp_noorder expected-list actual-list\n '\n \n test_done\n-- \n2.8.3\n"},{"id":"288603","messageId":"20160607140148.23242-3-tom.russello@grenoble-inp.org","threadId":"42426","inReplyTo":"20160607140148.23242-1-tom.russello@grenoble-inp.org","subject":"[PATCH v3 2/6] t9001: check email address is in Cc: field","fromName":"Tom Russello","fromEmail":"tom.russello@grenoble-inp.org","sentAt":"2016-06-07T14:01:44Z","receivedAt":"2016-06-07T14:01:44Z","isPatch":true,"sender":{"key":"tom.russello@grenoble-inp.org","avatar":null},"body":"Check if the given utf-8 email address is in the Cc: field.\n\nSigned-off-by: Tom RUSSELLO \nSigned-off-by: Samuel GROOT \nSigned-off-by: Matthieu MOY \n---\n t/t9001-send-email.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 4558e0f..7fdc876 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -888,8 +888,8 @@ test_expect_success $PREREQ 'utf8 Cc is rfc2047 encoded' '\n \t--to=nobody@example.com \\\n \t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n \toutdir/*.patch &&\n-\tgrep \"^\t\" msgtxt1 |\n-\tgrep \"=?UTF-8?q?=C3=A0=C3=A9=C3=AC=C3=B6=C3=BA?= \"\n+\tcc_adr=$(awk \"/^Cc: /{flag=1}/^Subject: /{flag=0} flag {print}\" msgtxt1) &&\n+\techo \"$cc_adr\" | grep \"=?UTF-8?q?=C3=A0=C3=A9=C3=AC=C3=B6=C3=BA?= \"\n '\n \n test_expect_success $PREREQ '--compose adds MIME for utf8 body' '\n-- \n2.8.3\n"},{"id":"288604","messageId":"20160607140148.23242-4-tom.russello@grenoble-inp.org","threadId":"42426","inReplyTo":"20160607140148.23242-1-tom.russello@grenoble-inp.org","subject":"[PATCH v3 3/6] t9001: shorten send-email's output","fromName":"Tom Russello","fromEmail":"tom.russello@grenoble-inp.org","sentAt":"2016-06-07T14:01:45Z","receivedAt":"2016-06-07T14:01:45Z","isPatch":true,"sender":{"key":"tom.russello@grenoble-inp.org","avatar":null},"body":"Messages displayed by `send-email` should be shortened to avoid displaying\nunnecesseray informations.\n\nSigned-off-by: Samuel GROOT \nSigned-off-by: Tom RUSSELLO \nSigned-off-by: Matthieu MOY \n---\n git-send-email.perl | 22 +++++++++----------\n t/t9001-send-email.sh | 58 +++++++++++++++++++++++++--------------------------\n 2 files changed, 40 insertions(+), 40 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 6958785..4822f41 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1478,14 +1478,14 @@ foreach my $t (@files) {\n \t\t\t\t$sauthor = sanitize_address($author);\n \t\t\t\tnext if $suppress_cc{'author'};\n \t\t\t\tnext if $suppress_cc{'self'} and $sauthor eq $sender;\n-\t\t\t\tprintf(\"(mbox) Adding cc: %s from line '%s'\\n\",\n-\t\t\t\t\t$1, $_) unless $quiet;\n+\t\t\t\tprintf(\"Adding cc: %s from From: header\\n\",\n+\t\t\t\t\t$1) unless $quiet;\n \t\t\t\tpush @cc, $1;\n \t\t\t}\n \t\t\telsif (/^To:\\s+(.*)$/i) {\n \t\t\t\tforeach my $addr (parse_address_line($1)) {\n-\t\t\t\t\tprintf(\"(mbox) Adding to: %s from line '%s'\\n\",\n-\t\t\t\t\t\t$addr, $_) unless $quiet;\n+\t\t\t\t\tprintf(\"Adding to: %s from To: header\\n\",\n+\t\t\t\t\t\t$addr) unless $quiet;\n \t\t\t\t\tpush @to, $addr;\n \t\t\t\t}\n \t\t\t}\n@@ -1498,8 +1498,8 @@ foreach my $t (@files) {\n \t\t\t\t\t} else {\n \t\t\t\t\t\tnext if ($suppress_cc{'cc'});\n \t\t\t\t\t}\n-\t\t\t\t\tprintf(\"(mbox) Adding cc: %s from line '%s'\\n\",\n-\t\t\t\t\t\t$addr, $_) unless $quiet;\n+\t\t\t\t\tprintf(\"Adding cc: %s from Cc: header\\n\",\n+\t\t\t\t\t\t$addr) unless $quiet;\n \t\t\t\t\tpush @cc, $addr;\n \t\t\t\t}\n \t\t\t}\n@@ -1532,8 +1532,8 @@ foreach my $t (@files) {\n \t\t\t# So let's support that, too.\n \t\t\t$input_format = 'lots';\n \t\t\tif (@cc == 0 && !$suppress_cc{'cc'}) {\n-\t\t\t\tprintf(\"(non-mbox) Adding cc: %s from line '%s'\\n\",\n-\t\t\t\t\t$_, $_) unless $quiet;\n+\t\t\t\tprintf(\"Adding cc: %s from Cc: header\\n\",\n+\t\t\t\t\t$_) unless $quiet;\n \t\t\t\tpush @cc, $_;\n \t\t\t} elsif (!defined $subject) {\n \t\t\t\t$subject = $_;\n@@ -1555,8 +1555,8 @@ foreach my $t (@files) {\n \t\t\t\tnext if $suppress_cc{'bodycc'} and $what =~ /Cc/i;\n \t\t\t}\n \t\t\tpush @cc, $c;\n-\t\t\tprintf(\"(body) Adding cc: %s from line '%s'\\n\",\n-\t\t\t\t$c, $_) unless $quiet;\n+\t\t\tprintf(\"Adding cc: %s from Signed-off-by trailer\\n\",\n+\t\t\t\t$c) unless $quiet;\n \t\t}\n \t}\n \tclose $fh;\n@@ -1660,7 +1660,7 @@ sub recipients_cmd {\n \t\t$address = sanitize_address($address);\n \t\tnext if ($address eq $sender and $suppress_cc{'self'});\n \t\tpush @addresses, $address;\n-\t\tprintf(\"($prefix) Adding %s: %s from: '%s'\\n\",\n+\t\tprintf(\"Adding %s: %s from: '%s'\\n\",\n \t\t $what, $address, $cmd) unless $quiet;\n \t\t}\n \tclose $fh\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 7fdc876..9b1e56f 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -150,9 +150,9 @@ test_expect_success $PREREQ 'Verify commandline' '\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-show-all-headers <<\\EOF\n 0001-Second.patch\n-(mbox) Adding cc: A from line 'From: A '\n-(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n-(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n+Adding cc: A from From: header\n+Adding cc: One from Cc: header\n+Adding cc: two@example.com from Cc: header\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n@@ -503,9 +503,9 @@ test_expect_success $PREREQ 'second message is patch' '\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-suppress-sob <<\\EOF\n 0001-Second.patch\n-(mbox) Adding cc: A from line 'From: A '\n-(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n-(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n+Adding cc: A from From: header\n+Adding cc: One from Cc: header\n+Adding cc: two@example.com from Cc: header\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n@@ -555,9 +555,9 @@ test_expect_success $PREREQ 'sendemail.cc set' '\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-suppress-sob <<\\EOF\n 0001-Second.patch\n-(mbox) Adding cc: A from line 'From: A '\n-(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n-(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n+Adding cc: A from From: header\n+Adding cc: One from Cc: header\n+Adding cc: two@example.com from Cc: header\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n@@ -587,10 +587,10 @@ test_expect_success $PREREQ 'sendemail.cc unset' '\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-suppress-cccmd <<\\EOF\n 0001-Second.patch\n-(mbox) Adding cc: A from line 'From: A '\n-(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n-(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n-(body) Adding cc: C O Mitter from line 'Signed-off-by: C O Mitter '\n+Adding cc: A from From: header\n+Adding cc: One from Cc: header\n+Adding cc: two@example.com from Cc: header\n+Adding cc: C O Mitter from Signed-off-by: trailer\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n@@ -647,10 +647,10 @@ test_expect_success $PREREQ '--suppress-cc=all' '\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-suppress-body <<\\EOF\n 0001-Second.patch\n-(mbox) Adding cc: A from line 'From: A '\n-(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n-(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n-(cc-cmd) Adding cc: cc-cmd@example.com from: './cccmd'\n+Adding cc: A from From: header\n+Adding cc: One from Cc: header\n+Adding cc: two@example.com from Cc: header\n+Adding cc: cc-cmd@example.com from: './cccmd'\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n@@ -681,9 +681,9 @@ test_expect_success $PREREQ '--suppress-cc=body' '\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-suppress-body-cccmd <<\\EOF\n 0001-Second.patch\n-(mbox) Adding cc: A from line 'From: A '\n-(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n-(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n+Adding cc: A from From: header\n+Adding cc: One from Cc: header\n+Adding cc: two@example.com from Cc: header\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n@@ -712,9 +712,9 @@ test_expect_success $PREREQ '--suppress-cc=body --suppress-cc=cccmd' '\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-suppress-sob <<\\EOF\n 0001-Second.patch\n-(mbox) Adding cc: A from line 'From: A '\n-(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n-(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n+Adding cc: A from From: header\n+Adding cc: One from Cc: header\n+Adding cc: two@example.com from Cc: header\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n@@ -744,10 +744,10 @@ test_expect_success $PREREQ '--suppress-cc=sob' '\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-suppress-bodycc <<\\EOF\n 0001-Second.patch\n-(mbox) Adding cc: A from line 'From: A '\n-(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n-(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n-(body) Adding cc: C O Mitter from line 'Signed-off-by: C O Mitter '\n+Adding cc: A from From: header\n+Adding cc: One from Cc: header\n+Adding cc: two@example.com from Cc: header\n+Adding cc: C O Mitter from Signed-off-by: trailer\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n@@ -778,8 +778,8 @@ test_expect_success $PREREQ '--suppress-cc=bodycc' '\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-suppress-cc <<\\EOF\n 0001-Second.patch\n-(mbox) Adding cc: A from line 'From: A '\n-(body) Adding cc: C O Mitter from line 'Signed-off-by: C O Mitter '\n+Adding cc: A from From: header\n+Adding cc: C O Mitter from Signed-off-by: trailer\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n-- \n2.8.3\n"},{"id":"288605","messageId":"20160607140148.23242-5-tom.russello@grenoble-inp.org","threadId":"42426","inReplyTo":"20160607140148.23242-1-tom.russello@grenoble-inp.org","subject":"[PATCH v3 4/6] send-email: create email parser subroutine","fromName":"Tom Russello","fromEmail":"tom.russello@grenoble-inp.org","sentAt":"2016-06-07T14:01:46Z","receivedAt":"2016-06-07T14:01:46Z","isPatch":true,"sender":{"key":"tom.russello@grenoble-inp.org","avatar":null},"body":"We need a simple and generic way to parse an email file.\n\nSince it would be hard to include and maintain an external library,\ncreate an simple email parser subroutine to parse an email file.\n\nSigned-off-by: Samuel GROOT \nSigned-off-by: Tom RUSSELLO \nSigned-off-by: Matthieu MOY \n---\nWe chose to create our own simple email parser and only use it for the\n\"quote email\" feature to pave the way for the refactorization of the patch\nparser [0] that may come after our current school project.\n\n[0] * http://thread.gmane.org/gmane.comp.version-control.git/295752\n\n git-send-email.perl | 28 ++++++++++++++++++++++++++++\n 1 file changed, 28 insertions(+)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 4822f41..db114ae 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1750,3 +1750,31 @@ sub body_or_subject_has_nonascii {\n \t}\n \treturn 0;\n }\n+\n+sub parse_email {\n+ my %mail = ();\n+ my $fh = shift;\n+ my $last_header;\n+\n+ # Unfold and parse multiline header fields\n+ while (<$fh>) {\n+ last if /^\\s*$/;\n+ s/\\r\\n|\\n|\\r//;\n+ if (/^([^\\s:]+):[\\s]+(.*)$/) {\n+ $last_header = lc($1);\n+ @{$mail{$last_header}} = ()\n+ unless defined $mail{$last_header};\n+ push @{$mail{$last_header}}, $2;\n+ } elsif (/^\\s+\\S/ and defined $last_header) {\n+ s/^\\s+/ /;\n+ push @{$mail{$last_header}}, $_;\n+ } else {\n+ die(\"Mail format undefined !\\n\");\n+ }\n+ }\n+\n+ # Separate body from header\n+ $mail{\"body\"} = [(<$fh>)];\n+\n+ return \\%mail;\n+}\n-- \n2.8.3\n"},{"id":"288606","messageId":"20160607140519.23418-1-tom.russello@grenoble-inp.org","threadId":"42426","inReplyTo":"20160607140148.23242-5-tom.russello@grenoble-inp.org","subject":"[PATCH v3 5/6] send-email: --in-reply-to= populates the fields","fromName":"Tom Russello","fromEmail":"tom.russello@grenoble-inp.org","sentAt":"2016-06-07T14:05:18Z","receivedAt":"2016-06-07T14:05:18Z","isPatch":true,"sender":{"key":"tom.russello@grenoble-inp.org","avatar":null},"body":"Take an email message file, parse it and fill the \"From\", \"To\", \"Cc\",\n\"In-reply-to\", \"References\" fields appropriately.\n\nIf `--compose` option is set, it will also fill the subject field with\n`Re: ['s subject]` in the introductory message.\n\nSigned-off-by: Tom RUSSELLO \nSigned-off-by: Samuel GROOT \nSigned-off-by: Matthieu MOY \n---\nCheck if the string given by argument with `--in-reply-to` leads to\nan existing plain text file. If not, consider it as a message-id.\n\nChanges sinces v2:\n\t- Fill the References: field to keep the thread even if some\n\t emails have been removed\n\t- Explicit error with a proper \"if\" when an error occured during\n\t email file opening\n\t- More precise comments\n\t- More tests\n\n Documentation/git-send-email.txt | 9 +++--\n git-send-email.perl | 49 +++++++++++++++++++++++-\n t/t9001-send-email.sh | 83 ++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 136 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex edbba3a..21776f0 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -84,13 +84,16 @@ See the CONFIGURATION section for 'sendemail.multiEdit'.\n \tthe value of GIT_AUTHOR_IDENT, or GIT_COMMITTER_IDENT if that is not\n \tset, as returned by \"git var -l\".\n \n---in-reply-to=::\n+--in-reply-to=::\n \tMake the first mail (or all the mails with `--no-thread`) appear as a\n-\treply to the given Message-Id, which avoids breaking threads to\n-\tprovide a new patch series.\n+\treply to the given Message-Id (given directly by argument or via the email\n+\tfile), which avoids breaking threads to provide a new patch series.\n \tThe second and subsequent emails will be sent as replies according to\n \tthe `--[no]-chain-reply-to` setting.\n +\n+Furthermore, if the argument is an email file, parse it and populate header\n+fields appropriately for the reply.\n++\n So for example when `--thread` and `--no-chain-reply-to` are specified, the\n second and subsequent patches will be replies to the first one like in the\n illustration below where `[PATCH v2 0/3]` is in reply to `[PATCH 0/2]`:\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex db114ae..66aa2cd 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -55,6 +55,7 @@ git send-email --dump-aliases\n --[no-]bcc * Email Bcc:\n --subject * Email \"Subject:\"\n --in-reply-to * Email \"In-Reply-To:\"\n+ --in-reply-to * 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@@ -160,7 +161,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,$initial_subject,@files,\n \t$author,$sender,$smtp_authpass,$annotate,$use_xmailer,$compose,$time);\n \n my $envelope_sender;\n@@ -639,6 +640,50 @@ if (@files) {\n \tusage();\n }\n \n+if ($initial_reply_to && -f $initial_reply_to) {\n+\tmy $error = validate_patch($initial_reply_to);\n+\tdie \"fatal: $initial_reply_to: $error\\nwarning: no patches were sent\\n\"\n+\t\tif $error;\n+\n+\topen my $fh, \"<\", $initial_reply_to or die \"can't open file $initial_reply_to\";\n+\tmy $mail = parse_email($fh);\n+\tclose $fh;\n+\n+\tmy $initial_sender = $sender || $repoauthor || $repocommitter || '';\n+\n+\tmy $prefix_re = \"\";\n+\tmy $subject_re = $mail->{\"subject\"}[0];\n+\tif ($subject_re =~ /^[^Re:]/) {\n+\t\t$prefix_re = \"Re: \";\n+\t}\n+\t$initial_subject = $prefix_re . $subject_re;\n+\n+\tpush @initial_to, $mail->{\"from\"}[0];\n+\n+\tforeach my $to_addr (parse_address_line(join \",\", @{$mail->{\"to\"}})) {\n+\t\tif (!($to_addr eq $initial_sender)) {\n+\t\t\tpush @initial_cc, $to_addr;\n+\t\t}\n+\t}\n+\n+\tforeach my $cc_addr (parse_address_line(join \",\", @{$mail->{\"cc\"}})) {\n+\t\tmy $qaddr = unquote_rfc2047($cc_addr);\n+\t\tmy $saddr = sanitize_address($qaddr);\n+\t\tif ($saddr eq $initial_sender) {\n+\t\t\tnext if ($suppress_cc{'self'});\n+\t\t} else {\n+\t\t\tnext if ($suppress_cc{'cc'});\n+\t\t}\n+\t\tpush @initial_cc, $cc_addr;\n+\t}\n+\n+\t$initial_reply_to = $mail->{\"message-id\"}[0];\n+\tif ($mail->{\"references\"}) {\n+\t\t$initial_references = join(\"\", @{$mail->{\"references\"}}) .\n+\t\t\t\" \" . $initial_reply_to;\n+\t}\n+}\n+\n sub get_patch_subject {\n \tmy $fn = shift;\n \topen (my $fh, '<', $fn);\n@@ -1426,7 +1471,7 @@ Message-Id: $message_id\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 9b1e56f..2d67f6d 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1892,4 +1892,87 @@ test_expect_success $PREREQ 'leading and trailing whitespaces are removed' '\n \ttest_cmp_noorder expected-list actual-list\n '\n \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+\tHave you seen my previous email?\n+\t> Previous content\n+\tEOF\n+'\n+\n+test_expect_success $PREREQ 'Fields with --in-reply-to are correct' '\n+\tclean_fake_sendmail &&\n+\tgit send-email \\\n+\t\t--in-reply-to=email \\\n+\t\t--from=\"Example \" \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t-2 \\\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 --in-reply-to and --compose are correct' '\n+\tclean_fake_sendmail &&\n+\tgit send-email \\\n+\t\t--in-reply-to=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 --in-reply-to and --compose ' '\n+\tgit send-email \\\n+\t\t--in-reply-to=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.8.3\n"},{"id":"288607","messageId":"20160607140519.23418-2-tom.russello@grenoble-inp.org","threadId":"42426","inReplyTo":"20160607140519.23418-1-tom.russello@grenoble-inp.org","subject":"[PATCH v3 6/6] send-email: add option --cite to quote the message body","fromName":"Tom Russello","fromEmail":"tom.russello@grenoble-inp.org","sentAt":"2016-06-07T14:05:19Z","receivedAt":"2016-06-07T14:05:19Z","isPatch":true,"sender":{"key":"tom.russello@grenoble-inp.org","avatar":null},"body":"If used with `in-reply-to=`, cite the message body of the given\nemail file. Otherwise, do nothing.\n\nIf `--compose` is set, quote the message body in the cover letter. Else,\nimply `--annotate` by default and quote the message body below the triple-dash\nsection in the first patch only.\n\nSigned-off-by: Tom RUSSELLO \nSigned-off-by: Samuel GROOT \nSigned-off-by: Matthieu MOY \n---\n Documentation/git-send-email.txt | 8 ++++\n git-send-email.perl | 81 ++++++++++++++++++++++++++++++++++++++--\n t/t9001-send-email.sh | 32 ++++++++++++++++\n 3 files changed, 117 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex 21776f0..23cbd17 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -109,6 +109,14 @@ 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+--cite::\n+\tWhen used with `--in-reply-to=`, quote the message body of the\n+\tgiven email file.\n++\n+If `--compose` is also set, the message cited will be in the cover letter. If\n+`--compose` is not set, `--annotate` option is implied by default and the\n+message body will be cited in the \"below-triple-dash\" section.\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 66aa2cd..03483f5 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(catfile);\n+use File::Copy;\n use Error qw(:try);\n use Git;\n \n@@ -56,6 +57,8 @@ git send-email --dump-aliases\n --subject * Email \"Subject:\"\n --in-reply-to * Email \"In-Reply-To:\"\n --in-reply-to * Populate header fields appropriately.\n+ --cite * Quote the message body in the cover if\n+ --compose is set, else in the first patch.\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@@ -161,7 +164,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_references,$initial_subject,@files,\n+\t$initial_reply_to,$initial_references,$cite,$initial_subject,@files,\n \t$author,$sender,$smtp_authpass,$annotate,$use_xmailer,$compose,$time);\n \n my $envelope_sender;\n@@ -305,6 +308,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 \"cite\" => \\$cite,\n \t\t \"to=s\" => \\@initial_to,\n \t\t \"to-cmd=s\" => \\$to_cmd,\n \t\t \"no-to\" => \\$no_to,\n@@ -640,6 +644,7 @@ if (@files) {\n \tusage();\n }\n \n+my $message_cited;\n if ($initial_reply_to && -f $initial_reply_to) {\n \tmy $error = validate_patch($initial_reply_to);\n \tdie \"fatal: $initial_reply_to: $error\\nwarning: no patches were sent\\n\"\n@@ -658,7 +663,8 @@ if ($initial_reply_to && -f $initial_reply_to) {\n \t}\n \t$initial_subject = $prefix_re . $subject_re;\n \n-\tpush @initial_to, $mail->{\"from\"}[0];\n+\tmy $recipient = $mail->{\"from\"}[0];\n+\tpush @initial_to, $recipient;\n \n \tforeach my $to_addr (parse_address_line(join \",\", @{$mail->{\"to\"}})) {\n \t\tif (!($to_addr eq $initial_sender)) {\n@@ -682,6 +688,25 @@ if ($initial_reply_to && -f $initial_reply_to) {\n \t\t$initial_references = join(\"\", @{$mail->{\"references\"}}) .\n \t\t\t\" \" . $initial_reply_to;\n \t}\n+\n+\tif ($cite) {\n+\t\tmy $date = $mail->{\"date\"}[0];\n+\t\tmy $tpl_date = $date && \"On $date, \" || '';\n+\t\t$message_cited = $tpl_date . $recipient . \" wrote:\\n\";\n+\n+\t\t# Quote the message body\n+\t\tforeach (@{$mail->{\"body\"}}) {\n+\t\t\tmy $space = \"\";\n+\t\t\tif (/^[^>]/) {\n+\t\t\t\t$space = \" \";\n+\t\t\t}\n+\t\t\t$message_cited .= \">\" . $space . $_;\n+\t\t}\n+\n+\t\tif (!$compose) {\n+\t\t\t$annotate = 1;\n+\t\t}\n+\t}\n }\n \n sub get_patch_subject {\n@@ -709,6 +734,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_cited &&\n+\t\t\"\\nGIT: Please, trim down irrelevant sections in the cited message\\n\".\n+\t\t\"GIT: to keep your email concise.\\n\" . $message_cited || '';\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_cited_message($cite_email_filename, $files[0]);\n+\n+\t\tmy $tmp = $files[0];\n+\t\t$files[0] = $cite_email_filename;\n+\n+\t\tdo_edit(@files);\n+\n+\t\t# Erase the original patch if the edition went well\n+\t\tmove($cite_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_cited_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_cited;\n+\twhile (<$c>) {\n+\t\tprint $c2 $_;\n+\t}\n+\n+\tclose $c;\n+\tclose $c2;\n }\n \n sub ask {\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 2d67f6d..a107bde 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1915,6 +1915,7 @@ test_expect_success $PREREQ 'Fields with --in-reply-to are correct' '\n \tgit send-email \\\n \t\t--in-reply-to=email \\\n \t\t--from=\"Example \" \\\n+\t\t--cite \\\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n \t\t-2 \\\n \t\t2>errors &&\n@@ -1936,10 +1937,22 @@ test_expect_success $PREREQ 'Fields with --in-reply-to are correct' '\n \techo \"$ref_adr\" | grep -v \"References: \"\n '\n \n+test_expect_success $PREREQ 'correct cited message with --in-reply-to' '\n+\tmsg_cited=$(grep -A 3 \"^---$\" msgtxt1) &&\n+\techo \"$msg_cited\" | grep \"On Sat, 12 Jun 2010 15:53:58 +0200, author@example.com wrote:\" &&\n+\techo \"$msg_cited\" | grep \"> Have you seen my previous email?\" &&\n+\techo \"$msg_cited\" | grep \">> Previous content\"\n+'\n+\n+test_expect_success $PREREQ 'second patch body is not modified by --in-reply-to' '\n+\t! grep \"Have you seen my previous email?\" msgtxt2\n+'\n+\n test_expect_success $PREREQ 'Fields with --in-reply-to and --compose are correct' '\n \tclean_fake_sendmail &&\n \tgit send-email \\\n \t\t--in-reply-to=email \\\n+\t\t--cite \\\n \t\t--compose \\\n \t\t--from=\"Example \" \\\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n@@ -1967,6 +1980,7 @@ test_expect_success $PREREQ 'Fields with --in-reply-to and --compose are correct\n test_expect_success $PREREQ 'Re: written only once with --in-reply-to and --compose ' '\n \tgit send-email \\\n \t\t--in-reply-to=msgtxt1 \\\n+\t\t--cite \\\n \t\t--compose \\\n \t\t--from=\"Example \" \\\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n@@ -1975,4 +1989,22 @@ test_expect_success $PREREQ 'Re: written only once with --in-reply-to and --comp\n \tgrep \"Subject: Re: subject goes here\" msgtxt3\n '\n \n+test_expect_success $PREREQ 'correct cited message with --in-reply-to 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_expect_success $PREREQ 'Message is not cited with only --in-reply-to' '\n+\tclean_fake_sendmail &&\n+\tgit send-email \\\n+\t\t--in-reply-to=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+\t! grep \"Have you seen my previous email?\" msgtxt1\n+'\n+\n test_done\n-- \n2.8.3\n"},{"id":"288674","messageId":"xmqqziqwmqth.fsf@gitster.mtv.corp.google.com","threadId":"42426","inReplyTo":"20160607140148.23242-2-tom.russello@grenoble-inp.org","subject":"Re: [PATCH v3 1/6] t9001: non order-sensitive file comparison","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-08T01:07:06Z","receivedAt":"2016-06-08T01:07:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tom Russello writes:\n\n> +# Check if two files have the same content, non-order sensitive\n> +test_cmp_noorder () {\n> +\tsort $1 >$1;\n\nHere is what I think happens:\n\n 0) the shell parses this command line;\n 1) the shell notices that the output has to go to $1;\n 2) the shell does open(2) of $1,\n 3) the shell spawns \"sort\" with a single argument, with its\n output connected to the file descriptor obtained in 2).\n\nBecause \"$1\" becomes an empty file at 2), \"sort\" reads nothing and\nwrites nothing.\n\n> +\tsort $2 >$2;\n> +\treturn $(test_cmp $1 $2)\n\nWhat is this return doing? I would understand if it were just\n\n\ttest_cmp $1 $2\n\nOf course, all the places you use test_cmp_noorder are happy when\nthis function returns 0/success, and because $1 and $2 at this point\nare both empty files and test_cmp will not say anything to its\nstandard output, the return will just yield 0/success to the caller\nof the function, so it is likely that with this patch t9001 would\nhave passed for you, but that is not necessarily a good thing X-<.\n\n> @@ -269,7 +276,7 @@ test_expect_success $PREREQ 'Show all headers' '\n> \t\t-e \"s/^\\(Message-Id:\\).*/\\1 MESSAGE-ID-STRING/\" \\\n> \t\t-e \"s/^\\(X-Mailer:\\).*/\\1 X-MAILER-STRING/\" \\\n> \t\t>actual-show-all-headers &&\n> -\ttest_cmp expected-show-all-headers actual-show-all-headers\n> +\ttest_cmp_noorder expected-show-all-headers actual-show-all-headers\n> '\n\nIt is dubious that it is a good idea to blindly sort two files and\ncompare, especially because expected-show-all-headers is actually\nsomething like this:\n\n cat >expected-show-all-headers <<\\EOF\n 0001-Second.patch\n (mbox) Adding cc: A from line 'From: A '\n (mbox) Adding cc: One from line 'Cc: One , two@example.com'\n (mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n RCPT TO:\n RCPT TO:\n ...\n To: to@example.com\n Cc: cc@example.com,\n A ,\n One ,\n two@example.com\n Subject: [PATCH 1/1] Second.\n Date: DATE-STRING\n Message-Id: MESSAGE-ID-STRING\n X-Mailer: X-MAILER-STRING\n In-Reply-To: \n References: \n\n Result: OK\n EOF\n\nWe do want to see MAIL FROM: as the first thing we give to the\nserver, followed by RCPT TO:, followed by the headers.\n\nI am having a hard time guessing what prompted you to sort the\noutput, i.e. what problem you were trying to solve. It cannot be\nbecause addresses on a list (e.g. Cc:) could come out in an\nindeterministic order, because the address that a test expects to be\nthe first (cc@example.com in the above example) may not appear as\nthe first one, but in the textual output it _is_ shown differently\nfrom the remainder (i.e. even if you sort, from \"Cc:\ncc@example.com,\" it is clear it was the first one output for Cc: and\ndiferent from \"A \".\n"},{"id":"288692","messageId":"f34e3636-ce71-8352-259d-9f723d63e2a8@grenoble-inp.org","threadId":"42426","inReplyTo":"xmqqziqwmqth.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 1/6] t9001: non order-sensitive file comparison","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T08:23:22Z","receivedAt":"2016-06-08T08:23:22Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/08/2016 03:07 AM, Junio C Hamano wrote:\n> I am having a hard time guessing what prompted you to sort the\n> output, i.e. what problem you were trying to solve. It cannot be\n> because addresses on a list (e.g. Cc:) could come out in an\n> indeterministic order, because the address that a test expects to be\n> the first (cc@example.com in the above example) may not appear as\n> the first one, but in the textual output it _is_ shown differently\n> from the remainder (i.e. even if you sort, from \"Cc:\n> cc@example.com,\" it is clear it was the first one output for Cc: and\n> diferent from \"A \".\n\nActually we had issues when trying to refactor send-email's email \nparsing loop [1]. Email addresses in output file `commandeline1` in \ntests weren't sorted the same way as the reference file it was compared \nto. E.g.:\n\n !nobody@example.com!\n !author@example.com!\n !one@example.com!\n !two@example.com!\n\nI agree replacing test_cmp with test_cmp_noorder is pointless, I will \nfix it and re-roll.\n\nThanks.\n"},{"id":"288693","messageId":"20160608083607.GA9107@dcvr.yhbt.net","threadId":"42426","inReplyTo":"20160607140148.23242-4-tom.russello@grenoble-inp.org","subject":"Re: [PATCH v3 3/6] t9001: shorten send-email's output","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-06-08T08:36:07Z","receivedAt":"2016-06-08T08:36:07Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Tom Russello wrote:\n> Messages displayed by `send-email` should be shortened to avoid displaying\n> unnecesseray informations.\n\nunnecessary information.\n\nIn some of your other patches, the 'grep' can probably\nbe better replaced by 'fgrep' for fixed strings.\nOtherwise, the '.' in the 'example.com' would match any\ncharacter instead of the intended dot.\n\nThanks.\n"},{"id":"288695","messageId":"94ee5590-2832-afc0-a717-f795d88f03c3@grenoble-inp.org","threadId":"42426","inReplyTo":"20160608083607.GA9107@dcvr.yhbt.net","subject":"Re: [PATCH v3 3/6] t9001: shorten send-email's output","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T09:30:52Z","receivedAt":"2016-06-08T09:30:52Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/08/2016 10:36 AM, Eric Wong wrote:\n> Tom Russello wrote:\n>> Messages displayed by `send-email` should be shortened to avoid displaying\n>> unnecesseray informations.\n>\n> unnecessary information.\n>\n> In some of your other patches, the 'grep' can probably\n> be better replaced by 'fgrep' for fixed strings.\n> Otherwise, the '.' in the 'example.com' would match any\n> character instead of the intended dot.\n\nThanks, will re-roll\n"},{"id":"288709","messageId":"20160608130142.29879-1-samuel.groot@grenoble-inp.org","threadId":"42426","inReplyTo":"20160607140148.23242-1-tom.russello@grenoble-inp.org","subject":"(unknown)","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T13:01:36Z","receivedAt":"2016-06-08T13:01:36Z","isPatch":false,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"\nThe purpose of this series of patches is to implement a new\n\"quote-email\" feature integrated in the current `--in-reply-to` option.\n\n\n * The first 2 patches make the tests less dependent to\n `git send-email`'s exact output.\n\n * Third patch makes `git send-email` a bit less verbose.\n\n * Fourth patch introduces our email parser subroutine.\n\n * Fifth patch makes the `--in-reply-to` open a email file (if it\n exists) and populates From:, To:, Cc:, In-reply-to and\n References: fields.\n\n * Sixth patch quotes the message body in the cover letter if\n `--compose` is set. Else, imply `--annotate` and insert quoted\n message body below triple-dash in the first patch.\n\n\nChanges since v3:\n - test_cmp_noorder shell function fixed (patch 1/6)\n - use fgrep instead of grep (patch 2/6)\n - typo fixed (patch 3/6)\n - email parser subroutine moved to Git.pm library (patch 4/6)\n - test if $mail->{\"cc\"} is defined (patch 5/6)\n\n [PATCH v4 1/6] t9001: non order-sensitive file comparison\n [PATCH v4 2/6] t9001: check email address is in Cc: field\n [PATCH v4 3/6] send-email: shorten send-email's output\n [PATCH v4 4/6] send-email: create email parser subroutine\n [PATCH v4 5/6] send-email: --in-reply-to= populate header\n [PATCH v4 6/6] send-email: add option --cite to quote the message\n\n Documentation/git-send-email.txt | 17 +++++-\n git-send-email.perl | 150 ++++++++++++++++++++++++++++++++++++++++++++-----\n perl/Git.pm | 34 ++++++++++++\n t/t9001-send-email.sh | 190 +++++++++++++++++++++++++++++++++++++++++++++++++++------------\n 4 files changed, 339 insertions(+), 52 deletions(-)\n"},{"id":"288711","messageId":"20160608130142.29879-2-samuel.groot@grenoble-inp.org","threadId":"42426","inReplyTo":"20160608130142.29879-1-samuel.groot@grenoble-inp.org","subject":"[PATCH v4 1/6] t9001: non order-sensitive file comparison","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T13:01:37Z","receivedAt":"2016-06-08T13:01:37Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"Tests might fail if lines compared in text files don't have the same\norder.\n\nSigned-off-by: Samuel GROOT \nSigned-off-by: Tom RUSSELLO \nSigned-off-by: Matthieu MOY \n---\n t/t9001-send-email.sh | 13 ++++++++++---\n 1 file changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex b3355d2..56ad8ce 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -39,6 +39,13 @@ test_expect_success $PREREQ 'Extract patches' '\n \tpatches=$(git format-patch -s --cc=\"One \" --cc=two@example.com -n HEAD^1)\n '\n \n+# Check if two files have the same content, non-order sensitive\n+test_cmp_noorder () {\n+\tsort \"$1\" >\"$1_noorder\"\n+\tsort \"$2\" >\"$2_noorder\"\n+\ttest_cmp $1 $2\n+}\n+\n # Test no confirm early to ensure remaining tests will not hang\n test_no_confirm () {\n \trm -f no_confirm_okay\n@@ -97,7 +104,7 @@ test_expect_success $PREREQ 'setup expect' '\n '\n \n test_expect_success $PREREQ 'Verify commandline' '\n-\ttest_cmp expected commandline1\n+\ttest_cmp_noorder expected commandline1\n '\n \n test_expect_success $PREREQ 'Send patches with --envelope-sender' '\n@@ -117,7 +124,7 @@ test_expect_success $PREREQ 'setup expect' '\n '\n \n test_expect_success $PREREQ 'Verify commandline' '\n-\ttest_cmp expected commandline1\n+\ttest_cmp_noorder expected commandline1\n '\n \n test_expect_success $PREREQ 'Send patches with --envelope-sender=auto' '\n@@ -137,7 +144,7 @@ test_expect_success $PREREQ 'setup expect' '\n '\n \n test_expect_success $PREREQ 'Verify commandline' '\n-\ttest_cmp expected commandline1\n+\ttest_cmp_noorder expected commandline1\n '\n \n test_expect_success $PREREQ 'setup expect' \"\n-- \n2.8.2.537.gb153d2a\n"},{"id":"288710","messageId":"20160608130142.29879-3-samuel.groot@grenoble-inp.org","threadId":"42426","inReplyTo":"20160608130142.29879-1-samuel.groot@grenoble-inp.org","subject":"[PATCH v4 2/6] t9001: check email address is in Cc: field","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T13:01:38Z","receivedAt":"2016-06-08T13:01:38Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"Check if the given utf-8 email address is in the Cc: field.\n\nSigned-off-by: Tom RUSSELLO \nSigned-off-by: Samuel GROOT \nSigned-off-by: Matthieu MOY \n---\n t/t9001-send-email.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 56ad8ce..943e6b7 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -888,8 +888,8 @@ test_expect_success $PREREQ 'utf8 Cc is rfc2047 encoded' '\n \t--to=nobody@example.com \\\n \t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n \toutdir/*.patch &&\n-\tgrep \"^\t\" msgtxt1 |\n-\tgrep \"=?UTF-8?q?=C3=A0=C3=A9=C3=AC=C3=B6=C3=BA?= \"\n+\tcc_adr=$(awk \"/^Cc: /{flag=1}/^Subject: /{flag=0} flag {print}\" msgtxt1) &&\n+\techo \"$cc_adr\" | fgrep \"=?UTF-8?q?=C3=A0=C3=A9=C3=AC=C3=B6=C3=BA?= \"\n '\n \n test_expect_success $PREREQ '--compose adds MIME for utf8 body' '\n-- \n2.8.2.537.gb153d2a\n"},{"id":"288712","messageId":"20160608130142.29879-4-samuel.groot@grenoble-inp.org","threadId":"42426","inReplyTo":"20160608130142.29879-1-samuel.groot@grenoble-inp.org","subject":"[PATCH v4 3/6] send-email: shorten send-email's output","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T13:01:39Z","receivedAt":"2016-06-08T13:01:39Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"Messages displayed by `send-email` should be shortened to avoid displaying\nunnecessary information.\n\nSigned-off-by: Samuel GROOT \nSigned-off-by: Tom RUSSELLO \nSigned-off-by: Matthieu MOY \n---\n git-send-email.perl | 22 +++++++++----------\n t/t9001-send-email.sh | 58 +++++++++++++++++++++++++--------------------------\n 2 files changed, 40 insertions(+), 40 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 6958785..9b51062 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1478,14 +1478,14 @@ foreach my $t (@files) {\n \t\t\t\t$sauthor = sanitize_address($author);\n \t\t\t\tnext if $suppress_cc{'author'};\n \t\t\t\tnext if $suppress_cc{'self'} and $sauthor eq $sender;\n-\t\t\t\tprintf(\"(mbox) Adding cc: %s from line '%s'\\n\",\n-\t\t\t\t\t$1, $_) unless $quiet;\n+\t\t\t\tprintf(\"Adding cc: %s from From: header\\n\",\n+\t\t\t\t\t$1) unless $quiet;\n \t\t\t\tpush @cc, $1;\n \t\t\t}\n \t\t\telsif (/^To:\\s+(.*)$/i) {\n \t\t\t\tforeach my $addr (parse_address_line($1)) {\n-\t\t\t\t\tprintf(\"(mbox) Adding to: %s from line '%s'\\n\",\n-\t\t\t\t\t\t$addr, $_) unless $quiet;\n+\t\t\t\t\tprintf(\"Adding to: %s from To: header\\n\",\n+\t\t\t\t\t\t$addr) unless $quiet;\n \t\t\t\t\tpush @to, $addr;\n \t\t\t\t}\n \t\t\t}\n@@ -1498,8 +1498,8 @@ foreach my $t (@files) {\n \t\t\t\t\t} else {\n \t\t\t\t\t\tnext if ($suppress_cc{'cc'});\n \t\t\t\t\t}\n-\t\t\t\t\tprintf(\"(mbox) Adding cc: %s from line '%s'\\n\",\n-\t\t\t\t\t\t$addr, $_) unless $quiet;\n+\t\t\t\t\tprintf(\"Adding cc: %s from Cc: header\\n\",\n+\t\t\t\t\t\t$addr) unless $quiet;\n \t\t\t\t\tpush @cc, $addr;\n \t\t\t\t}\n \t\t\t}\n@@ -1532,8 +1532,8 @@ foreach my $t (@files) {\n \t\t\t# So let's support that, too.\n \t\t\t$input_format = 'lots';\n \t\t\tif (@cc == 0 && !$suppress_cc{'cc'}) {\n-\t\t\t\tprintf(\"(non-mbox) Adding cc: %s from line '%s'\\n\",\n-\t\t\t\t\t$_, $_) unless $quiet;\n+\t\t\t\tprintf(\"Adding cc: %s from Cc: header\\n\",\n+\t\t\t\t\t$_) unless $quiet;\n \t\t\t\tpush @cc, $_;\n \t\t\t} elsif (!defined $subject) {\n \t\t\t\t$subject = $_;\n@@ -1555,8 +1555,8 @@ foreach my $t (@files) {\n \t\t\t\tnext if $suppress_cc{'bodycc'} and $what =~ /Cc/i;\n \t\t\t}\n \t\t\tpush @cc, $c;\n-\t\t\tprintf(\"(body) Adding cc: %s from line '%s'\\n\",\n-\t\t\t\t$c, $_) unless $quiet;\n+\t\t\tprintf(\"Adding cc: %s from Signed-off-by: trailer\\n\",\n+\t\t\t\t$c) unless $quiet;\n \t\t}\n \t}\n \tclose $fh;\n@@ -1660,7 +1660,7 @@ sub recipients_cmd {\n \t\t$address = sanitize_address($address);\n \t\tnext if ($address eq $sender and $suppress_cc{'self'});\n \t\tpush @addresses, $address;\n-\t\tprintf(\"($prefix) Adding %s: %s from: '%s'\\n\",\n+\t\tprintf(\"Adding %s: %s from: '%s'\\n\",\n \t\t $what, $address, $cmd) unless $quiet;\n \t\t}\n \tclose $fh\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 943e6b7..aca7d5c 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -150,9 +150,9 @@ test_expect_success $PREREQ 'Verify commandline' '\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-show-all-headers <<\\EOF\n 0001-Second.patch\n-(mbox) Adding cc: A from line 'From: A '\n-(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n-(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n+Adding cc: A from From: header\n+Adding cc: One from Cc: header\n+Adding cc: two@example.com from Cc: header\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n@@ -503,9 +503,9 @@ test_expect_success $PREREQ 'second message is patch' '\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-suppress-sob <<\\EOF\n 0001-Second.patch\n-(mbox) Adding cc: A from line 'From: A '\n-(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n-(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n+Adding cc: A from From: header\n+Adding cc: One from Cc: header\n+Adding cc: two@example.com from Cc: header\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n@@ -555,9 +555,9 @@ test_expect_success $PREREQ 'sendemail.cc set' '\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-suppress-sob <<\\EOF\n 0001-Second.patch\n-(mbox) Adding cc: A from line 'From: A '\n-(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n-(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n+Adding cc: A from From: header\n+Adding cc: One from Cc: header\n+Adding cc: two@example.com from Cc: header\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n@@ -587,10 +587,10 @@ test_expect_success $PREREQ 'sendemail.cc unset' '\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-suppress-cccmd <<\\EOF\n 0001-Second.patch\n-(mbox) Adding cc: A from line 'From: A '\n-(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n-(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n-(body) Adding cc: C O Mitter from line 'Signed-off-by: C O Mitter '\n+Adding cc: A from From: header\n+Adding cc: One from Cc: header\n+Adding cc: two@example.com from Cc: header\n+Adding cc: C O Mitter from Signed-off-by: trailer\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n@@ -647,10 +647,10 @@ test_expect_success $PREREQ '--suppress-cc=all' '\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-suppress-body <<\\EOF\n 0001-Second.patch\n-(mbox) Adding cc: A from line 'From: A '\n-(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n-(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n-(cc-cmd) Adding cc: cc-cmd@example.com from: './cccmd'\n+Adding cc: A from From: header\n+Adding cc: One from Cc: header\n+Adding cc: two@example.com from Cc: header\n+Adding cc: cc-cmd@example.com from: './cccmd'\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n@@ -681,9 +681,9 @@ test_expect_success $PREREQ '--suppress-cc=body' '\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-suppress-body-cccmd <<\\EOF\n 0001-Second.patch\n-(mbox) Adding cc: A from line 'From: A '\n-(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n-(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n+Adding cc: A from From: header\n+Adding cc: One from Cc: header\n+Adding cc: two@example.com from Cc: header\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n@@ -712,9 +712,9 @@ test_expect_success $PREREQ '--suppress-cc=body --suppress-cc=cccmd' '\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-suppress-sob <<\\EOF\n 0001-Second.patch\n-(mbox) Adding cc: A from line 'From: A '\n-(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n-(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n+Adding cc: A from From: header\n+Adding cc: One from Cc: header\n+Adding cc: two@example.com from Cc: header\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n@@ -744,10 +744,10 @@ test_expect_success $PREREQ '--suppress-cc=sob' '\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-suppress-bodycc <<\\EOF\n 0001-Second.patch\n-(mbox) Adding cc: A from line 'From: A '\n-(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n-(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n-(body) Adding cc: C O Mitter from line 'Signed-off-by: C O Mitter '\n+Adding cc: A from From: header\n+Adding cc: One from Cc: header\n+Adding cc: two@example.com from Cc: header\n+Adding cc: C O Mitter from Signed-off-by: trailer\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n@@ -778,8 +778,8 @@ test_expect_success $PREREQ '--suppress-cc=bodycc' '\n test_expect_success $PREREQ 'setup expect' \"\n cat >expected-suppress-cc <<\\EOF\n 0001-Second.patch\n-(mbox) Adding cc: A from line 'From: A '\n-(body) Adding cc: C O Mitter from line 'Signed-off-by: C O Mitter '\n+Adding cc: A from From: header\n+Adding cc: C O Mitter from Signed-off-by: trailer\n Dry-OK. Log says:\n Server: relay.example.com\n MAIL FROM:\n-- \n2.8.2.537.gb153d2a\n"},{"id":"288713","messageId":"20160608130142.29879-5-samuel.groot@grenoble-inp.org","threadId":"42426","inReplyTo":"20160608130142.29879-1-samuel.groot@grenoble-inp.org","subject":"[PATCH v4 4/6] send-email: create email parser subroutine","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T13:01:40Z","receivedAt":"2016-06-08T13:01:40Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"We need a simple and generic way to parse an email file.\n\nSince it would be hard to include and maintain an external library,\ncreate an simple email parser subroutine to parse an email file.\n\nSigned-off-by: Samuel GROOT \nSigned-off-by: Tom RUSSELLO \nSigned-off-by: Matthieu MOY \n---\nWe chose to create our own simple email parser and only use it for the\n\"quote email\" feature to pave the way for the refactorization of the patch\nparser [1] that may come after our current school project.\n\n[1] * http://thread.gmane.org/gmane.comp.version-control.git/295752\n\n perl/Git.pm | 34 ++++++++++++++++++++++++++++++++++\n 1 file changed, 34 insertions(+)\n\ndiff --git a/perl/Git.pm b/perl/Git.pm\nindex ce7e4e8..1af4805 100644\n--- a/perl/Git.pm\n+++ b/perl/Git.pm\n@@ -865,6 +865,40 @@ sub ident_person {\n \treturn \"$ident[0] <$ident[1]>\";\n }\n \n+=item parse_email\n+\n+Return a hash of email fields extracted from a file handler.\n+\n+=cut\n+\n+sub parse_email {\n+\tmy %mail = ();\n+\tmy $fh = shift;\n+\tmy $last_header;\n+\n+\t# Unfold and parse multiline header fields\n+\twhile (<$fh>) {\n+\t\tlast if /^\\s*$/;\n+\t\ts/\\r\\n|\\n|\\r//;\n+\t\tif (/^([^\\s:]+):[\\s]+(.*)$/) {\n+\t\t\t$last_header = lc($1);\n+\t\t\t@{$mail{$last_header}} = ()\n+\t\t\t\tunless defined $mail{$last_header};\n+\t\t\tpush @{$mail{$last_header}}, $2;\n+\t\t} elsif (/^\\s+\\S/ and defined $last_header) {\n+\t\t\ts/^\\s+/ /;\n+\t\t\tpush @{$mail{$last_header}}, $_;\n+\t\t} else {\n+\t\t\tdie(\"Mail format undefined!\\n\");\n+\t\t}\n+\t}\n+\n+\t# Separate body from header\n+\t$mail{\"body\"} = [(<$fh>)];\n+\n+\treturn \\%mail;\n+}\n+\n =item parse_mailboxes\n \n Return an array of mailboxes extracted from a string.\n-- \n2.8.2.537.gb153d2a\n"},{"id":"288714","messageId":"20160608130736.32163-1-samuel.groot@grenoble-inp.org","threadId":"42426","inReplyTo":"20160608130142.29879-1-samuel.groot@grenoble-inp.org","subject":"[PATCH v4 5/6] send-email: --in-reply-to= populate header fields","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T13:07:36Z","receivedAt":"2016-06-08T13:07:36Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"Take an email message file, parse it and fill the \"From\", \"To\", \"Cc\",\n\"In-reply-to\", \"References\" fields appropriately.\n\nIf `--compose` option is set, it will also fill the subject field with\n`Re: ['s subject]` in the introductory message.\n\nSigned-off-by: Tom RUSSELLO \nSigned-off-by: Samuel GROOT \nSigned-off-by: Matthieu MOY \n---\n Documentation/git-send-email.txt | 9 +++--\n git-send-email.perl | 51 +++++++++++++++++++++++-\n t/t9001-send-email.sh | 83 ++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 138 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex edbba3a..21776f0 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -84,13 +84,16 @@ See the CONFIGURATION section for 'sendemail.multiEdit'.\n \tthe value of GIT_AUTHOR_IDENT, or GIT_COMMITTER_IDENT if that is not\n \tset, as returned by \"git var -l\".\n \n---in-reply-to=::\n+--in-reply-to=::\n \tMake the first mail (or all the mails with `--no-thread`) appear as a\n-\treply to the given Message-Id, which avoids breaking threads to\n-\tprovide a new patch series.\n+\treply to the given Message-Id (given directly by argument or via the email\n+\tfile), which avoids breaking threads to provide a new patch series.\n \tThe second and subsequent emails will be sent as replies according to\n \tthe `--[no]-chain-reply-to` setting.\n +\n+Furthermore, if the argument is an email file, parse it and populate header\n+fields appropriately for the reply.\n++\n So for example when `--thread` and `--no-chain-reply-to` are specified, the\n second and subsequent patches will be replies to the first one like in the\n illustration below where `[PATCH v2 0/3]` is in reply to `[PATCH 0/2]`:\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 9b51062..b444ea6 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -55,6 +55,7 @@ git send-email --dump-aliases\n --[no-]bcc * Email Bcc:\n --subject * Email \"Subject:\"\n --in-reply-to * Email \"In-Reply-To:\"\n+ --in-reply-to * 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@@ -160,7 +161,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,$initial_subject,@files,\n \t$author,$sender,$smtp_authpass,$annotate,$use_xmailer,$compose,$time);\n \n my $envelope_sender;\n@@ -639,6 +640,52 @@ if (@files) {\n \tusage();\n }\n \n+if ($initial_reply_to && -f $initial_reply_to) {\n+\tmy $error = validate_patch($initial_reply_to);\n+\tdie \"fatal: $initial_reply_to: $error\\nwarning: no patches were sent\\n\"\n+\t\tif $error;\n+\n+\topen my $fh, \"<\", $initial_reply_to or die \"can't open file $initial_reply_to\";\n+\tmy $mail = Git::parse_email($fh);\n+\tclose $fh;\n+\n+\tmy $initial_sender = $sender || $repoauthor || $repocommitter || '';\n+\n+\tmy $prefix_re = \"\";\n+\tmy $subject_re = $mail->{\"subject\"}[0];\n+\tif ($subject_re =~ /^[^Re:]/) {\n+\t\t$prefix_re = \"Re: \";\n+\t}\n+\t$initial_subject = $prefix_re . $subject_re;\n+\n+\tpush @initial_to, $mail->{\"from\"}[0];\n+\n+\tforeach my $to_addr (parse_address_line(join \",\", @{$mail->{\"to\"}})) {\n+\t\tif (!($to_addr eq $initial_sender)) {\n+\t\t\tpush @initial_cc, $to_addr;\n+\t\t}\n+\t}\n+\n+\tif (defined $mail->{\"cc\"}) {\n+\t\tforeach my $cc_addr (parse_address_line(join \",\", @{$mail->{\"cc\"}})) {\n+\t\t\tmy $qaddr = unquote_rfc2047($cc_addr);\n+\t\t\tmy $saddr = sanitize_address($qaddr);\n+\t\t\tif ($saddr eq $initial_sender) {\n+\t\t\t\tnext if ($suppress_cc{'self'});\n+\t\t\t} else {\n+\t\t\t\tnext if ($suppress_cc{'cc'});\n+\t\t\t}\n+\t\t\tpush @initial_cc, $cc_addr;\n+\t\t}\n+\t}\n+\n+\t$initial_reply_to = $mail->{\"message-id\"}[0];\n+\tif ($mail->{\"references\"}) {\n+\t\t$initial_references = join(\"\", @{$mail->{\"references\"}}) .\n+\t\t\t\" \" . $initial_reply_to;\n+\t}\n+}\n+\n sub get_patch_subject {\n \tmy $fn = shift;\n \topen (my $fh, '<', $fn);\n@@ -1426,7 +1473,7 @@ Message-Id: $message_id\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 aca7d5c..7591342 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1892,4 +1892,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 '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+\tHave you seen my previous email?\n+\t> Previous content\n+\tEOF\n+'\n+\n+test_expect_success $PREREQ 'Fields with --in-reply-to are correct' '\n+\tclean_fake_sendmail &&\n+\tgit send-email \\\n+\t\t--in-reply-to=email \\\n+\t\t--from=\"Example \" \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n+\t\t-2 \\\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 --in-reply-to and --compose are correct' '\n+\tclean_fake_sendmail &&\n+\tgit send-email \\\n+\t\t--in-reply-to=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 --in-reply-to and --compose ' '\n+\tgit send-email \\\n+\t\t--in-reply-to=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.8.2.537.gb153d2a\n"},{"id":"288716","messageId":"20160608130833.32274-1-samuel.groot@grenoble-inp.org","threadId":"42426","inReplyTo":"20160608130142.29879-1-samuel.groot@grenoble-inp.org","subject":"[PATCH v4 6/6] send-email: add option --cite to quote the message body","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T13:08:33Z","receivedAt":"2016-06-08T13:08:33Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"If used with `in-reply-to=`, cite the message body of the given\nemail file. Otherwise, do nothing.\n\nIf `--compose` is also set, quote the message body in the cover letter. Else,\nimply `--annotate` by default and quote the message body below the triple-dash\nsection in the first patch only.\n\nSigned-off-by: Tom RUSSELLO \nSigned-off-by: Samuel GROOT \nSigned-off-by: Matthieu MOY \n---\n Documentation/git-send-email.txt | 8 ++++\n git-send-email.perl | 81 ++++++++++++++++++++++++++++++++++++++--\n t/t9001-send-email.sh | 32 ++++++++++++++++\n 3 files changed, 117 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex 21776f0..23cbd17 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -109,6 +109,14 @@ 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+--cite::\n+\tWhen used with `--in-reply-to=`, quote the message body of the\n+\tgiven email file.\n++\n+If `--compose` is also set, the message cited will be in the cover letter. If\n+`--compose` is not set, `--annotate` option is implied by default and the\n+message body will be cited in the \"below-triple-dash\" section.\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 b444ea6..6877ea7 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(catfile);\n+use File::Copy;\n use Error qw(:try);\n use Git;\n \n@@ -56,6 +57,8 @@ git send-email --dump-aliases\n --subject * Email \"Subject:\"\n --in-reply-to * Email \"In-Reply-To:\"\n --in-reply-to * Populate header fields appropriately.\n+ --cite * Quote the message body in the cover if\n+ --compose is set, else in the first patch.\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@@ -161,7 +164,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_references,$initial_subject,@files,\n+\t$initial_reply_to,$initial_references,$cite,$initial_subject,@files,\n \t$author,$sender,$smtp_authpass,$annotate,$use_xmailer,$compose,$time);\n \n my $envelope_sender;\n@@ -305,6 +308,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 \"cite\" => \\$cite,\n \t\t \"to=s\" => \\@initial_to,\n \t\t \"to-cmd=s\" => \\$to_cmd,\n \t\t \"no-to\" => \\$no_to,\n@@ -640,6 +644,7 @@ if (@files) {\n \tusage();\n }\n \n+my $message_cited;\n if ($initial_reply_to && -f $initial_reply_to) {\n \tmy $error = validate_patch($initial_reply_to);\n \tdie \"fatal: $initial_reply_to: $error\\nwarning: no patches were sent\\n\"\n@@ -658,7 +663,8 @@ if ($initial_reply_to && -f $initial_reply_to) {\n \t}\n \t$initial_subject = $prefix_re . $subject_re;\n \n-\tpush @initial_to, $mail->{\"from\"}[0];\n+\tmy $recipient = $mail->{\"from\"}[0];\n+\tpush @initial_to, $recipient;\n \n \tforeach my $to_addr (parse_address_line(join \",\", @{$mail->{\"to\"}})) {\n \t\tif (!($to_addr eq $initial_sender)) {\n@@ -684,6 +690,25 @@ if ($initial_reply_to && -f $initial_reply_to) {\n \t\t$initial_references = join(\"\", @{$mail->{\"references\"}}) .\n \t\t\t\" \" . $initial_reply_to;\n \t}\n+\n+\tif ($cite) {\n+\t\tmy $date = $mail->{\"date\"}[0];\n+\t\tmy $tpl_date = $date && \"On $date, \" || '';\n+\t\t$message_cited = $tpl_date . $recipient . \" wrote:\\n\";\n+\n+\t\t# Quote the message body\n+\t\tforeach (@{$mail->{\"body\"}}) {\n+\t\t\tmy $space = \"\";\n+\t\t\tif (/^[^>]/) {\n+\t\t\t\t$space = \" \";\n+\t\t\t}\n+\t\t\t$message_cited .= \">\" . $space . $_;\n+\t\t}\n+\n+\t\tif (!$compose) {\n+\t\t\t$annotate = 1;\n+\t\t}\n+\t}\n }\n \n sub get_patch_subject {\n@@ -711,6 +736,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_cited &&\n+\t\t\"\\nGIT: Please, trim down irrelevant sections in the cited message\\n\".\n+\t\t\"GIT: to keep your email concise.\\n\" . $message_cited || '';\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_cited_message($cite_email_filename, $files[0]);\n+\n+\t\tmy $tmp = $files[0];\n+\t\t$files[0] = $cite_email_filename;\n+\n+\t\tdo_edit(@files);\n+\n+\t\t# Erase the original patch if the edition went well\n+\t\tmove($cite_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_cited_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_cited;\n+\twhile (<$c>) {\n+\t\tprint $c2 $_;\n+\t}\n+\n+\tclose $c;\n+\tclose $c2;\n }\n \n sub ask {\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 7591342..29e28f2 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -1915,6 +1915,7 @@ test_expect_success $PREREQ 'Fields with --in-reply-to are correct' '\n \tgit send-email \\\n \t\t--in-reply-to=email \\\n \t\t--from=\"Example \" \\\n+\t\t--cite \\\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n \t\t-2 \\\n \t\t2>errors &&\n@@ -1936,10 +1937,22 @@ test_expect_success $PREREQ 'Fields with --in-reply-to are correct' '\n \techo \"$ref_adr\" | grep -v \"References: \"\n '\n \n+test_expect_success $PREREQ 'correct cited message with --in-reply-to' '\n+\tmsg_cited=$(grep -A 3 \"^---$\" msgtxt1) &&\n+\techo \"$msg_cited\" | grep \"On Sat, 12 Jun 2010 15:53:58 +0200, author@example.com wrote:\" &&\n+\techo \"$msg_cited\" | grep \"> Have you seen my previous email?\" &&\n+\techo \"$msg_cited\" | grep \">> Previous content\"\n+'\n+\n+test_expect_success $PREREQ 'second patch body is not modified by --in-reply-to' '\n+\t! grep \"Have you seen my previous email?\" msgtxt2\n+'\n+\n test_expect_success $PREREQ 'Fields with --in-reply-to and --compose are correct' '\n \tclean_fake_sendmail &&\n \tgit send-email \\\n \t\t--in-reply-to=email \\\n+\t\t--cite \\\n \t\t--compose \\\n \t\t--from=\"Example \" \\\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n@@ -1967,6 +1980,7 @@ test_expect_success $PREREQ 'Fields with --in-reply-to and --compose are correct\n test_expect_success $PREREQ 'Re: written only once with --in-reply-to and --compose ' '\n \tgit send-email \\\n \t\t--in-reply-to=msgtxt1 \\\n+\t\t--cite \\\n \t\t--compose \\\n \t\t--from=\"Example \" \\\n \t\t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n@@ -1975,4 +1989,22 @@ test_expect_success $PREREQ 'Re: written only once with --in-reply-to and --comp\n \tgrep \"Subject: Re: subject goes here\" msgtxt3\n '\n \n+test_expect_success $PREREQ 'correct cited message with --in-reply-to 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_expect_success $PREREQ 'Message is not cited with only --in-reply-to' '\n+\tclean_fake_sendmail &&\n+\tgit send-email \\\n+\t\t--in-reply-to=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+\t! grep \"Have you seen my previous email?\" msgtxt1\n+'\n+\n test_done\n-- \n2.8.2.537.gb153d2a\n"},{"id":"288719","messageId":"1755917243.420844.1465395729704.JavaMail.zimbra@ensimag.grenoble-inp.fr","threadId":"42426","inReplyTo":"20160608130142.29879-2-samuel.groot@grenoble-inp.org","subject":"Re: [PATCH v4 1/6] t9001: non order-sensitive file comparison","fromName":"Remi Galan Alfonso","fromEmail":"remi.galan-alfonso@ensimag.grenoble-inp.fr","sentAt":"2016-06-08T14:22:09Z","receivedAt":"2016-06-08T14:22:09Z","isPatch":true,"sender":{"key":"remi.galan-alfonso@ensimag.grenoble-inp.fr","avatar":"https://avatars.githubusercontent.com/u/12509162?v=4"},"body":"Hi Samuel,\n\nSamuel GROOT writes:\n> +test_cmp_noorder () {\n> + sort \"$1\" >\"$1_noorder\"\n> + sort \"$2\" >\"$2_noorder\"\n> + test_cmp $1 $2\n\nYou meant `test_cmp \"$1_noorder\" \"$2_noorder\"`, I guess.\n\nThanks,\nRémi\n"},{"id":"288721","messageId":"b79fa65c-c28b-534c-29f4-0998f75c2275@grenoble-inp.org","threadId":"42426","inReplyTo":"1755917243.420844.1465395729704.JavaMail.zimbra@ensimag.grenoble-inp.fr","subject":"Re: [PATCH v4 1/6] t9001: non order-sensitive file comparison","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T14:29:37Z","receivedAt":"2016-06-08T14:29:37Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/08/2016 04:22 PM, Remi Galan Alfonso wrote:\n>> +test_cmp_noorder () {\n>> + sort \"$1\" >\"$1_noorder\"\n>> + sort \"$2\" >\"$2_noorder\"\n>> + test_cmp $1 $2\n>\n> You meant `test_cmp \"$1_noorder\" \"$2_noorder\"`, I guess.\n\nYes, thanks for pointing it out!\n"},{"id":"288728","messageId":"xmqqvb1jmzly.fsf@gitster.mtv.corp.google.com","threadId":"42426","inReplyTo":"f34e3636-ce71-8352-259d-9f723d63e2a8@grenoble-inp.org","subject":"Re: [PATCH v3 1/6] t9001: non order-sensitive file comparison","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-08T16:09:29Z","receivedAt":"2016-06-08T16:09:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel GROOT writes:\n\n> Actually we had issues when trying to refactor send-email's email\n> parsing loop [1]. Email addresses in output file `commandeline1` in\n> tests weren't sorted the same way as the reference file it was\n> compared to. E.g.:\n>\n> !nobody@example.com!\n> !author@example.com!\n> !one@example.com!\n> !two@example.com!\n\nAnd the reason why these addresses that are collected from the same\ninput (i.e. command line, existing e-mail fields, footers, etc.) are\nshown in different order in your implementation is...?\n"},{"id":"288732","messageId":"3090c61c-533b-7119-f5e4-7d99e62f6da4@grenoble-inp.org","threadId":"42426","inReplyTo":"xmqqvb1jmzly.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 1/6] t9001: non order-sensitive file comparison","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T16:46:59Z","receivedAt":"2016-06-08T16:46:59Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/08/2016 06:09 PM, Junio C Hamano wrote:\n> Samuel GROOT writes:\n>\n>> Actually we had issues when trying to refactor send-email's email\n>> parsing loop [1]. Email addresses in output file `commandeline1` in\n>> tests weren't sorted the same way as the reference file it was\n>> compared to. E.g.:\n>>\n>> !nobody@example.com!\n>> !author@example.com!\n>> !one@example.com!\n>> !two@example.com!\n>\n> And the reason why these addresses that are collected from the same\n> input (i.e. command line, existing e-mail fields, footers, etc.) are\n> shown in different order in your implementation is...?\n\nIt's not shown in different order in our implementation, it's just a \nleftover of my refactor attempt [1].\n\nMaybe it's a bad idea to increase tests' complexity, but IMHO tests \nshould be independent to the addresses' order.\n\nPlus, it would help refactor in the future, the data being processed \ndifferently: parsing and processing in different subroutines rather than \ndoing everything in one gigantic loop.\n\nWe can drop it if necessary, but it may be useful to make \ngit-send-email.perl easier to maintain.\n\n[1] * http://article.gmane.org/gmane.comp.version-control.git/295753\n"},{"id":"288733","messageId":"xmqqr3c7mxg4.fsf@gitster.mtv.corp.google.com","threadId":"42426","inReplyTo":"20160608130142.29879-2-samuel.groot@grenoble-inp.org","subject":"Re: [PATCH v4 1/6] t9001: non order-sensitive file comparison","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-08T16:56:11Z","receivedAt":"2016-06-08T16:56:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel GROOT writes:\n\n> @@ -117,7 +124,7 @@ test_expect_success $PREREQ 'setup expect' '\n> '\n> \n> test_expect_success $PREREQ 'Verify commandline' '\n> -\ttest_cmp expected commandline1\n> +\ttest_cmp_noorder expected commandline1\n> '\n> \n> test_expect_success $PREREQ 'Send patches with --envelope-sender=auto' '\n\nI think this comment applies to all the other hunk in this patch (I\ndidn't check very carefully though), but this is trying to see if\nthe command line arguments that drives send-email are like this (one\narg per line, enclosed in !! pairs for clarity):\n\n\t!patch@example.com!\n\t!-i!\n\t!nobody@example.com!\n\t!author@example.com!\n\t!one@example.com!\n\t!two@example.com!\n\nwhen these addresses are given from the command line:\n\n\tgit send-email \\\n --envelope-sender=\"Patch Contributor \" \\\n --suppress-cc=sob \\\n --from=\"Example \" \\\n --to=nobody@example.com \\\n --smtp-server=\"$(pwd)/fake.sendmail\" $patches 2>errors\n\nthat creates something like\n\n$TRASH_DIRECTORY/fake.sendmail -f patch@example.com -i \\\n nobody@example.com author@example.com one@example.com two@example.com\n\n(all on a single line).\n\nThe earliest address patch@example.com and later addresses have\nquite different meaning (the first one is meant to be the envelope\nsender address, and does not name a recipient). While I think it is\na good idea to tell the test that the order of recipient addresses\ngiven to the sendmail command (i.e. nobody, author, one and two)\ndoes not matter by comparing sorted list of addresses, sorting the\nwhole argument list and comparing is making the test _too_ loose.\nDon't you want to catch a potential bug that adds the envelope\nsender address to the list of recipients by mistake, for example?\n"},{"id":"288734","messageId":"xmqqmvmvmwh7.fsf@gitster.mtv.corp.google.com","threadId":"42426","inReplyTo":"20160608130142.29879-2-samuel.groot@grenoble-inp.org","subject":"Re: [PATCH v4 1/6] t9001: non order-sensitive file comparison","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-08T17:17:08Z","receivedAt":"2016-06-08T17:17:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel GROOT writes:\n\n> @@ -97,7 +104,7 @@ test_expect_success $PREREQ 'setup expect' '\n> '\n> \n> test_expect_success $PREREQ 'Verify commandline' '\n> -\ttest_cmp expected commandline1\n> +\ttest_cmp_noorder expected commandline1\n> '\n> \n> test_expect_success $PREREQ 'Send patches with --envelope-sender' '\n\nBy the way, don't you find it irritating to review this patch that\nhas three hunks, all of which look like the above? You cannot\neasily tell which 3 among 27 instances of test_cmp are modified,\nbecause the hunks do not give useful context.\n\nThis is not at all your fault, but because the existing tests are\nstructured poorly. It separates one logical step into three pieces\nwithout a good reason.\n\nHere is an illustration of an organization that I think would be\neasier to read, and would result in a more readable patch when\nmodification is made on top. The first two hunks collapse the\noverall \"setup\" steps that appear as three separate tests into a\nsingle \"setup\" test. The last hunk that begin at -83/+79 collapses\na logically-single test that is split across three into one, and\nmakes the order of things done in the test to (1) set an\nexpectation, (2) execute the command and (3) compare the result with\nthe expectation.\n\nI am not going to commit this myself, because I do not want to\ncreate conflicts with the change your topic is trying to do, and\nbesides, almost all the remainder of the tests follow \"one logical\ntest split into three\" pattern and need to be corrected before this\n\"illustration\" can become a real patch.\n\nI do not mind if you take it and complete it as a preliminary\nclean-up step in your series; or you can \"keep it in mind, but\nignore it for now\", in which case this can be a \"low hanging fruit\"\nsomebody else, hopefully somebody new to the development community,\ncan use to dip their toes ;-)\n\n\n\n t/t9001-send-email.sh | 33 ++++++++++++++-------------------\n 1 file changed, 14 insertions(+), 19 deletions(-)\n\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex b3355d2..858bdbe 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -6,14 +6,16 @@ test_description='git send-email'\n # May be altered later in the test\n PREREQ=\"PERL\"\n \n-test_expect_success $PREREQ 'prepare reference tree' '\n+clean_fake_sendmail () {\n+\trm -f commandline* msgtxt*\n+}\n+\n+test_expect_success $PREREQ 'setup' '\n \techo \"1A quick brown fox jumps over the\" >file &&\n \techo \"lazy dog\" >>file &&\n \tgit add file &&\n-\tGIT_AUTHOR_NAME=\"A\" git commit -a -m \"Initial.\"\n-'\n+\tGIT_AUTHOR_NAME=\"A\" git commit -a -m \"Initial.\" &&\n \n-test_expect_success $PREREQ 'Setup helper tool' '\n \twrite_script fake.sendmail <<-\\EOF &&\n \tshift\n \toutput=1\n@@ -28,14 +30,8 @@ test_expect_success $PREREQ 'Setup helper tool' '\n \tcat >\"msgtxt$output\"\n \tEOF\n \tgit add fake.sendmail &&\n-\tGIT_AUTHOR_NAME=\"A\" git commit -a -m \"Second.\"\n-'\n-\n-clean_fake_sendmail () {\n-\trm -f commandline* msgtxt*\n-}\n+\tGIT_AUTHOR_NAME=\"A\" git commit -a -m \"Second.\" &&\n \n-test_expect_success $PREREQ 'Extract patches' '\n \tpatches=$(git format-patch -s --cc=\"One \" --cc=two@example.com -n HEAD^1)\n '\n \n@@ -83,20 +79,19 @@ test_expect_success $PREREQ 'No confirm with sendemail.confirm=never' '\n \tcheck_no_confirm\n '\n \n-test_expect_success $PREREQ 'Send patches' '\n-\tgit send-email --suppress-cc=sob --from=\"Example \" --to=nobody@example.com --smtp-server=\"$(pwd)/fake.sendmail\" $patches 2>errors\n-'\n-\n-test_expect_success $PREREQ 'setup expect' '\n-\tcat >expected <<-\\EOF\n+test_expect_success $PREREQ 'with --suppress-cc=sob --from and --to' '\n+\tcat >expected <<-\\EOF &&\n \t!nobody@example.com!\n \t!author@example.com!\n \t!one@example.com!\n \t!two@example.com!\n \tEOF\n-'\n \n-test_expect_success $PREREQ 'Verify commandline' '\n+\tgit send-email --suppress-cc=sob \\\n+\t\t--from=\"Example \" \\\n+\t\t--to=nobody@example.com \\\n+\t\t--smtp-server=\"$(pwd)/fake.sendmail\" $patches 2>errors &&\n+\n \ttest_cmp expected commandline1\n '\n \n"},{"id":"288738","messageId":"xmqqinxjmvop.fsf@gitster.mtv.corp.google.com","threadId":"42426","inReplyTo":"20160608130142.29879-3-samuel.groot@grenoble-inp.org","subject":"Re: [PATCH v4 2/6] t9001: check email address is in Cc: field","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-08T17:34:14Z","receivedAt":"2016-06-08T17:34:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel GROOT writes:\n\n> Check if the given utf-8 email address is in the Cc: field.\n>\n> Signed-off-by: Tom RUSSELLO \n> Signed-off-by: Samuel GROOT \n> Signed-off-by: Matthieu MOY \n> ---\n> t/t9001-send-email.sh | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n> index 56ad8ce..943e6b7 100755\n> --- a/t/t9001-send-email.sh\n> +++ b/t/t9001-send-email.sh\n> @@ -888,8 +888,8 @@ test_expect_success $PREREQ 'utf8 Cc is rfc2047 encoded' '\n> \t--to=nobody@example.com \\\n> \t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n> \toutdir/*.patch &&\n> -\tgrep \"^\t\" msgtxt1 |\n> -\tgrep \"=?UTF-8?q?=C3=A0=C3=A9=C3=AC=C3=B6=C3=BA?= \"\n> +\tcc_adr=$(awk \"/^Cc: /{flag=1}/^Subject: /{flag=0} flag {print}\" msgtxt1) &&\n> +\techo \"$cc_adr\" | fgrep \"=?UTF-8?q?=C3=A0=C3=A9=C3=AC=C3=B6=C3=BA?= \"\n> '\n\nThis still depends on that the output has Cc: before Subject: and\nthere is no other header that can have an address on it. E.g.\n\n\tTo: a@example.com\n Cc: b@example.com\n X-foo: <>\n Subject: [PATCH] A sample patch\n\nwould still say that the address is _on_ the CC: list.\n\nI do not usually do awk, but I think you should be able to avoid\ncapturing output from it, echoing and then grepping, which is way\ntoo ugly. Perhaps you can start from something like below?\n\n#!/bin/sh\nawk '\n\tBEGIN { in_cc = 0 }\n\t/^[Cc][Cc]: / {\n\t\tsub(\"^[Cc][Cc]: *\", \"\")\n\t\tin_cc = 1\n\t}\n\t/^[^ \t]*:/ {\n\t\tin_cc = 0\n\t}\n\t/^$/ { exit }\n\tin_cc {\n\t\tsub(\"^ *\", \"\")\n\t\tsub(\", *$\", \"\")\n\t\tprint\n\t}\n' <<\\EOF\nTo: a@example.com\nCc: b@example.com,\n c@example.com,\n d@example.com\nX-foo: e@example.com\nSubject: [PATCH] A sample patch\n\nCc: foo@example.com\nEOF\n"},{"id":"288737","messageId":"xmqqeg87mviz.fsf@gitster.mtv.corp.google.com","threadId":"42426","inReplyTo":"20160608130142.29879-4-samuel.groot@grenoble-inp.org","subject":"Re: [PATCH v4 3/6] send-email: shorten send-email's output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-08T17:37:40Z","receivedAt":"2016-06-08T17:37:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel GROOT writes:\n\n> +\t\t\t\tprintf(\"Adding cc: %s from From: header\\n\",\n> +\t\t\t\t\t$1) unless $quiet;\n\n> +\t\t\t\t\tprintf(\"Adding to: %s from To: header\\n\",\n> +\t\t\t\t\t\t$addr) unless $quiet;\n\n> +\t\t\t\t\tprintf(\"Adding cc: %s from Cc: header\\n\",\n> +\t\t\t\t\t\t$addr) unless $quiet;\n> \t\t\t\t\tpush @cc, $addr;\n\n> +\t\t\t\tprintf(\"Adding cc: %s from Cc: header\\n\",\n> +\t\t\t\t\t$_) unless $quiet;\n\nThese make the end result prettier by not repeating the same address\ntwice, but is it just me who finds these inexplicable case\ndifferences irritating? Shouldn't these field references in the\nresult mirror the field references in the origin of the information?\n"},{"id":"288746","messageId":"xmqq8tyfmuk4.fsf@gitster.mtv.corp.google.com","threadId":"42426","inReplyTo":"20160608130142.29879-5-samuel.groot@grenoble-inp.org","subject":"Re: [PATCH v4 4/6] send-email: create email parser subroutine","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-08T17:58:35Z","receivedAt":"2016-06-08T17:58:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel GROOT writes:\n\n> +sub parse_email {\n> +\tmy %mail = ();\n> +\tmy $fh = shift;\n> +\tmy $last_header;\n\n> +\t# Unfold and parse multiline header fields\n> +\twhile (<$fh>) {\n> +\t\tlast if /^\\s*$/;\n\nYou stop at the end of fields before the message body starts.\n\n> +\t\ts/\\r\\n|\\n|\\r//;\n\nThe pattern is not anchored at the right end of the string;\nintended? Is it worth worrying about a lone '\\r'?\n\n> +\t\tif (/^([^\\s:]+):[\\s]+(.*)$/) {\n> +\t\t\t$last_header = lc($1);\n> +\t\t\t@{$mail{$last_header}} = ()\n> +\t\t\t\tunless defined $mail{$last_header};\n> +\t\t\tpush @{$mail{$last_header}}, $2;\n\n> +\t\t} elsif (/^\\s+\\S/ and defined $last_header) {\n> +\t\t\ts/^\\s+/ /;\n> +\t\t\tpush @{$mail{$last_header}}, $_;\n\nEven though the comment said \"unfold\", you do not really do the\nunfolding here and the caller can (if it wants to) figure out where\none logical header was folded in the original into multiple physical\nlines, because you are returning an arrayref.\n\nHowever, that means the caller still cannot tell what the original\nwas if you are given:\n\n\tX-header: a b\n c\n\tX-header: d\n\nas you would return { 'X-header' => [\"a b\", \"c\", \"d\")] }\n\nIn that sense, it may be better to do a real unfolding here, so that\nit would return { 'X-header' => [\"a b c\", \"d\"] } from here instead?\n\nI.e. instead of \"push @{...}, $_\", append $_ to the last element of\nthat array?\n\n> +\t\t} else {\n> +\t\t\tdie(\"Mail format undefined!\\n\");\n\nWhat does that mean? It would probably help if you included the\nline that the code did not understand in the message.\n\n\n> +\t\t}\n> +\t}\n> +\n> +\t# Separate body from header\n> +\t$mail{\"body\"} = [(<$fh>)];\n> +\n> +\treturn \\%mail;\n\nThe name of the local thing is not observable from the caller, but\nbecause this is \"parse-email-header\" and returns \"header fields\"\nwithout reading the \"mail\", perhaps call it %header instead?\n\n> +}\n"},{"id":"288748","messageId":"CAPig+cTO+-aATxyNBt2HtctH_ofgqEc8ik3OLSN+THVgu6dhKQ@mail.gmail.com","threadId":"42426","inReplyTo":"xmqq8tyfmuk4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 4/6] send-email: create email parser subroutine","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-06-08T18:12:06Z","receivedAt":"2016-06-08T18:12:06Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jun 8, 2016 at 1:58 PM, Junio C Hamano wrote:\n> Samuel GROOT writes:\n>> +sub parse_email {\n>> + my %mail = ();\n>> + my $fh = shift;\n>> + my $last_header;\n>\n>> + # Unfold and parse multiline header fields\n>> + while (<$fh>) {\n>> + last if /^\\s*$/;\n>\n> You stop at the end of fields before the message body starts.\n>\n>> + s/\\r\\n|\\n|\\r//;\n>\n> The pattern is not anchored at the right end of the string;\n> intended? Is it worth worrying about a lone '\\r'?\n\nThanks, I think you covered pretty much everything I was going to say.\nI'd just add that if the matching is going to be kept loose like this\n(rather than anchoring it), then s/[\\r\\n]+//g might be easier to read,\nbut it's a minor point.\n\n>> + if (/^([^\\s:]+):[\\s]+(.*)$/) {\n>> + $last_header = lc($1);\n>> + @{$mail{$last_header}} = ()\n>> + unless defined $mail{$last_header};\n>> + push @{$mail{$last_header}}, $2;\n>\n>> + } elsif (/^\\s+\\S/ and defined $last_header) {\n>> + s/^\\s+/ /;\n>> + push @{$mail{$last_header}}, $_;\n>\n> Even though the comment said \"unfold\", you do not really do the\n> unfolding here and the caller can (if it wants to) figure out where\n> one logical header was folded in the original into multiple physical\n> lines, because you are returning an arrayref.\n\nAlso, the comment about folding lines should be moved down the part of\nthe code which is actually (supposed to be) doing the folding rather\nthan having the comment at the top of the loop.\n\n> However, that means the caller still cannot tell what the original\n> was if you are given:\n>\n> X-header: a b\n> c\n> X-header: d\n>\n> as you would return { 'X-header' => [\"a b\", \"c\", \"d\")] }\n>\n> In that sense, it may be better to do a real unfolding here, so that\n> it would return { 'X-header' => [\"a b c\", \"d\"] } from here instead?\n>\n> I.e. instead of \"push @{...}, $_\", append $_ to the last element of\n> that array?\n\nRight.\n\n>> + # Separate body from header\n>> + $mail{\"body\"} = [(<$fh>)];\n>> +\n>> + return \\%mail;\n>\n> The name of the local thing is not observable from the caller, but\n> because this is \"parse-email-header\" and returns \"header fields\"\n> without reading the \"mail\", perhaps call it %header instead?\n\nIf there is (for some reason) a mail header named 'body', then this\nassignment of the body portion of the message will overwrite it.\nPerhaps this function should instead return multiple values: the\nheader hash, and the message body.\n"},{"id":"288749","messageId":"xmqqvb1jletl.fsf@gitster.mtv.corp.google.com","threadId":"42426","inReplyTo":"20160608130736.32163-1-samuel.groot@grenoble-inp.org","subject":"Re: [PATCH v4 5/6] send-email: --in-reply-to= populate header fields","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-08T18:23:50Z","receivedAt":"2016-06-08T18:23:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel GROOT writes:\n\n> +if ($initial_reply_to && -f $initial_reply_to) {\n> +\tmy $error = validate_patch($initial_reply_to);\n\nThis call is wrong, isn't it?\n\nYou are not going to send out the message you are responding to (the\nmessage may not even be a patch), and you do not want to die with an\nerror message that says \"patch contains an overlong line\".\n\n> +\tdie \"fatal: $initial_reply_to: $error\\nwarning: no patches were sent\\n\"\n> +\t\tif $error;\n> +\n> +\topen my $fh, \"<\", $initial_reply_to or die \"can't open file $initial_reply_to\";\n> +\tmy $mail = Git::parse_email($fh);\n> +\tclose $fh;\n\n\tmy $header = Git::parse_email_header($fh);\n\nperhaps?\n\n> +\tmy $initial_sender = $sender || $repoauthor || $repocommitter || '';\n> +\n> +\tmy $prefix_re = \"\";\n> +\tmy $subject_re = $mail->{\"subject\"}[0];\n> +\tif ($subject_re =~ /^[^Re:]/) {\n> +\t\t$prefix_re = \"Re: \";\n> +\t}\n> +\t$initial_subject = $prefix_re . $subject_re;\n\nI am not sure what the significance of the fact that the subject\nhappens to begin with a letter other than 'R', 'e', or ':'.\n\nDid you mean to do something like this instead?\n\n\tmy $subject = $mail->{\"subject\"}[0];\n\t$subject =~ s/^(re:\\s*)+//i; # strip \"Re: Re: ...\"\n $initial_subject = \"Re: $subject\";\n\ninstead?\n\nBy the way, this is a good example why your \"unfold\" implementation\nin 4/6 is unwieldy for the caller. Imagine a rather long subject\nthat is folded, i.e.\n\n\tTo: Samuel\n Subject: Help! I am having a trouble running git-send-email\n correctly.\n\tMessage-id: <...>\n"},{"id":"288751","messageId":"xmqqr3c7lefw.fsf@gitster.mtv.corp.google.com","threadId":"42426","inReplyTo":"CAPig+cTO+-aATxyNBt2HtctH_ofgqEc8ik3OLSN+THVgu6dhKQ@mail.gmail.com","subject":"Re: [PATCH v4 4/6] send-email: create email parser subroutine","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-08T18:32:03Z","receivedAt":"2016-06-08T18:32:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n>>> + # Separate body from header\n>>> + $mail{\"body\"} = [(<$fh>)];\n>>> +\n>>> + return \\%mail;\n>>\n>> The name of the local thing is not observable from the caller, but\n>> because this is \"parse-email-header\" and returns \"header fields\"\n>> without reading the \"mail\", perhaps call it %header instead?\n>\n> If there is (for some reason) a mail header named 'body', then this\n> assignment of the body portion of the message will overwrite it.\n> Perhaps this function should instead return multiple values: the\n> header hash, and the message body.\n\nAh, I missed that it is attempting to return the body, too.\n\nBecause the function takes an open filehandle, I think it is better\nto leave it to the callers. A caller that is only interested in\nheaders can just close $fh after this helper returns without reading\nbody that it is not interested in, and a caller that wants to read\nthe body can do the slurping itself.\n"},{"id":"288755","messageId":"4253fcd6-dba5-fc9a-d63e-e40ba10bb9e7@grenoble-inp.org","threadId":"42426","inReplyTo":"xmqqeg87mviz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 3/6] send-email: shorten send-email's output","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T19:18:35Z","receivedAt":"2016-06-08T19:18:35Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/08/2016 07:37 PM, Junio C Hamano wrote:\n> Samuel GROOT writes:\n>> +\t\t\t\tprintf(\"Adding cc: %s from From: header\\n\",\n>> +\t\t\t\t\t$1) unless $quiet;\n>\n>> +\t\t\t\t\tprintf(\"Adding to: %s from To: header\\n\",\n>> +\t\t\t\t\t\t$addr) unless $quiet;\n>\n>> +\t\t\t\t\tprintf(\"Adding cc: %s from Cc: header\\n\",\n>> +\t\t\t\t\t\t$addr) unless $quiet;\n>> \t\t\t\t\tpush @cc, $addr;\n>\n>> +\t\t\t\tprintf(\"Adding cc: %s from Cc: header\\n\",\n>> +\t\t\t\t\t$_) unless $quiet;\n>\n> These make the end result prettier by not repeating the same address\n> twice, but is it just me who finds these inexplicable case\n> differences irritating? Shouldn't these field references in the\n> result mirror the field references in the origin of the information?\n\nIt makes sense only in the case below...\n\n >> +\t\tprintf(\"Adding cc: %s from From: header\\n\",\n >> +\t\t\t$1) unless $quiet;\n\n... because the sender should receive its own copy (at least to avoid \nbreaking threaded view in his mailer) and be cc-ed. By the way, we \nshould cc the sender when sending the cover letter too for the same reason.\n\nBut in other cases, it seems pointless to display identical field \nreference twice.\n"},{"id":"288757","messageId":"a0222724-673f-c07d-96fa-fab227955858@grenoble-inp.org","threadId":"42426","inReplyTo":"xmqqmvmvmwh7.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 1/6] t9001: non order-sensitive file comparison","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T19:19:38Z","receivedAt":"2016-06-08T19:19:38Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/08/2016 07:17 PM, Junio C Hamano wrote:\n> Here is an illustration of an organization that I think would be\n> easier to read, and would result in a more readable patch when\n> modification is made on top. The first two hunks collapse the\n> overall \"setup\" steps that appear as three separate tests into a\n> single \"setup\" test. The last hunk that begin at -83/+79 collapses\n> a logically-single test that is split across three into one, and\n> makes the order of things done in the test to (1) set an\n> expectation, (2) execute the command and (3) compare the result with\n> the expectation.\n\nI totally agree. (1), (2) and (3) aren't even always in that order, some \ntests are very confusing.\n\n> I am not going to commit this myself, because I do not want to\n> create conflicts with the change your topic is trying to do, and\n> besides, almost all the remainder of the tests follow \"one logical\n> test split into three\" pattern and need to be corrected before this\n> \"illustration\" can become a real patch.\n>\n> I do not mind if you take it and complete it as a preliminary\n> clean-up step in your series; or you can \"keep it in mind, but\n> ignore it for now\", in which case this can be a \"low hanging fruit\"\n> somebody else, hopefully somebody new to the development community,\n> can use to dip their toes ;-)\n\nAs said in my other reply, I will put this one aside for now, but t9001 \ndefinitely deserves its own cleanup patch series.\n"},{"id":"288758","messageId":"596b7473-817a-1a92-8253-ca77a7bd88a8@grenoble-inp.org","threadId":"42426","inReplyTo":"xmqqr3c7mxg4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 1/6] t9001: non order-sensitive file comparison","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T19:21:45Z","receivedAt":"2016-06-08T19:21:45Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/08/2016 06:56 PM, Junio C Hamano wrote:\n > The earliest address patch@example.com and later addresses have\n > quite different meaning (the first one is meant to be the envelope\n > sender address, and does not name a recipient). While I think it is\n > a good idea to tell the test that the order of recipient addresses\n > given to the sendmail command (i.e. nobody, author, one and two)\n > does not matter by comparing sorted list of addresses, sorting the\n > whole argument list and comparing is making the test _too_ loose.\n > Don't you want to catch a potential bug that adds the envelope\n > sender address to the list of recipients by mistake, for example?\n\nThat could be an idea to make a more precise test. But I rather focus on \nour `--quote-email` option, I will put this one aside for now.\n"},{"id":"288759","messageId":"6b2b9023-b50e-9bbd-840a-ec7142ec5a75@grenoble-inp.org","threadId":"42426","inReplyTo":"xmqqinxjmvop.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 2/6] t9001: check email address is in Cc: field","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T19:23:44Z","receivedAt":"2016-06-08T19:23:44Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/08/2016 07:34 PM, Junio C Hamano wrote:\n> Samuel GROOT writes:\n>> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\n>> index 56ad8ce..943e6b7 100755\n>> --- a/t/t9001-send-email.sh\n>> +++ b/t/t9001-send-email.sh\n>> @@ -888,8 +888,8 @@ test_expect_success $PREREQ 'utf8 Cc is rfc2047 encoded' '\n>> \t--to=nobody@example.com \\\n>> \t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n>> \toutdir/*.patch &&\n>> -\tgrep \"^\t\" msgtxt1 |\n>> -\tgrep \"=?UTF-8?q?=C3=A0=C3=A9=C3=AC=C3=B6=C3=BA?= \"\n>> +\tcc_adr=$(awk \"/^Cc: /{flag=1}/^Subject: /{flag=0} flag {print}\" msgtxt1) &&\n>> +\techo \"$cc_adr\" | fgrep \"=?UTF-8?q?=C3=A0=C3=A9=C3=AC=C3=B6=C3=BA?= \"\n>> '\n>\n> This still depends on that the output has Cc: before Subject: and\n> there is no other header that can have an address on it. E.g.\n>\n> \tTo: a@example.com\n> Cc: b@example.com\n> X-foo: <>\n> Subject: [PATCH] A sample patch\n>\n> would still say that the address is _on_ the CC: list.\n\nWe thought of that but did not find the proper way to do it.\n\n> I do not usually do awk, but I think you should be able to avoid\n> capturing output from it, echoing and then grepping, which is way\n> too ugly. Perhaps you can start from something like below?\n>\n> #!/bin/sh\n> awk '\n> \tBEGIN { in_cc = 0 }\n> \t/^[Cc][Cc]: / {\n> \t\tsub(\"^[Cc][Cc]: *\", \"\")\n> \t\tin_cc = 1\n> \t}\n> \t/^[^ \t]*:/ {\n> \t\tin_cc = 0\n> \t}\n> \t/^$/ { exit }\n> \tin_cc {\n> \t\tsub(\"^ *\", \"\")\n> \t\tsub(\", *$\", \"\")\n> \t\tprint\n> \t}\n> ' <<\\EOF\n> To: a@example.com\n> Cc: b@example.com,\n> c@example.com,\n> d@example.com\n> X-foo: e@example.com\n> Subject: [PATCH] A sample patch\n>\n> Cc: foo@example.com\n> EOF\n\nThanks, I will work on that :-)\n"},{"id":"288760","messageId":"116d56ee-afdf-b1f2-f141-7449e6503f30@grenoble-inp.org","threadId":"42426","inReplyTo":"xmqqr3c7lefw.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 4/6] send-email: create email parser subroutine","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T19:26:44Z","receivedAt":"2016-06-08T19:26:44Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/08/2016 08:32 PM, Junio C Hamano wrote:\n> Eric Sunshine writes:\n>>>> + # Separate body from header\n>>>> + $mail{\"body\"} = [(<$fh>)];\n>>>> +\n>>>> + return \\%mail;\n>>>\n>>> The name of the local thing is not observable from the caller, but\n>>> because this is \"parse-email-header\" and returns \"header fields\"\n>>> without reading the \"mail\", perhaps call it %header instead?\n>>\n>> If there is (for some reason) a mail header named 'body', then this\n>> assignment of the body portion of the message will overwrite it.\n>> Perhaps this function should instead return multiple values: the\n>> header hash, and the message body.\n>\n> Ah, I missed that it is attempting to return the body, too.\n>\n> Because the function takes an open filehandle, I think it is better\n> to leave it to the callers. A caller that is only interested in\n> headers can just close $fh after this helper returns without reading\n> body that it is not interested in, and a caller that wants to read\n> the body can do the slurping itself.\n\nI think it's the best way to do it indeed. Furthermore, we did trim CRs \nand LFs in header fields, but not in the message, making the subroutine \ninconsistent.\n\nShould we rename the subroutine to `parse_header` or leave it as it is?\n"},{"id":"288761","messageId":"fb7795c4-60db-94b3-a584-8fadd0440773@grenoble-inp.org","threadId":"42426","inReplyTo":"CAPig+cTO+-aATxyNBt2HtctH_ofgqEc8ik3OLSN+THVgu6dhKQ@mail.gmail.com","subject":"Re: [PATCH v4 4/6] send-email: create email parser subroutine","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T19:30:21Z","receivedAt":"2016-06-08T19:30:21Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/08/2016 08:12 PM, Eric Sunshine wrote:\n> On Wed, Jun 8, 2016 at 1:58 PM, Junio C Hamano wrote:\n>> Samuel GROOT writes:\n>>> +sub parse_email {\n>>> + my %mail = ();\n>>> + my $fh = shift;\n>>> + my $last_header;\n>>\n>>> + # Unfold and parse multiline header fields\n>>> + while (<$fh>) {\n>>> + last if /^\\s*$/;\n>>\n>> You stop at the end of fields before the message body starts.\n>>\n>>> + s/\\r\\n|\\n|\\r//;\n>>\n>> The pattern is not anchored at the right end of the string;\n>> intended? Is it worth worrying about a lone '\\r'?\n>\n> Thanks, I think you covered pretty much everything I was going to say.\n> I'd just add that if the matching is going to be kept loose like this\n> (rather than anchoring it), then s/[\\r\\n]+//g might be easier to read,\n> but it's a minor point.\n\nIndeed s/[\\r\\n]+//g is way better, it works even if there's a CR in the \nmiddle of the line.\n\n>>> + if (/^([^\\s:]+):[\\s]+(.*)$/) {\n>>> + $last_header = lc($1);\n>>> + @{$mail{$last_header}} = ()\n>>> + unless defined $mail{$last_header};\n>>> + push @{$mail{$last_header}}, $2;\n>>\n>>> + } elsif (/^\\s+\\S/ and defined $last_header) {\n>>> + s/^\\s+/ /;\n>>> + push @{$mail{$last_header}}, $_;\n>>\n>> Even though the comment said \"unfold\", you do not really do the\n>> unfolding here and the caller can (if it wants to) figure out where\n>> one logical header was folded in the original into multiple physical\n>> lines, because you are returning an arrayref.\n>\n> Also, the comment about folding lines should be moved down the part of\n> the code which is actually (supposed to be) doing the folding rather\n> than having the comment at the top of the loop.\n\nWill do in next re-roll.\n\n>>> + # Separate body from header\n>>> + $mail{\"body\"} = [(<$fh>)];\n>>> +\n>>> + return \\%mail;\n>>\n>> The name of the local thing is not observable from the caller, but\n>> because this is \"parse-email-header\" and returns \"header fields\"\n>> without reading the \"mail\", perhaps call it %header instead?\n>\n> If there is (for some reason) a mail header named 'body', then this\n> assignment of the body portion of the message will overwrite it.\n> Perhaps this function should instead return multiple values: the\n> header hash, and the message body.\n\nI will drop the body part in re-roll.\n"},{"id":"288762","messageId":"xmqqbn3blbpc.fsf@gitster.mtv.corp.google.com","threadId":"42426","inReplyTo":"116d56ee-afdf-b1f2-f141-7449e6503f30@grenoble-inp.org","subject":"Re: [PATCH v4 4/6] send-email: create email parser subroutine","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-08T19:31:11Z","receivedAt":"2016-06-08T19:31:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel GROOT writes:\n\n> I think it's the best way to do it indeed. Furthermore, we did trim\n> CRs and LFs in header fields, but not in the message, making the\n> subroutine inconsistent.\n>\n> Should we rename the subroutine to `parse_header` or leave it as it is?\n\nIf it lives inside git-send-email, then parse_header is sufficient\nas everybody would know it is about e-mail without being told. If\nit is in Git.pm, then parse_email_header would be more appropriate.\n"},{"id":"288763","messageId":"xmqq7fdzlbld.fsf@gitster.mtv.corp.google.com","threadId":"42426","inReplyTo":"4253fcd6-dba5-fc9a-d63e-e40ba10bb9e7@grenoble-inp.org","subject":"Re: [PATCH v4 3/6] send-email: shorten send-email's output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-08T19:33:34Z","receivedAt":"2016-06-08T19:33:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel GROOT writes:\n\n> On 06/08/2016 07:37 PM, Junio C Hamano wrote:\n>> Samuel GROOT writes:\n>>> +\t\t\t\tprintf(\"Adding cc: %s from From: header\\n\",\n>>> +\t\t\t\t\t$1) unless $quiet;\n>>\n>>> +\t\t\t\t\tprintf(\"Adding to: %s from To: header\\n\",\n>>> +\t\t\t\t\t\t$addr) unless $quiet;\n>>\n>>> +\t\t\t\t\tprintf(\"Adding cc: %s from Cc: header\\n\",\n>>> +\t\t\t\t\t\t$addr) unless $quiet;\n>>> \t\t\t\t\tpush @cc, $addr;\n>>\n>>> +\t\t\t\tprintf(\"Adding cc: %s from Cc: header\\n\",\n>>> +\t\t\t\t\t$_) unless $quiet;\n>>\n>> These make the end result prettier by not repeating the same address\n>> twice, but is it just me who finds these inexplicable case\n>> differences irritating? Shouldn't these field references in the\n>> result mirror the field references in the origin of the information?\n>\n> It makes sense only in the case below...\n>\n>>> +\t\tprintf(\"Adding cc: %s from From: header\\n\",\n>>> +\t\t\t$1) unless $quiet;\n>\n> ... because the sender should receive its own copy (at least to avoid\n> breaking threaded view in his mailer) and be cc-ed. By the way, we\n> should cc the sender when sending the cover letter too for the same\n> reason.\n>\n> But in other cases, it seems pointless to display identical field\n> reference twice.\n\nMy comment may have been a bit too oblique. What I meant was\n\n\tAdding cc: Samuel from From: header\n\nlooked strange, and I thought it would be better written\n\n\tAdding Cc: Samuel from From: header\n\nSame for\n\n\tAdding to: Samuel from To: header\n\nbeing strange, and a better version of it would be\n\n\tAdding To: Samuel from To: header\n"},{"id":"288764","messageId":"c5bd7c4f-614c-14c1-1395-2bd0e5c1d81c@grenoble-inp.org","threadId":"42426","inReplyTo":"xmqq8tyfmuk4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 4/6] send-email: create email parser subroutine","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T19:36:46Z","receivedAt":"2016-06-08T19:36:46Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/08/2016 07:58 PM, Junio C Hamano wrote:\n> Samuel GROOT writes:\n>> +sub parse_email {\n>> +\tmy %mail = ();\n>> +\tmy $fh = shift;\n>> +\tmy $last_header;\n>\n>> +\t# Unfold and parse multiline header fields\n>> +\twhile (<$fh>) {\n>> +\t\tlast if /^\\s*$/;\n>\n> You stop at the end of fields before the message body starts.\n>\n>> +\t\ts/\\r\\n|\\n|\\r//;\n>\n> The pattern is not anchored at the right end of the string;\n> intended? Is it worth worrying about a lone '\\r'?\n\nA lone '\\r' shouldn't happen, but we are never too careful. It's fixed \nwith what Eric suggested.\n\n>> +\t\tif (/^([^\\s:]+):[\\s]+(.*)$/) {\n>> +\t\t\t$last_header = lc($1);\n>> +\t\t\t@{$mail{$last_header}} = ()\n>> +\t\t\t\tunless defined $mail{$last_header};\n>> +\t\t\tpush @{$mail{$last_header}}, $2;\n>\n>> +\t\t} elsif (/^\\s+\\S/ and defined $last_header) {\n>> +\t\t\ts/^\\s+/ /;\n>> +\t\t\tpush @{$mail{$last_header}}, $_;\n>\n> Even though the comment said \"unfold\", you do not really do the\n> unfolding here and the caller can (if it wants to) figure out where\n> one logical header was folded in the original into multiple physical\n> lines, because you are returning an arrayref.\n>\n> However, that means the caller still cannot tell what the original\n> was if you are given:\n>\n> \tX-header: a b\n> c\n> \tX-header: d\n>\n> as you would return { 'X-header' => [\"a b\", \"c\", \"d\")] }\n>\n> In that sense, it may be better to do a real unfolding here, so that\n> it would return { 'X-header' => [\"a b c\", \"d\"] } from here instead?\n>\n> I.e. instead of \"push @{...}, $_\", append $_ to the last element of\n> that array?\n\nI will do that. It makes more sense regarding Subject split into several \nlines, for example.\n"},{"id":"288765","messageId":"9cb7c4aa-56e5-e4e7-a75f-d21943e8342e@grenoble-inp.org","threadId":"42426","inReplyTo":"xmqq7fdzlbld.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 3/6] send-email: shorten send-email's output","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T19:40:02Z","receivedAt":"2016-06-08T19:40:02Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/08/2016 09:33 PM, Junio C Hamano wrote:\n> Samuel GROOT writes:\n>\n>> On 06/08/2016 07:37 PM, Junio C Hamano wrote:\n>>> Samuel GROOT writes:\n>>>> +\t\t\t\tprintf(\"Adding cc: %s from From: header\\n\",\n>>>> +\t\t\t\t\t$1) unless $quiet;\n>>>\n>>>> +\t\t\t\t\tprintf(\"Adding to: %s from To: header\\n\",\n>>>> +\t\t\t\t\t\t$addr) unless $quiet;\n>>>\n>>>> +\t\t\t\t\tprintf(\"Adding cc: %s from Cc: header\\n\",\n>>>> +\t\t\t\t\t\t$addr) unless $quiet;\n>>>> \t\t\t\t\tpush @cc, $addr;\n>>>\n>>>> +\t\t\t\tprintf(\"Adding cc: %s from Cc: header\\n\",\n>>>> +\t\t\t\t\t$_) unless $quiet;\n>>>\n>>> These make the end result prettier by not repeating the same address\n>>> twice, but is it just me who finds these inexplicable case\n>>> differences irritating? Shouldn't these field references in the\n>>> result mirror the field references in the origin of the information?\n>>\n>> It makes sense only in the case below...\n>>\n>>>> +\t\tprintf(\"Adding cc: %s from From: header\\n\",\n>>>> +\t\t\t$1) unless $quiet;\n>>\n>> ... because the sender should receive its own copy (at least to avoid\n>> breaking threaded view in his mailer) and be cc-ed. By the way, we\n>> should cc the sender when sending the cover letter too for the same\n>> reason.\n>>\n>> But in other cases, it seems pointless to display identical field\n>> reference twice.\n>\n> My comment may have been a bit too oblique. What I meant was\n>\n> \tAdding cc: Samuel from From: header\n>\n> looked strange, and I thought it would be better written\n>\n> \tAdding Cc: Samuel from From: header\n>\n> Same for\n>\n> \tAdding to: Samuel from To: header\n>\n> being strange, and a better version of it would be\n>\n> \tAdding To: Samuel from To: header\n\nOh, I read your email a bit too fast, sorry.\n\nI kept the sentence as it was except for trimmed part, but it makes \nsense to have the same case. It will be fixed :-)\n"},{"id":"288767","messageId":"2d60a12f-5709-687c-4fc4-b6a0b4719b13@grenoble-inp.org","threadId":"42426","inReplyTo":"xmqqbn3blbpc.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 4/6] send-email: create email parser subroutine","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T19:42:08Z","receivedAt":"2016-06-08T19:42:08Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/08/2016 09:31 PM, Junio C Hamano wrote:\n> Samuel GROOT writes:\n>\n>> I think it's the best way to do it indeed. Furthermore, we did trim\n>> CRs and LFs in header fields, but not in the message, making the\n>> subroutine inconsistent.\n>>\n>> Should we rename the subroutine to `parse_header` or leave it as it is?\n>\n> If it lives inside git-send-email, then parse_header is sufficient\n> as everybody would know it is about e-mail without being told. If\n> it is in Git.pm, then parse_email_header would be more appropriate.\n\nIt currently lives in Git.pm, following Eric Wong's advice to have a \nmore packaged code.\n"},{"id":"288770","messageId":"CAPig+cQ5OKFTA8pEge=0e2RMT3ghmat-szw73aO_3WVW1=eAFQ@mail.gmail.com","threadId":"42426","inReplyTo":"fb7795c4-60db-94b3-a584-8fadd0440773@grenoble-inp.org","subject":"Re: [PATCH v4 4/6] send-email: create email parser subroutine","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-06-08T20:13:04Z","receivedAt":"2016-06-08T20:13:04Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jun 8, 2016 at 3:30 PM, Samuel GROOT\n wrote:\n> On 06/08/2016 08:12 PM, Eric Sunshine wrote:\n>> On Wed, Jun 8, 2016 at 1:58 PM, Junio C Hamano wrote:\n>>> Samuel GROOT writes:\n>>> The pattern is not anchored at the right end of the string;\n>>> intended? Is it worth worrying about a lone '\\r'?\n>>\n>> Thanks, I think you covered pretty much everything I was going to say.\n>> I'd just add that if the matching is going to be kept loose like this\n>> (rather than anchoring it), then s/[\\r\\n]+//g might be easier to read,\n>> but it's a minor point.\n>\n> Indeed s/[\\r\\n]+//g is way better, it works even if there's a CR in the\n> middle of the line.\n\nAn embedded CR probably shouldn't happen, but I'm not convinced that\nfolding it out is a good idea. I would think that you'd want to\npreserve the header's value verbatim. If anything, I'd expect to see\nthe regex tightened to:\n\n s/\\r?\\n$//;\n\nAlternately, consider using 'chop' or 'chomp'.\n"},{"id":"288771","messageId":"xmqqy46fjuzm.fsf@gitster.mtv.corp.google.com","threadId":"42426","inReplyTo":"CAPig+cQ5OKFTA8pEge=0e2RMT3ghmat-szw73aO_3WVW1=eAFQ@mail.gmail.com","subject":"Re: [PATCH v4 4/6] send-email: create email parser subroutine","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-08T20:17:33Z","receivedAt":"2016-06-08T20:17:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> An embedded CR probably shouldn't happen, but I'm not convinced that\n> folding it out is a good idea. I would think that you'd want to\n> preserve the header's value verbatim. If anything, I'd expect to see\n> the regex tightened to:\n>\n> s/\\r?\\n$//;\n\nYes, that would be more sensible than silently removing \\r in the\nmiddle which _is_ a sign of something funny going on.\n\n> Alternately, consider using 'chop' or 'chomp'.\n\nEven if you use chomp(), you'd still need to worry about possible \\r\nat the end, no?\n"},{"id":"288772","messageId":"20160608203858.GA513@dcvr.yhbt.net","threadId":"42426","inReplyTo":"20160608130142.29879-5-samuel.groot@grenoble-inp.org","subject":"Re: [PATCH v4 4/6] send-email: create email parser subroutine","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-06-08T20:38:58Z","receivedAt":"2016-06-08T20:38:58Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Samuel GROOT wrote:\n> +++ b/perl/Git.pm\n\n> +sub parse_email {\n> +\tmy %mail = ();\n> +\tmy $fh = shift;\n> +\tmy $last_header;\n> +\n> +\t# Unfold and parse multiline header fields\n\nWhen you libify, I suggest you localize $/ since $/\nmay be set to something other than \"\\n\" by a caller\nand change the behavior of <$fh> and $fh->getline.\n\n\tlocal $/ = \"\\n\";\n\n> +\twhile (<$fh>) {\n> +\t\tlast if /^\\s*$/;\n> +\t\ts/\\r\\n|\\n|\\r//;\n\nAnd, as Eric Sunshine stated:\n\n\t\ts/\\r?\\n$//;\n\nExplicitly localizing $/ means you wouldn't have to worry about\nmultiple \\n showing up in the line, either.\nAnd chomp/chop wouldn't work, here.\n\nOtherwise I like the move to Git.pm, thanks.\n"},{"id":"288781","messageId":"4c1dcf95-d8bb-4649-8980-fa947617b96e@grenoble-inp.org","threadId":"42426","inReplyTo":"xmqqy46fjuzm.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 4/6] send-email: create email parser subroutine","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-08T23:54:03Z","receivedAt":"2016-06-08T23:54:03Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/08/2016 10:17 PM, Junio C Hamano wrote:\n> Eric Sunshine writes:\n>\n>> An embedded CR probably shouldn't happen, but I'm not convinced that\n>> folding it out is a good idea. I would think that you'd want to\n>> preserve the header's value verbatim. If anything, I'd expect to see\n>> the regex tightened to:\n>>\n>> s/\\r?\\n$//;\n>\n> Yes, that would be more sensible than silently removing \\r in the\n> middle which _is_ a sign of something funny going on.\n>> Alternately, consider using 'chop' or 'chomp'.\n>\n> Even if you use chomp(), you'd still need to worry about possible \\r\n> at the end, no?\n\n'chomp' is what we used before, but with *.eml files (microsoft's file \nformat, with CRLF), '\\n' were removed but '\\r' remained, that's why we \nused s/\\r\\n|\\r|\\n//.\n\ns/\\r?\\n$// looks fine.\n\nEmail::Simple library uses qr/\\x0a\\x0d|\\x0d\\x0a|\\x0a|\\x0d/ [1]. Should \nwe handle \\n\\r at end of line as well?\n\n[1] * \nhttp://cpansearch.perl.org/src/RJBS/Email-Simple-2.210/lib/Email/Simple.pm\n"},{"id":"288787","messageId":"20160609002150.GA28547@dcvr.yhbt.net","threadId":"42426","inReplyTo":"4c1dcf95-d8bb-4649-8980-fa947617b96e@grenoble-inp.org","subject":"Re: [PATCH v4 4/6] send-email: create email parser subroutine","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-06-09T00:21:50Z","receivedAt":"2016-06-09T00:21:50Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Samuel GROOT wrote:\n> Email::Simple library uses qr/\\x0a\\x0d|\\x0d\\x0a|\\x0a|\\x0d/ [1].\n> Should we handle \\n\\r at end of line as well?\n\n\"\\n\\r\" can never happen with local $/ = \"\\n\"\n \n> [1] * http://cpansearch.perl.org/src/RJBS/Email-Simple-2.210/lib/Email/Simple.pm\n"},{"id":"288789","messageId":"vpqporqyko1.fsf@anie.imag.fr","threadId":"42426","inReplyTo":"xmqqziqwmqth.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 1/6] t9001: non order-sensitive file comparison","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-06-09T05:51:26Z","receivedAt":"2016-06-16T02:19:49Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Tom Russello writes:\n>\n>> +# Check if two files have the same content, non-order sensitive\n>> +test_cmp_noorder () {\n>> +\tsort $1 >$1;\n>\n> Here is what I think happens:\n>\n> 0) the shell parses this command line;\n> 1) the shell notices that the output has to go to $1;\n> 2) the shell does open(2) of $1,\n> 3) the shell spawns \"sort\" with a single argument, with its\n> output connected to the file descriptor obtained in 2).\n>\n> Because \"$1\" becomes an empty file at 2), \"sort\" reads nothing and\n> writes nothing.\n\nTom: in case you're not convinced, try this:\n\n$ (echo b; echo a) >f\n$ sort f\na\nb\n$ sort f >f\n$ cat f\n$\n\nAlso, useless ';' and missing double-quotes around \"$1\" to avoid bad\nsurprises ifever test_cmp_noorder is called on file names with special\ncharacters.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"288790","messageId":"vpqinxiykgk.fsf@anie.imag.fr","threadId":"42426","inReplyTo":"20160607140148.23242-3-tom.russello@grenoble-inp.org","subject":"Re: [PATCH v3 2/6] t9001: check email address is in Cc: field","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-06-09T05:55:55Z","receivedAt":"2016-06-16T02:19:49Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Tom Russello writes:\n\n> Check if the given utf-8 email address is in the Cc: field.\n\nI wouldn't harm to explain what was the problem with existing code here.\nIf I understand correctly, that would be:\n\n Existing code just checked that the address appeared in a line starting\n with a space, but not to which field the address belonged.\n\nBut probably the real motivation for this was that you want to introduce\ncode that changes the layout and breaks this \"address appears on a line\nstarting with space\"?\n\n> --- a/t/t9001-send-email.sh\n> +++ b/t/t9001-send-email.sh\n> @@ -888,8 +888,8 @@ test_expect_success $PREREQ 'utf8 Cc is rfc2047 encoded' '\n> \t--to=nobody@example.com \\\n> \t--smtp-server=\"$(pwd)/fake.sendmail\" \\\n> \toutdir/*.patch &&\n> -\tgrep \"^\t\" msgtxt1 |\n> -\tgrep \"=?UTF-8?q?=C3=A0=C3=A9=C3=AC=C3=B6=C3=BA?= \"\n> +\tcc_adr=$(awk \"/^Cc: /{flag=1}/^Subject: /{flag=0} flag {print}\" msgtxt1) &&\n> +\techo \"$cc_adr\" | grep \"=?UTF-8?q?=C3=A0=C3=A9=C3=AC=C3=B6=C3=BA?= \"\n> '\n> \n> test_expect_success $PREREQ '--compose adds MIME for utf8 body' '\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"288791","messageId":"vpqbn3ayk79.fsf@anie.imag.fr","threadId":"42426","inReplyTo":"3090c61c-533b-7119-f5e4-7d99e62f6da4@grenoble-inp.org","subject":"Re: [PATCH v3 1/6] t9001: non order-sensitive file comparison","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-06-09T06:01:30Z","receivedAt":"2016-06-16T02:19:49Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Samuel GROOT writes:\n\n> On 06/08/2016 06:09 PM, Junio C Hamano wrote:\n>> Samuel GROOT writes:\n>>\n>>> Actually we had issues when trying to refactor send-email's email\n>>> parsing loop [1]. Email addresses in output file `commandeline1` in\n>>> tests weren't sorted the same way as the reference file it was\n>>> compared to. E.g.:\n>>>\n>>> !nobody@example.com!\n>>> !author@example.com!\n>>> !one@example.com!\n>>> !two@example.com!\n>>\n>> And the reason why these addresses that are collected from the same\n>> input (i.e. command line, existing e-mail fields, footers, etc.) are\n>> shown in different order in your implementation is...?\n>\n> It's not shown in different order in our implementation, it's just a\n> leftover of my refactor attempt [1].\n\nI think the refactoring makes sense, but having this patch as PATCH 1/6\nin a series about --in-reply-to confuses reviewers: they would expect\nthis patch to be useful to the others in the series.\n\nIf you have \"reply to a message in a file\" ready without the\nrefactoring, and a mostly ready refactoring, then I think it makes sense\nto have two patch series, the first being only \"reply to a message in a\nfile\". If the refactoring itself is not ready, you may send a separate\nseries \"tests clean up\" and explain on the cover-letter that it's, well,\nonly a test clean up.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"288792","messageId":"vpq4m92yk3f.fsf@anie.imag.fr","threadId":"42426","inReplyTo":"20160607140148.23242-4-tom.russello@grenoble-inp.org","subject":"Re: [PATCH v3 3/6] t9001: shorten send-email's output","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-06-09T06:03:48Z","receivedAt":"2016-06-16T02:19:49Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Tom Russello writes:\n\n> Messages displayed by `send-email` should be shortened to avoid displaying\n> unnecesseray informations.\n\nWe usually use imperative tone in commit messages:\n\nShorten messages displayed by `send-email` to avoid displaying\nunnecessary information.\n\nActually, I'd rather have a bit more explanation about why this info is\n\"unnecessary\".\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"288793","messageId":"vpqr3c6x4vz.fsf@anie.imag.fr","threadId":"42426","inReplyTo":"20160608130142.29879-4-samuel.groot@grenoble-inp.org","subject":"Re: [PATCH v4 3/6] send-email: shorten send-email's output","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-06-09T06:17:36Z","receivedAt":"2016-06-16T02:19:49Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Samuel GROOT writes:\n\n> @@ -647,10 +647,10 @@ test_expect_success $PREREQ '--suppress-cc=all' '\n> test_expect_success $PREREQ 'setup expect' \"\n> cat >expected-suppress-body <<\\EOF\n> 0001-Second.patch\n> -(mbox) Adding cc: A from line 'From: A '\n> -(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n> -(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n> -(cc-cmd) Adding cc: cc-cmd@example.com from: './cccmd'\n> +Adding cc: A from From: header\n> +Adding cc: One from Cc: header\n> +Adding cc: two@example.com from Cc: header\n> +Adding cc: cc-cmd@example.com from: './cccmd'\n\nThis hunk differs from the others a bit. I totally agree that removing\nthe (mbox) prefix makes sense, but you're removing (cc-cmd) here, which\ndid carry some information.\n\nI'd write it as\n\nAdding cc: cc-cmd@example.com from --cc-cmd: ./cccmd\n\nIt might make sense to split this into two patches: one for (mbox) +\nheaders and one for (cc-cmd) and (to-cmd). Spotting special-cases like\nthe above inside a long patch is hard for reviewers.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"288794","messageId":"CAPig+cSn5LQ0OxDbg5y=dv16VC-jCaehnss_14mc34o95QbEpw@mail.gmail.com","threadId":"42426","inReplyTo":"4c1dcf95-d8bb-4649-8980-fa947617b96e@grenoble-inp.org","subject":"Re: [PATCH v4 4/6] send-email: create email parser subroutine","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-06-09T06:51:30Z","receivedAt":"2016-06-16T02:19:49Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jun 8, 2016 at 7:54 PM, Samuel GROOT\n wrote:\n> On 06/08/2016 10:17 PM, Junio C Hamano wrote:\n>> Eric Sunshine writes:\n>>> An embedded CR probably shouldn't happen, but I'm not convinced that\n>>> folding it out is a good idea. I would think that you'd want to\n>>> preserve the header's value verbatim. If anything, I'd expect to see\n>>> the regex tightened to:\n>>>\n>>> s/\\r?\\n$//;\n>>\n>> Yes, that would be more sensible than silently removing \\r in the\n>> middle which _is_ a sign of something funny going on.\n>\n> s/\\r?\\n$// looks fine.\n>\n> Email::Simple library uses qr/\\x0a\\x0d|\\x0d\\x0a|\\x0a|\\x0d/ [1].\n> [1] * http://cpansearch.perl.org/src/RJBS/Email-Simple-2.210/lib/Email/Simple.pm\n\nYou could certainly use s/\\x0d?\\x0a$// rather than s/\\r?\\n$// to be\nreally robust, but I doubt it matters much today. The reason for using\nhex codes is that \\r and \\n will resolve to CR and LF in the local\ncharacter encoding, and those may not be 0x0d and 0x0a, respectively.\nI believe the canonical example given in the Camel book was EBCIDIC in\nwhich \\r is 0x0d, but \\n is 0x25, not 0x0a as it is in ASCII.\n"},{"id":"288796","messageId":"214976cd-3876-62ca-b931-338d7f0fd51c@grenoble-inp.org","threadId":"42426","inReplyTo":"vpqporqyko1.fsf@anie.imag.fr","subject":"Re: [PATCH v3 1/6] t9001: non order-sensitive file comparison","fromName":"Tom Russello","fromEmail":"tom.russello@grenoble-inp.org","sentAt":"2016-06-09T08:15:43Z","receivedAt":"2016-06-16T02:19:49Z","isPatch":true,"sender":{"key":"tom.russello@grenoble-inp.org","avatar":null},"body":"On 06/09/16 07:51, Matthieu Moy wrote:\n> Junio C Hamano writes:\n> \n>> Tom Russello writes:\n>>\n>>> +# Check if two files have the same content, non-order sensitive\n>>> +test_cmp_noorder () {\n>>> +\tsort $1 >$1;\n>>\n>> Here is what I think happens:\n>>\n>> 0) the shell parses this command line;\n>> 1) the shell notices that the output has to go to $1;\n>> 2) the shell does open(2) of $1,\n>> 3) the shell spawns \"sort\" with a single argument, with its\n>> output connected to the file descriptor obtained in 2).\n>>\n>> Because \"$1\" becomes an empty file at 2), \"sort\" reads nothing and\n>> writes nothing.\n> \n> Tom: in case you're not convinced, try this:\n> \n> $ (echo b; echo a) >f\n> $ sort f\n> a\n> b\n> $ sort f >f\n> $ cat f\n> $\n> \n> Also, useless ';' and missing double-quotes around \"$1\" to avoid bad\n> surprises ifever test_cmp_noorder is called on file names with special\n> characters.\n\nI was totally convinced by Junio's explanation, it is partially fixed in\nv4 and will be entirely fixed in v5.\n\nThanks.\n"},{"id":"288798","messageId":"vpqoa7asnkw.fsf@anie.imag.fr","threadId":"42426","inReplyTo":"20160608130736.32163-1-samuel.groot@grenoble-inp.org","subject":"Re: [PATCH v4 5/6] send-email: --in-reply-to= populate header fields","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-06-09T09:45:03Z","receivedAt":"2016-06-16T02:19:49Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Samuel GROOT writes:\n\n> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\n> index edbba3a..21776f0 100644\n> --- a/Documentation/git-send-email.txt\n> +++ b/Documentation/git-send-email.txt\n> @@ -84,13 +84,16 @@ See the CONFIGURATION section for 'sendemail.multiEdit'.\n> \tthe value of GIT_AUTHOR_IDENT, or GIT_COMMITTER_IDENT if that is not\n> \tset, as returned by \"git var -l\".\n> \n> ---in-reply-to=::\n> +--in-reply-to=::\n> \tMake the first mail (or all the mails with `--no-thread`) appear as a\n> -\treply to the given Message-Id, which avoids breaking threads to\n> -\tprovide a new patch series.\n> +\treply to the given Message-Id (given directly by argument or via the email\n> +\tfile), which avoids breaking threads to provide a new patch series.\n> \tThe second and subsequent emails will be sent as replies according to\n> \tthe `--[no]-chain-reply-to` setting.\n> +\n> +Furthermore, if the argument is an email file, parse it and populate header\n> +fields appropriately for the reply.\n\n\"populate header fields appropriately\" would seem obscure to someone not\nhaving followed this converation. At least s/fields/To: and Cc: fields/.\n\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -55,6 +55,7 @@ git send-email --dump-aliases\n> --[no-]bcc * Email Bcc:\n> --subject * Email \"Subject:\"\n> --in-reply-to * Email \"In-Reply-To:\"\n> + --in-reply-to * Populate header fields appropriately.\n\nLikewise. To avoid an overly long line, I'd write just \"Populate\nTo/Cc/In-reply-to\".\n\nProbably should be .\n\n> +if ($initial_reply_to && -f $initial_reply_to) {\n> +\tmy $error = validate_patch($initial_reply_to);\n> +\tdie \"fatal: $initial_reply_to: $error\\nwarning: no patches were sent\\n\"\n> +\t\tif $error;\n> +\n> +\topen my $fh, \"<\", $initial_reply_to or die \"can't open file $initial_reply_to\";\n> +\tmy $mail = Git::parse_email($fh);\n> +\tclose $fh;\n> +\n> +\tmy $initial_sender = $sender || $repoauthor || $repocommitter || '';\n\nThis is duplicated from the \"if ($compose) { ... my $tpl_sender = ...\" a\nbit later in the existing file. It would be better to get this \"my\n$initial_sender = ...\" out of your \"if\" and use $initial_sender directly\nlater IMHO.\n\nActually, $initial_sender does not seem to be a good variable name. It's\nnot really \"initial\", right?\n\n> +\tmy $prefix_re = \"\";\n> +\tmy $subject_re = $mail->{\"subject\"}[0];\n> +\tif ($subject_re =~ /^[^Re:]/) {\n> +\t\t$prefix_re = \"Re: \";\n> +\t}\n> +\t$initial_subject = $prefix_re . $subject_re;\n\nWhy introduce $prefix_re. You can just\n\n\tmy $subject = $mail->{\"subject\"}[0];\n\tif (...) {\n \t$subject = \"Re: \" . $subject;\n }\n\n(preferably using sensible as '...' as noted by Junio ;-) ).\n\nIn previous iterations of this series, you had issues with non-ascii\ncharacters in at least To: and Cc: fields (perhaps in the Subject field\ntoo?). Are they solved? I don't see any tests about it ...\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"288802","messageId":"vpqvb1ir38m.fsf@anie.imag.fr","threadId":"42426","inReplyTo":"20160608130833.32274-1-samuel.groot@grenoble-inp.org","subject":"Re: [PATCH v4 6/6] send-email: add option --cite to quote the message body","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-06-09T11:49:45Z","receivedAt":"2016-06-16T02:19:49Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Samuel GROOT writes:\n\n> If used with `in-reply-to=`, cite the message body of the given\n> email file. Otherwise, do nothing.\n\nIt should at least warn when --in-reply-to= is not given\n(either no --in-reply-to or --in-reply-to=). I don't see any\nuse-case where a user would want --cite on the command-line and not want\n--in-reply-to=. OTOH, it seems a plausible user-error, and\nthe user would appreciate a message saying what's going on.\n\n> @@ -56,6 +57,8 @@ git send-email --dump-aliases\n> --subject * Email \"Subject:\"\n> --in-reply-to * Email \"In-Reply-To:\"\n> --in-reply-to * Populate header fields appropriately.\n> + --cite * Quote the message body in the cover if\n> + --compose is set, else in the first patch.\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\nJust wondering: would it make sense to activate --cite by default when\n--in-reply-to=file is used, and to allow --no-cite to disable this?\n\nThis is something we can easily do now without breaking backward\ncompatibility (--in-reply-to=file doesn't exist yet), but would be more\npainful to do later.\n\n> @@ -640,6 +644,7 @@ if (@files) {\n> \tusage();\n> }\n> \n> +my $message_cited;\n\nNit: I read \"$message_cited\" as \"Boolean saying whether the message was\ncited\". $cited_message would be clearer to me (but this is to be taken\nwith a grain of salt as I'm not a native speaker), since the variable\nholds the content of the cited message.\n\n> +sub do_insert_cited_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_cited;\n\nI would add a newline here to get a blank line between the message cited\nand the diffstat.\n\nI think non-ascii characters would deserve particular attention here\ntoo. For example, if the patch contain only ascii and the cited part\ncontains UTF-8, does the generated patch have a proper Content-type:\nheader?\n\nI can imagine worse, like a patch containing latin1 character and a\ncited message with another 8-bit encoding.\n\n> +test_expect_success $PREREQ 'correct cited message with --in-reply-to and --compose' '\n> +\tgrep \"> On Sat, 12 Jun 2010 15:53:58 +0200, author@example.com wrote:\" msgtxt3 &&\n\nI would prefer to have the full address including the real name here (A\n) in this example. Actually, after a quick look at\nthe code, I don't understand where the name has gone (what's shown here\nis extracted from the From: header).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"289171","messageId":"39dae1da-e300-3ebe-5f04-393b2f71017b@grenoble-inp.org","threadId":"42426","inReplyTo":"CAPig+cSn5LQ0OxDbg5y=dv16VC-jCaehnss_14mc34o95QbEpw@mail.gmail.com","subject":"Re: [PATCH v4 4/6] send-email: create email parser subroutine","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-13T22:15:28Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"\n\nOn 06/09/2016 08:51 AM, Eric Sunshine wrote:\n> On Wed, Jun 8, 2016 at 7:54 PM, Samuel GROOT\n> wrote:\n>> On 06/08/2016 10:17 PM, Junio C Hamano wrote:\n>>> Eric Sunshine writes:\n>>>> An embedded CR probably shouldn't happen, but I'm not convinced that\n>>>> folding it out is a good idea. I would think that you'd want to\n>>>> preserve the header's value verbatim. If anything, I'd expect to see\n>>>> the regex tightened to:\n>>>>\n>>>> s/\\r?\\n$//;\n>>>\n>>> Yes, that would be more sensible than silently removing \\r in the\n>>> middle which _is_ a sign of something funny going on.\n>>\n>> s/\\r?\\n$// looks fine.\n>>\n>> Email::Simple library uses qr/\\x0a\\x0d|\\x0d\\x0a|\\x0a|\\x0d/ [1].\n>> [1] * http://cpansearch.perl.org/src/RJBS/Email-Simple-2.210/lib/Email/Simple.pm\n>\n> You could certainly use s/\\x0d?\\x0a$// rather than s/\\r?\\n$// to be\n> really robust, but I doubt it matters much today. The reason for using\n> hex codes is that \\r and \\n will resolve to CR and LF in the local\n> character encoding, and those may not be 0x0d and 0x0a, respectively.\n> I believe the canonical example given in the Camel book was EBCIDIC in\n> which \\r is 0x0d, but \\n is 0x25, not 0x0a as it is in ASCII.\n\nMy question was more about the `\\n\\r` case handled by Email::Simple, \ndoes it make sense to handle it?\n"},{"id":"289172","messageId":"070d6313-8328-3086-b04f-0bf70e236264@grenoble-inp.org","threadId":"42426","inReplyTo":"20160609002150.GA28547@dcvr.yhbt.net","subject":"Re: [PATCH v4 4/6] send-email: create email parser subroutine","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-13T22:18:08Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/09/2016 02:21 AM, Eric Wong wrote:\n> Samuel GROOT wrote:\n>> Email::Simple library uses qr/\\x0a\\x0d|\\x0d\\x0a|\\x0a|\\x0d/ [1].\n>> Should we handle \\n\\r at end of line as well?\n>\n> \"\\n\\r\" can never happen with local $/ = \"\\n\"\n\nIf the email file contains \"\\n\\r\", setting $/ = \"\\n\" will leave \"\\r\" at\nthe beginning of each line.\n\nWe could trim them with:\n\n s/^\\r//;\n s/\\r?\\n$//;\n\nBut is it worth adding `s/^\\r//;` to handle that extremely rare case?\n"},{"id":"289173","messageId":"9f16b48f-3f74-d408-8309-3072904f43b7@grenoble-inp.org","threadId":"42426","inReplyTo":"vpqr3c6x4vz.fsf@anie.imag.fr","subject":"Re: [PATCH v4 3/6] send-email: shorten send-email's output","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-13T22:19:16Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/09/2016 08:17 AM, Matthieu Moy wrote:\n> Samuel GROOT writes:\n>\n>> @@ -647,10 +647,10 @@ test_expect_success $PREREQ '--suppress-cc=all' '\n>> test_expect_success $PREREQ 'setup expect' \"\n>> cat >expected-suppress-body <<\\EOF\n>> 0001-Second.patch\n>> -(mbox) Adding cc: A from line 'From: A '\n>> -(mbox) Adding cc: One from line 'Cc: One , two@example.com'\n>> -(mbox) Adding cc: two@example.com from line 'Cc: One , two@example.com'\n>> -(cc-cmd) Adding cc: cc-cmd@example.com from: './cccmd'\n>> +Adding cc: A from From: header\n>> +Adding cc: One from Cc: header\n>> +Adding cc: two@example.com from Cc: header\n>> +Adding cc: cc-cmd@example.com from: './cccmd'\n>\n> This hunk differs from the others a bit. I totally agree that removing\n> the (mbox) prefix makes sense, but you're removing (cc-cmd) here, which\n> did carry some information.\n>\n> I'd write it as\n>\n> Adding cc: cc-cmd@example.com from --cc-cmd: ./cccmd\n\nIndeed it's clearer, I will change that.\n\n> It might make sense to split this into two patches: one for (mbox) +\n> headers and one for (cc-cmd) and (to-cmd). Spotting special-cases like\n> the above inside a long patch is hard for reviewers.\n\nI will split in the future patch series dedicated to clean send-email.\n"},{"id":"289174","messageId":"e48890a0-06cb-4ea7-d168-d1df1f4c2e37@grenoble-inp.org","threadId":"42426","inReplyTo":"vpqbn3ayk79.fsf@anie.imag.fr","subject":"Re: [PATCH v3 1/6] t9001: non order-sensitive file comparison","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-13T22:21:56Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/09/2016 08:01 AM, Matthieu Moy wrote:\n> Samuel GROOT writes:\n>\n>> On 06/08/2016 06:09 PM, Junio C Hamano wrote:\n>>> Samuel GROOT writes:\n>>>\n>>>> Actually we had issues when trying to refactor send-email's email\n>>>> parsing loop [1]. Email addresses in output file `commandeline1` in\n>>>> tests weren't sorted the same way as the reference file it was\n>>>> compared to. E.g.:\n>>>>\n>>>> !nobody@example.com!\n>>>> !author@example.com!\n>>>> !one@example.com!\n>>>> !two@example.com!\n>>>\n>>> And the reason why these addresses that are collected from the same\n>>> input (i.e. command line, existing e-mail fields, footers, etc.) are\n>>> shown in different order in your implementation is...?\n>>\n>> It's not shown in different order in our implementation, it's just a\n>> leftover of my refactor attempt [1].\n>\n> I think the refactoring makes sense, but having this patch as PATCH 1/6\n> in a series about --in-reply-to confuses reviewers: they would expect\n> this patch to be useful to the others in the series.\n>\n> If you have \"reply to a message in a file\" ready without the\n> refactoring, and a mostly ready refactoring, then I think it makes sense\n> to have two patch series, the first being only \"reply to a message in a\n> file\". If the refactoring itself is not ready, you may send a separate\n> series \"tests clean up\" and explain on the cover-letter that it's, well,\n> only a test clean up.\n\nI think I will split the patch series into 3 smaller series:\n- \"quote-email\" feature\n- tests clean up\n- send-email code cleanup (including send-email's output)\n"},{"id":"289175","messageId":"274b83e5-3ac7-e1e5-4904-649c308cfaf4@grenoble-inp.org","threadId":"42426","inReplyTo":"vpqinxiykgk.fsf@anie.imag.fr","subject":"Re: [PATCH v3 2/6] t9001: check email address is in Cc: field","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-13T22:23:44Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/09/2016 07:55 AM, Matthieu Moy wrote:\n> Tom Russello writes:\n>\n>> Check if the given utf-8 email address is in the Cc: field.\n>\n> I wouldn't harm to explain what was the problem with existing code here.\n> If I understand correctly, that would be:\n>\n> Existing code just checked that the address appeared in a line starting\n> with a space, but not to which field the address belonged.\n>\n> But probably the real motivation for this was that you want to introduce\n> code that changes the layout and breaks this \"address appears on a line\n> starting with space\"?\n\nActually it was both, we wanted to make the tests less dependent to how \ndata was displayed.\n"},{"id":"289177","messageId":"20160613224753.GA4761@dcvr.yhbt.net","threadId":"42426","inReplyTo":"070d6313-8328-3086-b04f-0bf70e236264@grenoble-inp.org","subject":"Re: [PATCH v4 4/6] send-email: create email parser subroutine","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-06-13T22:47:53Z","receivedAt":"2016-06-16T06:55:52Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Samuel GROOT wrote:\n> On 06/09/2016 02:21 AM, Eric Wong wrote:\n> >Samuel GROOT wrote:\n> >>Email::Simple library uses qr/\\x0a\\x0d|\\x0d\\x0a|\\x0a|\\x0d/ [1].\n> >>Should we handle \\n\\r at end of line as well?\n> >\n> >\"\\n\\r\" can never happen with local $/ = \"\\n\"\n> \n> If the email file contains \"\\n\\r\", setting $/ = \"\\n\" will leave \"\\r\" at\n> the beginning of each line.\n> \n> We could trim them with:\n> \n> s/^\\r//;\n> s/\\r?\\n$//;\n> \n> But is it worth adding `s/^\\r//;` to handle that extremely rare case?\n\nI doubt it. Having a \"\\r\" in the wrong place is likely a bug in\nwhatever program that generated the email. It should be exposed\nso whoever generated that email has a chance to fix it on their\nend rather than being quietly hidden.\n"},{"id":"289263","messageId":"a6285923-9ef9-e0b4-2335-ff41990c8fd5@grenoble-inp.org","threadId":"42426","inReplyTo":"20160613224753.GA4761@dcvr.yhbt.net","subject":"Re: [PATCH v4 4/6] send-email: create email parser subroutine","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-14T22:18:04Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/14/2016 12:47 AM, Eric Wong wrote:\n> Samuel GROOT wrote:\n>> On 06/09/2016 02:21 AM, Eric Wong wrote:\n>>> Samuel GROOT wrote:\n>>>> Email::Simple library uses qr/\\x0a\\x0d|\\x0d\\x0a|\\x0a|\\x0d/ [1].\n>>>> Should we handle \\n\\r at end of line as well?\n>>>\n>>> \"\\n\\r\" can never happen with local $/ = \"\\n\"\n>>\n>> If the email file contains \"\\n\\r\", setting $/ = \"\\n\" will leave \"\\r\" at\n>> the beginning of each line.\n>>\n>> We could trim them with:\n>>\n>> s/^\\r//;\n>> s/\\r?\\n$//;\n>>\n>> But is it worth adding `s/^\\r//;` to handle that extremely rare case?\n>\n> I doubt it. Having a \"\\r\" in the wrong place is likely a bug in\n> whatever program that generated the email. It should be exposed\n> so whoever generated that email has a chance to fix it on their\n> end rather than being quietly hidden.\n\ns/\\r?\\n$// is fine then.\n\nThanks.\n"},{"id":"289264","messageId":"d576c59b-0196-6d7d-c357-3405fadbe303@grenoble-inp.org","threadId":"42426","inReplyTo":"xmqqvb1jletl.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 5/6] send-email: --in-reply-to= populate header fields","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-14T22:26:27Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/08/2016 08:23 PM, Junio C Hamano wrote:\n> Samuel GROOT writes:\n>\n>> +if ($initial_reply_to && -f $initial_reply_to) {\n>> +\tmy $error = validate_patch($initial_reply_to);\n>\n> This call is wrong, isn't it?\n>\n> You are not going to send out the message you are responding to (the\n> message may not even be a patch), and you do not want to die with an\n> error message that says \"patch contains an overlong line\".\n\nWe used to handle email files like patch files (with Cc:, To:, From:, \n... fields, a patch is almost an email, with some trailers).\n\nBut that `validate_patch` subroutine call is indeed useless here, I will \nremove it.\n\n>> +\tdie \"fatal: $initial_reply_to: $error\\nwarning: no patches were sent\\n\"\n>> +\t\tif $error;\n>> +\n>> +\topen my $fh, \"<\", $initial_reply_to or die \"can't open file $initial_reply_to\";\n>> +\tmy $mail = Git::parse_email($fh);\n>> +\tclose $fh;\n>\n> \tmy $header = Git::parse_email_header($fh);\n>\n> perhaps?\n\nGit::parse_email will be renamed into Git::parse_email_header in v5.\n\n>> +\tmy $initial_sender = $sender || $repoauthor || $repocommitter || '';\n>> +\n>> +\tmy $prefix_re = \"\";\n>> +\tmy $subject_re = $mail->{\"subject\"}[0];\n>> +\tif ($subject_re =~ /^[^Re:]/) {\n>> +\t\t$prefix_re = \"Re: \";\n>> +\t}\n>> +\t$initial_subject = $prefix_re . $subject_re;\n>\n> I am not sure what the significance of the fact that the subject\n> happens to begin with a letter other than 'R', 'e', or ':'.\n>\n> Did you mean to do something like this instead?\n>\n> \tmy $subject = $mail->{\"subject\"}[0];\n> \t$subject =~ s/^(re:\\s*)+//i; # strip \"Re: Re: ...\"\n> $initial_subject = \"Re: $subject\";\n>\n> instead?\n\nThat's way cleaner, thanks!\n\n> By the way, this is a good example why your \"unfold\" implementation\n> in 4/6 is unwieldy for the caller. Imagine a rather long subject\n> that is folded, i.e.\n>\n> \tTo: Samuel\n> Subject: Help! I am having a trouble running git-send-email\n> correctly.\n> \tMessage-id: <...>\n\nIt's a good point. What you suggested in 4/6 reply will be used in v5.\n"},{"id":"289266","messageId":"5221a7c5-3f4b-4061-6fc2-5019d784e03f@grenoble-inp.org","threadId":"42426","inReplyTo":"vpqoa7asnkw.fsf@anie.imag.fr","subject":"Re: [PATCH v4 5/6] send-email: --in-reply-to= populate header fields","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-14T22:35:15Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"\n\nOn 06/09/2016 11:45 AM, Matthieu Moy wrote:\n> Samuel GROOT writes:\n>\n>> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\n>> index edbba3a..21776f0 100644\n>> --- a/Documentation/git-send-email.txt\n>> +++ b/Documentation/git-send-email.txt\n>> @@ -84,13 +84,16 @@ See the CONFIGURATION section for 'sendemail.multiEdit'.\n>> \tthe value of GIT_AUTHOR_IDENT, or GIT_COMMITTER_IDENT if that is not\n>> \tset, as returned by \"git var -l\".\n>>\n>> ---in-reply-to=::\n>> +--in-reply-to=::\n>> \tMake the first mail (or all the mails with `--no-thread`) appear as a\n>> -\treply to the given Message-Id, which avoids breaking threads to\n>> -\tprovide a new patch series.\n>> +\treply to the given Message-Id (given directly by argument or via the email\n>> +\tfile), which avoids breaking threads to provide a new patch series.\n>> \tThe second and subsequent emails will be sent as replies according to\n>> \tthe `--[no]-chain-reply-to` setting.\n>> +\n>> +Furthermore, if the argument is an email file, parse it and populate header\n>> +fields appropriately for the reply.\n>\n> \"populate header fields appropriately\" would seem obscure to someone not\n> having followed this converation. At least s/fields/To: and Cc: fields/.\n\nWe weren't sure how precise the documentation had to be, and tried to \nkeep it concise.\n\n>> --- a/git-send-email.perl\n>> +++ b/git-send-email.perl\n>> @@ -55,6 +55,7 @@ git send-email --dump-aliases\n>> --[no-]bcc * Email Bcc:\n>> --subject * Email \"Subject:\"\n>> --in-reply-to * Email \"In-Reply-To:\"\n>> + --in-reply-to * Populate header fields appropriately.\n>\n> Likewise. To avoid an overly long line, I'd write just \"Populate\n> To/Cc/In-reply-to\".\n>\n> Probably should be .\n\nThanks, will do in v5.\n\n>> +if ($initial_reply_to && -f $initial_reply_to) {\n>> +\tmy $error = validate_patch($initial_reply_to);\n>> +\tdie \"fatal: $initial_reply_to: $error\\nwarning: no patches were sent\\n\"\n>> +\t\tif $error;\n>> +\n>> +\topen my $fh, \"<\", $initial_reply_to or die \"can't open file $initial_reply_to\";\n>> +\tmy $mail = Git::parse_email($fh);\n>> +\tclose $fh;\n>> +\n>> +\tmy $initial_sender = $sender || $repoauthor || $repocommitter || '';\n>\n> This is duplicated from the \"if ($compose) { ... my $tpl_sender = ...\" a\n> bit later in the existing file. It would be better to get this \"my\n> $initial_sender = ...\" out of your \"if\" and use $initial_sender directly\n> later IMHO.\n>\n> Actually, $initial_sender does not seem to be a good variable name. It's\n> not really \"initial\", right?\n\n$sender looks like a better name, I will work on that, thanks!\n\n>> +\tmy $prefix_re = \"\";\n>> +\tmy $subject_re = $mail->{\"subject\"}[0];\n>> +\tif ($subject_re =~ /^[^Re:]/) {\n>> +\t\t$prefix_re = \"Re: \";\n>> +\t}\n>> +\t$initial_subject = $prefix_re . $subject_re;\n>\n> Why introduce $prefix_re. You can just\n>\n> \tmy $subject = $mail->{\"subject\"}[0];\n> \tif (...) {\n> \t$subject = \"Re: \" . $subject;\n> }\n>\n> (preferably using sensible as '...' as noted by Junio ;-) ).\n\nI will keep Junio's suggestion :-)\n\n> In previous iterations of this series, you had issues with non-ascii\n> characters in at least To: and Cc: fields (perhaps in the Subject field\n> too?). Are they solved? I don't see any tests about it ...\n\nNon-ascii characters are still an issue, I will work on that next week.\n"},{"id":"289267","messageId":"e339c77f-ecb8-b7aa-1c8f-38a1f9eca93e@grenoble-inp.org","threadId":"42426","inReplyTo":"vpqvb1ir38m.fsf@anie.imag.fr","subject":"Re: [PATCH v4 6/6] send-email: add option --cite to quote the message body","fromName":"Samuel GROOT","fromEmail":"samuel.groot@grenoble-inp.org","sentAt":"2016-06-14T22:53:36Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":true,"sender":{"key":"samuel.groot@grenoble-inp.org","avatar":null},"body":"On 06/09/2016 01:49 PM, Matthieu Moy wrote:\n> Samuel GROOT writes:\n>\n>> If used with `in-reply-to=`, cite the message body of the given\n>> email file. Otherwise, do nothing.\n>\n> It should at least warn when --in-reply-to= is not given\n> (either no --in-reply-to or --in-reply-to=). I don't see any\n> use-case where a user would want --cite on the command-line and not want\n> --in-reply-to=. OTOH, it seems a plausible user-error, and\n> the user would appreciate a message saying what's going on.\n\nWe weren't sure how to warn the user.\n\nIf --in-reply-to is not an mail file, should we check it with a regex to \nmake sure it's a message-id?\n\n>> @@ -56,6 +57,8 @@ git send-email --dump-aliases\n>> --subject * Email \"Subject:\"\n>> --in-reply-to * Email \"In-Reply-To:\"\n>> --in-reply-to * Populate header fields appropriately.\n>> + --cite * Quote the message body in the cover if\n>> + --compose is set, else in the first patch.\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>\n> Just wondering: would it make sense to activate --cite by default when\n> --in-reply-to=file is used, and to allow --no-cite to disable this?\n>\n> This is something we can easily do now without breaking backward\n> compatibility (--in-reply-to=file doesn't exist yet), but would be more\n> painful to do later.\n\nIt's an interesting question.\n\nIMHO we should have `--cite` by default and allow `--no-cite` to disable \nquoting the message body, because it's easier to remove extra unwanted \nlines than copying lines from another file and adding \"> \" before each line.\n\n>> @@ -640,6 +644,7 @@ if (@files) {\n>> \tusage();\n>> }\n>>\n>> +my $message_cited;\n>\n> Nit: I read \"$message_cited\" as \"Boolean saying whether the message was\n> cited\". $cited_message would be clearer to me (but this is to be taken\n> with a grain of salt as I'm not a native speaker), since the variable\n> holds the content of the cited message.\n>\n>> +sub do_insert_cited_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_cited;\n>\n> I would add a newline here to get a blank line between the message cited\n> and the diffstat.\n\nA newline here makes it easier to distinguish the different sections in \nthe email file indeed.\n\n> I think non-ascii characters would deserve particular attention here\n> too. For example, if the patch contain only ascii and the cited part\n> contains UTF-8, does the generated patch have a proper Content-type:\n> header?\n\nNon-ascii characters are still an issue, I'll work on that next week.\n\n> I can imagine worse, like a patch containing latin1 character and a\n> cited message with another 8-bit encoding.\n>\n>> +test_expect_success $PREREQ 'correct cited message with --in-reply-to and --compose' '\n>> +\tgrep \"> On Sat, 12 Jun 2010 15:53:58 +0200, author@example.com wrote:\" msgtxt3 &&\n>\n> I would prefer to have the full address including the real name here (A\n> ) in this example. Actually, after a quick look at\n> the code, I don't understand where the name has gone (what's shown here\n> is extracted from the From: header).\n\nYep, after a quick look at the code involved, I don't understand either, \nI will investigate this week.\n"},{"id":"289326","messageId":"f049d7cd-d178-8a8e-3672-98e5113afd3a@grenoble-inp.org","threadId":"42426","inReplyTo":"vpqvb1ir38m.fsf@anie.imag.fr","subject":"Re: [PATCH v4 6/6] send-email: add option --cite to quote the message body","fromName":"Tom Russello","fromEmail":"tom.russello@grenoble-inp.org","sentAt":"2016-06-15T22:21:47Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":true,"sender":{"key":"tom.russello@grenoble-inp.org","avatar":null},"body":"Le 09/06/2016 à 13:49, Matthieu Moy a écrit :\n> Samuel GROOT writes:\n> \n>> If used with `in-reply-to=`, cite the message body of the given\n>> email file. Otherwise, do nothing.\n> \n> It should at least warn when --in-reply-to= is not given\n> (either no --in-reply-to or --in-reply-to=). I don't see any\n> use-case where a user would want --cite on the command-line and not want\n> --in-reply-to=. OTOH, it seems a plausible user-error, and\n> the user would appreciate a message saying what's going on.\n\nYou're right. We'll warn the user with the next version.\n\n>> @@ -56,6 +57,8 @@ git send-email --dump-aliases\n>> --subject * Email \"Subject:\"\n>> --in-reply-to * Email \"In-Reply-To:\"\n>> --in-reply-to * Populate header fields appropriately.\n>> + --cite * Quote the message body in the cover if\n>> + --compose is set, else in the first patch.\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> \n> Just wondering: would it make sense to activate --cite by default when\n> --in-reply-to=file is used, and to allow --no-cite to disable this?\n> This is something we can easily do now without breaking backward\n> compatibility (--in-reply-to=file doesn't exist yet), but would be more\n> painful to do later.\n\nIndeed, it can be more intuitive especially for a user who is used to\ncite messages.\n\n>> @@ -640,6 +644,7 @@ if (@files) {\n>> \tusage();\n>> }\n>> \n>> +my $message_cited;\n> \n> Nit: I read \"$message_cited\" as \"Boolean saying whether the message was\n> cited\". $cited_message would be clearer to me (but this is to be taken\n> with a grain of salt as I'm not a native speaker), since the variable\n> holds the content of the cited message.\n\nSorry, French habits.. :-)\n\n>> +sub do_insert_cited_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_cited;\n> \n> I would add a newline here to get a blank line between the message cited\n> and the diffstat.\n\nAgreed.\n\n> I think non-ascii characters would deserve particular attention here\n> too. For example, if the patch contain only ascii and the cited part\n> contains UTF-8, does the generated patch have a proper Content-type:\n> header?\n> \n> I can imagine worse, like a patch containing latin1 character and a\n> cited message with another 8-bit encoding.\n\nI tried to manage them with the built-in Base64 module but there is\nstill work in progress.\n\n>> +test_expect_success $PREREQ 'correct cited message with --in-reply-to and --compose' '\n>> +\tgrep \"> On Sat, 12 Jun 2010 15:53:58 +0200, author@example.com wrote:\" msgtxt3 &&\n> \n> I would prefer to have the full address including the real name here (A\n> ) in this example. Actually, after a quick look at\n> the code, I don't understand where the name has gone (what's shown here\n> is extracted from the From: header).\n\nAgreed, I'll figure out where the problem is.\n"}]}