On 06/09/2016 11:45 AM, Matthieu Moy wrote:
Samuel GROOT <[email protected]> writes:diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index edbba3a..21776f0 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -84,13 +84,16 @@ See the CONFIGURATION section for 'sendemail.multiEdit'. the value of GIT_AUTHOR_IDENT, or GIT_COMMITTER_IDENT if that is not set, as returned by "git var -l". ---in-reply-to=<identifier>:: +--in-reply-to=<Message-Id|email_file>:: Make the first mail (or all the mails with `--no-thread`) appear as a - reply to the given Message-Id, which avoids breaking threads to - provide a new patch series. + reply to the given Message-Id (given directly by argument or via the email + file), which avoids breaking threads to provide a new patch series. The second and subsequent emails will be sent as replies according to the `--[no]-chain-reply-to` setting. + +Furthermore, if the argument is an email file, parse it and populate header +fields appropriately for the reply."populate header fields appropriately" would seem obscure to someone not having followed this converation. At least s/fields/To: and Cc: fields/.
We weren't sure how precise the documentation had to be, and tried to keep it concise.
--- a/git-send-email.perl +++ b/git-send-email.perl @@ -55,6 +55,7 @@ git send-email --dump-aliases --[no-]bcc <str> * Email Bcc: --subject <str> * Email "Subject:" --in-reply-to <str> * Email "In-Reply-To:" + --in-reply-to <file> * Populate header fields appropriately.Likewise. To avoid an overly long line, I'd write just "Populate To/Cc/In-reply-to". Probably <file> should be <email_file>.
Thanks, will do in v5.
+if ($initial_reply_to && -f $initial_reply_to) { + my $error = validate_patch($initial_reply_to); + die "fatal: $initial_reply_to: $error\nwarning: no patches were sent\n" + if $error; + + open my $fh, "<", $initial_reply_to or die "can't open file $initial_reply_to"; + my $mail = Git::parse_email($fh); + close $fh; + + my $initial_sender = $sender || $repoauthor || $repocommitter || '';This is duplicated from the "if ($compose) { ... my $tpl_sender = ..." a bit later in the existing file. It would be better to get this "my $initial_sender = ..." out of your "if" and use $initial_sender directly later IMHO. Actually, $initial_sender does not seem to be a good variable name. It's not really "initial", right?
$sender looks like a better name, I will work on that, thanks!
+ my $prefix_re = ""; + my $subject_re = $mail->{"subject"}[0]; + if ($subject_re =~ /^[^Re:]/) { + $prefix_re = "Re: "; + } + $initial_subject = $prefix_re . $subject_re;Why introduce $prefix_re. You can just my $subject = $mail->{"subject"}[0]; if (...) { $subject = "Re: " . $subject; } (preferably using sensible as '...' as noted by Junio ;-) ).
I will keep Junio's suggestion :-)
In previous iterations of this series, you had issues with non-ascii characters in at least To: and Cc: fields (perhaps in the Subject field too?). Are they solved? I don't see any tests about it ...
Non-ascii characters are still an issue, I will work on that next week. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
