On 06/08/2016 08:32 PM, Junio C Hamano wrote:
Eric Sunshine <sunshine@xxxxxxxxxxxxxx> 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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html