Re: [PATCH 04/12] diff: fix incorrect counting of line numbers
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 10, 2025, 18:29 UTC
- Message-ID
- <xmqqjyzx213a.fsf@gitster.g>
- In-Reply-To
- <c41f3c65-d7ef-4e73-a1e0-03540df0b212@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
Show 23 quoted lines
> 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.
Show 51 quoted lines
> 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;
>> }
>>