From 77542a113843d4e0964442cf5a3840d18e846f6f Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 14 Feb 2022 11:23:04 -0600 Subject: [PATCH] refactor: Reduce visibility on App members The long term goals are - Easier refactoring - Identify needs for reflection API Shorter term, if I want to rename `App` to `Command` and deprecate `App`, it will mark all member access as deprecated. This works around that. I gave up in exploring abstractions when it came to `MKeyMap` access. This can be refined in the future. --- clap_mangen/src/render.rs | 2 +- src/build/app.rs | 138 ++++++++++++++++++++++-------- src/build/app_tests.rs | 16 ++-- src/build/debug_asserts.rs | 50 ++++++----- src/error/mod.rs | 6 +- src/output/help.rs | 89 +++++++++++-------- src/output/usage.rs | 44 ++++------ src/parse/arg_matcher.rs | 6 +- src/parse/features/suggestions.rs | 9 +- src/parse/parser.rs | 101 ++++++++++++---------- src/parse/validator.rs | 11 ++- 11 files changed, 283 insertions(+), 189 deletions(-) diff --git a/clap_mangen/src/render.rs b/clap_mangen/src/render.rs index 1515450e..b6f7a70e 100644 --- a/clap_mangen/src/render.rs +++ b/clap_mangen/src/render.rs @@ -2,7 +2,7 @@ use clap::AppSettings; use roff::{bold, italic, roman, Inline, Roff}; pub(crate) fn subcommand_heading(app: &clap::App) -> String { - match app.get_subommand_help_heading() { + match app.get_subcommand_help_heading() { Some(title) => title.to_string(), None => "SUBCOMMANDS".to_string(), } diff --git a/src/build/app.rs b/src/build/app.rs index 0c44a997..5f435ec4 100644 --- a/src/build/app.rs +++ b/src/build/app.rs @@ -3,6 +3,7 @@ // Std use std::collections::HashMap; use std::env; +use std::ffi::OsStr; use std::ffi::OsString; use std::fmt; use std::io; @@ -62,40 +63,40 @@ use crate::build::debug_asserts::assert_app; /// [`App::get_matches`]: App::get_matches() #[derive(Debug, Clone, PartialEq, Eq)] pub struct App<'help> { - pub(crate) id: Id, - pub(crate) name: String, - pub(crate) long_flag: Option<&'help str>, - pub(crate) short_flag: Option, - pub(crate) bin_name: Option, - pub(crate) author: Option<&'help str>, - pub(crate) version: Option<&'help str>, - pub(crate) long_version: Option<&'help str>, - pub(crate) about: Option<&'help str>, - pub(crate) long_about: Option<&'help str>, - pub(crate) before_help: Option<&'help str>, - pub(crate) before_long_help: Option<&'help str>, - pub(crate) after_help: Option<&'help str>, - pub(crate) after_long_help: Option<&'help str>, - pub(crate) aliases: Vec<(&'help str, bool)>, // (name, visible) - pub(crate) short_flag_aliases: Vec<(char, bool)>, // (name, visible) - pub(crate) long_flag_aliases: Vec<(&'help str, bool)>, // (name, visible) - pub(crate) usage_str: Option<&'help str>, - pub(crate) usage_name: Option, - pub(crate) help_str: Option<&'help str>, - pub(crate) disp_ord: Option, - pub(crate) term_w: Option, - pub(crate) max_w: Option, - pub(crate) template: Option<&'help str>, - pub(crate) settings: AppFlags, - pub(crate) g_settings: AppFlags, - pub(crate) args: MKeyMap<'help>, - pub(crate) subcommands: Vec>, - pub(crate) replacers: HashMap<&'help str, &'help [&'help str]>, - pub(crate) groups: Vec>, - pub(crate) current_help_heading: Option<&'help str>, - pub(crate) current_disp_ord: Option, - pub(crate) subcommand_value_name: Option<&'help str>, - pub(crate) subcommand_heading: Option<&'help str>, + id: Id, + name: String, + long_flag: Option<&'help str>, + short_flag: Option, + bin_name: Option, + author: Option<&'help str>, + version: Option<&'help str>, + long_version: Option<&'help str>, + about: Option<&'help str>, + long_about: Option<&'help str>, + before_help: Option<&'help str>, + before_long_help: Option<&'help str>, + after_help: Option<&'help str>, + after_long_help: Option<&'help str>, + aliases: Vec<(&'help str, bool)>, // (name, visible) + short_flag_aliases: Vec<(char, bool)>, // (name, visible) + long_flag_aliases: Vec<(&'help str, bool)>, // (name, visible) + usage_str: Option<&'help str>, + usage_name: Option, + help_str: Option<&'help str>, + disp_ord: Option, + term_w: Option, + max_w: Option, + template: Option<&'help str>, + settings: AppFlags, + g_settings: AppFlags, + args: MKeyMap<'help>, + subcommands: Vec>, + replacers: HashMap<&'help OsStr, &'help [&'help str]>, + groups: Vec>, + current_help_heading: Option<&'help str>, + current_disp_ord: Option, + subcommand_value_name: Option<&'help str>, + subcommand_heading: Option<&'help str>, } /// Basic API @@ -1933,7 +1934,7 @@ impl<'help> App<'help> { #[cfg(feature = "unstable-replace")] #[must_use] pub fn replace(mut self, name: &'help str, target: &'help [&'help str]) -> Self { - self.replacers.insert(name, target); + self.replacers.insert(OsStr::new(name), target); self } @@ -3151,6 +3152,11 @@ impl<'help> App<'help> { /// Reflection impl<'help> App<'help> { + #[inline] + pub(crate) fn get_usage_name(&self) -> Option<&str> { + self.usage_name.as_deref() + } + /// Get the name of the binary. #[inline] pub fn get_bin_name(&self) -> Option<&str> { @@ -3324,16 +3330,38 @@ impl<'help> App<'help> { /// Returns the help heading for listing subcommands. #[inline] - pub fn get_subommand_help_heading(&self) -> Option<&str> { + pub fn get_subcommand_help_heading(&self) -> Option<&str> { self.subcommand_heading } + /// Deprecated, replaced with [`App::get_subcommand_help_heading`] + #[inline] + #[deprecated( + since = "3.1.0", + note = "Replaced with `App::get_subcommand_help_heading`" + )] + pub fn get_subommand_help_heading(&self) -> Option<&str> { + self.get_subcommand_help_heading() + } + /// Returns the subcommand value name. #[inline] pub fn get_subcommand_value_name(&self) -> Option<&str> { self.subcommand_value_name } + /// Returns the help heading for listing subcommands. + #[inline] + pub fn get_before_help(&self) -> Option<&str> { + self.before_help + } + + /// Returns the help heading for listing subcommands. + #[inline] + pub fn get_before_long_help(&self) -> Option<&str> { + self.before_long_help + } + /// Returns the help heading for listing subcommands. #[inline] pub fn get_after_help(&self) -> Option<&str> { @@ -3369,6 +3397,12 @@ impl<'help> App<'help> { self.get_subcommands_mut().find(|s| s.aliases_to(name)) } + /// Iterate through the set of groups. + #[inline] + pub fn get_groups(&self) -> impl Iterator> { + self.groups.iter() + } + /// Iterate through the set of arguments. #[inline] pub fn get_arguments(&self) -> impl Iterator> { @@ -3867,6 +3901,38 @@ impl<'help> App<'help> { // Internally used only impl<'help> App<'help> { + pub(crate) fn get_id(&self) -> Id { + self.id.clone() + } + + pub(crate) fn get_override_usage(&self) -> Option<&str> { + self.usage_str + } + + pub(crate) fn get_override_help(&self) -> Option<&str> { + self.help_str + } + + pub(crate) fn get_help_template(&self) -> Option<&str> { + self.template + } + + pub(crate) fn get_term_width(&self) -> Option { + self.term_w + } + + pub(crate) fn get_max_term_width(&self) -> Option { + self.max_w + } + + pub(crate) fn get_replacement(&self, key: &OsStr) -> Option<&[&str]> { + self.replacers.get(key).copied() + } + + pub(crate) fn get_keymap(&self) -> &MKeyMap<'help> { + &self.args + } + fn get_used_global_args(&self, matches: &ArgMatches, global_arg_vec: &mut Vec) { global_arg_vec.extend( self.args diff --git a/src/build/app_tests.rs b/src/build/app_tests.rs index 5349d9f7..8317c8aa 100644 --- a/src/build/app_tests.rs +++ b/src/build/app_tests.rs @@ -7,7 +7,10 @@ fn propagate_version() { .version("1.1") .subcommand(App::new("sub1")); app._propagate(); - assert_eq!(app.subcommands[0].version, Some("1.1")); + assert_eq!( + app.get_subcommands().next().unwrap().get_version(), + Some("1.1") + ); } #[test] @@ -17,9 +20,8 @@ fn global_setting() { .subcommand(App::new("subcmd")); app._propagate(); assert!(app - .subcommands - .iter() - .find(|s| s.name == "subcmd") + .get_subcommands() + .find(|s| s.get_name() == "subcmd") .unwrap() .is_disable_version_flag_set()); } @@ -38,5 +40,9 @@ fn issue_2090() { .subcommand(App::new("sub")); app._build(); - assert!(app.subcommands[0].is_disable_version_flag_set()); + assert!(app + .get_subcommands() + .next() + .unwrap() + .is_disable_version_flag_set()); } diff --git a/src/build/debug_asserts.rs b/src/build/debug_asserts.rs index d4f51f2f..39ded828 100644 --- a/src/build/debug_asserts.rs +++ b/src/build/debug_asserts.rs @@ -12,7 +12,7 @@ pub(crate) fn assert_app(app: &App) { let mut long_flags = vec![]; // Invalid version flag settings - if app.version.is_none() && app.long_version.is_none() { + if app.get_version().is_none() && app.get_long_version().is_none() { // PropagateVersion is meaningless if there is no version assert!( !app.is_propagate_version_set(), @@ -22,37 +22,36 @@ pub(crate) fn assert_app(app: &App) { // Used `App::mut_arg("version", ..) but did not provide any version information to display let has_mutated_version = app - .args - .args() + .get_arguments() .any(|x| x.id == Id::version_hash() && x.provider == ArgProvider::GeneratedMutated); if has_mutated_version { - assert!(app.settings.is_set(AppSettings::NoAutoVersion), + assert!(app.is_set(AppSettings::NoAutoVersion), "App {}: Used App::mut_arg(\"version\", ..) without providing App::version, App::long_version or using AppSettings::NoAutoVersion" ,app.get_name() ); } } - for sc in &app.subcommands { - if let Some(s) = sc.short_flag.as_ref() { - short_flags.push(Flag::App(format!("-{}", s), &sc.name)); + for sc in app.get_subcommands() { + if let Some(s) = sc.get_short_flag().as_ref() { + short_flags.push(Flag::App(format!("-{}", s), sc.get_name())); } - for (short_alias, _) in &sc.short_flag_aliases { - short_flags.push(Flag::App(format!("-{}", short_alias), &sc.name)); + for short_alias in sc.get_all_short_flag_aliases() { + short_flags.push(Flag::App(format!("-{}", short_alias), sc.get_name())); } - if let Some(l) = sc.long_flag.as_ref() { - long_flags.push(Flag::App(format!("--{}", l), &sc.name)); + if let Some(l) = sc.get_long_flag().as_ref() { + long_flags.push(Flag::App(format!("--{}", l), sc.get_name())); } - for (long_alias, _) in &sc.long_flag_aliases { - long_flags.push(Flag::App(format!("--{}", long_alias), &sc.name)); + for long_alias in sc.get_all_long_flag_aliases() { + long_flags.push(Flag::App(format!("--{}", long_alias), sc.get_name())); } } - for arg in app.args.args() { + for arg in app.get_arguments() { assert_arg(arg); if let Some(s) = arg.short.as_ref() { @@ -223,10 +222,10 @@ pub(crate) fn assert_app(app: &App) { } } - for group in &app.groups { + for group in app.get_groups() { // Name conflicts assert!( - app.groups.iter().filter(|x| x.id == group.id).count() < 2, + app.get_groups().filter(|x| x.id == group.id).count() < 2, "App {}: Argument group name must be unique\n\n\t'{}' is already in use", app.get_name(), group.name, @@ -234,7 +233,7 @@ pub(crate) fn assert_app(app: &App) { // Groups should not have naming conflicts with Args assert!( - !app.args.args().any(|x| x.id == group.id), + !app.get_arguments().any(|x| x.id == group.id), "App {}: Argument group name '{}' must not conflict with argument name", app.get_name(), group.name, @@ -244,8 +243,7 @@ pub(crate) fn assert_app(app: &App) { if group.required && !group.args.is_empty() { assert!( group.args.iter().any(|arg| { - app.args - .args() + app.get_arguments() .any(|x| x.id == *arg && x.default_vals.is_empty()) }), "App {}: Argument group '{}' is required but all of it's arguments have a default value.", @@ -257,7 +255,7 @@ pub(crate) fn assert_app(app: &App) { for arg in &group.args { // Args listed inside groups should exist assert!( - app.args.args().any(|x| x.id == *arg), + app.get_arguments().any(|x| x.id == *arg), "App {}: Argument group '{}' contains non-existent argument '{:?}'", app.get_name(), group.name, @@ -276,7 +274,7 @@ pub(crate) fn assert_app(app: &App) { _verify_positionals(app); - if let Some(help_template) = app.template { + if let Some(help_template) = app.get_help_template() { assert!( !help_template.contains("{flags}"), "App {}: {}", @@ -422,7 +420,7 @@ fn _verify_positionals(app: &App) -> bool { // but no 2) let highest_idx = app - .args + .get_keymap() .keys() .filter_map(|x| { if let KeyType::Position(n) = x { @@ -434,7 +432,7 @@ fn _verify_positionals(app: &App) -> bool { .max() .unwrap_or(0); - let num_p = app.args.keys().filter(|x| x.is_position()).count(); + let num_p = app.get_keymap().keys().filter(|x| x.is_position()).count(); assert!( highest_idx == num_p, @@ -455,8 +453,8 @@ fn _verify_positionals(app: &App) -> bool { // We can't pass the closure (it.next()) to the macro directly because each call to // find() (iterator, not macro) gets called repeatedly. - let last = &app.args[&KeyType::Position(highest_idx)]; - let second_to_last = &app.args[&KeyType::Position(highest_idx - 1)]; + let last = &app.get_keymap()[&KeyType::Position(highest_idx)]; + let second_to_last = &app.get_keymap()[&KeyType::Position(highest_idx - 1)]; // Either the final positional is required // Or the second to last has a terminator or .last(true) set @@ -535,7 +533,7 @@ fn _verify_positionals(app: &App) -> bool { } else { // Check that if a required positional argument is found, all positions with a lower // index are also required - for p in (1..=num_p).rev().filter_map(|n| app.args.get(&n)) { + for p in (1..=num_p).rev().filter_map(|n| app.get_keymap().get(&n)) { if found { assert!( p.is_required_set(), diff --git a/src/error/mod.rs b/src/error/mod.rs index ee07a2a8..75aaa513 100644 --- a/src/error/mod.rs +++ b/src/error/mod.rs @@ -182,7 +182,7 @@ impl Error { } pub(crate) fn with_app(self, app: &App) -> Self { - self.set_wait_on_exit(app.settings.is_set(AppSettings::WaitOnError)) + self.set_wait_on_exit(app.is_set(AppSettings::WaitOnError)) .set_color(app.get_color()) .set_help_flag(get_help_flag(app)) } @@ -1047,9 +1047,9 @@ fn put_usage(c: &mut Colorizer, usage: impl Into) { } fn get_help_flag(app: &App) -> Option<&'static str> { - if !app.settings.is_set(AppSettings::DisableHelpFlag) { + if !app.is_disable_help_flag_set() { Some("--help") - } else if app.has_subcommands() && !app.settings.is_set(AppSettings::DisableHelpSubcommand) { + } else if app.has_subcommands() && !app.is_disable_help_subcommand_set() { Some("help") } else { None diff --git a/src/output/help.rs b/src/output/help.rs index 16439a43..0b5bed56 100644 --- a/src/output/help.rs +++ b/src/output/help.rs @@ -53,12 +53,12 @@ impl<'help, 'app, 'writer> Help<'help, 'app, 'writer> { use_long: bool, ) -> Self { debug!("Help::new"); - let term_w = match app.term_w { + let term_w = match app.get_term_width() { Some(0) => usize::MAX, Some(w) => w, None => cmp::min( dimensions().map_or(100, |(w, _)| w), - match app.max_w { + match app.get_max_term_width() { None | Some(0) => usize::MAX, Some(mw) => mw, }, @@ -80,9 +80,9 @@ impl<'help, 'app, 'writer> Help<'help, 'app, 'writer> { pub(crate) fn write_help(&mut self) -> io::Result<()> { debug!("Help::write_help"); - if let Some(h) = self.app.help_str { + if let Some(h) = self.app.get_override_help() { self.none(h)?; - } else if let Some(tmpl) = self.app.template { + } else if let Some(tmpl) = self.app.get_help_template() { self.write_templated_help(tmpl)?; } else { let pos = self @@ -353,9 +353,11 @@ impl<'help, 'app, 'writer> Help<'help, 'app, 'writer> { fn write_before_help(&mut self) -> io::Result<()> { debug!("Help::write_before_help"); let before_help = if self.use_long { - self.app.before_long_help.or(self.app.before_help) + self.app + .get_before_long_help() + .or_else(|| self.app.get_before_help()) } else { - self.app.before_help + self.app.get_before_help() }; if let Some(output) = before_help { self.none(text_wrapper(&output.replace("{n}", "\n"), self.term_w))?; @@ -367,9 +369,11 @@ impl<'help, 'app, 'writer> Help<'help, 'app, 'writer> { fn write_after_help(&mut self) -> io::Result<()> { debug!("Help::write_after_help"); let after_help = if self.use_long { - self.app.after_long_help.or(self.app.after_help) + self.app + .get_after_long_help() + .or_else(|| self.app.get_after_help()) } else { - self.app.after_help + self.app.get_after_help() }; if let Some(output) = after_help { self.none("\n\n")?; @@ -606,9 +610,9 @@ impl<'help, 'app, 'writer> Help<'help, 'app, 'writer> { fn write_about(&mut self, before_new_line: bool, after_new_line: bool) -> io::Result<()> { let about = if self.use_long { - self.app.long_about.or(self.app.about) + self.app.get_long_about().or_else(|| self.app.get_about()) } else { - self.app.about + self.app.get_about() }; if let Some(output) = about { if before_new_line { @@ -623,7 +627,7 @@ impl<'help, 'app, 'writer> Help<'help, 'app, 'writer> { } fn write_author(&mut self, before_new_line: bool, after_new_line: bool) -> io::Result<()> { - if let Some(author) = self.app.author { + if let Some(author) = self.app.get_author() { if before_new_line { self.none("\n")?; } @@ -636,7 +640,10 @@ impl<'help, 'app, 'writer> Help<'help, 'app, 'writer> { } fn write_version(&mut self) -> io::Result<()> { - let version = self.app.version.or(self.app.long_version); + let version = self + .app + .get_version() + .or_else(|| self.app.get_long_version()); if let Some(output) = version { self.none(text_wrapper(output, self.term_w))?; } @@ -657,20 +664,26 @@ impl<'help, 'app, 'writer> Help<'help, 'app, 'writer> { let spec_vals = &self.sc_spec_vals(app); - let about = app.about.or(app.long_about).unwrap_or(""); + let about = app + .get_about() + .or_else(|| app.get_long_about()) + .unwrap_or(""); self.subcmd(sc_str, next_line_help, longest)?; self.help(false, about, spec_vals, next_line_help, longest) } fn sc_spec_vals(&self, a: &App) -> String { - debug!("Help::sc_spec_vals: a={}", a.name); + debug!("Help::sc_spec_vals: a={}", a.get_name()); let mut spec_vals = vec![]; - if !a.aliases.is_empty() || !a.short_flag_aliases.is_empty() { - debug!("Help::spec_vals: Found aliases...{:?}", a.aliases); + if 0 < a.get_all_aliases().count() || 0 < a.get_all_short_flag_aliases().count() { + debug!( + "Help::spec_vals: Found aliases...{:?}", + a.get_all_aliases().collect::>() + ); debug!( "Help::spec_vals: Found short flag aliases...{:?}", - a.short_flag_aliases + a.get_all_short_flag_aliases().collect::>() ); let mut short_als = a @@ -697,7 +710,7 @@ impl<'help, 'app, 'writer> Help<'help, 'app, 'writer> { true } else { // force_next_line - let h = app.about.unwrap_or(""); + let h = app.get_about().unwrap_or(""); let h_w = display_width(h) + display_width(spec_vals); let taken = longest + 12; self.term_w >= taken @@ -738,8 +751,7 @@ impl<'help, 'app, 'writer> Help<'help, 'app, 'writer> { let custom_headings = self .app - .args - .args() + .get_arguments() .filter_map(|arg| arg.get_help_heading()) .collect::>(); @@ -764,8 +776,7 @@ impl<'help, 'app, 'writer> Help<'help, 'app, 'writer> { for heading in custom_headings { let args = self .app - .args - .args() + .get_arguments() .filter(|a| { if let Some(help_heading) = a.get_help_heading() { return help_heading == heading; @@ -791,7 +802,11 @@ impl<'help, 'app, 'writer> Help<'help, 'app, 'writer> { self.none("\n\n")?; } - self.warning(self.app.subcommand_heading.unwrap_or("SUBCOMMANDS"))?; + self.warning( + self.app + .get_subcommand_help_heading() + .unwrap_or("SUBCOMMANDS"), + )?; self.warning(":\n")?; self.write_subcommands(self.app)?; @@ -801,9 +816,16 @@ impl<'help, 'app, 'writer> Help<'help, 'app, 'writer> { } /// Will use next line help on writing subcommands. - fn will_subcommands_wrap(&self, subcommands: &[App<'help>], longest: usize) -> bool { + fn will_subcommands_wrap<'a>( + &self, + subcommands: impl IntoIterator>, + longest: usize, + ) -> bool + where + 'help: 'a, + { subcommands - .iter() + .into_iter() .filter(|&subcommand| should_show_subcommand(subcommand)) .any(|subcommand| { let spec_vals = &self.sc_spec_vals(subcommand); @@ -818,18 +840,17 @@ impl<'help, 'app, 'writer> Help<'help, 'app, 'writer> { let mut longest = 2; let mut ord_v = Vec::new(); for subcommand in app - .subcommands - .iter() + .get_subcommands() .filter(|subcommand| should_show_subcommand(subcommand)) { let mut sc_str = String::new(); - if let Some(short) = subcommand.short_flag { + if let Some(short) = subcommand.get_short_flag() { write!(sc_str, "-{}", short).unwrap(); } - if let Some(long) = subcommand.long_flag { + if let Some(long) = subcommand.get_long_flag() { write!(sc_str, "--{}", long).unwrap(); } - sc_str.push_str(&subcommand.name); + sc_str.push_str(subcommand.get_name()); longest = longest.max(display_width(&sc_str)); ord_v.push((subcommand.get_display_order(), sc_str, subcommand)); } @@ -837,7 +858,7 @@ impl<'help, 'app, 'writer> Help<'help, 'app, 'writer> { debug!("Help::write_subcommands longest = {}", longest); - let next_line_help = self.will_subcommands_wrap(&app.subcommands, longest); + let next_line_help = self.will_subcommands_wrap(app.get_subcommands(), longest); let mut first = true; for (_, sc_str, sc) in &ord_v { @@ -855,15 +876,15 @@ impl<'help, 'app, 'writer> Help<'help, 'app, 'writer> { fn write_bin_name(&mut self) -> io::Result<()> { debug!("Help::write_bin_name"); - let bin_name = if let Some(bn) = self.app.bin_name.as_ref() { + let bin_name = if let Some(bn) = self.app.get_bin_name() { if bn.contains(' ') { // In case we're dealing with subcommands i.e. git mv is translated to git-mv bn.replace(' ', "-") } else { - text_wrapper(&self.app.name.replace("{n}", "\n"), self.term_w) + text_wrapper(&self.app.get_name().replace("{n}", "\n"), self.term_w) } } else { - text_wrapper(&self.app.name.replace("{n}", "\n"), self.term_w) + text_wrapper(&self.app.get_name().replace("{n}", "\n"), self.term_w) }; self.good(&bin_name)?; Ok(()) diff --git a/src/output/usage.rs b/src/output/usage.rs index 3da9a5c4..ecbf85b6 100644 --- a/src/output/usage.rs +++ b/src/output/usage.rs @@ -39,7 +39,7 @@ impl<'help, 'app> Usage<'help, 'app> { // Creates a usage string (*without title*) if one was not provided by the user manually. pub(crate) fn create_usage_no_title(&self, used: &[Id]) -> String { debug!("Usage::create_usage_no_title"); - if let Some(u) = self.app.usage_str { + if let Some(u) = self.app.get_override_usage() { String::from(&*u) } else if used.is_empty() { self.create_help_usage(true) @@ -54,11 +54,10 @@ impl<'help, 'app> Usage<'help, 'app> { let mut usage = String::with_capacity(75); let name = self .app - .usage_name - .as_ref() - .or_else(|| self.app.bin_name.as_ref()) - .unwrap_or(&self.app.name); - usage.push_str(&*name); + .get_usage_name() + .or_else(|| self.app.get_bin_name()) + .unwrap_or_else(|| self.app.get_name()); + usage.push_str(name); let req_string = if incl_reqs { self.get_required_usage_from(&[], None, false) .iter() @@ -129,7 +128,7 @@ impl<'help, 'app> Usage<'help, 'app> { if self.app.has_visible_subcommands() && incl_reqs || self.app.is_allow_external_subcommands_set() { - let placeholder = self.app.subcommand_value_name.unwrap_or("SUBCOMMAND"); + let placeholder = self.app.get_subcommand_value_name().unwrap_or("SUBCOMMAND"); #[allow(deprecated)] if self.app.is_subcommand_negates_reqs_set() || self.app.is_args_conflicts_with_subcommands_set() @@ -172,17 +171,15 @@ impl<'help, 'app> Usage<'help, 'app> { .fold(String::new(), |acc, s| acc + " " + s); usage.push_str( - &self - .app - .usage_name - .as_ref() - .or_else(|| self.app.bin_name.as_ref()) - .unwrap_or(&self.app.name)[..], + self.app + .get_usage_name() + .or_else(|| self.app.get_bin_name()) + .unwrap_or_else(|| self.app.get_name()), ); usage.push_str(&*r_string); if self.app.is_subcommand_required_set() { usage.push_str(" <"); - usage.push_str(self.app.subcommand_value_name.unwrap_or("SUBCOMMAND")); + usage.push_str(self.app.get_subcommand_value_name().unwrap_or("SUBCOMMAND")); usage.push('>'); } usage.shrink_to_fit(); @@ -204,10 +201,7 @@ impl<'help, 'app> Usage<'help, 'app> { let required = self.app.groups_for_arg(&pos.id).any(|grp_s| { debug!("Usage::get_args_tag:iter:{:?}:iter:{:?}", pos.name, grp_s); // if it's part of a required group we don't want to count it - self.app - .groups - .iter() - .any(|g| g.required && (g.id == grp_s)) + self.app.get_groups().any(|g| g.required && (g.id == grp_s)) }); if !required { count += 1; @@ -234,10 +228,7 @@ impl<'help, 'app> Usage<'help, 'app> { && !self.app.groups_for_arg(&pos.id).any(|grp_s| { debug!("Usage::get_args_tag:iter:{:?}:iter:{:?}", pos.name, grp_s); // if it's part of a required group we don't want to count it - self.app - .groups - .iter() - .any(|g| g.required && (g.id == grp_s)) + self.app.get_groups().any(|g| g.required && (g.id == grp_s)) }) }) .expect(INTERNAL_ERROR_MSG); @@ -319,7 +310,7 @@ impl<'help, 'app> Usage<'help, 'app> { } for grp_s in self.app.groups_for_arg(&f.id) { debug!("Usage::needs_options_tag:iter:iter: grp_s={:?}", grp_s); - if self.app.groups.iter().any(|g| g.id == grp_s && g.required) { + if self.app.get_groups().any(|g| g.id == grp_s && g.required) { debug!("Usage::needs_options_tag:iter:iter: Group is required"); continue 'outer; } @@ -393,8 +384,7 @@ impl<'help, 'app> Usage<'help, 'app> { let args_in_groups = self .app - .groups - .iter() + .get_groups() .filter(|gn| required.contains(&gn.id)) .flat_map(|g| self.app.unroll_args_in_group(&g.id)) .collect::>(); @@ -403,7 +393,7 @@ impl<'help, 'app> Usage<'help, 'app> { .iter() .chain(incls.iter()) .filter(|name| !self.app.get_positionals().any(|p| &&p.id == name)) - .filter(|name| !self.app.groups.iter().any(|g| &&g.id == name)) + .filter(|name| !self.app.get_groups().any(|g| &&g.id == name)) .filter(|name| !args_in_groups.contains(name)) .filter(|name| !(matcher.is_some() && matcher.as_ref().unwrap().contains(name))) { @@ -414,7 +404,7 @@ impl<'help, 'app> Usage<'help, 'app> { let mut g_vec: Vec = vec![]; for g in unrolled_reqs .iter() - .filter(|n| self.app.groups.iter().any(|g| g.id == **n)) + .filter(|n| self.app.get_groups().any(|g| g.id == **n)) { // don't print requirement for required groups that have an arg. if let Some(m) = matcher { diff --git a/src/parse/arg_matcher.rs b/src/parse/arg_matcher.rs index 12e76514..8a6833bd 100644 --- a/src/parse/arg_matcher.rs +++ b/src/parse/arg_matcher.rs @@ -19,12 +19,12 @@ impl ArgMatcher { ArgMatcher(ArgMatches { #[cfg(debug_assertions)] valid_args: { - let args = _app.args.args().map(|a| a.id.clone()); - let groups = _app.groups.iter().map(|g| g.id.clone()); + let args = _app.get_arguments().map(|a| a.id.clone()); + let groups = _app.get_groups().map(|g| g.id.clone()); args.chain(groups).collect() }, #[cfg(debug_assertions)] - valid_subcommands: _app.subcommands.iter().map(|sc| sc.id.clone()).collect(), + valid_subcommands: _app.get_subcommands().map(|sc| sc.get_id()).collect(), // HACK: Allow an external subcommand's ArgMatches be a stand-in for any ArgMatches // since users can't detect it and avoid the asserts. // diff --git a/src/parse/features/suggestions.rs b/src/parse/features/suggestions.rs index b690ec63..ddb9c17d 100644 --- a/src/parse/features/suggestions.rs +++ b/src/parse/features/suggestions.rs @@ -33,13 +33,14 @@ where } /// Returns a suffix that can be empty, or is the standard 'did you mean' phrase -pub(crate) fn did_you_mean_flag( +pub(crate) fn did_you_mean_flag<'a, 'help, I, T>( arg: &str, remaining_args: &[&str], longs: I, - subcommands: &mut [App], + subcommands: impl IntoIterator>, ) -> Option<(String, Option)> where + 'help: 'a, T: AsRef, I: IntoIterator, { @@ -48,11 +49,11 @@ where match did_you_mean(arg, longs).pop() { Some(candidate) => Some((candidate, None)), None => subcommands - .iter_mut() + .into_iter() .filter_map(|subcommand| { subcommand._build(); - let longs = subcommand.args.keys().filter_map(|a| { + let longs = subcommand.get_keymap().keys().filter_map(|a| { if let KeyType::Long(v) = a { Some(v.to_string_lossy().into_owned()) } else { diff --git a/src/parse/parser.rs b/src/parse/parser.rs index 00a28567..980b2615 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -79,23 +79,23 @@ impl<'help, 'app> Parser<'help, 'app> { let mut trailing_values = false; // Count of positional args - let positional_count = self.app.args.keys().filter(|x| x.is_position()).count(); + let positional_count = self + .app + .get_keymap() + .keys() + .filter(|x| x.is_position()) + .count(); // If any arg sets .last(true) - let contains_last = self.app.args.args().any(|x| x.is_last_set()); + let contains_last = self.app.get_arguments().any(|x| x.is_last_set()); while let Some((arg_os, remaining_args)) = it.next() { // Recover the replaced items if any. - if let Some((_replacer, replaced_items)) = self - .app - .replacers - .iter() - .find(|(key, _)| OsStr::new(key) == arg_os) - { - it.insert(replaced_items); + if let Some(replaced_items) = self.app.get_replacement(arg_os) { debug!( "Parser::get_matches_with: found replacer: {:?}, target: {:?}", - _replacer, replaced_items + arg_os, replaced_items ); + it.insert(replaced_items); continue; } @@ -370,7 +370,7 @@ impl<'help, 'app> Parser<'help, 'app> { } } - if let Some(p) = self.app.args.get(&pos_counter) { + if let Some(p) = self.app.get_keymap().get(&pos_counter) { if p.is_last_set() && !trailing_values { return Err(ClapError::unknown_argument( self.app, @@ -464,8 +464,8 @@ impl<'help, 'app> Parser<'help, 'app> { .app .find_subcommand(pos_sc_name) .expect(INTERNAL_ERROR_MSG) - .name - .clone(); + .get_name() + .to_owned(); self.parse_subcommand(&sc_name, matcher, it, keep_state)?; } @@ -502,10 +502,9 @@ impl<'help, 'app> Parser<'help, 'app> { arg_os.to_str_lossy().into_owned(), candidates.join(" or "), self.app - .bin_name - .as_ref() - .unwrap_or(&self.app.name) - .to_string(), + .get_bin_name() + .unwrap_or_else(|| self.app.get_name()) + .to_owned(), Usage::new(self.app).create_usage_with_title(&[]), ); } @@ -516,10 +515,9 @@ impl<'help, 'app> Parser<'help, 'app> { self.app, arg_os.to_str_lossy().into_owned(), self.app - .bin_name - .as_ref() - .unwrap_or(&self.app.name) - .to_string(), + .get_bin_name() + .unwrap_or_else(|| self.app.get_name()) + .to_owned(), ); } ClapError::unknown_argument( @@ -552,7 +550,7 @@ impl<'help, 'app> Parser<'help, 'app> { // search. } if let Some(sc) = self.app.find_subcommand(arg_os) { - return Some(&sc.name); + return Some(sc.get_name()); } } None @@ -566,7 +564,7 @@ impl<'help, 'app> Parser<'help, 'app> { .app .get_subcommands() .fold(Vec::new(), |mut options, sc| { - if let Some(long) = sc.long_flag { + if let Some(long) = sc.get_long_flag() { if RawOsStr::from_str(long).starts_with_os(arg_os) { options.push(long); } @@ -595,7 +593,11 @@ impl<'help, 'app> Parser<'help, 'app> { fn parse_help_subcommand(&self, cmds: &[OsString]) -> ClapResult { debug!("Parser::parse_help_subcommand"); - let mut bin_name = self.app.bin_name.as_ref().unwrap_or(&self.app.name).clone(); + let mut bin_name = self + .app + .get_bin_name() + .unwrap_or_else(|| self.app.get_name()) + .to_owned(); let mut sc = { let mut sc = self.app.clone(); @@ -610,17 +612,16 @@ impl<'help, 'app> Parser<'help, 'app> { self.app, cmd.to_string_lossy().into_owned(), self.app - .bin_name - .as_ref() - .unwrap_or(&self.app.name) - .to_string(), + .get_bin_name() + .unwrap_or_else(|| self.app.get_name()) + .to_owned(), )); } .clone(); sc._build(); bin_name.push(' '); - bin_name.push_str(&sc.name); + bin_name.push_str(sc.get_name()); } sc @@ -676,7 +677,10 @@ impl<'help, 'app> Parser<'help, 'app> { if let Some(sc) = self.app._build_subcommand(sc_name) { let mut sc_matcher = ArgMatcher::new(sc); - debug!("Parser::parse_subcommand: About to parse sc={}", sc.name); + debug!( + "Parser::parse_subcommand: About to parse sc={}", + sc.get_name() + ); { let mut p = Parser::new(sc); @@ -699,8 +703,8 @@ impl<'help, 'app> Parser<'help, 'app> { } } matcher.subcommand(SubCommand { - id: sc.id.clone(), - name: sc.name.clone(), + id: sc.get_id(), + name: sc.get_name().to_owned(), matches: sc_matcher.into_inner(), }); } @@ -787,10 +791,10 @@ impl<'help, 'app> Parser<'help, 'app> { // Subcommands aren't checked because we prefer short help for them, deferring to // `cmd subcmd --help` for more. - self.app.long_about.is_some() - || self.app.before_long_help.is_some() - || self.app.after_long_help.is_some() - || self.app.args.args().any(should_long) + self.app.get_long_about().is_some() + || self.app.get_before_long_help().is_some() + || self.app.get_after_long_help().is_some() + || self.app.get_arguments().any(should_long) } fn parse_long_arg( @@ -827,7 +831,7 @@ impl<'help, 'app> Parser<'help, 'app> { (long_arg, None) }; - let opt = if let Some(opt) = self.app.args.get(&*arg.to_os_str()) { + let opt = if let Some(opt) = self.app.get_keymap().get(&*arg.to_os_str()) { debug!( "Parser::parse_long_arg: Found valid opt or flag '{}'", opt.to_string() @@ -835,7 +839,7 @@ impl<'help, 'app> Parser<'help, 'app> { Some(opt) } else if self.app.is_infer_long_args_set() { let arg_str = arg.to_str_lossy(); - self.app.args.args().find(|a| { + self.app.get_arguments().find(|a| { a.long.map_or(false, |long| long.starts_with(&*arg_str)) || a.aliases .iter() @@ -916,9 +920,14 @@ impl<'help, 'app> Parser<'help, 'app> { { debug!("Parser::parse_short_args: prior arg accepts hyphenated values",); return ParseResult::MaybeHyphenValue; - } else if self.app.args.get(&pos_counter).map_or(false, |arg| { - arg.is_allow_hyphen_values_set() && !arg.is_last_set() - }) { + } else if self + .app + .get_keymap() + .get(&pos_counter) + .map_or(false, |arg| { + arg.is_allow_hyphen_values_set() && !arg.is_last_set() + }) + { debug!( "Parser::parse_short_args: positional at {} allows hyphens", pos_counter @@ -945,7 +954,7 @@ impl<'help, 'app> Parser<'help, 'app> { // concatenated value: -oval // Option: -o // Value: val - if let Some(opt) = self.app.args.get(&c) { + if let Some(opt) = self.app.get_keymap().get(&c) { debug!( "Parser::parse_short_arg:iter:{}: Found valid opt or flag", c @@ -1353,7 +1362,7 @@ impl<'help, 'app> Parser<'help, 'app> { ) -> ClapResult<()> { use crate::util::str_to_bool; - self.app.args.args().try_for_each(|a| { + self.app.get_arguments().try_for_each(|a| { // Use env only if the arg was absent among command line args, // early return if this is not the case. if matcher @@ -1434,7 +1443,7 @@ impl<'help, 'app> Parser<'help, 'app> { // Didn't match a flag or option let longs = self .app - .args + .get_keymap() .keys() .filter_map(|x| match x { KeyType::Long(l) => Some(l.to_string_lossy().into_owned()), @@ -1447,12 +1456,12 @@ impl<'help, 'app> Parser<'help, 'app> { arg, remaining_args, longs.iter().map(|x| &x[..]), - self.app.subcommands.as_mut_slice(), + self.app.get_subcommands_mut(), ); // Add the arg to the matches to build a proper usage string if let Some((name, _)) = did_you_mean.as_ref() { - if let Some(opt) = self.app.args.get(&name.as_ref()) { + if let Some(opt) = self.app.get_keymap().get(&name.as_ref()) { self.inc_occurrence_of_arg(matcher, opt); } } diff --git a/src/parse/validator.rs b/src/parse/validator.rs index e6baa467..712f6dbb 100644 --- a/src/parse/validator.rs +++ b/src/parse/validator.rs @@ -68,7 +68,11 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> { } #[allow(deprecated)] if !has_subcmd && self.p.app.is_subcommand_required_set() { - let bn = self.p.app.bin_name.as_ref().unwrap_or(&self.p.app.name); + let bn = self + .p + .app + .get_bin_name() + .unwrap_or_else(|| self.p.app.get_name()); return Err(Error::missing_subcommand( self.p.app, bn.to_string(), @@ -487,7 +491,7 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> { } // Validate the conditionally required args - for a in self.p.app.args.args() { + for a in self.p.app.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) @@ -531,8 +535,7 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> { let failed_args: Vec<_> = self .p .app - .args - .args() + .get_arguments() .filter(|&a| { (!a.r_unless.is_empty() || !a.r_unless_all.is_empty()) && !matcher.contains(&a.id)