Skip to content

Commit 6988f3a

Browse files
Improve output flag binding (#117)
1 parent dc9c8fe commit 6988f3a

7 files changed

Lines changed: 129 additions & 113 deletions

File tree

CLAUDE.md

Lines changed: 84 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -439,20 +439,20 @@ RunE: func(cmd *cobra.Command, args []string) error {
439439
if len(args) < 1 {
440440
return fmt.Errorf("service ID is required")
441441
}
442-
442+
443443
// 2. Set SilenceUsage = true after argument validation
444444
cmd.SilenceUsage = true
445-
445+
446446
// 3. Proceed with business logic - errors here don't show usage
447447
if err := someAPICall(); err != nil {
448448
return fmt.Errorf("operation failed: %w", err)
449449
}
450-
450+
451451
return nil
452452
},
453453
```
454454

455-
**Philosophy**:
455+
**Philosophy**:
456456
- Early argument/syntax errors → show usage (helps users learn command syntax)
457457
- Operational errors after arguments are validated → don't show usage (avoids cluttering output with irrelevant usage info)
458458

@@ -465,7 +465,7 @@ Tiger CLI uses a pure functional builder pattern with **zero global command stat
465465
### Philosophy
466466

467467
- **No global variables** - All commands, flags, and state are locally scoped
468-
- **Functional builders** - Every command is built by a dedicated function
468+
- **Functional builders** - Every command is built by a dedicated function
469469
- **Complete tree building** - `buildRootCmd()` constructs the entire CLI structure
470470
- **Perfect test isolation** - Each test gets completely fresh command instances
471471
- **Self-contained commands** - All dependencies passed explicitly via parameters
@@ -513,48 +513,42 @@ func buildRootCmd() *cobra.Command {
513513
// Declare ALL flag variables locally within this function
514514
var configDir string
515515
var debug bool
516-
var serviceID string
517-
var analytics bool
518-
var passwordStorage string
516+
// ... other flag variables
519517

520518
cmd := &cobra.Command{
521519
Use: "tiger",
522520
Short: "Tiger CLI - Tiger Cloud Platform command-line interface",
523521
Long: `Complete CLI description...`,
524522
PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
525-
// Use local flag variables in scope
526-
if err := logging.Init(debug); err != nil {
527-
return fmt.Errorf("failed to initialize logging: %w", err)
523+
// Bind persistent flags to viper at execution time
524+
if err := errors.Join(
525+
viper.BindPFlag("debug", cmd.Flags().Lookup("debug")),
526+
// ... bind remaining flags
527+
); err != nil {
528+
return fmt.Errorf("failed to bind flags: %w", err)
528529
}
530+
531+
// Setup configuration and initialize logging
529532
// ... rest of initialization
530-
return nil
531533
},
532534
}
533535

534-
// Set up configuration and flags...
535-
cobra.OnInitialize(initConfigFunc)
536+
// Set up persistent flags
536537
cmd.PersistentFlags().StringVar(&configDir, "config-dir", config.GetDefaultConfigDir(), "config directory")
537538
cmd.PersistentFlags().BoolVar(&debug, "debug", false, "enable debug logging")
538-
cmd.PersistentFlags().StringVar(&serviceID, "service-id", "", "service ID")
539-
cmd.PersistentFlags().BoolVar(&analytics, "analytics", true, "enable/disable usage analytics")
540-
cmd.PersistentFlags().StringVar(&passwordStorage, "password-storage", config.DefaultPasswordStorage, "password storage method (keyring, pgpass, none)")
541-
542-
// Bind flags to viper
543-
viper.BindPFlag("debug", cmd.PersistentFlags().Lookup("debug"))
544-
// ... bind remaining flags
539+
// ... add remaining persistent flags
545540

546541
// Add all subcommands (complete tree building)
547542
cmd.AddCommand(buildVersionCmd())
548543
cmd.AddCommand(buildConfigCmd())
549-
cmd.AddCommand(buildAuthCmd())
550-
cmd.AddCommand(buildServiceCmd())
551-
cmd.AddCommand(buildDbCmd())
552-
cmd.AddCommand(buildMCPCmd())
544+
// ... add remaining subcommands
553545

554546
return cmd
555547
}
556548
```
557549

550+
See `internal/tiger/cmd/root.go` for the complete implementation.
551+
558552
### Simple Command Pattern
559553

560554
For commands without flags:
@@ -563,7 +557,7 @@ For commands without flags:
563557
func buildVersionCmd() *cobra.Command {
564558
return &cobra.Command{
565559
Use: "version",
566-
Short: "Show version information",
560+
Short: "Show version information",
567561
Long: `Display version, build time, and git commit information.`,
568562
Run: func(cmd *cobra.Command, args []string) {
569563
fmt.Printf("Tiger CLI %s\n", Version)
@@ -583,33 +577,69 @@ func buildMyFlaggedCmd() *cobra.Command {
583577
var myFlag string
584578
var enableFeature bool
585579
var retryCount int
586-
580+
587581
cmd := &cobra.Command{
588582
Use: "my-command",
589583
Short: "Command with local flags",
590584
RunE: func(cmd *cobra.Command, args []string) error {
591585
if len(args) < 1 {
592586
return fmt.Errorf("argument required")
593587
}
594-
588+
595589
cmd.SilenceUsage = true
596-
590+
597591
// Use flag variables (they're in scope)
598-
fmt.Printf("Flag: %s, Feature: %t, Retries: %d\n",
592+
fmt.Printf("Flag: %s, Feature: %t, Retries: %d\n",
599593
myFlag, enableFeature, retryCount)
600594
return nil
601595
},
602596
}
603-
597+
604598
// Add flags - bound to local variables
605-
cmd.Flags().StringVar(&myFlag, "flag", "", "My flag description")
599+
cmd.Flags().StringVar(&myFlag, "flag", "", "My flag description")
606600
cmd.Flags().BoolVar(&enableFeature, "enable", false, "Enable feature")
607601
cmd.Flags().IntVar(&retryCount, "retries", 3, "Retry count")
608-
602+
609603
return cmd
610604
}
611605
```
612606

607+
### Commands with Flags That Need Viper Binding
608+
609+
For commands that need their flags bound to viper for configuration precedence (flag > env > config > default), use the `bindFlags()` helper:
610+
611+
```go
612+
func buildMyConfigurableFlagCmd() *cobra.Command {
613+
var output string
614+
615+
cmd := &cobra.Command{
616+
Use: "my-command",
617+
Short: "Command with configurable flag",
618+
PreRunE: bindFlags("output"), // Binds flag to viper
619+
RunE: func(cmd *cobra.Command, args []string) error {
620+
cmd.SilenceUsage = true
621+
cfg, err := config.Load() // Now includes bound flag value
622+
// ... use cfg.Output which respects: flag > env > config > default
623+
},
624+
}
625+
626+
cmd.Flags().VarP((*outputFlag)(&output), "output", "o", "output format")
627+
return cmd
628+
}
629+
```
630+
631+
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")`.
632+
633+
**Why bind flags in PreRunE?**
634+
635+
Flags must be bound to viper at **execution time**, not at **build time**, for two critical reasons:
636+
637+
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.
638+
639+
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.
640+
641+
**Note:** Use `PreRunE` for command-specific flags, and `PersistentPreRunE` for persistent flags on the root command that apply to all subcommands.
642+
613643
### Parent Commands with Subcommands
614644

615645
For commands that contain subcommands, build the complete tree:
@@ -621,12 +651,12 @@ func buildParentCmd() *cobra.Command {
621651
Short: "Parent command with subcommands",
622652
Long: `Parent command containing multiple subcommands.`,
623653
}
624-
654+
625655
// Add all subcommands (builds complete subtree)
626656
cmd.AddCommand(buildChild1Cmd())
627657
cmd.AddCommand(buildChild2Cmd())
628658
cmd.AddCommand(buildChild3Cmd())
629-
659+
630660
return cmd
631661
}
632662
```
@@ -639,7 +669,7 @@ The main application uses a single builder call:
639669
func Execute() {
640670
// Build complete command tree fresh each time
641671
rootCmd := buildRootCmd()
642-
672+
643673
err := rootCmd.Execute()
644674
if err != nil {
645675
if exitErr, ok := err.(interface{ ExitCode() int }); ok {
@@ -673,24 +703,24 @@ Tests use the full root command builder:
673703
func executeCommand(args ...string) (string, error) {
674704
// Build complete CLI fresh for each test
675705
rootCmd := buildRootCmd()
676-
706+
677707
buf := new(bytes.Buffer)
678-
rootCmd.SetOut(buf)
708+
rootCmd.SetOut(buf)
679709
rootCmd.SetErr(buf)
680710
rootCmd.SetArgs(args)
681-
711+
682712
err := rootCmd.Execute()
683713
return buf.String(), err
684714
}
685715

686716
func TestMyCommand(t *testing.T) {
687717
// Each test gets completely fresh CLI instance
688718
output, err := executeCommand("my-command", "--flag", "value")
689-
719+
690720
if err != nil {
691721
t.Fatalf("Command failed: %v", err)
692722
}
693-
723+
694724
if !strings.Contains(output, "expected") {
695725
t.Errorf("Expected 'expected' in output: %s", output)
696726
}
@@ -704,22 +734,22 @@ For tests that need to verify flag values:
704734
```go
705735
func executeAndReturnRoot(args ...string) (*cobra.Command, string, error) {
706736
rootCmd := buildRootCmd()
707-
737+
708738
buf := new(bytes.Buffer)
709739
rootCmd.SetOut(buf)
710740
rootCmd.SetArgs(args)
711-
741+
712742
err := rootCmd.Execute()
713743
return rootCmd, buf.String(), err
714744
}
715745

716746
func TestFlagValues(t *testing.T) {
717747
rootCmd, output, err := executeAndReturnRoot("service", "create", "--name", "test")
718-
748+
719749
// Navigate to specific command
720750
serviceCmd, _, _ := rootCmd.Find([]string{"service"})
721751
createCmd, _, _ := serviceCmd.Find([]string{"create"})
722-
752+
723753
// Check flag value
724754
nameFlag := createCmd.Flags().Lookup("name")
725755
if nameFlag.Value.String() != "test" {
@@ -731,7 +761,7 @@ func TestFlagValues(t *testing.T) {
731761
### Benefits of This Architecture
732762

733763
1. **Zero Global State**: No shared variables between commands or tests
734-
2. **Perfect Test Isolation**: Each test builds completely fresh command trees
764+
2. **Perfect Test Isolation**: Each test builds completely fresh command trees
735765
3. **Simplified Initialization**: Single entry point builds everything
736766
4. **Maintainable Code**: No complex global variable management
737767
5. **Easy Development**: Add new commands by creating builders and adding to root
@@ -744,9 +774,10 @@ When adding new commands to this architecture:
744774

745775
1. **Create a builder function** following the `buildXXXCmd()` pattern
746776
2. **Declare flags locally** within the builder function scope
747-
3. **Add to root command** by calling `cmd.AddCommand(buildXXXCmd())` in `buildRootCmd()`
748-
4. **No init() function** required - everything goes through the root builder
749-
5. **Test with `buildRootCmd()`** instead of recreating flag setup
777+
3. **Bind flags to viper in PreRunE** if the flag needs to be configurable via config file or environment variables
778+
4. **Add to root command** by calling `cmd.AddCommand(buildXXXCmd())` in `buildRootCmd()`
779+
5. **No init() function** required - everything goes through the root builder
780+
6. **Test with `buildRootCmd()`** instead of recreating flag setup
750781

751782
This architecture ensures Tiger CLI remains maintainable and testable as it grows.
752783

@@ -817,7 +848,7 @@ if !confirmFlag {
817848
For asynchronous operations, provide consistent wait behavior:
818849

819850
1. **Default Wait** - Wait for completion by default
820-
2. **No-Wait Override** - `--no-wait` to return immediately
851+
2. **No-Wait Override** - `--no-wait` to return immediately
821852
3. **Timeout Control** - `--wait-timeout` with duration parsing
822853
4. **Exit Code 2** - Use exit code 2 for timeout scenarios
823854

@@ -843,23 +874,23 @@ if !noWait {
843874
### Help Text and Documentation
844875

845876
1. **Explain Default Behavior** - Always document what happens by default
846-
2. **Show Override Options** - Explain how to change default behavior
877+
2. **Show Override Options** - Explain how to change default behavior
847878
3. **Include Examples** - Show common usage patterns
848879
4. **AI Agent Notes** - Add warnings for destructive operations
849880

850881
**Example:**
851882
```go
852883
Long: `Create a new database service in the current project.
853884
854-
By default, the newly created service will be set as your default service for future
885+
By default, the newly created service will be set as your default service for future
855886
commands. Use --no-set-default to prevent this behavior.
856887
857888
Note for AI agents: Always confirm with the user before performing this destructive operation.
858889
859890
Examples:
860891
# Create service (sets as default by default)
861892
tiger service create --name my-db
862-
893+
863894
# Create service without setting as default
864895
tiger service create --name temp-db --no-set-default`,
865896
```

internal/tiger/cmd/auth.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ func buildStatusCmd() *cobra.Command {
174174
Long: "Displays whether you are logged in and shows your currently configured project ID.",
175175
Args: cobra.NoArgs,
176176
ValidArgsFunction: cobra.NoFileCompletions,
177+
PreRunE: bindFlags("output"),
177178
RunE: func(cmd *cobra.Command, args []string) error {
178179
cmd.SilenceUsage = true
179180

@@ -183,11 +184,6 @@ func buildStatusCmd() *cobra.Command {
183184
return fmt.Errorf("failed to load config: %w", err)
184185
}
185186

186-
// Use flag value if provided, otherwise use config value
187-
if cmd.Flags().Changed("output") {
188-
cfg.Output = output
189-
}
190-
191187
apiKey, _, err := config.GetCredentials()
192188
if err != nil {
193189
return err

internal/tiger/cmd/config.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ func buildConfigShowCmd() *cobra.Command {
2727
Long: `Display the current CLI configuration settings`,
2828
Args: cobra.NoArgs,
2929
ValidArgsFunction: cobra.NoFileCompletions,
30+
PreRunE: bindFlags("output"),
3031
RunE: func(cmd *cobra.Command, args []string) error {
3132
cmd.SilenceUsage = true
3233

@@ -35,12 +36,6 @@ func buildConfigShowCmd() *cobra.Command {
3536
return fmt.Errorf("failed to load config: %w", err)
3637
}
3738

38-
// Use flag value if provided, otherwise use config value
39-
outputFormat := cfg.Output
40-
if cmd.Flags().Changed("output") {
41-
outputFormat = output
42-
}
43-
4439
configFile, err := cfg.EnsureConfigDir()
4540
if err != nil {
4641
return err
@@ -69,7 +64,7 @@ func buildConfigShowCmd() *cobra.Command {
6964
}
7065

7166
output := cmd.OutOrStdout()
72-
switch outputFormat {
67+
switch cfg.Output {
7368
case "json":
7469
return util.SerializeToJSON(output, cfgOut)
7570
case "yaml":

internal/tiger/cmd/db.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,7 @@ PostgreSQL Configuration Parameters That May Be Set:
642642
(kills queries that exceed the specified duration, in milliseconds)`,
643643
Args: cobra.MaximumNArgs(1),
644644
ValidArgsFunction: serviceIDCompletion,
645+
PreRunE: bindFlags("output"),
645646
RunE: func(cmd *cobra.Command, args []string) error {
646647
// Validate arguments
647648
if roleName == "" {
@@ -656,11 +657,6 @@ PostgreSQL Configuration Parameters That May Be Set:
656657
return fmt.Errorf("failed to load config: %w", err)
657658
}
658659

659-
// Use flag value if provided, otherwise use config value
660-
if cmd.Flags().Changed("output") {
661-
cfg.Output = output
662-
}
663-
664660
// Get password
665661
rolePassword, err := getPasswordForRole(passwordFlag)
666662
if err != nil {

0 commit comments

Comments
 (0)