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 04/11/2025 02:09, Junio C Hamano wrote:
>> The "\ No newline at the end of the file" can come after any of the
>> "-" (deleted preimage line), " " (unchanged line), or "+" (added
>> postimage line).  Incrementing only the preimage line number upon
>> seeing it does not make any sense.
>> 
>> We can keep track of what the previous line was, and increment
>> lno_in_{pre,post}image variables properly, like this patch does.  I
>> do not think it matters, as these numbers are used only to compare
>> them with blank_at_eof_in_{pre,post}image to issue the warning every
>> time we see an added line, but by definition, after we see "\ No
>> newline at the end of the file" for an added line, we will not see
>> an added line for the file.
>> 
>> Keeping track of what the last line was (in other words, "is it that
>> the file used to end in an incomplete line?  The file ends in an
>> incomplete line after the change?  Both the file before and after
>> the change ends in an incomplete line that did not change?") will be
>> independently useful.
>
> 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.

> Thanks
>
> Phillip
>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>>   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;
>>   }
>>