Option.Default field?

[elithrar]

Has an Option.Default field been considered? I prefer to write options without struct tags (since struct tags aren't checked by the compiler, leading to my issue yesterday), but right now you need to write the following:

    {
        Names:       []string{"count"},
        Description: "The (maximum) number of logs to request.",
        Placeholder: "<num>",
        Decoder:     writ.NewDefaulter(writ.NewOptionDecoder(config.Count), "1"),
    },

This isn't bad, but it:

• Limits you to strings - if the field is an int64 or other time (say, for Unix timestamps) then you can't set a default.
• A Default field would be more explicit and would better map against the existing struct tag names.

[bobziuchkovski]

@elithrar The default can actually be anything Option's decoder can decode. The way it works is the default (string) value is passed to the decoder for the field, essentially behaving as though the user specified --count <defaultval>. That's actually what makes default:"-" work to open os.Stdin or os.Stdout in the convenience features example. The decoders returned by writ.NewOptionDecoder for io.Reader/io.Writer fields decode "-" to mean os.Stdin/os.Stdout and any other value to mean a file at the given path.

I thought about adding a Default field, but then that limited the flexibility for defaults. It would preclude environment variable defaults (currently supported by env struct fields and writ.NewEnvDefaulter) or require some sort of complex syntax and handling for those. I felt leaving OptionDefaulter as an optional interface that an OptionDecoder can implement is more flexible. If everything is decoded as a string argument and dispatched to the field's decoder, then it's simple and is conceptually easy to understand. It's just a way of passing an argument and is handled the same way as any other argument, albeit handled before Decode() processes the "real" arguments.

You're definitely welcome to use *writ.Option and *writ.Command directly, and they're exposed the way they are for that reason. However, I actually tried hard to make sure writ.New() performs the type checking analysis you mentioned and provides useful error content so that if your struct tags are semantically incorrect / don't match your field types, you get a panic. That way you get the same type of early feedback in development that you'd get statically from the compiler. The code will compile, but as long as your tests run writ.New(), you'll know whether your struct tags are valid or not before your code goes out the door.

Anyway, I don't think I'd implement the Default field quite as proposed, but I'm still open to alternate ideas. I suppose adding a Defaulter field directly for OptionDefaulter might be okay and be more explicit...I might have to spike that and see...?

[bobziuchkovski]

P.S. If I'm misunderstanding the suggestion, please feel free to correct me. 😄

[bobziuchkovski]

Oh, and for time fields, you can implement an OptionDecoder that knows how to parse and set a time.Time field. Then writ.NewDefaulter(your_time_decoder, "some time value your decoder understands") will work as expected. If you come up with a decoder that can decode time in an unambiguous way, I'm happy to add it as a supported decoder for writ.NewOptionDecoder. Or even if you have suggestions on how to decode time unambiguously, I'm happy to add the support myself (I don't work with time much).

[elithrar]

Great, thanks Bob. In my case I can get away with an int64 decoder (the API
my CLI program uses digests Unix timestamps).

On Tue, Jan 26, 2016 at 10:38 AM Bob Ziuchkovski notifications@github.com
wrote:

Oh, and for time fields, you can implement an OptionDecoder that knows how
to parse and set a time.Time field. Then
writ.NewDefaulter(your_time_decoder, "some time value your decoder
understands") will work as expected. If you come up with a decoder that can
decode time in an unambiguous way, I'm happy to add it as a supported
decoder for writ.NewOptionDecoder. Or even if you have suggestions on how
to decode time unambiguously, I'm happy to add the support myself (I don't
work with time much).

—
Reply to this email directly or view it on GitHub
#2 (comment).

[bobziuchkovski]

Excellent, then yeah, that support is already baked-in. If the field is int64, then the default value will work as expected (it's just going to parse that string default as an int64).

[bobziuchkovski]

@elithrar Just wanted to check-in. Were you able to get your decoders and defaults working? I'm trying to figure out what to do with this issue...whether I need to rethink the default handling or not.

[elithrar]

Thanks for following this up. I did get the decoders/defaults working. I'd still love to be able to specify options via a slice of *writ.Option though, as (like before) a "struct tag based DSL" can be extremely error prone. Typos aren't corrected for and options just disappear until then (which is also very hard to test against). writ isn't just affected by this: a lot of "ORMs" use struct tags to define PKs, FKs, column types, etc. and suffer a similar problem once you grow beyond a small handful of struct tags.

The downside is that you have to construct your own cmd: config, help output, etc (and that's a lot of work compared to calling New()).

[bobziuchkovski]

Yep, I don't plan to remove the explicit Command/Option support. While I like the struct tags personally, I'm definitely aware there's value in having the compiler statically type-check everything.

Just to confirm, the main issues you're facing are:

1. You'd like a more streamlined mechanism for explicit Command/Option construction, so that there's less work involved using those APIs?
2. You'd like a different way to specify default values for Options?

[elithrar]

Yes and yes. Especially #1, but #2 goes (at least for me) hand-in-hand with
that.

e.g. writ.New() with a method to Parse a struct and effectively continue on
as if it were tags would be great.

PS: Hoping to share the CLI client I've built with writ soon: it's been
great at handling a TON of options and the GNU-style flags are more
accessible, even in an organization with a lot of Go users who are
familiar with Go-style flags.

On Sun, Feb 7, 2016 at 8:11 AM Bob Ziuchkovski notifications@github.com
wrote:

Yep, I don't plan to remove the explicit Command/Option support. While I
like the struct tags personally, I'm definitely aware there's value in
having the compiler statically type-check everything.

Just to confirm, the main issues you're facing are:

1. You'd like an a more streamlines mechanism for explicit
   Command/Option construction, so that there's less work involved using those
   APIs?
2. You'd like a different way to specify default values for Options?

—
Reply to this email directly or view it on GitHub
#2 (comment).

[bobziuchkovski]

Gotcha, I'll mull this over. I've gotten the #1 feedback a couple times already, so it's definitely something that is affecting multiple people and something I'd like to resolve. Ideally, it should be just as easy to use either approach. If you have any ideas for what you think this should look like as an API consumer, please feel free to share.

And that's awesome about your CLI client! I'm really thrilled it's been a help to you and your organization. I personally believe that when you're writing a CLI app, option parsing is your UI, so it's important to provide a comfortable user experience through your UI. If writ is meeting that goal, then I'm really happy, though I still want to make it easier to use as a developer.

[bobziuchkovski]

@elithrar I've opened a new PR based on your feedback here. See PR #4 for the changes. Take a look and let me know what you think. I'm still giving thought to how to simplify explicit Option/Command creation, but PR #4 should at least make default values a bit simpler/cleaner.

[elithrar]

@bobziuchkovski That looks solid to me, and I appreciate the effort you've gone into to support this use-case!

[bobziuchkovski]

No problem! Thanks for providing the useful feedback in the first place. Please keep the suggestions coming. My next target is simplifying the explicit Command/Option use case, but I still need to give that one more thought.