Skip to content

Commit

Permalink
Auto merge of #796 - kbknapp:issue-665, r=kbknapp
Browse files Browse the repository at this point in the history 
Issue 665
  • Loading branch information
@homu
homu committed Dec 30, 2016
2 parents 03f7233 + 327c514 commit 7eb949b
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 81 deletions.
26 changes: 17 additions & 9 deletions src/app/parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,8 @@ impl<'a, 'b> Parser<'a, 'b>

pub fn add_subcommand(&mut self, mut subcmd: App<'a, 'b>) {
debugln!("Parser::add_subcommand;");
debugln!("Parser::add_subcommand: Term width...{:?}", self.meta.term_w);
debugln!("Parser::add_subcommand: Term width...{:?}",
self.meta.term_w);
subcmd.p.meta.term_w = self.meta.term_w;
debug!("Parser::add_subcommand: Is help...");
if subcmd.p.meta.name == "help" {
Expand All @@ -256,8 +257,10 @@ impl<'a, 'b> Parser<'a, 'b>
let vsc = self.settings.is_set(AppSettings::VersionlessSubcommands);
let gv = self.settings.is_set(AppSettings::GlobalVersion);

debugln!("Parser::propogate_settings:iter: VersionlessSubcommands set...{:?}", vsc);
debugln!("Parser::propogate_settings:iter: GlobalVersion set...{:?}", gv);
debugln!("Parser::propogate_settings:iter: VersionlessSubcommands set...{:?}",
vsc);
debugln!("Parser::propogate_settings:iter: GlobalVersion set...{:?}",
gv);

if vsc {
sc.p.settings.set(AppSettings::DisableVersion);
Expand Down Expand Up @@ -438,7 +441,7 @@ impl<'a, 'b> Parser<'a, 'b>
if grp.required {
// if it's part of a required group we don't want to count it
continue 'outer;
}
}
}
}
}
Expand Down Expand Up @@ -1540,7 +1543,9 @@ impl<'a, 'b> Parser<'a, 'b>
}
if let Some(vtor) = arg.validator_os() {
if let Err(e) = vtor(val) {
return Err(Error::value_validation(Some(arg), (*e).to_string_lossy().to_string(), self.color()));
return Err(Error::value_validation(Some(arg),
(*e).to_string_lossy().to_string(),
self.color()));
}
}
if matcher.needs_more_vals(arg) {
Expand Down Expand Up @@ -1703,6 +1708,10 @@ impl<'a, 'b> Parser<'a, 'b>
self.color()));
}
}
// Issue 665 (https://github.com/kbknapp/clap-rs/issues/665)
if a.takes_value() && !a.is_set(ArgSettings::EmptyValues) && ma.vals.is_empty() {
return Err(Error::empty_value(a, &*self.create_current_usage(matcher), self.color()));
}
Ok(())
}

