Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow specifying a StringSlice destination for StringSliceFlag #1078

Merged
merged 1 commit into from Feb 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 15 additions & 1 deletion flag_string_slice.go
Expand Up @@ -71,6 +71,7 @@ type StringSliceFlag struct {
Value *StringSlice
DefaultText string
HasBeenSet bool
Destination *StringSlice
}

// IsSet returns whether or not the flag has been set through env or file
Expand Down Expand Up @@ -117,20 +118,33 @@ func (f *StringSliceFlag) GetValue() string {
func (f *StringSliceFlag) Apply(set *flag.FlagSet) error {
if val, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok {
f.Value = &StringSlice{}
destination := f.Value
if f.Destination != nil {
destination = f.Destination
}

for _, s := range strings.Split(val, ",") {
if err := f.Value.Set(strings.TrimSpace(s)); err != nil {
if err := destination.Set(strings.TrimSpace(s)); err != nil {
return fmt.Errorf("could not parse %q as string value for flag %s: %s", val, f.Name, err)
}
}

// Set this to false so that we reset the slice if we then set values from
// flags that have already been set by the environment.
destination.hasBeenSet = false
f.HasBeenSet = true
}

for _, name := range f.Names() {
if f.Value == nil {
f.Value = &StringSlice{}
}

if f.Destination != nil {
set.Var(f.Destination, name, f.Usage)
continue
}

set.Var(f.Value, name, f.Usage)
}

Expand Down
64 changes: 63 additions & 1 deletion flag_test.go
Expand Up @@ -59,7 +59,7 @@ func TestFlagsFromEnv(t *testing.T) {

newSetStringSlice := func(defaults ...string) StringSlice {
s := NewStringSlice(defaults...)
s.hasBeenSet = true
s.hasBeenSet = false
return *s
}

Expand Down Expand Up @@ -917,6 +917,47 @@ func TestParseMultiStringSliceWithDefaults(t *testing.T) {
}).Run([]string{"run", "-s", "10", "-s", "20"})
}

func TestParseMultiStringSliceWithDestination(t *testing.T) {
dest := &StringSlice{}
_ = (&App{
Flags: []Flag{
&StringSliceFlag{Name: "serve", Aliases: []string{"s"}, Destination: dest},
},
Action: func(ctx *Context) error {
expected := []string{"10", "20"}
if !reflect.DeepEqual(dest.slice, expected) {
t.Errorf("main name not set: %v != %v", expected, ctx.StringSlice("serve"))
}
if !reflect.DeepEqual(dest.slice, expected) {
t.Errorf("short name not set: %v != %v", expected, ctx.StringSlice("s"))
}
return nil
},
}).Run([]string{"run", "-s", "10", "-s", "20"})
}

func TestParseMultiStringSliceWithDestinationAndEnv(t *testing.T) {
os.Clearenv()
_ = os.Setenv("APP_INTERVALS", "20,30,40")

dest := &StringSlice{}
_ = (&App{
Flags: []Flag{
&StringSliceFlag{Name: "serve", Aliases: []string{"s"}, Destination: dest, EnvVars: []string{"APP_INTERVALS"}},
},
Action: func(ctx *Context) error {
expected := []string{"10", "20"}
if !reflect.DeepEqual(dest.slice, expected) {
t.Errorf("main name not set: %v != %v", expected, ctx.StringSlice("serve"))
}
if !reflect.DeepEqual(dest.slice, expected) {
t.Errorf("short name not set: %v != %v", expected, ctx.StringSlice("s"))
}
return nil
},
}).Run([]string{"run", "-s", "10", "-s", "20"})
}

func TestParseMultiStringSliceWithDefaultsUnset(t *testing.T) {
_ = (&App{
Flags: []Flag{
Expand Down Expand Up @@ -1014,6 +1055,27 @@ func TestParseMultiStringSliceFromEnvCascadeWithDefaults(t *testing.T) {
}).Run([]string{"run"})
}

func TestParseMultiStringSliceFromEnvWithDestination(t *testing.T) {
os.Clearenv()
_ = os.Setenv("APP_INTERVALS", "20,30,40")

dest := &StringSlice{}
_ = (&App{
Flags: []Flag{
&StringSliceFlag{Name: "intervals", Aliases: []string{"i"}, Destination: dest, EnvVars: []string{"APP_INTERVALS"}},
},
Action: func(ctx *Context) error {
if !reflect.DeepEqual(dest.slice, []string{"20", "30", "40"}) {
t.Errorf("main name not set from env")
}
if !reflect.DeepEqual(dest.slice, []string{"20", "30", "40"}) {
t.Errorf("short name not set from env")
}
return nil
},
}).Run([]string{"run"})
}

func TestParseMultiInt(t *testing.T) {
_ = (&App{
Flags: []Flag{
Expand Down