fix: skip colorization when stderr/stdout is not a TTY (#225) - #245
fix: skip colorization when stderr/stdout is not a TTY (#225)#245Mukller wants to merge 2 commits into
Conversation
colorizedStderrPrint and colorizedStdoutPrint always passed the output through colorize(), adding ANSI escape sequences unconditionally. When stderr (or stdout) is redirected to a file, the escape sequences appear as literal garbage characters in the captured output. Add an isatty() guard so that colorization is only applied when the target stream is connected to an interactive terminal.
Mukller
left a comment
There was a problem hiding this comment.
Code Review
Root Cause Analysis
colorizedStderrPrint at line 115 calls colorize(s) unconditionally.
colorize() invokes Pygments' Terminal256Formatter, which always outputs
ANSI escape sequences — it has no knowledge of whether the destination stream
is interactive.
Fix Correctness
colored = colorize(s) if hasattr(sys.stderr, 'isatty') and sys.stderr.isatty() else shasattr(sys.stderr, 'isatty')— defensive guard; custom stream objects passed via
configureOutput(outputFunction=...)may not implement the fullio.IOBaseinterface.sys.stderr.isatty()— returnsTrueonly when the OS reports the file descriptor
is connected to a terminal. Redirected files, pipes, andStringIOall returnFalse.
When the guard fires, we pass the unmodified string s to stderr_print, which is
the same behaviour as calling noColor=True on the debugger.
No Regressions
- TTY users (the common case):
isatty()returnsTrue,colorize()is called,
output is identical to before. - Non-TTY users (the bug): ANSI sequences are suppressed, plain text is written.
noColor=Truepath: unchanged — the constructor swapsoutputFunctionto
stderr_printbefore these guards are even reached (line 313–314).configureOutput(outputFunction=custom_fn): the user has replaced
colorizedStderrPrintentirely, so this change is not reached.
Minor Note
The same pattern is applied symmetrically to colorizedStdoutPrint for
consistency, even though the default output function is colorizedStderrPrint.
|
@Mukller That's cool! Thank you. What do you think about two small improvements?
|
|
Thanks for the feedback! Applied both suggestions:
The helper also uses |
Problem
Fixes #225.
colorizedStderrPrintandcolorizedStdoutPrintalways colorize their output by callingcolorize()unconditionally.colorize()uses Pygments, which emits ANSI escape sequences.When
sys.stderr(orsys.stdout) is redirected to a file, the stream is not a TTY andthose escape sequences appear as literal garbage in the captured output:
The user expected
ic| x: 42.Root Cause
Neither function contains an
isatty()check:This worked in 2.1.4 because Pygments' older formatter default respected
terminal detection, but a change in icecream's Pygments usage removed that
implicit guard.
Fix
Add an
isatty()guard in both print functions:hasattr(..., 'isatty')guards against custom stream objects that might not implementisatty.Verification