From 9cfa45fa99cb1c4d45a6c443357b1cf3a8911842 Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Tue, 25 Nov 2025 16:49:02 -0500 Subject: [PATCH 1/5] Bind output flag instead of manually checking it --- internal/tiger/cmd/auth.go | 12 ++++++---- internal/tiger/cmd/config.go | 14 +++++------ internal/tiger/cmd/db.go | 12 ++++++---- internal/tiger/cmd/service.go | 44 +++++++++++++++++++---------------- 4 files changed, 45 insertions(+), 37 deletions(-) diff --git a/internal/tiger/cmd/auth.go b/internal/tiger/cmd/auth.go index cb1dfcfe..58a4adab 100644 --- a/internal/tiger/cmd/auth.go +++ b/internal/tiger/cmd/auth.go @@ -11,6 +11,7 @@ import ( "github.com/olekukonko/tablewriter" "github.com/spf13/cobra" + "github.com/spf13/viper" "golang.org/x/term" "golang.org/x/text/cases" "golang.org/x/text/language" @@ -174,6 +175,12 @@ func buildStatusCmd() *cobra.Command { Long: "Displays whether you are logged in and shows your currently configured project ID.", Args: cobra.NoArgs, ValidArgsFunction: cobra.NoFileCompletions, + PreRunE: func(cmd *cobra.Command, args []string) error { + if err := viper.BindPFlag("output", cmd.Flags().Lookup("output")); err != nil { + return fmt.Errorf("failed to bind output flag: %w", err) + } + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { cmd.SilenceUsage = true @@ -183,11 +190,6 @@ func buildStatusCmd() *cobra.Command { return fmt.Errorf("failed to load config: %w", err) } - // Use flag value if provided, otherwise use config value - if cmd.Flags().Changed("output") { - cfg.Output = output - } - apiKey, _, err := config.GetCredentials() if err != nil { return err diff --git a/internal/tiger/cmd/config.go b/internal/tiger/cmd/config.go index 3e3931f1..f2dae52b 100644 --- a/internal/tiger/cmd/config.go +++ b/internal/tiger/cmd/config.go @@ -27,6 +27,12 @@ func buildConfigShowCmd() *cobra.Command { Long: `Display the current CLI configuration settings`, Args: cobra.NoArgs, ValidArgsFunction: cobra.NoFileCompletions, + PreRunE: func(cmd *cobra.Command, args []string) error { + if err := viper.BindPFlag("output", cmd.Flags().Lookup("output")); err != nil { + return fmt.Errorf("failed to bind output flag: %w", err) + } + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { cmd.SilenceUsage = true @@ -35,12 +41,6 @@ func buildConfigShowCmd() *cobra.Command { return fmt.Errorf("failed to load config: %w", err) } - // Use flag value if provided, otherwise use config value - outputFormat := cfg.Output - if cmd.Flags().Changed("output") { - outputFormat = output - } - configFile, err := cfg.EnsureConfigDir() if err != nil { return err @@ -69,7 +69,7 @@ func buildConfigShowCmd() *cobra.Command { } output := cmd.OutOrStdout() - switch outputFormat { + switch cfg.Output { case "json": return util.SerializeToJSON(output, cfgOut) case "yaml": diff --git a/internal/tiger/cmd/db.go b/internal/tiger/cmd/db.go index fd85a310..12a1c587 100644 --- a/internal/tiger/cmd/db.go +++ b/internal/tiger/cmd/db.go @@ -14,6 +14,7 @@ import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgconn" "github.com/spf13/cobra" + "github.com/spf13/viper" "golang.org/x/term" "github.com/timescale/tiger-cli/internal/tiger/api" @@ -642,6 +643,12 @@ PostgreSQL Configuration Parameters That May Be Set: (kills queries that exceed the specified duration, in milliseconds)`, Args: cobra.MaximumNArgs(1), ValidArgsFunction: serviceIDCompletion, + PreRunE: func(cmd *cobra.Command, args []string) error { + if err := viper.BindPFlag("output", cmd.Flags().Lookup("output")); err != nil { + return fmt.Errorf("failed to bind output flag: %w", err) + } + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { // Validate arguments if roleName == "" { @@ -656,11 +663,6 @@ PostgreSQL Configuration Parameters That May Be Set: return fmt.Errorf("failed to load config: %w", err) } - // Use flag value if provided, otherwise use config value - if cmd.Flags().Changed("output") { - cfg.Output = output - } - // Get password rolePassword, err := getPasswordForRole(passwordFlag) if err != nil { diff --git a/internal/tiger/cmd/service.go b/internal/tiger/cmd/service.go index bec43980..4738b347 100644 --- a/internal/tiger/cmd/service.go +++ b/internal/tiger/cmd/service.go @@ -75,6 +75,12 @@ Examples: tiger service get svc-12345 --output yaml`, Args: cobra.MaximumNArgs(1), ValidArgsFunction: serviceIDCompletion, + PreRunE: func(cmd *cobra.Command, args []string) error { + if err := viper.BindPFlag("output", cmd.Flags().Lookup("output")); err != nil { + return fmt.Errorf("failed to bind output flag: %w", err) + } + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { // Get config cfg, err := config.Load() @@ -82,11 +88,6 @@ Examples: return fmt.Errorf("failed to load config: %w", err) } - // Use flag value if provided, otherwise use config value - if cmd.Flags().Changed("output") { - cfg.Output = output - } - // Determine service ID serviceID, err := getServiceID(cfg, args) if err != nil { @@ -148,6 +149,12 @@ func buildServiceListCmd() *cobra.Command { Long: `List all database services in the current project.`, Args: cobra.NoArgs, ValidArgsFunction: cobra.NoFileCompletions, + PreRunE: func(cmd *cobra.Command, args []string) error { + if err := viper.BindPFlag("output", cmd.Flags().Lookup("output")); err != nil { + return fmt.Errorf("failed to bind output flag: %w", err) + } + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { // Get config cfg, err := config.Load() @@ -155,11 +162,6 @@ func buildServiceListCmd() *cobra.Command { return fmt.Errorf("failed to load config: %w", err) } - // Use flag value if provided, otherwise use config value - if cmd.Flags().Changed("output") { - cfg.Output = output - } - cmd.SilenceUsage = true // Get API key and project ID for authentication @@ -281,6 +283,12 @@ Allowed CPU/Memory Configurations: Note: You can specify both CPU and memory together, or specify only one (the other will be automatically configured).`, Args: cobra.NoArgs, ValidArgsFunction: cobra.NoFileCompletions, + PreRunE: func(cmd *cobra.Command, args []string) error { + if err := viper.BindPFlag("output", cmd.Flags().Lookup("output")); err != nil { + return fmt.Errorf("failed to bind output flag: %w", err) + } + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { // Get config cfg, err := config.Load() @@ -288,11 +296,6 @@ Note: You can specify both CPU and memory together, or specify only one (the oth return fmt.Errorf("failed to load config: %w", err) } - // Use flag value if provided, otherwise use config value - if cmd.Flags().Changed("output") { - cfg.Output = output - } - // Auto-generate service name if not provided if createServiceName == "" { createServiceName = common.GenerateServiceName() @@ -1229,6 +1232,12 @@ Examples: tiger service fork svc-12345 --now --wait-timeout 45m`, Args: cobra.MaximumNArgs(1), ValidArgsFunction: serviceIDCompletion, + PreRunE: func(cmd *cobra.Command, args []string) error { + if err := viper.BindPFlag("output", cmd.Flags().Lookup("output")); err != nil { + return fmt.Errorf("failed to bind output flag: %w", err) + } + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { // Validate timing flags first - exactly one must be specified timingFlagsSet := 0 @@ -1262,11 +1271,6 @@ Examples: return fmt.Errorf("failed to load config: %w", err) } - // Use flag value if provided, otherwise use config value - if cmd.Flags().Changed("output") { - cfg.Output = output - } - // Determine source service ID serviceID, err := getServiceID(cfg, args) if err != nil { From 01a867f6cd682640ae428e7c575b45070fe4c7bb Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Tue, 25 Nov 2025 16:49:26 -0500 Subject: [PATCH 2/5] Follow same pattern for binding new-password flag --- internal/tiger/cmd/service.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/tiger/cmd/service.go b/internal/tiger/cmd/service.go index 4738b347..426fddc2 100644 --- a/internal/tiger/cmd/service.go +++ b/internal/tiger/cmd/service.go @@ -486,6 +486,12 @@ Examples: tiger service update-password svc-12345 --new-password new-secure-password --password-storage none`, Args: cobra.MaximumNArgs(1), ValidArgsFunction: serviceIDCompletion, + PreRunE: func(cmd *cobra.Command, args []string) error { + if err := viper.BindPFlag("new_password", cmd.Flags().Lookup("new-password")); err != nil { + return fmt.Errorf("failed to bind new-password flag: %w", err) + } + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { // Get config cfg, err := config.Load() @@ -556,9 +562,6 @@ Examples: // Add flags cmd.Flags().StringVar(&updatePasswordValue, "new-password", "", "New password for the tsdbadmin user (can also be set via TIGER_NEW_PASSWORD env var)") - // Bind flags to viper - viper.BindPFlag("new_password", cmd.Flags().Lookup("new-password")) - return cmd } From 95eb59634fb5953b423956d427325d3d4df26ba8 Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Tue, 25 Nov 2025 16:55:23 -0500 Subject: [PATCH 3/5] Follow same pattern for binding root flags --- internal/tiger/cmd/root.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/internal/tiger/cmd/root.go b/internal/tiger/cmd/root.go index 52b4f20e..5fa9e430 100644 --- a/internal/tiger/cmd/root.go +++ b/internal/tiger/cmd/root.go @@ -42,6 +42,18 @@ tiger auth login PersistentPreRunE: func(cmd *cobra.Command, args []string) error { cmd.SetContext(ctx) + // Bind persistent flags to viper + // Use cmd.Flags() which includes inherited persistent flags from parents + if err := errors.Join( + viper.BindPFlag("debug", cmd.Flags().Lookup("debug")), + viper.BindPFlag("service_id", cmd.Flags().Lookup("service-id")), + viper.BindPFlag("analytics", cmd.Flags().Lookup("analytics")), + viper.BindPFlag("password_storage", cmd.Flags().Lookup("password-storage")), + viper.BindPFlag("color", cmd.Flags().Lookup("color")), + ); err != nil { + return fmt.Errorf("failed to bind flags: %w", err) + } + // Setup configuration initialization configDirFlag := cmd.Flags().Lookup("config-dir") if err := config.SetupViper(config.GetEffectiveConfigDir(configDirFlag)); err != nil { @@ -99,18 +111,6 @@ tiger auth login cmd.PersistentFlags().BoolVar(&skipUpdateCheck, "skip-update-check", false, "skip checking for updates on startup") cmd.PersistentFlags().BoolVar(&colorFlag, "color", true, "enable colored output") - // Bind flags to viper - err := errors.Join( - viper.BindPFlag("debug", cmd.PersistentFlags().Lookup("debug")), - viper.BindPFlag("service_id", cmd.PersistentFlags().Lookup("service-id")), - viper.BindPFlag("analytics", cmd.PersistentFlags().Lookup("analytics")), - viper.BindPFlag("password_storage", cmd.PersistentFlags().Lookup("password-storage")), - viper.BindPFlag("color", cmd.PersistentFlags().Lookup("color")), - ) - if err != nil { - return nil, fmt.Errorf("failed to bind flags: %w", err) - } - // Note: api_url is intentionally not exposed as a CLI flag. // It can be configured via: // - Environment variable: TIGER_API_URL From 424b9a5c6dcb1076c10e5dd25501392bfcd47757 Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Tue, 25 Nov 2025 17:26:21 -0500 Subject: [PATCH 4/5] Create bindFlags helper function to reduce code duplication --- internal/tiger/cmd/auth.go | 8 +------- internal/tiger/cmd/config.go | 7 +------ internal/tiger/cmd/db.go | 8 +------- internal/tiger/cmd/flag.go | 25 ++++++++++++++++++++++++- internal/tiger/cmd/service.go | 35 +++++------------------------------ 5 files changed, 32 insertions(+), 51 deletions(-) diff --git a/internal/tiger/cmd/auth.go b/internal/tiger/cmd/auth.go index 58a4adab..886e8351 100644 --- a/internal/tiger/cmd/auth.go +++ b/internal/tiger/cmd/auth.go @@ -11,7 +11,6 @@ import ( "github.com/olekukonko/tablewriter" "github.com/spf13/cobra" - "github.com/spf13/viper" "golang.org/x/term" "golang.org/x/text/cases" "golang.org/x/text/language" @@ -175,12 +174,7 @@ func buildStatusCmd() *cobra.Command { Long: "Displays whether you are logged in and shows your currently configured project ID.", Args: cobra.NoArgs, ValidArgsFunction: cobra.NoFileCompletions, - PreRunE: func(cmd *cobra.Command, args []string) error { - if err := viper.BindPFlag("output", cmd.Flags().Lookup("output")); err != nil { - return fmt.Errorf("failed to bind output flag: %w", err) - } - return nil - }, + PreRunE: bindFlags("output"), RunE: func(cmd *cobra.Command, args []string) error { cmd.SilenceUsage = true diff --git a/internal/tiger/cmd/config.go b/internal/tiger/cmd/config.go index f2dae52b..1ea60f77 100644 --- a/internal/tiger/cmd/config.go +++ b/internal/tiger/cmd/config.go @@ -27,12 +27,7 @@ func buildConfigShowCmd() *cobra.Command { Long: `Display the current CLI configuration settings`, Args: cobra.NoArgs, ValidArgsFunction: cobra.NoFileCompletions, - PreRunE: func(cmd *cobra.Command, args []string) error { - if err := viper.BindPFlag("output", cmd.Flags().Lookup("output")); err != nil { - return fmt.Errorf("failed to bind output flag: %w", err) - } - return nil - }, + PreRunE: bindFlags("output"), RunE: func(cmd *cobra.Command, args []string) error { cmd.SilenceUsage = true diff --git a/internal/tiger/cmd/db.go b/internal/tiger/cmd/db.go index 12a1c587..940f196c 100644 --- a/internal/tiger/cmd/db.go +++ b/internal/tiger/cmd/db.go @@ -14,7 +14,6 @@ import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgconn" "github.com/spf13/cobra" - "github.com/spf13/viper" "golang.org/x/term" "github.com/timescale/tiger-cli/internal/tiger/api" @@ -643,12 +642,7 @@ PostgreSQL Configuration Parameters That May Be Set: (kills queries that exceed the specified duration, in milliseconds)`, Args: cobra.MaximumNArgs(1), ValidArgsFunction: serviceIDCompletion, - PreRunE: func(cmd *cobra.Command, args []string) error { - if err := viper.BindPFlag("output", cmd.Flags().Lookup("output")); err != nil { - return fmt.Errorf("failed to bind output flag: %w", err) - } - return nil - }, + PreRunE: bindFlags("output"), RunE: func(cmd *cobra.Command, args []string) error { // Validate arguments if roleName == "" { diff --git a/internal/tiger/cmd/flag.go b/internal/tiger/cmd/flag.go index 24164eb5..515fbbad 100644 --- a/internal/tiger/cmd/flag.go +++ b/internal/tiger/cmd/flag.go @@ -1,6 +1,13 @@ package cmd -import "github.com/timescale/tiger-cli/internal/tiger/config" +import ( + "fmt" + "strings" + + "github.com/spf13/cobra" + "github.com/spf13/viper" + "github.com/timescale/tiger-cli/internal/tiger/config" +) // outputFlag implements the [github.com/spf13/pflag.Value] interface. type outputFlag string @@ -39,3 +46,19 @@ func (o *outputWithEnvFlag) String() string { func (o *outputWithEnvFlag) Type() string { return "string" } + +type runE func(cmd *cobra.Command, args []string) error + +var flagNameReplacer = strings.NewReplacer("-", "_") + +func bindFlags(flags ...string) runE { + return func(cmd *cobra.Command, args []string) error { + for _, flag := range flags { + key := flagNameReplacer.Replace(flag) + if err := viper.BindPFlag(key, cmd.Flags().Lookup(flag)); err != nil { + return fmt.Errorf("failed to bind %s flag: %w", flag, err) + } + } + return nil + } +} diff --git a/internal/tiger/cmd/service.go b/internal/tiger/cmd/service.go index 426fddc2..4146da00 100644 --- a/internal/tiger/cmd/service.go +++ b/internal/tiger/cmd/service.go @@ -75,12 +75,7 @@ Examples: tiger service get svc-12345 --output yaml`, Args: cobra.MaximumNArgs(1), ValidArgsFunction: serviceIDCompletion, - PreRunE: func(cmd *cobra.Command, args []string) error { - if err := viper.BindPFlag("output", cmd.Flags().Lookup("output")); err != nil { - return fmt.Errorf("failed to bind output flag: %w", err) - } - return nil - }, + PreRunE: bindFlags("output"), RunE: func(cmd *cobra.Command, args []string) error { // Get config cfg, err := config.Load() @@ -149,12 +144,7 @@ func buildServiceListCmd() *cobra.Command { Long: `List all database services in the current project.`, Args: cobra.NoArgs, ValidArgsFunction: cobra.NoFileCompletions, - PreRunE: func(cmd *cobra.Command, args []string) error { - if err := viper.BindPFlag("output", cmd.Flags().Lookup("output")); err != nil { - return fmt.Errorf("failed to bind output flag: %w", err) - } - return nil - }, + PreRunE: bindFlags("output"), RunE: func(cmd *cobra.Command, args []string) error { // Get config cfg, err := config.Load() @@ -283,12 +273,7 @@ Allowed CPU/Memory Configurations: Note: You can specify both CPU and memory together, or specify only one (the other will be automatically configured).`, Args: cobra.NoArgs, ValidArgsFunction: cobra.NoFileCompletions, - PreRunE: func(cmd *cobra.Command, args []string) error { - if err := viper.BindPFlag("output", cmd.Flags().Lookup("output")); err != nil { - return fmt.Errorf("failed to bind output flag: %w", err) - } - return nil - }, + PreRunE: bindFlags("output"), RunE: func(cmd *cobra.Command, args []string) error { // Get config cfg, err := config.Load() @@ -486,12 +471,7 @@ Examples: tiger service update-password svc-12345 --new-password new-secure-password --password-storage none`, Args: cobra.MaximumNArgs(1), ValidArgsFunction: serviceIDCompletion, - PreRunE: func(cmd *cobra.Command, args []string) error { - if err := viper.BindPFlag("new_password", cmd.Flags().Lookup("new-password")); err != nil { - return fmt.Errorf("failed to bind new-password flag: %w", err) - } - return nil - }, + PreRunE: bindFlags("new-password"), RunE: func(cmd *cobra.Command, args []string) error { // Get config cfg, err := config.Load() @@ -1235,12 +1215,7 @@ Examples: tiger service fork svc-12345 --now --wait-timeout 45m`, Args: cobra.MaximumNArgs(1), ValidArgsFunction: serviceIDCompletion, - PreRunE: func(cmd *cobra.Command, args []string) error { - if err := viper.BindPFlag("output", cmd.Flags().Lookup("output")); err != nil { - return fmt.Errorf("failed to bind output flag: %w", err) - } - return nil - }, + PreRunE: bindFlags("output"), RunE: func(cmd *cobra.Command, args []string) error { // Validate timing flags first - exactly one must be specified timingFlagsSet := 0 From 12bf029345d754e535e4d4397b975036a5cb3ba5 Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Tue, 25 Nov 2025 17:44:12 -0500 Subject: [PATCH 5/5] Update CLAUDE.md, get rid of unnecessary root.go comment --- CLAUDE.md | 137 +++++++++++++++++++++++-------------- internal/tiger/cmd/root.go | 7 -- 2 files changed, 84 insertions(+), 60 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 50df4abc..98315d3f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -439,20 +439,20 @@ RunE: func(cmd *cobra.Command, args []string) error { if len(args) < 1 { return fmt.Errorf("service ID is required") } - + // 2. Set SilenceUsage = true after argument validation cmd.SilenceUsage = true - + // 3. Proceed with business logic - errors here don't show usage if err := someAPICall(); err != nil { return fmt.Errorf("operation failed: %w", err) } - + return nil }, ``` -**Philosophy**: +**Philosophy**: - Early argument/syntax errors → show usage (helps users learn command syntax) - Operational errors after arguments are validated → don't show usage (avoids cluttering output with irrelevant usage info) @@ -465,7 +465,7 @@ Tiger CLI uses a pure functional builder pattern with **zero global command stat ### Philosophy - **No global variables** - All commands, flags, and state are locally scoped -- **Functional builders** - Every command is built by a dedicated function +- **Functional builders** - Every command is built by a dedicated function - **Complete tree building** - `buildRootCmd()` constructs the entire CLI structure - **Perfect test isolation** - Each test gets completely fresh command instances - **Self-contained commands** - All dependencies passed explicitly via parameters @@ -513,48 +513,42 @@ func buildRootCmd() *cobra.Command { // Declare ALL flag variables locally within this function var configDir string var debug bool - var serviceID string - var analytics bool - var passwordStorage string + // ... other flag variables cmd := &cobra.Command{ Use: "tiger", Short: "Tiger CLI - Tiger Cloud Platform command-line interface", Long: `Complete CLI description...`, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { - // Use local flag variables in scope - if err := logging.Init(debug); err != nil { - return fmt.Errorf("failed to initialize logging: %w", err) + // Bind persistent flags to viper at execution time + if err := errors.Join( + viper.BindPFlag("debug", cmd.Flags().Lookup("debug")), + // ... bind remaining flags + ); err != nil { + return fmt.Errorf("failed to bind flags: %w", err) } + + // Setup configuration and initialize logging // ... rest of initialization - return nil }, } - // Set up configuration and flags... - cobra.OnInitialize(initConfigFunc) + // Set up persistent flags cmd.PersistentFlags().StringVar(&configDir, "config-dir", config.GetDefaultConfigDir(), "config directory") cmd.PersistentFlags().BoolVar(&debug, "debug", false, "enable debug logging") - cmd.PersistentFlags().StringVar(&serviceID, "service-id", "", "service ID") - cmd.PersistentFlags().BoolVar(&analytics, "analytics", true, "enable/disable usage analytics") - cmd.PersistentFlags().StringVar(&passwordStorage, "password-storage", config.DefaultPasswordStorage, "password storage method (keyring, pgpass, none)") - - // Bind flags to viper - viper.BindPFlag("debug", cmd.PersistentFlags().Lookup("debug")) - // ... bind remaining flags + // ... add remaining persistent flags // Add all subcommands (complete tree building) cmd.AddCommand(buildVersionCmd()) cmd.AddCommand(buildConfigCmd()) - cmd.AddCommand(buildAuthCmd()) - cmd.AddCommand(buildServiceCmd()) - cmd.AddCommand(buildDbCmd()) - cmd.AddCommand(buildMCPCmd()) + // ... add remaining subcommands return cmd } ``` +See `internal/tiger/cmd/root.go` for the complete implementation. + ### Simple Command Pattern For commands without flags: @@ -563,7 +557,7 @@ For commands without flags: func buildVersionCmd() *cobra.Command { return &cobra.Command{ Use: "version", - Short: "Show version information", + Short: "Show version information", Long: `Display version, build time, and git commit information.`, Run: func(cmd *cobra.Command, args []string) { fmt.Printf("Tiger CLI %s\n", Version) @@ -583,7 +577,7 @@ func buildMyFlaggedCmd() *cobra.Command { var myFlag string var enableFeature bool var retryCount int - + cmd := &cobra.Command{ Use: "my-command", Short: "Command with local flags", @@ -591,25 +585,61 @@ func buildMyFlaggedCmd() *cobra.Command { if len(args) < 1 { return fmt.Errorf("argument required") } - + cmd.SilenceUsage = true - + // Use flag variables (they're in scope) - fmt.Printf("Flag: %s, Feature: %t, Retries: %d\n", + fmt.Printf("Flag: %s, Feature: %t, Retries: %d\n", myFlag, enableFeature, retryCount) return nil }, } - + // Add flags - bound to local variables - cmd.Flags().StringVar(&myFlag, "flag", "", "My flag description") + cmd.Flags().StringVar(&myFlag, "flag", "", "My flag description") cmd.Flags().BoolVar(&enableFeature, "enable", false, "Enable feature") cmd.Flags().IntVar(&retryCount, "retries", 3, "Retry count") - + return cmd } ``` +### Commands with Flags That Need Viper Binding + +For commands that need their flags bound to viper for configuration precedence (flag > env > config > default), use the `bindFlags()` helper: + +```go +func buildMyConfigurableFlagCmd() *cobra.Command { + var output string + + cmd := &cobra.Command{ + Use: "my-command", + Short: "Command with configurable flag", + PreRunE: bindFlags("output"), // Binds flag to viper + RunE: func(cmd *cobra.Command, args []string) error { + cmd.SilenceUsage = true + cfg, err := config.Load() // Now includes bound flag value + // ... use cfg.Output which respects: flag > env > config > default + }, + } + + cmd.Flags().VarP((*outputFlag)(&output), "output", "o", "output format") + return cmd +} +``` + +The `bindFlags()` helper (defined in `internal/tiger/cmd/flag.go`) automatically converts flag names to config keys (e.g., `"new-password"` → `"new_password"`) and supports binding multiple flags: `bindFlags("output", "new-password")`. + +**Why bind flags in PreRunE?** + +Flags must be bound to viper at **execution time**, not at **build time**, for two critical reasons: + +1. **Prevents binding conflicts**: When all commands are built at startup (the builder pattern), binding flags at build time can cause commands' flags to bind to the same viper keys, silently overwriting each other. Only the last binding wins. + +2. **Ensures correct precedence**: Viper must bind flags after the command tree is built but before `config.Load()` is called. This happens in `PreRunE` (or `PersistentPreRunE` for persistent flags), ensuring the precedence order works correctly: command-line flags > environment variables > config file > defaults. + +**Note:** Use `PreRunE` for command-specific flags, and `PersistentPreRunE` for persistent flags on the root command that apply to all subcommands. + ### Parent Commands with Subcommands For commands that contain subcommands, build the complete tree: @@ -621,12 +651,12 @@ func buildParentCmd() *cobra.Command { Short: "Parent command with subcommands", Long: `Parent command containing multiple subcommands.`, } - + // Add all subcommands (builds complete subtree) cmd.AddCommand(buildChild1Cmd()) cmd.AddCommand(buildChild2Cmd()) cmd.AddCommand(buildChild3Cmd()) - + return cmd } ``` @@ -639,7 +669,7 @@ The main application uses a single builder call: func Execute() { // Build complete command tree fresh each time rootCmd := buildRootCmd() - + err := rootCmd.Execute() if err != nil { if exitErr, ok := err.(interface{ ExitCode() int }); ok { @@ -673,12 +703,12 @@ Tests use the full root command builder: func executeCommand(args ...string) (string, error) { // Build complete CLI fresh for each test rootCmd := buildRootCmd() - + buf := new(bytes.Buffer) - rootCmd.SetOut(buf) + rootCmd.SetOut(buf) rootCmd.SetErr(buf) rootCmd.SetArgs(args) - + err := rootCmd.Execute() return buf.String(), err } @@ -686,11 +716,11 @@ func executeCommand(args ...string) (string, error) { func TestMyCommand(t *testing.T) { // Each test gets completely fresh CLI instance output, err := executeCommand("my-command", "--flag", "value") - + if err != nil { t.Fatalf("Command failed: %v", err) } - + if !strings.Contains(output, "expected") { t.Errorf("Expected 'expected' in output: %s", output) } @@ -704,22 +734,22 @@ For tests that need to verify flag values: ```go func executeAndReturnRoot(args ...string) (*cobra.Command, string, error) { rootCmd := buildRootCmd() - + buf := new(bytes.Buffer) rootCmd.SetOut(buf) rootCmd.SetArgs(args) - + err := rootCmd.Execute() return rootCmd, buf.String(), err } func TestFlagValues(t *testing.T) { rootCmd, output, err := executeAndReturnRoot("service", "create", "--name", "test") - + // Navigate to specific command serviceCmd, _, _ := rootCmd.Find([]string{"service"}) createCmd, _, _ := serviceCmd.Find([]string{"create"}) - + // Check flag value nameFlag := createCmd.Flags().Lookup("name") if nameFlag.Value.String() != "test" { @@ -731,7 +761,7 @@ func TestFlagValues(t *testing.T) { ### Benefits of This Architecture 1. **Zero Global State**: No shared variables between commands or tests -2. **Perfect Test Isolation**: Each test builds completely fresh command trees +2. **Perfect Test Isolation**: Each test builds completely fresh command trees 3. **Simplified Initialization**: Single entry point builds everything 4. **Maintainable Code**: No complex global variable management 5. **Easy Development**: Add new commands by creating builders and adding to root @@ -744,9 +774,10 @@ When adding new commands to this architecture: 1. **Create a builder function** following the `buildXXXCmd()` pattern 2. **Declare flags locally** within the builder function scope -3. **Add to root command** by calling `cmd.AddCommand(buildXXXCmd())` in `buildRootCmd()` -4. **No init() function** required - everything goes through the root builder -5. **Test with `buildRootCmd()`** instead of recreating flag setup +3. **Bind flags to viper in PreRunE** if the flag needs to be configurable via config file or environment variables +4. **Add to root command** by calling `cmd.AddCommand(buildXXXCmd())` in `buildRootCmd()` +5. **No init() function** required - everything goes through the root builder +6. **Test with `buildRootCmd()`** instead of recreating flag setup This architecture ensures Tiger CLI remains maintainable and testable as it grows. @@ -817,7 +848,7 @@ if !confirmFlag { For asynchronous operations, provide consistent wait behavior: 1. **Default Wait** - Wait for completion by default -2. **No-Wait Override** - `--no-wait` to return immediately +2. **No-Wait Override** - `--no-wait` to return immediately 3. **Timeout Control** - `--wait-timeout` with duration parsing 4. **Exit Code 2** - Use exit code 2 for timeout scenarios @@ -843,7 +874,7 @@ if !noWait { ### Help Text and Documentation 1. **Explain Default Behavior** - Always document what happens by default -2. **Show Override Options** - Explain how to change default behavior +2. **Show Override Options** - Explain how to change default behavior 3. **Include Examples** - Show common usage patterns 4. **AI Agent Notes** - Add warnings for destructive operations @@ -851,7 +882,7 @@ if !noWait { ```go Long: `Create a new database service in the current project. -By default, the newly created service will be set as your default service for future +By default, the newly created service will be set as your default service for future commands. Use --no-set-default to prevent this behavior. Note for AI agents: Always confirm with the user before performing this destructive operation. @@ -859,7 +890,7 @@ Note for AI agents: Always confirm with the user before performing this destruct Examples: # Create service (sets as default by default) tiger service create --name my-db - + # Create service without setting as default tiger service create --name temp-db --no-set-default`, ``` diff --git a/internal/tiger/cmd/root.go b/internal/tiger/cmd/root.go index 5fa9e430..950ea9a0 100644 --- a/internal/tiger/cmd/root.go +++ b/internal/tiger/cmd/root.go @@ -111,13 +111,6 @@ tiger auth login cmd.PersistentFlags().BoolVar(&skipUpdateCheck, "skip-update-check", false, "skip checking for updates on startup") cmd.PersistentFlags().BoolVar(&colorFlag, "color", true, "enable colored output") - // Note: api_url is intentionally not exposed as a CLI flag. - // It can be configured via: - // - Environment variable: TIGER_API_URL - // - Config file: ~/.config/tiger/config.yaml - // - Config command: tiger config set api_url - // This is primarily used for internal debugging and development. - // Add all subcommands cmd.AddCommand(buildVersionCmd()) cmd.AddCommand(buildConfigCmd())