Merge pull request #1236 from vipally/ally_fix_default_val

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

.gitignore

@@ -5,3 +5,5 @@ vendor
 .idea
 internal/*/built-example
 coverage.txt
+
+*.exe

flag_float64_slice.go

@@ -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 {
@@ -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

flag_int64_slice.go

@@ -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 {
@@ -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

flag_int_slice.go

@@ -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) {
@@ -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

flag_string_slice.go

@@ -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 {
@@ -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

flag_test.go

@@ -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")