Comment 4 for bug 1637814

Revision history for this message
John A Meinel (jameinel) wrote :

As for the "why", it is because application and machine are lists vs model being a single string. And the way it formats its value is:
// Implements gnuflag.Value String.
func (v *StringsValue) String() string {
 return strings.Join(*v, ",")
}

And when len(v) is empty, you get nothing out. And the code that handles rendering the help template uses:
  format := "%s (= %s)\n %s\n"
  if _, ok := fs[0].Value.(*stringValue); ok {
   // put quotes on the value
   format = "%s (= %q)\n %s\n"
  }
  fmt.Fprintf(f.out(), format, line.Bytes(), fs[0].DefValue, fs[0].Usage)
 }

Which says that only if the exact content can be cast to a pointer-to-string does it format using quoted form (thus an empty string shows up as "", rather than literal nothing.)

We could probably change StringsValue to return a quoted form always:
func (v *StringsValue) String() string {
 return strconv.Quote(strings.Join(*v, ","))
}

I think that is better than '[]' because *users* don't supply them as a list, they supply "app1,app2", so we should show that it is a string that we expect.

--no-browser-login is certainly an interesting case where gnuflag/flag doesn't provide a clear way to indicate that you would want two flags "--browser-login" *and* "--no-browser-login" and a way to provide what setting is default for that boolean. (The 'go' way of doing it is to provide a single boolean and the user negates it with '--foo' means '--foo=true' and '--foo=false' is the way of turning off that option rather than '--no-foo', but that doesn't match a lot of other CLIs that people are used to.)

Since *Juju* doesn't follow the flag conventions of booleans and =false to disable them, I think having the (= false) is more harmful/noise than helpful. Though we don't actually have a way to target the same boolean (so all our booleans except for exceptional circumstances are default false so that you can just supply the option without =true for it to work as people expect.)