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

From: Phillip Wood <[email-masked]>
Date: Tue, 11 Nov 2025 14:26:01 +0000

On 10/11/2025 18:29, Junio C Hamano wrote:
> Phillip Wood <[email-masked]> 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.

Thanks

Phillip

>> Thanks
>>
>> Phillip
>>
>>> Signed-off-by: Junio C Hamano <[email-masked]>
>>> ---
>>>    diff.c | 18 +++++++++++++++++-
>>>    1 file changed, 17 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/diff.c b/diff.c
>>> index b9ef8550cc..e73320dfb1 100644
>>> --- a/diff.c
>>> +++ b/diff.c
>>> @@ -601,6 +601,7 @@ struct emit_callback {
>>>    	int blank_at_eof_in_postimage;
>>>    	int lno_in_preimage;
>>>    	int lno_in_postimage;
>>> +	int last_line_kind;
>>>    	const char **label_path;
>>>    	struct diff_words_data *diff_words;
>>>    	struct diff_options *opt;
>>> @@ -2426,13 +2427,28 @@ static int fn_out_consume(void *priv, char *line, unsigned long len)
>>>    		break;
>>>    	case '\\':
>>>    		/* incomplete line at the end */
>>> -		ecbdata->lno_in_preimage++;
>>> +		switch (ecbdata->last_line_kind) {
>>> +		case '+':
>>> +			ecbdata->lno_in_postimage++;
>>> +			break;
>>> +		case '-':
>>> +			ecbdata->lno_in_preimage++;
>>> +			break;
>>> +		case ' ':
>>> +			ecbdata->lno_in_preimage++;
>>> +			ecbdata->lno_in_postimage++;
>>> +			break;
>>> +		default:
>>> +			BUG("fn_out_consume: '\\No newline' after unknown line (%c)",
>>> +			    ecbdata->last_line_kind);
>>> +		}
>>>    		emit_diff_symbol(o, DIFF_SYMBOL_CONTEXT_INCOMPLETE,
>>>    				 line, len, 0);
>>>    		break;
>>>    	default:
>>>    		BUG("fn_out_consume: unknown line '%s'", line);
>>>    	}
>>> +	ecbdata->last_line_kind = line[0];
>>>    	return 0;
>>>    }
>>>