Skip to content
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

Add OptionParser#missing_option and OptionParser#invalid_option #3030

Merged
merged 1 commit into from
Jul 23, 2016

Conversation

jhass
Copy link
Member

@jhass jhass commented Jul 22, 2016

These callbacks provide a nicer API to toplevel option parsing,
removing the need to rescue the exceptions and store the parser in a
local in order to provide the help message in these events.

invalid_option { |flag| raise InvalidOption.new(flag) }
# TODO: these should work and remove the need for the property! calls above
# @missing_option = ->(option : String) { raise MissingOption.new(option) }
# @invalid_option = ->(option : String) { raise InvalidOption.new(option) }
Copy link
Member Author

@jhass jhass Jul 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@asterite whether this is accepted or not, this is a bug you should look into ;)

instance variable '@missing_option' of OptionParser must be Proc(String, Nil), not Proc(String, NoReturn)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed! Of course we can't immediately use this, but please check if the new compiler now compiles this fine :-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, works now ❤️

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
These callbacks provide a nicer API to toplevel option parsing,
removing the need to rescue the exceptions and store the parser in a
local in order to provide the help message in these events.
@jhass jhass force-pushed the option_parser_handlers branch from b6e9f52 to 12648f4 Compare July 22, 2016 14:04
@asterite asterite merged commit 0813841 into crystal-lang:master Jul 23, 2016
@asterite
Copy link
Member

Really good idea, thank you!

@asterite asterite added this to the 0.19.0 milestone Jul 23, 2016
@jhass jhass deleted the option_parser_handlers branch July 25, 2016 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants