refactor: Delay collecting required

The main goal is to allow centralizing some building logic currently
split between the parser and `App`.  It depends on this logic.

As a side benefit, this allowed us to decouple some operations from `Parser` in `App`.

The main impact I can see is that we'll calculate the required once for
parsing a subcommand and once for validation.
This commit is contained in:
Ed Page 2022-02-14 12:42:13 -06:00
parent b994789ee6
commit 3f32030f7f
4 changed files with 116 additions and 80 deletions

View file

@ -687,9 +687,8 @@ impl<'help> App<'help> {
let color = self.get_color();
let mut c = Colorizer::new(false, color);
let parser = Parser::new(self);
let usage = Usage::new(parser.app, &parser.required);
Help::new(HelpWriter::Buffer(&mut c), parser.app, &usage, false).write_help()?;
let usage = Usage::new(self);
Help::new(HelpWriter::Buffer(&mut c), self, &usage, false).write_help()?;
c.print()
}
@ -713,9 +712,8 @@ impl<'help> App<'help> {
let color = self.get_color();
let mut c = Colorizer::new(false, color);
let parser = Parser::new(self);
let usage = Usage::new(parser.app, &parser.required);
Help::new(HelpWriter::Buffer(&mut c), parser.app, &usage, true).write_help()?;
let usage = Usage::new(self);
Help::new(HelpWriter::Buffer(&mut c), self, &usage, true).write_help()?;
c.print()
}
@ -738,9 +736,8 @@ impl<'help> App<'help> {
pub fn write_help<W: io::Write>(&mut self, w: &mut W) -> io::Result<()> {
self._build();
let parser = Parser::new(self);
let usage = Usage::new(parser.app, &parser.required);
Help::new(HelpWriter::Normal(w), parser.app, &usage, false).write_help()?;
let usage = Usage::new(self);
Help::new(HelpWriter::Normal(w), self, &usage, false).write_help()?;
w.flush()
}
@ -763,9 +760,8 @@ impl<'help> App<'help> {
pub fn write_long_help<W: io::Write>(&mut self, w: &mut W) -> io::Result<()> {
self._build();
let parser = Parser::new(self);
let usage = Usage::new(parser.app, &parser.required);
Help::new(HelpWriter::Normal(w), parser.app, &usage, true).write_help()?;
let usage = Usage::new(self);
Help::new(HelpWriter::Normal(w), self, &usage, true).write_help()?;
w.flush()
}
@ -832,8 +828,7 @@ impl<'help> App<'help> {
// before parsing incase we run into a subcommand
self._build();
let parser = Parser::new(self);
Usage::new(parser.app, &parser.required).create_usage_with_title(&[])
Usage::new(self).create_usage_with_title(&[])
}
}

View file

@ -1,22 +1,29 @@
use indexmap::IndexSet;
// Internal
use crate::{
build::AppSettings as AS,
build::{App, Arg, ArgPredicate},
parse::ArgMatcher,
util::{ChildGraph, Id},
INTERNAL_ERROR_MSG,
};
use crate::build::AppSettings as AS;
use crate::build::{App, Arg, ArgPredicate};
use crate::parse::ArgMatcher;
use crate::util::ChildGraph;
use crate::util::Id;
use crate::INTERNAL_ERROR_MSG;
pub(crate) struct Usage<'help, 'app> {
app: &'app App<'help>,
required: &'app ChildGraph<Id>,
required: Option<&'app ChildGraph<Id>>,
}
impl<'help, 'app> Usage<'help, 'app> {
pub(crate) fn new(app: &'app App<'help>, required: &'app ChildGraph<Id>) -> Self {
Usage { app, required }
pub(crate) fn new(app: &'app App<'help>) -> Self {
Usage {
app,
required: None,
}
}
pub(crate) fn required(mut self, required: &'app ChildGraph<Id>) -> Self {
self.required = Some(required);
self
}
// Creates a usage string for display. This happens just after all arguments were parsed, but before
@ -42,7 +49,7 @@ impl<'help, 'app> Usage<'help, 'app> {
}
// Creates a usage string for display in help messages (i.e. not for errors)
pub(crate) fn create_help_usage(&self, incl_reqs: bool) -> String {
fn create_help_usage(&self, incl_reqs: bool) -> String {
debug!("Usage::create_help_usage; incl_reqs={:?}", incl_reqs);
let mut usage = String::with_capacity(75);
let name = self
@ -346,7 +353,15 @@ impl<'help, 'app> Usage<'help, 'app> {
let mut unrolled_reqs = IndexSet::new();
for a in self.required.iter() {
let required_owned;
let required = if let Some(required) = self.required {
required
} else {
required_owned = self.app.required_graph();
&required_owned
};
for a in required.iter() {
let is_relevant = |(val, req_arg): &(ArgPredicate<'_>, Id)| -> Option<Id> {
let required = match val {
ArgPredicate::Equals(_) => {
@ -380,7 +395,7 @@ impl<'help, 'app> Usage<'help, 'app> {
.app
.groups
.iter()
.filter(|gn| self.required.contains(&gn.id))
.filter(|gn| required.contains(&gn.id))
.flat_map(|g| self.app.unroll_args_in_group(&g.id))
.collect::<Vec<_>>();

View file

@ -18,12 +18,11 @@ use crate::output::{fmt::Colorizer, Help, HelpWriter, Usage};
use crate::parse::features::suggestions;
use crate::parse::{ArgMatcher, SubCommand};
use crate::parse::{Validator, ValueSource};
use crate::util::{color::ColorChoice, ChildGraph, Id};
use crate::util::{color::ColorChoice, Id};
use crate::{INTERNAL_ERROR_MSG, INVALID_UTF8};
pub(crate) struct Parser<'help, 'app> {
pub(crate) app: &'app mut App<'help>,
pub(crate) required: ChildGraph<Id>,
pub(crate) overridden: RefCell<Vec<Id>>,
seen: Vec<Id>,
cur_idx: Cell<usize>,
@ -37,11 +36,8 @@ pub(crate) struct Parser<'help, 'app> {
// Initializing Methods
impl<'help, 'app> Parser<'help, 'app> {
pub(crate) fn new(app: &'app mut App<'help>) -> Self {
let reqs = app.required_graph();
Parser {
app,
required: reqs,
overridden: Default::default(),
seen: Vec::new(),
cur_idx: Cell::new(0),
@ -239,7 +235,7 @@ impl<'help, 'app> Parser<'help, 'app> {
return Err(ClapError::no_equals(
self.app,
arg,
Usage::new(self.app, &self.required).create_usage_with_title(&[]),
Usage::new(self.app).create_usage_with_title(&[]),
));
}
ParseResult::NoMatchingArg { arg } => {
@ -254,7 +250,7 @@ impl<'help, 'app> Parser<'help, 'app> {
self.app,
rest,
arg,
Usage::new(self.app, &self.required).create_usage_no_title(&used),
Usage::new(self.app).create_usage_no_title(&used),
))
}
ParseResult::HelpFlag => {
@ -328,7 +324,7 @@ impl<'help, 'app> Parser<'help, 'app> {
return Err(ClapError::no_equals(
self.app,
arg,
Usage::new(self.app, &self.required).create_usage_with_title(&[]),
Usage::new(self.app).create_usage_with_title(&[]),
))
}
ParseResult::NoMatchingArg { arg } => {
@ -336,7 +332,7 @@ impl<'help, 'app> Parser<'help, 'app> {
self.app,
arg,
None,
Usage::new(self.app, &self.required).create_usage_with_title(&[]),
Usage::new(self.app).create_usage_with_title(&[]),
));
}
ParseResult::HelpFlag => {
@ -381,7 +377,7 @@ impl<'help, 'app> Parser<'help, 'app> {
self.app,
arg_os.to_str_lossy().into_owned(),
None,
Usage::new(self.app, &self.required).create_usage_with_title(&[]),
Usage::new(self.app).create_usage_with_title(&[]),
));
}
@ -422,7 +418,7 @@ impl<'help, 'app> Parser<'help, 'app> {
None => {
return Err(ClapError::invalid_utf8(
self.app,
Usage::new(self.app, &self.required).create_usage_with_title(&[]),
Usage::new(self.app).create_usage_with_title(&[]),
));
}
};
@ -437,7 +433,7 @@ impl<'help, 'app> Parser<'help, 'app> {
if !allow_invalid_utf8 && v.to_str().is_none() {
return Err(ClapError::invalid_utf8(
self.app,
Usage::new(self.app, &self.required).create_usage_with_title(&[]),
Usage::new(self.app).create_usage_with_title(&[]),
));
}
sc_m.add_val_to(
@ -490,7 +486,7 @@ impl<'help, 'app> Parser<'help, 'app> {
return ClapError::unnecessary_double_dash(
self.app,
arg_os.to_str_lossy().into_owned(),
Usage::new(self.app, &self.required).create_usage_with_title(&[]),
Usage::new(self.app).create_usage_with_title(&[]),
);
}
}
@ -511,7 +507,7 @@ impl<'help, 'app> Parser<'help, 'app> {
.as_ref()
.unwrap_or(&self.app.name)
.to_string(),
Usage::new(self.app, &self.required).create_usage_with_title(&[]),
Usage::new(self.app).create_usage_with_title(&[]),
);
}
// If the argument must be a subcommand.
@ -531,7 +527,7 @@ impl<'help, 'app> Parser<'help, 'app> {
self.app,
arg_os.to_str_lossy().into_owned(),
None,
Usage::new(self.app, &self.required).create_usage_with_title(&[]),
Usage::new(self.app).create_usage_with_title(&[]),
)
}
@ -679,8 +675,7 @@ impl<'help, 'app> Parser<'help, 'app> {
let mut mid_string = String::from(" ");
if !self.app.is_subcommand_negates_reqs_set() {
let reqs =
Usage::new(self.app, &self.required).get_required_usage_from(&[], None, true); // maybe Some(m)
let reqs = Usage::new(self.app).get_required_usage_from(&[], None, true); // maybe Some(m)
for s in &reqs {
mid_string.push_str(s);
@ -908,13 +903,14 @@ impl<'help, 'app> Parser<'help, 'app> {
);
self.parse_opt(val, opt, matcher, trailing_values)
} else if let Some(rest) = val {
let required = self.app.required_graph();
debug!("Parser::parse_long_arg: Got invalid literal `{:?}`", rest);
let used: Vec<Id> = matcher
.arg_names()
.filter(|&n| {
self.app.find(n).map_or(true, |a| {
!(a.is_hide_set() || self.required.contains(&a.id))
})
self.app
.find(n)
.map_or(true, |a| !(a.is_hide_set() || required.contains(&a.id)))
})
.cloned()
.collect();
@ -1509,12 +1505,13 @@ impl<'help, 'app> Parser<'help, 'app> {
}
}
let required = self.app.required_graph();
let used: Vec<Id> = matcher
.arg_names()
.filter(|n| {
self.app.find(n).map_or(true, |a| {
!(self.required.contains(&a.id) || a.is_hide_set())
})
self.app
.find(n)
.map_or(true, |a| !(required.contains(&a.id) || a.is_hide_set()))
})
.cloned()
.collect();
@ -1523,12 +1520,14 @@ impl<'help, 'app> Parser<'help, 'app> {
self.app,
format!("--{}", arg),
did_you_mean,
Usage::new(self.app, &self.required).create_usage_with_title(&*used),
Usage::new(self.app)
.required(&required)
.create_usage_with_title(&*used),
)
}
pub(crate) fn write_help_err(&self) -> ClapResult<Colorizer> {
let usage = Usage::new(self.app, &self.required);
let usage = Usage::new(self.app);
let mut c = Colorizer::new(true, self.color_help());
Help::new(HelpWriter::Buffer(&mut c), self.app, &usage, false).write_help()?;
Ok(c)
@ -1541,7 +1540,7 @@ impl<'help, 'app> Parser<'help, 'app> {
);
use_long = use_long && self.use_long_help();
let usage = Usage::new(self.app, &self.required);
let usage = Usage::new(self.app);
let mut c = Colorizer::new(false, self.color_help());
match Help::new(HelpWriter::Buffer(&mut c), self.app, &usage, use_long).write_help() {

View file

@ -3,16 +3,19 @@ use crate::build::{App, AppSettings, Arg, ArgPredicate, PossibleValue};
use crate::error::{Error, Result as ClapResult};
use crate::output::Usage;
use crate::parse::{ArgMatcher, MatchedArg, ParseState, Parser};
use crate::util::ChildGraph;
use crate::util::Id;
use crate::{INTERNAL_ERROR_MSG, INVALID_UTF8};
pub(crate) struct Validator<'help, 'app, 'parser> {
p: &'parser mut Parser<'help, 'app>,
required: ChildGraph<Id>,
}
impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
pub(crate) fn new(p: &'parser mut Parser<'help, 'app>) -> Self {
Validator { p }
let required = p.app.required_graph();
Validator { p, required }
}
pub(crate) fn validate(
@ -46,7 +49,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
.filter_map(PossibleValue::get_visible_name)
.collect::<Vec<_>>(),
o,
Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
Usage::new(self.p.app)
.required(&self.required)
.create_usage_with_title(&[]),
));
}
}
@ -67,7 +72,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
return Err(Error::missing_subcommand(
self.p.app,
bn.to_string(),
Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
Usage::new(self.p.app)
.required(&self.required)
.create_usage_with_title(&[]),
));
} else if !has_subcmd && self.p.app.is_set(AppSettings::SubcommandRequiredElseHelp) {
debug!("Validator::new::get_matches_with: SubcommandRequiredElseHelp=true");
@ -99,7 +106,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
);
return Err(Error::invalid_utf8(
self.p.app,
Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
Usage::new(self.p.app)
.required(&self.required)
.create_usage_with_title(&[]),
));
}
if !arg.possible_vals.is_empty() {
@ -118,7 +127,7 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
.filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent))
.filter(|&n| {
self.p.app.find(n).map_or(true, |a| {
!(a.is_hide_set() || self.p.required.contains(&a.id))
!(a.is_hide_set() || self.required.contains(&a.id))
})
})
.cloned()
@ -131,7 +140,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
.filter_map(PossibleValue::get_visible_name)
.collect::<Vec<_>>(),
arg,
Usage::new(self.p.app, &self.p.required).create_usage_with_title(&used),
Usage::new(self.p.app)
.required(&self.required)
.create_usage_with_title(&used),
));
}
}
@ -144,7 +155,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
.filter_map(PossibleValue::get_visible_name)
.collect::<Vec<_>>(),
arg,
Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
Usage::new(self.p.app)
.required(&self.required)
.create_usage_with_title(&[]),
));
}
@ -221,7 +234,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
self.p.app,
arg,
Vec::new(),
Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
Usage::new(self.p.app)
.required(&self.required)
.create_usage_with_title(&[]),
))
})
}
@ -277,7 +292,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
.chain(used_filtered.iter())
.cloned()
.collect();
Usage::new(self.p.app, &self.p.required).create_usage_with_title(&required)
Usage::new(self.p.app)
.required(&self.required)
.create_usage_with_title(&required)
}
fn gather_requires(&mut self, matcher: &ArgMatcher) {
@ -294,12 +311,12 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
};
for req in self.p.app.unroll_arg_requires(is_relevant, &arg.id) {
self.p.required.insert(req);
self.required.insert(req);
}
} else if let Some(g) = self.p.app.find_group(name) {
debug!("Validator::gather_requires:iter:{:?}:group", name);
for r in &g.requires {
self.p.required.insert(r.clone());
self.required.insert(r.clone());
}
}
}
@ -335,7 +352,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
return Err(Error::unexpected_multiple_usage(
self.p.app,
a,
Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
Usage::new(self.p.app)
.required(&self.required)
.create_usage_with_title(&[]),
));
}
if let Some(max_occurs) = a.max_occurs {
@ -350,7 +369,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
a,
max_occurs,
occurs,
Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
Usage::new(self.p.app)
.required(&self.required)
.create_usage_with_title(&[]),
));
}
}
@ -379,7 +400,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
} else {
total_num
},
Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
Usage::new(self.p.app)
.required(&self.required)
.create_usage_with_title(&[]),
));
}
}
@ -396,7 +419,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
.expect(INVALID_UTF8)
.to_string(),
a.to_string(),
Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
Usage::new(self.p.app)
.required(&self.required)
.create_usage_with_title(&[]),
));
}
}
@ -409,7 +434,9 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
a,
num,
ma.num_vals(),
Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
Usage::new(self.p.app)
.required(&self.required)
.create_usage_with_title(&[]),
));
}
num == 0
@ -426,20 +453,19 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
.filter_map(PossibleValue::get_visible_name)
.collect::<Vec<_>>(),
a,
Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]),
Usage::new(self.p.app)
.required(&self.required)
.create_usage_with_title(&[]),
));
}
Ok(())
}
fn validate_required(&mut self, matcher: &ArgMatcher) -> ClapResult<()> {
debug!(
"Validator::validate_required: required={:?}",
self.p.required
);
debug!("Validator::validate_required: required={:?}", self.required);
self.gather_requires(matcher);
for arg_or_group in self.p.required.iter().filter(|r| !matcher.contains(r)) {
for arg_or_group in self.required.iter().filter(|r| !matcher.contains(r)) {
debug!("Validator::validate_required:iter:aog={:?}", arg_or_group);
if let Some(arg) = self.p.app.find(arg_or_group) {
debug!("Validator::validate_required:iter: This is an arg");
@ -535,10 +561,10 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
debug!("Validator::missing_required_error; incl={:?}", incl);
debug!(
"Validator::missing_required_error: reqs={:?}",
self.p.required
self.required
);
let usg = Usage::new(self.p.app, &self.p.required);
let usg = Usage::new(self.p.app).required(&self.required);
let req_args = usg.get_required_usage_from(&incl, Some(matcher), true);
@ -552,9 +578,10 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> {
.filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent))
.filter(|n| {
// Filter out the args we don't want to specify.
self.p.app.find(n).map_or(true, |a| {
!a.is_hide_set() && !self.p.required.contains(&a.id)
})
self.p
.app
.find(n)
.map_or(true, |a| !a.is_hide_set() && !self.required.contains(&a.id))
})
.cloned()
.chain(incl)