diff --git a/internal/cmd/config/add.go b/internal/cmd/config/add.go index adeeac4b..d7c9b2b0 100644 --- a/internal/cmd/config/add.go +++ b/internal/cmd/config/add.go @@ -24,22 +24,8 @@ func NewAddCommand(s state.State) *cobra.Command { SilenceUsage: true, RunE: state.Wrap(s, runAdd), ValidArgsFunction: cmpl.NoFileCompletion(cmpl.SuggestArgs( - cmpl.SuggestCandidatesF(func() []string { - var keys []string - for key, opt := range config.Options { - if opt.IsSlice() && opt.HasFlag(config.OptionFlagPreference) { - keys = append(keys, key) - } - } - return keys - }), - cmpl.SuggestCandidatesCtx(func(_ *cobra.Command, args []string) []string { - var comps []string - if opt, ok := config.Options[args[0]]; ok { - comps = opt.Completions() - } - return comps - }), + cmpl.SuggestCandidates(getOptionNames(config.OptionFlagPreference|config.OptionFlagSlice)...), + cmpl.SuggestCandidatesCtx(suggestOptionCompletions), )), } cmd.Flags().Bool("global", false, "Set the value globally (for all contexts)") diff --git a/internal/cmd/config/get.go b/internal/cmd/config/get.go index 8f3ca184..bbee9d7b 100644 --- a/internal/cmd/config/get.go +++ b/internal/cmd/config/get.go @@ -21,15 +21,7 @@ func NewGetCommand(s state.State) *cobra.Command { DisableFlagsInUseLine: true, SilenceUsage: true, RunE: state.Wrap(s, runGet), - ValidArgsFunction: cmpl.NoFileCompletion( - cmpl.SuggestCandidatesF(func() []string { - var keys []string - for key := range config.Options { - keys = append(keys, key) - } - return keys - }), - ), + ValidArgsFunction: cmpl.NoFileCompletion(cmpl.SuggestCandidates(getOptionNames(0)...)), } cmd.Flags().Bool("global", false, "Get the value globally") cmd.Flags().Bool("allow-sensitive", false, "Allow showing sensitive values") @@ -53,7 +45,7 @@ func runGet(s state.State, cmd *cobra.Command, args []string) error { } val := opt.GetAsAny(s.Config()) - if opt.HasFlag(config.OptionFlagSensitive) && !allowSensitive { + if opt.HasFlags(config.OptionFlagSensitive) && !allowSensitive { return fmt.Errorf("'%s' is sensitive. use --allow-sensitive to show the value", key) } cmd.Println(val) diff --git a/internal/cmd/config/helptext/generate.go b/internal/cmd/config/helptext/generate.go index cfda083f..07f2d31d 100644 --- a/internal/cmd/config/helptext/generate.go +++ b/internal/cmd/config/helptext/generate.go @@ -44,7 +44,7 @@ func generateTable(outFile string, filterFlag config.OptionFlag, hasFlag bool) { var opts []config.IOption for _, opt := range config.Options { - if opt.HasFlag(filterFlag) != hasFlag { + if opt.HasFlags(filterFlag) != hasFlag { continue } opts = append(opts, opt) diff --git a/internal/cmd/config/list.go b/internal/cmd/config/list.go index f6deaa16..8b49361b 100644 --- a/internal/cmd/config/list.go +++ b/internal/cmd/config/list.go @@ -52,7 +52,7 @@ func runList(s state.State, cmd *cobra.Command, _ []string) error { var options []option for name, opt := range config.Options { val := opt.GetAsAny(s.Config()) - if opt.HasFlag(config.OptionFlagSensitive) && !allowSensitive { + if opt.HasFlags(config.OptionFlagSensitive) && !allowSensitive { val = "[redacted]" } if !all && !opt.Changed(s.Config()) { diff --git a/internal/cmd/config/remove.go b/internal/cmd/config/remove.go index e908d248..fab0a84f 100644 --- a/internal/cmd/config/remove.go +++ b/internal/cmd/config/remove.go @@ -24,22 +24,8 @@ func NewRemoveCommand(s state.State) *cobra.Command { SilenceUsage: true, RunE: state.Wrap(s, runRemove), ValidArgsFunction: cmpl.NoFileCompletion(cmpl.SuggestArgs( - cmpl.SuggestCandidatesF(func() []string { - var keys []string - for key, opt := range config.Options { - if opt.IsSlice() && opt.HasFlag(config.OptionFlagPreference) { - keys = append(keys, key) - } - } - return keys - }), - cmpl.SuggestCandidatesCtx(func(_ *cobra.Command, args []string) []string { - var comps []string - if opt, ok := config.Options[args[0]]; ok { - comps = opt.Completions() - } - return comps - }), + cmpl.SuggestCandidates(getOptionNames(config.OptionFlagPreference|config.OptionFlagSlice)...), + cmpl.SuggestCandidatesCtx(suggestOptionCompletions), )), } cmd.Flags().Bool("global", false, "Remove the value(s) globally (for all contexts)") diff --git a/internal/cmd/config/set.go b/internal/cmd/config/set.go index 4b2bb09f..0e1cb531 100644 --- a/internal/cmd/config/set.go +++ b/internal/cmd/config/set.go @@ -25,22 +25,8 @@ func NewSetCommand(s state.State) *cobra.Command { SilenceUsage: true, RunE: state.Wrap(s, runSet), ValidArgsFunction: cmpl.NoFileCompletion(cmpl.SuggestArgs( - cmpl.SuggestCandidatesF(func() []string { - var keys []string - for key, opt := range config.Options { - if opt.HasFlag(config.OptionFlagPreference) { - keys = append(keys, key) - } - } - return keys - }), - cmpl.SuggestCandidatesCtx(func(_ *cobra.Command, args []string) []string { - var comps []string - if opt, ok := config.Options[args[0]]; ok { - comps = opt.Completions() - } - return comps - }), + cmpl.SuggestCandidates(getOptionNames(config.OptionFlagPreference)...), + cmpl.SuggestCandidatesCtx(suggestOptionCompletions), )), } cmd.Flags().Bool("global", false, "Set the value globally (for all contexts)") diff --git a/internal/cmd/config/unset.go b/internal/cmd/config/unset.go index 0a1ad2d9..f24f54b5 100644 --- a/internal/cmd/config/unset.go +++ b/internal/cmd/config/unset.go @@ -22,17 +22,7 @@ func NewUnsetCommand(s state.State) *cobra.Command { DisableFlagsInUseLine: true, SilenceUsage: true, RunE: state.Wrap(s, runUnset), - ValidArgsFunction: cmpl.NoFileCompletion(cmpl.SuggestArgs( - cmpl.SuggestCandidatesF(func() []string { - var keys []string - for key, opt := range config.Options { - if opt.HasFlag(config.OptionFlagPreference) { - keys = append(keys, key) - } - } - return keys - }), - )), + ValidArgsFunction: cmpl.NoFileCompletion(cmpl.SuggestCandidates(getOptionNames(config.OptionFlagPreference)...)), } cmd.Flags().Bool("global", false, "Unset the value globally (for all contexts)") return cmd diff --git a/internal/cmd/config/util.go b/internal/cmd/config/util.go index 6a7a1bab..afbc3ef8 100644 --- a/internal/cmd/config/util.go +++ b/internal/cmd/config/util.go @@ -3,6 +3,8 @@ package config import ( "fmt" + "github.com/spf13/cobra" + "github.com/hetznercloud/cli/internal/cmd/util" "github.com/hetznercloud/cli/internal/state/config" ) @@ -22,8 +24,26 @@ func getPreferences(cfg config.Config, global bool) (ctx config.Context, prefs c func getPreference(key string) (config.IOption, error) { opt, ok := config.Options[key] - if !ok || !opt.HasFlag(config.OptionFlagPreference) { + if !ok || !opt.HasFlags(config.OptionFlagPreference) { return nil, fmt.Errorf("unknown preference: %s", key) } return opt, nil } + +func getOptionNames(flags config.OptionFlag) []string { + var names []string + for name, opt := range config.Options { + if opt.HasFlags(flags) { + names = append(names, name) + } + } + return names +} + +func suggestOptionCompletions(_ *cobra.Command, args []string) []string { + var comps []string + if opt, ok := config.Options[args[0]]; ok { + comps = opt.Completions() + } + return comps +} diff --git a/internal/state/config/options.go b/internal/state/config/options.go index 5bd5b27d..2e2ee111 100644 --- a/internal/state/config/options.go +++ b/internal/state/config/options.go @@ -2,7 +2,6 @@ package config import ( "fmt" - "reflect" "strconv" "strings" "time" @@ -26,6 +25,8 @@ const ( OptionFlagEnv // OptionFlagSensitive indicates that the option holds sensitive data and should not be printed OptionFlagSensitive + // OptionFlagSlice indicates that the option value is a slice + OptionFlagSlice DefaultPreferenceFlags = OptionFlagPreference | OptionFlagConfig | OptionFlagPFlag | OptionFlagEnv ) @@ -43,8 +44,8 @@ type IOption interface { EnvVar() string // FlagName returns the name of the flag. If the option is not configurable via a flag, an empty string is returned FlagName() string - // HasFlag returns true if the option has the provided flag set - HasFlag(src OptionFlag) bool + // HasFlags returns true if the option has all the provided flags set + HasFlags(src OptionFlag) bool // GetAsAny reads the option value from the config and returns it as an any GetAsAny(c Config) any // OverrideAny sets the option value in the config to the provided any value @@ -53,8 +54,6 @@ type IOption interface { Changed(c Config) bool // Completions returns a list of possible completions for the option (for example for boolean options: "true", "false") Completions() []string - // IsSlice returns true if the option is a slice - IsSlice() bool // T returns an instance of the type of the option as an any T() any } @@ -192,12 +191,8 @@ func (o *Option[T]) Changed(c Config) bool { return c.Viper().IsSet(o.Name) } -func (o *Option[T]) HasFlag(src OptionFlag) bool { - return o.Flags&src != 0 -} - -func (o *Option[T]) IsSlice() bool { - return reflect.TypeOf(o.T()).Kind() == reflect.Slice +func (o *Option[T]) HasFlags(src OptionFlag) bool { + return (^o.Flags)&src == 0 } func (o *Option[T]) GetName() string { @@ -209,7 +204,7 @@ func (o *Option[T]) GetDescription() string { } func (o *Option[T]) ConfigKey() string { - if !o.HasFlag(OptionFlagConfig) { + if !o.HasFlags(OptionFlagConfig) { return "" } if o.overrides != nil && o.overrides.configKey != "" { @@ -219,7 +214,7 @@ func (o *Option[T]) ConfigKey() string { } func (o *Option[T]) EnvVar() string { - if !o.HasFlag(OptionFlagEnv) { + if !o.HasFlags(OptionFlagEnv) { return "" } if o.overrides != nil && o.overrides.envVar != "" { @@ -229,7 +224,7 @@ func (o *Option[T]) EnvVar() string { } func (o *Option[T]) FlagName() string { - if !o.HasFlag(OptionFlagPFlag) { + if !o.HasFlags(OptionFlagPFlag) { return "" } if o.overrides != nil && o.overrides.flagName != "" { @@ -253,7 +248,7 @@ func (o *Option[T]) T() any { } func (o *Option[T]) addToFlagSet(fs *pflag.FlagSet) { - if !o.HasFlag(OptionFlagPFlag) { + if !o.HasFlags(OptionFlagPFlag) { return } switch v := any(o.Default).(type) { diff --git a/internal/state/config/options_test.go b/internal/state/config/options_test.go new file mode 100644 index 00000000..3b017c68 --- /dev/null +++ b/internal/state/config/options_test.go @@ -0,0 +1,36 @@ +package config + +import ( + "reflect" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestOptions(t *testing.T) { + for _, opt := range Options { + kind := reflect.TypeOf(opt.T()).Kind() + if kind == reflect.Slice && !opt.HasFlags(OptionFlagSlice) { + t.Errorf("option %s is a slice but does not have the slice flag", opt.GetName()) + } + if kind != reflect.Slice && opt.HasFlags(OptionFlagSlice) { + t.Errorf("option %s is not a slice but has the slice flag", opt.GetName()) + } + if opt.HasFlags(OptionFlagPFlag | OptionFlagSensitive) { + t.Errorf("%s: sensitive options shouldn't have pflags", opt.GetName()) + } + } +} + +func TestOption_HasFlags(t *testing.T) { + opt := &Option[any]{Flags: OptionFlagSensitive | OptionFlagPFlag | OptionFlagSlice} + assert.True(t, opt.HasFlags(OptionFlagSensitive)) + assert.True(t, opt.HasFlags(OptionFlagPFlag)) + assert.True(t, opt.HasFlags(OptionFlagSlice)) + assert.True(t, opt.HasFlags(OptionFlagSensitive|OptionFlagPFlag)) + assert.True(t, opt.HasFlags(OptionFlagSensitive|OptionFlagSlice)) + assert.True(t, opt.HasFlags(OptionFlagPFlag|OptionFlagSlice)) + assert.True(t, opt.HasFlags(OptionFlagSensitive|OptionFlagPFlag|OptionFlagSlice)) + assert.False(t, opt.HasFlags(OptionFlagConfig)) + assert.False(t, opt.HasFlags(OptionFlagConfig|OptionFlagSensitive)) +} diff --git a/internal/state/config/preferences.go b/internal/state/config/preferences.go index 674bc5bc..3633d563 100644 --- a/internal/state/config/preferences.go +++ b/internal/state/config/preferences.go @@ -105,7 +105,7 @@ func validate(m map[string]any, prefix string) error { continue } opt, ok := Options[key] - if !ok || !opt.HasFlag(OptionFlagPreference) { + if !ok || !opt.HasFlags(OptionFlagPreference) { return fmt.Errorf("unknown preference: %s", key) } }