Skip to content

Commit

Permalink
Merge pull request #1409 from joeycumines/main
Browse files Browse the repository at this point in the history 
Add SliceFlag wrapper and fix bugs in existing implementations
  • Loading branch information
@meatballhat
meatballhat committed Jun 18, 2022
2 parents 947f989 + 4f795e3 commit 8007c54
Show file tree
Hide file tree
Showing 8 changed files with 1,491 additions and 56 deletions.
40 changes: 28 additions & 12 deletions flag_float64_slice.go
View file
Expand Up @@ -56,7 +56,12 @@ func (f *Float64Slice) Set(value string) error {

// String returns a readable representation of this value (for usage defaults)
func (f *Float64Slice) String() string {
return fmt.Sprintf("%#v", f.slice)
v := f.slice
if v == nil {
// treat nil the same as zero length non-nil
v = make([]float64, 0)
}
return fmt.Sprintf("%#v", v)
}

// Serialize allows Float64Slice to fulfill Serializer
Expand Down Expand Up @@ -120,29 +125,40 @@ func (f *Float64SliceFlag) GetEnvVars() []string {

// Apply populates the flag given the flag set and environment
func (f *Float64SliceFlag) Apply(set *flag.FlagSet) error {
// apply any default
if f.Destination != nil && f.Value != nil {
f.Destination.slice = make([]float64, len(f.Value.slice))
copy(f.Destination.slice, f.Value.slice)
}

// resolve setValue (what we will assign to the set)
var setValue *Float64Slice
switch {
case f.Destination != nil:
setValue = f.Destination
case f.Value != nil:
setValue = f.Value.clone()
default:
setValue = new(Float64Slice)
}

if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
if val != "" {
f.Value = &Float64Slice{}

for _, s := range flagSplitMultiValues(val) {
if err := f.Value.Set(strings.TrimSpace(s)); err != nil {
return fmt.Errorf("could not parse %q as float64 slice value from %s for flag %s: %s", f.Value, source, f.Name, err)
if err := setValue.Set(strings.TrimSpace(s)); err != nil {
return fmt.Errorf("could not parse %q as float64 slice value from %s for flag %s: %s", val, source, 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.
f.Value.hasBeenSet = false
setValue.hasBeenSet = false
f.HasBeenSet = true
}
}

if f.Value == nil {
f.Value = &Float64Slice{}
}
copyValue := f.Value.clone()
for _, name := range f.Names() {
set.Var(copyValue, name, f.Usage)
set.Var(setValue, name, f.Usage)
}

return nil
Expand All @@ -165,7 +181,7 @@ func (cCtx *Context) Float64Slice(name string) []float64 {
func lookupFloat64Slice(name string, set *flag.FlagSet) []float64 {
f := set.Lookup(name)
if f != nil {
if slice, ok := f.Value.(*Float64Slice); ok {
if slice, ok := unwrapFlagValue(f.Value).(*Float64Slice); ok {
return slice.Value()
}
}
Expand Down
38 changes: 27 additions & 11 deletions flag_int64_slice.go
View file
Expand Up @@ -57,7 +57,12 @@ func (i *Int64Slice) Set(value string) error {

// String returns a readable representation of this value (for usage defaults)
func (i *Int64Slice) String() string {
return fmt.Sprintf("%#v", i.slice)
v := i.slice
if v == nil {
// treat nil the same as zero length non-nil
v = make([]int64, 0)
}
return fmt.Sprintf("%#v", v)
}

// Serialize allows Int64Slice to fulfill Serializer
Expand Down Expand Up @@ -121,27 +126,38 @@ func (f *Int64SliceFlag) GetEnvVars() []string {

// Apply populates the flag given the flag set and environment
func (f *Int64SliceFlag) Apply(set *flag.FlagSet) error {
if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
f.Value = &Int64Slice{}
// apply any default
if f.Destination != nil && f.Value != nil {
f.Destination.slice = make([]int64, len(f.Value.slice))
copy(f.Destination.slice, f.Value.slice)
}

// resolve setValue (what we will assign to the set)
var setValue *Int64Slice
switch {
case f.Destination != nil:
setValue = f.Destination
case f.Value != nil:
setValue = f.Value.clone()
default:
setValue = new(Int64Slice)
}

if val, source, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok && val != "" {
for _, s := range flagSplitMultiValues(val) {
if err := f.Value.Set(strings.TrimSpace(s)); err != nil {
if err := setValue.Set(strings.TrimSpace(s)); err != nil {
return fmt.Errorf("could not parse %q as int64 slice value from %s for flag %s: %s", val, source, 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.
f.Value.hasBeenSet = false
setValue.hasBeenSet = false
f.HasBeenSet = true
}

if f.Value == nil {
f.Value = &Int64Slice{}
}
copyValue := f.Value.clone()
for _, name := range f.Names() {
set.Var(copyValue, name, f.Usage)
set.Var(setValue, name, f.Usage)
}

return nil
Expand All @@ -164,7 +180,7 @@ func (cCtx *Context) Int64Slice(name string) []int64 {
func lookupInt64Slice(name string, set *flag.FlagSet) []int64 {
f := set.Lookup(name)
if f != nil {
if slice, ok := f.Value.(*Int64Slice); ok {
if slice, ok := unwrapFlagValue(f.Value).(*Int64Slice); ok {
return slice.Value()
}
}
Expand Down
38 changes: 27 additions & 11 deletions flag_int_slice.go
View file
Expand Up @@ -68,7 +68,12 @@ func (i *IntSlice) Set(value string) error {

// String returns a readable representation of this value (for usage defaults)
func (i *IntSlice) String() string {
return fmt.Sprintf("%#v", i.slice)
v := i.slice
if v == nil {
// treat nil the same as zero length non-nil
v = make([]int, 0)
}
return fmt.Sprintf("%#v", v)
}

// Serialize allows IntSlice to fulfill Serializer
Expand Down Expand Up @@ -132,27 +137,38 @@ func (f *IntSliceFlag) GetEnvVars() []string {

// Apply populates the flag given the flag set and environment
func (f *IntSliceFlag) Apply(set *flag.FlagSet) error {
if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
f.Value = &IntSlice{}
// apply any default
if f.Destination != nil && f.Value != nil {
f.Destination.slice = make([]int, len(f.Value.slice))
copy(f.Destination.slice, f.Value.slice)
}

// resolve setValue (what we will assign to the set)
var setValue *IntSlice
switch {
case f.Destination != nil:
setValue = f.Destination
case f.Value != nil:
setValue = f.Value.clone()
default:
setValue = new(IntSlice)
}

if val, source, ok := flagFromEnvOrFile(f.EnvVars, f.FilePath); ok && val != "" {
for _, s := range flagSplitMultiValues(val) {
if err := f.Value.Set(strings.TrimSpace(s)); err != nil {
if err := setValue.Set(strings.TrimSpace(s)); err != nil {
return fmt.Errorf("could not parse %q as int slice value from %s for flag %s: %s", val, source, 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.
f.Value.hasBeenSet = false
setValue.hasBeenSet = false
f.HasBeenSet = true
}

if f.Value == nil {
f.Value = &IntSlice{}
}
copyValue := f.Value.clone()
for _, name := range f.Names() {
set.Var(copyValue, name, f.Usage)
set.Var(setValue, name, f.Usage)
}

return nil
Expand All @@ -175,7 +191,7 @@ func (cCtx *Context) IntSlice(name string) []int {
func lookupIntSlice(name string, set *flag.FlagSet) []int {
f := set.Lookup(name)
if f != nil {
if slice, ok := f.Value.(*IntSlice); ok {
if slice, ok := unwrapFlagValue(f.Value).(*IntSlice); ok {
return slice.Value()
}
}
Expand Down
33 changes: 14 additions & 19 deletions flag_string_slice.go
View file
Expand Up @@ -115,41 +115,36 @@ func (f *StringSliceFlag) GetEnvVars() []string {

// Apply populates the flag given the flag set and environment
func (f *StringSliceFlag) Apply(set *flag.FlagSet) error {

// apply any default
if f.Destination != nil && f.Value != nil {
f.Destination.slice = make([]string, len(f.Value.slice))
copy(f.Destination.slice, f.Value.slice)
}

// resolve setValue (what we will assign to the set)
var setValue *StringSlice
switch {
case f.Destination != nil:
setValue = f.Destination
case f.Value != nil:
setValue = f.Value.clone()
default:
setValue = new(StringSlice)
}

if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
if f.Value == nil {
f.Value = &StringSlice{}
}
destination := f.Value
if f.Destination != nil {
destination = f.Destination
}

for _, s := range flagSplitMultiValues(val) {
if err := destination.Set(strings.TrimSpace(s)); err != nil {
if err := setValue.Set(strings.TrimSpace(s)); err != nil {
return fmt.Errorf("could not parse %q as string value from %s for flag %s: %s", val, source, 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
setValue.hasBeenSet = false
f.HasBeenSet = true
}

if f.Value == nil {
f.Value = &StringSlice{}
}
setValue := f.Destination
if f.Destination == nil {
setValue = f.Value.clone()
}
for _, name := range f.Names() {
set.Var(setValue, name, f.Usage)
}
Expand All @@ -174,7 +169,7 @@ func (cCtx *Context) StringSlice(name string) []string {
func lookupStringSlice(name string, set *flag.FlagSet) []string {
f := set.Lookup(name)
if f != nil {
if slice, ok := f.Value.(*StringSlice); ok {
if slice, ok := unwrapFlagValue(f.Value).(*StringSlice); ok {
return slice.Value()
}
}
Expand Down
90 changes: 87 additions & 3 deletions flag_test.go
View file
Expand Up @@ -114,7 +114,7 @@ func TestFlagsFromEnv(t *testing.T) {
{"foobar", 0, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int value from environment variable "SECONDS" for flag seconds: .*`},

{"1.0,2", newSetFloat64Slice(1, 2), &Float64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
{"foobar", newSetFloat64Slice(), &Float64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "\[\]float64{}" as float64 slice value from environment variable "SECONDS" for flag seconds: .*`},
{"foobar", newSetFloat64Slice(), &Float64SliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as float64 slice value from environment variable "SECONDS" for flag seconds: .*`},

{"1,2", newSetIntSlice(1, 2), &IntSliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
{"1.2,2", newSetIntSlice(), &IntSliceFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2,2" as int slice value from environment variable "SECONDS" for flag seconds: .*`},
Expand Down Expand Up @@ -604,7 +604,7 @@ func TestStringSliceFlagApply_SetsAllNames(t *testing.T) {
expect(t, err, nil)
}

func TestStringSliceFlagApply_UsesEnvValues(t *testing.T) {
func TestStringSliceFlagApply_UsesEnvValues_noDefault(t *testing.T) {
defer resetEnv(os.Environ())
os.Clearenv()
_ = os.Setenv("MY_GOAT", "vincent van goat,scape goat")
Expand All @@ -615,7 +615,22 @@ func TestStringSliceFlagApply_UsesEnvValues(t *testing.T) {

err := set.Parse(nil)
expect(t, err, nil)
expect(t, val.Value(), NewStringSlice("vincent van goat", "scape goat").Value())
expect(t, val.Value(), []string(nil))
expect(t, set.Lookup("goat").Value.(*StringSlice).Value(), []string{"vincent van goat", "scape goat"})
}

func TestStringSliceFlagApply_UsesEnvValues_withDefault(t *testing.T) {
defer resetEnv(os.Environ())
os.Clearenv()
_ = os.Setenv("MY_GOAT", "vincent van goat,scape goat")
val := NewStringSlice(`some default`, `values here`)
fl := StringSliceFlag{Name: "goat", EnvVars: []string{"MY_GOAT"}, Value: val}
set := flag.NewFlagSet("test", 0)
_ = fl.Apply(set)
err := set.Parse(nil)
expect(t, err, nil)
expect(t, val.Value(), []string{`some default`, `values here`})
expect(t, set.Lookup("goat").Value.(*StringSlice).Value(), []string{"vincent van goat", "scape goat"})
}

func TestStringSliceFlagApply_DefaultValueWithDestination(t *testing.T) {
Expand Down Expand Up @@ -1406,6 +1421,75 @@ func TestParseMultiStringSliceWithDestinationAndEnv(t *testing.T) {
}).Run([]string{"run", "-s", "10", "-s", "20"})
}

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

dest := &Float64Slice{}
_ = (&App{
Flags: []Flag{
&Float64SliceFlag{Name: "serve", Aliases: []string{"s"}, Destination: dest, EnvVars: []string{"APP_INTERVALS"}},
},
Action: func(ctx *Context) error {
expected := []float64{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 TestParseMultiInt64SliceWithDestinationAndEnv(t *testing.T) {
defer resetEnv(os.Environ())
os.Clearenv()
_ = os.Setenv("APP_INTERVALS", "20,30,40")

dest := &Int64Slice{}
_ = (&App{
Flags: []Flag{
&Int64SliceFlag{Name: "serve", Aliases: []string{"s"}, Destination: dest, EnvVars: []string{"APP_INTERVALS"}},
},
Action: func(ctx *Context) error {
expected := []int64{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 TestParseMultiIntSliceWithDestinationAndEnv(t *testing.T) {
defer resetEnv(os.Environ())
os.Clearenv()
_ = os.Setenv("APP_INTERVALS", "20,30,40")

dest := &IntSlice{}
_ = (&App{
Flags: []Flag{
&IntSliceFlag{Name: "serve", Aliases: []string{"s"}, Destination: dest, EnvVars: []string{"APP_INTERVALS"}},
},
Action: func(ctx *Context) error {
expected := []int{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

0 comments on commit 8007c54

Please sign in to comment.