Add --color=auto|always|never to control ANSI output (#81)#109
Merged
Conversation
colgrep emitted color through two independent channels: the `colored` crate (headers, line numbers) and raw ANSI escapes from `syntect` syntax highlighting. `colored` honors NO_COLOR, but syntect never did, so output always contained escape codes regardless of NO_COLOR/CLICOLOR/TERM — a problem for agents and pipes that consume the output directly. - New `color` module resolves one decision from --color (default auto) and applies it to BOTH channels: it sets the `colored` override and records the result for the syntect path to consult via colorize_enabled(). - `auto` honors CLICOLOR_FORCE, NO_COLOR, CLICOLOR, and stdout TTY, so piped output is now plain by default too. - `never` makes display.rs print plain line-numbered text (no syntect, no \x1b[0m); `.dimmed()`/`.cyan()` go through the colored override. - Global flag, so it works on every subcommand. NO_COLOR and CLICOLOR_FORCE are honored in auto mode; --color always wins when set explicitly. Verified: --color=never and default-auto-when-piped emit 0 ANSI bytes; --color=always emits color even with NO_COLOR set; plain output keeps line numbers and content intact.
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.
Closes #81.
Problem
colgrep emits color through two independent channels:
coloredcrate (.cyan()file headers,.dimmed()line numbers), andsyntectsyntax highlighting indisplay.rs.coloredhonorsNO_COLOR, but syntect doesn't — so output always contained escape sequences regardless ofNO_COLOR/CLICOLOR/CLICOLOR_FORCE/TERM. As @divaltor noted, that pollutes context for agents/plugins that consume the output directly rather than through a terminal.Change
A small
colormodule resolves a single decision from--colorand applies it to both channels:coloredcrate override (so every.cyan()/.dimmed()obeys), andcolorize_enabled().display.rsnow prints plain, line-numbered text (no syntect, no\x1b[0m) when color is off.The flag is global, so it works on every subcommand (
colgrep --color never ...,colgrep search --color never ..., etc.). Env handling inauto:CLICOLOR_FORCE(non-empty, non-0) forces on; elseNO_COLOR(non-empty) disables; elseCLICOLOR=0disables; else colorize only on a TTY. An explicit--color always/neveralways wins.A nice side effect: because
autonow respects the TTY, piped output is plain by default too — which is the behavior @divaltor originally expected fromNO_COLOR.Testing
Unit tests for the env/auto-detection and the global toggle. End-to-end (verbose
-cpath, which is what triggers syntax highlighting), counting actual ESC (0x1b) bytes in captured output:--color neverauto, piped (non-TTY)NO_COLOR=1(auto)--color never--color alwaysCLICOLOR_FORCE=1(auto, piped)NO_COLOR=1 --color alwaysPlain output keeps line numbers and content intact;
cargo fmt/clippy/full colgrep test suite (541 lib + 55 bin) green.Note on the follow-up ask
The issue also floated "might add an option to disable line numbers." I kept this PR scoped to color since that's the titled request and what the maintainer comment committed to; happy to add
--no-line-numbersin a follow-up if wanted.