diff --git a/CHANGELOG.md b/CHANGELOG.md index e24b907c..a8cb7694 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [Unreleased] + +### ⚠ BREAKING CHANGES + +* When stdout is not a TTY, the default `--output` format is now **json** instead of plaintext. Scripts that assumed plaintext when output was piped or redirected should set `LD_OUTPUT=plaintext`, run `ldcli config --set output plaintext`, or pass `--output plaintext` (or `--output json` explicitly if you want JSON regardless of TTY). You can also set **`FORCE_TTY`** or **`LD_FORCE_TTY`** to any non-empty value to keep plaintext as the default when stdout is not a TTY, without changing the saved `output` setting. + ## [2.2.0](https://github.com/launchdarkly/ldcli/compare/v2.1.0...v2.2.0) (2026-02-20) diff --git a/README.md b/README.md index f83e81f8..621da7a1 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,7 @@ Supported settings: * `base-uri` LaunchDarkly base URI (default "https://app.launchdarkly.com") - `environment`: Default environment key - `flag`: Default feature flag key -- `output`: Command response output format in either JSON or plain text +- `output`: Output format: json or plaintext (default: plaintext in a terminal, json otherwise) - `project`: Default project key Available `config` commands: @@ -90,6 +90,16 @@ ldcli config --set access-token api-00000000-0000-0000-0000-000000000000 Running this command creates a configuration file located at `$XDG_CONFIG_HOME/ldcli/config.yml` with the access token. Subsequent commands read from this file, so you do not need to specify the access token each time. +### Output format defaults + +When you do not pass `--output` or `--json`, the default format depends on whether standard output is a terminal: **plaintext** in an interactive terminal, **json** when stdout is not a TTY (for example when piped, in CI, or in agent environments). + +To force the plaintext default even when stdout is not a TTY, set either **`FORCE_TTY`** or **`LD_FORCE_TTY`** to any non-empty value (similar to tools that use `NO_COLOR`). That only affects the default; explicit `--output`, `--json`, `LD_OUTPUT`, and the `output` setting in your config file still apply. + +**`LD_OUTPUT`** is the same setting as `output` in the config file, exposed as an environment variable (see the `LD_` prefix above). It is not new with TTY detection; the test suite locks in that it overrides the non-TTY JSON default when set to `plaintext`. + +Effective output is resolved in this order: **`--json`** (if set, wins over `--output` when both are present), then **`--output`**, then **`LD_OUTPUT`**, then the **`output`** value from your config file, then the TTY-based default above. + ## Commands LaunchDarkly CLI commands: diff --git a/cmd/analytics/analytics_test.go b/cmd/analytics/analytics_test.go index ea6b6864..34d6fd33 100644 --- a/cmd/analytics/analytics_test.go +++ b/cmd/analytics/analytics_test.go @@ -8,8 +8,8 @@ import ( ) type mockEnvChecker struct { - envVars map[string]string - stdinTerminal bool + envVars map[string]string + stdinTerminal bool stdoutTerminal bool } diff --git a/cmd/cliflags/flags.go b/cmd/cliflags/flags.go index 822d93c5..592894d0 100644 --- a/cmd/cliflags/flags.go +++ b/cmd/cliflags/flags.go @@ -44,7 +44,7 @@ const ( EnvironmentFlagDescription = "Default environment key" FlagFlagDescription = "Default feature flag key" JSONFlagDescription = "Output JSON format (shorthand for --output json)" - OutputFlagDescription = "Command response output format in either JSON or plain text" + OutputFlagDescription = "Output format: json or plaintext (default: plaintext in a terminal, json otherwise)" PortFlagDescription = "Port for the dev server to run on" ProjectFlagDescription = "Default project key" SyncOnceFlagDescription = "Only sync new projects. Existing projects will neither be resynced nor have overrides specified by CLI flags applied." diff --git a/cmd/cmdtest.go b/cmd/cmdtest.go index 342bfd68..54ea73bd 100644 --- a/cmd/cmdtest.go +++ b/cmd/cmdtest.go @@ -19,6 +19,9 @@ var StubbedSuccessResponse = `{ "name": "test-name" }` +// CallCmd runs the root command for integration-style tests. It passes isTerminal always true so +// the default --output matches an interactive terminal (plaintext); non-TTY JSON defaults are +// covered in root_test.go. func CallCmd( t *testing.T, clients APIClients, @@ -31,6 +34,8 @@ func CallCmd( clients, "test", false, + func() bool { return true }, + nil, ) cmd := rootCmd.Cmd() require.NoError(t, err) diff --git a/cmd/config/testdata/help.golden b/cmd/config/testdata/help.golden index 71f3652f..a0aaeab2 100644 --- a/cmd/config/testdata/help.golden +++ b/cmd/config/testdata/help.golden @@ -9,7 +9,7 @@ Supported settings: - `dev-stream-uri`: Streaming service endpoint that the dev server uses to obtain authoritative flag data. This may be a LaunchDarkly or Relay Proxy endpoint - `environment`: Default environment key - `flag`: Default feature flag key -- `output`: Command response output format in either JSON or plain text +- `output`: Output format: json or plaintext (default: plaintext in a terminal, json otherwise) - `port`: Port for the dev server to run on - `project`: Default project key - `sync-once`: Only sync new projects. Existing projects will neither be resynced nor have overrides specified by CLI flags applied. @@ -28,4 +28,4 @@ Global Flags: --analytics-opt-out Opt out of analytics tracking --base-uri string LaunchDarkly base URI (default "https://app.launchdarkly.com") --json Output JSON format (shorthand for --output json) - -o, --output string Command response output format in either JSON or plain text (default "plaintext") + -o, --output string Output format: json or plaintext (default: plaintext in a terminal, json otherwise) (default "plaintext") diff --git a/cmd/root.go b/cmd/root.go index 67c2ce3d..684e50fc 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -13,6 +13,7 @@ import ( "github.com/google/uuid" "github.com/spf13/cobra" "github.com/spf13/viper" + "golang.org/x/term" cmdAnalytics "github.com/launchdarkly/ldcli/cmd/analytics" "github.com/launchdarkly/ldcli/cmd/cliflags" @@ -81,13 +82,40 @@ func init() { cobra.AddTemplateFunc("HasOptionalFlags", HasOptionalFlags) } +// forceTTYDefaultOutput is true when FORCE_TTY or LD_FORCE_TTY is non-empty, so the default +// --output is plaintext even if stdout is not a TTY (similar to NO_COLOR). Explicit --output, +// --json, LD_OUTPUT, and config file values still take precedence via Viper/Cobra after parse. +// +// getenv, if non-nil, is used only for those two keys (tests can inject without mutating +// process environment). If nil, os.Getenv is used. +func forceTTYDefaultOutput(getenv func(string) string) bool { + lookup := getenv + if lookup == nil { + lookup = os.Getenv + } + return lookup("FORCE_TTY") != "" || lookup("LD_FORCE_TTY") != "" +} + +// NewRootCommand constructs the ldcli root command tree. +// +// isTerminal must be non-nil; it should reflect whether stdout is a TTY (see Execute). When it +// returns false, the default --output is json unless forceTTYDefaultOutput applies. +// +// getenv is optional: when non-nil, it is used to read FORCE_TTY and LD_FORCE_TTY only; when +// nil, os.Getenv is used. Viper still reads LD_OUTPUT and other LD_ vars from the real +// environment. func NewRootCommand( configService config.Service, analyticsTrackerFn analytics.TrackerFn, clients APIClients, version string, useConfigFile bool, + isTerminal func() bool, + getenv func(string) string, ) (*RootCmd, error) { + if isTerminal == nil { + return nil, errors.New("NewRootCommand: isTerminal must not be nil") + } cmd := &cobra.Command{ Use: "ldcli", Short: "LaunchDarkly CLI", @@ -188,10 +216,17 @@ func NewRootCommand( return nil, err } + // When stdout is not a TTY (e.g. piped, CI, agent), default to JSON unless FORCE_TTY or + // LD_FORCE_TTY is set (any non-empty value), like NO_COLOR. + defaultOutput := "plaintext" + if !forceTTYDefaultOutput(getenv) && !isTerminal() { + defaultOutput = "json" + } + cmd.PersistentFlags().StringP( cliflags.OutputFlag, "o", - "plaintext", + defaultOutput, cliflags.OutputFlagDescription, ) err = viper.BindPFlag(cliflags.OutputFlag, cmd.PersistentFlags().Lookup(cliflags.OutputFlag)) @@ -252,6 +287,8 @@ func Execute(version string) { clients, version, true, + func() bool { return term.IsTerminal(int(os.Stdout.Fd())) }, + nil, ) if err != nil { log.Fatal(err) diff --git a/cmd/root_test.go b/cmd/root_test.go index 59bd69da..18229042 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -1,13 +1,20 @@ package cmd_test import ( + "bytes" + "io" + "os" + "path/filepath" "testing" + "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/launchdarkly/ldcli/cmd" + "github.com/launchdarkly/ldcli/cmd/cliflags" "github.com/launchdarkly/ldcli/internal/analytics" + "github.com/launchdarkly/ldcli/internal/config" "github.com/launchdarkly/ldcli/internal/resources" ) @@ -29,7 +36,7 @@ func TestCreate(t *testing.T) { }) } -func TestJSONFlag(t *testing.T) { +func TestOutputFlags(t *testing.T) { mockClient := &resources.MockClient{ Response: []byte(`{"key": "test-key", "name": "test-name"}`), } @@ -55,4 +62,275 @@ func TestJSONFlag(t *testing.T) { assert.Contains(t, string(output), `"key": "test-key"`) assert.NotContains(t, string(output), "Successfully updated") }) + + t.Run("--output json returns raw JSON output", func(t *testing.T) { + args := []string{ + "flags", "toggle-on", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + "--output", "json", + } + + output, err := cmd.CallCmd( + t, + cmd.APIClients{ResourcesClient: mockClient}, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.Contains(t, string(output), `"key": "test-key"`) + assert.NotContains(t, string(output), "Successfully updated") + }) + + t.Run("--output plaintext returns human-readable output", func(t *testing.T) { + args := []string{ + "flags", "toggle-on", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + "--output", "plaintext", + } + + output, err := cmd.CallCmd( + t, + cmd.APIClients{ResourcesClient: mockClient}, + analytics.NoopClientFn{}.Tracker(), + args, + ) + + require.NoError(t, err) + assert.Contains(t, string(output), "Successfully updated") + assert.Contains(t, string(output), "test-name (test-key)") + }) +} + +// getenv is passed to NewRootCommand for FORCE_TTY / LD_FORCE_TTY only; nil uses os.Getenv. +func newRootCmdWithTerminal(t *testing.T, isTerminal func() bool, getenv func(string) string) *cmd.RootCmd { + t.Helper() + rootCmd, err := cmd.NewRootCommand( + config.NewService(&resources.MockClient{}), + analytics.NoopClientFn{}.Tracker(), + cmd.APIClients{}, + "test", + false, + isTerminal, + getenv, + ) + require.NoError(t, err) + return rootCmd +} + +func execNonTTYCmd(t *testing.T, mockClient *resources.MockClient, extraArgs ...string) []byte { + t.Helper() + return execNonTTYCmdGetenv(t, mockClient, nil, extraArgs...) +} + +// getenv is forwarded to NewRootCommand for FORCE_TTY / LD_FORCE_TTY only (nil uses os.Getenv). +func execNonTTYCmdGetenv(t *testing.T, mockClient *resources.MockClient, getenv func(string) string, extraArgs ...string) []byte { + t.Helper() + rootCmd, err := cmd.NewRootCommand( + config.NewService(&resources.MockClient{}), + analytics.NoopClientFn{}.Tracker(), + cmd.APIClients{ResourcesClient: mockClient}, + "test", + false, + func() bool { return false }, + getenv, + ) + require.NoError(t, err) + + c := rootCmd.Cmd() + b := bytes.NewBufferString("") + c.SetOut(b) + args := []string{ + "flags", "toggle-on", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + } + args = append(args, extraArgs...) + c.SetArgs(args) + + err = c.Execute() + require.NoError(t, err) + + out, err := io.ReadAll(b) + require.NoError(t, err) + return out +} + +func TestOutputDefaultsAndOverrides(t *testing.T) { + mockClient := &resources.MockClient{ + Response: []byte(`{"key": "test-key", "name": "test-name"}`), + } + + t.Run("non-TTY defaults to json output", func(t *testing.T) { + rootCmd := newRootCmdWithTerminal(t, func() bool { return false }, func(string) string { return "" }) + + f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) + require.NotNil(t, f) + assert.Equal(t, "json", f.DefValue) + }) + + t.Run("TTY defaults to plaintext output", func(t *testing.T) { + rootCmd := newRootCmdWithTerminal(t, func() bool { return true }, func(string) string { return "" }) + + f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) + require.NotNil(t, f) + assert.Equal(t, "plaintext", f.DefValue) + }) + + t.Run("explicit --output plaintext overrides non-TTY", func(t *testing.T) { + out := execNonTTYCmd(t, mockClient, "--output", "plaintext") + assert.Contains(t, string(out), "Successfully updated") + }) + + t.Run("non-TTY without explicit flag returns JSON", func(t *testing.T) { + out := execNonTTYCmd(t, mockClient) + assert.Contains(t, string(out), `"key"`) + assert.NotContains(t, string(out), "Successfully updated") + }) + + t.Run("--json overrides non-TTY default", func(t *testing.T) { + out := execNonTTYCmd(t, mockClient, "--json") + assert.Contains(t, string(out), `"key"`) + assert.NotContains(t, string(out), "Successfully updated") + }) + + t.Run("LD_OUTPUT=plaintext overrides non-TTY default", func(t *testing.T) { + t.Setenv("LD_OUTPUT", "plaintext") + + out := execNonTTYCmd(t, mockClient) + assert.Contains(t, string(out), "Successfully updated") + assert.Contains(t, string(out), "test-name (test-key)") + }) + + t.Run("FORCE_TTY=0 yields plaintext DefValue when non-TTY", func(t *testing.T) { + getenv := func(k string) string { + if k == "FORCE_TTY" { + return "0" + } + return "" + } + rootCmd := newRootCmdWithTerminal(t, func() bool { return false }, getenv) + + f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) + require.NotNil(t, f) + assert.Equal(t, "plaintext", f.DefValue) + }) + + t.Run("FORCE_TTY=1 yields plaintext DefValue when non-TTY", func(t *testing.T) { + getenv := func(k string) string { + if k == "FORCE_TTY" { + return "1" + } + return "" + } + rootCmd := newRootCmdWithTerminal(t, func() bool { return false }, getenv) + + f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) + require.NotNil(t, f) + assert.Equal(t, "plaintext", f.DefValue) + }) + + t.Run("LD_FORCE_TTY=1 yields plaintext DefValue when non-TTY", func(t *testing.T) { + getenv := func(k string) string { + if k == "LD_FORCE_TTY" { + return "1" + } + return "" + } + rootCmd := newRootCmdWithTerminal(t, func() bool { return false }, getenv) + + f := rootCmd.Cmd().PersistentFlags().Lookup(cliflags.OutputFlag) + require.NotNil(t, f) + assert.Equal(t, "plaintext", f.DefValue) + }) + + t.Run("LD_FORCE_TTY=1 yields plaintext output when non-TTY", func(t *testing.T) { + getenv := func(k string) string { + if k == "LD_FORCE_TTY" { + return "1" + } + return "" + } + out := execNonTTYCmdGetenv(t, mockClient, getenv) + assert.Contains(t, string(out), "Successfully updated") + assert.Contains(t, string(out), "test-name (test-key)") + }) + + t.Run("FORCE_TTY=1 yields plaintext output when non-TTY", func(t *testing.T) { + getenv := func(k string) string { + if k == "FORCE_TTY" { + return "1" + } + return "" + } + out := execNonTTYCmdGetenv(t, mockClient, getenv) + assert.Contains(t, string(out), "Successfully updated") + assert.Contains(t, string(out), "test-name (test-key)") + }) +} + +func TestNewRootCommandNilIsTerminalRejects(t *testing.T) { + _, err := cmd.NewRootCommand( + config.NewService(&resources.MockClient{}), + analytics.NoopClientFn{}.Tracker(), + cmd.APIClients{}, + "test", + false, + nil, + nil, + ) + require.Error(t, err) + assert.Contains(t, err.Error(), "isTerminal") +} + +func TestConfigOutputPrecedenceNonTTY(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + + mockClient := &resources.MockClient{ + Response: []byte(`{"key": "test-key", "name": "test-name"}`), + } + + cfgRoot := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", cfgRoot) + + ldcliDir := filepath.Join(cfgRoot, "ldcli") + require.NoError(t, os.MkdirAll(ldcliDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(ldcliDir, "config.yml"), []byte("output: plaintext\n"), 0o644)) + + rootCmd, err := cmd.NewRootCommand( + config.NewService(&resources.MockClient{}), + analytics.NoopClientFn{}.Tracker(), + cmd.APIClients{ResourcesClient: mockClient}, + "test", + true, + func() bool { return false }, + nil, + ) + require.NoError(t, err) + + c := rootCmd.Cmd() + b := bytes.NewBufferString("") + c.SetOut(b) + c.SetArgs([]string{ + "flags", "toggle-on", + "--access-token", "abcd1234", + "--environment", "test-env", + "--flag", "test-flag", + "--project", "test-proj", + }) + require.NoError(t, c.Execute()) + + out, err := io.ReadAll(b) + require.NoError(t, err) + assert.Contains(t, string(out), "Successfully updated") + assert.Contains(t, string(out), "test-name (test-key)") } diff --git a/internal/output/output_test.go b/internal/output/output_test.go index cdb31746..df701f9d 100644 --- a/internal/output/output_test.go +++ b/internal/output/output_test.go @@ -1,13 +1,40 @@ package output_test import ( - "github.com/launchdarkly/ldcli/internal/output" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/launchdarkly/ldcli/internal/output" ) +func TestNewOutputKind(t *testing.T) { + t.Run("returns json for valid json input", func(t *testing.T) { + kind, err := output.NewOutputKind("json") + require.NoError(t, err) + assert.Equal(t, output.OutputKindJSON, kind) + }) + + t.Run("returns plaintext for valid plaintext input", func(t *testing.T) { + kind, err := output.NewOutputKind("plaintext") + require.NoError(t, err) + assert.Equal(t, output.OutputKindPlaintext, kind) + }) + + t.Run("returns error for invalid input", func(t *testing.T) { + kind, err := output.NewOutputKind("xml") + assert.ErrorIs(t, err, output.ErrInvalidOutputKind) + assert.Equal(t, output.OutputKindNull, kind) + }) + + t.Run("returns error for empty string", func(t *testing.T) { + kind, err := output.NewOutputKind("") + assert.ErrorIs(t, err, output.ErrInvalidOutputKind) + assert.Equal(t, output.OutputKindNull, kind) + }) +} + func TestCmdOutputSingular(t *testing.T) { tests := map[string]struct { expected string @@ -63,4 +90,17 @@ func TestCmdOutputSingular(t *testing.T) { assert.Equal(t, tt.expected, output) }) } + + t.Run("with json output kind returns raw JSON", func(t *testing.T) { + input := `{"key": "test-key", "name": "test-name"}` + + result, err := output.CmdOutputSingular( + "json", + []byte(input), + output.SingularPlaintextOutputFn, + ) + + require.NoError(t, err) + assert.JSONEq(t, input, result) + }) } diff --git a/internal/output/resource_output_test.go b/internal/output/resource_output_test.go index dae78fa0..1d1ec434 100644 --- a/internal/output/resource_output_test.go +++ b/internal/output/resource_output_test.go @@ -310,16 +310,54 @@ func TestCmdOutputError(t *testing.T) { assert.JSONEq(t, expected, result) }) - t.Run("with plaintext output", func(t *testing.T) { - expected := "invalid JSON" + t.Run("with plaintext output returns formatted message", func(t *testing.T) { + err := errors.NewError(`{"code":"conflict", "message":"an error"}`) + + result := output.CmdOutputError("plaintext", err) + + assert.Equal(t, "an error (code: conflict)", result) + }) + }) + + t.Run("with a json.UnmarshalTypeError", func(t *testing.T) { + t.Run("with plaintext output returns invalid JSON", func(t *testing.T) { + type testType any + invalid := []byte(`{"invalid": true}`) + err := json.Unmarshal(invalid, &[]testType{}) + require.Error(t, err) + + result := output.CmdOutputError("plaintext", err) + + assert.Equal(t, "invalid JSON", result) + }) + + t.Run("with JSON output returns invalid JSON message", func(t *testing.T) { type testType any invalid := []byte(`{"invalid": true}`) err := json.Unmarshal(invalid, &[]testType{}) require.Error(t, err) + result := output.CmdOutputError("json", err) + + assert.JSONEq(t, `{"message":"invalid JSON"}`, result) + }) + }) + + t.Run("with a generic error", func(t *testing.T) { + t.Run("with JSON output wraps message in JSON", func(t *testing.T) { + err := fmt.Errorf("something went wrong") + + result := output.CmdOutputError("json", err) + + assert.JSONEq(t, `{"message":"something went wrong"}`, result) + }) + + t.Run("with plaintext output returns message", func(t *testing.T) { + err := fmt.Errorf("something went wrong") + result := output.CmdOutputError("plaintext", err) - assert.Equal(t, expected, result) + assert.Equal(t, "something went wrong", result) }) }) }