Expand Down Expand Up @@ -1731,16 +1740,15 @@ impl<'a, 'b> Parser<'a, 'b>
let c = Colorizer {
use_stderr: true,
when: self.color(),
};
let mut reqs = self.required.iter().map(|&r| &*r).collect::<Vec<_>>();
};
let mut reqs = self.required.iter().map(|&r| &*r).collect::<Vec<_>>();
reqs.retain(|n| !matcher.contains(n));
reqs.dedup();
Err(Error::missing_required_argument(&*self.get_required_from(&self.required[..],
Some(matcher))
.iter()
.fold(String::new(), |acc, s| {
acc +
&format!("\n {}", c.error(s))[..]
acc + &format!("\n {}", c.error(s))[..]
}),
&*self.create_current_usage(matcher),
self.color()))
Expand Down
161 changes: 89 additions & 72 deletions tests/opts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use clap::{App, Arg, ErrorKind};
#[test]
fn stdin_char() {
let r = App::new("opts")
.arg( Arg::from_usage("-f [flag] 'some flag'") )
.arg(Arg::from_usage("-f [flag] 'some flag'"))
.get_matches_from_safe(vec!["", "-f", "-"]);
assert!(r.is_ok());
let m = r.unwrap();
Expand All @@ -19,10 +19,8 @@ fn stdin_char() {
#[test]
fn opts_using_short() {
let r = App::new("opts")
.args(&[
Arg::from_usage("-f [flag] 'some flag'"),
Arg::from_usage("-c [color] 'some other flag'")
])
.args(&[Arg::from_usage("-f [flag] 'some flag'"),
Arg::from_usage("-c [color] 'some other flag'")])
.get_matches_from_safe(vec!["", "-f", "some", "-c", "other"]);
assert!(r.is_ok());
let m = r.unwrap();
Expand All @@ -35,38 +33,50 @@ fn opts_using_short() {
#[test]
fn lots_o_vals() {
let r = App::new("opts")
.arg(
Arg::from_usage("-o [opt]... 'some opt'"),
)
.get_matches_from_safe(vec!["", "-o",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some", "some", "some", "some", "some",
]);
.arg(Arg::from_usage("-o [opt]... 'some opt'"))
.get_matches_from_safe(vec!["", "-o", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some", "some", "some", "some",
"some", "some", "some", "some"]);
assert!(r.is_ok());
let m = r.unwrap();
assert!(m.is_present("o"));
Expand All @@ -76,10 +86,8 @@ fn lots_o_vals() {
#[test]
fn opts_using_long_space() {
let r = App::new("opts")
.args(&[
Arg::from_usage("--flag [flag] 'some flag'"),
Arg::from_usage("--color [color] 'some other flag'")
])
.args(&[Arg::from_usage("--flag [flag] 'some flag'"),
Arg::from_usage("--color [color] 'some other flag'")])
.get_matches_from_safe(vec!["", "--flag", "some", "--color", "other"]);
assert!(r.is_ok());
let m = r.unwrap();
Expand All @@ -97,16 +105,15 @@ fn opts_with_empty_values() {
assert!(r.is_ok());
let m = r.unwrap();
assert!(m.is_present("flag"));
assert_eq!(m.values_of("flag").unwrap().collect::<Vec<_>>(), ["", "test"]);
assert_eq!(m.values_of("flag").unwrap().collect::<Vec<_>>(),
["", "test"]);
}

#[test]
fn opts_using_long_equals() {
let r = App::new("opts")
.args(&[
Arg::from_usage("--flag [flag] 'some flag'"),
Arg::from_usage("--color [color] 'some other flag'")
])
.args(&[Arg::from_usage("--flag [flag] 'some flag'"),
Arg::from_usage("--color [color] 'some other flag'")])
.get_matches_from_safe(vec!["", "--flag=some", "--color=other"]);
assert!(r.is_ok());
let m = r.unwrap();
Expand All @@ -119,10 +126,8 @@ fn opts_using_long_equals() {
#[test]
fn opts_using_mixed() {
let r = App::new("opts")
.args(&[
Arg::from_usage("-f, --flag [flag] 'some flag'"),
Arg::from_usage("-c, --color [color] 'some other flag'")
])
.args(&[Arg::from_usage("-f, --flag [flag] 'some flag'"),
Arg::from_usage("-c, --color [color] 'some other flag'")])
.get_matches_from_safe(vec!["", "-f", "some", "--color", "other"]);
assert!(r.is_ok());
let m = r.unwrap();
Expand All @@ -135,10 +140,8 @@ fn opts_using_mixed() {
#[test]
fn opts_using_mixed2() {
let r = App::new("opts")
.args(&[
Arg::from_usage("-f, --flag [flag] 'some flag'"),
Arg::from_usage("-c, --color [color] 'some other flag'")
])
.args(&[Arg::from_usage("-f, --flag [flag] 'some flag'"),
Arg::from_usage("-c, --color [color] 'some other flag'")])
.get_matches_from_safe(vec!["", "--flag=some", "-c", "other"]);
assert!(r.is_ok());
let m = r.unwrap();
Expand All @@ -151,8 +154,7 @@ fn opts_using_mixed2() {
#[test]
fn default_values_user_value() {
let r = App::new("df")
.arg( Arg::from_usage("-o [opt] 'some opt'")
.default_value("default"))
.arg(Arg::from_usage("-o [opt] 'some opt'").default_value("default"))
.get_matches_from_safe(vec!["", "-o", "value"]);
assert!(r.is_ok());
let m = r.unwrap();
Expand All @@ -163,8 +165,8 @@ fn default_values_user_value() {
#[test]
fn multiple_vals_pos_arg_equals() {
let r = App::new("mvae")
.arg( Arg::from_usage("-o [opt]... 'some opt'") )
.arg( Arg::from_usage("[file] 'some file'") )
.arg(Arg::from_usage("-o [opt]... 'some opt'"))
.arg(Arg::from_usage("[file] 'some file'"))
.get_matches_from_safe(vec!["", "-o=1", "some"]);
assert!(r.is_ok());
let m = r.unwrap();
Expand All @@ -177,8 +179,8 @@ fn multiple_vals_pos_arg_equals() {
#[test]
fn multiple_vals_pos_arg_delim() {
let r = App::new("mvae")
.arg( Arg::from_usage("-o [opt]... 'some opt'") )
.arg( Arg::from_usage("[file] 'some file'") )
.arg(Arg::from_usage("-o [opt]... 'some opt'"))
.arg(Arg::from_usage("[file] 'some file'"))
.get_matches_from_safe(vec!["", "-o", "1,2", "some"]);
assert!(r.is_ok());
let m = r.unwrap();
Expand All @@ -191,8 +193,8 @@ fn multiple_vals_pos_arg_delim() {
#[test]
fn require_delims_no_delim() {
let r = App::new("mvae")
.arg( Arg::from_usage("-o [opt]... 'some opt'").require_delimiter(true) )
.arg( Arg::from_usage("[file] 'some file'") )
.arg(Arg::from_usage("-o [opt]... 'some opt'").require_delimiter(true))
.arg(Arg::from_usage("[file] 'some file'"))
.get_matches_from_safe(vec!["mvae", "-o", "1", "2", "some"]);
assert!(r.is_err());
let err = r.unwrap_err();
Expand All @@ -202,8 +204,8 @@ fn require_delims_no_delim() {
#[test]
fn require_delims() {
let r = App::new("mvae")
.arg( Arg::from_usage("-o [opt]... 'some opt'").require_delimiter(true) )
.arg( Arg::from_usage("[file] 'some file'") )
.arg(Arg::from_usage("-o [opt]... 'some opt'").require_delimiter(true))
.arg(Arg::from_usage("[file] 'some file'"))
.get_matches_from_safe(vec!["", "-o", "1,2", "some"]);
assert!(r.is_ok());
let m = r.unwrap();
Expand All @@ -216,7 +218,7 @@ fn require_delims() {
#[test]
fn leading_hyphen_pass() {
let r = App::new("mvae")
.arg( Arg::from_usage("-o [opt]... 'some opt'").allow_hyphen_values(true))
.arg(Arg::from_usage("-o [opt]... 'some opt'").allow_hyphen_values(true))
.get_matches_from_safe(vec!["", "-o", "-2", "3"]);
assert!(r.is_ok());
let m = r.unwrap();
Expand All @@ -227,7 +229,7 @@ fn leading_hyphen_pass() {
#[test]
fn leading_hyphen_fail() {
let r = App::new("mvae")
.arg( Arg::from_usage("-o [opt] 'some opt'"))
.arg(Arg::from_usage("-o [opt] 'some opt'"))
.get_matches_from_safe(vec!["", "-o", "-2"]);
assert!(r.is_err());
let m = r.unwrap_err();
Expand All @@ -237,7 +239,7 @@ fn leading_hyphen_fail() {
#[test]
fn leading_hyphen_with_flag_after() {
let r = App::new("mvae")
.arg( Arg::from_usage("-o [opt]... 'some opt'").allow_hyphen_values(true))
.arg(Arg::from_usage("-o [opt]... 'some opt'").allow_hyphen_values(true))
.arg_from_usage("-f 'some flag'")
.get_matches_from_safe(vec!["", "-o", "-2", "-f"]);
assert!(r.is_ok());
Expand All @@ -250,7 +252,7 @@ fn leading_hyphen_with_flag_after() {
#[test]
fn leading_hyphen_with_flag_before() {
let r = App::new("mvae")
.arg( Arg::from_usage("-o [opt]... 'some opt'").allow_hyphen_values(true))
.arg(Arg::from_usage("-o [opt]... 'some opt'").allow_hyphen_values(true))
.arg_from_usage("-f 'some flag'")
.get_matches_from_safe(vec!["", "-f", "-o", "-2"]);
assert!(r.is_ok());
Expand All @@ -263,7 +265,7 @@ fn leading_hyphen_with_flag_before() {
#[test]
fn leading_hyphen_with_only_pos_follows() {
let r = App::new("mvae")
.arg( Arg::from_usage("-o [opt]... 'some opt'").allow_hyphen_values(true))
.arg(Arg::from_usage("-o [opt]... 'some opt'").allow_hyphen_values(true))
.arg_from_usage("[arg] 'some arg'")
.get_matches_from_safe(vec!["", "-o", "-2", "--", "val"]);
assert!(r.is_ok());
Expand All @@ -276,12 +278,27 @@ fn leading_hyphen_with_only_pos_follows() {
#[test]
#[cfg(feature="suggestions")]
fn did_you_mean() {
test::check_err_output(test::complex_app(), "clap-test --optio=foo",
"error: Found argument '--optio' which wasn't expected, or isn't valid in this context
test::check_err_output(test::complex_app(),
"clap-test --optio=foo",
"error: Found argument '--optio' which wasn't expected, or isn't valid in this context
\tDid you mean --option?
USAGE:
clap-test --option <opt>...
For more information try --help", true);
For more information try --help",
true);
}

#[test]
fn issue_665() {
let res = App::new("tester")
.arg_from_usage("-v, --reroll-count=[N] 'Mark the patch series as PATCH vN'")
.arg(Arg::from_usage(
"--subject-prefix [Subject-Prefix] 'Use [Subject-Prefix] instead of the standard [PATCH] prefix'")
.empty_values(false))
.get_matches_from_safe(vec!["test", "--subject-prefix", "-v", "2"]);

assert!(res.is_err());
assert_eq!(res.unwrap_err().kind, ErrorKind::EmptyValue);
}

0 comments on commit 7eb949b

Please sign in to comment.