Re: [RFC/PATCH 3/3] mailinfo: handle in-body header continuations
From: Jonathan Tan <hidden>
Date: 2016-09-17 00:22:33
On 09/16/2016 04:04 PM, Junio C Hamano wrote:
Jonathan Tan [off-list ref] writes:quoted
I'm concerned about what happens if check_header fails - we would then have some lines which need to be treated as log messages. (At least, they are currently treated that way.)I actually think we should refactor check_header() further so that in-body header processing does not even see things that shouldn't be changed. The current check_header() should be used only for real mail headers, and then a reduced version of check_header() that is called for in-body will _ONLY_ handle the header lines that are handled by the first "search for the interesting parts" loop. And of course we would update your "does it look like rfc2822?" to match what are handled by the "interesting parts" loop. That I think would match the current behaviour much better, I would think.
There would be a bit of code duplication in that this "does it look like rfc2822" function would also need to account for duplicate headers (e.g. 2 "Subject:" lines in the in-body headers) because check_header would reject the 2nd one, but that is minor. (Alternatively, we could just allow duplicate headers in the in-body headers.)
The ">From " and "[PATCH]" cases in check_header() should not even be there. We should handle them inside handle_commit_msg(), as these two cases should never appear in the real header part of a message.
And if we clean it up like that, I do not think we would ever need to worry about "ah, it looked like a header but it is not after all". And not having to worry about it is a good thing and should be one of the primary goals in this conversion, I whould think.
Yes, this makes sense. I'll go ahead and make a patch set implementing this (unless anyone has any objections).