Merge pull request #981 from urfave/string-slice-issue-v1

String slice issue v1

flag_int64_slice.go

@@ -22,7 +22,12 @@ func (f *Int64Slice) Set(value string) error {
 
 // String returns a readable representation of this value (for usage defaults)
 func (f *Int64Slice) String() string {
-	return fmt.Sprintf("%#v", *f)
+	slice := make([]string, len(*f))
+	for i, v := range *f {
+		slice[i] = strconv.FormatInt(v, 10)
+	}
+
+	return strings.Join(slice, ",")
 }
 
 // Value returns the slice of ints set by this flag
@@ -110,6 +115,7 @@ func (f Int64SliceFlag) ApplyWithError(set *flag.FlagSet) error {
 		}
 		set.Var(f.Value, name, f.Usage)
 	})
+
 	return nil
 }
 
@@ -131,11 +137,61 @@ func (c *Context) GlobalInt64Slice(name string) []int64 {
 func lookupInt64Slice(name string, set *flag.FlagSet) []int64 {
 	f := set.Lookup(name)
 	if f != nil {
-		parsed, err := (f.Value.(*Int64Slice)).Value(), error(nil)
-		if err != nil {
+		value, ok := f.Value.(*Int64Slice)
+		if !ok {
 			return nil
 		}
+
+		// extract the slice from asserted value
+		parsed := value.Value()
+
+		// extract default value from the flag
+		var defaultVal []int64
+		for _, v := range strings.Split(f.DefValue, ",") {
+			if v != "" {
+				int64Value, err := strconv.ParseInt(v, 10, 64)
+				if err != nil {
+					panic(err)
+				}
+				defaultVal = append(defaultVal, int64Value)
+			}
+		}
+		// if the current value is not equal to the default value
+		// remove the default values from the flag
+		if !isInt64SliceEqual(parsed, defaultVal) {
+			for _, v := range defaultVal {
+				parsed = removeFromInt64Slice(parsed, v)
+			}
+		}
 		return parsed
 	}
 	return nil
 }
+
+func removeFromInt64Slice(slice []int64, val int64) []int64 {
+	for i, v := range slice {
+		if v == val {
+			return append(slice[:i], slice[i+1:]...)
+		}
+	}
+	return slice
+}
+
+func isInt64SliceEqual(newValue, defaultValue []int64) bool {
+	// If one is nil, the other must also be nil.
+	if (newValue == nil) != (defaultValue == nil) {
+		return false
+	}
+
+	if len(newValue) != len(defaultValue) {
+		return false
+	}
+
+	for i, v := range newValue {
+		if v != defaultValue[i] {
+			return false
+		}
+	}
+
+	return true
+}

flag_int_slice.go

@@ -22,7 +22,12 @@ func (f *IntSlice) Set(value string) error {
 
 // String returns a readable representation of this value (for usage defaults)
 func (f *IntSlice) String() string {
-	return fmt.Sprintf("%#v", *f)
+	slice := make([]string, len(*f))
+	for i, v := range *f {
+		slice[i] = strconv.Itoa(v)
+	}
+
+	return strings.Join(slice, ",")
 }
 
 // Value returns the slice of ints set by this flag
@@ -132,11 +137,60 @@ func (c *Context) GlobalIntSlice(name string) []int {
 func lookupIntSlice(name string, set *flag.FlagSet) []int {
 	f := set.Lookup(name)
 	if f != nil {
-		parsed, err := (f.Value.(*IntSlice)).Value(), error(nil)
-		if err != nil {
+		value, ok := f.Value.(*IntSlice)
+		if !ok {
 			return nil
 		}
-		return parsed
+		// extract the slice from asserted value
+		slice := value.Value()
+
+		// extract default value from the flag
+		var defaultVal []int
+		for _, v := range strings.Split(f.DefValue, ",") {
+			if v != "" {
+				intValue, err := strconv.Atoi(v)
+				if err != nil {
+					panic(err)
+				}
+				defaultVal = append(defaultVal, intValue)
+			}
+		}
+		// if the current value is not equal to the default value
+		// remove the default values from the flag
+		if !isIntSliceEqual(slice, defaultVal) {
+			for _, v := range defaultVal {
+				slice = removeFromIntSlice(slice, v)
+			}
+		}
+		return slice
 	}
 	return nil
 }
+
+func removeFromIntSlice(slice []int, val int) []int {
+	for i, v := range slice {
+		if v == val {
+			return append(slice[:i], slice[i+1:]...)
+		}
+	}
+	return slice
+}
+
+func isIntSliceEqual(newValue, defaultValue []int) bool {
+	// If one is nil, the other must also be nil.
+	if (newValue == nil) != (defaultValue == nil) {
+		return false
+	}
+
+	if len(newValue) != len(defaultValue) {
+		return false
+	}
+
+	for i, v := range newValue {
+		if v != defaultValue[i] {
+			return false
+		}
+	}
+
+	return true
+}

flag_string_slice.go

@@ -17,7 +17,7 @@ func (f *StringSlice) Set(value string) error {
 
 // String returns a readable representation of this value (for usage defaults)
 func (f *StringSlice) String() string {
-	return fmt.Sprintf("%s", *f)
+	return strings.Join(*f, ",")
 }
 
 // Value returns the slice of strings set by this flag
@@ -128,11 +128,55 @@ func (c *Context) GlobalStringSlice(name string) []string {
 func lookupStringSlice(name string, set *flag.FlagSet) []string {
 	f := set.Lookup(name)
 	if f != nil {
-		parsed, err := (f.Value.(*StringSlice)).Value(), error(nil)
-		if err != nil {
+		value, ok := f.Value.(*StringSlice)
+		if !ok {
 			return nil
 		}
-		return parsed
+		// extract the slice from asserted value
+		slice := value.Value()
+
+		// extract default value from the flag
+		var defaultVal []string
+		for _, v := range strings.Split(f.DefValue, ",") {
+			defaultVal = append(defaultVal, v)
+		}
+
+		// if the current value is not equal to the default value
+		// remove the default values from the flag
+		if !isStringSliceEqual(slice, defaultVal) {
+			for _, v := range defaultVal {
+				slice = removeFromStringSlice(slice, v)
+			}
+		}
+		return slice
 	}
 	return nil
 }
+
+func removeFromStringSlice(slice []string, val string) []string {
+	for i, v := range slice {
+		if v == val {
+			return append(slice[:i], slice[i+1:]...)
+		}
+	}
+	return slice
+}
+
+func isStringSliceEqual(newValue, defaultValue []string) bool {
+	// If one is nil, the other must also be nil.
+	if (newValue == nil) != (defaultValue == nil) {
+		return false
+	}
+
+	if len(newValue) != len(defaultValue) {
+		return false
+	}
+
+	for i, v := range newValue {
+		if v != defaultValue[i] {
+			return false
+		}
+	}
+
+	return true
+}

flag_test.go

@@ -1330,3 +1330,96 @@ func TestFlagFromFile(t *testing.T) {
 		}
 	}
 }
+
+func TestSliceFlag_WithDefaults(t *testing.T) {
+	tests := []struct {
+		args   []string
+		app    *App
+	}{
+		{
+			args: []string{""},
+			app: &App{
+				Flags: []Flag{
+					StringSliceFlag{Name: "names, n", Value: &StringSlice{"john"}},
+					IntSliceFlag{Name: "userIds, u", Value: &IntSlice{3}},
+					Int64SliceFlag{Name: "phoneNumbers, p", Value: &Int64Slice{123456789}},
+				},
+				Action: func(ctx *Context) error {
+					expect(t, len(ctx.StringSlice("n")), 1)
+					for _, name := range ctx.StringSlice("names") {
+						expect(t, name == "john", true)
+					}
+
+					expect(t, len(ctx.IntSlice("u")), 1)
+					for _, userId := range ctx.IntSlice("userIds") {
+						expect(t, userId == 3, true)
+					}
+
+					expect(t, len(ctx.Int64Slice("p")), 1)
+					for _, phoneNumber := range ctx.Int64Slice("phoneNumbers") {
+						expect(t, phoneNumber == 123456789, true)
+					}
+					return nil
+				},
+			},
+		},
+		{
+			args: []string{"", "-n", "jane", "-n", "bob", "-u", "5", "-u", "10", "-p", "987654321"},
+			app: &App{
+				Flags: []Flag{
+					StringSliceFlag{Name: "names, n", Value: &StringSlice{"john"}},
+					IntSliceFlag{Name: "userIds, u", Value: &IntSlice{3}},
+					Int64SliceFlag{Name: "phoneNumbers, p", Value: &Int64Slice{123456789}},
+				},
+				Action: func(ctx *Context) error {
+					expect(t, len(ctx.StringSlice("n")), 2)
+					for _, name := range ctx.StringSlice("names") {
+						expect(t, name != "john", true)
+					}
+
+					expect(t, len(ctx.IntSlice("u")), 2)
+					for _, userId := range ctx.IntSlice("userIds") {
+						expect(t, userId != 3, true)
+					}
+
+					expect(t, len(ctx.Int64Slice("p")), 1)
+					for _, phoneNumber := range ctx.Int64Slice("phoneNumbers") {
+						expect(t, phoneNumber != 123456789, true)
+					}
+					return nil
+				},
+			},
+		},
+		{
+			args: []string{"", "--names", "john", "--userIds", "3", "--phoneNumbers", "123456789"},
+			app: &App{
+				Flags: []Flag{
+					StringSliceFlag{Name: "names, n", Value: &StringSlice{"john"}},
+					IntSliceFlag{Name: "userIds, u", Value: &IntSlice{3}},
+					Int64SliceFlag{Name: "phoneNumbers, p", Value: &Int64Slice{123456789}},
+				},
+				Action: func(ctx *Context) error {
+					expect(t, len(ctx.StringSlice("n")), 1)
+					for _, name := range ctx.StringSlice("names") {
+						expect(t, name == "john", true)
+					}
+
+					expect(t, len(ctx.IntSlice("u")), 1)
+					for _, userId := range ctx.IntSlice("userIds") {
+						expect(t, userId == 3, true)
+					}
+
+					expect(t, len(ctx.Int64Slice("p")), 1)
+					for _, phoneNumber := range ctx.Int64Slice("phoneNumbers") {
+						expect(t, phoneNumber == 123456789, true)
+					}
+					return nil
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		_ = tt.app.Run(tt.args)
+	}
+}