Re: [PATCH 04/12] diff: fix incorrect counting of line numbers

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

Phillip Wood <phillip.wood123@gmail.com> writes:

> On 10/11/2025 18:29, Junio C Hamano wrote:
>> Phillip Wood <phillip.wood123@gmail.com> writes:
>>> On 04/11/2025 02:09, Junio C Hamano wrote:
>>>
>>> The "\ No newline at end of file" line is an annotation on the previous
>>> line in the diff so why are we incrementing any {pre,post}image line
>>> numbers here?
>> 
>> No particular reason ;-)  As I said, I do not think these numbers
>> are used after these lines are seen.  At least this change makes
>> these unused data incremented in a more coherent way than the
>> previous one, which unconditionally incremented the number for the
>> preimage without even checking which side the "\ No newline" is for.
>
> It maybe coherent but it is still wrong to increment the line numbers 
> here. To be correct we should remove the erroneous increment of 
> lno_in_postimage.

Ah, if your lno_in_postimage is not a typo for preimage side, then I
can buy that and it would be even safer than the version under
discussion.  I do not think anybody has audited the code to be
absolutely sure that the existing increment for preimage line
number, which we think is wrong in this discussion thread, is not
compensated by something else, which would make removal of the
increment break it, so in the absense of such an audit (and the
theme of this topic certainly is not about it), it is prudent to let
this sleeping dog lie.

The primary thing this step needed to do was to introduce the
last_line_kind member to the emit-callback structure and switch on
its value to allow us to process "\ No newline" differently
depending on what kind of line the previous line was.  The
"consistently increment on both sides" was a "while at it" change
that does not have to be done and probably better left out.

Thanks.