Re: git-apply: warn/fail on *changed* end of line (eol) *only*?
- From
- Igor Djordjevic BugA <igor.d.djordjevic@gmail.com>
- Date
- Dec 27, 2016, 22:14 UTC
- Message-ID
- <00a4a54c-bc8e-9fef-0f70-232c85b0139b@gmail.com>
- In-Reply-To
- <xmqq8tr1cmf9.fsf@gitster.mtv.corp.google.com>
On 27/12/2016 18:27, Junio C Hamano wrote:
Show 25 quoted lines
> 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.Show 5 quoted lines
> 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 :/