Skip to content

Disable color output when stdout is not a TTY#176

Open
rgarcia wants to merge 1 commit into
mainfrom
hypeship/disable-color-non-tty
Open

Disable color output when stdout is not a TTY#176
rgarcia wants to merge 1 commit into
mainfrom
hypeship/disable-color-non-tty

Conversation

@rgarcia
Copy link
Copy Markdown
Contributor

@rgarcia rgarcia commented Jun 5, 2026

Summary

When the CLI runs in a non-interactive environment (stdout piped or captured), it emitted raw ANSI escape codes instead of plain text — e.g. ^[[30;42m SUCCESS ^[[0m^[[0m. This is because pterm (v0.12.80) forces color on regardless of the output target and does not auto-detect a TTY. The previous initConfig() made this worse by calling pterm.EnableStyling() unconditionally.

This change gates pterm styling on an interactive-terminal check:

  • Color is enabled only when stdout is a TTY and NO_COLOR is unset.
  • Otherwise pterm styling is disabled, so piped/captured output is plain text.
  • The existing --no-color flag and its handling in PersistentPreRunE are unchanged (it runs after initConfig and can only further-disable).

fang/lipgloss (help text and the error renderer) already strip color correctly on a non-TTY, so no change was needed there. The fix is isolated to pterm in cmd/root.go.

Changes

  • cmd/root.go: add shouldEnableColor(noColorEnv, isTTY) helper; rewrite initConfig() to enable/disable pterm styling based on it (reusing pkg/table.IsStdoutTTY()).
  • cmd/color_test.go: table test for the precedence logic.

Test plan

  • go build ./... and make build succeed
  • go test ./cmd/... passes (incl. new TestShouldEnableColor)
  • go vet ./cmd/ and gofmt clean
  • Piped output verified free of ANSI escapes: confirmed an auth-exempt pterm path (upgrade --dry-run) prints plain INFO:/WARNING: when piped, and that the pre-fix build leaked ^[[…m on the same path
  • Manual check that an interactive terminal still shows color (not exercisable in CI/headless; covered by the isTTY=true unit case)

Note

Low Risk
Startup-only output styling change with unit tests; no auth, data, or API behavior changes.

Overview
Fixes pterm emitting raw ANSI escapes when stdout is piped or captured by no longer calling pterm.EnableStyling() unconditionally at startup.

initConfig() now turns styling on only when stdout is a TTY and NO_COLOR is unset; otherwise it calls pterm.DisableStyling(). A new shouldEnableColor helper encodes that precedence (any non-empty NO_COLOR disables color). TTY detection reuses table.IsStdoutTTY(). Table tests cover the four combinations of TTY and env.

The --no-color flag path in PersistentPreRunE is unchanged and can still disable styling after init.

Reviewed by Cursor Bugbot for commit b86de28. Bugbot is set up for automated code reviews on this repo. Configure here.

pterm v0.12.80 forces color on regardless of the output target, so the
CLI emitted raw ANSI escape codes when stdout was piped or captured
(e.g. in a headless/programmatic environment), garbling the output.

Gate pterm styling in initConfig on an interactive-terminal check and
honor the NO_COLOR convention, so non-TTY output is plain text.
Interactive terminals are unaffected and the --no-color flag still works.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@rgarcia rgarcia marked this pull request as ready for review June 5, 2026 22:39
@firetiger-agent
Copy link
Copy Markdown

Firetiger deploy monitoring skipped

This PR didn't match the auto-monitor filter configured on your GitHub connection:

PRs in the kernel, infra, hypeman, and hypeship repos. kernel is a ~mono repo with many logical services underneath, ensure to focus on the implicated service for the PR

Reason: PR affects CLI color output behavior but doesn't clearly identify which service in the kernel mono repo is implicated; please confirm the affected service and opt in manually if this requires deploy monitoring.

To monitor this PR anyway, reply with @firetiger monitor this.

@rgarcia rgarcia requested a review from hiroTamada June 5, 2026 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant