Skip to content

Commit 232286b

Browse files
Introduce introduce helper to add placeholders to all flags (#509)
Adds `setAllFlagPlaceholders` helper, to be used in #510. This helper allows you to annotate all local flags with a placeholder in a single function call. It panics if: - You try to annotate a flag that does not exist - You do not annotate all flags These panics happen during command _construction_. So this validation is caught at dev / test time. Not intended to be a real runtime check. Commit 2 of 3 for GROW-2511 GitOrigin-RevId: 216bdbdc51fa7c4ddf144905f9962243f2b910e5
1 parent c3225d1 commit 232286b

2 files changed

Lines changed: 160 additions & 6 deletions

File tree

cmd/flagusages.go

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/charmbracelet/lipgloss"
99
"github.com/render-oss/cli/pkg/style"
10+
"github.com/spf13/cobra"
1011
"github.com/spf13/pflag"
1112
)
1213

@@ -29,9 +30,47 @@ func setAnnotationBestEffort(flags *pflag.FlagSet, flagName, key string, values
2930
}
3031

3132
// setFlagPlaceholder applies help-output placeholder metadata to a flag.
32-
// Returns true when the placeholder was applied successfully, false otherwise.
33-
func setFlagPlaceholder(flags *pflag.FlagSet, flagName, placeholder string) bool {
34-
return setAnnotationBestEffort(flags, flagName, flagPlaceholderAnnotation, []string{placeholder})
33+
// Panics when called with an invalid flag name so command wiring mistakes (e.g., wrong flag name) fail loudly in dev and test
34+
func setFlagPlaceholder(flags *pflag.FlagSet, flagName, placeholder string) {
35+
if flags == nil {
36+
panic("setFlagPlaceholder called with nil flag set")
37+
}
38+
if flags.Lookup(flagName) == nil {
39+
panic(fmt.Sprintf("setFlagPlaceholder called with unknown flag %q", flagName))
40+
}
41+
_ = flags.SetAnnotation(flagName, flagPlaceholderAnnotation, []string{placeholder})
42+
}
43+
44+
// setAllFlagPlaceholders applies help-output placeholder metadata to every local value flag.
45+
// Panics when any visible value flag is missing a placeholder or any placeholder names an invalid flag.
46+
// These panics happen during command construction, so they are intended as a dev and test aid.
47+
func setAllFlagPlaceholders(cmd *cobra.Command, placeholders map[string]string) {
48+
if cmd == nil {
49+
panic("setAllFlagPlaceholders called with nil command")
50+
}
51+
if len(placeholders) == 0 {
52+
panic("setAllFlagPlaceholders called with no placeholders")
53+
}
54+
for flagName, placeholder := range placeholders {
55+
setFlagPlaceholder(cmd.Flags(), flagName, placeholder)
56+
}
57+
cmd.LocalFlags().VisitAll(func(flag *pflag.Flag) {
58+
if flag.Hidden || isBoolFlag(flag) {
59+
return
60+
}
61+
if _, ok := placeholders[flag.Name]; !ok {
62+
panic(fmt.Sprintf("setAllFlagPlaceholders missing placeholder for flag %q", flag.Name))
63+
}
64+
})
65+
}
66+
67+
func isBoolFlag(flag *pflag.Flag) bool {
68+
switch flag.Value.Type() {
69+
case "bool", "boolfunc":
70+
return true
71+
default:
72+
return false
73+
}
3574
}
3675

3776
func placeholderFromAnnotation(flag *pflag.Flag) (string, bool) {

cmd/root_test.go

Lines changed: 118 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ func TestRootHelpOmitsEmptyGroupHeaders(t *testing.T) {
402402
func TestGetDescriptiveTypeNameUsesAnnotationWhenPresent(t *testing.T) {
403403
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
404404
flags.String("region", "", "Filter by region.")
405-
require.True(t, setFlagPlaceholder(flags, "region", "REGION"))
405+
setFlagPlaceholder(flags, "region", "REGION")
406406

407407
require.Equal(t, "REGION", getDescriptiveTypeName(flags.Lookup("region"), "string"))
408408
require.Equal(t, "string", getDescriptiveTypeName(flags.Lookup("missing"), "string"))
@@ -426,6 +426,96 @@ func TestServicesEnvironmentIDsFlagHasPlaceholderAnnotation(t *testing.T) {
426426
require.Equal(t, placeholderEnvIDs, placeholder)
427427
}
428428

429+
func requireFlagPlaceholder(t *testing.T, flags *pflag.FlagSet, flagName, expected string) {
430+
t.Helper()
431+
432+
require.NotNil(t, flags)
433+
flag := flags.Lookup(flagName)
434+
require.NotNil(t, flag)
435+
placeholder, ok := placeholderFromAnnotation(flag)
436+
require.True(t, ok)
437+
require.Equal(t, expected, placeholder)
438+
}
439+
440+
func TestSetAllFlagPlaceholders(t *testing.T) {
441+
t.Run("applies placeholders for all value flags", func(t *testing.T) {
442+
cmd := &cobra.Command{Use: "test"}
443+
cmd.Flags().String("region", "", "region")
444+
cmd.Flags().String("plan", "", "plan")
445+
446+
setAllFlagPlaceholders(cmd, map[string]string{
447+
"region": "REGION",
448+
"plan": "PLAN",
449+
})
450+
451+
requireFlagPlaceholder(t, cmd.Flags(), "region", "REGION")
452+
requireFlagPlaceholder(t, cmd.Flags(), "plan", "PLAN")
453+
})
454+
455+
t.Run("requires placeholders for local value flags only", func(t *testing.T) {
456+
root := &cobra.Command{Use: "root"}
457+
root.PersistentFlags().String("output", "", "output")
458+
cmd := &cobra.Command{Use: "test"}
459+
cmd.Flags().String("region", "", "region")
460+
cmd.Flags().Bool("confirm", false, "confirm")
461+
cmd.Flags().String("hidden", "", "hidden")
462+
require.NoError(t, cmd.Flags().MarkHidden("hidden"))
463+
root.AddCommand(cmd)
464+
465+
setAllFlagPlaceholders(cmd, map[string]string{
466+
"region": "REGION",
467+
})
468+
469+
requireFlagPlaceholder(t, cmd.Flags(), "region", "REGION")
470+
})
471+
472+
t.Run("panics when a value flag is missing a placeholder", func(t *testing.T) {
473+
cmd := &cobra.Command{Use: "test"}
474+
cmd.Flags().String("region", "", "region")
475+
cmd.Flags().String("plan", "", "plan")
476+
477+
require.Panics(t, func() {
478+
setAllFlagPlaceholders(cmd, map[string]string{
479+
"region": "REGION",
480+
})
481+
})
482+
})
483+
484+
t.Run("panics when any flag does not exist", func(t *testing.T) {
485+
cmd := &cobra.Command{Use: "test"}
486+
487+
require.Panics(t, func() {
488+
setAllFlagPlaceholders(cmd, map[string]string{
489+
"missing": "MISSING",
490+
})
491+
})
492+
})
493+
494+
t.Run("panics for empty placeholder set", func(t *testing.T) {
495+
cmd := &cobra.Command{Use: "test"}
496+
497+
require.Panics(t, func() {
498+
setAllFlagPlaceholders(cmd, map[string]string{})
499+
})
500+
})
501+
502+
t.Run("panics for nil placeholder set", func(t *testing.T) {
503+
cmd := &cobra.Command{Use: "test"}
504+
505+
require.Panics(t, func() {
506+
setAllFlagPlaceholders(cmd, nil)
507+
})
508+
})
509+
510+
t.Run("panics for nil command", func(t *testing.T) {
511+
require.Panics(t, func() {
512+
setAllFlagPlaceholders(nil, map[string]string{
513+
"region": "REGION",
514+
})
515+
})
516+
})
517+
}
518+
429519
func TestSetAnnotationBestEffort(t *testing.T) {
430520
t.Run("returns true for existing flag", func(t *testing.T) {
431521
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
@@ -440,13 +530,38 @@ func TestSetAnnotationBestEffort(t *testing.T) {
440530
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
441531

442532
require.NotPanics(t, func() {
443-
ok := setFlagPlaceholder(flags, "missing", "FORMAT")
533+
ok := setAnnotationBestEffort(flags, "missing", flagPlaceholderAnnotation, []string{"FORMAT"})
444534
require.False(t, ok)
445535
})
446536
})
447537

448538
t.Run("returns false for nil flagset", func(t *testing.T) {
449-
require.False(t, setFlagPlaceholder(nil, "output", "FORMAT"))
539+
require.False(t, setAnnotationBestEffort(nil, "output", flagPlaceholderAnnotation, []string{"FORMAT"}))
540+
})
541+
}
542+
543+
func TestSetFlagPlaceholder(t *testing.T) {
544+
t.Run("applies placeholder to existing flag", func(t *testing.T) {
545+
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
546+
flags.String("output", "", "output format")
547+
548+
setFlagPlaceholder(flags, "output", "FORMAT")
549+
550+
requireFlagPlaceholder(t, flags, "output", "FORMAT")
551+
})
552+
553+
t.Run("panics for missing flag", func(t *testing.T) {
554+
flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
555+
556+
require.Panics(t, func() {
557+
setFlagPlaceholder(flags, "missing", "FORMAT")
558+
})
559+
})
560+
561+
t.Run("panics for nil flagset", func(t *testing.T) {
562+
require.Panics(t, func() {
563+
setFlagPlaceholder(nil, "output", "FORMAT")
564+
})
450565
})
451566
}
452567

0 commit comments

Comments
 (0)