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

[Phillip Wood <phillip.wood123@gmail.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 <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;
>   }
>