From de89c050c9e20690c3abbbe53f5868299ba15d46 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 21 Apr 2022 15:03:56 -0500 Subject: [PATCH 1/8] fix(validate): Consistent conflict behavior We had two different implementations of conflicts - Validating conflicts - Allowing conflicts to override `required` - Missing members of a group conflicting with each other - Missing symmetric conflicts (A conflicts with B so B conflicts with A) This consolidates their implementations which should fix overriding of `required`. --- src/parse/validator.rs | 48 ++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/src/parse/validator.rs b/src/parse/validator.rs index e1d17ac2..d2cd3538 100644 --- a/src/parse/validator.rs +++ b/src/parse/validator.rs @@ -25,6 +25,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { trailing_values: bool, ) -> ClapResult<()> { debug!("Validator::validate"); + let mut conflicts = Conflicts::new(); let has_subcmd = matcher.subcommand_name().is_some(); #[cfg(feature = "env")] @@ -87,9 +88,9 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { return Err(Error::display_help_error(self.p.cmd, message)); } - self.validate_conflicts(matcher)?; + self.validate_conflicts(matcher, &mut conflicts)?; if !(self.p.cmd.is_subcommand_negates_reqs_set() && has_subcmd) { - self.validate_required(matcher)?; + self.validate_required(matcher, &mut conflicts)?; } self.validate_matched_args(matcher)?; @@ -202,12 +203,15 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { Ok(()) } - fn validate_conflicts(&self, matcher: &ArgMatcher) -> ClapResult<()> { + fn validate_conflicts( + &mut self, + matcher: &ArgMatcher, + conflicts: &mut Conflicts, + ) -> ClapResult<()> { debug!("Validator::validate_conflicts"); self.validate_exclusive(matcher)?; - let mut conflicts = Conflicts::new(); for arg_id in matcher .arg_names() .filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent)) @@ -469,7 +473,11 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { Ok(()) } - fn validate_required(&mut self, matcher: &ArgMatcher) -> ClapResult<()> { + fn validate_required( + &mut self, + matcher: &ArgMatcher, + conflicts: &mut Conflicts, + ) -> ClapResult<()> { debug!("Validator::validate_required: required={:?}", self.required); self.gather_requires(matcher); @@ -477,7 +485,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { debug!("Validator::validate_required:iter:aog={:?}", arg_or_group); if let Some(arg) = self.p.cmd.find(arg_or_group) { debug!("Validator::validate_required:iter: This is an arg"); - if !self.is_missing_required_ok(arg, matcher) { + if !self.is_missing_required_ok(arg, matcher, conflicts) { return self.missing_required_error(matcher, vec![]); } } else if let Some(group) = self.p.cmd.find_group(arg_or_group) { @@ -517,21 +525,25 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { Ok(()) } - fn is_missing_required_ok(&self, a: &Arg<'help>, matcher: &ArgMatcher) -> bool { + fn is_missing_required_ok( + &self, + a: &Arg<'help>, + matcher: &ArgMatcher, + conflicts: &mut Conflicts, + ) -> bool { debug!("Validator::is_missing_required_ok: {}", a.name); - self.validate_arg_conflicts(a, matcher) || self.p.overridden.borrow().contains(&a.id) + self.arg_has_conflicts(a, matcher, conflicts) || self.p.overridden.borrow().contains(&a.id) } - fn validate_arg_conflicts(&self, a: &Arg<'help>, matcher: &ArgMatcher) -> bool { - debug!("Validator::validate_arg_conflicts: a={:?}", a.name); - a.blacklist.iter().any(|conf| { - matcher.contains(conf) - || self - .p - .cmd - .find_group(conf) - .map_or(false, |g| g.args.iter().any(|arg| matcher.contains(arg))) - }) + fn arg_has_conflicts( + &self, + a: &Arg<'help>, + matcher: &ArgMatcher, + conflicts: &mut Conflicts, + ) -> bool { + debug!("Validator::arg_has_conflicts: a={:?}", a.name); + let conflicts = conflicts.gather_conflicts(self.p.cmd, matcher, &a.id); + !conflicts.is_empty() } fn validate_required_unless(&self, matcher: &ArgMatcher) -> ClapResult<()> { From a484f622dec469d1151c538610aff6a5bb76b8db Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 21 Apr 2022 15:18:02 -0500 Subject: [PATCH 2/8] fix(validate): Overrides always ignore required Before, if two arguments were required *and* overrode each other, then `cmd --opt=1 --other=2` succeded but `cmd --other=2` failed despite ignoring `--opt=1`. Requiring `--opt=1` to be present but unavailable doesn't help anyone and makes the behavior less predictable. Now both commands will have the same behavior. --- src/build/arg.rs | 4 ++++ src/parse/parser.rs | 6 +----- src/parse/validator.rs | 14 ++++--------- tests/builder/app_settings.rs | 38 +++++++++++++++++++++++++++++++++++ 4 files changed, 47 insertions(+), 15 deletions(-) diff --git a/src/build/arg.rs b/src/build/arg.rs index 7be3cc9f..6f73b4c0 100644 --- a/src/build/arg.rs +++ b/src/build/arg.rs @@ -4339,6 +4339,8 @@ impl<'help> Arg<'help> { /// **NOTE:** When an argument is overridden it is essentially as if it never was used, any /// conflicts, requirements, etc. are evaluated **after** all "overrides" have been removed /// + /// **NOTE:** Overriding an argument implies they [conflict][Arg::conflicts_with`]. + /// /// **WARNING:** Positional arguments and options which accept /// [`Arg::multiple_occurrences`] cannot override themselves (or we /// would never be able to advance to the next positional). If a positional @@ -4454,6 +4456,8 @@ impl<'help> Arg<'help> { /// **NOTE:** When an argument is overridden it is essentially as if it never was used, any /// conflicts, requirements, etc. are evaluated **after** all "overrides" have been removed /// + /// **NOTE:** Overriding an argument implies they [conflict][Arg::conflicts_with_all`]. + /// /// # Examples /// /// ```rust diff --git a/src/parse/parser.rs b/src/parse/parser.rs index 95f027e7..4697ff01 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -1,6 +1,6 @@ // Std use std::{ - cell::{Cell, RefCell}, + cell::Cell, ffi::{OsStr, OsString}, }; @@ -22,7 +22,6 @@ use crate::{INTERNAL_ERROR_MSG, INVALID_UTF8}; pub(crate) struct Parser<'help, 'cmd> { pub(crate) cmd: &'cmd mut Command<'help>, - pub(crate) overridden: RefCell>, seen: Vec, cur_idx: Cell, /// Index of the previous flag subcommand in a group of flags. @@ -37,7 +36,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { pub(crate) fn new(cmd: &'cmd mut Command<'help>) -> Self { Parser { cmd, - overridden: Default::default(), seen: Vec::new(), cur_idx: Cell::new(0), flag_subcmd_at: None, @@ -1248,7 +1246,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { for override_id in &arg.overrides { debug!("Parser::remove_overrides:iter:{:?}: removing", override_id); matcher.remove(override_id); - self.overridden.borrow_mut().push(override_id.clone()); } // Override anything that can override us @@ -1263,7 +1260,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { for overrider_id in transitive { debug!("Parser::remove_overrides:iter:{:?}: removing", overrider_id); matcher.remove(overrider_id); - self.overridden.borrow_mut().push(overrider_id.clone()); } } diff --git a/src/parse/validator.rs b/src/parse/validator.rs index d2cd3538..1a27fc5c 100644 --- a/src/parse/validator.rs +++ b/src/parse/validator.rs @@ -532,16 +532,6 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { conflicts: &mut Conflicts, ) -> bool { debug!("Validator::is_missing_required_ok: {}", a.name); - self.arg_has_conflicts(a, matcher, conflicts) || self.p.overridden.borrow().contains(&a.id) - } - - fn arg_has_conflicts( - &self, - a: &Arg<'help>, - matcher: &ArgMatcher, - conflicts: &mut Conflicts, - ) -> bool { - debug!("Validator::arg_has_conflicts: a={:?}", a.name); let conflicts = conflicts.gather_conflicts(self.p.cmd, matcher, &a.id); !conflicts.is_empty() } @@ -666,6 +656,10 @@ impl Conflicts { } } } + + // Overrides are implicitly conflicts + conf.extend(arg.overrides.iter().cloned()); + conf } else if let Some(group) = cmd.find_group(arg_id) { group.conflicts.clone() diff --git a/tests/builder/app_settings.rs b/tests/builder/app_settings.rs index 6c6453e4..cdcdd804 100644 --- a/tests/builder/app_settings.rs +++ b/tests/builder/app_settings.rs @@ -1028,6 +1028,44 @@ fn aaos_opts_w_other_overrides_rev_2() { assert_eq!(m.value_of("other"), Some("val")); } +#[test] +fn aaos_opts_w_override_as_conflict_1() { + // opts with other overrides, rev + let res = Command::new("posix") + .args_override_self(true) + .arg( + arg!(--opt "some option") + .required(true) + .overrides_with("other"), + ) + .arg(arg!(--other "some other option").required(true)) + .try_get_matches_from(vec!["", "--opt=some"]); + assert!(res.is_ok(), "{}", res.unwrap_err()); + let m = res.unwrap(); + assert!(m.is_present("opt")); + assert!(!m.is_present("other")); + assert_eq!(m.value_of("opt"), Some("some")); +} + +#[test] +fn aaos_opts_w_override_as_conflict_2() { + // opts with other overrides, rev + let res = Command::new("posix") + .args_override_self(true) + .arg( + arg!(--opt "some option") + .required(true) + .overrides_with("other"), + ) + .arg(arg!(--other "some other option").required(true)) + .try_get_matches_from(vec!["", "--other=some"]); + assert!(res.is_ok(), "{}", res.unwrap_err()); + let m = res.unwrap(); + assert!(!m.is_present("opt")); + assert!(m.is_present("other")); + assert_eq!(m.value_of("other"), Some("some")); +} + #[test] fn aaos_opts_mult() { // opts with multiple From 639f9e88497c435ddd906e8539d9ec47f6525aba Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 21 Apr 2022 15:49:00 -0500 Subject: [PATCH 3/8] refactor(parser): Pull out long-help determination This isn't a parser policy but command-level policy. --- src/build/command.rs | 23 +++++++++++++++++++++++ src/parse/parser.rs | 28 +++------------------------- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/build/command.rs b/src/build/command.rs index 86da3041..312e77b2 100644 --- a/src/build/command.rs +++ b/src/build/command.rs @@ -24,6 +24,7 @@ use crate::output::{fmt::Colorizer, Help, HelpWriter, Usage}; use crate::parse::{ArgMatcher, ArgMatches, Parser}; use crate::util::ChildGraph; use crate::util::{color::ColorChoice, Id, Key}; +use crate::PossibleValue; use crate::{Error, INTERNAL_ERROR_MSG}; #[cfg(debug_assertions)] @@ -4675,6 +4676,28 @@ impl<'help> App<'help> { pub(crate) fn get_display_order(&self) -> usize { self.disp_ord.unwrap_or(999) } + + pub(crate) fn use_long_help(&self) -> bool { + debug!("Command::use_long_help"); + // In this case, both must be checked. This allows the retention of + // original formatting, but also ensures that the actual -h or --help + // specified by the user is sent through. If hide_short_help is not included, + // then items specified with hidden_short_help will also be hidden. + let should_long = |v: &Arg| { + v.long_help.is_some() + || v.is_hide_long_help_set() + || v.is_hide_short_help_set() + || cfg!(feature = "unstable-v4") + && v.possible_vals.iter().any(PossibleValue::should_show_help) + }; + + // Subcommands aren't checked because we prefer short help for them, deferring to + // `cmd subcmd --help` for more. + self.get_long_about().is_some() + || self.get_before_long_help().is_some() + || self.get_after_long_help().is_some() + || self.get_arguments().any(should_long) + } } impl<'help> Default for App<'help> { diff --git a/src/parse/parser.rs b/src/parse/parser.rs index 4697ff01..b344fed2 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -8,6 +8,7 @@ use std::{ use clap_lex::RawOsStr; // Internal +use crate::build::AppSettings as AS; use crate::build::{Arg, Command}; use crate::error::Error as ClapError; use crate::error::Result as ClapResult; @@ -17,7 +18,6 @@ use crate::parse::features::suggestions; use crate::parse::{ArgMatcher, SubCommand}; use crate::parse::{Validator, ValueSource}; use crate::util::{color::ColorChoice, Id}; -use crate::{build::AppSettings as AS, PossibleValue}; use crate::{INTERNAL_ERROR_MSG, INVALID_UTF8}; pub(crate) struct Parser<'help, 'cmd> { @@ -805,28 +805,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { None } - fn use_long_help(&self) -> bool { - debug!("Parser::use_long_help"); - // In this case, both must be checked. This allows the retention of - // original formatting, but also ensures that the actual -h or --help - // specified by the user is sent through. If hide_short_help is not included, - // then items specified with hidden_short_help will also be hidden. - let should_long = |v: &Arg| { - v.long_help.is_some() - || v.is_hide_long_help_set() - || v.is_hide_short_help_set() - || cfg!(feature = "unstable-v4") - && v.possible_vals.iter().any(PossibleValue::should_show_help) - }; - - // Subcommands aren't checked because we prefer short help for them, deferring to - // `cmd subcmd --help` for more. - self.cmd.get_long_about().is_some() - || self.cmd.get_before_long_help().is_some() - || self.cmd.get_after_long_help().is_some() - || self.cmd.get_arguments().any(should_long) - } - fn parse_long_arg( &mut self, matcher: &mut ArgMatcher, @@ -1539,10 +1517,10 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { fn help_err(&self, mut use_long: bool) -> ClapError { debug!( "Parser::help_err: use_long={:?}", - use_long && self.use_long_help() + use_long && self.cmd.use_long_help() ); - use_long = use_long && self.use_long_help(); + use_long = use_long && self.cmd.use_long_help(); let usage = Usage::new(self.cmd); let mut c = Colorizer::new(false, self.color_help()); From 6a9a5d05b0bb3bf96ea5aafdf001f2202ea07e8d Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 21 Apr 2022 16:01:47 -0500 Subject: [PATCH 4/8] refactor(help): Make bool's meaning clearer --- src/build/command.rs | 5 +++-- src/error/mod.rs | 19 ++++++++++------- src/macros.rs | 2 +- src/output/fmt.rs | 49 +++++++++++++++++++++++++------------------- src/parse/parser.rs | 7 ++++--- 5 files changed, 48 insertions(+), 34 deletions(-) diff --git a/src/build/command.rs b/src/build/command.rs index 312e77b2..71fcda65 100644 --- a/src/build/command.rs +++ b/src/build/command.rs @@ -20,6 +20,7 @@ use crate::build::{arg::ArgProvider, Arg, ArgGroup, ArgPredicate}; use crate::error::ErrorKind; use crate::error::Result as ClapResult; use crate::mkeymap::MKeyMap; +use crate::output::fmt::Stream; use crate::output::{fmt::Colorizer, Help, HelpWriter, Usage}; use crate::parse::{ArgMatcher, ArgMatches, Parser}; use crate::util::ChildGraph; @@ -697,7 +698,7 @@ impl<'help> App<'help> { self._build(); let color = self.get_color(); - let mut c = Colorizer::new(false, color); + let mut c = Colorizer::new(Stream::Stdout, color); let usage = Usage::new(self); Help::new(HelpWriter::Buffer(&mut c), self, &usage, false).write_help()?; c.print() @@ -722,7 +723,7 @@ impl<'help> App<'help> { self._build(); let color = self.get_color(); - let mut c = Colorizer::new(false, color); + let mut c = Colorizer::new(Stream::Stdout, color); let usage = Usage::new(self); Help::new(HelpWriter::Buffer(&mut c), self, &usage, true).write_help()?; c.print() diff --git a/src/error/mod.rs b/src/error/mod.rs index 2b8b8380..d4fe8a32 100644 --- a/src/error/mod.rs +++ b/src/error/mod.rs @@ -15,6 +15,7 @@ use std::{ use crate::{ build::Arg, output::fmt::Colorizer, + output::fmt::Stream, parse::features::suggestions, util::{color::ColorChoice, safe_exit, SUCCESS_CODE, USAGE_CODE}, AppSettings, Command, @@ -97,10 +98,14 @@ impl Error { /// Should the message be written to `stdout` or not? #[inline] pub fn use_stderr(&self) -> bool { - !matches!( - self.kind(), - ErrorKind::DisplayHelp | ErrorKind::DisplayVersion - ) + self.stream() == Stream::Stderr + } + + pub(crate) fn stream(&self) -> Stream { + match self.kind() { + ErrorKind::DisplayHelp | ErrorKind::DisplayVersion => Stream::Stdout, + _ => Stream::Stderr, + } } /// Prints the error and exits. @@ -608,7 +613,7 @@ impl Error { if let Some(message) = self.inner.message.as_ref() { message.formatted() } else { - let mut c = Colorizer::new(self.use_stderr(), self.inner.color_when); + let mut c = Colorizer::new(self.stream(), self.inner.color_when); start_error(&mut c); @@ -1090,7 +1095,7 @@ impl Message { fn format(&mut self, cmd: &Command, usage: String) { match self { Message::Raw(s) => { - let mut c = Colorizer::new(true, cmd.get_color()); + let mut c = Colorizer::new(Stream::Stderr, cmd.get_color()); let mut message = String::new(); std::mem::swap(s, &mut message); @@ -1107,7 +1112,7 @@ impl Message { fn formatted(&self) -> Cow { match self { Message::Raw(s) => { - let mut c = Colorizer::new(true, ColorChoice::Never); + let mut c = Colorizer::new(Stream::Stderr, ColorChoice::Never); start_error(&mut c); c.none(s); Cow::Owned(c) diff --git a/src/macros.rs b/src/macros.rs index a9b99537..f43573b4 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -933,7 +933,7 @@ macro_rules! debug { ($($arg:tt)*) => ({ let prefix = format!("[{:>w$}] \t", module_path!(), w = 28); let body = format!($($arg)*); - let mut color = $crate::output::fmt::Colorizer::new(true, $crate::ColorChoice::Auto); + let mut color = $crate::output::fmt::Colorizer::new($crate::output::fmt::Stream::Stderr, $crate::ColorChoice::Auto); color.hint(prefix); color.hint(body); color.none("\n"); diff --git a/src/output/fmt.rs b/src/output/fmt.rs index d524982b..85ba1774 100644 --- a/src/output/fmt.rs +++ b/src/output/fmt.rs @@ -5,9 +5,15 @@ use std::{ io::{self, Write}, }; +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub(crate) enum Stream { + Stdout, + Stderr, +} + #[derive(Clone, Debug)] pub(crate) struct Colorizer { - use_stderr: bool, + stream: Stream, #[allow(unused)] color_when: ColorChoice, pieces: Vec<(String, Style)>, @@ -15,9 +21,9 @@ pub(crate) struct Colorizer { impl Colorizer { #[inline(never)] - pub(crate) fn new(use_stderr: bool, color_when: ColorChoice) -> Self { + pub(crate) fn new(stream: Stream, color_when: ColorChoice) -> Self { Colorizer { - use_stderr, + stream, color_when, pieces: vec![], } @@ -58,14 +64,13 @@ impl Colorizer { let color_when = match self.color_when { ColorChoice::Always => DepColorChoice::Always, - ColorChoice::Auto if is_a_tty(self.use_stderr) => DepColorChoice::Auto, + ColorChoice::Auto if is_a_tty(self.stream) => DepColorChoice::Auto, _ => DepColorChoice::Never, }; - let writer = if self.use_stderr { - BufferWriter::stderr(color_when) - } else { - BufferWriter::stdout(color_when) + let writer = match self.stream { + Stream::Stdout => BufferWriter::stderr(color_when), + Stream::Stderr => BufferWriter::stdout(color_when), }; let mut buffer = writer.buffer(); @@ -101,14 +106,17 @@ impl Colorizer { pub(crate) fn print(&self) -> io::Result<()> { // [e]println can't be used here because it panics // if something went wrong. We don't want that. - if self.use_stderr { - let stderr = std::io::stderr(); - let mut stderr = stderr.lock(); - write!(stderr, "{}", self) - } else { - let stdout = std::io::stdout(); - let mut stdout = stdout.lock(); - write!(stdout, "{}", self) + match self.stream { + Stream::Stdout => { + let stdout = std::io::stdout(); + let mut stdout = stdout.lock(); + write!(stdout, "{}", self) + } + Stream::Stderr => { + let stderr = std::io::stderr(); + let mut stderr = stderr.lock(); + write!(stderr, "{}", self) + } } } } @@ -140,11 +148,10 @@ impl Default for Style { } #[cfg(feature = "color")] -fn is_a_tty(stderr: bool) -> bool { - let stream = if stderr { - atty::Stream::Stderr - } else { - atty::Stream::Stdout +fn is_a_tty(stream: Stream) -> bool { + let stream = match stream { + Stream::Stdout => atty::Stream::Stdout, + Stream::Stderr => atty::Stream::Stderr, }; atty::is(stream) diff --git a/src/parse/parser.rs b/src/parse/parser.rs index b344fed2..d2662914 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -13,6 +13,7 @@ use crate::build::{Arg, Command}; use crate::error::Error as ClapError; use crate::error::Result as ClapResult; use crate::mkeymap::KeyType; +use crate::output::fmt::Stream; use crate::output::{fmt::Colorizer, Help, HelpWriter, Usage}; use crate::parse::features::suggestions; use crate::parse::{ArgMatcher, SubCommand}; @@ -1509,7 +1510,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { pub(crate) fn write_help_err(&self) -> ClapResult { let usage = Usage::new(self.cmd); - let mut c = Colorizer::new(true, self.color_help()); + let mut c = Colorizer::new(Stream::Stderr, self.color_help()); Help::new(HelpWriter::Buffer(&mut c), self.cmd, &usage, false).write_help()?; Ok(c) } @@ -1522,7 +1523,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { use_long = use_long && self.cmd.use_long_help(); let usage = Usage::new(self.cmd); - let mut c = Colorizer::new(false, self.color_help()); + let mut c = Colorizer::new(Stream::Stdout, self.color_help()); match Help::new(HelpWriter::Buffer(&mut c), self.cmd, &usage, use_long).write_help() { Err(e) => e.into(), @@ -1534,7 +1535,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { debug!("Parser::version_err"); let msg = self.cmd._render_version(use_long); - let mut c = Colorizer::new(false, self.color_help()); + let mut c = Colorizer::new(Stream::Stdout, self.color_help()); c.none(msg); ClapError::display_version(self.cmd, c) } From ebeade91bff46878c4940b3e1b63900e947af459 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 21 Apr 2022 16:12:01 -0500 Subject: [PATCH 5/8] refactor(help): Consolidate help errors --- src/parse/parser.rs | 39 +++++++++++++++++++++------------------ src/parse/validator.rs | 5 +++-- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/parse/parser.rs b/src/parse/parser.rs index d2662914..8d10f81b 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -258,7 +258,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { )) } ParseResult::HelpFlag => { - return Err(self.help_err(true)); + return Err(self.help_err(true, Stream::Stdout)); } ParseResult::VersionFlag => { return Err(self.version_err(true)); @@ -341,7 +341,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { )); } ParseResult::HelpFlag => { - return Err(self.help_err(false)); + return Err(self.help_err(false, Stream::Stdout)); } ParseResult::VersionFlag => { return Err(self.version_err(false)); @@ -646,7 +646,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { let parser = Parser::new(&mut sc); - Err(parser.help_err(true)) + Err(parser.help_err(true, Stream::Stdout)) } fn is_new_arg(&self, next: &clap_lex::ParsedArg<'_>, current_positional: &Arg) -> bool { @@ -1418,7 +1418,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { // Early return on `HelpFlag` or `VersionFlag`. match self.check_for_help_and_version_str(&val) { Some(ParseResult::HelpFlag) => { - return Err(self.help_err(true)); + return Err(self.help_err(true, Stream::Stdout)); } Some(ParseResult::VersionFlag) => { return Err(self.version_err(true)); @@ -1508,26 +1508,29 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { ) } - pub(crate) fn write_help_err(&self) -> ClapResult { - let usage = Usage::new(self.cmd); - let mut c = Colorizer::new(Stream::Stderr, self.color_help()); - Help::new(HelpWriter::Buffer(&mut c), self.cmd, &usage, false).write_help()?; - Ok(c) - } - - fn help_err(&self, mut use_long: bool) -> ClapError { + pub(crate) fn write_help_err( + &self, + mut use_long: bool, + stream: Stream, + ) -> ClapResult { debug!( - "Parser::help_err: use_long={:?}", - use_long && self.cmd.use_long_help() + "Parser::write_help_err: use_long={:?}, stream={:?}", + use_long && self.cmd.use_long_help(), + stream ); use_long = use_long && self.cmd.use_long_help(); let usage = Usage::new(self.cmd); - let mut c = Colorizer::new(Stream::Stdout, self.color_help()); - match Help::new(HelpWriter::Buffer(&mut c), self.cmd, &usage, use_long).write_help() { - Err(e) => e.into(), - _ => ClapError::display_help(self.cmd, c), + let mut c = Colorizer::new(stream, self.color_help()); + Help::new(HelpWriter::Buffer(&mut c), self.cmd, &usage, use_long).write_help()?; + Ok(c) + } + + fn help_err(&self, use_long: bool, stream: Stream) -> ClapError { + match self.write_help_err(use_long, stream) { + Ok(c) => ClapError::display_help(self.cmd, c), + Err(e) => e, } } diff --git a/src/parse/validator.rs b/src/parse/validator.rs index 1a27fc5c..77895a57 100644 --- a/src/parse/validator.rs +++ b/src/parse/validator.rs @@ -1,6 +1,7 @@ // Internal use crate::build::{AppSettings, Arg, ArgPredicate, Command, PossibleValue}; use crate::error::{Error, Result as ClapResult}; +use crate::output::fmt::Stream; use crate::output::Usage; use crate::parse::{ArgMatcher, MatchedArg, ParseState, Parser}; use crate::util::ChildGraph; @@ -64,7 +65,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { .filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent)) .count(); if num_user_values == 0 { - let message = self.p.write_help_err()?; + let message = self.p.write_help_err(false, Stream::Stderr)?; return Err(Error::display_help_error(self.p.cmd, message)); } } @@ -84,7 +85,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { )); } else if !has_subcmd && self.p.cmd.is_set(AppSettings::SubcommandRequiredElseHelp) { debug!("Validator::new::get_matches_with: SubcommandRequiredElseHelp=true"); - let message = self.p.write_help_err()?; + let message = self.p.write_help_err(false, Stream::Stderr)?; return Err(Error::display_help_error(self.p.cmd, message)); } From 02ffd5983051da1c6bcd6f8c05aace1923c737df Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 21 Apr 2022 16:14:11 -0500 Subject: [PATCH 6/8] refactor(help): Move help writing down a layer --- src/build/command.rs | 29 +++++++++++++++++++++++++++++ src/parse/parser.rs | 37 ++++--------------------------------- src/parse/validator.rs | 4 ++-- 3 files changed, 35 insertions(+), 35 deletions(-) diff --git a/src/build/command.rs b/src/build/command.rs index 71fcda65..7cd03ffc 100644 --- a/src/build/command.rs +++ b/src/build/command.rs @@ -4678,6 +4678,25 @@ impl<'help> App<'help> { self.disp_ord.unwrap_or(999) } + pub(crate) fn write_help_err( + &self, + mut use_long: bool, + stream: Stream, + ) -> ClapResult { + debug!( + "Parser::write_help_err: use_long={:?}, stream={:?}", + use_long && self.use_long_help(), + stream + ); + + use_long = use_long && self.use_long_help(); + let usage = Usage::new(self); + + let mut c = Colorizer::new(stream, self.color_help()); + Help::new(HelpWriter::Buffer(&mut c), self, &usage, use_long).write_help()?; + Ok(c) + } + pub(crate) fn use_long_help(&self) -> bool { debug!("Command::use_long_help"); // In this case, both must be checked. This allows the retention of @@ -4699,6 +4718,16 @@ impl<'help> App<'help> { || self.get_after_long_help().is_some() || self.get_arguments().any(should_long) } + + // Should we color the help? + pub(crate) fn color_help(&self) -> ColorChoice { + #[cfg(feature = "color")] + if self.is_disable_colored_help_set() { + return ColorChoice::Never; + } + + self.get_color() + } } impl<'help> Default for App<'help> { diff --git a/src/parse/parser.rs b/src/parse/parser.rs index 8d10f81b..c6b83a06 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -14,11 +14,11 @@ use crate::error::Error as ClapError; use crate::error::Result as ClapResult; use crate::mkeymap::KeyType; use crate::output::fmt::Stream; -use crate::output::{fmt::Colorizer, Help, HelpWriter, Usage}; +use crate::output::{fmt::Colorizer, Usage}; use crate::parse::features::suggestions; use crate::parse::{ArgMatcher, SubCommand}; use crate::parse::{Validator, ValueSource}; -use crate::util::{color::ColorChoice, Id}; +use crate::util::Id; use crate::{INTERNAL_ERROR_MSG, INVALID_UTF8}; pub(crate) struct Parser<'help, 'cmd> { @@ -43,16 +43,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { flag_subcmd_skip: 0, } } - - // Should we color the help? - pub(crate) fn color_help(&self) -> ColorChoice { - #[cfg(feature = "color")] - if self.cmd.is_disable_colored_help_set() { - return ColorChoice::Never; - } - - self.cmd.get_color() - } } // Parsing Methods @@ -1508,27 +1498,8 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { ) } - pub(crate) fn write_help_err( - &self, - mut use_long: bool, - stream: Stream, - ) -> ClapResult { - debug!( - "Parser::write_help_err: use_long={:?}, stream={:?}", - use_long && self.cmd.use_long_help(), - stream - ); - - use_long = use_long && self.cmd.use_long_help(); - let usage = Usage::new(self.cmd); - - let mut c = Colorizer::new(stream, self.color_help()); - Help::new(HelpWriter::Buffer(&mut c), self.cmd, &usage, use_long).write_help()?; - Ok(c) - } - fn help_err(&self, use_long: bool, stream: Stream) -> ClapError { - match self.write_help_err(use_long, stream) { + match self.cmd.write_help_err(use_long, stream) { Ok(c) => ClapError::display_help(self.cmd, c), Err(e) => e, } @@ -1538,7 +1509,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { debug!("Parser::version_err"); let msg = self.cmd._render_version(use_long); - let mut c = Colorizer::new(Stream::Stdout, self.color_help()); + let mut c = Colorizer::new(Stream::Stdout, self.cmd.color_help()); c.none(msg); ClapError::display_version(self.cmd, c) } diff --git a/src/parse/validator.rs b/src/parse/validator.rs index 77895a57..f3f902ce 100644 --- a/src/parse/validator.rs +++ b/src/parse/validator.rs @@ -65,7 +65,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { .filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent)) .count(); if num_user_values == 0 { - let message = self.p.write_help_err(false, Stream::Stderr)?; + let message = self.p.cmd.write_help_err(false, Stream::Stderr)?; return Err(Error::display_help_error(self.p.cmd, message)); } } @@ -85,7 +85,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { )); } else if !has_subcmd && self.p.cmd.is_set(AppSettings::SubcommandRequiredElseHelp) { debug!("Validator::new::get_matches_with: SubcommandRequiredElseHelp=true"); - let message = self.p.write_help_err(false, Stream::Stderr)?; + let message = self.p.cmd.write_help_err(false, Stream::Stderr)?; return Err(Error::display_help_error(self.p.cmd, message)); } From 6229e78bc6f31956a3b88242f6031724352a2598 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 21 Apr 2022 16:24:10 -0500 Subject: [PATCH 7/8] refactor(parser): Parser is solely responsible for populating ArgMatches --- src/parse/parser.rs | 10 ++++++++-- src/parse/validator.rs | 6 ------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/parse/parser.rs b/src/parse/parser.rs index c6b83a06..4678fddd 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -448,7 +448,10 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { matches: sc_m.into_inner(), }); - return Validator::new(self).validate(parse_state, matcher, trailing_values); + #[cfg(feature = "env")] + self.add_env(matcher, trailing_values)?; + self.add_defaults(matcher, trailing_values); + return Validator::new(self).validate(parse_state, matcher); } else { // Start error processing return Err(self.match_arg_error(&arg_os, valid_arg_found, trailing_values)); @@ -465,7 +468,10 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { self.parse_subcommand(&sc_name, matcher, raw_args, args_cursor, keep_state)?; } - Validator::new(self).validate(parse_state, matcher, trailing_values) + #[cfg(feature = "env")] + self.add_env(matcher, trailing_values)?; + self.add_defaults(matcher, trailing_values); + Validator::new(self).validate(parse_state, matcher) } fn match_arg_error( diff --git a/src/parse/validator.rs b/src/parse/validator.rs index f3f902ce..ac07100e 100644 --- a/src/parse/validator.rs +++ b/src/parse/validator.rs @@ -23,17 +23,11 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { &mut self, parse_state: ParseState, matcher: &mut ArgMatcher, - trailing_values: bool, ) -> ClapResult<()> { debug!("Validator::validate"); let mut conflicts = Conflicts::new(); let has_subcmd = matcher.subcommand_name().is_some(); - #[cfg(feature = "env")] - self.p.add_env(matcher, trailing_values)?; - - self.p.add_defaults(matcher, trailing_values); - if let ParseState::Opt(a) = parse_state { debug!("Validator::validate: needs_val_of={:?}", a); From 9d523012fec81a54881c0a3e8d6ea9e7610c216d Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 21 Apr 2022 16:31:04 -0500 Subject: [PATCH 8/8] refactor(validator): Decouple parser --- src/parse/parser.rs | 4 +- src/parse/validator.rs | 135 ++++++++++++++++++++--------------------- 2 files changed, 67 insertions(+), 72 deletions(-) diff --git a/src/parse/parser.rs b/src/parse/parser.rs index 4678fddd..ee981ca0 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -451,7 +451,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { #[cfg(feature = "env")] self.add_env(matcher, trailing_values)?; self.add_defaults(matcher, trailing_values); - return Validator::new(self).validate(parse_state, matcher); + return Validator::new(self.cmd).validate(parse_state, matcher); } else { // Start error processing return Err(self.match_arg_error(&arg_os, valid_arg_found, trailing_values)); @@ -471,7 +471,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { #[cfg(feature = "env")] self.add_env(matcher, trailing_values)?; self.add_defaults(matcher, trailing_values); - Validator::new(self).validate(parse_state, matcher) + Validator::new(self.cmd).validate(parse_state, matcher) } fn match_arg_error( diff --git a/src/parse/validator.rs b/src/parse/validator.rs index ac07100e..e39a39a4 100644 --- a/src/parse/validator.rs +++ b/src/parse/validator.rs @@ -3,20 +3,20 @@ use crate::build::{AppSettings, Arg, ArgPredicate, Command, PossibleValue}; use crate::error::{Error, Result as ClapResult}; use crate::output::fmt::Stream; use crate::output::Usage; -use crate::parse::{ArgMatcher, MatchedArg, ParseState, Parser}; +use crate::parse::{ArgMatcher, MatchedArg, ParseState}; use crate::util::ChildGraph; use crate::util::Id; use crate::{INTERNAL_ERROR_MSG, INVALID_UTF8}; -pub(crate) struct Validator<'help, 'cmd, 'parser> { - p: &'parser mut Parser<'help, 'cmd>, +pub(crate) struct Validator<'help, 'cmd> { + cmd: &'cmd Command<'help>, required: ChildGraph, } -impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { - pub(crate) fn new(p: &'parser mut Parser<'help, 'cmd>) -> Self { - let required = p.cmd.required_graph(); - Validator { p, required } +impl<'help, 'cmd> Validator<'help, 'cmd> { + pub(crate) fn new(cmd: &'cmd Command<'help>) -> Self { + let required = cmd.required_graph(); + Validator { cmd, required } } pub(crate) fn validate( @@ -31,7 +31,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { if let ParseState::Opt(a) = parse_state { debug!("Validator::validate: needs_val_of={:?}", a); - let o = &self.p.cmd[&a]; + let o = &self.cmd[&a]; let should_err = if let Some(v) = matcher.args.get(&o.id) { v.all_val_groups_empty() && !(o.min_vals.is_some() && o.min_vals.unwrap() == 0) } else { @@ -39,52 +39,51 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { }; if should_err { return Err(Error::empty_value( - self.p.cmd, + self.cmd, &o.possible_vals .iter() .filter(|pv| !pv.is_hide_set()) .map(PossibleValue::get_name) .collect::>(), o, - Usage::new(self.p.cmd) + Usage::new(self.cmd) .required(&self.required) .create_usage_with_title(&[]), )); } } - if !has_subcmd && self.p.cmd.is_arg_required_else_help_set() { + if !has_subcmd && self.cmd.is_arg_required_else_help_set() { let num_user_values = matcher .arg_names() .filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent)) .count(); if num_user_values == 0 { - let message = self.p.cmd.write_help_err(false, Stream::Stderr)?; - return Err(Error::display_help_error(self.p.cmd, message)); + let message = self.cmd.write_help_err(false, Stream::Stderr)?; + return Err(Error::display_help_error(self.cmd, message)); } } #[allow(deprecated)] - if !has_subcmd && self.p.cmd.is_subcommand_required_set() { + if !has_subcmd && self.cmd.is_subcommand_required_set() { let bn = self - .p .cmd .get_bin_name() - .unwrap_or_else(|| self.p.cmd.get_name()); + .unwrap_or_else(|| self.cmd.get_name()); return Err(Error::missing_subcommand( - self.p.cmd, + self.cmd, bn.to_string(), - Usage::new(self.p.cmd) + Usage::new(self.cmd) .required(&self.required) .create_usage_with_title(&[]), )); - } else if !has_subcmd && self.p.cmd.is_set(AppSettings::SubcommandRequiredElseHelp) { + } else if !has_subcmd && self.cmd.is_set(AppSettings::SubcommandRequiredElseHelp) { debug!("Validator::new::get_matches_with: SubcommandRequiredElseHelp=true"); - let message = self.p.cmd.write_help_err(false, Stream::Stderr)?; - return Err(Error::display_help_error(self.p.cmd, message)); + let message = self.cmd.write_help_err(false, Stream::Stderr)?; + return Err(Error::display_help_error(self.cmd, message)); } self.validate_conflicts(matcher, &mut conflicts)?; - if !(self.p.cmd.is_subcommand_negates_reqs_set() && has_subcmd) { + if !(self.cmd.is_subcommand_negates_reqs_set() && has_subcmd) { self.validate_required(matcher, &mut conflicts)?; } self.validate_matched_args(matcher)?; @@ -106,8 +105,8 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { val ); return Err(Error::invalid_utf8( - self.p.cmd, - Usage::new(self.p.cmd) + self.cmd, + Usage::new(self.cmd) .required(&self.required) .create_usage_with_title(&[]), )); @@ -127,14 +126,14 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { .arg_names() .filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent)) .filter(|&n| { - self.p.cmd.find(n).map_or(true, |a| { + self.cmd.find(n).map_or(true, |a| { !(a.is_hide_set() || self.required.contains(&a.id)) }) }) .cloned() .collect(); return Err(Error::invalid_value( - self.p.cmd, + self.cmd, val_str.into_owned(), &arg.possible_vals .iter() @@ -142,7 +141,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { .map(PossibleValue::get_name) .collect::>(), arg, - Usage::new(self.p.cmd) + Usage::new(self.cmd) .required(&self.required) .create_usage_with_title(&used), )); @@ -151,14 +150,14 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { if arg.is_forbid_empty_values_set() && val.is_empty() && matcher.contains(&arg.id) { debug!("Validator::validate_arg_values: illegal empty val found"); return Err(Error::empty_value( - self.p.cmd, + self.cmd, &arg.possible_vals .iter() .filter(|pv| !pv.is_hide_set()) .map(PossibleValue::get_name) .collect::>(), arg, - Usage::new(self.p.cmd) + Usage::new(self.cmd) .required(&self.required) .create_usage_with_title(&[]), )); @@ -174,7 +173,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { val.to_string_lossy().into_owned(), e, ) - .with_cmd(self.p.cmd)); + .with_cmd(self.cmd)); } else { debug!("good"); } @@ -189,7 +188,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { val.to_string_lossy().into(), e, ) - .with_cmd(self.p.cmd)); + .with_cmd(self.cmd)); } else { debug!("good"); } @@ -210,10 +209,10 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { for arg_id in matcher .arg_names() .filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent)) - .filter(|arg_id| self.p.cmd.find(arg_id).is_some()) + .filter(|arg_id| self.cmd.find(arg_id).is_some()) { debug!("Validator::validate_conflicts::iter: id={:?}", arg_id); - let conflicts = conflicts.gather_conflicts(self.p.cmd, matcher, arg_id); + let conflicts = conflicts.gather_conflicts(self.cmd, matcher, arg_id); self.build_conflict_err(arg_id, &conflicts, matcher)?; } @@ -228,8 +227,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { .arg_names() .filter_map(|name| { debug!("Validator::validate_exclusive:iter:{:?}", name); - self.p - .cmd + self.cmd .find(name) // Find `arg`s which are exclusive but also appear with other args. .filter(|&arg| arg.is_exclusive_set() && args_count > 1) @@ -237,10 +235,10 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { // Throw an error for the first conflict found. .try_for_each(|arg| { Err(Error::argument_conflict( - self.p.cmd, + self.cmd, arg, Vec::new(), - Usage::new(self.p.cmd) + Usage::new(self.cmd) .required(&self.required) .create_usage_with_title(&[]), )) @@ -262,24 +260,24 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { let conflicts = conflict_ids .iter() .flat_map(|c_id| { - if self.p.cmd.find_group(c_id).is_some() { - self.p.cmd.unroll_args_in_group(c_id) + if self.cmd.find_group(c_id).is_some() { + self.cmd.unroll_args_in_group(c_id) } else { vec![c_id.clone()] } }) .filter_map(|c_id| { seen.insert(c_id.clone()).then(|| { - let c_arg = self.p.cmd.find(&c_id).expect(INTERNAL_ERROR_MSG); + let c_arg = self.cmd.find(&c_id).expect(INTERNAL_ERROR_MSG); c_arg.to_string() }) }) .collect(); - let former_arg = self.p.cmd.find(name).expect(INTERNAL_ERROR_MSG); + let former_arg = self.cmd.find(name).expect(INTERNAL_ERROR_MSG); let usg = self.build_conflict_err_usage(matcher, conflict_ids); Err(Error::argument_conflict( - self.p.cmd, former_arg, conflicts, usg, + self.cmd, former_arg, conflicts, usg, )) } @@ -292,13 +290,13 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { .collect(); let required: Vec = used_filtered .iter() - .filter_map(|key| self.p.cmd.find(key)) + .filter_map(|key| self.cmd.find(key)) .flat_map(|arg| arg.requires.iter().map(|item| &item.1)) .filter(|key| !used_filtered.contains(key) && !conflicting_keys.contains(key)) .chain(used_filtered.iter()) .cloned() .collect(); - Usage::new(self.p.cmd) + Usage::new(self.cmd) .required(&self.required) .create_usage_with_title(&required) } @@ -310,16 +308,16 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { .filter(|arg_id| matcher.check_explicit(arg_id, ArgPredicate::IsPresent)) { debug!("Validator::gather_requires:iter:{:?}", name); - if let Some(arg) = self.p.cmd.find(name) { + if let Some(arg) = self.cmd.find(name) { let is_relevant = |(val, req_arg): &(ArgPredicate<'_>, Id)| -> Option { let required = matcher.check_explicit(&arg.id, *val); required.then(|| req_arg.clone()) }; - for req in self.p.cmd.unroll_arg_requires(is_relevant, &arg.id) { + for req in self.cmd.unroll_arg_requires(is_relevant, &arg.id) { self.required.insert(req); } - } else if let Some(g) = self.p.cmd.find_group(name) { + } else if let Some(g) = self.cmd.find_group(name) { debug!("Validator::gather_requires:iter:{:?}:group", name); for r in &g.requires { self.required.insert(r.clone()); @@ -336,7 +334,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { name, ma.vals_flatten() ); - if let Some(arg) = self.p.cmd.find(name) { + if let Some(arg) = self.cmd.find(name) { self.validate_arg_num_vals(arg, ma)?; self.validate_arg_values(arg, ma, matcher)?; self.validate_arg_num_occurs(arg, ma)?; @@ -356,9 +354,9 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { if ma.get_occurrences() > 1 && !a.is_multiple_occurrences_set() && !a.is_positional() { // Not the first time, and we don't allow multiples return Err(Error::unexpected_multiple_usage( - self.p.cmd, + self.cmd, a, - Usage::new(self.p.cmd) + Usage::new(self.cmd) .required(&self.required) .create_usage_with_title(&[]), )); @@ -371,11 +369,11 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { let occurs = ma.get_occurrences() as usize; if occurs > max_occurs { return Err(Error::too_many_occurrences( - self.p.cmd, + self.cmd, a, max_occurs, occurs, - Usage::new(self.p.cmd) + Usage::new(self.cmd) .required(&self.required) .create_usage_with_title(&[]), )); @@ -398,7 +396,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { if should_err { debug!("Validator::validate_arg_num_vals: Sending error WrongNumberOfValues"); return Err(Error::wrong_number_of_values( - self.p.cmd, + self.cmd, a, num, if a.is_multiple_occurrences_set() { @@ -406,7 +404,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { } else { total_num }, - Usage::new(self.p.cmd) + Usage::new(self.cmd) .required(&self.required) .create_usage_with_title(&[]), )); @@ -417,7 +415,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { if ma.num_vals() > num { debug!("Validator::validate_arg_num_vals: Sending error TooManyValues"); return Err(Error::too_many_values( - self.p.cmd, + self.cmd, ma.vals_flatten() .last() .expect(INTERNAL_ERROR_MSG) @@ -425,7 +423,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { .expect(INVALID_UTF8) .to_string(), a.to_string(), - Usage::new(self.p.cmd) + Usage::new(self.cmd) .required(&self.required) .create_usage_with_title(&[]), )); @@ -436,11 +434,11 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { if ma.num_vals() < num && num != 0 { debug!("Validator::validate_arg_num_vals: Sending error TooFewValues"); return Err(Error::too_few_values( - self.p.cmd, + self.cmd, a, num, ma.num_vals(), - Usage::new(self.p.cmd) + Usage::new(self.cmd) .required(&self.required) .create_usage_with_title(&[]), )); @@ -453,14 +451,14 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { // Issue 1105 (https://github.com/clap-rs/clap/issues/1105) if a.is_takes_value_set() && !min_vals_zero && ma.all_val_groups_empty() { return Err(Error::empty_value( - self.p.cmd, + self.cmd, &a.possible_vals .iter() .filter(|pv| !pv.is_hide_set()) .map(PossibleValue::get_name) .collect::>(), a, - Usage::new(self.p.cmd) + Usage::new(self.cmd) .required(&self.required) .create_usage_with_title(&[]), )); @@ -478,15 +476,14 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { 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.cmd.find(arg_or_group) { + if let Some(arg) = self.cmd.find(arg_or_group) { debug!("Validator::validate_required:iter: This is an arg"); if !self.is_missing_required_ok(arg, matcher, conflicts) { return self.missing_required_error(matcher, vec![]); } - } else if let Some(group) = self.p.cmd.find_group(arg_or_group) { + } else if let Some(group) = self.cmd.find_group(arg_or_group) { debug!("Validator::validate_required:iter: This is a group"); if !self - .p .cmd .unroll_args_in_group(&group.id) .iter() @@ -498,7 +495,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { } // Validate the conditionally required args - for a in self.p.cmd.get_arguments() { + for a in self.cmd.get_arguments() { for (other, val) in &a.r_ifs { if matcher.check_explicit(other, ArgPredicate::Equals(std::ffi::OsStr::new(*val))) && !matcher.contains(&a.id) @@ -527,14 +524,13 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { conflicts: &mut Conflicts, ) -> bool { debug!("Validator::is_missing_required_ok: {}", a.name); - let conflicts = conflicts.gather_conflicts(self.p.cmd, matcher, &a.id); + let conflicts = conflicts.gather_conflicts(self.cmd, matcher, &a.id); !conflicts.is_empty() } fn validate_required_unless(&self, matcher: &ArgMatcher) -> ClapResult<()> { debug!("Validator::validate_required_unless"); let failed_args: Vec<_> = self - .p .cmd .get_arguments() .filter(|&a| { @@ -568,7 +564,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { self.required ); - let usg = Usage::new(self.p.cmd).required(&self.required); + let usg = Usage::new(self.cmd).required(&self.required); let req_args = usg.get_required_usage_from(&incl, Some(matcher), true); @@ -582,8 +578,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, '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 - .cmd + self.cmd .find(n) .map_or(true, |a| !a.is_hide_set() && !self.required.contains(&a.id)) }) @@ -592,7 +587,7 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { .collect(); Err(Error::missing_required_argument( - self.p.cmd, + self.cmd, req_args, usg.create_usage_with_title(&used), ))