From: Adrian Ratiu <adrian.ratiu@collabora•com>
To: Johannes Sixt <j6t@kdbg•org>
Cc: Junio C Hamano <gitster@pobox•com>,
Patrick Steinhardt <ps@pks•im>,
Emily Shaffer <emilyshaffer@google•com>,
git@vger•kernel.org
Subject: Re: [PATCH v2] ws: add new tab-between-non-ws check
Date: Fri, 09 Jan 2026 15:33:47 +0200 [thread overview]
Message-ID: <87h5sux64k.fsf@collabora.com> (raw)
In-Reply-To: <dcd87fc4-6514-4146-9e44-1276bd739d2f@kdbg.org>
On Thu, 08 Jan 2026, Johannes Sixt <j6t@kdbg•org> wrote:
> Am 07.01.26 um 18:33 schrieb Johannes Sixt:
>> Am 07.01.26 um 02:30 schrieb Adrian Ratiu:
>>> The check is a bit complex because we want to detect places where
>>> a SP was intended (HT can expand to more than one display column),
>>> so we need to count both the display columns (col) and the string
>>> character columns (i) to determine if a HT looks identical to a SP
>>> or can cause confusion.
>>>
>>> Highlighting support for tools like git diff/show/log is added, as
>>> well as git apply --whitespace=fix capability.
>>>
>>> The middle section of the line used to be assumed non-highlighted,
>>> which is obviously not true anymore, so we split its logic into a
>>> separate function named emit_middle_section().
>>>
>>> The new check is enabled for Documentation/**/*.adoc, where these
>>> kinds of mistakes were seen in practice. It can also be enabled in
>>> other locations where it can be useful, by adding to the relevant
>>> attributes file.
>
> This makes me wonder how useful this check is. Yes, I has happened that
> I didn't spot at TAB that should have been a SP, but perhaps a handful
> of times in my career. Compare this to the many times that the other
> kinds of whitespace errors happened.
>
> Applying the rule to all documentation files is questionable: I can't
> format a table with TAB characters between columns reliably, because if
> a column happens to be 7 characters wide, the TAB at the 8th position
> would be diagnosed, but I certainly do *not* want it to be replaced by a
> SP. Yet, I might want legitimate cases outside tables to be diagnosed, so...
>
> Maybe I'm too much of a devil's advocate here...
I'll let Junio decide on the usefulness of this check since he's the one
who asked for it. :)
Maybe we could improve the heuristic to detect tables, for example
patterns like a\tb\tc.
I'm ok either way, just let me know if I should pursue this further.
prev parent reply other threads:[~2026-01-09 13:34 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-07 1:30 [PATCH v2] ws: add new tab-between-non-ws check Adrian Ratiu
2026-01-07 2:12 ` Junio C Hamano
2026-01-07 11:34 ` Adrian Ratiu
2026-01-07 17:33 ` Johannes Sixt
2026-01-07 18:11 ` Adrian Ratiu
2026-01-08 8:36 ` Johannes Sixt
2026-01-08 9:01 ` Johannes Sixt
2026-01-09 13:33 ` Adrian Ratiu [this message]
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=87h5sux64k.fsf@collabora.com \
--to=adrian.ratiu@collabora$(echo .)com \
--cc=emilyshaffer@google$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=j6t@kdbg$(echo .)org \
--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