From: Junio C Hamano Date: Mon, 10 Nov 2025 18:29:13 GMT Subject: Re: [PATCH 04/12] diff: fix incorrect counting of line numbers Message-ID: In-Reply-To: Phillip Wood 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 >> --- >> 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; >> } >>