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

bug fix #1235 : default value changes with parsed values on slice flags #1236

Merged
merged 2 commits into from Apr 24, 2021
Merged

bug fix #1235 : default value changes with parsed values on slice flags #1236

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
c98b85d
bug fix #1235 : default value changes with parsed values on slice flags
vipally Feb 5, 2021
ab52d6f
Merge branch 'master' into ally_fix_default_val
rliebz Apr 24, 2021
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
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Expand Up @@ -5,3 +5,5 @@ vendor
.idea
internal/*/built-example
coverage.txt

*.exe
19 changes: 15 additions & 4 deletions flag_float64_slice.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Expand Up @@ -1974,6 +1974,71 @@ func TestTimestampFlagApply_Fail_Parse_Wrong_Time(t *testing.T) {
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)
}
}
}

func TestTimestampFlagApply_WithDestination(t *testing.T) {
var destination Timestamp
expectedResult, _ := time.Parse(time.RFC3339, "2006-01-02T15:04:05Z")
Expand Down