diff --git a/CHANGELOG.md b/CHANGELOG.md index d7ea0a6..ea6c483 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,16 @@ **ATTN**: This project uses [semantic versioning](http://semver.org/). -## [Unreleased] +## 2.0.0 - (unreleased 2.x series) +### Added +- `NewStringSlice` and `NewIntSlice` for creating their related types + +### Removed +- the ability to specify `&StringSlice{...string}` or `&IntSlice{...int}`. +To migrate to the new API, you may choose to run [this helper +(python) script](./cli-migrate-slice-types). + +## [Unreleased] - (1.x series) ### Added - Pluggable flag-level help text rendering via `cli.DefaultFlagStringFunc` diff --git a/altsrc/flag.go b/altsrc/flag.go index 0a11ad5..d934e6a 100644 --- a/altsrc/flag.go +++ b/altsrc/flag.go @@ -122,7 +122,7 @@ func (f *StringSliceFlag) ApplyInputSourceValue(context *cli.Context, isc InputS return err } if value != nil { - var sliceValue cli.StringSlice = value + var sliceValue cli.StringSlice = *(cli.NewStringSlice(value...)) eachName(f.Name, func(name string) { underlyingFlag := f.set.Lookup(f.Name) if underlyingFlag != nil { @@ -163,7 +163,7 @@ func (f *IntSliceFlag) ApplyInputSourceValue(context *cli.Context, isc InputSour return err } if value != nil { - var sliceValue cli.IntSlice = value + var sliceValue cli.IntSlice = *(cli.NewIntSlice(value...)) eachName(f.Name, func(name string) { underlyingFlag := f.set.Lookup(f.Name) if underlyingFlag != nil { diff --git a/app_test.go b/app_test.go index bf2887e..ca4abce 100644 --- a/app_test.go +++ b/app_test.go @@ -329,8 +329,8 @@ func TestApp_ParseSliceFlags(t *testing.T) { command := Command{ Name: "cmd", Flags: []Flag{ - IntSliceFlag{Name: "p", Value: &IntSlice{}, Usage: "set one or more ip addr"}, - StringSliceFlag{Name: "ip", Value: &StringSlice{}, Usage: "set one or more ports to open"}, + IntSliceFlag{Name: "p", Value: NewIntSlice(), Usage: "set one or more ip addr"}, + StringSliceFlag{Name: "ip", Value: NewStringSlice(), Usage: "set one or more ports to open"}, }, Action: func(c *Context) error { parsedIntSlice = c.IntSlice("p") diff --git a/cli-migrate-slice-types b/cli-migrate-slice-types new file mode 100755 index 0000000..5c6cb1f --- /dev/null +++ b/cli-migrate-slice-types @@ -0,0 +1,75 @@ +#!/usr/bin/env python +from __future__ import print_function, unicode_literals + +import argparse +import os +import re +import sys + + +DESCRIPTION = """\ +Migrate arbitrary `.go` sources from the pre-v2.0.0 API for StringSlice and +IntSlice types, optionally writing the changes back to file. +""" +SLICE_TYPE_RE = re.compile( + '&cli\\.(?PIntSlice|StringSlice){(?P[^}]*)}', + flags=re.DOTALL +) + + +def main(sysargs=sys.argv[:]): + parser = argparse.ArgumentParser( + description=DESCRIPTION, + formatter_class=argparse.ArgumentDefaultsHelpFormatter) + parser.add_argument('basedir', nargs='?', metavar='BASEDIR', + type=os.path.abspath, default=os.getcwd()) + parser.add_argument('-w', '--write', help='write changes back to file', + action='store_true', default=False) + + args = parser.parse_args(sysargs[1:]) + + for filepath in _find_candidate_files(args.basedir): + updated_source = _update_source(filepath) + if args.write: + print('Updating {!r}'.format(filepath)) + + with open(filepath, 'w') as outfile: + outfile.write(updated_source) + else: + print('// -> Updated {!r}'.format(filepath)) + print(updated_source) + + return 0 + + +def _update_source(filepath): + with open(filepath) as infile: + source = infile.read() + return SLICE_TYPE_RE.sub(_slice_type_repl, source) + + +def _slice_type_repl(match): + return 'cli.New{}({})'.format( + match.groupdict()['type'], match.groupdict()['args'] + ) + + +def _find_candidate_files(basedir): + for curdir, dirs, files in os.walk(basedir): + for i, dirname in enumerate(dirs[:]): + if dirname.startswith('.'): + dirs.pop(i) + + for filename in files: + if not filename.endswith('.go'): + continue + + filepath = os.path.join(curdir, filename) + if not os.access(filepath, os.R_OK | os.W_OK): + continue + + yield filepath + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/context.go b/context.go index ef3d2fc..94cd085 100644 --- a/context.go +++ b/context.go @@ -385,7 +385,8 @@ func lookupBoolT(name string, set *flag.FlagSet) bool { func copyFlag(name string, ff *flag.Flag, set *flag.FlagSet) { switch ff.Value.(type) { - case *StringSlice: + case Serializeder: + set.Set(name, ff.Value.(Serializeder).Serialized()) default: set.Set(name, ff.Value.String()) } diff --git a/flag.go b/flag.go index 3b6a2e1..c25ad25 100644 --- a/flag.go +++ b/flag.go @@ -1,6 +1,7 @@ package cli import ( + "encoding/json" "flag" "fmt" "os" @@ -13,6 +14,8 @@ import ( const defaultPlaceholder = "value" +var slPfx = fmt.Sprintf("sl:::%d:::", time.Now().UTC().UnixNano()) + // This flag enables bash-completion for all commands and subcommands var BashCompletionFlag = BoolFlag{ Name: "generate-bash-completion", @@ -35,6 +38,11 @@ var HelpFlag = BoolFlag{ var FlagStringer FlagStringFunc = stringifyFlag +// Serializeder is used to circumvent the limitations of flag.FlagSet.Set +type Serializeder interface { + Serialized() string +} + // Flag is a common interface related to parsing flags in cli. // For more advanced flag parsing techniques, it is recommended that // this interface be implemented. @@ -107,26 +115,52 @@ func (f GenericFlag) GetName() string { return f.Name } -// StringSlice is an opaque type for []string to satisfy flag.Value -type StringSlice []string +// StringSlice wraps a []string to satisfy flag.Value +type StringSlice struct { + slice []string + hasBeenSet bool +} + +// NewStringSlice creates a *StringSlice with default values +func NewStringSlice(defaults ...string) *StringSlice { + return &StringSlice{slice: append([]string{}, defaults...)} +} // Set appends the string value to the list of values func (f *StringSlice) Set(value string) error { - *f = append(*f, value) + if !f.hasBeenSet { + f.slice = []string{} + f.hasBeenSet = true + } + + if strings.HasPrefix(value, slPfx) { + // Deserializing assumes overwrite + _ = json.Unmarshal([]byte(strings.Replace(value, slPfx, "", 1)), &f.slice) + f.hasBeenSet = true + return nil + } + + f.slice = append(f.slice, value) return nil } // String returns a readable representation of this value (for usage defaults) func (f *StringSlice) String() string { - return fmt.Sprintf("%s", *f) + return fmt.Sprintf("%s", f.slice) +} + +// Serialized allows StringSlice to fulfill Serializeder +func (f *StringSlice) Serialized() string { + jsonBytes, _ := json.Marshal(f.slice) + return fmt.Sprintf("%s%s", slPfx, string(jsonBytes)) } // Value returns the slice of strings set by this flag func (f *StringSlice) Value() []string { - return *f + return f.slice } -// StringSlice is a string flag that can be specified multiple times on the +// StringSliceFlag is a string flag that can be specified multiple times on the // command-line type StringSliceFlag struct { Name string @@ -147,7 +181,7 @@ func (f StringSliceFlag) Apply(set *flag.FlagSet) { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) if envVal := os.Getenv(envVar); envVal != "" { - newVal := &StringSlice{} + newVal := NewStringSlice() for _, s := range strings.Split(envVal, ",") { s = strings.TrimSpace(s) newVal.Set(s) @@ -158,10 +192,11 @@ func (f StringSliceFlag) Apply(set *flag.FlagSet) { } } + if f.Value == nil { + f.Value = NewStringSlice() + } + eachName(f.Name, func(name string) { - if f.Value == nil { - f.Value = &StringSlice{} - } set.Var(f.Value, name, f.Usage) }) } @@ -170,28 +205,64 @@ func (f StringSliceFlag) GetName() string { return f.Name } -// StringSlice is an opaque type for []int to satisfy flag.Value -type IntSlice []int +// IntSlice wraps an []int to satisfy flag.Value +type IntSlice struct { + slice []int + hasBeenSet bool +} + +// NewIntSlice makes an *IntSlice with default values +func NewIntSlice(defaults ...int) *IntSlice { + return &IntSlice{slice: append([]int{}, defaults...)} +} + +// SetInt directly adds an integer to the list of values +func (i *IntSlice) SetInt(value int) { + if !i.hasBeenSet { + i.slice = []int{} + i.hasBeenSet = true + } + + i.slice = append(i.slice, value) +} // Set parses the value into an integer and appends it to the list of values -func (f *IntSlice) Set(value string) error { +func (i *IntSlice) Set(value string) error { + if !i.hasBeenSet { + i.slice = []int{} + i.hasBeenSet = true + } + + if strings.HasPrefix(value, slPfx) { + // Deserializing assumes overwrite + _ = json.Unmarshal([]byte(strings.Replace(value, slPfx, "", 1)), &i.slice) + i.hasBeenSet = true + return nil + } + tmp, err := strconv.Atoi(value) if err != nil { return err } else { - *f = append(*f, tmp) + i.slice = append(i.slice, tmp) } return nil } // String returns a readable representation of this value (for usage defaults) -func (f *IntSlice) String() string { - return fmt.Sprintf("%d", *f) +func (i *IntSlice) String() string { + return fmt.Sprintf("%v", i.slice) +} + +// Serialized allows IntSlice to fulfill Serializeder +func (i *IntSlice) Serialized() string { + jsonBytes, _ := json.Marshal(i.slice) + return fmt.Sprintf("%s%s", slPfx, string(jsonBytes)) } // Value returns the slice of ints set by this flag -func (f *IntSlice) Value() []int { - return *f +func (i *IntSlice) Value() []int { + return i.slice } // IntSliceFlag is an int flag that can be specified multiple times on the @@ -215,7 +286,7 @@ func (f IntSliceFlag) Apply(set *flag.FlagSet) { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) if envVal := os.Getenv(envVar); envVal != "" { - newVal := &IntSlice{} + newVal := NewIntSlice() for _, s := range strings.Split(envVal, ",") { s = strings.TrimSpace(s) err := newVal.Set(s) @@ -229,10 +300,11 @@ func (f IntSliceFlag) Apply(set *flag.FlagSet) { } } + if f.Value == nil { + f.Value = NewIntSlice() + } + eachName(f.Name, func(name string) { - if f.Value == nil { - f.Value = &IntSlice{} - } set.Var(f.Value, name, f.Usage) }) } diff --git a/flag_test.go b/flag_test.go index e0df23b..f914da3 100644 --- a/flag_test.go +++ b/flag_test.go @@ -76,26 +76,11 @@ var stringSliceFlagTests = []struct { value *StringSlice expected string }{ - {"foo", func() *StringSlice { - s := &StringSlice{} - s.Set("") - return s - }(), "--foo value\t"}, - {"f", func() *StringSlice { - s := &StringSlice{} - s.Set("") - return s - }(), "-f value\t"}, - {"f", func() *StringSlice { - s := &StringSlice{} - s.Set("Lipstick") - return s - }(), "-f value\t(default: \"Lipstick\")"}, - {"test", func() *StringSlice { - s := &StringSlice{} - s.Set("Something") - return s - }(), "--test value\t(default: \"Something\")"}, + {"foo", NewStringSlice(""), "--foo value\t"}, + {"f", NewStringSlice(""), "-f value\t"}, + {"f", NewStringSlice("Lipstick"), "-f value\t(default: \"Lipstick\")"}, + {"test", NewStringSlice("Something"), "--test value\t(default: \"Something\")"}, + {"d, dee", NewStringSlice("Inka", "Dinka", "dooo"), "-d value, --dee value\t(default: \"Inka\", \"Dinka\", \"dooo\")"}, } func TestStringSliceFlagHelpOutput(t *testing.T) { @@ -203,14 +188,9 @@ var intSliceFlagTests = []struct { value *IntSlice expected string }{ - {"heads", &IntSlice{}, "--heads value\t"}, - {"H", &IntSlice{}, "-H value\t"}, - {"H, heads", func() *IntSlice { - i := &IntSlice{} - i.Set("9") - i.Set("3") - return i - }(), "-H value, --heads value\t(default: 9, 3)"}, + {"heads", NewIntSlice(), "--heads value\t"}, + {"H", NewIntSlice(), "-H value\t"}, + {"H, heads", NewIntSlice(9, 3), "-H value, --heads value\t(default: 9, 3)"}, } func TestIntSliceFlagHelpOutput(t *testing.T) { @@ -391,27 +371,80 @@ func TestParseMultiStringFromEnvCascade(t *testing.T) { func TestParseMultiStringSlice(t *testing.T) { (&App{ Flags: []Flag{ - StringSliceFlag{Name: "serve, s", Value: &StringSlice{}}, + StringSliceFlag{Name: "serve, s", Value: NewStringSlice()}, }, Action: func(ctx *Context) error { - if !reflect.DeepEqual(ctx.StringSlice("serve"), []string{"10", "20"}) { - t.Errorf("main name not set") + expected := []string{"10", "20"} + if !reflect.DeepEqual(ctx.StringSlice("serve"), expected) { + t.Errorf("main name not set: %v != %v", expected, ctx.StringSlice("serve")) } - if !reflect.DeepEqual(ctx.StringSlice("s"), []string{"10", "20"}) { - t.Errorf("short name not set") + if !reflect.DeepEqual(ctx.StringSlice("s"), expected) { + t.Errorf("short name not set: %v != %v", expected, ctx.StringSlice("s")) } return nil }, }).Run([]string{"run", "-s", "10", "-s", "20"}) } +func TestParseMultiStringSliceWithDefaults(t *testing.T) { + (&App{ + Flags: []Flag{ + StringSliceFlag{Name: "serve, s", Value: NewStringSlice("9", "2")}, + }, + Action: func(ctx *Context) { + expected := []string{"10", "20"} + if !reflect.DeepEqual(ctx.StringSlice("serve"), expected) { + t.Errorf("main name not set: %v != %v", expected, ctx.StringSlice("serve")) + } + if !reflect.DeepEqual(ctx.StringSlice("s"), expected) { + t.Errorf("short name not set: %v != %v", expected, ctx.StringSlice("s")) + } + }, + }).Run([]string{"run", "-s", "10", "-s", "20"}) +} + +func TestParseMultiStringSliceWithDefaultsUnset(t *testing.T) { + (&App{ + Flags: []Flag{ + StringSliceFlag{Name: "serve, s", Value: NewStringSlice("9", "2")}, + }, + Action: func(ctx *Context) { + if !reflect.DeepEqual(ctx.StringSlice("serve"), []string{"9", "2"}) { + t.Errorf("main name not set: %v", ctx.StringSlice("serve")) + } + if !reflect.DeepEqual(ctx.StringSlice("s"), []string{"9", "2"}) { + t.Errorf("short name not set: %v", ctx.StringSlice("s")) + } + }, + }).Run([]string{"run"}) +} + func TestParseMultiStringSliceFromEnv(t *testing.T) { os.Clearenv() os.Setenv("APP_INTERVALS", "20,30,40") (&App{ Flags: []Flag{ - StringSliceFlag{Name: "intervals, i", Value: &StringSlice{}, EnvVar: "APP_INTERVALS"}, + StringSliceFlag{Name: "intervals, i", Value: NewStringSlice(), EnvVar: "APP_INTERVALS"}, + }, + Action: func(ctx *Context) { + if !reflect.DeepEqual(ctx.StringSlice("intervals"), []string{"20", "30", "40"}) { + t.Errorf("main name not set from env") + } + if !reflect.DeepEqual(ctx.StringSlice("i"), []string{"20", "30", "40"}) { + t.Errorf("short name not set from env") + } + }, + }).Run([]string{"run"}) +} + +func TestParseMultiStringSliceFromEnvWithDefaults(t *testing.T) { + os.Clearenv() + os.Setenv("APP_INTERVALS", "20,30,40") + + (&App{ + Flags: []Flag{ + StringSliceFlag{Name: "intervals, i", Value: NewStringSlice("1", "2", "5"), EnvVar: "APP_INTERVALS"}, }, Action: func(ctx *Context) error { if !reflect.DeepEqual(ctx.StringSlice("intervals"), []string{"20", "30", "40"}) { @@ -431,7 +464,26 @@ func TestParseMultiStringSliceFromEnvCascade(t *testing.T) { (&App{ Flags: []Flag{ - StringSliceFlag{Name: "intervals, i", Value: &StringSlice{}, EnvVar: "COMPAT_INTERVALS,APP_INTERVALS"}, + StringSliceFlag{Name: "intervals, i", Value: NewStringSlice(), EnvVar: "COMPAT_INTERVALS,APP_INTERVALS"}, + }, + Action: func(ctx *Context) { + if !reflect.DeepEqual(ctx.StringSlice("intervals"), []string{"20", "30", "40"}) { + t.Errorf("main name not set from env") + } + if !reflect.DeepEqual(ctx.StringSlice("i"), []string{"20", "30", "40"}) { + t.Errorf("short name not set from env") + } + }, + }).Run([]string{"run"}) +} + +func TestParseMultiStringSliceFromEnvCascadeWithDefaults(t *testing.T) { + os.Clearenv() + os.Setenv("APP_INTERVALS", "20,30,40") + + (&App{ + Flags: []Flag{ + StringSliceFlag{Name: "intervals, i", Value: NewStringSlice("1", "2", "5"), EnvVar: "COMPAT_INTERVALS,APP_INTERVALS"}, }, Action: func(ctx *Context) error { if !reflect.DeepEqual(ctx.StringSlice("intervals"), []string{"20", "30", "40"}) { @@ -525,7 +577,7 @@ func TestParseMultiIntFromEnvCascade(t *testing.T) { func TestParseMultiIntSlice(t *testing.T) { (&App{ Flags: []Flag{ - IntSliceFlag{Name: "serve, s", Value: &IntSlice{}}, + IntSliceFlag{Name: "serve, s", Value: NewIntSlice()}, }, Action: func(ctx *Context) error { if !reflect.DeepEqual(ctx.IntSlice("serve"), []int{10, 20}) { @@ -539,13 +591,64 @@ func TestParseMultiIntSlice(t *testing.T) { }).Run([]string{"run", "-s", "10", "-s", "20"}) } +func TestParseMultiIntSliceWithDefaults(t *testing.T) { + (&App{ + Flags: []Flag{ + IntSliceFlag{Name: "serve, s", Value: NewIntSlice(9, 2)}, + }, + Action: func(ctx *Context) { + if !reflect.DeepEqual(ctx.IntSlice("serve"), []int{10, 20}) { + t.Errorf("main name not set") + } + if !reflect.DeepEqual(ctx.IntSlice("s"), []int{10, 20}) { + t.Errorf("short name not set") + } + }, + }).Run([]string{"run", "-s", "10", "-s", "20"}) +} + +func TestParseMultiIntSliceWithDefaultsUnset(t *testing.T) { + (&App{ + Flags: []Flag{ + IntSliceFlag{Name: "serve, s", Value: NewIntSlice(9, 2)}, + }, + Action: func(ctx *Context) { + if !reflect.DeepEqual(ctx.IntSlice("serve"), []int{9, 2}) { + t.Errorf("main name not set") + } + if !reflect.DeepEqual(ctx.IntSlice("s"), []int{9, 2}) { + t.Errorf("short name not set") + } + }, + }).Run([]string{"run"}) +} + func TestParseMultiIntSliceFromEnv(t *testing.T) { os.Clearenv() os.Setenv("APP_INTERVALS", "20,30,40") (&App{ Flags: []Flag{ - IntSliceFlag{Name: "intervals, i", Value: &IntSlice{}, EnvVar: "APP_INTERVALS"}, + IntSliceFlag{Name: "intervals, i", Value: NewIntSlice(), EnvVar: "APP_INTERVALS"}, + }, + Action: func(ctx *Context) { + if !reflect.DeepEqual(ctx.IntSlice("intervals"), []int{20, 30, 40}) { + t.Errorf("main name not set from env") + } + if !reflect.DeepEqual(ctx.IntSlice("i"), []int{20, 30, 40}) { + t.Errorf("short name not set from env") + } + }, + }).Run([]string{"run"}) +} + +func TestParseMultiIntSliceFromEnvWithDefaults(t *testing.T) { + os.Clearenv() + os.Setenv("APP_INTERVALS", "20,30,40") + + (&App{ + Flags: []Flag{ + IntSliceFlag{Name: "intervals, i", Value: NewIntSlice(1, 2, 5), EnvVar: "APP_INTERVALS"}, }, Action: func(ctx *Context) error { if !reflect.DeepEqual(ctx.IntSlice("intervals"), []int{20, 30, 40}) { @@ -565,7 +668,7 @@ func TestParseMultiIntSliceFromEnvCascade(t *testing.T) { (&App{ Flags: []Flag{ - IntSliceFlag{Name: "intervals, i", Value: &IntSlice{}, EnvVar: "COMPAT_INTERVALS,APP_INTERVALS"}, + IntSliceFlag{Name: "intervals, i", Value: NewIntSlice(), EnvVar: "COMPAT_INTERVALS,APP_INTERVALS"}, }, Action: func(ctx *Context) error { if !reflect.DeepEqual(ctx.IntSlice("intervals"), []int{20, 30, 40}) { @@ -882,3 +985,35 @@ func TestParseGenericFromEnvCascade(t *testing.T) { } a.Run([]string{"run"}) } + +func TestStringSlice_Serialized_Set(t *testing.T) { + sl0 := NewStringSlice("a", "b") + ser0 := sl0.Serialized() + + if len(ser0) < len(slPfx) { + t.Fatalf("serialized shorter than expected: %q", ser0) + } + + sl1 := NewStringSlice("c", "d") + sl1.Set(ser0) + + if sl0.String() != sl1.String() { + t.Fatalf("pre and post serialization do not match: %v != %v", sl0, sl1) + } +} + +func TestIntSlice_Serialized_Set(t *testing.T) { + sl0 := NewIntSlice(1, 2) + ser0 := sl0.Serialized() + + if len(ser0) < len(slPfx) { + t.Fatalf("serialized shorter than expected: %q", ser0) + } + + sl1 := NewIntSlice(3, 4) + sl1.Set(ser0) + + if sl0.String() != sl1.String() { + t.Fatalf("pre and post serialization do not match: %v != %v", sl0, sl1) + } +}