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

[Samuel GROOT <samuel.groot@grenoble-inp.org>]

On 06/08/2016 08:32 PM, Junio C Hamano wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
>>>> +     # Separate body from header
>>>> +     $mail{"body"} = [(<$fh>)];
>>>> +
>>>> +     return \%mail;
>>>
>>> The name of the local thing is not observable from the caller, but
>>> because this is "parse-email-header" and returns "header fields"
>>> without reading the "mail", perhaps call it %header instead?
>>
>> If there is (for some reason) a mail header named 'body', then this
>> assignment of the body portion of the message will overwrite it.
>> Perhaps this function should instead return multiple values: the
>> header hash, and the message body.
>
> Ah, I missed that it is attempting to return the body, too.
>
> Because the function takes an open filehandle, I think it is better
> to leave it to the callers.  A caller that is only interested in
> headers can just close $fh after this helper returns without reading
> body that it is not interested in, and a caller that wants to read
> the body can do the slurping itself.

I think it's the best way to do it indeed. Furthermore, we did trim CRs 
and LFs in header fields, but not in the message, making the subroutine 
inconsistent.

Should we rename the subroutine to `parse_header` or leave it as it is?