Skip to content

Commit

Permalink
perf: doesn't run arg_post_processing on multiple values anymore
Browse files Browse the repository at this point in the history
  • Loading branch information
kbknapp committed Mar 4, 2017
1 parent 1160203 commit ec51618
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 13 deletions.
7 changes: 5 additions & 2 deletions src/app/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,12 @@ macro_rules! parse_positional {
$matcher.inc_occurrence_of($p.b.name);
let _ = $_self.groups_for_arg($p.b.name)
.and_then(|vec| Some($matcher.inc_occurrences_of(&*vec)));
arg_post_processing!($_self, $p, $matcher);
if $_self.cache.map_or(true, |name| name != $p.b.name) {
arg_post_processing!($_self, $p, $matcher);
$_self.cache = Some($p.b.name);
}

$_self.set(AS::ValidArgFound);
$_self.settings.set(AS::ValidArgFound);
// Only increment the positional counter if it doesn't allow multiples
if !$p.b.settings.is_set(ArgSettings::Multiple) {
$pos_counter += 1;
Expand Down
36 changes: 26 additions & 10 deletions src/app/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ pub struct Parser<'a, 'b>
overrides: Vec<&'b str>,
help_short: Option<char>,
version_short: Option<char>,
cache: Option<&'a str>,
}

impl<'a, 'b> Parser<'a, 'b>
Expand Down Expand Up @@ -1422,8 +1423,10 @@ impl<'a, 'b> Parser<'a, 'b>
opt.to_string());
self.settings.set(AS::ValidArgFound);
let ret = try!(self.parse_opt(val, opt, val.is_some(), matcher));
self.set(AS::ValidArgFound);
arg_post_processing!(self, opt, matcher);
if self.cache.map_or(true, |name| name != opt.b.name) {
arg_post_processing!(self, opt, matcher);
self.cache = Some(opt.b.name);
}

return Ok(ret);
} else if let Some(flag) = find_flag_by_long!(@os self, &arg) {
Expand All @@ -1437,9 +1440,11 @@ impl<'a, 'b> Parser<'a, 'b>
try!(self.parse_flag(flag, matcher));

// Handle conflicts, requirements, etc.
arg_post_processing!(self, flag, matcher);
if self.cache.map_or(true, |name| name != flag.b.name) {
arg_post_processing!(self, flag, matcher);
self.cache = Some(flag.b.name);
}

self.set(AS::ValidArgFound);
return Ok(None);
} else if self.is_set(AS::AllowLeadingHyphen) {
return Ok(None);
Expand Down Expand Up @@ -1507,9 +1512,11 @@ impl<'a, 'b> Parser<'a, 'b>
// Default to "we're expecting a value later"
let ret = try!(self.parse_opt(val, opt, false, matcher));

arg_post_processing!(self, opt, matcher);
if self.cache.map_or(true, |name| name != opt.b.name) {
arg_post_processing!(self, opt, matcher);
self.cache = Some(opt.b.name);
}

self.set(AS::ValidArgFound);
return Ok(ret);
} else if let Some(flag) = find_flag_by_short!(self, c) {
debugln!("Parser::parse_short_arg:iter: Found valid short flag -{}",
Expand All @@ -1519,10 +1526,12 @@ impl<'a, 'b> Parser<'a, 'b>
try!(self.check_for_help_and_version_char(c));
try!(self.parse_flag(flag, matcher));

self.set(AS::ValidArgFound);
// Handle conflicts, requirements, overrides, etc.
// Must be called here due to mutablilty
arg_post_processing!(self, flag, matcher);
if self.cache.map_or(true, |name| name != flag.b.name) {
arg_post_processing!(self, flag, matcher);
self.cache = Some(flag.b.name);
}
} else {
let arg = format!("-{}", c);
return Err(Error::unknown_argument(&*arg,
Expand Down Expand Up @@ -2205,7 +2214,11 @@ impl<'a, 'b> Parser<'a, 'b>
if let Some(ref val) = $a.v.default_val {
if $m.get($a.b.name).is_none() {
try!($_self.add_val_to_arg($a, OsStr::new(val), $m));
arg_post_processing!($_self, $a, $m);

if $_self.cache.map_or(true, |name| name != $a.name()) {
arg_post_processing!($_self, $a, $m);
$_self.cache = Some($a.name());
}
}
}
};
Expand All @@ -2225,7 +2238,10 @@ impl<'a, 'b> Parser<'a, 'b>
};
if add {
try!($_self.add_val_to_arg($a, OsStr::new(default), $m));
arg_post_processing!($_self, $a, $m);
if $_self.cache.map_or(true, |name| name != $a.name()) {
arg_post_processing!($_self, $a, $m);
$_self.cache = Some($a.name());
}
done = true;
break;
}
Expand Down
11 changes: 10 additions & 1 deletion src/app/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -527,12 +527,18 @@ pub enum AppSettings {
/// This can be useful if there are many values, or they are explained elsewhere.
HidePossibleValuesInHelp,

/// Tries to match unknown args to partial [`subcommands`] or their aliases. For example to
/// Tries to match unknown args to partial [`subcommands`] or their [aliases]. For example to
/// match a subcommand named `test`, one could use `t`, `te`, `tes`, and `test`.
///
/// **NOTE:** The match *must not* be ambiguous at all in order to succeed. i.e. to match `te`
/// to `test` there could not also be a subcommand or alias `temp` because both start with `te`
///
/// **CAUTION:** This setting can interfere with [positional/free arguments], take care when
/// designing CLIs which allow inferred subcommands and have potential positional/free
/// arguments who's values could start with the same characters as subcommands. If this is the
/// case, it's recommended to use settings such as [`AppSeettings::ArgsNegateSubcommands`] in
/// conjuction with this setting.
///
/// # Examples
///
/// ```no_run
Expand All @@ -546,6 +552,9 @@ pub enum AppSettings {
/// assert_eq!(m.subcommand_name(), Some("test"));
/// ```
/// [`subcommands`]: ./struct.SubCommand.html
/// [positional/free arguments]: ./struct.Arg.html#method.index
/// [aliases]: ./struct.App.html#method.alias
/// [`AppSeettings::ArgsNegateSubcommands`]: ./enum.AppSettings.html#variant.ArgsNegateSubcommands
InferSubcommands,

/// Specifies that the parser should not assume the first argument passed is the binary name.
Expand Down

0 comments on commit ec51618

Please sign in to comment.