Jonathan Tan <jonathanta...@google.com> writes: > This is an iteration of the patch set after the discussion in > <cover.1474047135.git.jonathanta...@google.com>. > > Changes: > o largely rewritten to follow Junio's suggested design (refactor of > check_header to separate out ">From" and "[PATCH]", and an > is_inbody_header function performing an airtight check on whether a > line is an in-body header) > o simpler try_convert_to_utf8 API > o one file of the expected output of t/t5100/*0015 is modified (instead > of the input) > o t/t5100/*0018--no-inbody-headers test files added > o example in commit message improved following Peff's suggestion
Overall it looks much nicer, but I still do not understand why we want to do try-convert-to-utf8. The _only_ caller of that helper function is in 4/4 where it tries to convert the line to see if a line is a scissors line. It seems to me that the only thing doing so gives us is that you could later enhance the definition of what a scissors-line looks like by adding unicode characters [*1*], but I would strongly suggest not doing that kind of enhancement [*2*]. With the current definition of what a scissors-line looks like, it can be checked before you do the UTF-8 conversion, I think, which would mean that the helper is not strictly necessary. So... [Footnotes] *1* E.g. ✂ encoded in non-UTF8 may have to be converted to UTF-8 first before being recognized as such. *2* Encouraging people to use non-ASCII that older version of Git did not take for structural things like "what is a scissors line" merely for the looks is a bad trade-off; patch e-mails that use the enhanced definition will break for those with older version of Git, and the benefit of "prettier scissors" is not really worth it, I would think.
