Add required an default value support #107
Conversation
This code emits a compiler crash with zig-0.11
voroskoi
force-pushed
the
default-optional
branch
from
3f1325d to
62e713f
Compare
| @@ -92,6 +92,7 @@ pub fn Param(comptime Id: type) type { | |||
| id: Id, | |||
| names: Names = Names{}, | |||
| takes_value: Values = .none, | |||
| required: bool = false, | |||
There was a problem hiding this comment.
I think required only makes sense for things that takes values and probably only for taking 1 value. So I would modify the Values enum to something along the lines of:
pub const Values = enum {
none,
zero_or_one,
one,
many,
};
I'd argue that there is no place for optional and required many. Optional many is 0..N values and required would be 1..N. But why only support 1..N and not 2..N, 3..N and so on.
There was a problem hiding this comment.
Ofc, the question then becomes. How is many parsed? Is it --many <v>... or --many [v]...? I would argue for <v>... as each value is not optional, but there can be 0..N of them
There was a problem hiding this comment.
For me [v]... means that I can omit the flag. <v>... indicates that I have to add the flag with at least one value. This could be even enhanced later to at least k values, but that does not seem a severe requirement to me.
If I want only the flag without value there is <bool>.
There was a problem hiding this comment.
For me [v]... means that I can omit the flag. ... indicates that I have to add the flag with at least one value.
Hmm. Alright. I think we have to look out and see what the standard is here and pick that :) I'll try to have a look around later.
There was a problem hiding this comment.
I quite like this: http://docopt.org/
The project is sort-of dead, but the idea is nice imo. After element... You can find a couple of examples.
| /// Parse boolean value. False, off and 0 considered false, everything else is true. | ||
| pub fn boolean(in: []const u8) error{ParameterTooLong}!bool { | ||
| // check if input is not too long | ||
| const MAX_LEN = 16; | ||
| if (in.len >= MAX_LEN) return error.ParameterTooLong; | ||
|
|
||
| // convert to lowercase | ||
| var lower: [MAX_LEN]u8 = undefined; | ||
| for (in, 0..) |c, i| { | ||
| lower[i] = std.ascii.toLower(c); | ||
| } | ||
|
|
||
| if (std.mem.eql(u8, "0", lower[0..in.len]) or | ||
| std.mem.eql(u8, "off", lower[0..in.len]) or | ||
| std.mem.eql(u8, "false", lower[0..in.len])) return false; | ||
| return true; | ||
| } | ||
|
|
There was a problem hiding this comment.
Hmm. Idk about this one. This seems quite opinionated and I can easily imagine anyone would expect different behaviour for bool values.
- Not wanting the default to
truebehaviour. If someone passesofthat does not seem like atrueto me. yes/nocould be wanted- Someone might not wanna have
0/1as that seems quite "programmer" specific
People should use flags for boolean options in most cases. If they don't want a default, they can easily just use parsers.enumeration(enum { on, off }) or whatever else they like.
There was a problem hiding this comment.
Thanks for the feedback, I will make those changes.
|
This idea got dropped, see discussion in original issue. |
This is not ready yet, I just push it for reference.
Tasks:
?valueand the required is justvalueboolfor supporting on/off switches0/off/falseas off and everything else as onCloses #82