"PAYRE NATHAN p1508475" <nathan.payre@xxxxxxxxxxxxxxxxx> wrote:
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -685,7 +685,7 @@ Lines beginning in "GIT:" will be removed.
> Consider including an overall diffstat or table of contents
> for the patch you are writing.
>
> -Clear the body content if you don't wish to send a summary.
> +Clear the body content if you dont wish to send a summary.
This is not part of your patch. Use "git add -p" to specify
exactly which hunks should go into the patch and don't let this
kind of change end up in the version you send.
> + my %parsed_email;
> + $parsed_email{'body'} = '';
> + while (my $line = <$c>) {
> + next if $line =~ m/^GIT:/;
> + parse_header_line($line, \%parsed_email);
> + if ($line =~ /^$/) {
> + $parsed_email{'body'} = filter_body($c);
> }
> - print $c2 $_;
I didn't notice this at first, but you're modifying the behavior here:
the old code used to print to $c2 anything that didn't match any of
the if/else if branches.
To keep this behavior, you need to keep all these extra headers in
$parsed_email (you do, in this version) and print them after taking
care of all the known headers (AFAICT, you don't).
> }
> - close $c;
> - close $c2;
You'll still need $c2, but you don't need $c anymore, so I'd keep the
"close $c" here. OTOH, $c2 is not needed before this point (actually a
bit later), so it would make sense to move the "open" down a little.
This would materialize the "read input, then write output" scheme (as
opposed to "write output while reading input" in the previous code).
It's not a new issue in your patch, but giving variables meaningful
names (i.e. not $c and $c2) would help, too.
> + if ($parsed_email{'mime-version'}) {
> + 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 ($parsed_email{'content-type'}) {
> + print $c2 "MIME-Version: 1.0\n",
> + "Content-Type: $parsed_email{'content-type'};\n",
> + "Content-Transfer-Encoding: 8bit\n";
This "if ($parsed_email{'content-type'})" does not correspond to
anything in the old code, and ...
> + } elsif (file_has_nonascii($compose_filename)) {
> + my $content_type = ($parsed_email{'content-type'} or
> + "text/plain; charset=$compose_encoding");
Here, your're dealing explicitly with $parsed_email{'content-type'} !=
false (you're in the 'else' branch where it can only be false).
I think you just meant to drop the "if
($parsed_email{'content-type'})" part, and plug the "elseif" directly
after the "if ($parsed_email{'mime-version'})". That's what I
suggested in my earlier email.
> + my $content_type =3D ($parsed_email{'content-type'} or
> + "text/plain; charset=3D$compose_encoding");
> + print $c2 "MIME-Version: 1.0\n",
> + "Content-Type: $content_type\n",
> + "Content-Transfer-Encoding: 8bit\n";
> + }
This part is indented with spaces, please use tabs.
--
Matthieu Moy
https://matthieu-moy.fr/
