Re: [PATCH] ref-filter: treat CRLF as same as LF in find_subpos

[DOAN Tran Cong Danh <congdanhqx@gmail.com>]

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.