On 05/22/2017 09:19 AM, Junio C Hamano wrote: > 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. > > So, while I think the realization that we have a problem, and the > analysis above i.e. "emptiness of the line matters", are both good, > but I do not think this is suffucient to get back the old behaviour. > > The thing is, we never treated CRLF as line separator in this code. > What we did was to treat LF as line separator, but trimmed trailing > bytes that are isspace(). That is what the analysis you quoted from > J6t says.
If I understand correctly, we should trim all whitespaces at the end of subject line, and we should treat lines contain only whitespaces as empty lines, right? > Here is your test addition: > If you change this test to > >> + git branch branch-two $(printf "%s\n" one " " line3_long line4 | >> + git commit-tree HEAD:) > > then the old code before the problematic change will only show the > first line i.e. "one" in "branch -v" output. I think with or > without your code change, the new code would still show line3_long > and line4 in the output. Agree! If you could confirm my understand, I would happily provide new patch to trim all trailing whitespaces at the end of subject line and treat next all-whitespace-line as empty line.
