git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
DOAN Tran Cong Danh <congdanhqx@gmail.com>
Date
May 22, 2017, 11:29 UTC
Message-ID
<9d42e133-75aa-d20a-0d38-bfbdc64d46fa@gmail.com>
In-Reply-To
<xmqqy3tppu13.fsf@gitster.mtv.corp.google.com>
On 05/22/2017 09:19 AM, Junio C Hamano wrote:
Show 18 quoted lines
> 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?

Show 10 quoted lines
> 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.

Previous: Junio C HamanoNext: Jeff King
Message 10 of 21 in “[Bug] git branch -v has problems with carriage returns”
  1. Animi VulpisMay 16, 2017
  2. Atousa DupratMay 19, 2017
  3. Animi VulpisMay 19, 2017
  4. Johannes SixtMay 20, 2017
  5. Atousa DupratMay 31, 2017
  6. Stefan BellerMay 31, 2017
  7. Junio C HamanoJun 1, 2017
  8. ref-filter: treat CRLF as same as LF in find_subposDOAN Tran Cong Danh, May 21, 2017
  9. Junio C HamanoMay 22, 2017
  10. DOAN Tran Cong DanhMay 22, 2017
  11. Jeff KingMay 22, 2017
  12. Junio C HamanoMay 23, 2017
  13. Junio C HamanoMay 23, 2017
  14. Junio C HamanoMay 23, 2017
  15. ref-filter: treat CRLF as same as LF in find_subposDOAN Tran Cong Danh, May 21, 2017
  16. ref-filter: trim end whitespace in subjectDOAN Tran Cong Danh, May 22, 2017
  17. ref-filter: trim end whitespace in subjectDOAN Tran Cong Danh, May 22, 2017
  18. Johannes SixtMay 22, 2017
  19. Jeff KingMay 22, 2017
  20. Johannes SixtMay 22, 2017
  21. Atousa DupratMay 19, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.