From 3199eb55da62cf9b7a44160fc238f9bb9068a3d3 Mon Sep 17 00:00:00 2001 From: Jarrett Lusso Date: Tue, 26 May 2026 11:49:00 -0400 Subject: [PATCH] Drop --no-color flag in favor of NO_COLOR env var MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CLI already auto-detects TTY (suppressing colors when piped) and honors the cross-tool [NO_COLOR](https://no-color.org/) standard. The dedicated `--no-color` flag was redundant — `NO_COLOR=1 emailable …` covers the same use case. Removes the flag, its `ui.SetNoColor` plumbing, related tests, and the dedicated "Color output" README section. `NO_COLOR` stays mentioned in the env vars table. --- README.md | 14 +----------- cmd/context.go | 12 +++++------ cmd/root.go | 16 -------------- cmd/root_test.go | 43 ++++--------------------------------- cmd/testutil_test.go | 3 --- cmd/verify_test.go | 28 ------------------------ internal/output/human.go | 3 +-- internal/ui/spinner.go | 18 ---------------- internal/ui/spinner_test.go | 27 ----------------------- 9 files changed, 11 insertions(+), 153 deletions(-) diff --git a/README.md b/README.md index 963a6a4..9685a92 100644 --- a/README.md +++ b/README.md @@ -309,18 +309,6 @@ emailable account status --debug EMAILABLE_DEBUG=1 emailable verify hello@example.com ``` -### Color output - -The CLI uses ANSI colors on TTY output and suppresses them when stdout is -piped or redirected. To force-disable colors even on a TTY, pass -`--no-color` or set [`NO_COLOR`](https://no-color.org/) to any non-empty -value: - -```bash -emailable account status --no-color -NO_COLOR=1 emailable account status -``` - ### Quiet mode Pass `--quiet` (or `-q`) to suppress non-error human output — success @@ -416,7 +404,7 @@ All of the env vars the CLI honors, in one place: | `EMAILABLE_API_KEY` | API key for non-interactive auth. Takes precedence over a stored API key or OAuth token. | | `EMAILABLE_OUTPUT` | Default output format when `--json` isn't passed. Set to `json` to make every command emit JSON. | | `EMAILABLE_DEBUG` | Any non-empty value dumps HTTP requests/responses to stderr (with `Authorization` redacted). Equivalent to `--debug`. | -| `NO_COLOR` | Standard [no-color.org](https://no-color.org/) convention — any non-empty value suppresses ANSI colors. Equivalent to `--no-color`. | +| `NO_COLOR` | Standard [no-color.org](https://no-color.org/) convention — any non-empty value suppresses ANSI colors. | | `EMAILABLE_NO_UPDATE_NOTIFIER` | Any truthy value (`1`/`true`/`yes`/`on`) disables the daily "new release available" notifier. See [Update notifier](#update-notifier). | | `CI` | When set, the update notifier is silently skipped (common-sense default in CI environments). | | `XDG_CONFIG_HOME` | Where the config file lives. Defaults to `~/.config`; the CLI stores credentials under `$XDG_CONFIG_HOME/emailable/config.yml`. | diff --git a/cmd/context.go b/cmd/context.go index b0f6ae3..ab6aebb 100644 --- a/cmd/context.go +++ b/cmd/context.go @@ -43,8 +43,8 @@ const outputEnv = "EMAILABLE_OUTPUT" // loaded config, persistent flags. Commands grab one via newCmdCtx() in their // RunE. // -// JSONMode / Quiet / NoColor are populated from the persistent flag state at -// the time the cmdCtx is built. Commands should prefer reading these fields +// JSONMode and Quiet are populated from the persistent flag state at the +// time the cmdCtx is built. Commands should prefer reading these fields // over the package-level globals so behavior remains consistent even when a // command-local helper (e.g. applyStreamImplications) overrides the effective // value for its caller. @@ -54,7 +54,6 @@ type cmdCtx struct { Config *config.Config JSONMode bool Quiet bool - NoColor bool // refreshNoticeWriter, when non-nil, receives a short stderr message the // first time requireAuth performs an OAuth refresh during this command's @@ -83,9 +82,9 @@ func newCmdCtxFor(cmd *cobra.Command, jsonMode bool) (*cmdCtx, error) { // path, and loads (or returns empty) the config. Does not enforce that the // user is logged in — that's the per-command's job via requireAuth. // -// Quiet and NoColor are read off the package-level flag globals at call time -// (cobra has already populated them by the time any RunE fires) so callers -// only need to thread the JSON value through. +// Quiet is read off the package-level flag global at call time (cobra has +// already populated it by the time any RunE fires) so callers only need to +// thread the JSON value through. func newCmdCtx(jsonMode bool) (*cmdCtx, error) { e, err := env.Current() if err != nil { @@ -105,7 +104,6 @@ func newCmdCtx(jsonMode bool) (*cmdCtx, error) { Config: cfg, JSONMode: jsonMode, Quiet: quietMode, - NoColor: noColor, }, nil } diff --git a/cmd/root.go b/cmd/root.go index ab92575..95e4cab 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -52,12 +52,6 @@ var debugMode bool // modifier). Mirrors the convention in curl, docker, gh. var quietMode bool -// noColor is the value of the persistent --no-color flag. When true (or when -// NO_COLOR is set non-empty) ANSI escape codes are suppressed even on a real -// TTY. The flag's effect is plumbed into internal/ui via ui.SetNoColor so a -// single decision point feeds both the env-var path and the explicit flag. -var noColor bool - // Command group IDs. Used both by cobra's command grouping and by the custom // help renderer to emit gh-style section headers in a stable order. const ( @@ -210,15 +204,6 @@ func newRootCmd(v string) *cobra.Command { if !cmd.Flags().Changed("json") && strings.EqualFold(os.Getenv(outputEnv), "json") { jsonOutput = true } - // Propagate --no-color into the ui package so every IsTTY check - // in the process honors it. The env var NO_COLOR is consulted - // independently inside ui.IsTTY, so we only force-on when the - // flag was explicitly set — letting the env-var convention - // remain the default mechanism. (Mirrors how --debug interacts - // with EMAILABLE_DEBUG: explicit flag is just an alternative.) - if cmd.Flags().Changed("no-color") { - ui.SetNoColor(noColor) - } return nil }, } @@ -229,7 +214,6 @@ func newRootCmd(v string) *cobra.Command { root.PersistentFlags().BoolVar(&jsonOutput, "json", false, "Return JSON response") root.PersistentFlags().BoolVar(&debugMode, "debug", false, "Dump HTTP requests/responses to stderr (also EMAILABLE_DEBUG)") root.PersistentFlags().BoolVarP(&quietMode, "quiet", "q", false, "Suppress non-error human output (success lines, hints, progress)") - root.PersistentFlags().BoolVar(&noColor, "no-color", false, "Disable ANSI colors (also NO_COLOR)") // Register groups so cobra knows about them; the custom usage func is // what actually renders them under gh-style headings. diff --git a/cmd/root_test.go b/cmd/root_test.go index 3aa4567..4180bfa 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -4,8 +4,6 @@ import ( "bytes" "strings" "testing" - - "github.com/emailable/emailable-cli/internal/ui" ) func TestRootCommand_VersionFlag(t *testing.T) { @@ -47,10 +45,9 @@ func TestRootCommand_HelpFlag(t *testing.T) { } } -// TestRootCommand_HelpListsNewPersistentFlags asserts that --quiet/-q and -// --no-color are both surfaced in --help, both at the root and (since they -// are persistent) on a subcommand's help. -func TestRootCommand_HelpListsNewPersistentFlags(t *testing.T) { +// TestRootCommand_HelpListsPersistentFlags asserts that the persistent +// flags are surfaced in --help, both at the root and on a subcommand. +func TestRootCommand_HelpListsPersistentFlags(t *testing.T) { for _, args := range [][]string{ {"--help"}, {"verify", "--help"}, @@ -64,42 +61,10 @@ func TestRootCommand_HelpListsNewPersistentFlags(t *testing.T) { t.Fatalf("execute %v: %v", args, err) } got := buf.String() - for _, want := range []string{"--quiet", "-q", "--no-color"} { + for _, want := range []string{"--quiet", "-q"} { if !strings.Contains(got, want) { t.Errorf("args=%v: help missing %q\n--- help ---\n%s", args, want, got) } } } } - -// TestNoColorFlag_PropagatesToUI verifies that passing --no-color on the -// root command flips both the package-level noColor flag AND calls -// ui.SetNoColor — which is the wiring the rest of the binary relies on for -// suppressing ANSI escapes everywhere ui.IsTTY is consulted. -func TestNoColorFlag_PropagatesToUI(t *testing.T) { - t.Cleanup(func() { ui.SetNoColor(false) }) - t.Cleanup(func() { noColor = false }) - t.Setenv("NO_COLOR", "") - clearEnvOverrides(t) - - root := newRootCmd("dev") - var buf bytes.Buffer - root.SetOut(&buf) - root.SetErr(&buf) - root.SetArgs([]string{"status", "--no-color"}) - _ = root.Execute() // exit code irrelevant; we only need PersistentPreRunE to fire. - - if !noColor { - t.Errorf("expected --no-color flag to set the package-level noColor var") - } - // Render a styled heading: ui.Heading only emits ANSI when tty=true. - // After --no-color, ui.IsTTY(writer) must return false even if the - // writer "looks" like a TTY — exercise by passing the IsTTY result - // straight into Heading. We use a non-file writer (bytes.Buffer) so - // isTerminal is naturally false; what we're proving here is that the - // pipeline cmd→ui.SetNoColor→ui.IsTTY is wired (no panic / no failure). - got := ui.Heading("USAGE", ui.IsTTY(&buf)) - if got != "USAGE" { - t.Errorf("expected plain heading, got %q", got) - } -} diff --git a/cmd/testutil_test.go b/cmd/testutil_test.go index 2274f48..1303416 100644 --- a/cmd/testutil_test.go +++ b/cmd/testutil_test.go @@ -59,9 +59,6 @@ func newTestEnv(t *testing.T, handler http.Handler) *testEnv { prevQuiet := quietMode quietMode = false t.Cleanup(func() { quietMode = prevQuiet }) - prevNoColor := noColor - noColor = false - t.Cleanup(func() { noColor = prevNoColor }) // env.Current() returns "custom" when EMAILABLE_API_URL is set. path, err := config.DefaultPath("custom") diff --git a/cmd/verify_test.go b/cmd/verify_test.go index 04ec70c..9434dc6 100644 --- a/cmd/verify_test.go +++ b/cmd/verify_test.go @@ -6,8 +6,6 @@ import ( "net/http" "strings" "testing" - - "github.com/emailable/emailable-cli/internal/ui" ) // TestVerify_Help asserts that the slim `verify` help surface carries no @@ -170,32 +168,6 @@ func TestVerify_QuietJSON_StillEmitsJSON(t *testing.T) { } } -// TestStatus_NoColor_StripsANSI verifies that --no-color is plumbed through -// to ui.SetNoColor, so ANSI escape codes are suppressed even on real TTY -// writers. We can't easily fake a *os.File terminal in a unit test, but we -// can prove the upstream wiring: after running with --no-color, ui.IsTTY -// must return false regardless of the input writer. (The independent test -// in internal/ui covers the styling-suppression half of the contract.) -func TestStatus_NoColor_StripsANSI(t *testing.T) { - t.Cleanup(func() { ui.SetNoColor(false) }) - t.Cleanup(func() { noColor = false }) - t.Setenv("NO_COLOR", "") - clearEnvOverrides(t) - - res := runRoot(t, "status", "--no-color") - _ = res.Err // status w/o creds still exits 0 in human mode - - // No ANSI codes should be present even though parts of status human - // output (state, labels) would normally style themselves on a TTY. - combined := res.Stdout.String() + res.Stderr.String() - if strings.Contains(combined, "\x1b[") { - t.Errorf("expected no ANSI escape codes under --no-color, got %q", combined) - } - // And the global noColor flag must have been observed. - if !noColor { - t.Errorf("expected --no-color to flip the package-level noColor var") - } -} func TestBatchVerify_Help(t *testing.T) { root := newRootCmd("dev") var buf bytes.Buffer diff --git a/internal/output/human.go b/internal/output/human.go index 1441f40..147ab21 100644 --- a/internal/output/human.go +++ b/internal/output/human.go @@ -201,8 +201,7 @@ func hyperlink(url, text string, enabled bool) string { } // isTTY reports whether w is a terminal AND ANSI styling is enabled. Delegates -// to ui.IsTTY so the NO_COLOR env var and the --no-color flag (via -// ui.SetNoColor) both suppress styling here too. +// to ui.IsTTY so the NO_COLOR env var suppresses styling here too. func isTTY(w io.Writer) bool { return ui.IsTTY(w) } diff --git a/internal/ui/spinner.go b/internal/ui/spinner.go index 02c3db9..1bca686 100644 --- a/internal/ui/spinner.go +++ b/internal/ui/spinner.go @@ -19,21 +19,6 @@ import ( // non-empty value the CLI suppresses ANSI color/styling even on a real TTY. const noColorEnv = "NO_COLOR" -// noColorForce, when true, forces IsTTY to return false regardless of whether -// the underlying writer is a real terminal. Set by SetNoColor — typically -// from the cmd package's --no-color persistent flag binding. The env var -// NO_COLOR is consulted independently inside IsTTY so either input suppresses -// color. -var noColorForce bool - -// SetNoColor toggles the process-wide "force no color" state. Idempotent and -// safe to call multiple times (e.g. once per root command construction). -// Mirrors the NO_COLOR env var: once on, IsTTY returns false for every writer -// for the remainder of the process. -func SetNoColor(on bool) { - noColorForce = on -} - // SpinnerStyle is the shared lipgloss style applied to the spinner glyph // across every animated component (Spinner, Bar, …) so the indicator // reads as the same icon everywhere. Color 69 is the same purple/blue @@ -56,9 +41,6 @@ const TickInterval = 100 * time.Millisecond // var is set to any non-empty value, returns false even when w is a real // terminal. This lets users (and AI agents) opt out of color globally. func IsTTY(w io.Writer) bool { - if noColorForce { - return false - } if os.Getenv(noColorEnv) != "" { return false } diff --git a/internal/ui/spinner_test.go b/internal/ui/spinner_test.go index e16e61e..6a83c65 100644 --- a/internal/ui/spinner_test.go +++ b/internal/ui/spinner_test.go @@ -65,33 +65,6 @@ func TestIsTTY_NoColor(t *testing.T) { } } -// TestSetNoColor verifies the --no-color "force off" path: with isTerminal -// stubbed to true, NO_COLOR unset, and noColorForce=true, IsTTY must report -// false. Toggling back to false restores the prior behavior. -func TestSetNoColor(t *testing.T) { - orig := isTerminal - t.Cleanup(func() { isTerminal = orig }) - isTerminal = func(io.Writer) bool { return true } - t.Setenv("NO_COLOR", "") - - t.Cleanup(func() { SetNoColor(false) }) - - var buf bytes.Buffer - if !IsTTY(&buf) { - t.Fatalf("baseline: IsTTY should be true with isTerminal=true, NO_COLOR unset, no force") - } - - SetNoColor(true) - if IsTTY(&buf) { - t.Errorf("SetNoColor(true) should force IsTTY=false") - } - - SetNoColor(false) - if !IsTTY(&buf) { - t.Errorf("SetNoColor(false) should restore IsTTY=true") - } -} - func TestSpinner_SetMessage_NonTTY(t *testing.T) { var buf bytes.Buffer s := NewTo(&buf, "first")