fix(tools): make audit_colours.py run to completion on a legacy-code-page console - #4475
#4475fix(tools): make audit_colours.py run to completion on a legacy-code-page console#4475nigelfenton wants to merge 1 commit intoaethersdr:mainaethersdr/AetherSDR:mainfrom nigelfenton:fix/audit-colours-windows-encodingnigelfenton/AetherSDR:fix/audit-colours-windows-encodingCopy head branch name to clipboard
Conversation
The colour audit crashed on Windows before printing its summary. Windows
consoles commonly default to cp1252, which cannot encode the non-ASCII
characters in the output, so the first such print raised UnicodeEncodeError:
UnicodeEncodeError: 'charmap' codec can't encode character '→'
File "tools/audit_colours.py", line 271, in main
The CSV was written first, so the data survived — but the run aborted
*before* the "Top 20 most-used colours (good first migration targets)"
summary, which is the part a developer actually acts on. The tool that
drives and verifies the RFC aethersdr#3076 migration was therefore unusable for its
main purpose on Windows.
- Reconfigure stdout/stderr to UTF-8 with errors='replace', guarded by
hasattr() so a redirected or wrapped stream is left alone. A no-op where
the console is already UTF-8 (Linux/macOS); a terminal that genuinely
cannot render a glyph now substitutes it instead of aborting the run.
- Pin the CSV write to UTF-8 explicitly. It previously inherited the
locale encoding, so a source path containing a non-ASCII character
would have failed the write for the same underlying reason.
Verified on Windows (Python 3.14, cp1252 console): the run now completes
with exit 0 and prints the full summary — 883 files, 629 unique colours,
2824 references, 1103 setStyleSheet sites.
Refs aethersdr#3184
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Clean, well-scoped fix — the diagnosis is exactly right (the run died encoding → on a cp1252 console, after the CSV but before the summary), and both edits address it correctly. The hasattr(_stream, 'reconfigure') guard safely no-ops on redirected or None streams, reconfigure has been available on TextIOWrapper since Python 3.7 so there's no version concern, and the explicit encoding='utf-8' on the CSV write closes the same locale-encoding hole for non-ASCII source paths. No behaviour change on Linux/macOS. Nice write-up too. Thanks for the contribution.
Polish (non-blocking)
- The comment says
errors='replace'lets "a terminal that genuinely cannot render a glyph substitute it instead." Minor: withencoding='utf-8'the replace applies to the UTF-8 encode step (which effectively never fails for validstr), not to terminal rendering — on a true legacy cp1252 console the arrow will render as mojibake rather than?, though the run does complete either way. The code is fine; only the comment slightly overstates whatreplacebuys you here.
Non-blocking notes
- The CodeGuard
CG-PATH-001findings all point atsrc/gui/MainWindow.cpp, which this PR doesn't touch — not applicable to this diff.
🤖 aethersdr-agent · cost: $4.0021 · model: claude-opus-4-8
|
Cisco CodeGuard — static analysis of this PR (5 finding(s))
Automated static scan by Cisco DefenseClaw CodeGuard on the changed files. Advisory — some may be false positives; the review above verifies them. 🤖 aethersdr-agent · cost: $4.5277 · model: claude-opus-4-8 |
|
Antares Detector — candidate vulnerable file(s)
Localized by Cisco Foundation AI Antares-1B running locally in the AetherClaude sandbox, seeded by the Cartographer security map. Advisory only — please verify before acting. 🤖 aethersdr-agent · cost: $4.5277 · model: claude-opus-4-8 |
There was a problem hiding this comment.
Clean, well-scoped fix for a real annoyance — the diagnosis (CSV written, summary swallowed by UnicodeEncodeError before it prints) is exactly right, the two changes are minimal, and the hasattr(reconfigure) guard keeps it safe when stdout is a non-TextIOWrapper (captured/piped). No behaviour change on Linux/macOS. Thanks for the tidy write-up and the honest "happy to fold into a wider pass" offer.
Nothing blocking. One non-blocking nuance about the stdout reconfigure below.
Non-blocking notes
- The stdout/stderr
reconfigure(encoding='utf-8', errors='replace')makeserrors='replace'effectively inert on that stream (UTF-8 encodes→), so a genuine legacy cp1252 console shows mojibake rather than a substituted glyph — the run still completes (which is the title's promise), just noting the comment slightly oversells the "substitutes it" behaviour. Inline comment sketches the alternative.
Not applicable / FYI
- The CodeGuard
CG-PATH-001path-traversal findings are all anchored tosrc/gui/MainWindow.cpp, which this PR doesn't touch — false positives for this diff, dropping them.
🤖 aethersdr-agent · cost: $3.8360 · model: claude-opus-4-8
| # useful (if occasionally substituted) summary everywhere instead of aborting. | ||
| for _stream in (sys.stdout, sys.stderr): | ||
| if hasattr(_stream, 'reconfigure'): | ||
| _stream.reconfigure(encoding='utf-8', errors='replace') |
There was a problem hiding this comment.
Minor, non-blocking: forcing encoding='utf-8' here means errors='replace' never actually fires on stdout/stderr (UTF-8 can encode every character, including →). So on a true cp1252 console this emits raw UTF-8 bytes and the arrow renders as mojibake — the run completes (the fix works for the title's goal), but it doesn't "substitute" anything the way the comment describes.
If clean substitution on legacy consoles is what you want, dropping the forced encoding and keeping just errors='replace' preserves the console's own code page and turns → into a clean ?:
| _stream.reconfigure(encoding='utf-8', errors='replace') | |
| _stream.reconfigure(errors='replace') |
Trade-off: forcing UTF-8 is friendlier when output is redirected to a file/pipe; keeping the native encoding is friendlier for an interactive legacy console. Either satisfies "runs to completion," so this is purely a preference call — feel free to leave as-is.
|
Cisco CodeGuard — static analysis of this PR (5 finding(s))
Automated static scan by Cisco DefenseClaw CodeGuard on the changed files. Advisory — some may be false positives; the review above verifies them. 🤖 aethersdr-agent · cost: $3.9515 · model: claude-opus-4-8 |
tools/audit_colours.pyaborts partway through on a stock Windows console.Windows consoles commonly default to a legacy code page (cp1252), which cannot
encode the
→in the migration-target summary. The run dies after the CSV iswritten but before the summary prints — so it looks like the tool half-worked,
and the most useful part of the output is exactly the part you never see.
Before (pre-fix script, cp1252 console):
exit 1
After, same console:
exit 0
The change
Two lines of intent, in one file:
stdout/stderras UTF-8 witherrors='replace', so a terminal thatgenuinely cannot render a glyph substitutes it instead of killing the run.
encoding='utf-8'. The default islocale-dependent, so a source path containing a non-ASCII character would fail
the write for the same underlying reason.
No behaviour change on Linux/macOS, where stdout is already UTF-8.
Context
Found while running the tool for the theming audit discussed in #3184 — it is
currently unrunnable-to-completion on Windows, which makes the colour inventory
hard to reproduce on that platform. Small and self-contained on purpose; happy to
close this if you would rather fold it into a wider theming pass.