Re: [PATCH 11/12] diff: highlight and error out on incomplete lines

[Junio C Hamano <gitster@pobox.com>]

Phillip Wood <phillip.wood123@gmail.com> writes:

> On 04/11/2025 02:09, Junio C Hamano wrote:
>> Teach "git diff" to highlight "\ No newline at end of file" message
>> as a whitespace error when incomplete-line whitespace error class is
>> in effect.  Thanks to the previous refactoring of complete rewrite
>> code path, we can do this at a single place.
>> 
>> Unlike whitespace errors in the payload where we need to annotate in
>> line, possibly using colors, the line that has whitespace problems,
>> we have a dedicated line already that can serve as the error
>> message, so paint it as a whitespace error message.
>
> This explains why we don't need to call emit_line_ws_markup() in this case

True.

Also, even if we were to call it on the previous, problematic line
without terminating newline, there is no good spot on that line to
paint red to grab attention to the reader, as we are trying to
highlight lack of something, not presence of unwanted things, like
trailing whitespaces ;-)

>> +test_expect_success "incomplete line in both pre- and post-image context" '
>> +	(echo foo && echo baz | tr -d "\012") >x &&
>
> 'printf "foo\nbaz"' might be clearer and save us forking "tr"

Perhaps.  I find it much harder to read and uglier, though.

>> +	git add x &&
>> +	(echo bar && echo baz | tr -d "\012") >x &&
>> +	git diff x &&
>> +	git -c core.whitespace=incomplete diff --check x &&
>> +	git diff -R x &&
>> +	git -c core.whitespace=incomplete diff -R --check x
>> +'
>> +
>> +test_expect_success "incomplete lines on both pre- and post-image" '
>> +	# The interpretation taken here is "since you are toucing
>
> s/toucing/touching/

Thanks.

>
>> +	# the line anyway, you would better fix the incomplete line
>> +	# while you are at it."  but this is debatable.
>
> I think it is a reasonable default.
>> +	echo foo | tr -d "\012" >x &&
>> +	git add x &&
>> +	echo bar | tr -d "\012" >x &&
>> +	git diff x &&
>> +	test_must_fail git -c core.whitespace=incomplete diff --check x &&
>
> Do we want to check the error message here?

Probably an overkill, but I could try.

> Looking at the tests below the coverage looks good for "diff --check" 
> and for diff.wsErrorHighlight