Skip to content

Commit

Permalink
bug fix urfave#1235 : default value changes with parsed values on sli…
Browse files Browse the repository at this point in the history
…ce flags
  • Loading branch information
vipally committed Feb 5, 2021
1 parent 92d7784 commit c98b85d
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 22 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@ vendor
.idea
internal/*/built-example
coverage.txt

*.exe
19 changes: 15 additions & 4 deletions flag_float64_slice.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ func NewFloat64Slice(defaults ...float64) *Float64Slice {
return &Float64Slice{slice: append([]float64{}, defaults...)}
}

// clone allocate a copy of self object
func (f *Float64Slice) clone() *Float64Slice {
n := &Float64Slice{
slice: make([]float64, len(f.slice)),
hasBeenSet: f.hasBeenSet,
}
copy(n.slice, f.slice)
return n
}

// Set parses the value into a float64 and appends it to the list of values
func (f *Float64Slice) Set(value string) error {
if !f.hasBeenSet {
Expand Down Expand Up @@ -133,11 +143,12 @@ func (f *Float64SliceFlag) Apply(set *flag.FlagSet) error {
}
}

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

return nil
Expand Down
19 changes: 15 additions & 4 deletions flag_int64_slice.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ func NewInt64Slice(defaults ...int64) *Int64Slice {
return &Int64Slice{slice: append([]int64{}, defaults...)}
}

// clone allocate a copy of self object
func (i *Int64Slice) clone() *Int64Slice {
n := &Int64Slice{
slice: make([]int64, len(i.slice)),
hasBeenSet: i.hasBeenSet,
}
copy(n.slice, i.slice)
return n
}

// Set parses the value into an integer and appends it to the list of values
func (i *Int64Slice) Set(value string) error {
if !i.hasBeenSet {
Expand Down Expand Up @@ -132,11 +142,12 @@ func (f *Int64SliceFlag) Apply(set *flag.FlagSet) error {
f.HasBeenSet = true
}

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

return nil
Expand Down
19 changes: 15 additions & 4 deletions flag_int_slice.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,16 @@ func NewIntSlice(defaults ...int) *IntSlice {
return &IntSlice{slice: append([]int{}, defaults...)}
}

// clone allocate a copy of self object
func (i *IntSlice) clone() *IntSlice {
n := &IntSlice{
slice: make([]int, len(i.slice)),
hasBeenSet: i.hasBeenSet,
}
copy(n.slice, i.slice)
return n
}

// TODO: Consistently have specific Set function for Int64 and Float64 ?
// SetInt directly adds an integer to the list of values
func (i *IntSlice) SetInt(value int) {
Expand Down Expand Up @@ -143,11 +153,12 @@ func (f *IntSliceFlag) Apply(set *flag.FlagSet) error {
f.HasBeenSet = true
}

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

return nil
Expand Down
28 changes: 18 additions & 10 deletions flag_string_slice.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@ func NewStringSlice(defaults ...string) *StringSlice {
return &StringSlice{slice: append([]string{}, defaults...)}
}

// clone allocate a copy of self object
func (s *StringSlice) clone() *StringSlice {
n := &StringSlice{
slice: make([]string, len(s.slice)),
hasBeenSet: s.hasBeenSet,
}
copy(n.slice, s.slice)
return n
}

// Set appends the string value to the list of values
func (s *StringSlice) Set(value string) error {
if !s.hasBeenSet {
Expand Down Expand Up @@ -144,17 +154,15 @@ func (f *StringSliceFlag) Apply(set *flag.FlagSet) error {
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() {
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)
set.Var(setValue, name, f.Usage)
}

return nil
Expand Down
65 changes: 65 additions & 0 deletions flag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1973,3 +1973,68 @@ func TestTimestampFlagApply_Fail_Parse_Wrong_Time(t *testing.T) {
err := set.Parse([]string{"--time", "2006-01-02T15:04:05Z"})
expect(t, err, fmt.Errorf("invalid value \"2006-01-02T15:04:05Z\" for flag -time: parsing time \"2006-01-02T15:04:05Z\" as \"Jan 2, 2006 at 3:04pm (MST)\": cannot parse \"2006-01-02T15:04:05Z\" as \"Jan\""))
}

type flagDefaultTestCase struct {
name string
flag Flag
toParse []string
expect string
}

func TestFlagDefaultValue(t *testing.T) {
cases := []*flagDefaultTestCase{
&flagDefaultTestCase{
name: "stringSclice",
flag: &StringSliceFlag{Name: "flag", Value: NewStringSlice("default1", "default2")},
toParse: []string{"--flag", "parsed"},
expect: `--flag value (default: "default1", "default2") (accepts multiple inputs)`,
},
&flagDefaultTestCase{
name: "float64Sclice",
flag: &Float64SliceFlag{Name: "flag", Value: NewFloat64Slice(1.1, 2.2)},
toParse: []string{"--flag", "13.3"},
expect: `--flag value (default: 1.1, 2.2) (accepts multiple inputs)`,
},
&flagDefaultTestCase{
name: "int64Sclice",
flag: &Int64SliceFlag{Name: "flag", Value: NewInt64Slice(1, 2)},
toParse: []string{"--flag", "13"},
expect: `--flag value (default: 1, 2) (accepts multiple inputs)`,
},
&flagDefaultTestCase{
name: "intSclice",
flag: &IntSliceFlag{Name: "flag", Value: NewIntSlice(1, 2)},
toParse: []string{"--flag", "13"},
expect: `--flag value (default: 1, 2) (accepts multiple inputs)`,
},
&flagDefaultTestCase{
name: "string",
flag: &StringFlag{Name: "flag", Value: "default"},
toParse: []string{"--flag", "parsed"},
expect: `--flag value (default: "default")`,
},
&flagDefaultTestCase{
name: "bool",
flag: &BoolFlag{Name: "flag", Value: true},
toParse: []string{"--flag", "false"},
expect: `--flag (default: true)`,
},
&flagDefaultTestCase{
name: "uint64",
flag: &Uint64Flag{Name: "flag", Value: 1},
toParse: []string{"--flag", "13"},
expect: `--flag value (default: 1)`,
},
}
for i, v := range cases {
set := flag.NewFlagSet("test", 0)
set.SetOutput(ioutil.Discard)
_ = v.flag.Apply(set)
if err := set.Parse(v.toParse); err != nil {
t.Error(err)
}
if got := v.flag.String(); got != v.expect {
t.Errorf("TestFlagDefaultValue %d %s\nexpect:%s\ngot:%s", i, v.name, v.expect, got)
}
}
}

0 comments on commit c98b85d

Please sign in to comment.