{"thread":{"id":"64671","subject":"[PATCH] ws: add new tab-between-non-ws check","startedAt":"2025-12-23T13:28:29Z","lastAt":"2026-01-05T21:00:05Z","messageCount":3,"participants":["Adrian Ratiu","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"532655","messageId":"20251223132756.604036-1-adrian.ratiu@collabora.com","threadId":"64671","inReplyTo":null,"subject":"[PATCH] ws: add new tab-between-non-ws check","fromName":"Adrian Ratiu","fromEmail":"adrian.ratiu@collabora.com","sentAt":"2025-12-23T13:27:56Z","receivedAt":"2025-12-23T13:28:29Z","isPatch":true,"sender":{"key":"adrian.ratiu@collabora.com","avatar":"https://avatars.githubusercontent.com/u/12472556?v=4"},"body":"This adds a new check to detect HT in the middle of sentences that\nshould have been a SP, as suggested by Junio in\nhttps://public-inbox.org/git/xmqqy0mwsedz.fsf@gitster.g/\n\nThe check is a bit complex because we want to detect places where\na SP was intended and the naive before/after character check can\nissue false positives in cases like \"a\\tb\".\n\nThe new check is enabled for Documentation/**/*.adoc, where these\nkinds of mistakes were seen in practice.\n\nSuggested-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>\n---\nThis is based on the latest master branch.\nPushed to GitHub: https://github.com/10ne1/git/tree/dev/aratiu/whitespace-new-test-v1\nCI run: https://github.com/10ne1/git/actions/runs/20457905508\n---\n .gitattributes             |  2 +-\n t/t4015-diff-whitespace.sh | 56 ++++++++++++++++++++++++++++++++++++++\n ws.c                       | 32 ++++++++++++++++++++++\n ws.h                       |  1 +\n 4 files changed, 90 insertions(+), 1 deletion(-)\n\ndiff --git a/.gitattributes b/.gitattributes\nindex 700743c3f5..d3c40a038b 100644\n--- a/.gitattributes\n+++ b/.gitattributes\n@@ -7,7 +7,7 @@\n *.py text eol=lf diff=python\n *.bat text eol=crlf\n CODE_OF_CONDUCT.md -whitespace\n-/Documentation/**/*.adoc text eol=lf whitespace=trail,space,incomplete\n+/Documentation/**/*.adoc text eol=lf whitespace=trail,space,incomplete,tab-between-non-ws\n /command-list.txt text eol=lf\n /GIT-VERSION-GEN text eol=lf\n /mergetools/* text eol=lf\ndiff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh\nindex 3c8eb02e4f..afe95f5209 100755\n--- a/t/t4015-diff-whitespace.sh\n+++ b/t/t4015-diff-whitespace.sh\n@@ -2440,4 +2440,60 @@ test_expect_success 'combine --ignore-blank-lines with --function-context 2' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'check tab between non-whitespace (tab-between-non-ws: off)' '\n+\tgit config core.whitespace \"-tab-between-non-ws\" &&\n+\tprintf \"1234567\\tb\" >x &&\n+\tgit add x &&\n+\tgit diff --cached --check\n+'\n+\n+test_expect_success 'check tab between non-whitespace at tab stop (tab-between-non-ws: on)' '\n+\tgit config core.whitespace \"tab-between-non-ws,tabwidth=8\" &&\n+\tprintf \"1234567\\tb\" >x &&\n+\tgit add x &&\n+\ttest_must_fail git diff --cached --check\n+'\n+\n+test_expect_success 'check tab between non-whitespace not at tab stop (tab-between-non-ws: on)' '\n+\tgit config core.whitespace \"tab-between-non-ws,tabwidth=8\" &&\n+\tprintf \"a\\tb\" >x &&\n+\tgit add x &&\n+\tgit diff --cached --check\n+'\n+\n+test_expect_success 'check tab between non-whitespace with tabwidth=4 (tab-between-non-ws: on)' '\n+\tgit config core.whitespace \"tab-between-non-ws,tabwidth=4\" &&\n+\tprintf \"123\\tb\" >x &&\n+\tgit add x &&\n+\ttest_must_fail git diff --cached --check\n+'\n+\n+test_expect_success 'check tab between non-whitespace with tabwidth=4 (tab-between-non-ws: on)' '\n+\tgit config core.whitespace \"tab-between-non-ws,tabwidth=4\" &&\n+\tprintf \"1234\\tb\" >x &&\n+\tgit add x &&\n+\tgit diff --cached --check\n+'\n+\n+test_expect_success 'check multiple tabs with one error (tab-between-non-ws: on)' '\n+\tgit config core.whitespace \"tab-between-non-ws,tabwidth=8\" &&\n+\tprintf \"a\\t1234567\\tb\" >x &&\n+\tgit add x &&\n+\ttest_must_fail git diff --cached --check\n+'\n+\n+test_expect_success 'check tab at beginning of line (tab-between-non-ws: on)' '\n+\tgit config core.whitespace \"tab-between-non-ws,tabwidth=8\" &&\n+\tprintf \"\\ta\" >x &&\n+\tgit add x &&\n+\tgit diff --cached --check\n+'\n+\n+test_expect_success 'check tab at end of line(tab-between-non-ws: on)' '\n+\tgit config core.whitespace \"tab-between-non-ws,-trailing-space,tabwidth=8\" &&\n+\tprintf \"a\\t\" >x &&\n+\tgit add x &&\n+\tgit diff --cached --check\n+'\n+\n test_done\ndiff --git a/ws.c b/ws.c\nindex 6cc2466c0c..fcd81250ad 100644\n--- a/ws.c\n+++ b/ws.c\n@@ -26,6 +26,7 @@ static struct whitespace_rule {\n \t{ \"blank-at-eol\", WS_BLANK_AT_EOL, 0 },\n \t{ \"blank-at-eof\", WS_BLANK_AT_EOF, 0 },\n \t{ \"tab-in-indent\", WS_TAB_IN_INDENT, 0, 1 },\n+\t{ \"tab-between-non-ws\", WS_TAB_BETWEEN_NON_WS, 0 },\n \t{ \"incomplete-line\", WS_INCOMPLETE_LINE, 0, 0 },\n };\n \n@@ -140,6 +141,11 @@ char *whitespace_error_string(unsigned ws)\n \t\t\tstrbuf_addstr(&err, \", \");\n \t\tstrbuf_addstr(&err, \"tab in indent\");\n \t}\n+\tif (ws & WS_TAB_BETWEEN_NON_WS) {\n+\t\tif (err.len)\n+\t\t\tstrbuf_addstr(&err, \", \");\n+\t\tstrbuf_addstr(&err, \"tab between non-whitespace characters\");\n+\t}\n \tif (ws & WS_INCOMPLETE_LINE) {\n \t\tif (err.len)\n \t\t\tstrbuf_addstr(&err, \", \");\n@@ -228,6 +234,32 @@ static unsigned ws_check_emit_1(const char *line, int len, unsigned ws_rule,\n \t\twritten = i;\n \t}\n \n+\tif (ws_rule & WS_TAB_BETWEEN_NON_WS) {\n+\t\t/*\n+\t\t * A tab surrounded by non-whitespace characters is a typo candidate\n+\t\t * (a space might have been intended). This checks for a tab that\n+\t\t * would be expanded to a single space, which is when it appears at\n+\t\t * a column that is one less than a multiple of the tabwidth.\n+\t\t */\n+\t\tint col = 0;\n+\t\tint tabwidth = ws_tab_width(ws_rule);\n+\n+\t\tif (!tabwidth)\n+\t\t\tBUG(\"a known tabwidth is required by WS_TAB_BETWEEN_NON_WS\");\n+\n+\t\tfor (i = 0; i < len; i++) {\n+\t\t\tif (line[i] == '\\t') {\n+\t\t\t\tif (i > 0 && i < len - 1 &&\n+\t\t\t\t    !isspace(line[i - 1]) && !isspace(line[i + 1]) &&\n+\t\t\t\t    (col % tabwidth) == (tabwidth - 1))\n+\t\t\t\t\tresult |= WS_TAB_BETWEEN_NON_WS;\n+\t\t\t\tcol += tabwidth - (col % tabwidth);\n+\t\t\t} else {\n+\t\t\t\tcol++;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n \tif (stream) {\n \t\t/*\n \t\t * Now the rest of the line starts at \"written\".\ndiff --git a/ws.h b/ws.h\nindex 06d5cb73f8..35475fd320 100644\n--- a/ws.h\n+++ b/ws.h\n@@ -16,6 +16,7 @@ struct strbuf;\n #define WS_BLANK_AT_EOF         (1<<10)\n #define WS_TAB_IN_INDENT        (1<<11)\n #define WS_INCOMPLETE_LINE      (1<<12)\n+#define WS_TAB_BETWEEN_NON_WS   (1<<13)\n \n #define WS_TRAILING_SPACE       (WS_BLANK_AT_EOL|WS_BLANK_AT_EOF)\n #define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB|8)\n-- \n2.51.2\n\n"},{"id":"532671","messageId":"xmqq5x9wpvor.fsf@gitster.g","threadId":"64671","inReplyTo":"20251223132756.604036-1-adrian.ratiu@collabora.com","subject":"Re: [PATCH] ws: add new tab-between-non-ws check","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-12-24T00:31:16Z","receivedAt":"2025-12-24T00:31:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adrian Ratiu <adrian.ratiu@collabora.com> writes:\n\n> This adds a new check to detect HT in the middle of sentences that\n> should have been a SP, as suggested by Junio in\n> https://public-inbox.org/git/xmqqy0mwsedz.fsf@gitster.g/\n>\n> The check is a bit complex because we want to detect places where\n> a SP was intended and the naive before/after character check can\n> issue false positives in cases like \"a\\tb\".\n\nAdd something like \"where the tab expands to more than one display\ncolumn\" at the end of the string, probably.  Otherwise what you\nmeant by \"false positive\" is unclear.\n\n> +test_expect_success 'check tab between non-whitespace (tab-between-non-ws: off)' '\n> +\tgit config core.whitespace \"-tab-between-non-ws\" &&\n> +\tprintf \"1234567\\tb\" >x &&\n> +\tgit add x &&\n> +\tgit diff --cached --check\n> +'\n\nI think the cases covered by these tests are wide enough.  The\nfeature covered by the code change in this patch is insufficient, so\nwhen it is improved, the tests for missing features would need to be\nadded to each of these cases.\n\n> @@ -228,6 +234,32 @@ static unsigned ws_check_emit_1(const char *line, int len, unsigned ws_rule,\n>  \t\twritten = i;\n>  \t}\n>  \n> +\tif (ws_rule & WS_TAB_BETWEEN_NON_WS) {\n> +\t\t/*\n> +\t\t * A tab surrounded by non-whitespace characters is a typo candidate\n> +\t\t * (a space might have been intended). This checks for a tab that\n> +\t\t * would be expanded to a single space, which is when it appears at\n> +\t\t * a column that is one less than a multiple of the tabwidth.\n> +\t\t */\n> +\t\tint col = 0;\n> +\t\tint tabwidth = ws_tab_width(ws_rule);\n> +\n> +\t\tif (!tabwidth)\n> +\t\t\tBUG(\"a known tabwidth is required by WS_TAB_BETWEEN_NON_WS\");\n> +\n> +\t\tfor (i = 0; i < len; i++) {\n> +\t\t\tif (line[i] == '\\t') {\n> +\t\t\t\tif (i > 0 && i < len - 1 &&\n> +\t\t\t\t    !isspace(line[i - 1]) && !isspace(line[i + 1]) &&\n> +\t\t\t\t    (col % tabwidth) == (tabwidth - 1))\n> +\t\t\t\t\tresult |= WS_TAB_BETWEEN_NON_WS;\n> +\t\t\t\tcol += tabwidth - (col % tabwidth);\n\nChecking \"i < len -1\" is good.  We do not need to check for a tab at\nthe end of the string here, as it is *not* \"between no-ws\".  If a\nfile type does not want a trailing tab, it can be marked with\ntrailing-whitespace.\n\n> +\t\t\t} else {\n> +\t\t\t\tcol++;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\n>  \tif (stream) {\n>  \t\t/*\n>  \t\t * Now the rest of the line starts at \"written\".\n> diff --git a/ws.h b/ws.h\n> index 06d5cb73f8..35475fd320 100644\n> --- a/ws.h\n> +++ b/ws.h\n> @@ -16,6 +16,7 @@ struct strbuf;\n>  #define WS_BLANK_AT_EOF         (1<<10)\n>  #define WS_TAB_IN_INDENT        (1<<11)\n>  #define WS_INCOMPLETE_LINE      (1<<12)\n> +#define WS_TAB_BETWEEN_NON_WS   (1<<13)\n>  \n>  #define WS_TRAILING_SPACE       (WS_BLANK_AT_EOL|WS_BLANK_AT_EOF)\n>  #define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB|8)\n\nThe above is sufficient to make \"git diff --check\" and \"git apply\n--whitespace=warn\" notice, but the code change in this patch is not\nsufficient.\n\n * Colored \"git diff\" output needs to highlight whitespace errors.\n   diff.c:diff_colors[] tells us to use BG_RED to paint them by\n   default.  At the end of ws_check_emit_1(), when stream is not\n   NULL, we emit the middle segment as-is, but when this new\n   whitespace error class is in effect, that part needs to paint the\n   tab at 7th column between !isspace() bytes.  Introduce a helper\n   function \"static emit_middle_section()\" to do so, perhaps.\n\n * \"git apply --whitespace=fix\" needs to turn such a HT to a SP.\n   This is probably done in ws_fix_copy().\n\nThanks.\n"},{"id":"533076","messageId":"875x9fai7s.fsf@gentoo.mail-host-address-is-not-set","threadId":"64671","inReplyTo":"xmqq5x9wpvor.fsf@gitster.g","subject":"Re: [PATCH] ws: add new tab-between-non-ws check","fromName":"Adrian Ratiu","fromEmail":"adrian.ratiu@collabora.com","sentAt":"2026-01-05T20:59:51Z","receivedAt":"2026-01-05T21:00:05Z","isPatch":true,"sender":{"key":"adrian.ratiu@collabora.com","avatar":"https://avatars.githubusercontent.com/u/12472556?v=4"},"body":"On Wed, 24 Dec 2025, Junio C Hamano <gitster@pobox.com> wrote:\n> The above is sufficient to make \"git diff --check\" and \"git apply\n> --whitespace=warn\" notice, but the code change in this patch is not\n> sufficient.\n>\n>  * Colored \"git diff\" output needs to highlight whitespace errors.\n>    diff.c:diff_colors[] tells us to use BG_RED to paint them by\n>    default.  At the end of ws_check_emit_1(), when stream is not\n>    NULL, we emit the middle segment as-is, but when this new\n>    whitespace error class is in effect, that part needs to paint the\n>    tab at 7th column between !isspace() bytes.  Introduce a helper\n>    function \"static emit_middle_section()\" to do so, perhaps.\n>\n>  * \"git apply --whitespace=fix\" needs to turn such a HT to a SP.\n>    This is probably done in ws_fix_copy().\n>\n> Thanks.\n\nIt took me a while, however I now have all functionality working\nproperly. Just need to cleanup the code a bit, add the extra tests and\nwill send a v2 very soon.\n\nThank you for your feedback and patience.\n"}]}