From b7be22e90bf4c18a2152b9765b0252fdf25c7079 Mon Sep 17 00:00:00 2001 From: CreepySkeleton Date: Thu, 18 Jun 2020 23:16:46 +0300 Subject: [PATCH] Ditch opts! and positionals! --- clap_generate/src/generators/shells/bash.rs | 6 +- clap_generate/src/generators/shells/elvish.rs | 2 +- clap_generate/src/generators/shells/fish.rs | 2 +- .../src/generators/shells/powershell.rs | 2 +- clap_generate/src/generators/shells/zsh.rs | 6 +- src/build/app/mod.rs | 16 ++++- src/macros.rs | 24 ------- src/output/help.rs | 15 ++-- src/output/usage.rs | 69 ++++++++++++++----- src/parse/parser.rs | 29 +++++--- 10 files changed, 105 insertions(+), 66 deletions(-) diff --git a/clap_generate/src/generators/shells/bash.rs b/clap_generate/src/generators/shells/bash.rs index 3e2d6be3..b1b35474 100644 --- a/clap_generate/src/generators/shells/bash.rs +++ b/clap_generate/src/generators/shells/bash.rs @@ -146,7 +146,7 @@ fn option_details_for_path(app: &App, path: &str) -> String { let p = Bash::find_subcommand_with_path(app, path.split("__").skip(1).collect()); let mut opts = String::new(); - for o in opts!(p) { + for o in p.get_opts_no_heading() { if let Some(l) = o.get_long() { opts = format!( "{} @@ -201,7 +201,9 @@ fn all_options_for_path(app: &App, path: &str) -> String { longs = Bash::longs(p) .iter() .fold(String::new(), |acc, l| format!("{} --{}", acc, l)), - pos = positionals!(p).fold(String::new(), |acc, p| format!("{} {}", acc, p)), + pos = p + .get_positionals() + .fold(String::new(), |acc, p| format!("{} {}", acc, p)), subcmds = scs.join(" "), ); diff --git a/clap_generate/src/generators/shells/elvish.rs b/clap_generate/src/generators/shells/elvish.rs index da3a57c7..f5d751c9 100644 --- a/clap_generate/src/generators/shells/elvish.rs +++ b/clap_generate/src/generators/shells/elvish.rs @@ -77,7 +77,7 @@ fn generate_inner<'b>( let mut completions = String::new(); let preamble = String::from("\n cand "); - for option in opts!(p) { + for option in p.get_opts_no_heading() { if let Some(data) = option.get_short() { let tooltip = get_tooltip(option.get_about(), data); diff --git a/clap_generate/src/generators/shells/fish.rs b/clap_generate/src/generators/shells/fish.rs index ac2d43d0..8a9536ee 100644 --- a/clap_generate/src/generators/shells/fish.rs +++ b/clap_generate/src/generators/shells/fish.rs @@ -54,7 +54,7 @@ fn gen_fish_inner(root_command: &str, app: &App, buffer: &mut String) { debug!("gen_fish_inner: bin_name={}", bin_name); - for option in opts!(app) { + for option in app.get_opts_no_heading() { let mut template = basic_template.clone(); if let Some(data) = option.get_short() { diff --git a/clap_generate/src/generators/shells/powershell.rs b/clap_generate/src/generators/shells/powershell.rs index 5cf5daca..e0fbdde9 100644 --- a/clap_generate/src/generators/shells/powershell.rs +++ b/clap_generate/src/generators/shells/powershell.rs @@ -84,7 +84,7 @@ fn generate_inner<'b>( let mut completions = String::new(); let preamble = String::from("\n [CompletionResult]::new("); - for option in opts!(p) { + for option in p.get_opts_no_heading() { if let Some(data) = option.get_short() { let tooltip = get_tooltip(option.get_about(), data); diff --git a/clap_generate/src/generators/shells/zsh.rs b/clap_generate/src/generators/shells/zsh.rs index f1b6d3ea..dc8ca409 100644 --- a/clap_generate/src/generators/shells/zsh.rs +++ b/clap_generate/src/generators/shells/zsh.rs @@ -248,7 +248,7 @@ esac", name = p.get_name(), name_hyphen = p.get_bin_name().unwrap().replace(" ", "-"), subcommands = subcmds.join("\n"), - pos = positionals!(p).count() + 1 + pos = p.get_positionals().count() + 1 ) } @@ -354,7 +354,7 @@ fn write_opts_of(p: &App) -> String { let mut ret = vec![]; - for o in opts!(p) { + for o in p.get_opts_no_heading() { debug!("write_opts_of:iter: o={}", o.get_name()); let help = o.get_about().map_or(String::new(), escape_help); @@ -490,7 +490,7 @@ fn write_positionals_of(p: &App) -> String { let mut ret = vec![]; - for arg in positionals!(p) { + for arg in p.get_positionals() { debug!("write_positionals_of:iter: arg={}", arg.get_name()); let optional = if !arg.is_set(ArgSettings::Required) { diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 3cbed189..427a49e8 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -151,6 +151,12 @@ impl<'b> App<'b> { &self.args.args } + /// Get the list of *positional* arguments. + #[inline] + pub fn get_positionals(&self) -> impl Iterator> { + self.get_arguments().iter().filter(|a| a.is_positional()) + } + /// Iterate through the *flags* that don't have custom heading. pub fn get_flags_no_heading(&self) -> impl Iterator> { self.get_arguments() @@ -159,6 +165,14 @@ impl<'b> App<'b> { .filter(|a| !a.get_help_heading().is_some()) } + /// Iterate through the *options* that don't have custom heading. + pub fn get_opts_no_heading(&self) -> impl Iterator> { + self.get_arguments() + .iter() + .filter(|a| a.is_set(ArgSettings::TakesValue) && a.get_index().is_none()) + .filter(|a| !a.get_help_heading().is_some()) + } + /// Get the list of arguments the given argument conflicts with /// /// ### Panics @@ -1963,7 +1977,7 @@ impl<'b> App<'b> { } pub(crate) fn has_opts(&self) -> bool { - opts!(self).count() > 0 + self.get_opts_no_heading().count() > 0 } pub(crate) fn has_flags(&self) -> bool { diff --git a/src/macros.rs b/src/macros.rs index 73b99edd..d922e981 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -553,30 +553,6 @@ macro_rules! debug { ($($arg:tt)*) => {}; } -#[macro_export] -#[doc(hidden)] -macro_rules! opts { - ($app:expr, $how:ident) => {{ - $app.get_arguments() - .$how() - .filter(|a| a.is_set($crate::ArgSettings::TakesValue) && a.get_index().is_none()) - .filter(|a| !a.get_help_heading().is_some()) - }}; - ($app:expr) => { - opts!($app, iter) - }; -} - -#[macro_export] -#[doc(hidden)] -macro_rules! positionals { - ($app:expr) => {{ - $app.get_arguments() - .iter() - .filter(|a| !(a.get_short().is_some() || a.get_long().is_some())) - }}; -} - macro_rules! groups_for_arg { ($app:expr, $grp:expr) => {{ debug!("groups_for_arg: name={:?}", $grp); diff --git a/src/output/help.rs b/src/output/help.rs index 4cab0f28..0692a30b 100644 --- a/src/output/help.rs +++ b/src/output/help.rs @@ -656,15 +656,18 @@ impl<'b, 'c, 'd, 'w> Help<'b, 'c, 'd, 'w> { pub(crate) fn write_all_args(&mut self) -> ClapResult<()> { debug!("Help::write_all_args"); let flags = self.parser.has_flags(); - // Strange filter/count vs fold... https://github.com/rust-lang/rust/issues/33038 - let pos = positionals!(self.parser.app).fold(0, |acc, arg| { + // FIXME: Strange filter/count vs fold... https://github.com/rust-lang/rust/issues/33038 + let pos = self.parser.app.get_positionals().fold(0, |acc, arg| { if should_show_arg(self.use_long, arg) { acc + 1 } else { acc } }) > 0; - let opts = opts!(self.parser.app) + let opts = self + .parser + .app + .get_opts_no_heading() .filter(|arg| should_show_arg(self.use_long, arg)) .collect::>(); let subcmds = self.parser.has_visible_subcommands(); @@ -680,7 +683,7 @@ impl<'b, 'c, 'd, 'w> Help<'b, 'c, 'd, 'w> { let mut first = if pos { self.warning("ARGS:\n")?; - self.write_args_unsorted(&*positionals!(self.parser.app).collect::>())?; + self.write_args_unsorted(&self.parser.app.get_positionals().collect::>())?; false } else { true @@ -1059,10 +1062,10 @@ impl<'b, 'c, 'd, 'w> Help<'b, 'c, 'd, 'w> { self.write_args(&self.parser.app.get_flags_no_heading().collect::>())?; } b"options" => { - self.write_args(&*opts!(self.parser.app).collect::>())?; + self.write_args(&self.parser.app.get_opts_no_heading().collect::>())?; } b"positionals" => { - self.write_args(&*positionals!(self.parser.app).collect::>())?; + self.write_args(&self.parser.app.get_positionals().collect::>())?; } b"subcommands" => { self.write_subcommands(self.parser.app)?; diff --git a/src/output/usage.rs b/src/output/usage.rs index 97379d28..cc09165b 100644 --- a/src/output/usage.rs +++ b/src/output/usage.rs @@ -71,7 +71,10 @@ impl<'b, 'c, 'z> Usage<'b, 'c, 'z> { usage.push_str(" [OPTIONS]"); } if !self.p.is_set(AS::UnifiedHelpMessage) - && opts!(self.p.app) + && self + .p + .app + .get_opts_no_heading() .any(|o| !o.is_set(ArgSettings::Required) && !o.is_set(ArgSettings::Hidden)) { usage.push_str(" [OPTIONS]"); @@ -79,11 +82,23 @@ impl<'b, 'c, 'z> Usage<'b, 'c, 'z> { usage.push_str(&req_string[..]); - let has_last = positionals!(self.p.app).any(|p| p.is_set(ArgSettings::Last)); + let has_last = self + .p + .app + .get_positionals() + .any(|p| p.is_set(ArgSettings::Last)); // places a '--' in the usage string if there are args and options // supporting multiple values - if opts!(self.p.app).any(|o| o.is_set(ArgSettings::MultipleValues)) - && positionals!(self.p.app).any(|p| !p.is_set(ArgSettings::Required)) + if self + .p + .app + .get_opts_no_heading() + .any(|o| o.is_set(ArgSettings::MultipleValues)) + && self + .p + .app + .get_positionals() + .any(|p| !p.is_set(ArgSettings::Required)) && !(self.p.app.has_visible_subcommands() || self.p.is_set(AS::AllowExternalSubcommands)) && !has_last @@ -94,19 +109,28 @@ impl<'b, 'c, 'z> Usage<'b, 'c, 'z> { (!p.is_set(ArgSettings::Required) || p.is_set(ArgSettings::Last)) && !p.is_set(ArgSettings::Hidden) }; - if positionals!(self.p.app).any(not_req_or_hidden) { + if self.p.app.get_positionals().any(not_req_or_hidden) { if let Some(args_tag) = self.get_args_tag(incl_reqs) { usage.push_str(&*args_tag); } else { usage.push_str(" [ARGS]"); } if has_last && incl_reqs { - let pos = positionals!(self.p.app) + let pos = self + .p + .app + .get_positionals() .find(|p| p.is_set(ArgSettings::Last)) .expect(INTERNAL_ERROR_MSG); debug!("Usage::create_help_usage: '{}' has .last(true)", pos.name); let req = pos.is_set(ArgSettings::Required); - if req && positionals!(self.p.app).any(|p| !p.is_set(ArgSettings::Required)) { + if req + && self + .p + .app + .get_positionals() + .any(|p| !p.is_set(ArgSettings::Required)) + { usage.push_str(" -- <"); } else if req { usage.push_str(" [--] <"); @@ -181,7 +205,10 @@ impl<'b, 'c, 'z> Usage<'b, 'c, 'z> { fn get_args_tag(&self, incl_reqs: bool) -> Option { debug!("Usage::get_args_tag; incl_reqs = {:?}", incl_reqs); let mut count = 0; - 'outer: for pos in positionals!(self.p.app) + 'outer: for pos in self + .p + .app + .get_positionals() .filter(|pos| !pos.is_set(ArgSettings::Required)) .filter(|pos| !pos.is_set(ArgSettings::Hidden)) .filter(|pos| !pos.is_set(ArgSettings::Last)) @@ -210,7 +237,10 @@ impl<'b, 'c, 'z> Usage<'b, 'c, 'z> { debug!("Usage::get_args_tag:iter: More than one, returning [ARGS]"); return None; // [ARGS] } else if count == 1 && incl_reqs { - let pos = positionals!(self.p.app) + let pos = self + .p + .app + .get_positionals() .find(|pos| { !pos.is_set(ArgSettings::Required) && !pos.is_set(ArgSettings::Hidden) @@ -232,7 +262,9 @@ impl<'b, 'c, 'z> Usage<'b, 'c, 'z> { { debug!("Usage::get_args_tag:iter: Don't collapse returning all"); return Some( - positionals!(self.p.app) + self.p + .app + .get_positionals() .filter(|pos| !pos.is_set(ArgSettings::Required)) .filter(|pos| !pos.is_set(ArgSettings::Hidden)) .filter(|pos| !pos.is_set(ArgSettings::Last)) @@ -242,7 +274,10 @@ impl<'b, 'c, 'z> Usage<'b, 'c, 'z> { ); } else if !incl_reqs { debug!("Usage::get_args_tag:iter: incl_reqs=false, building secondary usage string"); - let highest_req_pos = positionals!(self.p.app) + let highest_req_pos = self + .p + .app + .get_positionals() .filter_map(|pos| { if pos.is_set(ArgSettings::Required) && !pos.is_set(ArgSettings::Last) { Some(pos.index) @@ -251,9 +286,11 @@ impl<'b, 'c, 'z> Usage<'b, 'c, 'z> { } }) .max() - .unwrap_or_else(|| Some(positionals!(self.p.app).count() as u64)); + .unwrap_or_else(|| Some(self.p.app.get_positionals().count() as u64)); return Some( - positionals!(self.p.app) + self.p + .app + .get_positionals() .filter_map(|pos| { if pos.index <= highest_req_pos { Some(pos) @@ -356,7 +393,7 @@ impl<'b, 'c, 'z> Usage<'b, 'c, 'z> { unrolled_reqs .iter() .chain(incls.iter()) - .filter(|a| positionals!(self.p.app).any(|p| &&p.id == a)) + .filter(|a| self.p.app.get_positionals().any(|p| &&p.id == a)) .filter(|&pos| !m.contains(pos)) .filter_map(|pos| self.p.app.find(pos)) .filter(|&pos| incl_last || !pos.is_set(ArgSettings::Last)) @@ -367,7 +404,7 @@ impl<'b, 'c, 'z> Usage<'b, 'c, 'z> { unrolled_reqs .iter() .chain(incls.iter()) - .filter(|a| positionals!(self.p.app).any(|p| &&p.id == a)) + .filter(|a| self.p.app.get_positionals().any(|p| &&p.id == a)) .filter_map(|pos| self.p.app.find(pos)) .filter(|&pos| incl_last || !pos.is_set(ArgSettings::Last)) .filter(|pos| !args_in_groups.contains(&pos.id)) @@ -383,7 +420,7 @@ impl<'b, 'c, 'z> Usage<'b, 'c, 'z> { for a in unrolled_reqs .iter() .chain(incls.iter()) - .filter(|name| !positionals!(self.p.app).any(|p| &&p.id == name)) + .filter(|name| !self.p.app.get_positionals().any(|p| &&p.id == name)) .filter(|name| !self.p.app.groups.iter().any(|g| &&g.id == name)) .filter(|name| !args_in_groups.contains(name)) .filter(|name| !(matcher.is_some() && matcher.as_ref().unwrap().contains(name))) diff --git a/src/parse/parser.rs b/src/parse/parser.rs index 1e05593f..5259b782 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -166,7 +166,7 @@ where let only_highest = |a: &Arg| { a.is_set(ArgSettings::MultipleValues) && (a.index.unwrap_or(0) != highest_idx) }; - if positionals!(self.app).any(only_highest) { + if self.app.get_positionals().any(only_highest) { // First we make sure if there is a positional that allows multiple values // the one before it (second to last) has one of these: // * a value terminator @@ -201,7 +201,9 @@ where ); // Next we check how many have both Multiple and not a specific number of values set - let count = positionals!(self.app) + let count = self + .app + .get_positionals() .filter(|p| p.settings.is_set(ArgSettings::MultipleValues) && p.num_vals.is_none()) .count(); let ok = count <= 1 @@ -222,7 +224,7 @@ where let mut found = false; let mut foundx2 = false; - for p in positionals!(self.app) { + for p in self.app.get_positionals() { if foundx2 && !p.is_set(ArgSettings::Required) { assert!( p.is_set(ArgSettings::Required), @@ -278,13 +280,16 @@ where } } assert!( - positionals!(self.app) + self.app + .get_positionals() .filter(|p| p.is_set(ArgSettings::Last)) .count() < 2, "Only one positional argument may have last(true) set. Found two." ); - if positionals!(self.app) + if self + .app + .get_positionals() .any(|p| p.is_set(ArgSettings::Last) && p.is_set(ArgSettings::Required)) && self.has_subcommands() && !self.is_set(AS::SubcommandsNegateReqs) @@ -331,7 +336,7 @@ where self._verify_positionals(); // Set the LowIndexMultiple flag if required - if positionals!(self.app).any(|a| { + if self.app.get_positionals().any(|a| { a.is_set(ArgSettings::MultipleValues) && (a.index.unwrap_or(0) as usize != self @@ -341,7 +346,7 @@ where .iter() .filter(|x| x.key.is_position()) .count()) - }) && positionals!(self.app).last().map_or(false, |p_name| { + }) && self.app.get_positionals().last().map_or(false, |p_name| { !self.app[&p_name.id].is_set(ArgSettings::Last) }) { self.app.settings.set(AS::LowIndexMultiplePositional); @@ -555,8 +560,10 @@ where ParseResult::ValuesDone(id) => ParseResult::ValuesDone(id), _ => { - if let Some(p) = - positionals!(self.app).find(|p| p.index == Some(pos_counter as u64)) + if let Some(p) = self + .app + .get_positionals() + .find(|p| p.index == Some(pos_counter as u64)) { ParseResult::Pos(p.id.clone()) } else { @@ -1469,12 +1476,12 @@ where pub(crate) fn add_defaults(&mut self, matcher: &mut ArgMatcher) -> ClapResult<()> { debug!("Parser::add_defaults"); - for o in opts!(self.app) { + for o in self.app.get_opts_no_heading() { debug!("Parser::add_defaults:iter:{}:", o.name); self.add_value(o, matcher, ValueType::DefaultValue)?; } - for p in positionals!(self.app) { + for p in self.app.get_positionals() { debug!("Parser::add_defaults:iter:{}:", p.name); self.add_value(p, matcher, ValueType::DefaultValue)?; }