Re: [docs] [PATCH v5 2/2] tools: Add check-confusables pre-commit hook

On 8/3/26 11:30 AM, Antonin Godard wrote:
> Hi,
> 
> On Mon Aug 3, 2026 at 5:48 AM CEST, Niko Mauno via lists.yoctoproject.org wrote:
>> From: Niko Mauno <[email protected]>
>>
>> Add a check-confusables script, in the same fashion as
>> check-glossaries, that scans the documentation .rst sources for
>> non-ASCII "confusable" characters (curly quotes, en/em dashes,
>> non-breaking and zero-width spaces, etc.) and reports each occurrence
>> with its location and suggested ASCII replacement, exiting non-zero if
>> any are found. This guards against the class of breakage fixed in the
>> preceding commit, e.g. curly quotes causing recipe ParseErrors.
>>
>> Legitimate non-ASCII such as box-drawing characters used in directory
>> trees, accented letters in contributor names and CJK characters are
>> intentionally left untouched. No-break spaces are likewise tolerated
>> on lines containing box-drawing characters, since the tree command
>> emits them as indentation in directory listings.
>>
>> The set of flagged characters is intentionally small and curated
>> rather than exhaustive. A general-purpose dependency such as the
>> confusables PyPI package targets Unicode homoglyph detection against
>> the full confusables table; it would also flag the accented names, CJK
>> and box-drawing characters we deliberately keep, so we would still need
>> our own allow-list and replacement policy on top of it. A short,
>> dependency-free table kept in-tree matches check-glossaries and is
>> trivial to extend if a new problematic character shows up.
>>
>> Wire it up both as a local pre-commit hook, which checks the changed
>> files, and in the Makefile "checks" target, which scans the whole
>> tree, alongside check-glossaries.
>>
>> Suggested-by: Quentin Schulz <[email protected]>
>> Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
>> Signed-off-by: Niko Mauno <[email protected]>
>> ---
>>   .pre-commit-config.yaml               |   5 ++
>>   documentation/Makefile                |   1 +
>>   documentation/tools/check-confusables | 123 ++++++++++++++++++++++++++
>>   3 files changed, 129 insertions(+)
>>   create mode 100755 documentation/tools/check-confusables
>>
>> diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
>> index f2b73a481..876546f9a 100644
>> --- a/.pre-commit-config.yaml
>> +++ b/.pre-commit-config.yaml
>> @@ -6,3 +6,8 @@ repos:
>>           entry: ./documentation/tools/check-glossaries
>>           language: python
>>           pass_filenames: false
>> +      - id: check-confusables
>> +        name: Check for non-ASCII confusable characters
>> +        entry: ./documentation/tools/check-confusables
>> +        language: python
>> +        files: \.rst$
>> diff --git a/documentation/Makefile b/documentation/Makefile
>> index fe0574537..87a6f8a8b 100644
>> --- a/documentation/Makefile
>> +++ b/documentation/Makefile
>> @@ -37,6 +37,7 @@ clean:
>>   
>>   checks:
>>   	$(SOURCEDIR)/tools/check-glossaries --docs-dir $(SOURCEDIR)
>> +	$(SOURCEDIR)/tools/check-confusables --docs-dir $(SOURCEDIR)
>>   
>>   stylecheck:
>>   	vale sync
>> diff --git a/documentation/tools/check-confusables b/documentation/tools/check-confusables
>> new file mode 100755
>> index 000000000..359386e7c
>> --- /dev/null
>> +++ b/documentation/tools/check-confusables
>> @@ -0,0 +1,123 @@
>> +#!/usr/bin/env python3
>> +#
>> +# Check documentation sources for non-ASCII typographic characters that
>> +# should be plain ASCII.
>> +#
>> +# Copyright (c) Vaisala Oyj. All rights reserved.
>> +#
>> +# SPDX-License-Identifier: MIT
>> +#
>> +
>> +import argparse
>> +import sys
>> +
>> +from pathlib import Path
>> +
>> +
>> +def parse_arguments() -> argparse.Namespace:
>> +    parser = argparse.ArgumentParser(
>> +        description="Check documentation sources for non-ASCII typographic "
>> +                    "characters that should be plain ASCII")
>> +
>> +    parser.add_argument("files",
>> +                        nargs="*",
>> +                        type=Path,
>> +                        help="Specific files to check; if none are given, "
>> +                             "all *.rst files under --docs-dir are scanned")
>> +
>> +    parser.add_argument("-d", "--docs-dir",
>> +                        type=Path,
>> +                        default=Path(__file__).resolve().parent.parent,
>> +                        help="Path to documentation/ directory in yocto-docs")
>> +
>> +    return parser.parse_args()
>> +
>> +
>> +# Map of "confusable" characters that are frequently introduced by editors,
>> +# word processors or copy-pasting, to their plain ASCII replacement. These
>> +# look almost identical to regular ASCII but break tooling, e.g. a curly
>> +# quote in a recipe example causes:
>> +#
>> +#   ERROR: ParseError ...: unparsed line: 'RDEPENDS:${PN} = “foo”'
>> +#
>> +# Only these characters are flagged; legitimate non-ASCII such as box-drawing
>> +# characters used in directory trees, accented letters in contributor names
>> +# and CJK characters are intentionally left alone.
>> +confusables = {
> 
> Nit, but this is defined in lowercase, but NO_BREAK_SPACE is uppercase, while
> both are global variables. Maybe make those all uppercase?
> 
>> +    "\u2018": "'",      # LEFT SINGLE QUOTATION MARK
>> +    "\u2019": "'",      # RIGHT SINGLE QUOTATION MARK
>> +    "\u201c": '"',      # LEFT DOUBLE QUOTATION MARK
>> +    "\u201d": '"',      # RIGHT DOUBLE QUOTATION MARK
>> +    "\u2032": "'",      # PRIME
>> +    "\u2033": '"',      # DOUBLE PRIME
>> +    "\u2013": "-",      # EN DASH
>> +    "\u2014": "--",     # EM DASH
>> +    "\u2010": "-",      # HYPHEN
>> +    "\u2011": "-",      # NON-BREAKING HYPHEN
>> +    "\u2212": "-",      # MINUS SIGN
>> +    "\u00a0": " ",      # NO-BREAK SPACE
> 
> You could reuse NO_BREAK_SPACE here, if you define it before.
> 
>> +    "\u202f": " ",      # NARROW NO-BREAK SPACE
>> +    "\u200b": "",       # ZERO WIDTH SPACE
>> +    "\ufeff": "",       # ZERO WIDTH NO-BREAK SPACE / BOM
>> +    "\u00ad": "",       # SOFT HYPHEN
>> +}
>> +
>> +NO_BREAK_SPACE = "\u00a0"
>> +
>> +
>> +def is_box_drawing(char: str) -> bool:
>> +    # Box Drawing Unicode block (U+2500..U+257F), used for the directory
>> +    # trees rendered in the manuals.
>> +    return "\u2500" <= char <= "\u257f"
>> +
>> +
>> +def check_file(path: Path, display: str) -> bool:
>> +    found = False
>> +
>> +    with open(path, "r", encoding="utf-8") as f:
>> +        for lineno, line in enumerate(f, start=1):
>> +            # The tree(1) command indents its directory listings with
>> +            # no-break spaces; such listings are embedded verbatim in the
>> +            # manuals. A no-break space is therefore tolerated on any line
>> +            # that also contains box-drawing characters (i.e. inside a
>> +            # rendered directory tree), but still flagged elsewhere.
>> +            in_tree = any(is_box_drawing(c) for c in line)
> 
> Not a blocking change, but at this point you could probably continue if in_tree
> is True and move onto the next line. Since we know we're in a box-drawing,
> leaving confusables is fine since it's not a command to run?

