Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF

[Junio C Hamano <gitster@pobox.com>]

Johannes Sixt <j6t@kdbg.org> writes:

> Am 27.11.18 um 00:31 schrieb Junio C Hamano:
>> Johannes Sixt <j6t@kdbg.org> writes:
>>> Am 26.11.18 um 04:04 schrieb Junio C Hamano:
>>> ... this goes too far, IMO. It is the pager's task to decode control
>>> characters.
>>
>> It was tongue-in-cheek suggestion to split a CR into caret-em on our
>> end, but we'd get essentially the same visual effect if we added a
>> rule:
>>
>> 	When producing a colored output (not limited to whitespace
>> 	error coloring of diff output), insert <RESET> before a CR
>> 	that comes immediately before a LF.

This was misspoken a bit.  Let's revise it to

 	When producing a colored output (not limited to whitespace
 	error coloring of diff output) for contents that are not
 	marked as eol=crlf (and other historical means), insert
 	<RESET> before a CR that comes immediately before a LF.

to exempt CR in CRLF sequence that the contents and the user agree
to be part of the end-of-line marker.

> I wouldn't want that to happen for all output (context lines, - lines,
> + lines): I really am not interested to see all the CRs in my CRLF
> files.

So your CRLF files will not give caret-em.

>> And a good thing is that I do not think that new rule is doing any
>> decode of control chars on our end.  We are just producing colored
>> output normally.
>
> But we already have it, as Brian pointed out:
>
>    git diff --ws-error-highlight=old,new
>
> or by setting diff.wsErrorHighlight accordingly.

Not really.  If the context lines end with CRLF and the material is
not CRLF, I do not think CR at the end of them should be highlighted
as whitespace errors (as we tell to detect errors on "old" and "new"
lines), but I think we still should make an effort to help them be
visible to the users.  Otherwise, next Frank will come and complain
after seeing

	-something
	 some common thing
	+something_new^M

With the suggested change, you can distinguish the following two
(and use your imagination that there are many "some common thing"
lines), which would help the user guide if the file should be marked
as CRLF file, or the contents mistakenly has some lines that end
with CRLF by mistake.  The corrective action between the two cases
would vastly be different.

	-something^M          	-something          
	 some common thing^M  	 some common thing
	 some common thing^M  	 some common thing
	 some common thing^M  	 some common thing
	+something_new^M      	+something_new^M      

Without, both would look like the RHS of the illustration, and with
the "highlight both", you'd only get an extra caret-em on the
removed line, and need to judge without the help from common context
lines.

Besides, --ws-error-highlight shows all whitespace errors, and
showing CR as caret-em is mere side effect of the interaction
between its coloring and the pager.