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

Support for required options #218

Merged
merged 1 commit into from
Oct 5, 2017

Conversation

woodruffw
Copy link
Contributor

@woodruffw woodruffw commented Sep 29, 2017

I've been needing this in one of my own projects and I saw that someone previously requested it in #203, so I went ahead and implemented it 😄

Let me know if you'd like me to update the README and/or Gem version in this PR as well.

Thanks for the awesome library!

Fixes #203

@@ -80,6 +80,13 @@ def parse(strings)

@arguments += ignored_args

unused_options.each do |o|
if o.config[:required] && !suppress_errors?
Copy link
Collaborator

@olleolleolle olleolleolle Sep 29, 2017

Choose a reason for hiding this comment

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

Perhaps the whole loop can be obviated when !suppress_errors? - that is, move that expression out of the loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Very true. I'll fix that.

@olleolleolle
Copy link
Collaborator

olleolleolle commented Sep 29, 2017

This is a cool change. Can you teach users the required: true option somehow, in some text file or the like?

@woodruffw
Copy link
Contributor Author

Can you teach users the required: true option somehow, in some text file or the like?

Sure thing. Should I add it to the README, or is somewhere else better?

@olleolleolle
Copy link
Collaborator

@woodruffw README's best!

@olleolleolle
Copy link
Collaborator

(To fully answer: gem version should not be changed with a feature. That's a separate release acitivity.)

This commit introduces support for required options,
which are options that cause the parser to raise
a `MissingRequiredOption` exception if not present.

Options can be marked as required by passing
`required: true` in their configuration, and any errors
caused by missing required options can be suppressed via
`suppress_errors: true`.
@woodruffw
Copy link
Contributor Author

Updated the README, plus I added Option#required? for consistency with Option#suppress_errors? and Option#default_value.

(To fully answer: gem version should not be changed with a feature. That's a separate release acitivity.)

👍

@woodruffw
Copy link
Contributor Author

Any update on this @olleolleolle?

@olleolleolle olleolleolle merged commit 193a826 into leejarvis:master Oct 5, 2017
@woodruffw woodruffw deleted the required-options branch October 5, 2017 21:15
@woodruffw
Copy link
Contributor Author

Thanks! Would it be possible to get a version released for this change? I'd love to start using it 😄

leejarvis added a commit that referenced this pull request Oct 6, 2017
includes (closes #218)
@leejarvis
Copy link
Owner

Thanks both. I've released 4.6.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants