Skip to content

Commit

Permalink
Auto merge of #720 - kbknapp:issues-714,718, r=kbknapp
Browse files Browse the repository at this point in the history 
Issues 714,718
  • Loading branch information
@homu
homu committed Oct 31, 2016
2 parents 174a577 + 518f577 commit 813489d
Show file tree
Hide file tree
Showing 10 changed files with 196 additions and 87 deletions.
20 changes: 20 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,23 @@
<a name="v2.16.4"></a>
### v2.16.4 (2016-10-31)


#### Improvements

* **Error Output:** conflicting errors are now symetrical, meaning more consistent and less confusing ([3d37001d](https://github.com/kbknapp/clap-rs/commit/3d37001d1dc647d73cc597ff172f1072d4beb80d), closes [#718](https://github.com/kbknapp/clap-rs/issues/718))

#### Documentation

* Fix typo in example `13a_enum_values_automatic` ([c22fbc07](https://github.com/kbknapp/clap-rs/commit/c22fbc07356e556ffb5d1a79ec04597d149b915e))
* **README.md:** fixes failing yaml example (#715) ([21fba9e6](https://github.com/kbknapp/clap-rs/commit/21fba9e6cd8c163012999cd0ce271ec8780c5695))

#### Bug Fixes

* **ZSH Completions:** fixes bug that caused panic on subcommands with aliases ([5c70e1a0](https://github.com/kbknapp/clap-rs/commit/5c70e1a01bc977e44c10015d18bb8e215c32dfc8), closes [#714](https://github.com/kbknapp/clap-rs/issues/714))
* **debug:** fixes the debug feature (#716) ([6c11ccf4](https://github.com/kbknapp/clap-rs/commit/6c11ccf443d46258d51f7cda33fbcc81e7fe8e90))



<a name="v2.16.3"></a>
### v2.16.3 (2016-10-28)

Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]

name = "clap"
version = "2.16.3"
version = "2.16.4"
authors = ["Kevin K. <kbknapp@gmail.com>"]
exclude = ["examples/*", "clap-test/*", "tests/*", "benches/*", "*.png", "clap-perf/*", "*.dot"]
repository = "https://github.com/kbknapp/clap-rs.git"
Expand Down
8 changes: 8 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ Created by [gh-md-toc](https://github.com/ekalinin/github-markdown-toc)

## What's New

Here's what's new in v2.16.4

* Fixes bug that caused panic on subcommands with aliases
* Conflicting argument errors are now symetrical, meaning more consistent and better usage suggestions
* Fixes typo in example `13a_enum_values_automatic`
* Fixes failing yaml example (#715)
* Fixes the `debug` feature (#716)

Here's the highlights for v2.16.3

* Fixes a bug where the derived display order isn't propagated
Expand Down
88 changes: 87 additions & 1 deletion src/app/macros.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ macro_rules! arg_post_processing {
// Handle POSIX overrides
debug!("Is '{}' in overrides...", $arg.to_string());
if $me.overrides.contains(&$arg.name()) {
if let Some(ref name) = $me.overriden_from($arg.name(), $matcher) {
if let Some(ref name) = find_name_from!($me, &$arg.name(), overrides, $matcher) {
sdebugln!("Yes by {}", name);
$matcher.remove(name);
remove_overriden!($me, name);
Expand All @@ -48,6 +48,32 @@ macro_rules! arg_post_processing {
debug!("Does '{}' have conflicts...", $arg.to_string());
if let Some(bl) = $arg.blacklist() {
sdebugln!("Yes");

for c in bl {
// Inject two-way conflicts
debug!("Has '{}' already been matched...", c);
if $matcher.contains(c) {
sdebugln!("Yes");
// find who blacklisted us...
$me.blacklist.push(&$arg.name);
// if let Some(f) = $me.find_flag_mut(c) {
// if let Some(ref mut bl) = f.blacklist {
// bl.push(&$arg.name);
// }
// } else if let Some(o) = $me.find_option_mut(c) {
// if let Some(ref mut bl) = o.blacklist {
// bl.push(&$arg.name);
// }
// } else if let Some(p) = $me.find_positional_mut(c) {
// if let Some(ref mut bl) = p.blacklist {
// bl.push(&$arg.name);
// }
// }
} else {
sdebugln!("No");
}
}

$me.blacklist.extend(bl);
vec_remove_all!($me.overrides, bl);
vec_remove_all!($me.required, bl);
Expand Down Expand Up @@ -143,3 +169,63 @@ macro_rules! parse_positional {
}
};
}

macro_rules! find_from {
($_self:ident, $arg_name:expr, $from:ident, $matcher:expr) => {{
let mut ret = None;
for k in $matcher.arg_names() {
if let Some(f) = $_self.find_flag(k) {
if let Some(ref v) = f.$from {
if v.contains($arg_name) {
ret = Some(f.to_string());
}
}
}
if let Some(o) = $_self.find_option(k) {
if let Some(ref v) = o.$from {
if v.contains(&$arg_name) {
ret = Some(o.to_string());
}
}
}
if let Some(pos) = $_self.find_positional(k) {
if let Some(ref v) = pos.$from {
if v.contains($arg_name) {
ret = Some(pos.name.to_owned());
}
}
}
}
ret
}};
}

macro_rules! find_name_from {
($_self:ident, $arg_name:expr, $from:ident, $matcher:expr) => {{
let mut ret = None;
for k in $matcher.arg_names() {
if let Some(f) = $_self.find_flag(k) {
if let Some(ref v) = f.$from {
if v.contains($arg_name) {
ret = Some(f.name);
}
}
}
if let Some(o) = $_self.find_option(k) {
if let Some(ref v) = o.$from {
if v.contains(&$arg_name) {
ret = Some(o.name);
}
}
}
if let Some(pos) = $_self.find_positional(k) {
if let Some(ref v) = pos.$from {
if v.contains($arg_name) {
ret = Some(pos.name);
}
}
}
}
ret
}};
}
133 changes: 59 additions & 74 deletions src/app/parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1007,59 +1007,6 @@ impl<'a, 'b> Parser<'a, 'b>
Ok(())
}

fn blacklisted_from(&self, name: &str, matcher: &ArgMatcher) -> Option<String> {
for k in matcher.arg_names() {
if let Some(f) = self.flags.iter().find(|f| &f.name == &k) {
if let Some(ref bl) = f.blacklist {
if bl.contains(&name) {
return Some(f.to_string());
}
}
}
if let Some(o) = self.opts.iter().find(|o| &o.name == &k) {
if let Some(ref bl) = o.blacklist {
if bl.contains(&name) {
return Some(o.to_string());
}
}
}
if let Some(pos) = self.positionals.values().find(|p| &p.name == &k) {
if let Some(ref bl) = pos.blacklist {
if bl.contains(&name) {
return Some(pos.name.to_owned());
}
}
}
}
None
}

fn overriden_from(&self, name: &str, matcher: &ArgMatcher) -> Option<&'a str> {
for k in matcher.arg_names() {
if let Some(f) = self.flags.iter().find(|f| &f.name == &k) {
if let Some(ref bl) = f.overrides {
if bl.contains(&name.into()) {
return Some(f.name);
}
}
}
if let Some(o) = self.opts.iter().find(|o| &o.name == &k) {
if let Some(ref bl) = o.overrides {
if bl.contains(&name.into()) {
return Some(o.name);
}
}
}
if let Some(pos) = self.positionals.values().find(|p| &p.name == &k) {
if let Some(ref bl) = pos.overrides {
if bl.contains(&name.into()) {
return Some(pos.name);
}
}
}
}
None
}

fn groups_for_arg(&self, name: &str) -> Option<Vec<&'a str>> {
debugln!("fn=groups_for_arg; name={}", name);
Expand Down Expand Up @@ -1535,26 +1482,43 @@ impl<'a, 'b> Parser<'a, 'b>
}

fn validate_blacklist(&self, matcher: &mut ArgMatcher) -> ClapResult<()> {
debugln!("fn=validate_blacklist;");
debugln!("fn=validate_blacklist;blacklist={:?}", self.blacklist);
macro_rules! build_err {
($me:ident, $name:expr, $matcher:ident) => ({
debugln!("macro=build_err;");
let c_with = $me.blacklisted_from($name, &$matcher);
debugln!("macro=build_err;name={}", $name);
let mut c_with = find_from!($me, $name, blacklist, &$matcher);
c_with = if c_with.is_none() {
if let Some(aa) = $me.find_any_arg($name) {
if let Some(bl) = aa.blacklist() {
if let Some(arg_name) = bl.iter().find(|arg| $matcher.contains(arg)) {
if let Some(aa) = $me.find_any_arg(arg_name) {
Some(aa.to_string())
} else {
c_with
}
} else {
c_with
}
} else {
c_with
}
} else {
c_with
}
} else {
c_with
};
debugln!("'{:?}' conflicts with '{}'", c_with, $name);
$matcher.remove($name);
let usg = $me.create_current_usage($matcher);
if let Some(f) = $me.flags.iter().filter(|f| f.name == $name).next() {
if let Some(f) = $me.find_flag($name) {
debugln!("It was a flag...");
Error::argument_conflict(f, c_with, &*usg, self.color())
} else if let Some(o) = $me.opts.iter()
.filter(|o| o.name == $name)
.next() {
debugln!("It was an option...");
} else if let Some(o) = $me.find_option($name) {
debugln!("It was an option...");
Error::argument_conflict(o, c_with, &*usg, self.color())
} else {
match $me.positionals.values()
.filter(|p| p.name == $name)
.next() {
match $me.find_positional($name) {
Some(p) => {
debugln!("It was a positional...");
Error::argument_conflict(p, c_with, &*usg, self.color())
Expand All @@ -1572,12 +1536,12 @@ impl<'a, 'b> Parser<'a, 'b>
debugln!("Checking arg '{}' in group...", n);
if matcher.contains(n) {
debugln!("matcher contains it...");
return Err(build_err!(self, n, matcher));
return Err(build_err!(self, &n, matcher));
}
}
} else if matcher.contains(name) {
debugln!("matcher contains it...");
return Err(build_err!(self, *name, matcher));
return Err(build_err!(self, name, matcher));
}
}
Ok(())
Expand Down Expand Up @@ -1940,15 +1904,15 @@ impl<'a, 'b> Parser<'a, 'b>
Ok(())
}

pub fn flags(&self) -> Iter<FlagBuilder> {
pub fn flags(&self) -> Iter<FlagBuilder<'a, 'b>> {
self.flags.iter()
}

pub fn opts(&self) -> Iter<OptBuilder> {
pub fn opts(&self) -> Iter<OptBuilder<'a, 'b>> {
self.opts.iter()
}

pub fn positionals(&self) -> vec_map::Values<PosBuilder> {
pub fn positionals(&self) -> vec_map::Values<PosBuilder<'a, 'b>> {
self.positionals.values()
}

Expand All @@ -1973,19 +1937,40 @@ impl<'a, 'b> Parser<'a, 'b>
}
}

pub fn find_arg(&self, arg: &str) -> Option<&AnyArg> {
pub fn find_any_arg(&self, arg: &str) -> Option<&AnyArg> {
if let Some(f) = self.find_flag(arg) {
return Some(f);
}
if let Some(o) = self.find_option(arg) {
return Some(o);
}
if let Some(p) = self.find_positional(arg) {
return Some(p);
}
None
}

fn find_flag(&self, name: &str) -> Option<&FlagBuilder<'a, 'b>> {
for f in self.flags() {
if f.name == arg {
if f.name == name || f.aliases.as_ref().unwrap_or(&vec![("",false)]).iter().any(|&(n,_)| n == name) {
return Some(f);
}
}
None
}

fn find_option(&self, name: &str) -> Option<&OptBuilder<'a, 'b>> {
for o in self.opts() {
if o.name == arg {
if o.name == name || o.aliases.as_ref().unwrap_or(&vec![("",false)]).iter().any(|&(n,_)| n == name) {
return Some(o);
}
}
None
}

fn find_positional(&self, name: &str) -> Option<&PosBuilder<'a, 'b>> {
for p in self.positionals() {
if p.name == arg {
if p.name == name {
return Some(p);
}
}
Expand All @@ -1998,7 +1983,7 @@ impl<'a, 'b> Parser<'a, 'b>
debugln!("Looking for sc...{}", sc);
debugln!("Currently in Parser...{}", self.meta.bin_name.as_ref().unwrap());
for s in self.subcommands.iter() {
if s.p.meta.bin_name.as_ref().unwrap_or(&String::new()) == sc {
if s.p.meta.bin_name.as_ref().unwrap_or(&String::new()) == sc || (s.p.meta.aliases.is_some() && s.p.meta.aliases.as_ref().unwrap().iter().any(|&(s,_)| s == sc.split(' ').rev().next().expect(INTERNAL_ERROR_MSG))) {
return Some(s);
}
if let Some(app) = s.p.find_subcommand(sc) {
Expand Down
3 changes: 2 additions & 1 deletion src/args/any_arg.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Std
use std::rc::Rc;
use std::fmt as std_fmt;

// Third Party
use vec_map::VecMap;
Expand All @@ -8,7 +9,7 @@ use vec_map::VecMap;
use args::settings::ArgSettings;

#[doc(hidden)]
pub trait AnyArg<'n, 'e> {
pub trait AnyArg<'n, 'e>: std_fmt::Display {
fn name(&self) -> &'n str;
fn overrides(&self) -> Option<&[&'e str]>;
fn aliases(&self) -> Option<Vec<&'e str>>;
Expand Down
2 changes: 1 addition & 1 deletion src/completions/macros.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ macro_rules! get_zsh_arg_conflicts {
if let Some(conf_vec) = $arg.blacklist() {
let mut v = vec![];
for arg_name in conf_vec {
let arg = $p.find_arg(arg_name).expect($msg);
let arg = $p.find_any_arg(arg_name).expect($msg);
if let Some(s) = arg.short() {
v.push(format!("-{}", s));
}
Expand Down
Loading

0 comments on commit 813489d

Please sign in to comment.