On 27/12/2016 18:27, Junio C Hamano wrote:
> To see the problem with "check existing lines", it probably is
> easier to extend the above example to start from a file with one
> more line, like this:
>
> 1 <CRLF>
> 3 <CRLF>
> 4 <LF>
> 5 <CRLF>
>
> and extend all the example patches to remove "4 <LF>" line as well,
> where they remove "3 <CRLF>", making the first example patch like
> so:
>
> 1 <CRLF>
> -3 <CRLF>
> -4 <LF>
> +three <CRLF>
> 5 <CRLF>
>
> Now, if you take "three <CRLF>" to be replacing "3 <CRLF>", then you
> may feel that not warning on the CRLF would be the right thing, but
> there is no reason (other than the fact you, a human, understand
> what 'three' means) to choose "3 <CRLF>" over "4 <LF>" as the
> original. If you take "three <CRLF>" to be replacing "4 <LF>", you
> would need to warn.
Hmm, but why do you keep insisting on knowing what the lines are about,
and still making a 'per line' end of line comparison? Besides, your
example falls within the (only?) special case I mentioned as the one Git
probably shouldn`t be acting upon, as both <LF> and <CRLF> are present
among the the old hunk lines already (lines starting with '-').
The logic should be simple, what I tried to describe in the previous
message:
1. Examine all '-' lines line endings.
1.1. If not all line endings are the same (like in your example, no
matter the line content), do nothing.
1.2. If all line endings _are_ the same within the old hunk ('-'
lines), check if any of the '+' lines (new hunk) has a different
line ending (still no matter the line content).
1.2.1. If no different line endings among '+' lines in comparison to
unique line ending found in '-' lines, do nothing.
1.2.2. If _any_ of the '+' lines _has_ a different line ending in
comparison to unique line ending found in '-' lines, then do
warn/fail.
This even might seem as the most sensible approach, no matter the
overall project end of line setting, which is exactly why I find it
interesting - it seems to 'just work', being beginner friendly _and_
also watching your back on unexpected end of line changes.
> A totally uninteresting special case is when the original is all
> <CRLF>, but in that case, as you already said in the original
> message, the project wants <CRLF> and you can configure it as such.
> Then <CR> in the line-end <CRLF> won't be mistaken as if it is a
> whitespace character <CR> at the end of a line terminated with <LF>.
But this is exactly the most confusing case, especially for beginners,
where project configuration is incorrect, and you get warned about
'whitespace errors' where all line endings (old/new) are in fact still
the same (confusing).
Yet worse, you don`t get warned of different line endings being applied,
as these are considered 'correct' due to current (incorrect) setting, no
matter the whole file is actually different, which you don`t get warned
about in the first place, and you may discover the file line ending
breakage only when other contributors start complaining.
Also, would it be sensible to account for a possible file inside the
project having different line endings than project in general...? This
would help there as well, without unintentionally breaking the file.
Regards, BugA
P.S. I guess you don`t need me to tell you that, but please feel free
to drop out of this discussion at your own discretion, even though I
enjoy sharing thoughts (and learning new stuff!), I do kind of feel I`m
wasting your time for something that might not be that important, if at
all, otherwise someone would have probably addressed it so far :/