-
-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Required arguments #30
base: master
Are you sure you want to change the base?
Conversation
@@ -98,7 +97,22 @@ fn parseInternal(comptime Generic: type, comptime MaybeVerb: ?type, args_iterato | |||
|
|||
var last_error: ?anyerror = null; | |||
|
|||
while (args_iterator.next()) |item| { | |||
// Create map for required arguments | |||
var required_map = std.StringHashMap(bool).init(allocator); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you replace the StringHashMap
with a std.EnumSet
? As we know all possible fields, we can use std.meta.FieldEnum
instead of runtime allocation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm having some trouble using the EnumSet. How do you get a proper enum to insert in this set from a field? Also, how would this work with the required items for the tagged enum of the Verb set, do we need a set per verb?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do you get a proper enum to insert in this set from a field?
const Fields = std.meta.FieldEnum(Type);
const field_member = @field(Fields, field.name);
set.insert(field_member);
Also, how would this work with the required items for the tagged enum of the Verb set, do we need a set per verb?
Yes, but that would not hurt much, as we have to process the verb separately anyways. As we only process more errors when mandatory verb parameters are missing, this should not be much of a problem
Added option to not set the default argument of a struct variable, to make it required. Also got rid of the 'found' variable by using a blocked while.