Not terribly important, but your patch has trailing newlines. "git diff
--staged --check" to see them. More below.
PAYRE NATHAN p1508475 <nathan.payre@xxxxxxxxxxxxxxxxx> writes:
> the part of code which parses the header a last time to prepare the
> email and send it.
The important point is not that it's the last time the code parses
headers, so I'd drop the "a last time".
> + my %parsed_email;
> + $parsed_email{'body'} = '';
> + while (my $line = <$c>) {
> + next if $line =~ m/^GIT:/;
> + parse_header_line($line, \%parsed_email);
> + if ($line =~ /^\n$/i) {
You don't need the /i (case-Insensitive) here, there are no letters to
match.
> + if ($parsed_email{'mime-version'}) {
> + $need_8bit_cte = 0;
This $need_8bit_cte is a leftover of the old code, which processed the
headers in the order it found them in the message and had to remember
the content of MIME-Version while parsing Content-Type.
I believe you can apply this on top of your patch:
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -709,7 +709,6 @@ EOT3
open $c, "<", $compose_filename
or die sprintf(__("Failed to open %s: %s"), $compose_filename, $!);
- my $need_8bit_cte = file_has_nonascii($compose_filename);
my $in_body = 0;
my $summary_empty = 1;
if (!defined $compose_encoding) {
@@ -740,12 +739,10 @@ EOT3
"\n";
}
if ($parsed_email{'mime-version'}) {
- $need_8bit_cte = 0;
print $c2 "MIME-Version: $parsed_email{'mime-version'}\n",
"Content-Type: $parsed_email{'content-type'};\n",
"Content-Transfer-Encoding: $parsed_email{'content-transfer-encoding'}\n";
- }
- if ($need_8bit_cte) {
+ } else if (file_has_nonascii($compose_filename)) {
if ($parsed_email{'content-type'}) {
print $c2 "MIME-Version: 1.0\n",
"Content-Type: $parsed_email{'content-type'};",
It reads much better: "If the original message already had a
MIME-Version header, then use that, else see if the file has non-ascii
characters and if so, use MIME-Version: 1.0".
Actually, you can even simplify further by factoring the if/else below:
> + if ($parsed_email{'content-type'}) {
> + print $c2 "MIME-Version: 1.0\n",
> + "Content-Type: $parsed_email{'content-type'};",
(Suspicious ";", and suspicious absence of "\n" here, I don't think it's
intentional and I'm fixing it below, but correct me if I'm wrong)
> + "Content-Transfer-Encoding: 8bit\n";
> + } else {
(Broken indentation, this is not aligned with the "if" above)
> print $c2 "MIME-Version: 1.0\n",
> "Content-Type: text/plain; ",
> - "charset=$compose_encoding\n",
> + "charset=$compose_encoding\n",
> "Content-Transfer-Encoding: 8bit\n";
> }
This could become stg like (untested):
} else if (file_has_nonascii($compose_filename)) {
my $content_type = ($parsed_email{'content-type'} or
"text/plain; charset=$compose_encoding");
print $c2 "MIME-Version: 1.0\n",
"Content-Type: $content_type\n",
"Content-Transfer-Encoding: 8bit\n";
}
> + open $c2, "<", $compose_filename . ".final"
> + or die sprintf(__("Failed to open %s.final: %s"), $compose_filename, $!);
> + close $c2;
What is this? Cut-and-paste mistake?
> +sub parse_header_line {
> + my $lines = shift;
> + my $parsed_line = shift;
> + my $pattern1 = join "|", qw(To Cc Bcc);
> + my $pattern2 = join "|",
> + qw(From Subject Date In-Reply-To Message-ID MIME-Version
> + Content-Type Content-Transfer-Encoding References);
> +
> + foreach (split(/\n/, $lines)) {
> + if (/^($pattern1):\s*(.+)$/i) {
> + $parsed_line->{lc $1} = [ parse_address_line($2) ];
> + } elsif (/^($pattern2):\s*(.+)\s*$/i) {
> + $parsed_line->{lc $1} = $2;
> + }
I don't think you need to list the possibilities in the "else" branch.
Just matching /^([^:]*):\s*(.+)\s*$/i should do the trick.
> + $body = $body . $body_line;
Or just: $body .= $body_line;
--
Matthieu Moy
https://matthieu-moy.fr/
