On Mon, May 22, 2017 at 10:19:52AM +0900, Junio C Hamano wrote: > However. > > If you look at how `git branch -v` before that problematic change > removed the extra CR, you would notice that pretty_print_commit() > was used for that, which eventually called format_subject() with > "one\r\n\r\nline3...", got one line "one\r\n" by calling > get_one_line(), adjusted the line length from 5 to 3 by calling > is_blank_line() which as a side effect trims all whitespaces (not > just LF and CR), and emitted "one". The reason why the next \r\n > was not mistaken as a non-empty line is the same---is_blank_line() > call onthe next line said that "\r\n" is an all-white space line.
I noticed a similar thing regarding pretty_print_commit(). Which really made me wonder whether the right solution is to drop the custom parsing code in ref-filter.c and use the bits from pretty.c. Then we'd have only one parser. That's less code, but more importantly, there can't be inconsistencies between the two (we're fixing one now, but we have no idea if there are others). I suspect that's more work because we'd need to refactor pretty.c a bit to make the right functionality available. But the end result would be much more maintainable. -Peff PS I'm also a bit curious how a CRLF got into a commit message in the first place. Stripspace should be removing that for normal "git commit" runs. I don't know that we've ever said it explicitly, but I think we'd consider the canonical in-repo object representation to be LF-only (just like we do for smudge/clean filters). Which means that whatever is generating these commit objects is arguably buggy.