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

Test for Issue 1135 #1136

Merged
merged 8 commits into from
Jan 22, 2018
Merged

Test for Issue 1135 #1136

merged 8 commits into from
Jan 22, 2018

Conversation

willmurphyscode
Copy link
Contributor

@willmurphyscode willmurphyscode commented Dec 26, 2017

It looks like required_unless_one has incorrect behavior if the
argument it applies to also has a short form. This adds a failing test, but I could use help fixing the issue.


This change is Reviewable

It looks like required_unless_one has incorrect behavior if the
argument it applies to also has a short form.
@mention-bot
Copy link

@willmurphyscode, thanks for your PR! By analyzing the history of the files in this pull request, we identified @kbknapp to be a potential reviewer.

@@ -67,7 +67,7 @@ impl<'n, 'e> AnyArg<'n, 'e> for FlagBuilder<'n, 'e> {
self.b.requires.as_ref().map(|o| &o[..])
}
fn blacklist(&self) -> Option<&[&'e str]> { self.b.blacklist.as_ref().map(|o| &o[..]) }
fn required_unless(&self) -> Option<&[&'e str]> { None }
fn required_unless(&self) -> Option<&[&'e str]> { self.b.r_unless.as_ref().map(|o| &o[..]) }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line is copied from the analogous line in the Positional arg builder: https://github.com/kbknapp/clap-rs/blob/f85fd9fbc8a1b2f5f319dad272405ac8e43c790f/src/args/arg_builder/positional.rs#L141 - I assume that the logic is supposed to be the same for both types of args.

@kbknapp
Copy link
Member

kbknapp commented Jan 20, 2018

Nice, good catch! Thanks @willmurphyscode 🎉

@kbknapp kbknapp merged commit 969c3ad into clap-rs:master Jan 22, 2018
@willmurphyscode willmurphyscode deleted the issue-1135 branch January 25, 2018 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants