git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v4] send-email: extract email-parsing code into a subroutine

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Dec 7, 2017, 14:14 UTC
Message-ID
<87po7qzktt.fsf@evledraar.gmail.com>
In-Reply-To
<q7h94lp2oepu.fsf@orange.lip.ens-lyon.fr>
On Thu, Dec 07 2017, Matthieu Moy jotted:
Show 20 quoted lines
> Not terribly important, but your patch has trailing newlines. "git diff
> --staged --check" to see them. More below.
>
> PAYRE NATHAN p1508475 <nathan.payre@etu.univ-lyon1.fr> 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.

Good catch, actually this can just be: /^$/. The $ syntax already matches the ending newline, no need for /^\n$/.

Show 17 quoted lines
>> +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.

Although you'll end up with a lot of stuff in the $parsed_line hash you don't need, which makes dumping it for debugging verbose.

I also wonder about multi-line headers, but then again that probably breaks already on e.g. Message-ID and Refererences, but that's an existing bug unrelated to this patch...

>> +			$body = $body . $body_line;
>
> Or just: $body .= $body_line;
Previous: Matthieu MoyNext: Matthieu Moy
Message 19 of 25 in “send-email: extract email-parsing code into a subroutine”
  1. send-email: extract email-parsing code into a subroutinePayre Nathan, Dec 2, 2017
  2. Nathan PAYREDec 2, 2017
  3. Ævar Arnfjörð BjarmasonDec 3, 2017
  4. Nathan PAYREDec 3, 2017
  5. Junio C HamanoDec 4, 2017
  6. send-email: extract email-parsing code into a subroutineNathan Payre, Dec 6, 2017
  7. Junio C HamanoDec 6, 2017
  8. Nathan PAYREDec 6, 2017
  9. send-email: extract email-parsing code into a subroutineNathan Payre, Dec 6, 2017
  10. Ævar Arnfjörð BjarmasonDec 6, 2017
  11. send-email: extract email-parsing code into a subroutineNathan Payre, Dec 7, 2017
  12. Junio C HamanoDec 6, 2017
  13. Eric SunshineDec 7, 2017
  14. send-email: extract email-parsing code into a subroutineNathan Payre, Dec 9, 2017
  15. send-email: extract email-parsing code into a subroutineNathan Payre, Dec 6, 2017
  16. Matthieu MoyDec 3, 2017
  17. Matthieu MoyDec 7, 2017
  18. Matthieu MoyDec 7, 2017
  19. Ævar Arnfjörð BjarmasonDec 7, 2017
  20. Matthieu MoyDec 11, 2017
  21. Nathan PAYREDec 14, 2017
  22. send-email: extract email-parsing code into a subroutineNathan Payre, Dec 14, 2017
  23. Matthieu MoyDec 14, 2017
  24. send-email: extract email-parsing code into a subroutineNathan Payre, Dec 15, 2017
  25. Matthieu MoyDec 15, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.