Looks reasonable - some suggestions below.
https://codereview.appspot.com/39150045/diff/1/cmd/args.go File cmd/args.go (right):
https://codereview.appspot.com/39150045/diff/1/cmd/args.go#newcode15 cmd/args.go:15: type StringsValue struct { why not:
type StringsValue []string
?
https://codereview.appspot.com/39150045/diff/1/cmd/args.go#newcode21 cmd/args.go:21: func NewStringsValue(defaultValue []string, target *[]string) *StringsValue { If the type is as above, this function has less value.
https://codereview.appspot.com/39150045/diff/1/cmd/args.go#newcode31 cmd/args.go:31: trimmed := strings.TrimSpace(v) I'm not sure I'd do this - I think it's not a bad thing to be able to include spaces.
If you do decide it's necessary, this behaviour should at least be documented.
https://codereview.appspot.com/39150045/diff/1/cmd/args.go#newcode32 cmd/args.go:32: if trimmed != "" { I definitely wouldn't do this. It's perfectly reasonable to want an empty item in a list.
https://codereview.appspot.com/39150045/
« Back to merge proposal
Looks reasonable - some suggestions below.
https:/ /codereview. appspot. com/39150045/ diff/1/ cmd/args. go
File cmd/args.go (right):
https:/ /codereview. appspot. com/39150045/ diff/1/ cmd/args. go#newcode15
cmd/args.go:15: type StringsValue struct {
why not:
type StringsValue []string
?
https:/ /codereview. appspot. com/39150045/ diff/1/ cmd/args. go#newcode21 (defaultValue []string, target
cmd/args.go:21: func NewStringsValue
*[]string) *StringsValue {
If the type is as above, this function has less value.
https:/ /codereview. appspot. com/39150045/ diff/1/ cmd/args. go#newcode31 TrimSpace( v)
cmd/args.go:31: trimmed := strings.
I'm not sure I'd do this - I think it's not a bad thing to be able to
include spaces.
If you do decide it's necessary, this behaviour should at least be
documented.
https:/ /codereview. appspot. com/39150045/ diff/1/ cmd/args. go#newcode32
cmd/args.go:32: if trimmed != "" {
I definitely wouldn't do this.
It's perfectly reasonable to want an empty item in a list.
https:/ /codereview. appspot. com/39150045/