Skip to content

Commit

Permalink
add OptionFlagSlice, refactor, remove duplicated code
Browse files Browse the repository at this point in the history
  • Loading branch information
phm07 committed Jun 5, 2024
1 parent ce0e5d3 commit 7279a4f
Show file tree
Hide file tree
Showing 11 changed files with 79 additions and 88 deletions.
18 changes: 2 additions & 16 deletions internal/cmd/config/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)")
Expand Down
12 changes: 2 additions & 10 deletions internal/cmd/config/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/config/helptext/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/config/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
18 changes: 2 additions & 16 deletions internal/cmd/config/remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)")
Expand Down
18 changes: 2 additions & 16 deletions internal/cmd/config/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)")
Expand Down
12 changes: 1 addition & 11 deletions internal/cmd/config/unset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 21 additions & 1 deletion internal/cmd/config/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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
}
25 changes: 10 additions & 15 deletions internal/state/config/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package config

import (
"fmt"
"reflect"
"strconv"
"strings"
"time"
Expand All @@ -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
)
Expand All @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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 != "" {
Expand All @@ -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 != "" {
Expand All @@ -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 != "" {
Expand All @@ -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) {
Expand Down
36 changes: 36 additions & 0 deletions internal/state/config/options_test.go
Original file line number Diff line number Diff line change
@@ -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))
}
2 changes: 1 addition & 1 deletion internal/state/config/preferences.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down

0 comments on commit 7279a4f

Please sign in to comment.