From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Payre Nathan <second.payre@gmail.com>
Cc: git@vger.kernel.org, ryan@michonline.com, e@80x24.org,
gitster@pobox.com, Nathan Payre <nathan.payre@etu.univ-lyon1.fr>,
Matthieu Moy <matthieu.moy@univ-lyon1.fr>,
Timothee Albertin <timothee.albertin@etu.univ-lyon1.fr>,
Daniel Bensoussan <daniel.bensoussan--bohm@etu.univ-lyon1.fr>
Subject: Re: [PATCH] send-email: extract email-parsing code into a subroutine
Date: Sun, 03 Dec 2017 23:00:29 +0100 [thread overview]
Message-ID: <874lp7v5du.fsf@evledraar.booking.com> (raw)
In-Reply-To: <20171202170220.10073-1-second.payre@gmail.com>
On Sat, Dec 02 2017, Payre Nathan jotted:
> From: Nathan Payre <second.payre@gmail.com>
>
> 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>
> ---
>
> This patch is a first step to implement a new feature.
> See new feature discussion here: https://public-inbox.org/git/20171030223444.5052-1-nathan.payre@etu.univ-lyon1.fr/
>
> git-send-email.perl | 106 +++++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 80 insertions(+), 26 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 2208dcc21..98c2e461c 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -715,41 +715,64 @@ EOT3
> if (!defined $compose_encoding) {
> $compose_encoding = "UTF-8";
> }
> - while(<$c>) {
> +
> + my %parsed_email;
> + while (<$c>) {
> next if m/^GIT:/;
> - if ($in_body) {
> - $summary_empty = 0 unless (/^\n$/);
> - } elsif (/^\n$/) {
> - $in_body = 1;
> - if ($need_8bit_cte) {
> + parse_header_line($_, \%parsed_email);
> + if (/^\n$/i) {
> + while (my $row = <$c>) {
> + if (!($row =~ m/^GIT:/)) {
> + $parsed_email{'body'} = $parsed_email{'body'} . $row;
> + }
> + }
> + }
> + }
> + if ($parsed_email{'from'}) {
> + $sender = $parsed_email{'from'};
> + }
> + if ($parsed_email{'in_reply_to'}) {
> + $initial_reply_to = $parsed_email{'in_reply_to'};
> + }
> + if ($parsed_email{'subject'}) {
> + $initial_subject = $parsed_email{'subject'};
> + print $c2 "Subject: " .
> + quote_subject($parsed_email{'subject'}, $compose_encoding) .
> + "\n";
> + }
> + if ($parsed_email{'mime-version'}) {
> + $need_8bit_cte = 0;
> + }
> + if ($need_8bit_cte) {
> + if ($parsed_email{'content-type'}) {
> + print $c2 "MIME-Version: 1.0\n",
> + "Content-Type: $parsed_email{'content-type'};",
> + "Content-Transfer-Encoding: 8bit\n";
> + } else {
> print $c2 "MIME-Version: 1.0\n",
> "Content-Type: text/plain; ",
> - "charset=$compose_encoding\n",
> + "charset=$compose_encoding\n",
> "Content-Transfer-Encoding: 8bit\n";
> }
> - } elsif (/^MIME-Version:/i) {
> - $need_8bit_cte = 0;
> - } elsif (/^Subject:\s*(.+)\s*$/i) {
> - $initial_subject = $1;
> - my $subject = $initial_subject;
> - $_ = "Subject: " .
> - quote_subject($subject, $compose_encoding) .
> - "\n";
> - } elsif (/^In-Reply-To:\s*(.+)\s*$/i) {
> - $initial_reply_to = $1;
> - next;
> - } elsif (/^From:\s*(.+)\s*$/i) {
> - $sender = $1;
> - next;
> - } elsif (/^(?:To|Cc|Bcc):/i) {
> - print __("To/Cc/Bcc fields are not interpreted yet, they have been ignored\n");
> - next;
> - }
> - print $c2 $_;
> }
> + if ($parsed_email{'body'}) {
> + $summary_empty = 0;
> + print $c2 "\n$parsed_email{'body'}\n";
> + }
> +
> close $c;
> close $c2;
>
> + open $c2, "<", $compose_filename . ".final"
> + or die sprintf(__("Failed to open %s.final: %s"), $compose_filename, $!);
> +
> + print "affichage : \n";
> + while (<$c2>) {
> + print $_;
> + }
> +
> + close $c2;
> +
> if ($summary_empty) {
> print __("Summary email is empty, skipping it\n");
> $compose = -1;
> @@ -792,6 +815,37 @@ sub ask {
> return;
> }
>
> +sub parse_header_line {
> + my $lines = shift;
> + my $parsed_line = shift;
> +
> + foreach (split(/\n/, $lines)) {
> + if (/^From:\s*(.+)$/i) {
> + $parsed_line->{'from'} = $1;
> + } elsif (/^To:\s*(.+)$/i) {
> + $parsed_line->{'to'} = [ parse_address_line($1) ];
> + } elsif (/^Cc:\s*(.+)$/i) {
> + $parsed_line->{'cc'} = [ parse_address_line($1) ];
> + } elsif (/^Bcc:\s*(.+)$/i) {
> + $parsed_line->{'bcc'} = [ parse_address_line($1) ];
> + } elsif (/^Subject:\s*(.+)\s*$/i) {
> + $parsed_line->{'subject'} = $1;
> + } elsif (/^Date: (.*)/i) {
> + $parsed_line->{'date'} = $1;
> + } elsif (/^In-Reply-To:\s*(.+)\s*$/i) {
> + $parsed_line->{'in_reply_to'} = $1;
> + } elsif (/^Message-ID: (.*)$/i) {
> + $parsed_line->{'message_id'} = $1;
> + } elsif (/^MIME-Version:$/i) {
> + $parsed_line->{'mime-version'} = $1;
> + } elsif (/^Content-Type:\s+(.*)\s*$/i) {
> + $parsed_line->{'content-type'} = $1;
> + } elsif (/^References:\s+(.*)/i) {
> + $parsed_line->{'references'} = $1;
> + }
> + }
> +}
> +
> my %broken_encoding;
>
> sub file_declares_8bit_cte {
I haven't read the patches that follow. Completely untested, But just a
diff on top I came up with while reading this:
Rationale:
* Once you start passing $_ to functions you should probably just give
it a name.
* !($x =~ m//) you can just write as $x !~ m//
* There's a lot of copy/paste programming in parse_header_line() and an
inconsistency between you seeing A-Header and turning it into either
a_header or a-header. If you just stick with a-header and use dash
you end up with just two cases.
The resulting line is quite long, so it's worth doing:
my $header_parsed = join "|", qw(To CC ...);
my $header_unparsed = join "|", qw(From Subject Message-ID ...);
[...]
if ($str =~ /^($header_unparsed)
diff --git a/git-send-email.perl b/git-send-email.perl
index 98c2e461cf..3696cad456 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -717,12 +717,12 @@ EOT3
}
my %parsed_email;
- while (<$c>) {
- next if m/^GIT:/;
- parse_header_line($_, \%parsed_email);
- if (/^\n$/i) {
+ while (my $line = <$c>) {
+ next if $line =~ m/^GIT:/;
+ parse_header_line($line, \%parsed_email);
+ if ($line =~ /^\n$/i) {
while (my $row = <$c>) {
- if (!($row =~ m/^GIT:/)) {
+ if ($row !~ m/^GIT:/) {
$parsed_email{'body'} = $parsed_email{'body'} . $row;
}
}
@@ -731,7 +731,7 @@ EOT3
if ($parsed_email{'from'}) {
$sender = $parsed_email{'from'};
}
- if ($parsed_email{'in_reply_to'}) {
+ if ($parsed_email{'in-reply-to'}) {
$initial_reply_to = $parsed_email{'in_reply_to'};
}
if ($parsed_email{'subject'}) {
@@ -820,28 +820,10 @@ sub parse_header_line {
my $parsed_line = shift;
foreach (split(/\n/, $lines)) {
- if (/^From:\s*(.+)$/i) {
- $parsed_line->{'from'} = $1;
- } elsif (/^To:\s*(.+)$/i) {
- $parsed_line->{'to'} = [ parse_address_line($1) ];
- } elsif (/^Cc:\s*(.+)$/i) {
- $parsed_line->{'cc'} = [ parse_address_line($1) ];
- } elsif (/^Bcc:\s*(.+)$/i) {
- $parsed_line->{'bcc'} = [ parse_address_line($1) ];
- } elsif (/^Subject:\s*(.+)\s*$/i) {
- $parsed_line->{'subject'} = $1;
- } elsif (/^Date: (.*)/i) {
- $parsed_line->{'date'} = $1;
- } elsif (/^In-Reply-To:\s*(.+)\s*$/i) {
- $parsed_line->{'in_reply_to'} = $1;
- } elsif (/^Message-ID: (.*)$/i) {
- $parsed_line->{'message_id'} = $1;
- } elsif (/^MIME-Version:$/i) {
- $parsed_line->{'mime-version'} = $1;
- } elsif (/^Content-Type:\s+(.*)\s*$/i) {
- $parsed_line->{'content-type'} = $1;
- } elsif (/^References:\s+(.*)/i) {
- $parsed_line->{'references'} = $1;
+ if (/^(To|Cc|Bcc):\s*(.+)$/i) {
+ $parsed_line->{lc $1} = [ parse_address_line($2) ];
+ } elsif (/^(From|Subject|Date|In-Reply-To|Message-ID|MIME-Version|Content-Type|References):\s*(.+)\s*$/i) {
+ $parsed_line->{lc $1} = $2;
}
}
}
next prev parent reply other threads:[~2017-12-03 22:00 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-02 17:02 [PATCH] send-email: extract email-parsing code into a subroutine Payre Nathan
2017-12-02 17:11 ` Nathan PAYRE
[not found] ` <445d0838cf2a4107bad95d5cc2d38a05@BPMBX2013-01.univ-lyon1.fr>
2017-12-03 21:20 ` Matthieu Moy
2017-12-03 22:00 ` Ævar Arnfjörð Bjarmason [this message]
2017-12-03 23:41 ` Nathan PAYRE
2017-12-04 13:45 ` Junio C Hamano
2017-12-06 15:38 ` [PATCH v2] " Nathan Payre
2017-12-06 21:39 ` Junio C Hamano
2017-12-06 22:55 ` Nathan PAYRE
2017-12-06 23:02 ` [PATCH v3] " Nathan Payre
2017-12-06 23:12 ` Ævar Arnfjörð Bjarmason
2017-12-07 10:28 ` [PATCH v4] " Nathan Payre
[not found] ` <ff9066a7209b4e21867d933542f8eece@BPMBX2013-01.univ-lyon1.fr>
2017-12-07 13:22 ` Matthieu Moy
2017-12-07 14:14 ` Ævar Arnfjörð Bjarmason
2017-12-06 23:06 ` [PATCH v2] " Junio C Hamano
2017-12-07 7:32 ` Eric Sunshine
[not found] ` <2b59497271cd4fada4ff590a001446cf@BPMBX2013-01.univ-lyon1.fr>
2017-12-07 7:57 ` [PATCH v3] " Matthieu Moy
2017-12-09 15:37 ` [PATCH] " Nathan Payre
[not found] ` <34c53164f4054ee88354f19fc38ae0c4@BPMBX2013-01.univ-lyon1.fr>
2017-12-11 21:12 ` Matthieu Moy
2017-12-14 10:06 ` Nathan PAYRE
2017-12-14 11:12 ` Nathan Payre
[not found] ` <ce70816f94c24754bea9bc8175de4bc4@BPMBX2013-01.univ-lyon1.fr>
2017-12-14 14:05 ` Matthieu Moy
2017-12-15 15:33 ` Nathan Payre
[not found] ` <3cafddfe825a4fb4a554f02aa3c025a3@BPMBX2013-01.univ-lyon1.fr>
2017-12-15 15:44 ` Matthieu Moy
2017-12-06 15:32 ` [PATCH v2] " Nathan Payre
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=874lp7v5du.fsf@evledraar.booking.com \
--to=avarab@gmail.com \
--cc=daniel.bensoussan--bohm@etu.univ-lyon1.fr \
--cc=e@80x24.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=matthieu.moy@univ-lyon1.fr \
--cc=nathan.payre@etu.univ-lyon1.fr \
--cc=ryan@michonline.com \
--cc=second.payre@gmail.com \
--cc=timothee.albertin@etu.univ-lyon1.fr \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).