-
Notifications
You must be signed in to change notification settings - Fork 0
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
more options to parse #20
Conversation
support options mentioned in #16
Maybe, if @reynir has some time, could do a review? Thanks a lot. |
A local `ignore_line` was shadowing the outer `ignore_line` which was what we want for parsing comments; so the local `ignore_line` is renamed to `ignore_directive`. lease_time without a 'm', 'h' or similar qualifier should not expect `end_of_input`. All `Angstrom.choice` invocations now receive a `~failure_msg` argument for better error messages.
I pushed some changes. About comments, it seems you can have comments at the end of lines as long as there is whitespace immediately before the
Otherwise it's not allowed
I'm not sure how to adapt the parser for this. I think it's fine to defer that to another PR. |
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.
Thanks, this looks good to me.
mirage/unikernel.ml
Outdated
let interface = | ||
let doc = | ||
Arg.info ~docs:ignored ~doc:"Interface to listen on." [ "interface" ] | ||
in | ||
Mirage_runtime.register_arg | ||
Arg.(value & opt Config_parser.(some (ignore_c "interface")) None doc) |
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.
Annoyingly this collides with the mirage unix argument --interface /o\
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.
ok, I'm not sure what to do...
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 guess we can figure out a way to rename the unix interface option
fails in CI, I'll push my changes from my laptop in the morning (I did some changes in |
No description provided.