Samuel GROOT <samuel.groot@xxxxxxxxxxxxxxxx> 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/.
> --- 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>.
> +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?
> + 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 ;-) ).
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 ...
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html