From: Phillip Wood <phillip.wood123@gmail•com>
To: Junio C Hamano <gitster@pobox•com>, git@vger•kernel.org
Cc: Patrick Steinhardt <ps@pks•im>
Subject: Re: [PATCH 11/12] diff: highlight and error out on incomplete lines
Date: Mon, 10 Nov 2025 14:55:11 +0000 [thread overview]
Message-ID: <7aa91693-bece-4fa6-ab14-f914d6fd49bd@gmail.com> (raw)
In-Reply-To: <20251104020928.582199-12-gitster@pobox.com>
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
> Also teach "git diff --check" to notice incomplete lines as
> whitespace errors and report when incomplete-line whitespace error
> class is in effect.
Nice. The implementation looks good, I've left a few comments on the tests
> diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
> index 9de7f73f42..138730cbce 100755
> --- a/t/t4015-diff-whitespace.sh
> +++ b/t/t4015-diff-whitespace.sh
> @@ -43,6 +43,49 @@ do
> '
> done
>
> +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"
> + 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/
> + # 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?
Looking at the tests below the coverage looks good for "diff --check"
and for diff.wsErrorHighlight
Thanks
Phillip
> + git diff -R x &&
> + test_must_fail git -c core.whitespace=incomplete diff -R --check x
> +'
> +
> +test_expect_success "fix incomplete line in pre-image" '
> + echo foo | tr -d "\012" >x &&
> + git add x &&
> + echo bar >x &&
> + git diff x &&
> + git -c core.whitespace=incomplete diff --check x &&
> + git diff -R x &&
> + test_must_fail git -c core.whitespace=incomplete diff -R --check x
> +'
> +
> +test_expect_success "new incomplete line in post-image" '
> + echo foo >x &&
> + git add x &&
> + echo bar | tr -d "\012" >x &&
> + git diff x &&
> + test_must_fail git -c core.whitespace=incomplete diff --check x &&
> + git diff -R x &&
> + git -c core.whitespace=incomplete diff -R --check x
> +'
> +
> test_expect_success "Ray Lehtiniemi's example" '
> cat <<-\EOF >x &&
> do {
> @@ -1040,7 +1083,8 @@ test_expect_success 'ws-error-highlight test setup' '
> {
> echo "0. blank-at-eol " &&
> echo "1. still-blank-at-eol " &&
> - echo "2. and a new line "
> + echo "2. and a new line " &&
> + printf "3. and more"
> } >x &&
> new_hash_x=$(git hash-object x) &&
> after=$(git rev-parse --short "$new_hash_x") &&
> @@ -1050,11 +1094,13 @@ test_expect_success 'ws-error-highlight test setup' '
> <BOLD>index $before..$after 100644<RESET>
> <BOLD>--- a/x<RESET>
> <BOLD>+++ b/x<RESET>
> - <CYAN>@@ -1,2 +1,3 @@<RESET>
> + <CYAN>@@ -1,2 +1,4 @@<RESET>
> 0. blank-at-eol <RESET>
> <RED>-<RESET><RED>1. blank-at-eol<RESET><BLUE> <RESET>
> <GREEN>+<RESET><GREEN>1. still-blank-at-eol<RESET><BLUE> <RESET>
> <GREEN>+<RESET><GREEN>2. and a new line<RESET><BLUE> <RESET>
> + <GREEN>+<RESET><GREEN>3. and more<RESET>
> + <BLUE>\ No newline at end of file<RESET>
> EOF
>
> cat >expect.all <<-EOF &&
> @@ -1062,11 +1108,13 @@ test_expect_success 'ws-error-highlight test setup' '
> <BOLD>index $before..$after 100644<RESET>
> <BOLD>--- a/x<RESET>
> <BOLD>+++ b/x<RESET>
> - <CYAN>@@ -1,2 +1,3 @@<RESET>
> + <CYAN>@@ -1,2 +1,4 @@<RESET>
> <RESET>0. blank-at-eol<RESET><BLUE> <RESET>
> <RED>-<RESET><RED>1. blank-at-eol<RESET><BLUE> <RESET>
> <GREEN>+<RESET><GREEN>1. still-blank-at-eol<RESET><BLUE> <RESET>
> <GREEN>+<RESET><GREEN>2. and a new line<RESET><BLUE> <RESET>
> + <GREEN>+<RESET><GREEN>3. and more<RESET>
> + <BLUE>\ No newline at end of file<RESET>
> EOF
>
> cat >expect.none <<-EOF
> @@ -1074,16 +1122,19 @@ test_expect_success 'ws-error-highlight test setup' '
> <BOLD>index $before..$after 100644<RESET>
> <BOLD>--- a/x<RESET>
> <BOLD>+++ b/x<RESET>
> - <CYAN>@@ -1,2 +1,3 @@<RESET>
> + <CYAN>@@ -1,2 +1,4 @@<RESET>
> 0. blank-at-eol <RESET>
> <RED>-1. blank-at-eol <RESET>
> <GREEN>+1. still-blank-at-eol <RESET>
> <GREEN>+2. and a new line <RESET>
> + <GREEN>+3. and more<RESET>
> + \ No newline at end of file<RESET>
> EOF
>
> '
>
> test_expect_success 'test --ws-error-highlight option' '
> + git config core.whitespace blank-at-eol,incomplete-line &&
>
> git diff --color --ws-error-highlight=default,old >current.raw &&
> test_decode_color <current.raw >current &&
> @@ -1100,6 +1151,7 @@ test_expect_success 'test --ws-error-highlight option' '
> '
>
> test_expect_success 'test diff.wsErrorHighlight config' '
> + git config core.whitespace blank-at-eol,incomplete-line &&
>
> git -c diff.wsErrorHighlight=default,old diff --color >current.raw &&
> test_decode_color <current.raw >current &&
> @@ -1116,6 +1168,7 @@ test_expect_success 'test diff.wsErrorHighlight config' '
> '
>
> test_expect_success 'option overrides diff.wsErrorHighlight' '
> + git config core.whitespace blank-at-eol,incomplete-line &&
>
> git -c diff.wsErrorHighlight=none \
> diff --color --ws-error-highlight=default,old >current.raw &&
> @@ -1135,6 +1188,8 @@ test_expect_success 'option overrides diff.wsErrorHighlight' '
> '
>
> test_expect_success 'detect moved code, complete file' '
> + git config core.whitespace blank-at-eol &&
> +
> git reset --hard &&
> cat <<-\EOF >test.c &&
> #include<stdio.h>
next prev parent reply other threads:[~2025-11-10 14:55 UTC|newest]
Thread overview: 73+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-04 2:09 [PATCH 00/12] Incomplete lines Junio C Hamano
2025-11-04 2:09 ` [PATCH 01/12] whitespace: correct bit assignment comments Junio C Hamano
2025-11-04 2:09 ` [PATCH 02/12] diff: emit_line_ws_markup() if/else style fix Junio C Hamano
2025-11-04 2:09 ` [PATCH 03/12] diff: correct suppress_blank_empty hack Junio C Hamano
2025-11-04 2:09 ` [PATCH 04/12] diff: fix incorrect counting of line numbers Junio C Hamano
2025-11-10 14:54 ` Phillip Wood
2025-11-10 18:29 ` Junio C Hamano
2025-11-11 14:26 ` Phillip Wood
2025-11-11 14:37 ` Junio C Hamano
2025-11-04 2:09 ` [PATCH 05/12] diff: refactor output of incomplete line Junio C Hamano
2025-11-04 2:09 ` [PATCH 06/12] diff: call emit_callback ecbdata everywhere Junio C Hamano
2025-11-04 2:09 ` [PATCH 07/12] diff: update the way rewrite diff handles incomplete lines Junio C Hamano
2025-11-10 14:54 ` Phillip Wood
2025-11-10 18:33 ` Junio C Hamano
2025-11-04 2:09 ` [PATCH 08/12] apply: revamp the parsing of " Junio C Hamano
2025-11-04 2:09 ` [PATCH 09/12] whitespace: allocate a few more bits Junio C Hamano
2025-11-04 2:09 ` [PATCH 10/12] apply: check and fix incomplete lines Junio C Hamano
2025-11-04 2:09 ` [PATCH 11/12] diff: highlight and error out on " Junio C Hamano
2025-11-10 14:55 ` Phillip Wood [this message]
2025-11-10 18:38 ` Junio C Hamano
2025-11-10 23:56 ` D. Ben Knoble
2025-11-04 2:09 ` [PATCH 12/12] attr: enable incomplete-line whitespace error for this project Junio C Hamano
2025-11-10 14:55 ` Phillip Wood
2025-11-10 18:40 ` Junio C Hamano
2025-11-05 21:30 ` [PATCH v2 00/12] Incomplete lines Junio C Hamano
2025-11-05 21:30 ` [PATCH v2 01/12] whitespace: correct bit assignment comments Junio C Hamano
2025-11-05 21:30 ` [PATCH v2 02/12] diff: emit_line_ws_markup() if/else style fix Junio C Hamano
2025-11-05 21:30 ` [PATCH v2 03/12] diff: correct suppress_blank_empty hack Junio C Hamano
2025-11-05 21:30 ` [PATCH v2 04/12] diff: fix incorrect counting of line numbers Junio C Hamano
2025-11-05 21:30 ` [PATCH v2 05/12] diff: refactor output of incomplete line Junio C Hamano
2025-11-10 10:06 ` Patrick Steinhardt
2025-11-10 17:58 ` Junio C Hamano
2025-11-05 21:30 ` [PATCH v2 06/12] diff: call emit_callback ecbdata everywhere Junio C Hamano
2025-11-05 21:30 ` [PATCH v2 07/12] diff: update the way rewrite diff handles incomplete lines Junio C Hamano
2025-11-10 10:06 ` Patrick Steinhardt
2025-11-10 18:14 ` Junio C Hamano
2025-11-05 21:30 ` [PATCH v2 08/12] apply: revamp the parsing of " Junio C Hamano
2025-11-05 21:30 ` [PATCH v2 09/12] whitespace: allocate a few more bits and define WS_INCOMPLETE_LINE Junio C Hamano
2025-11-05 21:30 ` [PATCH v2 10/12] apply: check and fix incomplete lines Junio C Hamano
2025-11-05 21:30 ` [PATCH v2 11/12] diff: highlight and error out on " Junio C Hamano
2025-11-05 21:30 ` [PATCH v2 12/12] attr: enable incomplete-line whitespace error for this project Junio C Hamano
2025-11-10 10:09 ` [PATCH v2 00/12] Incomplete lines Patrick Steinhardt
2025-11-10 14:53 ` Phillip Wood
2025-11-11 0:04 ` [PATCH v3 " Junio C Hamano
2025-11-11 0:04 ` [PATCH v3 01/12] whitespace: correct bit assignment comments Junio C Hamano
2025-11-11 0:04 ` [PATCH v3 02/12] diff: emit_line_ws_markup() if/else style fix Junio C Hamano
2025-11-11 0:04 ` [PATCH v3 03/12] diff: correct suppress_blank_empty hack Junio C Hamano
2025-11-11 0:04 ` [PATCH v3 04/12] diff: fix incorrect counting of line numbers Junio C Hamano
2025-11-11 0:04 ` [PATCH v3 05/12] diff: refactor output of incomplete line Junio C Hamano
2025-11-11 0:04 ` [PATCH v3 06/12] diff: call emit_callback ecbdata everywhere Junio C Hamano
2025-11-11 0:04 ` [PATCH v3 07/12] diff: update the way rewrite diff handles incomplete lines Junio C Hamano
2025-11-11 0:04 ` [PATCH v3 08/12] apply: revamp the parsing of " Junio C Hamano
2025-11-11 0:04 ` [PATCH v3 09/12] whitespace: allocate a few more bits and define WS_INCOMPLETE_LINE Junio C Hamano
2025-11-11 0:04 ` [PATCH v3 10/12] apply: check and fix incomplete lines Junio C Hamano
2025-11-11 0:04 ` [PATCH v3 11/12] diff: highlight and error out on " Junio C Hamano
2025-11-11 0:04 ` [PATCH v3 12/12] attr: enable incomplete-line whitespace error for this project Junio C Hamano
2025-11-11 14:29 ` [PATCH v3 00/12] Incomplete lines Phillip Wood
2025-11-12 22:02 ` [PATCH v4 " Junio C Hamano
2025-11-12 22:02 ` [PATCH v4 01/12] whitespace: correct bit assignment comments Junio C Hamano
2025-11-12 22:02 ` [PATCH v4 02/12] diff: emit_line_ws_markup() if/else style fix Junio C Hamano
2025-11-12 22:02 ` [PATCH v4 03/12] diff: correct suppress_blank_empty hack Junio C Hamano
2025-11-12 22:02 ` [PATCH v4 04/12] diff: keep track of the type of the last line seen Junio C Hamano
2025-11-12 22:02 ` [PATCH v4 05/12] diff: refactor output of incomplete line Junio C Hamano
2025-11-12 22:02 ` [PATCH v4 06/12] diff: call emit_callback ecbdata everywhere Junio C Hamano
2025-11-12 22:02 ` [PATCH v4 07/12] diff: update the way rewrite diff handles incomplete lines Junio C Hamano
2025-11-12 22:02 ` [PATCH v4 08/12] apply: revamp the parsing of " Junio C Hamano
2025-11-12 22:02 ` [PATCH v4 09/12] whitespace: allocate a few more bits and define WS_INCOMPLETE_LINE Junio C Hamano
2025-11-12 22:02 ` [PATCH v4 10/12] apply: check and fix incomplete lines Junio C Hamano
2025-11-12 22:02 ` [PATCH v4 11/12] diff: highlight and error out on " Junio C Hamano
2025-11-12 22:02 ` [PATCH v4 12/12] attr: enable incomplete-line whitespace error for this project Junio C Hamano
2025-11-14 10:24 ` [PATCH v4 00/12] Incomplete lines Phillip Wood
2025-11-14 16:25 ` Junio C Hamano
2025-11-23 2:35 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7aa91693-bece-4fa6-ab14-f914d6fd49bd@gmail.com \
--to=phillip.wood123@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=phillip.wood@dunelm$(echo .)org.uk \
--cc=ps@pks$(echo .)im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox