feat(output): respect NO_COLOR and disable ANSI when piped#34
feat(output): respect NO_COLOR and disable ANSI when piped#34thinkingjet wants to merge 4 commits intocodesoda:mainfrom
Conversation
Implements issue codesoda#15 behavior across main and claude-code output paths. Changes: - Add src/output.rs with color_enabled() and ansi() helpers - Export output module from src/lib.rs - Gate ANSI banner and styled run labels in src/main.rs - Gate verbose ANSI formatting in src/claude_code.rs Validation run: - cargo check -q - cargo test --all-targets - cargo test --doc - cargo run --quiet -- test DOES_NOT_EXIST.test.toml 2>&1 | cat -v - script -q /dev/null cargo run --quiet -- test DOES_NOT_EXIST.test.toml | cat -v - NO_COLOR=1 script -q /dev/null cargo run --quiet -- test DOES_NOT_EXIST.test.toml | cat -v
Use OnceLock-backed Colors palette to avoid repeated NO_COLOR/TTY checks on every color lookup. - Add public Colors fields for shared styling constants - Provide colors() singleton accessor and output prelude exports - Switch main.rs and claude_code.rs call sites to shared palette Validation: - cargo check -q - cargo test -q --all-targets - runtime checks for piped, TTY default, and NO_COLOR=1
|
Follow-up update based on review feedback is now pushed. Summary of what changed: Refactored ANSI/color values into a dedicated output module. Piped output is plain (no ANSI). cargo check -q |
There was a problem hiding this comment.
Pull request overview
Adds a shared output helper to consistently disable ANSI styling when NO_COLOR is set or when output is not going to a terminal, and updates the CLI/Claude Code paths to use it.
Changes:
- Introduces
src/output.rswith a lazily-initialized ANSI palette andcolor_enabled()/ansi()helpers. - Switches
src/main.rsbanner and per-test labels to use the shared palette (no hardcoded escape codes). - Switches
src/claude_code.rsverbose formatting to use the shared palette; exports the new module fromsrc/lib.rs.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/output.rs | Centralizes ANSI enable/disable logic and provides a shared color palette. |
| src/main.rs | Replaces hardcoded ANSI sequences with palette-driven formatting. |
| src/claude_code.rs | Replaces verbose ANSI constants with the shared palette. |
| src/lib.rs | Exposes the new output module for use by binaries/adapters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/output.rs
Outdated
| fn detect_color_enabled() -> bool { | ||
| std::env::var_os("NO_COLOR").is_none() && std::io::stdout().is_terminal() | ||
| } |
There was a problem hiding this comment.
Done:
What changed:
Separate lazy color palettes are now used for stdout and stderr, so each stream checks terminal capability only once.
Verbose stderr output now uses the stderr palette, so ANSI no longer leaks when stderr is redirected but stdout is still a TTY.
NO_COLOR remains respected for both streams.
The refactor still avoids repeated environment reads and keeps the singleton approach.
Validation:
cargo check -q
cargo test -q --all-targets
Runtime smoke checks for piped output, TTY default, NO_COLOR, and redirected stderr
Result: behavior is correct and consistent, with no ANSI escapes in redirected stderr output.
Use separate lazy palettes for stdout and stderr to avoid ANSI leaks when only one stream is redirected. - Add Stream enum and per-stream OnceLock palettes - Add stdout_colors()/stderr_colors() accessors - Keep existing helpers for compatibility - Route claude verbose eprintln! paths to stderr palette Validation: - cargo check -q - cargo test -q --all-targets - smoke checks for piped, TTY, NO_COLOR, and redirected stderr
Implements issue #15 behavior across main and claude-code output paths.
Changes:
Add src/output.rs with color_enabled() and ansi() helpers
Export output module from src/lib.rs
Gate ANSI banner and styled run labels in src/main.rs
Gate verbose ANSI formatting in src/claude_code.rs
Validation run:
cargo check -q
cargo test --all-targets
cargo test --doc
cargo run --quiet -- test DOES_NOT_EXIST.test.toml 2>&1 | cat -v
script -q /dev/null cargo run --quiet -- test DOES_NOT_EXIST.test.toml | cat -v
NO_COLOR=1 script -q /dev/null cargo run --quiet -- test DOES_NOT_EXIST.test.toml | cat -v