Re: [PATCH v4 4/6] send-email: create email parser subroutine

From: Samuel GROOT <samuel.groot@grenoble-inp.org>
To: Junio C Hamano <gitster@pobox.com>,
	Eric Sunshine <sunshine@sunshineco.com>
Cc: Git List <git@vger.kernel.org>,
	tom.russello@grenoble-inp.org, erwan.mathoniere@grenoble-inp.org,
	jordan.de-gea@grenoble-inp.org,
	Matthieu Moy <matthieu.moy@grenoble-inp.fr>,
	aaron@schrab.com, Eric Wong <e@80x24.org>
Subject: Re: [PATCH v4 4/6] send-email: create email parser subroutine
Date: Thu, 9 Jun 2016 01:54:03 +0200	[thread overview]
Message-ID: <4c1dcf95-d8bb-4649-8980-fa947617b96e@grenoble-inp.org> (raw)
In-Reply-To: <xmqqy46fjuzm.fsf@gitster.mtv.corp.google.com>

On 06/08/2016 10:17 PM, Junio C Hamano wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> An embedded CR probably shouldn't happen, but I'm not convinced that
>> folding it out is a good idea. I would think that you'd want to
>> preserve the header's value verbatim. If anything, I'd expect to see
>> the regex tightened to:
>>
>>     s/\r?\n$//;
>
> Yes, that would be more sensible than silently removing \r in the
> middle which _is_ a sign of something funny going on.
>> Alternately, consider using 'chop' or 'chomp'.
>
> Even if you use chomp(), you'd still need to worry about possible \r
> at the end, no?

'chomp' is what we used before, but with *.eml files (microsoft's file 
format, with CRLF), '\n' were removed but '\r' remained, that's why we 
used s/\r\n|\r|\n//.

s/\r?\n$// looks fine.

Email::Simple library uses qr/\x0a\x0d|\x0d\x0a|\x0a|\x0d/ [1]. Should 
we handle \n\r at end of line as well?

[1] * 
http://cpansearch.perl.org/src/RJBS/Email-Simple-2.210/lib/Email/Simple.pm