From: Junio C Hamano Date: Tue, 11 Nov 2025 14:37:39 GMT Subject: Re: [PATCH 04/12] diff: fix incorrect counting of line numbers Message-ID: In-Reply-To: <4506b9c3-f4ae-488c-988c-e12b2d95195f@gmail.com> Phillip Wood writes: > On 10/11/2025 18:29, Junio C Hamano wrote: >> Phillip Wood 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.