fix: respect TERM=dumb and NO_COLOR for ANSI escape codes#412
Conversation
PrettyPrintErrorMessage unconditionally emitted ANSI escape codes, which broke output in terminals that don't support them (TERM=dumb) and tools that pipe the output. Skip ANSI formatting when TERM is set to "dumb" or when NO_COLOR is set, following the NO_COLOR convention (https://no-color.org/). Fixes uber-go#105
|
This looks like a good issue for a first-time contributor. I'd be happy to work on respecting TERM=dumb and NO_COLOR environment variables. Could you assign this to me? |
|
Hey @vincent067 — this is actually the PR that addresses #105 (TERM=dumb / NO_COLOR support). It's been sitting waiting for maintainer review since March. If you're looking for a first-time contribution on nilaway, the issue tracker has a few open ones you could pick up. Review/testing on this PR would also be welcome if you want to dig in here. |
|
Hey, thanks for the contributions! Perhaps a better place to do this is at https://github.com/uber-go/nilaway/blob/main/config/config.go#L167 where we calculate the default value (currently it's always |
Move the env check from PrettyPrintErrorMessage into the PrettyPrint flag default in config/config.go per @yuxincs review. The -pretty-print flag continues to override the default via the existing override path in run. Closes uber-go#105
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #412 +/- ##
==========================================
- Coverage 87.25% 87.21% -0.04%
==========================================
Files 72 72
Lines 8306 8312 +6
==========================================
+ Hits 7247 7249 +2
- Misses 867 869 +2
- Partials 192 194 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks @yuxincs, that is a much better placement. Pushed 234b235:
The override path through |
Summary
Skips ANSI escape codes in
PrettyPrintErrorMessagewhenTERM=dumborNO_COLORis set.Why this matters
PrettyPrintErrorMessageinnilaway.go:61unconditionally wraps error text with ANSI color codes (\x1b[31mfor red,\x1b[95mfor magenta, etc.). In terminals that don't support ANSI (like Emacs shell mode, some CI runners, orTERM=dumb), these appear as raw escape sequences in the output (#105).Changes
nilaway.go: Added an early return inPrettyPrintErrorMessagethat checksos.Getenv("TERM") == "dumb"andos.Getenv("NO_COLOR") != ""before applying ANSI codes. When either is set, returns plain text with "error: " prefix but no escape sequences. Also follows the NO_COLOR convention.Testing
go build ./...passes. The change is a 4-line guard at the top of an existing function. The existing-pretty-print=falseflag is unaffected - it controls whetherPrettyPrintErrorMessageis called at all (nilaway.go:47-48).Fixes #105
This contribution was developed with AI assistance (Claude Code).