Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StringsOption with delimiter #204

Merged
merged 6 commits into from
Jan 27, 2021
Merged

StringsOption with delimiter #204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
382fd11
feat: StringsOption.Parse(string) now returns a slice containing the …
aschmahmann Jan 14, 2021
cde56b1
testing: added DelimitedStringsOption parsing tests
aschmahmann Jan 15, 2021
d48b43d
testing: refactor to extract testOptionHelper for reuse
aschmahmann Jan 15, 2021
16d8195
feat: calling WithDefaults with the wrong type now panics
aschmahmann Jan 15, 2021
e2c3e22
testing: add tests for constructing Options with defaults and parsing…
aschmahmann Jan 15, 2021
c6690cc
feat: DelimitedStringsOption now panics if an empty delimiter is pass…
aschmahmann Jan 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cli/parse.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func setOpts(kv kv, kvType reflect.Kind, opts cmds.OptMap) error {

if kvType == cmds.Strings {
res, _ := opts[kv.Key].([]string)
opts[kv.Key] = append(res, kv.Value.(string))
opts[kv.Key] = append(res, kv.Value.([]string)...)
} else if _, exists := opts[kv.Key]; !exists {
opts[kv.Key] = kv.Value
} else {
Expand Down
190 changes: 170 additions & 20 deletions cli/parse_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,32 @@ func TestSameWords(t *testing.T) {
test(f, f, true)
}

func testOptionHelper(t *testing.T, cmd *cmds.Command, args string, expectedOpts kvs, expectedWords words, expectErr bool) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was just extracted from the function below

req := &cmds.Request{}
err := parse(req, strings.Split(args, " "), cmd)
if err == nil {
err = req.FillDefaults()
}
if expectErr {
if err == nil {
t.Errorf("Command line '%v' parsing should have failed", args)
}
} else if err != nil {
t.Errorf("Command line '%v' failed to parse: %v", args, err)
} else if !sameWords(req.Arguments, expectedWords) || !sameKVs(kvs(req.Options), expectedOpts) {
t.Errorf("Command line '%v':\n parsed as %v %v\n instead of %v %v",
args, req.Options, req.Arguments, expectedOpts, expectedWords)
}
}

func TestOptionParsing(t *testing.T) {
cmd := &cmds.Command{
Options: []cmds.Option{
cmds.StringOption("string", "s", "a string"),
cmds.StringOption("flag", "alias", "multiple long"),
cmds.BoolOption("bool", "b", "a bool"),
cmds.StringsOption("strings", "r", "strings array"),
cmds.DelimitedStringsOption(",", "delimstrings", "d", "comma delimited string array"),
},
Subcommands: map[string]*cmds.Command{
"test": &cmds.Command{},
Expand All @@ -95,30 +114,12 @@ func TestOptionParsing(t *testing.T) {
},
}

testHelper := func(args string, expectedOpts kvs, expectedWords words, expectErr bool) {
req := &cmds.Request{}
err := parse(req, strings.Split(args, " "), cmd)
if err == nil {
err = req.FillDefaults()
}
if expectErr {
if err == nil {
t.Errorf("Command line '%v' parsing should have failed", args)
}
} else if err != nil {
t.Errorf("Command line '%v' failed to parse: %v", args, err)
} else if !sameWords(req.Arguments, expectedWords) || !sameKVs(kvs(req.Options), expectedOpts) {
t.Errorf("Command line '%v':\n parsed as %v %v\n instead of %v %v",
args, req.Options, req.Arguments, expectedOpts, expectedWords)
}
}

testFail := func(args string) {
testHelper(args, kvs{}, words{}, true)
testOptionHelper(t, cmd, args, kvs{}, words{}, true)
}

test := func(args string, expectedOpts kvs, expectedWords words) {
testHelper(args, expectedOpts, expectedWords, false)
testOptionHelper(t, cmd, args, expectedOpts, expectedWords, false)
}

test("test -", kvs{}, words{"-"})
Expand Down Expand Up @@ -154,6 +155,13 @@ func TestOptionParsing(t *testing.T) {
test("-b --string foo test bar", kvs{"bool": true, "string": "foo"}, words{"bar"})
test("-b=false --string bar", kvs{"bool": false, "string": "bar"}, words{})
test("--strings a --strings b", kvs{"strings": []string{"a", "b"}}, words{})

test("--delimstrings a,b", kvs{"delimstrings": []string{"a", "b"}}, words{})
test("--delimstrings=a,b", kvs{"delimstrings": []string{"a", "b"}}, words{})
test("-d a,b", kvs{"delimstrings": []string{"a", "b"}}, words{})
test("-d=a,b", kvs{"delimstrings": []string{"a", "b"}}, words{})
test("-d=a,b -d c --delimstrings d", kvs{"delimstrings": []string{"a", "b", "c", "d"}}, words{})

testFail("foo test")
test("defaults", kvs{"opt": "def"}, words{})
test("defaults -o foo", kvs{"opt": "foo"}, words{})
Expand All @@ -170,6 +178,148 @@ func TestOptionParsing(t *testing.T) {
testFail("-zz--- --")
}

func TestDefaultOptionParsing(t *testing.T) {
testPanic := func(f func()) {
fnFinished := false
defer func() {
if r := recover(); fnFinished == true {
panic(r)
}
}()
f()
fnFinished = true
t.Error("expected panic")
}

testPanic(func() { cmds.StringOption("string", "s", "a string").WithDefault(0) })
testPanic(func() { cmds.StringOption("string", "s", "a string").WithDefault(false) })
testPanic(func() { cmds.StringOption("string", "s", "a string").WithDefault(nil) })
testPanic(func() { cmds.StringOption("string", "s", "a string").WithDefault([]string{"foo"}) })
testPanic(func() { cmds.StringsOption("strings", "a", "a string array").WithDefault(0) })
testPanic(func() { cmds.StringsOption("strings", "a", "a string array").WithDefault(false) })
testPanic(func() { cmds.StringsOption("strings", "a", "a string array").WithDefault(nil) })
testPanic(func() { cmds.StringsOption("strings", "a", "a string array").WithDefault("foo") })
testPanic(func() { cmds.StringsOption("strings", "a", "a string array").WithDefault([]bool{false}) })
testPanic(func() { cmds.DelimitedStringsOption(",", "dstrings", "d", "delimited string array").WithDefault(0) })
testPanic(func() { cmds.DelimitedStringsOption(",", "dstrs", "d", "delimited string array").WithDefault(false) })
testPanic(func() { cmds.DelimitedStringsOption(",", "dstrings", "d", "delimited string array").WithDefault(nil) })
testPanic(func() { cmds.DelimitedStringsOption(",", "dstrs", "d", "delimited string array").WithDefault("foo") })
testPanic(func() { cmds.DelimitedStringsOption(",", "dstrs", "d", "delimited string array").WithDefault([]int{0}) })

testPanic(func() { cmds.BoolOption("bool", "b", "a bool").WithDefault(0) })
testPanic(func() { cmds.BoolOption("bool", "b", "a bool").WithDefault(1) })
testPanic(func() { cmds.BoolOption("bool", "b", "a bool").WithDefault(nil) })
testPanic(func() { cmds.BoolOption("bool", "b", "a bool").WithDefault([]bool{false}) })
testPanic(func() { cmds.BoolOption("bool", "b", "a bool").WithDefault([]string{"foo"}) })

testPanic(func() { cmds.UintOption("uint", "u", "a uint").WithDefault(int(0)) })
testPanic(func() { cmds.UintOption("uint", "u", "a uint").WithDefault(int32(0)) })
testPanic(func() { cmds.UintOption("uint", "u", "a uint").WithDefault(int64(0)) })
testPanic(func() { cmds.UintOption("uint", "u", "a uint").WithDefault(uint64(0)) })
testPanic(func() { cmds.UintOption("uint", "u", "a uint").WithDefault(uint32(0)) })
testPanic(func() { cmds.UintOption("uint", "u", "a uint").WithDefault(float32(0)) })
testPanic(func() { cmds.UintOption("uint", "u", "a uint").WithDefault(float64(0)) })
testPanic(func() { cmds.UintOption("uint", "u", "a uint").WithDefault(nil) })
testPanic(func() { cmds.UintOption("uint", "u", "a uint").WithDefault([]uint{0}) })
testPanic(func() { cmds.UintOption("uint", "u", "a uint").WithDefault([]string{"foo"}) })
testPanic(func() { cmds.Uint64Option("uint64", "v", "a uint64").WithDefault(int(0)) })
testPanic(func() { cmds.Uint64Option("uint64", "v", "a uint64").WithDefault(int32(0)) })
testPanic(func() { cmds.Uint64Option("uint64", "v", "a uint64").WithDefault(int64(0)) })
testPanic(func() { cmds.Uint64Option("uint64", "v", "a uint64").WithDefault(uint(0)) })
testPanic(func() { cmds.Uint64Option("uint64", "v", "a uint64").WithDefault(uint32(0)) })
testPanic(func() { cmds.Uint64Option("uint64", "v", "a uint64").WithDefault(float32(0)) })
testPanic(func() { cmds.Uint64Option("uint64", "v", "a uint64").WithDefault(float64(0)) })
testPanic(func() { cmds.Uint64Option("uint64", "v", "a uint64").WithDefault(nil) })
testPanic(func() { cmds.Uint64Option("uint64", "v", "a uint64").WithDefault([]uint64{0}) })
testPanic(func() { cmds.Uint64Option("uint64", "v", "a uint64").WithDefault([]string{"foo"}) })
testPanic(func() { cmds.IntOption("int", "i", "an int").WithDefault(int32(0)) })
testPanic(func() { cmds.IntOption("int", "i", "an int").WithDefault(int64(0)) })
testPanic(func() { cmds.IntOption("int", "i", "an int").WithDefault(uint(0)) })
testPanic(func() { cmds.IntOption("int", "i", "an int").WithDefault(uint32(0)) })
testPanic(func() { cmds.IntOption("int", "i", "an int").WithDefault(uint64(0)) })
testPanic(func() { cmds.IntOption("int", "i", "an int").WithDefault(float32(0)) })
testPanic(func() { cmds.IntOption("int", "i", "an int").WithDefault(float64(0)) })
testPanic(func() { cmds.IntOption("int", "i", "an int").WithDefault(nil) })
testPanic(func() { cmds.IntOption("int", "i", "an int").WithDefault([]int{0}) })
testPanic(func() { cmds.IntOption("int", "i", "an int").WithDefault([]string{"foo"}) })
testPanic(func() { cmds.Int64Option("int64", "j", "an int64").WithDefault(int(0)) })
testPanic(func() { cmds.Int64Option("int64", "j", "an int64").WithDefault(int32(0)) })
testPanic(func() { cmds.Int64Option("int64", "j", "an int64").WithDefault(uint(0)) })
testPanic(func() { cmds.Int64Option("int64", "j", "an int64").WithDefault(uint32(0)) })
testPanic(func() { cmds.Int64Option("int64", "j", "an int64").WithDefault(uint64(0)) })
testPanic(func() { cmds.Int64Option("int64", "j", "an int64").WithDefault(float32(0)) })
testPanic(func() { cmds.Int64Option("int64", "j", "an int64").WithDefault(float64(0)) })
testPanic(func() { cmds.Int64Option("int64", "j", "an int64").WithDefault(nil) })
testPanic(func() { cmds.Int64Option("int64", "j", "an int64").WithDefault([]int64{0}) })
testPanic(func() { cmds.Int64Option("int64", "j", "an int64").WithDefault([]string{"foo"}) })
testPanic(func() { cmds.FloatOption("float", "f", "a float64").WithDefault(int(0)) })
testPanic(func() { cmds.FloatOption("float", "f", "a float64").WithDefault(int32(0)) })
testPanic(func() { cmds.FloatOption("float", "f", "a float64").WithDefault(int64(0)) })
testPanic(func() { cmds.FloatOption("float", "f", "a float64").WithDefault(uint(0)) })
testPanic(func() { cmds.FloatOption("float", "f", "a float64").WithDefault(uint32(0)) })
testPanic(func() { cmds.FloatOption("float", "f", "a float64").WithDefault(uint64(0)) })
testPanic(func() { cmds.FloatOption("float", "f", "a float64").WithDefault(float32(0)) })
testPanic(func() { cmds.FloatOption("float", "f", "a float64").WithDefault(nil) })
testPanic(func() { cmds.FloatOption("float", "f", "a float64").WithDefault([]int{0}) })
testPanic(func() { cmds.FloatOption("float", "f", "a float64").WithDefault([]string{"foo"}) })

cmd := &cmds.Command{
Subcommands: map[string]*cmds.Command{
"defaults": &cmds.Command{
Options: []cmds.Option{
cmds.StringOption("string", "s", "a string").WithDefault("foo"),
cmds.StringsOption("strings1", "a", "a string array").WithDefault([]string{"foo"}),
cmds.StringsOption("strings2", "b", "a string array").WithDefault([]string{"foo", "bar"}),
cmds.DelimitedStringsOption(",", "dstrings1", "c", "a delimited string array").WithDefault([]string{"foo"}),
cmds.DelimitedStringsOption(",", "dstrings2", "d", "a delimited string array").WithDefault([]string{"foo", "bar"}),

cmds.BoolOption("boolT", "t", "a bool").WithDefault(true),
cmds.BoolOption("boolF", "a bool").WithDefault(false),

cmds.UintOption("uint", "u", "a uint").WithDefault(uint(1)),
cmds.Uint64Option("uint64", "v", "a uint64").WithDefault(uint64(1)),
cmds.IntOption("int", "i", "an int").WithDefault(int(1)),
cmds.Int64Option("int64", "j", "an int64").WithDefault(int64(1)),
cmds.FloatOption("float", "f", "a float64").WithDefault(float64(1)),
},
},
},
}

test := func(args string, expectedOpts kvs, expectedWords words) {
testOptionHelper(t, cmd, args, expectedOpts, expectedWords, false)
}

test("defaults", kvs{
"string": "foo",
"strings1": []string{"foo"},
"strings2": []string{"foo", "bar"},
"dstrings1": []string{"foo"},
"dstrings2": []string{"foo", "bar"},
"boolT": true,
"boolF": false,
"uint": uint(1),
"uint64": uint64(1),
"int": int(1),
"int64": int64(1),
"float": float64(1),
}, words{})
test("defaults --string baz --strings1=baz -b baz -b=foo -c=foo -d=foo,baz,bing -d=zip,zap -d=zorp -t=false --boolF -u=0 -v=10 -i=-5 -j=10 -f=-3.14", kvs{
"string": "baz",
"strings1": []string{"baz"},
"strings2": []string{"baz", "foo"},
"dstrings1": []string{"foo"},
"dstrings2": []string{"foo", "baz", "bing", "zip", "zap", "zorp"},
"boolT": false,
"boolF": true,
"uint": uint(0),
"uint64": uint64(10),
"int": int(-5),
"int64": int64(10),
"float": float64(-3.14),
}, words{})
}

func TestArgumentParsing(t *testing.T) {
rootCmd := &cmds.Command{
Subcommands: map[string]*cmds.Command{
Expand Down
58 changes: 57 additions & 1 deletion option.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,18 @@ func NewOption(kind reflect.Kind, names ...string) Option {
}

func (o *option) WithDefault(v interface{}) Option {
if v == nil {
panic(fmt.Errorf("cannot use nil as a default"))
}
Comment on lines +152 to +154
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not seeing any problem with this ATM, it seems like so far all of our types are concrete and even for the pointer types we do have (i.e. []string) as long as people pass in var emptyStingrArray []string instead of just the untyped nil it will be fine.


// if type of value does not match the option type
if vKind, oKind := reflect.TypeOf(v).Kind(), o.Type(); vKind != oKind {
aschmahmann marked this conversation as resolved.
Show resolved Hide resolved
// if the reason they do not match is not because of Slice vs Array equivalence
// Note: Figuring out if the type of Slice/Array matches is not done in this function
if !((vKind == reflect.Array || vKind == reflect.Slice) && (oKind == reflect.Array || oKind == reflect.Slice)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be removed, but basically is just a hack for StringsOption so that it can still opaquely call this function and everything works. Doesn't seem too unreasonable though.

panic(fmt.Errorf("invalid default for the given type, expected %s got %s", o.Type(), vKind))
}
}
o.defaultVal = v
return o
}
Expand Down Expand Up @@ -184,6 +196,50 @@ func FloatOption(names ...string) Option {
func StringOption(names ...string) Option {
return NewOption(String, names...)
}

// StringsOption is a command option that can handle a slice of strings
func StringsOption(names ...string) Option {
return NewOption(Strings, names...)
return &stringsOption{
Option: NewOption(Strings, names...),
delimiter: "",
}
}

// DelimitedStringsOption like StringsOption is a command option that can handle a slice of strings.
// However, DelimitedStringsOption will automatically break up the associated CLI inputs based on the delimiter.
// For example, instead of passing `command --option=val1 --option=val2` you can pass `command --option=val1,val2` or
// even `command --option=val1,val2 --option=val3,val4`.
//
// A delimiter of "" is invalid
func DelimitedStringsOption(delimiter string, names ...string) Option {
aschmahmann marked this conversation as resolved.
Show resolved Hide resolved
if delimiter == "" {
panic("cannot create a DelimitedStringsOption with no delimiter")
}
return &stringsOption{
Option: NewOption(Strings, names...),
delimiter: delimiter,
}
}

type stringsOption struct {
Option
delimiter string
}

func (s *stringsOption) WithDefault(v interface{}) Option {
if v == nil {
return s.Option.WithDefault(v)
}

defVal := v.([]string)
s.Option = s.Option.WithDefault(defVal)
return s
}

func (s *stringsOption) Parse(v string) (interface{}, error) {
if s.delimiter == "" {
return []string{v}, nil
}

return strings.Split(v, s.delimiter), nil
}