Thanks Antonin, submitted v6 based on your feedback on other counts,
except the above one:

   ▎ On skipping the rest of the line when in_tree is true: I kept the
   ▎ narrower per-character rule. The no-break space is the only confusable
   ▎ the tree command is known to emit in directory listings, so exempting
   ▎ the whole line would also stop flagging curly quotes and dashes there,
   ▎ without a known false positive to justify it. Happy to change it if
   ▎ you would rather have the simpler loop.

-Niko


> 
>> +            for col, char in enumerate(line, start=1):
>> +                if char not in confusables:
>> +                    continue
>> +                if char == NO_BREAK_SPACE and in_tree:
>> +                    continue
>> +                replacement = confusables[char]
>> +                hint = f"'{replacement}'" if replacement else "(remove)"
>> +                print(f"WARNING: {display}:{lineno}:{col}: non-ASCII "
>> +                      f"character U+{ord(char):04X} should be "
>> +                      f"replaced with {hint}")
>> +                found = True
>> +
>> +    return found
>> +
>> +
>> +def main():
>> +
>> +    args = parse_arguments()
>> +
>> +    # When invoked with explicit files (e.g. by pre-commit, which passes the
>> +    # staged filenames) only those are checked; otherwise the whole tree of
>> +    # *.rst files under --docs-dir is scanned (e.g. by "make checks").
>> +    if args.files:
>> +        targets = [(path, str(path)) for path in args.files]
> 
> Those are Path objects, I don't think you need to store their string
> representation separately to display them? Why not simply passing the Path
> object when calling print() directly? It should get converted to strings
> directly. And this would make check_file() simpler:
> 
> def check_file(path: Path) -> bool:
> 
>      ...
> 
>      print(f"WARNING: {path}:{lineno}:{col}: non-ASCII "
>            f"character U+{ord(char):04X} should be "
>            f"replaced with {hint}")
> 
>> +    else:
>> +        docs_dir = Path(args.docs_dir)
>> +        targets = [(path, str(path.relative_to(docs_dir)))
>> +                   for path in sorted(docs_dir.rglob("*.rst"))]
>> +
>> +    exit_code = 0
>> +    for path, display in targets:
>> +        if check_file(path, display):
>> +            exit_code = 1
>> +
>> +    sys.exit(exit_code)
>> +
>> +
>> +if __name__ == "__main__":
>> +    main()
> 
> Thanks,
> Antonin