Skip to content

Commit 7279a4f

Browse files
phm07phm07
committed
add OptionFlagSlice, refactor, remove duplicated code
1 parent ce0e5d3 commit 7279a4f

File tree

11 files changed

+79
-88
lines changed

11 files changed

+79
-88
lines changed

‎internal/cmd/config/add.go

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,22 +24,8 @@ func NewAddCommand(s state.State) *cobra.Command {
2424
SilenceUsage: true,
2525
RunE: state.Wrap(s, runAdd),
2626
ValidArgsFunction: cmpl.NoFileCompletion(cmpl.SuggestArgs(
27-
cmpl.SuggestCandidatesF(func() []string {
28-
var keys []string
29-
for key, opt := range config.Options {
30-
if opt.IsSlice() && opt.HasFlag(config.OptionFlagPreference) {
31-
keys = append(keys, key)
32-
}
33-
}
34-
return keys
35-
}),
36-
cmpl.SuggestCandidatesCtx(func(_ *cobra.Command, args []string) []string {
37-
var comps []string
38-
if opt, ok := config.Options[args[0]]; ok {
39-
comps = opt.Completions()
40-
}
41-
return comps
42-
}),
27+
cmpl.SuggestCandidates(getOptionNames(config.OptionFlagPreference|config.OptionFlagSlice)...),
28+
cmpl.SuggestCandidatesCtx(suggestOptionCompletions),
4329
)),
4430
}
4531
cmd.Flags().Bool("global", false, "Set the value globally (for all contexts)")

‎internal/cmd/config/get.go

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,7 @@ func NewGetCommand(s state.State) *cobra.Command {
2121
DisableFlagsInUseLine: true,
2222
SilenceUsage: true,
2323
RunE: state.Wrap(s, runGet),
24-
ValidArgsFunction: cmpl.NoFileCompletion(
25-
cmpl.SuggestCandidatesF(func() []string {
26-
var keys []string
27-
for key := range config.Options {
28-
keys = append(keys, key)
29-
}
30-
return keys
31-
}),
32-
),
24+
ValidArgsFunction: cmpl.NoFileCompletion(cmpl.SuggestCandidates(getOptionNames(0)...)),
3325
}
3426
cmd.Flags().Bool("global", false, "Get the value globally")
3527
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 {
5345
}
5446

