From: Phillip Wood Date: Mon, 10 Nov 2025 14:54:23 GMT Subject: Re: [PATCH 04/12] diff: fix incorrect counting of line numbers Message-ID: In-Reply-To: <20251104020928.582199-5-gitster@pobox.com> 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? 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; > } >