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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:

> 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.
> ...
> 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.

Yes, before I sent my response I debated myself if I should suggest
going that route (I would have done so if I were working with a
contributor who is already familiar with our codebase).  I agree
that it is the right direction to go in the longer term.

> PS I'm also a bit curious how a CRLF got into a commit message in the
>    first place.

I think I read somebody mumbled gitlab somewhere upthread.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]