5547
val := opt.GetAsAny(s.Config())
56-
if opt.HasFlag(config.OptionFlagSensitive) && !allowSensitive {
48+
if opt.HasFlags(config.OptionFlagSensitive) && !allowSensitive {
5749
return fmt.Errorf("'%s' is sensitive. use --allow-sensitive to show the value", key)
5850
}
5951
cmd.Println(val)

‎internal/cmd/config/helptext/generate.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func generateTable(outFile string, filterFlag config.OptionFlag, hasFlag bool) {
4444

4545
var opts []config.IOption
4646
for _, opt := range config.Options {
47-
if opt.HasFlag(filterFlag) != hasFlag {
47+
if opt.HasFlags(filterFlag) != hasFlag {
4848
continue
4949
}
5050
opts = append(opts, opt)

‎internal/cmd/config/list.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func runList(s state.State, cmd *cobra.Command, _ []string) error {
5252
var options []option
5353
for name, opt := range config.Options {
5454
val := opt.GetAsAny(s.Config())
55-
if opt.HasFlag(config.OptionFlagSensitive) && !allowSensitive {
55+
if opt.HasFlags(config.OptionFlagSensitive) && !allowSensitive {
5656
val = "[redacted]"
5757
}
5858
if !all && !opt.Changed(s.Config()) {

‎internal/cmd/config/remove.go

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,22 +24,8 @@ func NewRemoveCommand(s state.State) *cobra.Command {
2424
SilenceUsage: true,
2525
RunE: state.Wrap(s, runRemove),
2626
ValidArgsFunction: cmpl.NoFileCompletion(cmpl.SuggestArgs(
27-
cmpl.SuggestCandidatesF(func() []string {
28-
var keys []string
29-
for key, opt := range config.Options {
30-
if opt.IsSlice() && opt.HasFlag(config.OptionFlagPreference) {
31-
keys = append(keys, key)
32-
}
33-
}
34-
return keys
35-
}),
36-
cmpl.SuggestCandidatesCtx(func(_ *cobra.Command, args []string) []string {
37-
var comps []string
38-
if opt, ok := config.Options[args[0]]; ok {
39-
comps = opt.Completions()
40-
}
41-
return comps
42-
}),
27+
cmpl.SuggestCandidates(getOptionNames(config.OptionFlagPreference|config.OptionFlagSlice)...),
28+
cmpl.SuggestCandidatesCtx(suggestOptionCompletions),
4329
)),
4430
}
4531
cmd.Flags().Bool("global", false, "Remove the value(s) globally (for all contexts)")

‎internal/cmd/config/set.go

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,8 @@ func NewSetCommand(s state.State) *cobra.Command {
2525
SilenceUsage: true,
2626
RunE: state.Wrap(s, runSet),
2727
ValidArgsFunction: cmpl.NoFileCompletion(cmpl.SuggestArgs(
28-
cmpl.SuggestCandidatesF(func() []string {
29-
var keys []string
30-
for key, opt := range config.Options {
31-
if opt.HasFlag(config.OptionFlagPreference) {
32-
keys = append(keys, key)
33-
}
34-
}
35-
return keys
36-
}),
37-
cmpl.SuggestCandidatesCtx(func(_ *cobra.Command, args []string) []string {
38-
var comps []string
39-
if opt, ok := config.Options[args[0]]; ok {
40-
comps = opt.Completions()
41-
}
42-
return comps
43-
}),
28+
cmpl.SuggestCandidates(getOptionNames(config.OptionFlagPreference)...),
29+
cmpl.SuggestCandidatesCtx(suggestOptionCompletions),
4430
)),
4531
}
4632
cmd.Flags().Bool("global", false, "Set the value globally (for all contexts)")

‎internal/cmd/config/unset.go

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,7 @@ func NewUnsetCommand(s state.State) *cobra.Command {
2222
DisableFlagsInUseLine: true,
2323
SilenceUsage: true,
2424
RunE: state.Wrap(s, runUnset),
25-
ValidArgsFunction: cmpl.NoFileCompletion(cmpl.SuggestArgs(
26-
cmpl.SuggestCandidatesF(func() []string {
27-
var keys []string
28-
for key, opt := range config.Options {
29-
if opt.HasFlag(config.OptionFlagPreference) {
30-
keys = append(keys, key)
31-
}
32-
}
33-
return keys
34-
}),
35-
)),
25+
ValidArgsFunction: cmpl.NoFileCompletion(cmpl.SuggestCandidates(getOptionNames(config.OptionFlagPreference)...)),
3626
}
3727
cmd.Flags().Bool("global", false, "Unset the value globally (for all contexts)")
3828
return cmd

‎internal/cmd/config/util.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package config
33
import (
44
"fmt"
55

6+
"github.com/spf13/cobra"
7+
68
"github.com/hetznercloud/cli/internal/cmd/util"
79
"github.com/hetznercloud/cli/internal/state/config"
810
)
@@ -22,8 +24,26 @@ func getPreferences(cfg config.Config, global bool) (ctx config.Context, prefs c
2224

2325
func getPreference(key string) (config.IOption, error) {
2426
opt, ok := config.Options[key]
25-
if !ok || !opt.HasFlag(config.OptionFlagPreference) {
27+
if !ok || !opt.HasFlags(config.OptionFlagPreference) {
2628
return nil, fmt.Errorf("unknown preference: %s", key)
2729
}
2830
return opt, nil
2931
}
32+
33+
func getOptionNames(flags config.OptionFlag) []string {
34+
var names []string
35+
for name, opt := range config.Options {
36+
if opt.HasFlags(flags) {
37+
names = append(names, name)
38+
}
39+
}
40+
return names
41+
}
42+
43+
func suggestOptionCompletions(_ *cobra.Command, args []string) []string {
44+
var comps []string
45+
if opt, ok := config.Options[args[0]]; ok {
46+
comps = opt.Completions()
47+
}
48+
return comps
49+
}

‎internal/state/config/options.go

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package config
22

33
import (
44
"fmt"
5-
"reflect"
65
"strconv"
76
"strings"
87
"time"
@@ -26,6 +25,8 @@ const (
2625
OptionFlagEnv
2726
// OptionFlagSensitive indicates that the option holds sensitive data and should not be printed
2827
OptionFlagSensitive
28+
// OptionFlagSlice indicates that the option value is a slice
29+
OptionFlagSlice
2930

3031
DefaultPreferenceFlags = OptionFlagPreference | OptionFlagConfig | OptionFlagPFlag | OptionFlagEnv
3132
)
@@ -43,8 +44,8 @@ type IOption interface {
4344
EnvVar() string
4445
// FlagName returns the name of the flag. If the option is not configurable via a flag, an empty string is returned
4546
FlagName() string
46-
// HasFlag returns true if the option has the provided flag set
47-
HasFlag(src OptionFlag) bool
47+
// HasFlags returns true if the option has all the provided flags set
48+
HasFlags(src OptionFlag) bool
4849
// GetAsAny reads the option value from the config and returns it as an any
4950
GetAsAny(c Config) any
5051
// OverrideAny sets the option value in the config to the provided any value
@@ -53,8 +54,6 @@ type IOption interface {
5354
Changed(c Config) bool
5455
// Completions returns a list of possible completions for the option (for example for boolean options: "true", "false")
5556
Completions() []string
56-
// IsSlice returns true if the option is a slice
57-
IsSlice() bool
5857
// T returns an instance of the type of the option as an any
5958
T() any
6059
}
@@ -192,12 +191,8 @@ func (o *Option[T]) Changed(c Config) bool {
192191
return c.Viper().IsSet(o.Name)
193192
}
194193

195-
func (o *Option[T]) HasFlag(src OptionFlag) bool {
196-
return o.Flags&src != 0
197-
}
198-
199-
func (o *Option[T]) IsSlice() bool {
200-
return reflect.TypeOf(o.T()).Kind() == reflect.Slice
194+
func (o *Option[T]) HasFlags(src OptionFlag) bool {
195+
return (^o.Flags)&src == 0
201196
}
202197

203198
func (o *Option[T]) GetName() string {
@@ -209,7 +204,7 @@ func (o *Option[T]) GetDescription() string {
209204
}
210205

211206
func (o *Option[T]) ConfigKey() string {
212-
if !o.HasFlag(OptionFlagConfig) {
207+
if !o.HasFlags(OptionFlagConfig) {
213208
return ""
214209
}
215210
if o.overrides != nil && o.overrides.configKey != "" {
@@ -219,7 +214,7 @@ func (o *Option[T]) ConfigKey() string {
219214
}
220215

221216
func (o *Option[T]) EnvVar() string {
222-
if !o.HasFlag(OptionFlagEnv) {
217+
if !o.HasFlags(OptionFlagEnv) {
223218
return ""
224219
}
225220
if o.overrides != nil && o.overrides.envVar != "" {
@@ -229,7 +224,7 @@ func (o *Option[T]) EnvVar() string {
229224
}
230225

231226
func (o *Option[T]) FlagName() string {
232-
if !o.HasFlag(OptionFlagPFlag) {
227+
if !o.HasFlags(OptionFlagPFlag) {
233228
return ""
234229
}
235230
if o.overrides != nil && o.overrides.flagName != "" {
@@ -253,7 +248,7 @@ func (o *Option[T]) T() any {
253248
}
254249

255250
func (o *Option[T]) addToFlagSet(fs *pflag.FlagSet) {
256-
if !o.HasFlag(OptionFlagPFlag) {
251+
if !o.HasFlags(OptionFlagPFlag) {
257252
return
258253
}
259254
switch v := any(o.Default).(type) {

‎internal/state/config/options_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
package config
2+
3+
import (
4+
"reflect"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
)
9+
10+
func TestOptions(t *testing.T) {
11+
for _, opt := range Options {
12+
kind := reflect.TypeOf(opt.T()).Kind()
13+
if kind == reflect.Slice && !opt.HasFlags(OptionFlagSlice) {
14+
t.Errorf("option %s is a slice but does not have the slice flag", opt.GetName())
15+
}
16+
if kind != reflect.Slice && opt.HasFlags(OptionFlagSlice) {
17+
t.Errorf("option %s is not a slice but has the slice flag", opt.GetName())
18+
}
19+
if opt.HasFlags(OptionFlagPFlag | OptionFlagSensitive) {
20+
t.Errorf("%s: sensitive options shouldn't have pflags", opt.GetName())
21+
}
22+
}
23+
}
24+
25+
func TestOption_HasFlags(t *testing.T) {
26+
opt := &Option[any]{Flags: OptionFlagSensitive | OptionFlagPFlag | OptionFlagSlice}
27+
assert.True(t, opt.HasFlags(OptionFlagSensitive))
28+
assert.True(t, opt.HasFlags(OptionFlagPFlag))
29+
assert.True(t, opt.HasFlags(OptionFlagSlice))
30+
assert.True(t, opt.HasFlags(OptionFlagSensitive|OptionFlagPFlag))
31+
assert.True(t, opt.HasFlags(OptionFlagSensitive|OptionFlagSlice))
32+
assert.True(t, opt.HasFlags(OptionFlagPFlag|OptionFlagSlice))
33+
assert.True(t, opt.HasFlags(OptionFlagSensitive|OptionFlagPFlag|OptionFlagSlice))
34+
assert.False(t, opt.HasFlags(OptionFlagConfig))
35+
assert.False(t, opt.HasFlags(OptionFlagConfig|OptionFlagSensitive))
36+
}

‎internal/state/config/preferences.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func validate(m map[string]any, prefix string) error {
105105
continue
106106
}
107107
opt, ok := Options[key]
108-
if !ok || !opt.HasFlag(OptionFlagPreference) {
108+
if !ok || !opt.HasFlags(OptionFlagPreference) {
109109
return fmt.Errorf("unknown preference: %s", key)
110110
}
111111
}

0 commit comments

Comments
 (0)