Re: [PATCH v2 2/2] diff: --ignore-cr-at-eol

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 8, 2017, 00:43 UTC
Message-ID: <xmqqh8u5ob0h.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <alpine.DEB.2.21.1.1711071345430.6482@virtualbox>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 14 quoted lines

> On Tue, 7 Nov 2017, Junio C Hamano wrote:
>
>> A new option --ignore-cr-at-eol tells the diff machinery to treat a
>> carriage-return at the end of a (complete) line as if it does not
>> exist.
>> 
>> This would make it easier to review a change whose only effect is to
>> turn line endings from CRLF to LF or the other way around.
>
> If the goal is to make CR/LF -> LF conversions easier to review (or for
> that matter, LF -> CR/LF), then this option may not be *completely*
> satisfactory, as it would hide mixed changes (i.e. where some lines are
> converted from CR/LF to LF and others are converted in the other direction
> *in the same patch*).

You are 100% right.

This feature is not about helping to review a patch that wanted to do CRLF-to-LF (or the other way around) conversion at all. Just like the --ignore-space-at-eol is not a feature to make sure that the only thing you did was to remove trailing whitespaces---it will also ignore lines you added trailing whitespaces as irrelevant and uninteresting.

In general, selling these "--ignore-*" whitespace options as a tool for such a verification is incorrect.

These "--ignore-*" whitespace options are to help reviewing _other_ changes without getting distracted by the class of changes these options represent. I guess I may have to update the log message (I do not think I wrote anything like that in the documentation update).

Thanks for pointing it out.