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

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

From
Junio C Hamano <gitster@pobox.com>
Date
Dec 6, 2017, 21:39 UTC
Message-ID
<xmqqvahjfsdx.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20171206153821.24435-1-nathan.payre@etu.univ-lyon1.fr>
Nathan Payre <nathan.payre@etu.univ-lyon1.fr> writes:
Show 10 quoted lines
> The existing code mixes parsing of email header with regular
> expression and actual code. Extract the parsing code into a new
> subroutine 'parse_header_line()'. This improves the code readability
> and make parse_header_line reusable in other place.
>
> Signed-off-by: Nathan Payre <nathan.payre@etu.univ-lyon1.fr>
> Signed-off-by: Matthieu Moy <matthieu.moy@univ-lyon1.fr>
> Signed-off-by: Timothee Albertin <timothee.albertin@etu.univ-lyon1.fr>
> Signed-off-by: Daniel Bensoussan <daniel.bensoussan--bohm@etu.univ-lyon1.fr>
> Thanks-to: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Thanks, but... 
Show 14 quoted lines
> +    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) {
> +	        while (my $body_line = <$c>) {
> +                if ($body_line !~ m/^GIT:/) {
> +                    $parsed_email{'body'} = $parsed_email{'body'} . $body_line;
> +                }
> +	        }
> +		}
> +		print "la : $line\n";
> +	}

... throughout this patch, not limited to this section, indentation is strange and there seem to be many "print" that show messages that do not seem to be meant for end-user consumption. I can see that this aspires to improve the readability, but not quite yet ;-).

Also "reusable in other place" is by itself not an unconditional plus, until readers can be convinced that that 'other place' really wants to be able to call this function. Is there some untold motivation behind this change---such as a planned update to actually use this helper subroutine?

Previous: Nathan PayreNext: Nathan PAYRE
Message 7 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.