Please take a look.
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 { On 2013/12/18 23:10:33, rog wrote: > why not:
> type StringsValue []string
I ended up basing this on the constraints.Value work.
Using the type alias now.
https://codereview.appspot.com/39150045/diff/1/cmd/args.go#newcode21 cmd/args.go:21: func NewStringsValue(defaultValue []string, target *[]string) *StringsValue { On 2013/12/18 23:10:33, rog wrote: > If the type is as above, this function has less value.
It is still necessary as it handles the cast and the assignment of values.
https://codereview.appspot.com/39150045/diff/1/cmd/args.go#newcode31 cmd/args.go:31: trimmed := strings.TrimSpace(v) On 2013/12/18 23:10:33, rog wrote: > 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.
well, it was only trimming off leading and trailing spaces, but fair enough
https://codereview.appspot.com/39150045/diff/1/cmd/args.go#newcode32 cmd/args.go:32: if trimmed != "" { On 2013/12/18 23:10:33, rog wrote: > I definitely wouldn't do this. > It's perfectly reasonable to want an empty item in a list.
/me shrugs
ok.
https://codereview.appspot.com/39150045/
« Back to merge proposal
Please take a look.
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 {
On 2013/12/18 23:10:33, rog wrote:
> why not:
> type StringsValue []string
I ended up basing this on the constraints.Value work.
Using the type alias now.
https:/ /codereview. appspot. com/39150045/ diff/1/ cmd/args. go#newcode21 (defaultValue []string, target
cmd/args.go:21: func NewStringsValue
*[]string) *StringsValue {
On 2013/12/18 23:10:33, rog wrote:
> If the type is as above, this function has less value.
It is still necessary as it handles the cast and the assignment of
values.
https:/ /codereview. appspot. com/39150045/ diff/1/ cmd/args. go#newcode31 TrimSpace( v)
cmd/args.go:31: trimmed := strings.
On 2013/12/18 23:10:33, rog wrote:
> 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.
well, it was only trimming off leading and trailing spaces, but fair
enough
https:/ /codereview. appspot. com/39150045/ diff/1/ cmd/args. go#newcode32
cmd/args.go:32: if trimmed != "" {
On 2013/12/18 23:10:33, rog wrote:
> I definitely wouldn't do this.
> It's perfectly reasonable to want an empty item in a list.
/me shrugs
ok.
https:/ /codereview. appspot. com/39150045/