Code review comment for lp:~thumper/juju-core/support-string-slice-args

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/