Honor CLICOLOR_FORCE when stdout isn't a TTY #1008
umekikazuya
started this conversation in
Ideas
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Related issue: #968
Related discussion: #943 - Same underlying need, approached from the flag side.
I have a working patch and would like a read on the approach before proposing it.
Problem
When stdout isn't a terminal, glow replaces the requested style with
nottyand there is no way to override that from the environment.CLICOLOR_FORCE=1has no effect, so any caller that captures glow's output but can render ANSI itself gets plain text instead.Concrete cases where this comes up:
fzf --preview 'glow {}'. The preview pane is a pipe from the child's perspective, so markdown markers show up verbatim.glow README.mdin a workflow can't produce it.Passing
--styleexplicitly works around it today, but that requires knowing about the interaction, and it means choosing a fixed style rather than letting glow decide.Root cause
main.go:The style is swapped out before rendering, so nothing downstream can bring color back. None of
NO_COLOR,CLICOLORorCLICOLOR_FORCEare read anywhere in the codebase.Proposed change
Use
charmbracelet/colorprofilefor the decision instead of a bare isatty check. It's already in glow's dependency graph (currently indirect), and it's the org's own implementation of these conventions — so this follows the existing Charm mechanism rather than adding a parallel one.and write the rendered output through a
colorprofile.Writer, so downsampling is the profile's job rather than hand-rolled logic:glamour v2 emits ANSI unconditionally — it has no
colorprofilereferences — so applying the profile is the caller's responsibility. Right now glow doesn't, which is also whyglow -s dark file.mdsends 256-color sequences to a 16-color terminal.What doesn't change
This is the part I'd most like checked:
profileisNoTTY, thenottystyle is still selected, and the bytes are identical to today.isTerminalwas doing two unrelated jobs. Only style selection changes;term.GetSizestill usesisTerminal.glow -s dark file.md | cat. The writer is deliberately skipped when there's no color support, so this existing escape hatch keeps working. Applying the writer there would have stripped the colors and changed current behavior.--stylehandling. Thecmd.Flags().Changed("style")check is untouched.Known gap
That last compatibility choice has a consequence worth naming:
NO_COLOR=1 glow -s dark file.md | catstill emits color.colorprofilemaps both "NO_COLOR is set" and "output is a plain pipe" toNoTTY, so the profile alone can't distinguish them, and the guard that protects the escape hatch also skips stripping here.To be clear this is pre-existing rather than introduced here — I verified that case produces identical output on
masterand on the branch.Closing it would mean reading
NO_COLORdirectly, and deciding whether it should override an explicit--style. I left it out to keep the change additive, but happy to include it if you'd rather have fullNO_COLORcompliance in one go.Tests
I've written tests that render through the real CLI path and assert the actual bytes:
NoTTYkeeps# Hiand**bold**verbatim with no escape sequences,ANSIoutput has no\x1b[38;5;sequences left,ANSI256keeps them.Questions
--color=always|auto|neverflag (option A in Feature request: support --color=always / CLICOLOR_FORCE for non-TTY rendering (e.g. embedded in AI coding tools) #968) be wanted as a follow-up? It'd be a thin layer over the detected profile.colorprofilealso already honorsTTY_FORCE=1, which covers the--force-ttyidea from that issue without a new flag.style:value is also discarded when piping, because the check iscmd.Flags().Changed("style")and doesn't see config values. Is that intentional, or worth fixing separately?The patch is on a branch if you want to look at the actual change before deciding — compare view. Not opening it as a PR yet, per
CONTRIBUTING.md. Happy to do that once there's a direction.All reactions