diff --git a/CLAUDE.md b/CLAUDE.md index 8355e70e..63b6b5a4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -74,13 +74,13 @@ Generated files are marked `-diff linguist-generated=true` in `.gitattributes`: The CLI has **zero global command state**: `buildRootCmd(ctx)` builds the entire command tree fresh on each invocation, and no command uses `init()` registration or package-level flag variables. - `buildRootCmd` creates the per-invocation `*common.App` and passes it to a `build*Cmd(app)` builder function for each command; group builders add their own subcommand builders in turn. Any command body can reach the config and API client through the App without loading them itself. -- `wrapCommands` wraps the `RunE` of every command in the tree with the shared per-invocation lifecycle: `app.SetFlags(cmd.Flags())` followed by `app.Load(ctx)` (the single config + API client load), setting `color.NoColor` from `cfg.Color`, starting a background version check (whose output is deferred until after the command's own), and deferred analytics tracking. There are no `PersistentPreRunE`/`PersistentPostRunE` hooks. +- `wrapCommands` wraps the `RunE` of every command in the tree with the shared per-invocation lifecycle: `app.SetFlags(cmd.Flags())` followed by `app.Load(ctx)` (the single config + API client load), disabling color on the output writers when `cfg.Color` is off, starting a background version check (whose output is deferred until after the command's own), and deferred analytics tracking. There are no `PersistentPreRunE`/`PersistentPostRunE` hooks. - Always use `RunE`, never `Run` — a `Run` command would silently skip the lifecycle. Cobra's built-in `help`, `completion`, and `__complete` commands are added after `wrapCommands` runs and are deliberately left unwrapped, so they never touch the config file, the system keyring, or the network. Completion functions that do need the config or client wrap themselves with `withAppLoad` (`completion_helper.go`). - Declare flags as local variables inside the builder function. Flags that override config values are the exception — see [Configuration](#configuration). ### One File Per Command -Every command gets its own file in `internal/cmd/`, named to match the command in snake_case: `tiger service create` → `service_create.go`. Group commands with no `RunE` of their own still get a file (`tiger service` → `service.go`). Within a file, constants and package-level variables come first, then the `build*Cmd()` function, then helpers used only by that command. Tests mirror this layout (`service_create.go` → `service_create_test.go`), with package-wide test scaffolding in `main_test.go` and `integration_test.go` as a single cross-command suite. +Every command gets its own file in `internal/cmd/`, named to match the command in snake_case: `tiger service create` → `service_create.go`. Group commands with no `RunE` of their own still get a file (`tiger service` → `service.go`). Within a file, package-level constants and variables that other code may reference come first, then the `build*Cmd()` function, then helpers used only by that command. Constants and variables used by just one helper sit directly above it (`colorizeLogEntry`'s styles in `service_logs.go`). Tests mirror this layout (`service_create.go` → `service_create_test.go`), with package-wide test scaffolding in `main_test.go` and `integration_test.go` as a single cross-command suite. ### Where Helpers Go @@ -146,7 +146,7 @@ A command that identifies a service takes a **ref** — its ID, a read replica s ### Streams -`buildRootCmd` calls `cmd.SetOut(os.Stdout)` and `cmd.SetErr(os.Stderr)`. Both are **required**: cobra's `cmd.Print*` helpers write to `OutOrStderr()`, which falls back to stderr when no out writer is set, so without them every `cmd.Printf` in the CLI would silently land on the wrong stream. +`buildRootCmd` wraps `os.Stdout` and `os.Stderr` in `util.TermWriter`s and sets them with `cmd.SetOut` and `cmd.SetErr`. Both are **required**: cobra's `cmd.Print*` helpers write to `OutOrStderr()`, which falls back to stderr when no out writer is set, so without them every `cmd.Printf` in the CLI would silently land on the wrong stream. A `TermWriter` routes writes through a colorprofile writer (which downsamples or strips ANSI sequences to suit the terminal) while still exposing the underlying file, so `util.IsTerminal` and BubbleTea see a real terminal. When handing a stream to a child process (`exec.Cmd`), unwrap it with `util.TryUnwrapFile` so the child inherits the file descriptor and its TTY rather than a pipe (`db psql` is the model). - **stdout** gets the command's primary output: the data payload (table, JSON, YAML, env vars), and in the plain-text path the result text itself. - **stderr** gets everything else: errors and warnings; interactive UI (confirmation prompts, password prompts, spinners); and progress/status messages that accompany structured output, so a piped stdout stays clean. `tiger service create` is the model — every status line is `cmd.PrintErrf`, and only the final service payload goes to stdout. @@ -157,7 +157,7 @@ Helper functions inside `internal/cmd` take the `*cobra.Command` and print throu - Commands with structured output take `-o`/`--output` with `json`, `yaml`, and `table` (the default); some commands support extras (`env`, `bare`). Register the flag with the validating flag types in `flag_helper.go` (`new(outputFlag)` and friends, with no bound variable) and read the value from `cfg.Output` — `output` is a config value. - Serialize with `util.SerializeToJSON` and `util.SerializeToYAML`. **Don't add `yaml:` struct tags to output types** — `SerializeToYAML` encodes to JSON first and converts, so only `json:` tags matter and the two formats stay consistent (including for generated types that only carry `json:` tags). -- Colored output uses `fatih/color`; the lifecycle sets `color.NoColor` from `cfg.Color`, so commands need no per-command wiring. +- Colored output uses lipgloss styles, restricted to the base ANSI colors (`lipgloss.Red`, `lipgloss.Cyan`, …) rather than 256-color or true-color values, which can clash with some terminal themes. Render styles unconditionally: the output writers strip colors when the stream isn't a terminal, and the lifecycle disables color on them when `cfg.Color` is off, so commands need no per-command wiring. ### Tone diff --git a/go.mod b/go.mod index 67aa2ccc..44e1363d 100644 --- a/go.mod +++ b/go.mod @@ -12,11 +12,11 @@ tool ( require ( charm.land/bubbletea/v2 v2.0.9 + charm.land/lipgloss/v2 v2.0.6 github.com/Masterminds/semver/v3 v3.5.0 github.com/charmbracelet/colorprofile v0.4.3 github.com/cli/safeexec v1.0.1 github.com/docker/go-units v0.5.0 - github.com/fatih/color v1.19.0 github.com/google/go-cmp v0.7.0 github.com/google/jsonschema-go v0.4.3 github.com/jackc/pgx/v5 v5.10.0 @@ -50,6 +50,7 @@ require ( github.com/cpuguy83/go-md2man/v2 v2.0.6 // indirect github.com/danieljoos/wincred v1.2.3 // indirect github.com/dprotaso/go-yit v0.0.0-20220510233725-9ba8df137936 // indirect + github.com/fatih/color v1.19.0 // indirect github.com/fsnotify/fsnotify v1.10.1 // indirect github.com/getkin/kin-openapi v0.149.0 // indirect github.com/go-openapi/jsonpointer v1.0.1 // indirect diff --git a/go.sum b/go.sum index c7893f98..e58d51db 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,7 @@ charm.land/bubbletea/v2 v2.0.9 h1:DpJCMWKgzQK8SJv4zbKKFHAI10ymWy/evClPFk0k0f8= charm.land/bubbletea/v2 v2.0.9/go.mod h1:2SkdgoTXluXJHOUwAoRlRXF/28vklb1rFl6GcgV1/ss= +charm.land/lipgloss/v2 v2.0.6 h1:EaGKeuA8FvF+v2BT5VmZd2LoYLaMZJXA5n34th8nCIQ= +charm.land/lipgloss/v2 v2.0.6/go.mod h1:ipDDJNSGa1hlwDtSfW1s2/xR8Vdhbut4PXh2zEKZd0Q= github.com/BurntSushi/toml v1.6.0 h1:dRaEfpa2VI55EwlIW72hMRHdWouJeRF7TPYhI+AUQjk= github.com/BurntSushi/toml v1.6.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2lLoLwho= github.com/Masterminds/semver/v3 v3.5.0 h1:kQceYJfbupGfZOKZQg0kou0DgAKhzDg2NZPAwZ/2OOE= @@ -7,8 +9,8 @@ github.com/Masterminds/semver/v3 v3.5.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lpr github.com/RaveNoX/go-jsoncommentstrip v1.0.0/go.mod h1:78ihd09MekBnJnxpICcwzCMzGrKSKYe4AqU6PDYYpjk= github.com/apapsch/go-jsonmerge/v2 v2.0.0 h1:axGnT1gRIfimI7gJifB699GoE/oq+F2MU7Dml6nw9rQ= github.com/apapsch/go-jsonmerge/v2 v2.0.0/go.mod h1:lvDnEdqiQrp0O42VQGgmlKpxL1AP2+08jFMw88y4klk= -github.com/aymanbagabas/go-udiff v0.2.0 h1:TK0fH4MteXUDspT88n8CKzvK0X9O2xu9yQjWpi6yML8= -github.com/aymanbagabas/go-udiff v0.2.0/go.mod h1:RE4Ex0qsGkTAJoQdQQCA0uG+nAzJO/pI/QwceO5fgrA= +github.com/aymanbagabas/go-udiff v0.4.1 h1:OEIrQ8maEeDBXQDoGCbbTTXYJMYRCRO1fnodZ12Gv5o= +github.com/aymanbagabas/go-udiff v0.4.1/go.mod h1:0L9PGwj20lrtmEMeyw4WKJ/TMyDtvAoK9bf2u/mNo3w= github.com/bmatcuk/doublestar v1.1.1/go.mod h1:UD6OnuiIn0yFxxA2le/rnRU1G4RaI4UvFv1sNto9p6w= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= @@ -18,8 +20,8 @@ github.com/charmbracelet/ultraviolet v0.0.0-20260903151058-ae99b731b8c5 h1:omidy github.com/charmbracelet/ultraviolet v0.0.0-20260903151058-ae99b731b8c5/go.mod h1:nAw0d9PhFp1qdzi2xhQU5YOu5sVpDIHWlaW2Uz/bCro= github.com/charmbracelet/x/ansi v0.11.8 h1:JMFwp0CgDC2+jcOB162HH5k7I3FVbgFSMMYg7dSPBQQ= github.com/charmbracelet/x/ansi v0.11.8/go.mod h1:ZNN+3mXny/516oTQPLMPIBeSINvNJJQ8uQXDgbeJxY0= -github.com/charmbracelet/x/exp/golden v0.0.0-20241212170349-ad4b7ae0f25f h1:UytXHv0UxnsDFmL/7Z9Q5SBYPwSuRLXHbwx+6LycZ2w= -github.com/charmbracelet/x/exp/golden v0.0.0-20241212170349-ad4b7ae0f25f/go.mod h1:wDlXFlCrmJ8J+swcL/MnGUuYnqgQdW9rhSD61oNMb6U= +github.com/charmbracelet/x/exp/golden v0.0.0-20250806222409-83e3a29d542f h1:pk6gmGpCE7F3FcjaOEKYriCvpmIN4+6OS/RD0vm4uIA= +github.com/charmbracelet/x/exp/golden v0.0.0-20250806222409-83e3a29d542f/go.mod h1:IfZAMTHB6XkZSeXUqriemErjAWCCzT0LwjKFYCZyw0I= github.com/charmbracelet/x/term v0.2.2 h1:xVRT/S2ZcKdhhOuSP4t5cLi5o+JxklsoEObBSgfgZRk= github.com/charmbracelet/x/term v0.2.2/go.mod h1:kF8CY5RddLWrsgVwpw4kAa6TESp6EB5y3uxGLeCqzAI= github.com/charmbracelet/x/termios v0.1.1 h1:o3Q2bT8eqzGnGPOYheoYS8eEleT5ZVNYNy8JawjaNZY= diff --git a/internal/cmd/db_psql.go b/internal/cmd/db_psql.go index 62b2fe4c..34ca6967 100644 --- a/internal/cmd/db_psql.go +++ b/internal/cmd/db_psql.go @@ -656,8 +656,8 @@ func buildPsqlCommand(cfg *config.Config, details *common.ConnectionDetails, psq // Use cmd's input/output streams for testability while maintaining CLI behavior psqlCmd.Stdin = cmd.InOrStdin() - psqlCmd.Stdout = cmd.OutOrStdout() - psqlCmd.Stderr = cmd.ErrOrStderr() + psqlCmd.Stdout = util.TryUnwrapFile(cmd.OutOrStdout()) + psqlCmd.Stderr = util.TryUnwrapFile(cmd.ErrOrStderr()) // Use provided password directly if available if password != "" { diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 9e29b839..c4890b8d 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -8,7 +8,6 @@ import ( "strings" "time" - "github.com/fatih/color" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -43,6 +42,9 @@ func buildRootCmd(ctx context.Context) (*cobra.Command, *common.App, error) { Experimental: experimental, } + stdoutWriter := util.NewTermWriter(os.Stdout) + stderrWriter := util.NewTermWriter(os.Stderr) + cmd := &cobra.Command{ Use: "tiger", Short: "Tiger CLI - Tiger Cloud Platform command-line interface", @@ -63,8 +65,12 @@ To get started, run: // Wire up the output streams explicitly. Cobra's cmd.Print* helpers write to // OutOrStderr(), which falls back to stderr when no out writer is set, so // without this every cmd.Printf would land on stderr. - cmd.SetOut(os.Stdout) - cmd.SetErr(os.Stderr) + // + // Both are wrapped in colorprofile writers that automatically downsample or + // strip ANSI color sequences based on terminal capabilities, giving us a + // single place to control color output. + cmd.SetOut(stdoutWriter) + cmd.SetErr(stderrWriter) // Complete nothing where no completion is registered, rather than falling // back to filenames. Cobra checks this on the command and its parents, so @@ -109,7 +115,7 @@ To get started, run: cmd.AddCommand(buildMCPCmd(app)) cmd.AddCommand(buildFeedbackCmd(app)) - wrapCommands(cmd, app) + wrapCommands(cmd, app, stdoutWriter, stderrWriter) return cmd, app, nil } @@ -125,7 +131,7 @@ To get started, run: // keyring, and the network. Completion functions that do need the config or // client load on demand via withAppLoad. Group commands (`tiger service`) have no // RunE of their own and only print help, so they're skipped as well. -func wrapCommands(cmd *cobra.Command, app *common.App) { +func wrapCommands(cmd *cobra.Command, app *common.App, stdoutWriter, stderrWriter *util.TermWriter) { // Wrap this command's RunE if it exists if cmd.RunE != nil { originalRunE := cmd.RunE @@ -143,7 +149,8 @@ func wrapCommands(cmd *cobra.Command, app *common.App) { } if !cfg.Color { - color.NoColor = true + stdoutWriter.DisableColor() + stderrWriter.DisableColor() } // Check for a newer release in the background, printing the result @@ -173,7 +180,7 @@ func wrapCommands(cmd *cobra.Command, app *common.App) { // Recursively wrap all children for _, child := range cmd.Commands() { - wrapCommands(child, app) + wrapCommands(child, app, stdoutWriter, stderrWriter) } } @@ -230,7 +237,7 @@ func versionCheck(cmd *cobra.Command, cfg *config.Config) func() { return } - version.PrintUpdateWarning(res.result, cfg, cmd.ErrOrStderr()) + version.PrintUpdateWarning(res.result, cmd.ErrOrStderr()) } } diff --git a/internal/cmd/service_logs.go b/internal/cmd/service_logs.go index c42b4bdf..4eef7ff8 100644 --- a/internal/cmd/service_logs.go +++ b/internal/cmd/service_logs.go @@ -4,7 +4,7 @@ import ( "strings" "time" - "github.com/fatih/color" + lipgloss "charm.land/lipgloss/v2" "github.com/spf13/cobra" "github.com/timescale/tiger-cli/internal/common" @@ -106,22 +106,13 @@ service from your configuration.`, case "yaml": return util.SerializeToYAML(outputWriter, logs) default: // text format (default) - // Apply colorization if color is enabled and output is a terminal - shouldColorize := cfg.Color && util.IsTerminal(outputWriter) - if shouldColorize { - // Temporarily enable color for this output - original := color.NoColor - defer func() { color.NoColor = original }() - color.NoColor = false - } - for _, entry := range logs { line := entry.Message if !entry.Timestamp.IsZero() { // Local timezone for terminal output; MCP and public API use UTC. line = entry.Timestamp.Local().Format("2006-01-02 15:04:05 MST") + " " + line } - cmd.Println(colorizeLogEntry(line, entry.Severity, shouldColorize)) + cmd.Println(colorizeLogEntry(line, entry.Severity)) } } @@ -140,31 +131,35 @@ service from your configuration.`, return cmd } +// Log severity styles +var ( + logErrorStyle = lipgloss.NewStyle().Foreground(lipgloss.Red) + logWarningStyle = lipgloss.NewStyle().Foreground(lipgloss.Yellow) + logInfoStyle = lipgloss.NewStyle().Foreground(lipgloss.Blue) + logDebugStyle = lipgloss.NewStyle().Foreground(lipgloss.Magenta) +) + // colorizeLogEntry colorizes the severity token (e.g. "ERROR:") within the log // line using the API-provided severity field. Using the structured field avoids // false positives where a severity word appears in the message body rather than -// as an actual log level. If colorEnabled is false, returns the line unchanged. +// as an actual log level. // // PostgreSQL severity levels: https://www.postgresql.org/docs/current/runtime-config-logging.html#RUNTIME-CONFIG-SEVERITY-LEVELS -func colorizeLogEntry(line, severity string, colorEnabled bool) string { - if !colorEnabled || severity == "" { - return line - } - - var colorFn func(string, ...any) string +func colorizeLogEntry(line, severity string) string { + var style lipgloss.Style switch strings.ToUpper(severity) { case "ERROR", "FATAL", "PANIC": - colorFn = color.RedString + style = logErrorStyle case "WARNING": - colorFn = color.YellowString + style = logWarningStyle case "LOG", "INFO", "NOTICE": - colorFn = color.BlueString + style = logInfoStyle case "DEBUG": - colorFn = color.MagentaString + style = logDebugStyle default: return line } token := strings.ToUpper(severity) + ":" - return strings.Replace(line, token, colorFn(token), 1) + return strings.Replace(line, token, style.Render(token), 1) } diff --git a/internal/cmd/version.go b/internal/cmd/version.go index ea45ef6c..61b197f9 100644 --- a/internal/cmd/version.go +++ b/internal/cmd/version.go @@ -55,7 +55,7 @@ func buildVersionCmd(app *common.App) *cobra.Command { versionOutput.UpdateAvailable = &result.UpdateAvailable updateAvailable = result.UpdateAvailable // Print warning _after_ other output - defer version.PrintUpdateWarning(result, cfg, cmd.ErrOrStderr()) + defer version.PrintUpdateWarning(result, cmd.ErrOrStderr()) } } diff --git a/internal/util/environment.go b/internal/util/environment.go index f0a97a7f..eba4cabd 100644 --- a/internal/util/environment.go +++ b/internal/util/environment.go @@ -2,20 +2,8 @@ package util import ( "os" - - "golang.org/x/term" ) -// IsTerminal is a helper for detecting whether an [io.Writer] or [io.Reader] -// is an interactive terminal / TTY. It is a variable so that tests can -// override it. -var IsTerminal = func(v any) bool { - if f, ok := v.(*os.File); ok { - return term.IsTerminal(int(f.Fd())) - } - return false -} - // IsCI determines if the current execution context is within a known CI/CD system. // This is based on https://github.com/watson/ci-info/blob/HEAD/index.js. func IsCI() bool { diff --git a/internal/util/term.go b/internal/util/term.go new file mode 100644 index 00000000..5f241b0b --- /dev/null +++ b/internal/util/term.go @@ -0,0 +1,64 @@ +package util + +import ( + "io" + "os" + + "github.com/charmbracelet/colorprofile" + "golang.org/x/term" +) + +// fileDescriptor is implemented by *os.File and by wrappers around one (*TermWriter). +type fileDescriptor interface { + Fd() uintptr +} + +// IsTerminal is a helper for detecting whether an [io.Writer] or [io.Reader] +// is an interactive terminal / TTY. It is a variable so that tests can +// override it. +var IsTerminal = func(v any) bool { + if f, ok := v.(fileDescriptor); ok { + return term.IsTerminal(int(f.Fd())) + } + return false +} + +// TermWriter embeds *os.File so that Fd(), Read(), and Close() are promoted +// automatically (satisfying bubbletea's term.File interface for terminal +// detection), while overriding Write() to route through a colorprofile.Writer +// for automatic color downsampling. +type TermWriter struct { + *os.File + colorProfile *colorprofile.Writer +} + +// NewTermWriter creates a TermWriter that writes through a colorprofile.Writer +// while exposing the underlying file's terminal capabilities. +func NewTermWriter(f *os.File) *TermWriter { + return &TermWriter{ + File: f, + colorProfile: colorprofile.NewWriter(f, os.Environ()), + } +} + +func (tw *TermWriter) Write(p []byte) (int, error) { + return tw.colorProfile.Write(p) +} + +// DisableColor makes the writer strip all ANSI color sequences. +func (tw *TermWriter) DisableColor() { + tw.colorProfile.Profile = colorprofile.Ascii +} + +// TryUnwrapFile attempts to extract the underlying *os.File from a writer. +// If the writer is a *TermWriter, the embedded *os.File is returned; otherwise +// the original writer is returned unchanged. This is needed when passing +// writers to exec.Cmd, since os/exec only passes file descriptors directly to +// child processes when the writer is an *os.File. Any other io.Writer causes +// os/exec to create a pipe, which breaks TTY detection in the child process. +func TryUnwrapFile(w io.Writer) io.Writer { + if tw, ok := w.(*TermWriter); ok { + return tw.File + } + return w +} diff --git a/internal/version/check.go b/internal/version/check.go index 066d199e..173ecfe8 100644 --- a/internal/version/check.go +++ b/internal/version/check.go @@ -12,12 +12,12 @@ import ( "strings" "time" + lipgloss "charm.land/lipgloss/v2" "github.com/Masterminds/semver/v3" "github.com/cli/safeexec" - "github.com/fatih/color" + "github.com/timescale/tiger-cli/internal/api" "github.com/timescale/tiger-cli/internal/config" - "github.com/timescale/tiger-cli/internal/util" ) // InstallMethod represents how Tiger CLI was installed @@ -226,8 +226,9 @@ func CheckForUpdate(ctx context.Context, cfg *config.Config) (*CheckResult, erro } // PrintUpdateWarning writes a warning to output if an update is available. -// Callers pass the command's stderr writer. -func PrintUpdateWarning(result *CheckResult, cfg *config.Config, output io.Writer) { +// Callers pass the command's stderr writer, whose colorprofile writer strips +// the colors when they're disabled or unsupported. +func PrintUpdateWarning(result *CheckResult, output io.Writer) { if result == nil || output == nil { return } @@ -235,16 +236,12 @@ func PrintUpdateWarning(result *CheckResult, cfg *config.Config, output io.Write return } - // need to set color.NoColor correctly for the `output` (stderr) - if cfg.Color && util.IsTerminal(output) { - original := color.NoColor - defer func() { color.NoColor = original }() - color.NoColor = false - } + yellow := lipgloss.NewStyle().Foreground(lipgloss.Yellow) + cyan := lipgloss.NewStyle().Foreground(lipgloss.Cyan) fmt.Fprintf(output, "\n\n%s %s → %s\nTo upgrade: %s\n", - color.YellowString("A new version of Tiger CLI is available:"), - color.CyanString(result.CurrentVersion), - color.CyanString(result.LatestVersion), + yellow.Render("A new version of Tiger CLI is available:"), + cyan.Render(result.CurrentVersion), + cyan.Render(result.LatestVersion), result.UpdateCommand, ) }