From 867aa0912da4ccb7498eac8cb90f07c7abbc4563 Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Tue, 5 Apr 2016 12:35:30 -0400 Subject: [PATCH 1/6] Overwrite slice flag defaults when set Closes #160 --- altsrc/flag.go | 4 +- flag.go | 71 ++++++++++++++++----- flag_test.go | 169 +++++++++++++++++++++++++++++++++++++++---------- 3 files changed, 191 insertions(+), 53 deletions(-) diff --git a/altsrc/flag.go b/altsrc/flag.go index f13ffb4875..286990545e 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/flag.go b/flag.go index e951c2df77..c7b367b5af 100644 --- a/flag.go +++ b/flag.go @@ -111,26 +111,39 @@ 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 + } + + 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) } // 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 @@ -163,10 +176,11 @@ func (f StringSliceFlag) Apply(set *flag.FlagSet) { } } + if f.Value == nil { + f.Value = &StringSlice{} + } + eachName(f.Name, func(name string) { - if f.Value == nil { - f.Value = &StringSlice{} - } set.Var(f.Value, name, f.Usage) }) } @@ -175,28 +189,51 @@ 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 + } + 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) } // 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 diff --git a/flag_test.go b/flag_test.go index 3caa70a4b9..ba8cc0d4c4 100644 --- a/flag_test.go +++ b/flag_test.go @@ -4,9 +4,9 @@ import ( "fmt" "os" "reflect" + "runtime" "strings" "testing" - "runtime" ) var boolFlagTests = []struct { @@ -64,7 +64,7 @@ func TestStringFlagWithEnvVarHelpOutput(t *testing.T) { expectedSuffix = " [%APP_FOO%]" } if !strings.HasSuffix(output, expectedSuffix) { - t.Errorf("%s does not end with" + expectedSuffix, output) + t.Errorf("%s does not end with"+expectedSuffix, output) } } } @@ -74,30 +74,14 @@ var stringSliceFlagTests = []struct { value *StringSlice expected string }{ - {"help", func() *StringSlice { - s := &StringSlice{} - s.Set("") - return s - }(), "--help [--help option --help option]\t"}, - {"h", func() *StringSlice { - s := &StringSlice{} - s.Set("") - return s - }(), "-h [-h option -h option]\t"}, - {"h", func() *StringSlice { - s := &StringSlice{} - s.Set("") - return s - }(), "-h [-h option -h option]\t"}, - {"test", func() *StringSlice { - s := &StringSlice{} - s.Set("Something") - return s - }(), "--test [--test option --test option]\t"}, + {"help", NewStringSlice(""), "--help [--help option --help option]\t"}, + {"h", NewStringSlice(""), "-h [-h option -h option]\t"}, + {"h", NewStringSlice(""), "-h [-h option -h option]\t"}, + {"test", NewStringSlice("Something"), "--test [--test option --test option]\t"}, + {"d, dee", NewStringSlice("Inka", "Dinka", "dooo"), "-d, --dee [-d option -d option]\t"}, } func TestStringSliceFlagHelpOutput(t *testing.T) { - for _, test := range stringSliceFlagTests { flag := StringSliceFlag{Name: test.name, Value: test.value} output := flag.String() @@ -120,7 +104,7 @@ func TestStringSliceFlagWithEnvVarHelpOutput(t *testing.T) { expectedSuffix = " [%APP_QWWX%]" } if !strings.HasSuffix(output, expectedSuffix) { - t.Errorf("%q does not end with" + expectedSuffix, output) + t.Errorf("%q does not end with"+expectedSuffix, output) } } } @@ -157,7 +141,7 @@ func TestIntFlagWithEnvVarHelpOutput(t *testing.T) { expectedSuffix = " [%APP_BAR%]" } if !strings.HasSuffix(output, expectedSuffix) { - t.Errorf("%s does not end with" + expectedSuffix, output) + t.Errorf("%s does not end with"+expectedSuffix, output) } } } @@ -194,7 +178,7 @@ func TestDurationFlagWithEnvVarHelpOutput(t *testing.T) { expectedSuffix = " [%APP_BAR%]" } if !strings.HasSuffix(output, expectedSuffix) { - t.Errorf("%s does not end with" + expectedSuffix, output) + t.Errorf("%s does not end with"+expectedSuffix, output) } } } @@ -207,11 +191,7 @@ var intSliceFlagTests = []struct { {"help", &IntSlice{}, "--help [--help option --help option]\t"}, {"h", &IntSlice{}, "-h [-h option -h option]\t"}, {"h", &IntSlice{}, "-h [-h option -h option]\t"}, - {"test", func() *IntSlice { - i := &IntSlice{} - i.Set("9") - return i - }(), "--test [--test option --test option]\t"}, + {"test", NewIntSlice(9), "--test [--test option --test option]\t"}, } func TestIntSliceFlagHelpOutput(t *testing.T) { @@ -238,7 +218,7 @@ func TestIntSliceFlagWithEnvVarHelpOutput(t *testing.T) { expectedSuffix = " [%APP_SMURF%]" } if !strings.HasSuffix(output, expectedSuffix) { - t.Errorf("%q does not end with" + expectedSuffix, output) + t.Errorf("%q does not end with"+expectedSuffix, output) } } } @@ -275,7 +255,7 @@ func TestFloat64FlagWithEnvVarHelpOutput(t *testing.T) { expectedSuffix = " [%APP_BAZ%]" } if !strings.HasSuffix(output, expectedSuffix) { - t.Errorf("%s does not end with" + expectedSuffix, output) + t.Errorf("%s does not end with"+expectedSuffix, output) } } } @@ -313,7 +293,7 @@ func TestGenericFlagWithEnvVarHelpOutput(t *testing.T) { expectedSuffix = " [%APP_ZAP%]" } if !strings.HasSuffix(output, expectedSuffix) { - t.Errorf("%s does not end with" + expectedSuffix, output) + t.Errorf("%s does not end with"+expectedSuffix, output) } } } @@ -404,6 +384,38 @@ func TestParseMultiStringSlice(t *testing.T) { }).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) { + if !reflect.DeepEqual(ctx.StringSlice("serve"), []string{"10", "20"}) { + t.Errorf("main name not set: %v", ctx.StringSlice("serve")) + } + if !reflect.DeepEqual(ctx.StringSlice("s"), []string{"10", "20"}) { + t.Errorf("short name not set: %v", 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") @@ -423,6 +435,25 @@ func TestParseMultiStringSliceFromEnv(t *testing.T) { }).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) { + 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 TestParseMultiStringSliceFromEnvCascade(t *testing.T) { os.Clearenv() os.Setenv("APP_INTERVALS", "20,30,40") @@ -442,6 +473,25 @@ func TestParseMultiStringSliceFromEnvCascade(t *testing.T) { }).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) { + 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 TestParseMultiInt(t *testing.T) { a := App{ Flags: []Flag{ @@ -531,6 +581,38 @@ 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") @@ -550,6 +632,25 @@ func TestParseMultiIntSliceFromEnv(t *testing.T) { }).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) { + 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 TestParseMultiIntSliceFromEnvCascade(t *testing.T) { os.Clearenv() os.Setenv("APP_INTERVALS", "20,30,40") From cb433e7468f95fdf70780e87cf770e4a1c2ebabf Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Tue, 5 Apr 2016 22:38:31 -0400 Subject: [PATCH 2/6] Use NewIntSlice and NewStringSlice internally --- app_test.go | 4 ++-- flag.go | 11 ++++++----- flag_test.go | 18 +++++++++--------- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/app_test.go b/app_test.go index ebf26c70fb..0d1820e7b9 100644 --- a/app_test.go +++ b/app_test.go @@ -313,8 +313,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) { parsedIntSlice = c.IntSlice("p") diff --git a/flag.go b/flag.go index c7b367b5af..f93d3e85d2 100644 --- a/flag.go +++ b/flag.go @@ -165,7 +165,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) @@ -177,7 +177,7 @@ func (f StringSliceFlag) Apply(set *flag.FlagSet) { } if f.Value == nil { - f.Value = &StringSlice{} + f.Value = NewStringSlice() } eachName(f.Name, func(name string) { @@ -272,10 +272,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 ba8cc0d4c4..3bc6be7ff8 100644 --- a/flag_test.go +++ b/flag_test.go @@ -188,9 +188,9 @@ var intSliceFlagTests = []struct { value *IntSlice expected string }{ - {"help", &IntSlice{}, "--help [--help option --help option]\t"}, - {"h", &IntSlice{}, "-h [-h option -h option]\t"}, - {"h", &IntSlice{}, "-h [-h option -h option]\t"}, + {"help", NewIntSlice(), "--help [--help option --help option]\t"}, + {"h", NewIntSlice(), "-h [-h option -h option]\t"}, + {"h", NewIntSlice(), "-h [-h option -h option]\t"}, {"test", NewIntSlice(9), "--test [--test option --test option]\t"}, } @@ -371,7 +371,7 @@ 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) { if !reflect.DeepEqual(ctx.StringSlice("serve"), []string{"10", "20"}) { @@ -422,7 +422,7 @@ func TestParseMultiStringSliceFromEnv(t *testing.T) { (&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"}) { @@ -460,7 +460,7 @@ 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"}) { @@ -568,7 +568,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) { if !reflect.DeepEqual(ctx.IntSlice("serve"), []int{10, 20}) { @@ -619,7 +619,7 @@ func TestParseMultiIntSliceFromEnv(t *testing.T) { (&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}) { @@ -657,7 +657,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) { if !reflect.DeepEqual(ctx.IntSlice("intervals"), []int{20, 30, 40}) { From 7a5bfc850d32c2062bdaefb8aaa33347119d95ba Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Tue, 5 Apr 2016 22:40:12 -0400 Subject: [PATCH 3/6] Update dangling IntSlice literal --- flag.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flag.go b/flag.go index f93d3e85d2..a96325e2ed 100644 --- a/flag.go +++ b/flag.go @@ -258,7 +258,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) From 47f6721faea7f216e7f355ed2ca5a0b1d193a3f7 Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Thu, 28 Apr 2016 12:35:56 -0400 Subject: [PATCH 4/6] Update CHANGELOG and add a script to help with slice type migration --- CHANGELOG.md | 10 ++++++ cli-migrate-slice-types | 75 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100755 cli-migrate-slice-types diff --git a/CHANGELOG.md b/CHANGELOG.md index 074bfb2d6a..f17f2d8857 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ ### Added - Support for placeholders in flag usage strings +## [2.0.0] +### 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). + ## [1.14.0] - 2016-04-03 (backfilled 2016-04-25) ### Added - Codebeat badge @@ -220,6 +229,7 @@ - Initial implementation. [Unreleased]: https://github.com/codegangsta/cli/compare/v1.14.0...HEAD +[2.0.0]: https://github.com/codegangsta/cli/compare/v1.14.0...v2.0.0 [1.14.0]: https://github.com/codegangsta/cli/compare/v1.13.0...v1.14.0 [1.13.0]: https://github.com/codegangsta/cli/compare/v1.12.0...v1.13.0 [1.12.0]: https://github.com/codegangsta/cli/compare/v1.11.1...v1.12.0 diff --git a/cli-migrate-slice-types b/cli-migrate-slice-types new file mode 100755 index 0000000000..5c6cb1fe86 --- /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()) From d1b0c49a988426023ab923a5b3b6baf4fae372df Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Fri, 29 Apr 2016 02:30:49 -0400 Subject: [PATCH 5/6] Ensure slice types can safely round-trip through flag.FlagSet --- context.go | 3 ++- flag.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/context.go b/context.go index c542a67b78..a4c3904d7a 100644 --- a/context.go +++ b/context.go @@ -362,7 +362,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 95eefc4edc..38029c4c15 100644 --- a/flag.go +++ b/flag.go @@ -1,6 +1,7 @@ package cli import ( + "encoding/json" "flag" "fmt" "os" @@ -10,6 +11,10 @@ import ( "time" ) +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", @@ -29,6 +34,11 @@ var HelpFlag = BoolFlag{ Usage: "show help", } +// 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. @@ -130,6 +140,13 @@ func (f *StringSlice) Set(value string) error { f.hasBeenSet = true } + if strings.HasPrefix(value, slPfx) { + v := []string{} + _ = json.Unmarshal([]byte(strings.Replace(value, slPfx, "", 1)), v) + f.slice = append(f.slice, v...) + return nil + } + f.slice = append(f.slice, value) return nil } @@ -139,6 +156,12 @@ func (f *StringSlice) String() string { 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.slice @@ -219,6 +242,13 @@ func (i *IntSlice) Set(value string) error { i.hasBeenSet = true } + if strings.HasPrefix(slPfx, value) { + v := []int{} + _ = json.Unmarshal([]byte(strings.Replace(value, slPfx, "", 1)), v) + i.slice = append(i.slice, v...) + return nil + } + tmp, err := strconv.Atoi(value) if err != nil { return err From 1a91f3dce5b5847ac96cd3f459efe2fcb4ba29ba Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Fri, 29 Apr 2016 02:53:58 -0400 Subject: [PATCH 6/6] Ensure IntSlice & StringSlice serialization works as expected --- flag.go | 20 +++++++++++++------- flag_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 55 insertions(+), 16 deletions(-) diff --git a/flag.go b/flag.go index 38029c4c15..a5c8d2cfcd 100644 --- a/flag.go +++ b/flag.go @@ -141,9 +141,9 @@ func (f *StringSlice) Set(value string) error { } if strings.HasPrefix(value, slPfx) { - v := []string{} - _ = json.Unmarshal([]byte(strings.Replace(value, slPfx, "", 1)), v) - f.slice = append(f.slice, v...) + // Deserializing assumes overwrite + _ = json.Unmarshal([]byte(strings.Replace(value, slPfx, "", 1)), &f.slice) + f.hasBeenSet = true return nil } @@ -242,10 +242,10 @@ func (i *IntSlice) Set(value string) error { i.hasBeenSet = true } - if strings.HasPrefix(slPfx, value) { - v := []int{} - _ = json.Unmarshal([]byte(strings.Replace(value, slPfx, "", 1)), v) - i.slice = append(i.slice, v...) + if strings.HasPrefix(value, slPfx) { + // Deserializing assumes overwrite + _ = json.Unmarshal([]byte(strings.Replace(value, slPfx, "", 1)), &i.slice) + i.hasBeenSet = true return nil } @@ -263,6 +263,12 @@ 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 (i *IntSlice) Value() []int { return i.slice diff --git a/flag_test.go b/flag_test.go index 1288a73321..843f175ab5 100644 --- a/flag_test.go +++ b/flag_test.go @@ -43,7 +43,6 @@ var stringFlagTests = []struct { } func TestStringFlagHelpOutput(t *testing.T) { - for _, test := range stringFlagTests { flag := StringFlag{Name: test.name, Usage: test.usage, Value: test.value} output := flag.String() @@ -376,11 +375,12 @@ func TestParseMultiStringSlice(t *testing.T) { StringSliceFlag{Name: "serve, s", Value: NewStringSlice()}, }, Action: func(ctx *Context) { - 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")) } }, }).Run([]string{"run", "-s", "10", "-s", "20"}) @@ -392,11 +392,12 @@ func TestParseMultiStringSliceWithDefaults(t *testing.T) { StringSliceFlag{Name: "serve, s", Value: NewStringSlice("9", "2")}, }, Action: func(ctx *Context) { - if !reflect.DeepEqual(ctx.StringSlice("serve"), []string{"10", "20"}) { - t.Errorf("main name not set: %v", ctx.StringSlice("serve")) + 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: %v", ctx.StringSlice("s")) + 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"}) @@ -960,3 +961,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) + } +}