Skip to content

Commit 5a03a5d

Browse files
ammarioammario
committed
fix: allow overridding default string array
1 parent 1c7adc0 commit 5a03a5d

File tree

6 files changed

+46
-56
lines changed

6 files changed

+46
-56
lines changed

‎cli/clibase/cmd.go

Lines changed: 18 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -219,29 +219,7 @@ func copyFlagSetWithout(fs *pflag.FlagSet, without string) *pflag.FlagSet {
219219
// allArgs is wired through the stack so that global flags can be accepted
220220
// anywhere in the command invocation.
221221
func (i *Invocation) run(state *runState) error {
222-
err := i.Command.Options.SetDefaults()
223-
if err != nil {
224-
return xerrors.Errorf("setting defaults: %w", err)
225-
}
226-
227-
// If we set the Default of an array but later see a flag for it, we
228-
// don't want to append, we want to replace. So, we need to keep the state
229-
// of defaulted array options.
230-
defaultedArrays := make(map[string]int)
231-
for _, opt := range i.Command.Options {
232-
sv, ok := opt.Value.(pflag.SliceValue)
233-
if !ok {
234-
continue
235-
}
236-
237-
if opt.Flag == "" {
238-
continue
239-
}
240-
241-
defaultedArrays[opt.Flag] = len(sv.GetSlice())
242-
}
243-
244-
err = i.Command.Options.ParseEnv(i.Environ)
222+
err := i.Command.Options.ParseEnv(i.Environ)
245223
if err != nil {
246224
return xerrors.Errorf("parsing env: %w", err)
247225
}
@@ -282,34 +260,24 @@ func (i *Invocation) run(state *runState) error {
282260
// so we check the error after looking for a child command.
283261
state.flagParseErr = i.parsedFlags.Parse(state.allArgs)
284262
parsedArgs = i.parsedFlags.Args()
263+
}
285264

286-
i.parsedFlags.VisitAll(func(f *pflag.Flag) {
287-
i, ok := defaultedArrays[f.Name]
288-
if !ok {
289-
return
290-
}
291-
292-
if !f.Changed {
293-
return
294-
}
295-
296-
// If flag was changed, we need to remove the default values.
297-
sv, ok := f.Value.(pflag.SliceValue)
298-
if !ok {
299-
panic("defaulted array option is not a slice value")
300-
}
301-
ss := sv.GetSlice()
302-
if len(ss) == 0 {
303-
// Slice likely zeroed by a flag.
304-
// E.g. "--fruit" may default to "apples,oranges" but the user
305-
// provided "--fruit=""".
306-
return
307-
}
308-
err := sv.Replace(ss[i:])
309-
if err != nil {
310-
panic(err)
311-
}
312-
})
265+
// Set defaults for flags that weren't set by the user.
266+
skipDefaults := make(map[string]struct{}, len(i.Command.Options))
267+
i.parsedFlags.VisitAll(func(f *pflag.Flag) {
268+
if !f.Changed {
269+
return
270+
}
271+
skipDefaults[f.Name] = struct{}{}
272+
})
273+
for _, opt := range i.Command.Options {
274+
if opt.envSet {
275+
skipDefaults[opt.Name] = struct{}{}
276+
}
277+
}
278+
err = i.Command.Options.SetDefaults(skipDefaults)
279+
if err != nil {
280+
return xerrors.Errorf("setting defaults: %w", err)
313281
}
314282

315283
// Run child command if found (next child only)

‎cli/clibase/cmd_test.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ func TestCommand_FlagOverride(t *testing.T) {
247247
Use: "1",
248248
Options: clibase.OptionSet{
249249
{
250+
Name: "flag",
250251
Flag: "f",
251252
Value: clibase.DiscardValue,
252253
},
@@ -256,6 +257,7 @@ func TestCommand_FlagOverride(t *testing.T) {
256257
Use: "2",
257258
Options: clibase.OptionSet{
258259
{
260+
Name: "flag",
259261
Flag: "f",
260262
Value: clibase.StringOf(&flag),
261263
},
@@ -527,11 +529,17 @@ func TestCommand_EmptySlice(t *testing.T) {
527529
}
528530
}
529531

530-
// Base-case
532+
// Base-case, uses default.
531533
err := cmd("bad", "bad", "bad").Invoke().Run()
532534
require.NoError(t, err)
533535

536+
// Reset to nothing at all.
534537
inv := cmd().Invoke("--arr", "")
535538
err = inv.Run()
536539
require.NoError(t, err)
540+
541+
// Override
542+
inv = cmd("great").Invoke("--arr", "great")
543+
err = inv.Run()
544+
require.NoError(t, err)
537545
}

‎cli/clibase/option.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ type Option struct {
4646
UseInstead []Option `json:"use_instead,omitempty"`
4747

4848
Hidden bool `json:"hidden,omitempty"`
49+
50+
envSet bool
4951
}
5052

5153
// OptionSet is a group of options that can be applied to a command.
@@ -133,6 +135,7 @@ func (s *OptionSet) ParseEnv(vs []EnvVar) error {
133135
continue
134136
}
135137

138+
opt.envSet = true
136139
if err := opt.Value.Set(envVal); err != nil {
137140
merr = multierror.Append(
138141
merr, xerrors.Errorf("parse %q: %w", opt.Name, err),
@@ -145,17 +148,28 @@ func (s *OptionSet) ParseEnv(vs []EnvVar) error {
145148

146149
// SetDefaults sets the default values for each Option.
147150
// It should be called before all parsing (e.g. ParseFlags, ParseEnv).
148-
func (s *OptionSet) SetDefaults() error {
151+
func (s *OptionSet) SetDefaults(skip map[string]struct{}) error {
149152
if s == nil {
150153
return nil
151154
}
152155

153156
var merr *multierror.Error
154157

155158
for _, opt := range *s {
159+
if opt.Name == "" {
160+
merr = multierror.Append(
161+
merr, xerrors.Errorf("parse: no Name field set"),
162+
)
163+
continue
164+
}
165+
if _, ok := skip[opt.Name]; ok {
166+
continue
167+
}
168+
156169
if opt.Default == "" {
157170
continue
158171
}
172+
159173
if opt.Value == nil {
160174
merr = multierror.Append(
161175
merr,

‎cli/clibase/option_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func TestOptionSet_ParseFlags(t *testing.T) {
4949
},
5050
}
5151

52-
err := os.SetDefaults()
52+
err := os.SetDefaults(nil)
5353
require.NoError(t, err)
5454

5555
err = os.FlagSet().Parse([]string{"--name", "foo", "--name", "bar"})
@@ -111,7 +111,7 @@ func TestOptionSet_ParseEnv(t *testing.T) {
111111
},
112112
}
113113

114-
err := os.SetDefaults()
114+
err := os.SetDefaults(nil)
115115
require.NoError(t, err)
116116

117117
err = os.ParseEnv(clibase.ParseEnviron([]string{"CODER_WORKSPACE_NAME="}, "CODER_"))

‎cli/clibase/yaml_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func TestOption_ToYAML(t *testing.T) {
4444
},
4545
}
4646

47-
err := os.SetDefaults()
47+
err := os.SetDefaults(nil)
4848
require.NoError(t, err)
4949

5050
n, err := os.ToYAML()

‎coderd/coderdtest/coderdtest.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1075,7 +1075,7 @@ QastnN77KfUwdj3SJt44U/uh1jAIv4oSLBr8HYUkbnI8
10751075
func DeploymentValues(t *testing.T) *codersdk.DeploymentValues {
10761076
var cfg codersdk.DeploymentValues
10771077
opts := cfg.Options()
1078-
err := opts.SetDefaults()
1078+
err := opts.SetDefaults(nil)
10791079
require.NoError(t, err)
10801080
return &cfg
10811081
}

0 commit comments

Comments
 (0)