Merge pull request #392 from codegangsta/string-slice-overwrite-on-set

Overwrite slice flag defaults when set
This commit is contained in:
Jesse Szwedko 2016-05-05 08:42:14 -07:00
commit eb8680b7d7
7 changed files with 358 additions and 66 deletions

View File

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

View File

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

View File

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

75
cli-migrate-slice-types Executable file
View File

@ -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\\.(?P<type>IntSlice|StringSlice){(?P<args>[^}]*)}',
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())

View File

@ -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())
}

116
flag.go
View File

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

View File

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