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

DOAN Tran Cong Danh [off-list ref] writes:

Starting from commit 949af06 (branch: use ref-filter printing APIs, 2017-01-10),
`git branch -v` doesn't treat CRLF as line separator anymore.

A seemingly good problem identification (but not quite; see below) ...

Quote from git mailing-list:

quoted

Here is a recipe to reproduce the error:

   git init
   git commit --allow-empty -m initial
   git branch crlf $(printf '%s\r\n' subject '' line3_long line4 |
       git commit-tree HEAD:)
The reason for the "bug" is obviously that a line having CR in addition
to LF is not "an empty line". Consequently, the second line is not
treated as a separator between subject and body, whereupon Git
concatenates all line into one large subject line. This strips the LFs
but leaves the CRS in tact, which, when printed on a terminal move the
cursor to the beginning of the line, so that text after the CRs
overwrites what is already in the terminal.

... and good analysis.

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.

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.

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.