Write redirected and piped output as UTF-8, and stop the suite inheriting colour settings - #1753
Open
tleonhardt wants to merge 4 commits into
Open
Write redirected and piped output as UTF-8, and stop the suite inheriting colour settings#1753tleonhardt wants to merge 4 commits into
tleonhardt wants to merge 4 commits into
Conversation
Rich and cmd2 consult several environment variables when deciding whether to emit styling, and the suite inherited them. Exporting any one of them made large numbers of unrelated tests fail depending on who ran the suite: NO_COLOR failed 15 tests, and FORCE_COLOR and TTY_COMPATIBLE 53 each. The failures look like product regressions, which makes them expensive to diagnose. Neutralize them for every test. Tests that exercise these variables set them explicitly, which still works because a test's own monkeypatching runs after the fixture. A guard test fails if any of them reaches a test again. (cherry picked from commit 2f786cb)
Command output is rendered by Rich and routinely contains non-ASCII, but redirection targets and pipes were opened with the locale's encoding. On any system whose default is not UTF-8 -- a Windows console using a legacy code page, for instance -- redirecting output raised UnicodeEncodeError, and the user was left with an empty file and advice to set PYTHONIOENCODING. Open both with UTF-8 explicitly. Two tests that read redirected output back were relying on the locale encoding for decoding as well, so they now name it too. (cherry picked from commit b490a36)
The test piped through `cat`, which cmd.exe does not provide. On a Windows system without Unix utilities installed it would fail before reaching the encoding behavior it exists to check -- and Windows is exactly what the UTF-8 redirection fix targets. Use a sys.executable pass-through instead, matching the pipe tests already in tests/test_command_toolbar.py. Reverting either the pipe or the redirect encoding still fails these tests. (cherry picked from commit 7b70fbc)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1753 +/- ##
==========================================
- Coverage 99.64% 99.63% -0.02%
==========================================
Files 23 23
Lines 5973 5973
==========================================
- Hits 5952 5951 -1
- Misses 21 22 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Member
Author
|
@kmvanbrunt @bambu This is a bug fix to merge to |
Measured which Windows ANSI code pages can represent the box-drawing characters Rich emits: every cp125x code page fails, covering US and Western European, Central European, Cyrillic, Greek, Turkish, Hebrew, Arabic, Baltic and Vietnamese systems. Only the CJK double-byte code pages survive. Calling it a legacy code page was wrong: cp1252 is the default on a current, fully updated Windows 11, and Python only defaults to UTF-8 mode in 3.15.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent fixes, both already reviewed and merged to
consolidated_toolbarin #1752.Ported here because
mainis the next patch release and both problems are present on ittoday. The third commit from #1752 — a
CommandToolbarrace — is not included, becausethat module does not exist on
main.1. Redirection and piping fail on current Windows (user-visible data loss)
Command output is rendered by Rich and contains box-drawing characters, but redirection
targets and pipes are opened with the locale's encoding. On Windows that is the ANSI code
page, and no
cp125xcode page can represent them, so redirecting raisesUnicodeEncodeErrorand the user is left with an empty file plus advice to setPYTHONIOENCODING.Measured on
mainatca0ddbe7, runninghelp > out.txtunder a non-UTF-8 default encoding:After this change the same probe writes 153 bytes containing the help text.
Who this affects
This is not limited to old systems. A fully updated Windows 11 with default settings is
affected. The character that breaks is
U+2500(─), and encodability by code page is:U+2500?Only the CJK double-byte code pages escape, because they happen to include box drawing.
By interpreter,
sys.flags.utf8_modeis0on Python 3.11, 3.12, 3.13 and 3.14, and1on 3.15 — PEP 686 makes UTF-8 mode the default there. So every Python version this project
currently supports below 3.15 is affected.
The other escapes are Windows' "Beta: Use Unicode UTF-8 for worldwide language support"
option, which is off by default, and setting
PYTHONUTF8=1or-X utf8.In short: a user on Windows 11, US locale, Python 3.13, typing
help > out.txtin any cmd2app loses their output. That is the common case, not an edge case.
main's own tests do not catch this — nothing there redirects Rich-heavy output and reads itback, so the suite passes while the product is broken. This PR adds tests that do.
2. The suite inherits the caller's colour environment
Rich and cmd2 consult several environment variables when deciding whether to emit styling,
and the suite inherits them. Exporting any one makes large numbers of unrelated tests fail
depending on who runs the suite — measured on
main:NO_COLOR=1FORCE_COLOR=1TTY_COMPATIBLE=1The failures look like product regressions, which makes them expensive to diagnose; this
cost a reviewer and me a full round trip before the cause was found. An autouse fixture now
neutralizes them, and a guard test fails if any reaches a test again. Tests that exercise
these variables still set them explicitly, because a test's own monkeypatching runs after
the fixture.
Verification
Full suite under every environment that previously broke it:
make check,make testandmake docs-testall pass. Reverting either encoding changefails the new tests.
The pipe test runs its filter through
sys.executablerather thancat, matching theconvention in the existing pipe tests —
cmd.exehas nocat, and this fix exists forWindows. Note that CI would not have caught that: GitHub's Windows runners have Git for
Windows' utilities on
PATH.Note for merging
These commits also exist on
consolidated_toolbarvia #1752, so the same change appears inboth histories. The content is identical, so the eventual merge resolves cleanly.