Open EpicEric opened 5 years ago
The reason I didn't make them partial is that at consumption time, it is quite inconvenient for the user. I do acknowledge that silently doing the wrong thing is not great either. Maybe we can come up with better sane conversions and error handling at parse time for cases where we can't convert.
@EpicEric any thoughts on this?
I still believe the best solution is the one I put forward -- make these functions partial.
I tend to agree @EpicEric.
I would consider the existing functionality a bug.
@ponylang/committer, any input?
@EpicEric I think the other option would be to use a result type.
I would consider the existing functionality a bug. @ponylang/committer, any input?
I don't consider it to be a bug because it's working as designed by Carl. We may not all agree with the design, but it was part of the design in the RFC to adopt the package (I assume, but I did not actually go back and look at it).
I personally think that the signature of the flag "getters" should remain as they are, for the convenience reasons that Carl mentioned above. However, we should have a way to report them as explicit errors at parse time, rather than at "value get" time.
That is, I believe it makes sense to handle all errors from CLI arg parsing in a first phase, before you proceed to assign those values to your program's data structures or use them in whatever way you deem fit.
This approach would give you just one place to deal with the error and a convenient way to print a user-friendly error list to show the user, rather than having to handle the partial result (or Result value) at every call site for every arg getter, and then figure out how to format human friendly errors for each one yourself.
This is caused by a programmer error, not a parser error. The problem that triggered this issue was when the user defined an ArgSpec.u64
and later tried to fetch the value with cmd.arg(...).i64()
instead of the correct cmd.arg(...).u64()
. Here's the original playground for clarity:
https://playground.ponylang.io/?gist=71ac00a9662bedfeac0541a43d3acc1c
I don't see how keeping the signatures and avoiding the issue would be possible.
@jemc I didn't really think through the ramifications in the initial RFC. Perhaps "bug" is the wrong word, but I think it's a flaw.
Perhaps there's a way to leverage the type system more. Really what we are talking about is creating a generic type that if you say "this returns a U64", you can only get back and only try to get back a U64.
In Scala, you'd do this with a type safe builder. I think seeing what we can do with Pony's type system towards that end would be a good idea.
Brought up on Zulip.
Arg.i64()
(or any other function incli.Arg
/cli.Option
returning its arg value) will silently return a default value if its actual value has a different type (eg. callingArg.i64()
when the arg value isU64
will always returnI64(0)
). This can result in unexpected values (0
,false
,""
) that will cause programs to completely ignore command-line input for a given parameter.The most straight-forward solution is to make these functions partial and let the user handle any mistakes in the code. In that case, we'd have to see if missing options/args with default values will still work (i.e. not raise an error) with
cli
.