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.

Sign up for GitHub

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

Jump to bottom

Add SliceFlag wrapper and fix bugs in existing implementations #1409

Merged
merged 2 commits into from Jun 18, 2022
Merged

Add SliceFlag wrapper and fix bugs in existing implementations #1409

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e77dd7b
Add SliceFlag wrapper and fix bugs in existing implementations
joeycumines Jun 6, 2022
4f795e3
Fix build for go < 1.18
joeycumines Jun 12, 2022
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
40 changes: 28 additions & 12 deletions flag_float64_slice.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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