From c89718fa57ed984539df5a438f6e083ccfd5c4b0 Mon Sep 17 00:00:00 2001 From: NickHackman Date: Sat, 13 Jun 2020 11:32:24 -0400 Subject: [PATCH 01/30] tests: FlagSubCommand Test all methods of `FlagSubCommand` and interactions with aliases --- tests/flag_subcommands.rs | 151 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 tests/flag_subcommands.rs diff --git a/tests/flag_subcommands.rs b/tests/flag_subcommands.rs new file mode 100644 index 00000000000..f5f6797ca30 --- /dev/null +++ b/tests/flag_subcommands.rs @@ -0,0 +1,151 @@ +use clap::{App, Arg, FlagSubCommand}; + +#[test] +fn flag_subcommand_normal() { + let matches = App::new("test") + .subcommand( + FlagSubCommand::new('S', "some").arg( + Arg::new("test") + .short('t') + .long("test") + .about("testing testing"), + ), + ) + .get_matches_from(vec!["myprog", "some", "--test"]); + assert_eq!(matches.subcommand_name().unwrap(), "some"); + let sub_matches = matches.subcommand_matches("some").unwrap(); + assert!(sub_matches.is_present("test")); +} + +#[test] +fn flag_subcommand_normal_with_alias() { + let matches = App::new("test") + .subcommand( + FlagSubCommand::new('S', "some") + .arg( + Arg::new("test") + .short('t') + .long("test") + .about("testing testing"), + ) + .alias("result"), + ) + .get_matches_from(vec!["myprog", "result", "--test"]); + assert_eq!(matches.subcommand_name().unwrap(), "some"); + let sub_matches = matches.subcommand_matches("some").unwrap(); + assert!(sub_matches.is_present("test")); +} + +#[test] +fn flag_subcommand_short() { + let matches = App::new("test") + .subcommand( + FlagSubCommand::new_short("some", 'S').arg( + Arg::new("test") + .short('t') + .long("test") + .about("testing testing"), + ), + ) + .get_matches_from(vec!["myprog", "-S", "--test"]); + assert_eq!(matches.subcommand_name().unwrap(), "some"); + let sub_matches = matches.subcommand_matches("some").unwrap(); + assert!(sub_matches.is_present("test")); +} + +#[test] +fn flag_subcommand_short_with_args() { + let matches = App::new("test") + .subcommand( + FlagSubCommand::new_short("some", 'S').arg( + Arg::new("test") + .short('t') + .long("test") + .about("testing testing"), + ), + ) + .get_matches_from(vec!["myprog", "-St"]); + assert_eq!(matches.subcommand_name().unwrap(), "some"); + let sub_matches = matches.subcommand_matches("some").unwrap(); + assert!(sub_matches.is_present("test")); +} + +#[test] +fn flag_subcommand_short_with_alias() { + let matches = App::new("test") + .subcommand( + FlagSubCommand::new_short("some", 'S') + .arg( + Arg::new("test") + .short('t') + .long("test") + .about("testing testing"), + ) + .alias("M") + .alias("B"), + ) + .get_matches_from(vec!["myprog", "-Bt"]); + assert_eq!(matches.subcommand_name().unwrap(), "some"); + let sub_matches = matches.subcommand_matches("some").unwrap(); + assert!(sub_matches.is_present("test")); +} + +#[test] +fn flag_subcommand_long() { + let matches = App::new("test") + .subcommand( + FlagSubCommand::new_long("some", "some").arg( + Arg::new("test") + .short('t') + .long("test") + .about("testing testing"), + ), + ) + .get_matches_from(vec!["myprog", "--some", "--test"]); + assert_eq!(matches.subcommand_name().unwrap(), "some"); + let sub_matches = matches.subcommand_matches("some").unwrap(); + assert!(sub_matches.is_present("test")); +} + +#[test] +fn flag_subcommand_long_with_alias() { + let matches = App::new("test") + .subcommand( + FlagSubCommand::new_long("some", "some") + .arg( + Arg::new("test") + .short('t') + .long("test") + .about("testing testing"), + ) + .alias("result"), + ) + .get_matches_from(vec!["myprog", "--result", "--test"]); + assert_eq!(matches.subcommand_name().unwrap(), "some"); + let sub_matches = matches.subcommand_matches("some").unwrap(); + assert!(sub_matches.is_present("test")); +} + +#[test] +fn flag_subcommand_multiple() { + let matches = App::new("test") + .subcommand( + FlagSubCommand::new('S', "some") + .arg(Arg::from("-f, --flag 'some flag'")) + .arg(Arg::from("-p, --print 'print something'")) + .subcommand( + FlagSubCommand::new('R', "result") + .arg(Arg::from("-f, --flag 'some flag'")) + .arg(Arg::from("-p, --print 'print something'")), + ), + ) + .get_matches_from(vec!["myprog", "-SfpRfp"]); + assert_eq!(matches.subcommand_name().unwrap(), "some"); + let sub_matches = matches.subcommand_matches("some").unwrap(); + assert!(sub_matches.is_present("flag")); + assert!(sub_matches.is_present("print")); + assert_eq!(sub_matches.subcommand_name().unwrap(), "result"); + let result_matches = sub_matches.subcommand_matches("result").unwrap(); + assert!(result_matches.is_present("flag")); + assert!(result_matches.is_present("print")); +} From 50829711efc141a24d41bd5951ce1f0e9749390b Mon Sep 17 00:00:00 2001 From: NickHackman Date: Sat, 13 Jun 2020 12:28:10 -0400 Subject: [PATCH 02/30] examples: FlagSubCommand pacman Using `pacman` as an example for `FlagSubCommand` because of #1361 --- examples/23_flag_subcommands_pacman.rs | 153 +++++++++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 examples/23_flag_subcommands_pacman.rs diff --git a/examples/23_flag_subcommands_pacman.rs b/examples/23_flag_subcommands_pacman.rs new file mode 100644 index 00000000000..aa0a694c639 --- /dev/null +++ b/examples/23_flag_subcommands_pacman.rs @@ -0,0 +1,153 @@ +// Working with flag subcommands allows behavior similar to the popular Archlinux package manager Pacman. +// Man page: https://jlk.fjfi.cvut.cz/arch/manpages/man/pacman.8 +// +// It's suggested that you read examples/20_subcommands.rs prior to learning about `FlagSubCommand`s +// +// This differs from normal subcommands because it allows passing subcommand as `clap::Arg` in short or long args. +// +// Top Level App (pacman) TOP +// | +// --------------------------------------------------- +// / | | | | \ \ +// sync database remove files query deptest upgrade LEVEL 1 +// +// Given the above hierachy, valid runtime uses would be (not an all inclusive list): +// +// $ pacman -Ss +// ^--- subcommand followed by an arg in its scope. +// +// $ pacman -Qs +// +// $ pacman -Rns +// +// NOTE: Subcommands short flags don't have to be uppercase. +// +// $ pacman --sync --search +// ^--- subcommand +// +// $ pacman sync -s +// ^--- subcommand +// +// NOTE: this isn't valid for pacman, but `clap::FlagSubCommand` +// adds support for both flags and standard subcommands out of the box. +// Allowing your users to make the choice of what feels more intuitive for them. +// +// Notice only one command per "level" may be used. You could not, for example, do: +// +// $ pacman -SQR +// +// It's also important to know that subcommands each have their own set of matches and may have args +// with the same name as other subcommands in a different part of the tree heirachy (i.e. the arg +// names aren't in a flat namespace). +// +// In order to use subcommands in clap, you only need to know which subcommand you're at in your +// tree, and which args are defined on that subcommand. +// +// Let's make a quick program to illustrate. We'll be using the same example as above but for +// brevity sake we won't implement all of the subcommands, only a few. +use clap::{App, AppSettings, Arg, FlagSubCommand}; + +fn main() { + let matches = App::new("pacman") + .about("package manager utility") + .version("5.2.1") + .setting(AppSettings::SubcommandRequiredElseHelp) + .author("Pacman Development Team") + // Query subcommand + // + // Only a few of its arguments are implemented below. + .subcommand( + // When getting the subcommand name the long version is used (in this case "query"). + // If you want to use a different name then `FlagSubCommand::with_name("name", 's', "long")`. + // + // NOTE: if the name has been changed then it can be used as that: + // + // $ MyProg name + // + // $ MyProg --long + // + // $ MyProg -s + FlagSubCommand::new('Q', "query") + .about("Query the package database.") + .arg( + Arg::new("search") + .short('s') + .long("search") + .about("search locally-installed packages for matching strings") + .takes_value(true), + ) + .arg( + Arg::new("info") + .long("info") + .short('i') + .about("view package information (-ii for backup files)") + .multiple(true), + ), + ) + // Sync subcommand + // + // Only a few of its arguments are implemented below. + .subcommand( + FlagSubCommand::new('S', "sync") + .about("Synchronize packages.") + .arg( + Arg::new("search") + .short('s') + .long("search") + .about("search remote repositories for matching strings") + .takes_value(true), + ) + .arg( + Arg::new("info") + .long("info") + .short('i') + .about("view package information (-ii for extended information)") + .multiple(true), + ) + .arg( + Arg::new("package") + .about("package") + .multiple(true) + .takes_value(true), + ), + ) + .get_matches(); + + match matches.subcommand() { + ("sync", Some(sync_matches)) => { + if sync_matches.is_present("info") { + // Values required here, so it's safe to unwrap + let packages: Vec<_> = sync_matches.values_of("info").unwrap().collect(); + println!("Retrieving info for {:?}...", packages); + } else if sync_matches.is_present("search") { + // Values required here, so it's safe to unwrap + let queries: Vec<_> = sync_matches.values_of("search").unwrap().collect(); + println!("Searching for {:?}...", queries); + } else { + match sync_matches.values_of("package") { + Some(packages) => { + let pkgs: Vec<_> = packages.collect(); + println!("Installing {:?}...", pkgs); + } + None => panic!("No targets specified (use -h for help)"), + } + } + } + ("query", Some(query_matches)) => { + if query_matches.is_present("info") { + // Values required here, so it's safe to unwrap + let packages: Vec<_> = query_matches.values_of("info").unwrap().collect(); + println!("Retrieving info for {:?}...", packages); + } else if query_matches.is_present("search") { + // Values required here, so it's safe to unwrap + let queries: Vec<_> = query_matches.values_of("search").unwrap().collect(); + println!("Searching Locally for {:?}...", queries); + } else { + // Query was called without any arguments + println!("Displaying all locally installed packages..."); + } + } + ("", None) => panic!("error: no operation specified (use -h for help)"), // If no subcommand was used + _ => unreachable!(), // If all subcommands are defined above, anything else is unreachable + } +} From 0b179fe8ef94508a1a7ad6ae62354d92ac6357d5 Mon Sep 17 00:00:00 2001 From: NickHackman Date: Sat, 13 Jun 2020 17:40:09 -0400 Subject: [PATCH 03/30] api: FlagSubCommand, subcommand flags #1361 Adds support for Flag subcommands, for example: $ pacman -Ss $ pacman --sync -s --- src/build/app/flag_subcommand.rs | 216 +++++++++++++++++++++++++++++++ src/build/app/mod.rs | 5 +- src/build/mod.rs | 2 +- src/lib.rs | 2 +- src/macros.rs | 24 ++++ src/output/help.rs | 25 ++-- src/parse/parser.rs | 48 ++++++- 7 files changed, 307 insertions(+), 15 deletions(-) create mode 100644 src/build/app/flag_subcommand.rs diff --git a/src/build/app/flag_subcommand.rs b/src/build/app/flag_subcommand.rs new file mode 100644 index 00000000000..bcbefdecbef --- /dev/null +++ b/src/build/app/flag_subcommand.rs @@ -0,0 +1,216 @@ +use super::App; + +use crate::util::Id; + +/// Used to create a subcommand that can be used as if it were a Flag or a regular subcommand. +/// +/// # Examples +/// ```no_run +/// # use clap::{App, FlagSubCommand}; +/// +/// let m = App::new("MyProgram") +/// .author("Me, me@mail.com") +/// .version("1.0.2") +/// .about("Explains in brief what the program does") +/// .subcommand( +/// FlagSubCommand::new('S', "sub") +/// .about("Explains purpose of subcommand") +/// ) +/// .after_help("Longer explanation to appear after the options when \ +/// displaying the help information from --help or -h") +/// .get_matches(); +/// +/// // Your program logic starts here... +/// ``` +/// +/// The above example's subcommand can be executed with any one of the following +/// +/// ```sh +/// $ MyProgram subcommand +/// ``` +/// +/// ```sh +/// $ MyProgram -S +/// ``` +/// +/// ```sh +/// $ MyProgram --sub +/// ``` +#[derive(Debug, Clone, Copy)] +pub struct FlagSubCommand {} + +#[allow(clippy::new_ret_no_self)] +impl FlagSubCommand { + /// Creates a Subcommand that can be used either as a short Flag or a full named subcommand + /// + /// # Examples + /// ```no_run + /// # use clap::{App, FlagSubCommand}; + /// + /// let m = App::new("MyProgram") + /// .author("Me, me@mail.com") + /// .version("1.0.2") + /// .about("Explains in brief what the program does") + /// .subcommand( + /// FlagSubCommand::new_short("subcommand", 'S') + /// .about("Explains purpose of subcommand") + /// ) + /// .after_help("Longer explanation to appear after the options when \ + /// displaying the help information from --help or -h") + /// .get_matches(); + /// + /// // Your program logic starts here... + /// ``` + /// + /// The above example's subcommand can be executed with any one of the following + /// + /// ```sh + /// $ MyProgram subcommand + /// ``` + /// + /// ```sh + /// $ MyProgram -S + /// ``` + pub fn new_short<'b, S: Into>(n: S, short: char) -> App<'b> { + let name = n.into(); + App { + id: Id::from(&*name), + name, + short: Some(short), + ..Default::default() + } + } + + /// Creates a Subcommand that can be used either as a long Flag or a full named subcommand + /// + /// # Examples + /// ```no_run + /// # use clap::{App, FlagSubCommand}; + /// + /// let m = App::new("MyProgram") + /// .author("Me, me@mail.com") + /// .version("1.0.2") + /// .about("Explains in brief what the program does") + /// .subcommand( + /// FlagSubCommand::new_long("subcommand", "sub") + /// .about("Explains purpose of subcommand") + /// ) + /// .after_help("Longer explanation to appear after the options when \ + /// displaying the help information from --help or -h") + /// .get_matches(); + /// + /// // Your program logic starts here... + /// ``` + /// + /// The above example's subcommand can be executed with any one of the following + /// + /// ```sh + /// $ MyProgram subcommand + /// ``` + /// + /// ```sh + /// $ MyProgram --sub + /// ``` + pub fn new_long>(n: S, long: &str) -> App { + let name = n.into(); + App { + id: Id::from(&*name), + name, + long: Some(long), + ..Default::default() + } + } + + /// Used to create a subcommand that can be used as if it were a Flag or a regular subcommand. + /// + /// NOTE: `long` parameter is used as the name of the subcommand + /// + /// # Examples + /// ```no_run + /// # use clap::{App, FlagSubCommand}; + /// + /// let m = App::new("MyProgram") + /// .author("Me, me@mail.com") + /// .version("1.0.2") + /// .about("Explains in brief what the program does") + /// .subcommand( + /// FlagSubCommand::new('S', "subcommand") + /// .about("Explains purpose of subcommand") + /// ) + /// .after_help("Longer explanation to appear after the options when \ + /// displaying the help information from --help or -h") + /// .get_matches(); + /// + /// // Your program logic starts here... + /// ``` + /// + /// The above example's subcommand can be executed with any one of the following + /// + /// ```sh + /// $ MyProgram subcommand + /// ``` + /// + /// ```sh + /// $ MyProgram -S + /// ``` + /// + /// ```sh + /// $ MyProgram --subcommand + /// ``` + pub fn new(short: char, long: &str) -> App { + let name = long.to_string(); + App { + id: Id::from(long), + name, + short: Some(short), + long: Some(long), + ..Default::default() + } + } + + /// Used to create a subcommand that can be used as if it were a Flag or a regular subcommand. + /// Where the name differs from the long flag. + /// + /// # Examples + /// ```no_run + /// # use clap::{App, FlagSubCommand}; + /// + /// let m = App::new("MyProgram") + /// .author("Me, me@mail.com") + /// .version("1.0.2") + /// .about("Explains in brief what the program does") + /// .subcommand( + /// FlagSubCommand::with_name("subcommand2", 'S', "subcommand") + /// .about("Explains purpose of subcommand") + /// ) + /// .after_help("Longer explanation to appear after the options when \ + /// displaying the help information from --help or -h") + /// .get_matches(); + /// + /// // Your program logic starts here... + /// ``` + /// + /// The above example's subcommand can be executed with any one of the following + /// + /// ```sh + /// $ MyProgram subcommand2 + /// ``` + /// + /// ```sh + /// $ MyProgram -S + /// ``` + /// + /// ```sh + /// $ MyProgram --subcommand + /// ``` + pub fn with_name>(n: S, short: char, long: &str) -> App { + let name = n.into(); + App { + id: Id::from(&*name), + name, + short: Some(short), + long: Some(long), + ..Default::default() + } + } +} diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 372152d4078..ccb2dfbf0f4 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -1,8 +1,9 @@ +mod flag_subcommand; mod settings; #[cfg(test)] mod tests; -pub use self::settings::AppSettings; +pub use self::{flag_subcommand::FlagSubCommand, settings::AppSettings}; // Std use std::{ @@ -72,6 +73,8 @@ pub(crate) enum Propagation { pub struct App<'b> { pub(crate) id: Id, pub(crate) name: String, + pub(crate) long: Option<&'b str>, + pub(crate) short: Option, pub(crate) bin_name: Option, pub(crate) author: Option<&'b str>, pub(crate) version: Option<&'b str>, diff --git a/src/build/mod.rs b/src/build/mod.rs index 9ed76dacb2c..3eb184a03a1 100644 --- a/src/build/mod.rs +++ b/src/build/mod.rs @@ -8,7 +8,7 @@ mod arg_group; mod usage_parser; pub use self::{ - app::{App, AppSettings}, + app::{App, AppSettings, FlagSubCommand}, arg::{Arg, ArgSettings}, arg_group::ArgGroup, }; diff --git a/src/lib.rs b/src/lib.rs index 7c628cfaed8..cd4279fc826 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,7 +22,7 @@ compile_error!("`std` feature is currently required to build `clap`"); pub use crate::{ - build::{App, AppSettings, Arg, ArgGroup, ArgSettings}, + build::{App, AppSettings, Arg, ArgGroup, ArgSettings, FlagSubCommand}, derive::{ArgEnum, Clap, FromArgMatches, IntoApp, Subcommand}, parse::errors::{Error, ErrorKind, Result}, parse::{ArgMatches, Indices, OsValues, Values}, diff --git a/src/macros.rs b/src/macros.rs index 3a4f3f5b707..430c5f8ce4e 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -630,6 +630,30 @@ macro_rules! find_subcmd_mut { }}; } +#[macro_export] +#[doc(hidden)] +macro_rules! find_short_subcmd { + ($app:expr, $c:expr) => {{ + $app.get_subcommands().iter().find(|sc| { + sc.short == Some($c) || sc.get_all_aliases().any(|alias| alias == $c.to_string()) + }) + }}; +} + +#[macro_export] +#[doc(hidden)] +macro_rules! find_long_subcmd { + ($app:expr, $s:expr) => {{ + $app.get_subcommands().iter().find(|sc| { + if let Some(long) = sc.long { + match_alias!(sc, $s, long) + } else { + false + } + }) + }}; +} + macro_rules! longs { ($app:expr, $how:ident) => {{ use crate::mkeymap::KeyType; diff --git a/src/output/help.rs b/src/output/help.rs index dbbddd9e635..e481870af28 100644 --- a/src/output/help.rs +++ b/src/output/help.rs @@ -546,29 +546,30 @@ impl<'b, 'c, 'd, 'w> Help<'b, 'c, 'd, 'w> { /// Methods to write a single subcommand impl<'b, 'c, 'd, 'w> Help<'b, 'c, 'd, 'w> { - fn write_subcommand(&mut self, app: &App<'b>) -> io::Result<()> { + fn write_subcommand(&mut self, sc_str: &str, app: &App<'b>) -> io::Result<()> { debug!("Help::write_subcommand"); self.none(TAB)?; - self.good(&app.name)?; - let spec_vals = self.sc_val(app)?; + self.good(sc_str)?; + let spec_vals = self.sc_val(sc_str, app)?; self.sc_help(app, &*spec_vals)?; Ok(()) } - fn sc_val(&mut self, app: &App<'b>) -> Result { + fn sc_val(&mut self, sc_str: &str, app: &App<'b>) -> Result { debug!("Help::sc_val: app={}", app.name); let spec_vals = self.sc_spec_vals(app); let h = app.about.unwrap_or(""); let h_w = str_width(h) + str_width(&*spec_vals); let nlh = self.next_line_help; let taken = self.longest + 12; + debug!("@NickHackman taken = {}", taken); self.force_next_line = !nlh && self.term_w >= taken && (taken as f32 / self.term_w as f32) > 0.40 && h_w > (self.term_w - taken); if !(nlh || self.force_next_line) { - write_nspaces!(self, self.longest + 4 - (str_width(&app.name))); + write_nspaces!(self, self.longest + 4 - (str_width(sc_str))); } Ok(spec_vals) } @@ -769,19 +770,25 @@ impl<'b, 'c, 'd, 'w> Help<'b, 'c, 'd, 'w> { .filter(|s| !s.is_set(AppSettings::Hidden)) { let btm = ord_m.entry(sc.disp_ord).or_insert(BTreeMap::new()); - self.longest = cmp::max(self.longest, str_width(sc.name.as_str())); - btm.insert(sc.name.clone(), sc.clone()); + let mut sc_str = String::new(); + sc_str.push_str(&sc.short.map_or(String::new(), |c| format!("-{}, ", c))); + sc_str.push_str(&sc.long.map_or(String::new(), |c| format!("--{}, ", c))); + sc_str.push_str(&sc.name); + self.longest = cmp::max(self.longest, str_width(&sc_str)); + btm.insert(sc_str, sc.clone()); } + debug!("Help::write_subcommands longest = {}", self.longest); + let mut first = true; for btm in ord_m.values() { - for sc in btm.values() { + for (sc_str, sc) in btm { if first { first = false; } else { self.none("\n")?; } - self.write_subcommand(sc)?; + self.write_subcommand(sc_str, sc)?; } } Ok(()) diff --git a/src/parse/parser.rs b/src/parse/parser.rs index 1e05593f553..e132b734d51 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -25,6 +25,9 @@ use crate::{ #[derive(Debug, PartialEq, Clone)] pub(crate) enum ParseResult { Flag(Id), + FlagSubCommand(String), + // subcommand name, whether there are more shorts args remaining + FlagSubCommandShort(String, bool), Opt(Id), Pos(Id), MaybeHyphenValue, @@ -78,6 +81,10 @@ impl Input { } } + pub(crate) fn previous(&mut self) { + self.cursor -= 1; + } + pub(crate) fn remaining(&self) -> &[OsString] { &self.items[self.cursor..] } @@ -92,6 +99,7 @@ where pub(crate) overriden: Vec, pub(crate) seen: Vec, pub(crate) cur_idx: Cell, + pub(crate) skip_idxs: usize, } // Initializing Methods @@ -116,6 +124,7 @@ where overriden: Vec::new(), seen: Vec::new(), cur_idx: Cell::new(0), + skip_idxs: 0, } } @@ -456,7 +465,13 @@ where self.maybe_inc_pos_counter(&mut pos_counter, id); continue; } - + ParseResult::FlagSubCommand(ref name) => { + debug!("Parser::get_matches_with: FlagSubCommand found in long arg {:?}", name); + self.parse_subcommand(name, matcher, it, false)?; + external_subcommand = true; + continue; + } + ParseResult::FlagSubCommandShort(_, _) => unreachable!(), _ => (), } } else if arg_os.starts_with("-") && arg_os.len() != 1 { @@ -489,6 +504,17 @@ where self.maybe_inc_pos_counter(&mut pos_counter, id); continue; } + ParseResult::FlagSubCommandShort(ref name, done) => { + // There are more short args, revist the current short args skipping the subcommand + if !done { + it.previous(); + self.skip_idxs = self.cur_idx.get(); + } + self.parse_subcommand(name, matcher, it, !done)?; + external_subcommand = true; + continue; + } + ParseResult::FlagSubCommand(_) => unreachable!(), _ => (), } } @@ -723,7 +749,7 @@ where .expect(INTERNAL_ERROR_MSG) .name .clone(); - self.parse_subcommand(&sc_name, matcher, it)?; + self.parse_subcommand(&sc_name, matcher, it, false)?; } else if self.is_set(AS::SubcommandRequired) { let bn = self.app.bin_name.as_ref().unwrap_or(&self.app.name); return Err(ClapError::missing_subcommand( @@ -969,6 +995,7 @@ where sc_name: &str, matcher: &mut ArgMatcher, it: &mut Input, + keep_state: bool, ) -> ClapResult<()> { use std::fmt::Write; @@ -1019,6 +1046,12 @@ where { let mut p = Parser::new(sc); + // HACK: maintain indexes between parsers + // FlagSubCommand short arg needs to revist the current short args, but skip the subcommand itself + if keep_state { + p.cur_idx.set(self.cur_idx.get()); + p.skip_idxs = self.skip_idxs; + } p.get_matches_with(&mut sc_matcher, it)?; } let name = &sc.name; @@ -1140,6 +1173,8 @@ where self.parse_flag(opt, matcher)?; return Ok(ParseResult::Flag(opt.id.clone())); + } else if let Some(sc_name) = find_long_subcmd!(self.app, arg) { + return Ok(ParseResult::FlagSubCommand(sc_name.to_string())); } else if self.is_set(AS::AllowLeadingHyphen) { return Ok(ParseResult::MaybeHyphenValue); } else if self.is_set(AS::ValidNegNumFound) { @@ -1178,7 +1213,9 @@ where } let mut ret = ParseResult::NotFound; - for c in arg.chars() { + let skip = self.skip_idxs; + self.skip_idxs = 0; + for c in arg.chars().skip(skip) { debug!("Parser::parse_short_arg:iter:{}", c); // update each index because `-abcd` is four indices to clap @@ -1223,6 +1260,11 @@ where // Default to "we're expecting a value later" return self.parse_opt(&val, opt, false, matcher); + } else if let Some(sc_name) = find_short_subcmd!(self.app, c) { + debug!("Parser::parse_short_arg:iter:{}: subcommand={}", c, sc_name); + let name = sc_name.to_string(); + let done_short_args = self.cur_idx.get() == arg.len(); + return Ok(ParseResult::FlagSubCommandShort(name, done_short_args)); } else { let arg = format!("-{}", c); From a9091d141c950d4b0628a1bab584dd3c27c059cc Mon Sep 17 00:00:00 2001 From: NickHackman Date: Sat, 13 Jun 2020 18:02:05 -0400 Subject: [PATCH 04/30] chore: removed debug print Removed a `debug!` line that referenced me. --- src/output/help.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/output/help.rs b/src/output/help.rs index e481870af28..b6a5af523c0 100644 --- a/src/output/help.rs +++ b/src/output/help.rs @@ -562,7 +562,6 @@ impl<'b, 'c, 'd, 'w> Help<'b, 'c, 'd, 'w> { let h_w = str_width(h) + str_width(&*spec_vals); let nlh = self.next_line_help; let taken = self.longest + 12; - debug!("@NickHackman taken = {}", taken); self.force_next_line = !nlh && self.term_w >= taken && (taken as f32 / self.term_w as f32) > 0.40 From 458736bee8c0b051f3084104a4ce6523c13173bc Mon Sep 17 00:00:00 2001 From: NickHackman Date: Mon, 15 Jun 2020 21:52:36 -0400 Subject: [PATCH 05/30] imp: Improved Flag Subcommand API Instead of a `FlagSubCommand` struct the addition of two simple methods to `App`. `App::long_flag` and `App::short_flag` that cover all the bases of the many methods that were provided in `FlagSubCommand`. This API is far simpler to use and more akin to the present `Arg::long` and `Arg::short`. --- examples/23_flag_subcommands_pacman.rs | 10 +- src/build/app/flag_subcommand.rs | 216 ------------------------- src/build/app/mod.rs | 83 +++++++++- src/build/mod.rs | 2 +- src/lib.rs | 2 +- tests/flag_subcommands.rs | 28 ++-- 6 files changed, 108 insertions(+), 233 deletions(-) delete mode 100644 src/build/app/flag_subcommand.rs diff --git a/examples/23_flag_subcommands_pacman.rs b/examples/23_flag_subcommands_pacman.rs index aa0a694c639..d9ef9de2755 100644 --- a/examples/23_flag_subcommands_pacman.rs +++ b/examples/23_flag_subcommands_pacman.rs @@ -45,7 +45,7 @@ // // Let's make a quick program to illustrate. We'll be using the same example as above but for // brevity sake we won't implement all of the subcommands, only a few. -use clap::{App, AppSettings, Arg, FlagSubCommand}; +use clap::{App, AppSettings, Arg}; fn main() { let matches = App::new("pacman") @@ -67,7 +67,9 @@ fn main() { // $ MyProg --long // // $ MyProg -s - FlagSubCommand::new('Q', "query") + App::new("query") + .short_flag('Q') + .long_flag("query") .about("Query the package database.") .arg( Arg::new("search") @@ -88,7 +90,9 @@ fn main() { // // Only a few of its arguments are implemented below. .subcommand( - FlagSubCommand::new('S', "sync") + App::new("sync") + .short_flag('S') + .long_flag("sync") .about("Synchronize packages.") .arg( Arg::new("search") diff --git a/src/build/app/flag_subcommand.rs b/src/build/app/flag_subcommand.rs deleted file mode 100644 index bcbefdecbef..00000000000 --- a/src/build/app/flag_subcommand.rs +++ /dev/null @@ -1,216 +0,0 @@ -use super::App; - -use crate::util::Id; - -/// Used to create a subcommand that can be used as if it were a Flag or a regular subcommand. -/// -/// # Examples -/// ```no_run -/// # use clap::{App, FlagSubCommand}; -/// -/// let m = App::new("MyProgram") -/// .author("Me, me@mail.com") -/// .version("1.0.2") -/// .about("Explains in brief what the program does") -/// .subcommand( -/// FlagSubCommand::new('S', "sub") -/// .about("Explains purpose of subcommand") -/// ) -/// .after_help("Longer explanation to appear after the options when \ -/// displaying the help information from --help or -h") -/// .get_matches(); -/// -/// // Your program logic starts here... -/// ``` -/// -/// The above example's subcommand can be executed with any one of the following -/// -/// ```sh -/// $ MyProgram subcommand -/// ``` -/// -/// ```sh -/// $ MyProgram -S -/// ``` -/// -/// ```sh -/// $ MyProgram --sub -/// ``` -#[derive(Debug, Clone, Copy)] -pub struct FlagSubCommand {} - -#[allow(clippy::new_ret_no_self)] -impl FlagSubCommand { - /// Creates a Subcommand that can be used either as a short Flag or a full named subcommand - /// - /// # Examples - /// ```no_run - /// # use clap::{App, FlagSubCommand}; - /// - /// let m = App::new("MyProgram") - /// .author("Me, me@mail.com") - /// .version("1.0.2") - /// .about("Explains in brief what the program does") - /// .subcommand( - /// FlagSubCommand::new_short("subcommand", 'S') - /// .about("Explains purpose of subcommand") - /// ) - /// .after_help("Longer explanation to appear after the options when \ - /// displaying the help information from --help or -h") - /// .get_matches(); - /// - /// // Your program logic starts here... - /// ``` - /// - /// The above example's subcommand can be executed with any one of the following - /// - /// ```sh - /// $ MyProgram subcommand - /// ``` - /// - /// ```sh - /// $ MyProgram -S - /// ``` - pub fn new_short<'b, S: Into>(n: S, short: char) -> App<'b> { - let name = n.into(); - App { - id: Id::from(&*name), - name, - short: Some(short), - ..Default::default() - } - } - - /// Creates a Subcommand that can be used either as a long Flag or a full named subcommand - /// - /// # Examples - /// ```no_run - /// # use clap::{App, FlagSubCommand}; - /// - /// let m = App::new("MyProgram") - /// .author("Me, me@mail.com") - /// .version("1.0.2") - /// .about("Explains in brief what the program does") - /// .subcommand( - /// FlagSubCommand::new_long("subcommand", "sub") - /// .about("Explains purpose of subcommand") - /// ) - /// .after_help("Longer explanation to appear after the options when \ - /// displaying the help information from --help or -h") - /// .get_matches(); - /// - /// // Your program logic starts here... - /// ``` - /// - /// The above example's subcommand can be executed with any one of the following - /// - /// ```sh - /// $ MyProgram subcommand - /// ``` - /// - /// ```sh - /// $ MyProgram --sub - /// ``` - pub fn new_long>(n: S, long: &str) -> App { - let name = n.into(); - App { - id: Id::from(&*name), - name, - long: Some(long), - ..Default::default() - } - } - - /// Used to create a subcommand that can be used as if it were a Flag or a regular subcommand. - /// - /// NOTE: `long` parameter is used as the name of the subcommand - /// - /// # Examples - /// ```no_run - /// # use clap::{App, FlagSubCommand}; - /// - /// let m = App::new("MyProgram") - /// .author("Me, me@mail.com") - /// .version("1.0.2") - /// .about("Explains in brief what the program does") - /// .subcommand( - /// FlagSubCommand::new('S', "subcommand") - /// .about("Explains purpose of subcommand") - /// ) - /// .after_help("Longer explanation to appear after the options when \ - /// displaying the help information from --help or -h") - /// .get_matches(); - /// - /// // Your program logic starts here... - /// ``` - /// - /// The above example's subcommand can be executed with any one of the following - /// - /// ```sh - /// $ MyProgram subcommand - /// ``` - /// - /// ```sh - /// $ MyProgram -S - /// ``` - /// - /// ```sh - /// $ MyProgram --subcommand - /// ``` - pub fn new(short: char, long: &str) -> App { - let name = long.to_string(); - App { - id: Id::from(long), - name, - short: Some(short), - long: Some(long), - ..Default::default() - } - } - - /// Used to create a subcommand that can be used as if it were a Flag or a regular subcommand. - /// Where the name differs from the long flag. - /// - /// # Examples - /// ```no_run - /// # use clap::{App, FlagSubCommand}; - /// - /// let m = App::new("MyProgram") - /// .author("Me, me@mail.com") - /// .version("1.0.2") - /// .about("Explains in brief what the program does") - /// .subcommand( - /// FlagSubCommand::with_name("subcommand2", 'S', "subcommand") - /// .about("Explains purpose of subcommand") - /// ) - /// .after_help("Longer explanation to appear after the options when \ - /// displaying the help information from --help or -h") - /// .get_matches(); - /// - /// // Your program logic starts here... - /// ``` - /// - /// The above example's subcommand can be executed with any one of the following - /// - /// ```sh - /// $ MyProgram subcommand2 - /// ``` - /// - /// ```sh - /// $ MyProgram -S - /// ``` - /// - /// ```sh - /// $ MyProgram --subcommand - /// ``` - pub fn with_name>(n: S, short: char, long: &str) -> App { - let name = n.into(); - App { - id: Id::from(&*name), - name, - short: Some(short), - long: Some(long), - ..Default::default() - } - } -} diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index ccb2dfbf0f4..8b953fab7d9 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -1,9 +1,8 @@ -mod flag_subcommand; mod settings; #[cfg(test)] mod tests; -pub use self::{flag_subcommand::FlagSubCommand, settings::AppSettings}; +pub use self::settings::AppSettings; // Std use std::{ @@ -360,6 +359,86 @@ impl<'b> App<'b> { self } + /// Allows the subcommand to be used as if it were an [`Arg::short`] + /// + /// Sets the short version of the subcommand flag without the preceeding `-`. + /// + /// # Examples + /// + /// ``` + /// # use clap::{App, Arg}; + /// App::new("pacman").subcommand( + /// App::new("sync") + /// .short_flag('S'), + /// ) + /// # ; + /// ``` + /// + /// ``` + /// # use clap::{App, Arg}; + /// let matches = App::new("pacman") + /// .subcommand( + /// App::new("sync").short_flag('S').arg( + /// Arg::new("search") + /// .short('s') + /// .long("search") + /// .about("search remote repositories for matching strings"), + /// ), + /// ) + /// .get_matches_from(vec!["pacman", "-Ss"]); + /// + /// assert_eq!(matches.subcommand_name().unwrap(), "sync"); + /// let sync_matches = matches.subcommand_matches("sync").unwrap(); + /// assert!(sync_matches.is_present("search")); + /// ``` + pub fn short_flag(mut self, short: char) -> Self { + self.short = Some(short); + self + } + + /// Allows the subcommand to be used as if it were an [`Arg::long`] + /// + /// Sets the long version of the subcommand flag without the preceeding `--`. + /// + /// **NOTE:** Any leading `-` characters will be stripped + /// + /// # Examples + /// + /// To set `long_flag` use a word containing valid UTF-8 codepoints. If you supply a double leading + /// `--` such as `--sync` they will be stripped. Hyphens in the middle of the word; however, + /// will *not* be stripped (i.e. `sync-file` is allowed) + /// + /// ``` + /// # use clap::{App, Arg}; + /// App::new("pacman").subcommand( + /// App::new("sync") + /// .long_flag("sync"), + /// ) + /// # ; + /// ``` + /// + /// ``` + /// # use clap::{App, Arg}; + /// let matches = App::new("pacman") + /// .subcommand( + /// App::new("sync").short_flag('S').arg( + /// Arg::new("search") + /// .short('s') + /// .long("search") + /// .about("search remote repositories for matching strings"), + /// ), + /// ) + /// .get_matches_from(vec!["pacman", "-Ss"]); + /// + /// assert_eq!(matches.subcommand_name().unwrap(), "sync"); + /// let sync_matches = matches.subcommand_matches("sync").unwrap(); + /// assert!(sync_matches.is_present("search")); + /// ``` + pub fn long_flag(mut self, long: &'b str) -> Self { + self.long = Some(long.trim_start_matches(|c| c == '-')); + self + } + /// Sets a string of the version number to be displayed when displaying version or help /// information with `-V`. /// diff --git a/src/build/mod.rs b/src/build/mod.rs index 3eb184a03a1..9ed76dacb2c 100644 --- a/src/build/mod.rs +++ b/src/build/mod.rs @@ -8,7 +8,7 @@ mod arg_group; mod usage_parser; pub use self::{ - app::{App, AppSettings, FlagSubCommand}, + app::{App, AppSettings}, arg::{Arg, ArgSettings}, arg_group::ArgGroup, }; diff --git a/src/lib.rs b/src/lib.rs index cd4279fc826..7c628cfaed8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,7 +22,7 @@ compile_error!("`std` feature is currently required to build `clap`"); pub use crate::{ - build::{App, AppSettings, Arg, ArgGroup, ArgSettings, FlagSubCommand}, + build::{App, AppSettings, Arg, ArgGroup, ArgSettings}, derive::{ArgEnum, Clap, FromArgMatches, IntoApp, Subcommand}, parse::errors::{Error, ErrorKind, Result}, parse::{ArgMatches, Indices, OsValues, Values}, diff --git a/tests/flag_subcommands.rs b/tests/flag_subcommands.rs index f5f6797ca30..447b3767555 100644 --- a/tests/flag_subcommands.rs +++ b/tests/flag_subcommands.rs @@ -1,10 +1,10 @@ -use clap::{App, Arg, FlagSubCommand}; +use clap::{App, Arg}; #[test] fn flag_subcommand_normal() { let matches = App::new("test") .subcommand( - FlagSubCommand::new('S', "some").arg( + App::new("some").short_flag('S').long_flag("some").arg( Arg::new("test") .short('t') .long("test") @@ -21,7 +21,9 @@ fn flag_subcommand_normal() { fn flag_subcommand_normal_with_alias() { let matches = App::new("test") .subcommand( - FlagSubCommand::new('S', "some") + App::new("some") + .short_flag('S') + .long_flag("S") .arg( Arg::new("test") .short('t') @@ -40,7 +42,7 @@ fn flag_subcommand_normal_with_alias() { fn flag_subcommand_short() { let matches = App::new("test") .subcommand( - FlagSubCommand::new_short("some", 'S').arg( + App::new("some").short_flag('S').arg( Arg::new("test") .short('t') .long("test") @@ -57,7 +59,7 @@ fn flag_subcommand_short() { fn flag_subcommand_short_with_args() { let matches = App::new("test") .subcommand( - FlagSubCommand::new_short("some", 'S').arg( + App::new("some").short_flag('S').arg( Arg::new("test") .short('t') .long("test") @@ -74,7 +76,8 @@ fn flag_subcommand_short_with_args() { fn flag_subcommand_short_with_alias() { let matches = App::new("test") .subcommand( - FlagSubCommand::new_short("some", 'S') + App::new("some") + .short_flag('S') .arg( Arg::new("test") .short('t') @@ -94,7 +97,7 @@ fn flag_subcommand_short_with_alias() { fn flag_subcommand_long() { let matches = App::new("test") .subcommand( - FlagSubCommand::new_long("some", "some").arg( + App::new("some").long_flag("some").arg( Arg::new("test") .short('t') .long("test") @@ -111,7 +114,8 @@ fn flag_subcommand_long() { fn flag_subcommand_long_with_alias() { let matches = App::new("test") .subcommand( - FlagSubCommand::new_long("some", "some") + App::new("some") + .long_flag("some") .arg( Arg::new("test") .short('t') @@ -130,11 +134,15 @@ fn flag_subcommand_long_with_alias() { fn flag_subcommand_multiple() { let matches = App::new("test") .subcommand( - FlagSubCommand::new('S', "some") + App::new("some") + .short_flag('S') + .long_flag("some") .arg(Arg::from("-f, --flag 'some flag'")) .arg(Arg::from("-p, --print 'print something'")) .subcommand( - FlagSubCommand::new('R', "result") + App::new("result") + .short_flag('R') + .long_flag("result") .arg(Arg::from("-f, --flag 'some flag'")) .arg(Arg::from("-p, --print 'print something'")), ), From acb2f755f81518a17e1549852b5fd0b5a312ae86 Mon Sep 17 00:00:00 2001 From: NickHackman Date: Tue, 16 Jun 2020 00:13:21 -0400 Subject: [PATCH 06/30] api: short_flag_aliases and corresponding methods Aliases exclusively for `App::short_flag` that allow visible and hidden short flag subcommands. --- src/build/app/mod.rs | 118 +++++++++++++++++++++++++++++++++++++++++++ src/macros.rs | 6 +-- 2 files changed, 121 insertions(+), 3 deletions(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 8b953fab7d9..898d4df1f05 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -83,6 +83,7 @@ pub struct App<'b> { pub(crate) before_help: Option<&'b str>, pub(crate) after_help: Option<&'b str>, pub(crate) aliases: Vec<(&'b str, bool)>, // (name, visible) + pub(crate) short_flag_aliases: Vec<(char, bool)>, // (name, visible) pub(crate) usage_str: Option<&'b str>, pub(crate) usage: Option, pub(crate) help_str: Option<&'b str>, @@ -129,12 +130,27 @@ impl<'b> App<'b> { self.aliases.iter().filter(|(_, vis)| *vis).map(|a| a.0) } + /// Iterate through the *visible* short aliases for this subcommand. + #[inline] + pub fn get_visible_short_aliases(&self) -> impl Iterator + '_ { + self.short_flag_aliases + .iter() + .filter(|(_, vis)| *vis) + .map(|a| a.0) + } + /// Iterate through the set of *all* the aliases for this subcommand, both visible and hidden. #[inline] pub fn get_all_aliases(&self) -> impl Iterator { self.aliases.iter().map(|a| a.0) } + /// Iterate through the set of *all* the short aliases for this subcommand, both visible and hidden. + #[inline] + pub fn get_all_short_aliases(&self) -> impl Iterator + '_ { + self.short_flag_aliases.iter().map(|a| a.0) + } + /// Get the list of subcommands #[inline] pub fn get_subcommands(&self) -> &[App<'b>] { @@ -844,6 +860,30 @@ impl<'b> App<'b> { self } + /// Allows adding a [``] alias, which function as "hidden" short flag subcommands that + /// automatically dispatch as if this subcommand was used. This is more efficient, and easier + /// than creating multiple hidden subcommands as one only needs to check for the existence of + /// this command, and not all variants. + /// + /// # Examples + /// + /// ```no_run + /// # use clap::{App, Arg, }; + /// let m = App::new("myprog") + /// .subcommand(App::new("test").short_flag('t') + /// .short_flag_alias('d')) + /// .get_matches_from(vec!["myprog", "d"]); + /// assert_eq!(m.subcommand_name(), Some("test")); + /// ``` + /// [``]: ./struct..html + pub fn short_flag_alias(mut self, name: char) -> Self { + if name == '-' { + panic!("short alias name cannot be `-`"); + } + self.short_flag_aliases.push((name, false)); + self + } + /// Allows adding [``] aliases, which function as "hidden" subcommands that /// automatically dispatch as if this subcommand was used. This is more efficient, and easier /// than creating multiple hidden subcommands as one only needs to check for the existence of @@ -869,6 +909,36 @@ impl<'b> App<'b> { self } + /// Allows adding [``] aliases, which function as "hidden" short flag subcommands that + /// automatically dispatch as if this subcommand was used. This is more efficient, and easier + /// than creating multiple hidden subcommands as one only needs to check for the existence of + /// this command, and not all variants. + /// + /// # Examples + /// + /// ```rust + /// # use clap::{App, Arg, }; + /// let m = App::new("myprog") + /// .subcommand(App::new("test").short_flag('t') + /// .short_flag_aliases(&['a', 'b', 'c'])) + /// .arg(Arg::new("input") + /// .about("the file to add") + /// .index(1) + /// .required(false)) + /// .get_matches_from(vec!["myprog", "-a"]); + /// assert_eq!(m.subcommand_name(), Some("test")); + /// ``` + /// [``]: ./struct..html + pub fn short_flag_aliases(mut self, names: &[char]) -> Self { + for s in names { + if s == &'-' { + panic!("short alias name cannot be `-`"); + } + self.short_flag_aliases.push((*s, false)); + } + self + } + /// Allows adding a [``] alias that functions exactly like those defined with /// [`App::alias`], except that they are visible inside the help message. /// @@ -889,6 +959,29 @@ impl<'b> App<'b> { self } + /// Allows adding a [``] alias that functions exactly like those defined with + /// [`App::short_flag_alias`], except that they are visible inside the help message. + /// + /// # Examples + /// + /// ```no_run + /// # use clap::{App, Arg, }; + /// let m = App::new("myprog") + /// .subcommand(App::new("test").short_flag('t') + /// .visible_short_flag_alias('d')) + /// .get_matches_from(vec!["myprog", "-d"]); + /// assert_eq!(m.subcommand_name(), Some("test")); + /// ``` + /// [``]: ./struct..html + /// [`App::short_flag_alias`]: ./struct.App.html#method.short_flag_alias + pub fn visible_short_flag_alias(mut self, name: char) -> Self { + if name == '-' { + panic!("short alias name cannot be `-`"); + } + self.short_flag_aliases.push((name, true)); + self + } + /// Allows adding multiple [``] aliases that functions exactly like those defined /// with [`App::aliases`], except that they are visible inside the help message. /// @@ -909,6 +1002,31 @@ impl<'b> App<'b> { self } + /// Allows adding multiple [``] short flag aliases that functions exactly like those defined + /// with [`App::short_flag_aliases`], except that they are visible inside the help message. + /// + /// # Examples + /// + /// ```no_run + /// # use clap::{App, Arg, }; + /// let m = App::new("myprog") + /// .subcommand(App::new("test").short_flag('b') + /// .visible_short_flag_aliases(&['t'])) + /// .get_matches_from(vec!["myprog", "-t"]); + /// assert_eq!(m.subcommand_name(), Some("test")); + /// ``` + /// [``]: ./struct..html + /// [`App::short_flag_aliases`]: ./struct.App.html#method.short_flag_aliases + pub fn visible_short_flag_aliases(mut self, names: &[char]) -> Self { + for s in names { + if s == &'-' { + panic!("short alias name cannot be `-`"); + } + self.short_flag_aliases.push((*s, false)); + } + self + } + /// Replaces an argument to this application with other arguments. /// /// Below, when the given args are `app install`, they will be changed to `app module install`. diff --git a/src/macros.rs b/src/macros.rs index 430c5f8ce4e..6baf876acad 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -634,9 +634,9 @@ macro_rules! find_subcmd_mut { #[doc(hidden)] macro_rules! find_short_subcmd { ($app:expr, $c:expr) => {{ - $app.get_subcommands().iter().find(|sc| { - sc.short == Some($c) || sc.get_all_aliases().any(|alias| alias == $c.to_string()) - }) + $app.get_subcommands() + .iter() + .find(|sc| sc.short == Some($c) || sc.get_all_short_aliases().any(|alias| alias == $c)) }}; } From 383606e099329a6589d6b58abea7ce685ccde458 Mon Sep 17 00:00:00 2001 From: NickHackman Date: Tue, 16 Jun 2020 00:15:47 -0400 Subject: [PATCH 07/30] tests: `short_flag_alias` methods Tests that cover all methods for `short_flag_alias` corresponding methods --- tests/flag_subcommands.rs | 137 +++++++++++++++++++++++++++++++++++++- 1 file changed, 135 insertions(+), 2 deletions(-) diff --git a/tests/flag_subcommands.rs b/tests/flag_subcommands.rs index 447b3767555..dc49ed6d670 100644 --- a/tests/flag_subcommands.rs +++ b/tests/flag_subcommands.rs @@ -84,8 +84,8 @@ fn flag_subcommand_short_with_alias() { .long("test") .about("testing testing"), ) - .alias("M") - .alias("B"), + .short_flag_alias('M') + .short_flag_alias('B'), ) .get_matches_from(vec!["myprog", "-Bt"]); assert_eq!(matches.subcommand_name().unwrap(), "some"); @@ -93,6 +93,89 @@ fn flag_subcommand_short_with_alias() { assert!(sub_matches.is_present("test")); } +#[test] +fn flag_subcommand_short_with_aliases_vis_and_hidden() { + let app = App::new("test").subcommand( + App::new("some") + .short_flag('S') + .arg( + Arg::new("test") + .short('t') + .long("test") + .about("testing testing"), + ) + .visible_short_flag_aliases(&['M', 'B']) + .short_flag_alias('C'), + ); + let app1 = app.clone(); + let matches1 = app1.get_matches_from(vec!["test", "-M"]); + assert_eq!(matches1.subcommand_name().unwrap(), "some"); + + let app2 = app.clone(); + let matches2 = app2.get_matches_from(vec!["test", "-C"]); + assert_eq!(matches2.subcommand_name().unwrap(), "some"); + + let app3 = app.clone(); + let matches3 = app3.get_matches_from(vec!["test", "-B"]); + assert_eq!(matches3.subcommand_name().unwrap(), "some"); +} + +#[test] +fn flag_subcommand_short_with_aliases() { + let matches = App::new("test") + .subcommand( + App::new("some") + .short_flag('S') + .arg( + Arg::new("test") + .short('t') + .long("test") + .about("testing testing"), + ) + .short_flag_aliases(&['M', 'B']), + ) + .get_matches_from(vec!["myprog", "-Bt"]); + assert_eq!(matches.subcommand_name().unwrap(), "some"); + let sub_matches = matches.subcommand_matches("some").unwrap(); + assert!(sub_matches.is_present("test")); +} + +#[test] +#[should_panic] +fn flag_subcommand_short_with_alias_hyphen() { + let _ = App::new("test") + .subcommand( + App::new("some") + .short_flag('S') + .arg( + Arg::new("test") + .short('t') + .long("test") + .about("testing testing"), + ) + .short_flag_alias('-'), + ) + .get_matches_from(vec!["myprog", "-Bt"]); +} + +#[test] +#[should_panic] +fn flag_subcommand_short_with_aliases_hyphen() { + let _ = App::new("test") + .subcommand( + App::new("some") + .short_flag('S') + .arg( + Arg::new("test") + .short('t') + .long("test") + .about("testing testing"), + ) + .short_flag_aliases(&['-', '-', '-']), + ) + .get_matches_from(vec!["myprog", "-Bt"]); +} + #[test] fn flag_subcommand_long() { let matches = App::new("test") @@ -157,3 +240,53 @@ fn flag_subcommand_multiple() { assert!(result_matches.is_present("flag")); assert!(result_matches.is_present("print")); } + +// #[test] +// #[should_panic] +// fn flag_subcommand_short_conflict_with_arg() { +// let matches = App::new("test") +// .subcommand( +// App::new("some") +// .short_flag('f') +// .long_flag("some") +// .arg(Arg::from("-f, --flag 'some flag'")), +// ) +// .get_matches_from(vec!["myprog", "-ff"]); +// } + +// #[test] +// #[should_panic] +// fn flag_subcommand_long_conflict_with_arg() { +// let matches = App::new("test") +// .subcommand( +// App::new("some") +// .short_flag('a') +// .long_flag("flag") +// .arg(Arg::from("-f, --flag 'some flag'")), +// ) +// .get_matches_from(vec!["myprog", "--flag", "--flag"]); +// } + +// #[test] +// fn flag_subcommand_conflict_with_help() { +// let matches = App::new("test") +// .subcommand( +// App::new("halp") +// .short_flag('h') +// .long_flag("help") +// .arg(Arg::from("-f, --flag 'some flag'")), +// ) +// .get_matches_from(vec!["myprog", "--help"]); +// } + +// #[test] +// fn flag_subcommand_conflict_with_version() { +// let matches = App::new("test") +// .subcommand( +// App::new("ver") +// .short_flag('V') +// .long_flag("version") +// .arg(Arg::from("-f, --flag 'some flag'")), +// ) +// .get_matches_from(vec!["myprog", "--version"]); +// } From 02ebf4f816481b6e1dfc63085dd2b7861c5d48e4 Mon Sep 17 00:00:00 2001 From: NickHackman Date: Tue, 16 Jun 2020 01:13:58 -0400 Subject: [PATCH 08/30] fix: display visible short flags, visible flags Flags weren't being set as visible in one instance. Flags were not being printed in help message. --- src/build/app/mod.rs | 2 +- src/output/help.rs | 27 +++++++++++++++++---------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 898d4df1f05..f39a96ffa2e 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -1022,7 +1022,7 @@ impl<'b> App<'b> { if s == &'-' { panic!("short alias name cannot be `-`"); } - self.short_flag_aliases.push((*s, false)); + self.short_flag_aliases.push((*s, true)); } self } diff --git a/src/output/help.rs b/src/output/help.rs index b6a5af523c0..3fce6f20b98 100644 --- a/src/output/help.rs +++ b/src/output/help.rs @@ -576,19 +576,26 @@ impl<'b, 'c, 'd, 'w> Help<'b, 'c, 'd, 'w> { fn sc_spec_vals(&self, a: &App) -> String { debug!("Help::sc_spec_vals: a={}", a.name); let mut spec_vals = vec![]; - if !a.aliases.is_empty() { + if !a.aliases.is_empty() || !a.short_flag_aliases.is_empty() { debug!("Help::spec_vals: Found aliases...{:?}", a.aliases); + debug!( + "Help::spec_vals: Found short flag aliases...{:?}", + a.short_flag_aliases + ); - let als = a - .aliases - .iter() - .filter(|&als| als.1) // visible - .map(|&als| als.0) // name - .collect::>() - .join(", "); + let mut short_als = a + .get_visible_short_aliases() + .map(|a| format!("-{}", a)) + .collect::>(); - if !als.is_empty() { - spec_vals.push(format!(" [aliases: {}]", als)); + let als = a.get_visible_aliases().map(|s| s.to_string()); + + short_als.extend(als); + + let all_als = short_als.join(", "); + + if !all_als.is_empty() { + spec_vals.push(format!(" [aliases: {}]", all_als)); } } spec_vals.join(" ") From 806d7487c697e233fa63403457296e75191953b9 Mon Sep 17 00:00:00 2001 From: NickHackman Date: Tue, 16 Jun 2020 20:01:19 -0400 Subject: [PATCH 09/30] example: updated 23 docs & refractor Improved printing instead of printing out the `Vec` using `{:?}`, `join(", ")` for improved printing. Improved documentation to b emore explicit and apply to removal of `FlagSubCommand` -> `App::short_flag` and `App::long_flag` --- examples/23_flag_subcommands_pacman.rs | 34 ++++++++++++-------------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/examples/23_flag_subcommands_pacman.rs b/examples/23_flag_subcommands_pacman.rs index d9ef9de2755..4bc48d012b3 100644 --- a/examples/23_flag_subcommands_pacman.rs +++ b/examples/23_flag_subcommands_pacman.rs @@ -3,7 +3,7 @@ // // It's suggested that you read examples/20_subcommands.rs prior to learning about `FlagSubCommand`s // -// This differs from normal subcommands because it allows passing subcommand as `clap::Arg` in short or long args. +// This differs from normal subcommands because it allows passing subcommands in the same fashion as `clap::Arg` in short or long args. // // Top Level App (pacman) TOP // | @@ -20,7 +20,7 @@ // // $ pacman -Rns // -// NOTE: Subcommands short flags don't have to be uppercase. +// NOTE: Subcommands short flags can be uppercase or lowercase. // // $ pacman --sync --search // ^--- subcommand @@ -28,7 +28,7 @@ // $ pacman sync -s // ^--- subcommand // -// NOTE: this isn't valid for pacman, but `clap::FlagSubCommand` +// NOTE: this isn't valid for pacman, but is done implicitly by Clap which // adds support for both flags and standard subcommands out of the box. // Allowing your users to make the choice of what feels more intuitive for them. // @@ -57,16 +57,6 @@ fn main() { // // Only a few of its arguments are implemented below. .subcommand( - // When getting the subcommand name the long version is used (in this case "query"). - // If you want to use a different name then `FlagSubCommand::with_name("name", 's', "long")`. - // - // NOTE: if the name has been changed then it can be used as that: - // - // $ MyProg name - // - // $ MyProg --long - // - // $ MyProg -s App::new("query") .short_flag('Q') .long_flag("query") @@ -76,6 +66,7 @@ fn main() { .short('s') .long("search") .about("search locally-installed packages for matching strings") + .multiple(true) .takes_value(true), ) .arg( @@ -99,6 +90,7 @@ fn main() { .short('s') .long("search") .about("search remote repositories for matching strings") + .multiple(true) .takes_value(true), ) .arg( @@ -122,16 +114,20 @@ fn main() { if sync_matches.is_present("info") { // Values required here, so it's safe to unwrap let packages: Vec<_> = sync_matches.values_of("info").unwrap().collect(); - println!("Retrieving info for {:?}...", packages); + let comma_sep = packages.join(", "); + println!("Retrieving info for {}...", comma_sep); } else if sync_matches.is_present("search") { // Values required here, so it's safe to unwrap let queries: Vec<_> = sync_matches.values_of("search").unwrap().collect(); - println!("Searching for {:?}...", queries); + let comma_sep = queries.join(", "); + println!("Searching for {}...", comma_sep); } else { + // Sync was called without any arguments match sync_matches.values_of("package") { Some(packages) => { let pkgs: Vec<_> = packages.collect(); - println!("Installing {:?}...", pkgs); + let comma_sep = pkgs.join(", "); + println!("Installing {}...", comma_sep); } None => panic!("No targets specified (use -h for help)"), } @@ -141,11 +137,13 @@ fn main() { if query_matches.is_present("info") { // Values required here, so it's safe to unwrap let packages: Vec<_> = query_matches.values_of("info").unwrap().collect(); - println!("Retrieving info for {:?}...", packages); + let comma_sep = packages.join(", "); + println!("Retrieving info for {}...", comma_sep); } else if query_matches.is_present("search") { // Values required here, so it's safe to unwrap let queries: Vec<_> = query_matches.values_of("search").unwrap().collect(); - println!("Searching Locally for {:?}...", queries); + let comma_sep = queries.join(", "); + println!("Searching Locally for {}...", comma_sep); } else { // Query was called without any arguments println!("Displaying all locally installed packages..."); From bf3d947f017f19f699560ab5900902b0726018fa Mon Sep 17 00:00:00 2001 From: NickHackman Date: Tue, 16 Jun 2020 23:45:21 -0400 Subject: [PATCH 10/30] fix: Flag Subcommands conflicts panic Conflicts with Flag subcommands and Args now panics when `debug_assertions` is enabled, rather than causing bizarre behavior. --- src/build/app/mod.rs | 97 ++++++++++++++++++++++++++++++++++++++- tests/flag_subcommands.rs | 74 +++++++++++------------------ 2 files changed, 123 insertions(+), 48 deletions(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index f39a96ffa2e..042bd1b0c2b 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -107,6 +107,18 @@ impl<'b> App<'b> { &self.name } + /// Get the short flag of the subcommand + #[inline] + pub fn get_short_flag(&self) -> Option { + self.short + } + + /// Get the long flag of the subcommand + #[inline] + pub fn get_long_flag(&self) -> Option<&str> { + self.long + } + /// Get the name of the binary #[inline] pub fn get_bin_name(&self) -> Option<&str> { @@ -1648,6 +1660,40 @@ impl<'b> App<'b> { } } +// Allows checking for conflicts between `Args` and `Apps` with subcommand flags +#[cfg(debug_assertions)] +#[derive(Debug)] +enum Flag<'a> { + App(App<'a>), + Arg(Arg<'a>), +} + +#[cfg(debug_assertions)] +impl<'a> Flag<'_> { + fn short(&self) -> Option { + match self { + Self::App(app) => app.short, + Self::Arg(arg) => arg.short, + } + } + + fn long(&self) -> Option<&str> { + match self { + Self::App(app) => app.long, + Self::Arg(arg) => arg.long, + } + } +} + +impl<'a> fmt::Display for Flag<'_> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::App(app) => write!(f, "App named '{}'", app.get_name()), + Self::Arg(arg) => write!(f, "Arg named '{}'", arg.get_name()), + } + } +} + // Internally used only impl<'b> App<'b> { fn _do_parse(&mut self, it: &mut Input) -> ClapResult { @@ -1749,6 +1795,21 @@ impl<'b> App<'b> { } } + // Checks for conflicts in `Arg::long`/`App::long` or `Arg::short`/`App::short` + #[cfg(debug_assertions)] + fn two_flags_of(&self, condition: F) -> Option<(Flag, Flag)> + where + F: Fn(&Flag<'_>) -> bool, + { + let mut flags: Vec<_> = self + .subcommands + .iter() + .map(|a| Flag::App(a.clone())) + .collect(); + flags.extend(self.args.args.iter().map(|a| Flag::Arg(a.clone()))); + two_elements_of(flags.into_iter().filter(|f| condition(f))) + } + #[cfg(debug_assertions)] fn two_args_of(&self, condition: F) -> Option<(&Arg, &Arg)> where @@ -1772,6 +1833,30 @@ impl<'b> App<'b> { fn _debug_asserts(&self) { debug!("App::_debug_asserts"); + for sc in &self.subcommands { + // Conflicts between flag subcommands and long args + if let Some(l) = sc.long { + if let Some((first, second)) = self.two_flags_of(|f| f.long() == Some(l)) { + panic!( + "Long option names must be unique for each argument, \ + but '--{}' is used by both an {} and an {}", + l, first, second + ); + } + } + + // Conflicts between flag subcommands and short args + if let Some(s) = sc.short { + if let Some((first, second)) = self.two_flags_of(|f| f.short() == Some(s)) { + panic!( + "Short option names must be unique for each argument, \ + but '-{}' is used by both an {} and an {}", + s, first, second + ); + } + } + } + for arg in &self.args.args { arg._debug_asserts(); @@ -1971,7 +2056,11 @@ impl<'b> App<'b> { .args .iter() .any(|x| x.long == Some("help") || x.id == Id::help_hash()) - || self.is_set(AppSettings::DisableHelpFlags)) + || self.is_set(AppSettings::DisableHelpFlags) + || self + .subcommands + .iter() + .any(|sc| sc.short == Some('h') || sc.long == Some("help"))) { debug!("App::_create_help_and_version: Building --help"); let mut help = Arg::new("help") @@ -1988,7 +2077,11 @@ impl<'b> App<'b> { .args .iter() .any(|x| x.long == Some("version") || x.id == Id::version_hash()) - || self.is_set(AppSettings::DisableVersion)) + || self.is_set(AppSettings::DisableVersion) + || self + .subcommands + .iter() + .any(|sc| sc.short == Some('V') || sc.long == Some("version"))) { debug!("App::_create_help_and_version: Building --version"); let mut version = Arg::new("version") diff --git a/tests/flag_subcommands.rs b/tests/flag_subcommands.rs index dc49ed6d670..efb104819c7 100644 --- a/tests/flag_subcommands.rs +++ b/tests/flag_subcommands.rs @@ -241,52 +241,34 @@ fn flag_subcommand_multiple() { assert!(result_matches.is_present("print")); } -// #[test] -// #[should_panic] -// fn flag_subcommand_short_conflict_with_arg() { -// let matches = App::new("test") -// .subcommand( -// App::new("some") -// .short_flag('f') -// .long_flag("some") -// .arg(Arg::from("-f, --flag 'some flag'")), -// ) -// .get_matches_from(vec!["myprog", "-ff"]); -// } +#[test] +#[should_panic = "Short option names must be unique for each argument, but \'-f\' is used by both an App named \'some\' and an Arg named \'test\'"] +fn flag_subcommand_short_conflict_with_arg() { + let _ = App::new("test") + .subcommand(App::new("some").short_flag('f').long_flag("some")) + .arg(Arg::new("test").short('f')) + .get_matches_from(vec!["myprog", "-ff"]); +} -// #[test] -// #[should_panic] -// fn flag_subcommand_long_conflict_with_arg() { -// let matches = App::new("test") -// .subcommand( -// App::new("some") -// .short_flag('a') -// .long_flag("flag") -// .arg(Arg::from("-f, --flag 'some flag'")), -// ) -// .get_matches_from(vec!["myprog", "--flag", "--flag"]); -// } +#[test] +#[should_panic = "Long option names must be unique for each argument, but \'--flag\' is used by both an App named \'some\' and an Arg named \'flag\'"] +fn flag_subcommand_long_conflict_with_arg() { + let _ = App::new("test") + .subcommand(App::new("some").short_flag('a').long_flag("flag")) + .arg(Arg::new("flag").long("flag")) + .get_matches_from(vec!["myprog", "--flag", "--flag"]); +} -// #[test] -// fn flag_subcommand_conflict_with_help() { -// let matches = App::new("test") -// .subcommand( -// App::new("halp") -// .short_flag('h') -// .long_flag("help") -// .arg(Arg::from("-f, --flag 'some flag'")), -// ) -// .get_matches_from(vec!["myprog", "--help"]); -// } +#[test] +fn flag_subcommand_conflict_with_help() { + let _ = App::new("test") + .subcommand(App::new("help").short_flag('h').long_flag("help")) + .get_matches_from(vec!["myprog", "--help"]); +} -// #[test] -// fn flag_subcommand_conflict_with_version() { -// let matches = App::new("test") -// .subcommand( -// App::new("ver") -// .short_flag('V') -// .long_flag("version") -// .arg(Arg::from("-f, --flag 'some flag'")), -// ) -// .get_matches_from(vec!["myprog", "--version"]); -// } +#[test] +fn flag_subcommand_conflict_with_version() { + let _ = App::new("test") + .subcommand(App::new("ver").short_flag('V').long_flag("version")) + .get_matches_from(vec!["myprog", "--version"]); +} From b09afcf653c20497b4aa62a2758ac54381d808d4 Mon Sep 17 00:00:00 2001 From: NickHackman Date: Wed, 17 Jun 2020 00:09:25 -0400 Subject: [PATCH 11/30] fix: Gate `fmt::Display` for Flag w/ debug_asserts Implementation of `fmt::Display` for `Flag` caused tests to fail when not ran with `debug_assertions` feature. --- src/build/app/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 042bd1b0c2b..21eb0299bfd 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -1685,6 +1685,7 @@ impl<'a> Flag<'_> { } } +#[cfg(debug_assertions)] impl<'a> fmt::Display for Flag<'_> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { From f7c5b098b92d65ecb4dd3414437d40276e6b1989 Mon Sep 17 00:00:00 2001 From: NickHackman Date: Wed, 17 Jun 2020 00:56:05 -0400 Subject: [PATCH 12/30] fix: duplicate flags in Flag Subcommand test --- tests/flag_subcommands.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/flag_subcommands.rs b/tests/flag_subcommands.rs index efb104819c7..4eebdb0e555 100644 --- a/tests/flag_subcommands.rs +++ b/tests/flag_subcommands.rs @@ -256,7 +256,7 @@ fn flag_subcommand_long_conflict_with_arg() { let _ = App::new("test") .subcommand(App::new("some").short_flag('a').long_flag("flag")) .arg(Arg::new("flag").long("flag")) - .get_matches_from(vec!["myprog", "--flag", "--flag"]); + .get_matches_from(vec!["myprog", "--flag"]); } #[test] From 32db427706c5f3f000911bb4854f6a3ec80b5b0b Mon Sep 17 00:00:00 2001 From: NickHackman Date: Wed, 17 Jun 2020 01:14:48 -0400 Subject: [PATCH 13/30] fix: duplicate short flags in Flag Subcommand test --- tests/flag_subcommands.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/flag_subcommands.rs b/tests/flag_subcommands.rs index 4eebdb0e555..e4f4683e64e 100644 --- a/tests/flag_subcommands.rs +++ b/tests/flag_subcommands.rs @@ -247,7 +247,7 @@ fn flag_subcommand_short_conflict_with_arg() { let _ = App::new("test") .subcommand(App::new("some").short_flag('f').long_flag("some")) .arg(Arg::new("test").short('f')) - .get_matches_from(vec!["myprog", "-ff"]); + .get_matches_from(vec!["myprog", "-f"]); } #[test] From d785ebc14e8d467d2387596562da280a19bfc726 Mon Sep 17 00:00:00 2001 From: NickHackman Date: Wed, 17 Jun 2020 01:53:53 -0400 Subject: [PATCH 14/30] fix: Flag Subcommands tests behind debug_asserts Tests that check conflicts are behind the `debug_assertions` feature --- tests/flag_subcommands.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/flag_subcommands.rs b/tests/flag_subcommands.rs index e4f4683e64e..f86f5a1e91d 100644 --- a/tests/flag_subcommands.rs +++ b/tests/flag_subcommands.rs @@ -241,6 +241,7 @@ fn flag_subcommand_multiple() { assert!(result_matches.is_present("print")); } +#[cfg(debug_assertions)] #[test] #[should_panic = "Short option names must be unique for each argument, but \'-f\' is used by both an App named \'some\' and an Arg named \'test\'"] fn flag_subcommand_short_conflict_with_arg() { @@ -250,6 +251,7 @@ fn flag_subcommand_short_conflict_with_arg() { .get_matches_from(vec!["myprog", "-f"]); } +#[cfg(debug_assertions)] #[test] #[should_panic = "Long option names must be unique for each argument, but \'--flag\' is used by both an App named \'some\' and an Arg named \'flag\'"] fn flag_subcommand_long_conflict_with_arg() { From 8a68f9975002f16da072302d96fd629facf0b53a Mon Sep 17 00:00:00 2001 From: NickHackman Date: Tue, 7 Jul 2020 21:26:38 -0400 Subject: [PATCH 15/30] example: more pacman-like and refractor Removed user facing panics and improved ergonomics to behave more like pacman behaves. --- examples/23_flag_subcommands_pacman.rs | 67 +++++++++++--------------- 1 file changed, 29 insertions(+), 38 deletions(-) diff --git a/examples/23_flag_subcommands_pacman.rs b/examples/23_flag_subcommands_pacman.rs index 4bc48d012b3..343865ef0cb 100644 --- a/examples/23_flag_subcommands_pacman.rs +++ b/examples/23_flag_subcommands_pacman.rs @@ -66,15 +66,16 @@ fn main() { .short('s') .long("search") .about("search locally-installed packages for matching strings") - .multiple(true) - .takes_value(true), + .conflicts_with("info") + .multiple_values(true), ) .arg( Arg::new("info") .long("info") .short('i') - .about("view package information (-ii for backup files)") - .multiple(true), + .conflicts_with("search") + .about("view package information") + .multiple_values(true), ), ) // Sync subcommand @@ -89,21 +90,23 @@ fn main() { Arg::new("search") .short('s') .long("search") - .about("search remote repositories for matching strings") - .multiple(true) - .takes_value(true), + .conflicts_with("info") + .takes_value(true) + .multiple_values(true) + .about("search remote repositories for matching strings"), ) .arg( Arg::new("info") .long("info") + .conflicts_with("search") .short('i') - .about("view package information (-ii for extended information)") - .multiple(true), + .about("view package information"), ) .arg( Arg::new("package") - .about("package") + .about("packages") .multiple(true) + .required_unless_one(&["search"]) .takes_value(true), ), ) @@ -111,45 +114,33 @@ fn main() { match matches.subcommand() { ("sync", Some(sync_matches)) => { + if sync_matches.is_present("search") { + let packages: Vec<_> = sync_matches.values_of("search").unwrap().collect(); + let values = packages.join(", "); + println!("Searching for {}...", values); + return; + } + + let packages: Vec<_> = sync_matches.values_of("package").unwrap().collect(); + let values = packages.join(", "); + if sync_matches.is_present("info") { - // Values required here, so it's safe to unwrap - let packages: Vec<_> = sync_matches.values_of("info").unwrap().collect(); - let comma_sep = packages.join(", "); - println!("Retrieving info for {}...", comma_sep); - } else if sync_matches.is_present("search") { - // Values required here, so it's safe to unwrap - let queries: Vec<_> = sync_matches.values_of("search").unwrap().collect(); - let comma_sep = queries.join(", "); - println!("Searching for {}...", comma_sep); + println!("Retrieving info for {}...", values); } else { - // Sync was called without any arguments - match sync_matches.values_of("package") { - Some(packages) => { - let pkgs: Vec<_> = packages.collect(); - let comma_sep = pkgs.join(", "); - println!("Installing {}...", comma_sep); - } - None => panic!("No targets specified (use -h for help)"), - } + println!("Installing {}...", values); } } ("query", Some(query_matches)) => { - if query_matches.is_present("info") { - // Values required here, so it's safe to unwrap - let packages: Vec<_> = query_matches.values_of("info").unwrap().collect(); - let comma_sep = packages.join(", "); + if let Some(packages) = query_matches.values_of("info") { + let comma_sep = packages.collect::>().join(", "); println!("Retrieving info for {}...", comma_sep); - } else if query_matches.is_present("search") { - // Values required here, so it's safe to unwrap - let queries: Vec<_> = query_matches.values_of("search").unwrap().collect(); - let comma_sep = queries.join(", "); + } else if let Some(queries) = query_matches.values_of("search") { + let comma_sep = queries.collect::>().join(", "); println!("Searching Locally for {}...", comma_sep); } else { - // Query was called without any arguments println!("Displaying all locally installed packages..."); } } - ("", None) => panic!("error: no operation specified (use -h for help)"), // If no subcommand was used _ => unreachable!(), // If all subcommands are defined above, anything else is unreachable } } From 5db25e6a72c168bd6dd38c3647587c5ed61fc62a Mon Sep 17 00:00:00 2001 From: NickHackman Date: Tue, 7 Jul 2020 22:26:44 -0400 Subject: [PATCH 16/30] refractor: improved parser ergonomics No longer abusing external_subcommand, but using subcmd_name in order to parse a subcommand, added a `keep_state` variable in order to handle the hacky solution of short command line arguments. --- src/parse/parser.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/parse/parser.rs b/src/parse/parser.rs index e132b734d51..c5b4534581e 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -81,10 +81,6 @@ impl Input { } } - pub(crate) fn previous(&mut self) { - self.cursor -= 1; - } - pub(crate) fn remaining(&self) -> &[OsString] { &self.items[self.cursor..] } @@ -386,6 +382,7 @@ where let has_args = self.has_args(); let mut subcmd_name: Option = None; + let mut keep_state = false; let mut external_subcommand = false; let mut needs_val_of: ParseResult = ParseResult::NotFound; let mut pos_counter = 1; @@ -467,9 +464,8 @@ where } ParseResult::FlagSubCommand(ref name) => { debug!("Parser::get_matches_with: FlagSubCommand found in long arg {:?}", name); - self.parse_subcommand(name, matcher, it, false)?; - external_subcommand = true; - continue; + subcmd_name = Some(name.to_owned()); + break; } ParseResult::FlagSubCommandShort(_, _) => unreachable!(), _ => (), @@ -506,13 +502,14 @@ where } ParseResult::FlagSubCommandShort(ref name, done) => { // There are more short args, revist the current short args skipping the subcommand - if !done { - it.previous(); + keep_state = !done; + if keep_state { + it.cursor -= 1; self.skip_idxs = self.cur_idx.get(); } - self.parse_subcommand(name, matcher, it, !done)?; - external_subcommand = true; - continue; + + subcmd_name = Some(name.to_owned()); + break; } ParseResult::FlagSubCommand(_) => unreachable!(), _ => (), @@ -535,12 +532,14 @@ where if !(self.is_set(AS::ArgsNegateSubcommands) && self.is_set(AS::ValidArgFound) || self.is_set(AS::AllowExternalSubcommands) || self.is_set(AS::InferSubcommands)) + && subcmd_name.is_none() { let cands = suggestions::did_you_mean(&*arg_os.to_string_lossy(), sc_names!(self.app)); if !cands.is_empty() { let cands: Vec<_> = cands.iter().map(|cand| format!("'{}'", cand)).collect(); + debug!("@NickHackman In arg parsing HELP - regression?"); return Err(ClapError::invalid_subcommand( arg_os.to_string_lossy().into_owned(), cands.join(" or "), @@ -708,6 +707,7 @@ where && arg_os.starts_with("-")) && !self.is_set(AS::InferSubcommands) { + debug!("@NickHackman Other place - regresion?"); return Err(ClapError::unknown_argument( &*arg_os.to_string_lossy(), None, @@ -749,7 +749,7 @@ where .expect(INTERNAL_ERROR_MSG) .name .clone(); - self.parse_subcommand(&sc_name, matcher, it, false)?; + self.parse_subcommand(&sc_name, matcher, it, keep_state)?; } else if self.is_set(AS::SubcommandRequired) { let bn = self.app.bin_name.as_ref().unwrap_or(&self.app.name); return Err(ClapError::missing_subcommand( From 59a14f8e6a8cbdc4cf12fdbe8f6971dad28178b3 Mon Sep 17 00:00:00 2001 From: NickHackman Date: Tue, 7 Jul 2020 22:36:12 -0400 Subject: [PATCH 17/30] fix: no longer cloning all Apps and Args for debug Previously `Flag::App` and `Flag::Arg` both owned their App/Arg which required many, many clones. This is unnecessary. --- src/build/app/mod.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 21eb0299bfd..47cdbc6596e 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -1664,8 +1664,8 @@ impl<'b> App<'b> { #[cfg(debug_assertions)] #[derive(Debug)] enum Flag<'a> { - App(App<'a>), - Arg(Arg<'a>), + App(&'a App<'a>), + Arg(&'a Arg<'a>), } #[cfg(debug_assertions)] @@ -1802,12 +1802,8 @@ impl<'b> App<'b> { where F: Fn(&Flag<'_>) -> bool, { - let mut flags: Vec<_> = self - .subcommands - .iter() - .map(|a| Flag::App(a.clone())) - .collect(); - flags.extend(self.args.args.iter().map(|a| Flag::Arg(a.clone()))); + let mut flags: Vec<_> = self.subcommands.iter().map(|a| Flag::App(a)).collect(); + flags.extend(self.args.args.iter().map(|a| Flag::Arg(a))); two_elements_of(flags.into_iter().filter(|f| condition(f))) } From 1ea7178629fb73d81a24a8bc7fe4cdea83dc8d4d Mon Sep 17 00:00:00 2001 From: NickHackman Date: Tue, 7 Jul 2020 22:57:38 -0400 Subject: [PATCH 18/30] refractor: find_*_subcmd macro -> fn Moved from macro implementations to function implementations. --- src/build/app/mod.rs | 24 +++++++++++++++++++++++- src/macros.rs | 24 ------------------------ src/parse/parser.rs | 4 ++-- 3 files changed, 25 insertions(+), 27 deletions(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 47cdbc6596e..5abdfba7f6f 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -25,7 +25,7 @@ use crate::{ mkeymap::MKeyMap, output::{fmt::Colorizer, Help, HelpWriter, Usage}, parse::{ArgMatcher, ArgMatches, Input, Parser}, - util::{safe_exit, termcolor::ColorChoice, Id, Key}, + util::{safe_exit, termcolor::ColorChoice, ArgStr, Id, Key}, Result as ClapResult, INTERNAL_ERROR_MSG, }; @@ -2336,6 +2336,28 @@ impl<'b> App<'b> { args } + + /// Find a flag subcommand name by short flag or an alias + pub(crate) fn find_short_subcmd(&self, c: char) -> Option<&str> { + self.get_subcommands() + .iter() + .find(|sc| sc.short == Some(c) || sc.get_all_short_aliases().any(|alias| alias == c)) + .map(|sc| sc.get_name()) + } + + /// Find a flag subcommand name by long flag or an alias + pub(crate) fn find_long_subcmd(&self, long: &ArgStr<'_>) -> Option<&str> { + self.get_subcommands() + .iter() + .find(|sc| { + if let Some(sc_long) = sc.long { + match_alias!(sc, long, sc_long) + } else { + false + } + }) + .map(|sc| sc.get_name()) + } } impl<'b> Index<&'_ Id> for App<'b> { diff --git a/src/macros.rs b/src/macros.rs index 6baf876acad..3a4f3f5b707 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -630,30 +630,6 @@ macro_rules! find_subcmd_mut { }}; } -#[macro_export] -#[doc(hidden)] -macro_rules! find_short_subcmd { - ($app:expr, $c:expr) => {{ - $app.get_subcommands() - .iter() - .find(|sc| sc.short == Some($c) || sc.get_all_short_aliases().any(|alias| alias == $c)) - }}; -} - -#[macro_export] -#[doc(hidden)] -macro_rules! find_long_subcmd { - ($app:expr, $s:expr) => {{ - $app.get_subcommands().iter().find(|sc| { - if let Some(long) = sc.long { - match_alias!(sc, $s, long) - } else { - false - } - }) - }}; -} - macro_rules! longs { ($app:expr, $how:ident) => {{ use crate::mkeymap::KeyType; diff --git a/src/parse/parser.rs b/src/parse/parser.rs index c5b4534581e..c5ad047de7e 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -1173,7 +1173,7 @@ where self.parse_flag(opt, matcher)?; return Ok(ParseResult::Flag(opt.id.clone())); - } else if let Some(sc_name) = find_long_subcmd!(self.app, arg) { + } else if let Some(sc_name) = self.app.find_long_subcmd(&arg) { return Ok(ParseResult::FlagSubCommand(sc_name.to_string())); } else if self.is_set(AS::AllowLeadingHyphen) { return Ok(ParseResult::MaybeHyphenValue); @@ -1260,7 +1260,7 @@ where // Default to "we're expecting a value later" return self.parse_opt(&val, opt, false, matcher); - } else if let Some(sc_name) = find_short_subcmd!(self.app, c) { + } else if let Some(sc_name) = self.app.find_short_subcmd(c) { debug!("Parser::parse_short_arg:iter:{}: subcommand={}", c, sc_name); let name = sc_name.to_string(); let done_short_args = self.cur_idx.get() == arg.len(); From ec35ab881371ddf8e5eb242c6d8bb199aeb7f32a Mon Sep 17 00:00:00 2001 From: NickHackman Date: Wed, 8 Jul 2020 00:11:28 -0400 Subject: [PATCH 19/30] feat: long flag subcommand infer Added tests and feature to infer long flag subcommands similarly to normal subcommands. --- src/parse/parser.rs | 37 +++++++++++++++++++++++++- tests/flag_subcommands.rs | 56 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 91 insertions(+), 2 deletions(-) diff --git a/src/parse/parser.rs b/src/parse/parser.rs index c5ad047de7e..34d6bd7859f 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -865,6 +865,41 @@ where None } + // Checks if the arg matches a long flag subcommand name, or any of it's aliases (if defined) + fn possible_long_flag_subcommand(&self, arg_os: &ArgStr<'_>) -> Option<&str> { + debug!("Parser::possible_long_flag_subcommand: arg={:?}", arg_os); + if self.is_set(AS::InferSubcommands) { + let options = self + .app + .get_subcommands() + .iter() + .fold(Vec::new(), |mut options, sc| { + if let Some(long) = sc.long { + if arg_os.is_prefix_of(long) { + options.push(long); + } + options.extend( + sc.get_all_aliases() + .filter(|alias| arg_os.is_prefix_of(alias)), + ) + } + options + }); + if options.len() == 1 { + return Some(options[0]); + } + + for sc in &options { + if sc == arg_os { + return Some(sc); + } + } + } else if let Some(sc_name) = self.app.find_long_subcmd(arg_os) { + return Some(sc_name); + } + None + } + fn parse_help_subcommand(&self, cmds: &[OsString]) -> ClapResult { debug!("Parser::parse_help_subcommand"); @@ -1173,7 +1208,7 @@ where self.parse_flag(opt, matcher)?; return Ok(ParseResult::Flag(opt.id.clone())); - } else if let Some(sc_name) = self.app.find_long_subcmd(&arg) { + } else if let Some(sc_name) = self.possible_long_flag_subcommand(&arg) { return Ok(ParseResult::FlagSubCommand(sc_name.to_string())); } else if self.is_set(AS::AllowLeadingHyphen) { return Ok(ParseResult::MaybeHyphenValue); diff --git a/tests/flag_subcommands.rs b/tests/flag_subcommands.rs index f86f5a1e91d..81cf9548f4d 100644 --- a/tests/flag_subcommands.rs +++ b/tests/flag_subcommands.rs @@ -1,4 +1,4 @@ -use clap::{App, Arg}; +use clap::{App, AppSettings, Arg, ErrorKind}; #[test] fn flag_subcommand_normal() { @@ -274,3 +274,57 @@ fn flag_subcommand_conflict_with_version() { .subcommand(App::new("ver").short_flag('V').long_flag("version")) .get_matches_from(vec!["myprog", "--version"]); } + +#[test] +fn flag_subcommand_long_infer_pass() { + let m = App::new("prog") + .setting(AppSettings::InferSubcommands) + .subcommand(App::new("test").long_flag("test")) + .get_matches_from(vec!["prog", "--te"]); + assert_eq!(m.subcommand_name(), Some("test")); +} + +#[cfg(not(feature = "suggestions"))] +#[test] +fn flag_subcommand_long_infer_fail() { + let m = App::new("prog") + .setting(AppSettings::InferSubcommands) + .subcommand(App::new("test").long_flag("test")) + .subcommand(App::new("temp").long_flag("temp")) + .try_get_matches_from(vec!["prog", "--te"]); + assert!(m.is_err(), "{:#?}", m.unwrap()); + assert_eq!(m.unwrap_err().kind, ErrorKind::UnknownArgument); +} + +#[cfg(feature = "suggestions")] +#[test] +fn flag_subcommand_long_infer_fail() { + let m = App::new("prog") + .setting(AppSettings::InferSubcommands) + .subcommand(App::new("test").long_flag("test")) + .subcommand(App::new("temp").long_flag("temp")) + .try_get_matches_from(vec!["prog", "--te"]); + assert!(m.is_err(), "{:#?}", m.unwrap()); + assert_eq!(m.unwrap_err().kind, ErrorKind::UnknownArgument); +} + +#[test] +fn flag_subcommands_long_infer_pass_close() { + let m = App::new("prog") + .setting(AppSettings::InferSubcommands) + .subcommand(App::new("test").long_flag("test")) + .subcommand(App::new("temp").long_flag("temp")) + .get_matches_from(vec!["prog", "--tes"]); + assert_eq!(m.subcommand_name(), Some("test")); +} + +#[test] +fn flag_subcommands_long_infer_exact_match() { + let m = App::new("prog") + .setting(AppSettings::InferSubcommands) + .subcommand(App::new("test").long_flag("test")) + .subcommand(App::new("testa").long_flag("testa")) + .subcommand(App::new("testb").long_flag("testb")) + .get_matches_from(vec!["prog", "--test"]); + assert_eq!(m.subcommand_name(), Some("test")); +} From 1127ca6e1341194d246ea0af43ee3401557bba61 Mon Sep 17 00:00:00 2001 From: NickHackman Date: Wed, 8 Jul 2020 21:37:56 -0400 Subject: [PATCH 20/30] feat: Usage displays short, long, and normal scs Displayed in the form of pacman {query, --query, -Q} [OPTIONS] --- src/parse/parser.rs | 26 ++++-- tests/flag_subcommands.rs | 166 +++++++++++++++++++++++++++++++++++++- 2 files changed, 184 insertions(+), 8 deletions(-) diff --git a/src/parse/parser.rs b/src/parse/parser.rs index 34d6bd7859f..d6c585551e1 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -532,14 +532,12 @@ where if !(self.is_set(AS::ArgsNegateSubcommands) && self.is_set(AS::ValidArgFound) || self.is_set(AS::AllowExternalSubcommands) || self.is_set(AS::InferSubcommands)) - && subcmd_name.is_none() { let cands = suggestions::did_you_mean(&*arg_os.to_string_lossy(), sc_names!(self.app)); if !cands.is_empty() { let cands: Vec<_> = cands.iter().map(|cand| format!("'{}'", cand)).collect(); - debug!("@NickHackman In arg parsing HELP - regression?"); return Err(ClapError::invalid_subcommand( arg_os.to_string_lossy().into_owned(), cands.join(" or "), @@ -707,7 +705,6 @@ where && arg_os.starts_with("-")) && !self.is_set(AS::InferSubcommands) { - debug!("@NickHackman Other place - regresion?"); return Err(ClapError::unknown_argument( &*arg_os.to_string_lossy(), None, @@ -1055,8 +1052,22 @@ where if let Some(sc) = self.app.subcommands.iter_mut().find(|s| s.name == sc_name) { let mut sc_matcher = ArgMatcher::default(); - // bin_name should be parent's bin_name + [] + the sc's name separated by - // a space + // Display subcommand name, short and long in usage + let mut sc_names = sc.name.clone(); + let mut flag_subcmd = false; + if let Some(l) = sc.long { + sc_names.push_str(&format!(", --{}", l)); + flag_subcmd = true; + } + if let Some(s) = sc.short { + sc_names.push_str(&format!(", -{}", s)); + flag_subcmd = true; + } + + if flag_subcmd { + sc_names = format!("{{{}}}", sc_names); + } + sc.usage = Some(format!( "{}{}{}", self.app.bin_name.as_ref().unwrap_or(&String::new()), @@ -1065,8 +1076,11 @@ where } else { "" }, - &*sc.name + sc_names )); + + // bin_name should be parent's bin_name + [] + the sc's name separated by + // a space sc.bin_name = Some(format!( "{}{}{}", self.app.bin_name.as_ref().unwrap_or(&String::new()), diff --git a/tests/flag_subcommands.rs b/tests/flag_subcommands.rs index 81cf9548f4d..ac0a0338ffc 100644 --- a/tests/flag_subcommands.rs +++ b/tests/flag_subcommands.rs @@ -1,3 +1,5 @@ +mod utils; + use clap::{App, AppSettings, Arg, ErrorKind}; #[test] @@ -309,7 +311,7 @@ fn flag_subcommand_long_infer_fail() { } #[test] -fn flag_subcommands_long_infer_pass_close() { +fn flag_subcommand_long_infer_pass_close() { let m = App::new("prog") .setting(AppSettings::InferSubcommands) .subcommand(App::new("test").long_flag("test")) @@ -319,7 +321,7 @@ fn flag_subcommands_long_infer_pass_close() { } #[test] -fn flag_subcommands_long_infer_exact_match() { +fn flag_subcommand_long_infer_exact_match() { let m = App::new("prog") .setting(AppSettings::InferSubcommands) .subcommand(App::new("test").long_flag("test")) @@ -328,3 +330,163 @@ fn flag_subcommands_long_infer_exact_match() { .get_matches_from(vec!["prog", "--test"]); assert_eq!(m.subcommand_name(), Some("test")); } + +static FLAG_SUBCOMMAND_HELP: &str = "pacman-query +Query the package database. + +USAGE: + pacman {query, --query, -Q} [OPTIONS] + +FLAGS: + -h, --help Prints help information + -V, --version Prints version information + +OPTIONS: + -i, --info ... view package information + -s, --search ... search locally-installed packages for matching strings"; + +#[test] +fn flag_subcommand_long_short_normal_usage_string() { + let app = App::new("pacman") + .about("package manager utility") + .version("5.2.1") + .setting(AppSettings::SubcommandRequiredElseHelp) + .author("Pacman Development Team") + // Query subcommand + // + // Only a few of its arguments are implemented below. + .subcommand( + App::new("query") + .short_flag('Q') + .long_flag("query") + .about("Query the package database.") + .arg( + Arg::new("search") + .short('s') + .long("search") + .about("search locally-installed packages for matching strings") + .conflicts_with("info") + .multiple_values(true), + ) + .arg( + Arg::new("info") + .long("info") + .short('i') + .conflicts_with("search") + .about("view package information") + .multiple_values(true), + ), + ); + assert!(utils::compare_output( + app, + "pacman -Qh", + FLAG_SUBCOMMAND_HELP, + false + )); +} + +static FLAG_SUBCOMMAND_NO_SHORT_HELP: &str = "pacman-query +Query the package database. + +USAGE: + pacman {query, --query} [OPTIONS] + +FLAGS: + -h, --help Prints help information + -V, --version Prints version information + +OPTIONS: + -i, --info ... view package information + -s, --search ... search locally-installed packages for matching strings"; + +#[test] +fn flag_subcommand_long_normal_usage_string() { + let app = App::new("pacman") + .about("package manager utility") + .version("5.2.1") + .setting(AppSettings::SubcommandRequiredElseHelp) + .author("Pacman Development Team") + // Query subcommand + // + // Only a few of its arguments are implemented below. + .subcommand( + App::new("query") + .long_flag("query") + .about("Query the package database.") + .arg( + Arg::new("search") + .short('s') + .long("search") + .about("search locally-installed packages for matching strings") + .conflicts_with("info") + .multiple_values(true), + ) + .arg( + Arg::new("info") + .long("info") + .short('i') + .conflicts_with("search") + .about("view package information") + .multiple_values(true), + ), + ); + assert!(utils::compare_output( + app, + "pacman query --help", + FLAG_SUBCOMMAND_NO_SHORT_HELP, + false + )); +} + +static FLAG_SUBCOMMAND_NO_LONG_HELP: &str = "pacman-query +Query the package database. + +USAGE: + pacman {query, -Q} [OPTIONS] + +FLAGS: + -h, --help Prints help information + -V, --version Prints version information + +OPTIONS: + -i, --info ... view package information + -s, --search ... search locally-installed packages for matching strings"; + +#[test] +fn flag_subcommand_short_normal_usage_string() { + let app = App::new("pacman") + .about("package manager utility") + .version("5.2.1") + .setting(AppSettings::SubcommandRequiredElseHelp) + .author("Pacman Development Team") + // Query subcommand + // + // Only a few of its arguments are implemented below. + .subcommand( + App::new("query") + .short_flag('Q') + .about("Query the package database.") + .arg( + Arg::new("search") + .short('s') + .long("search") + .about("search locally-installed packages for matching strings") + .conflicts_with("info") + .multiple_values(true), + ) + .arg( + Arg::new("info") + .long("info") + .short('i') + .conflicts_with("search") + .about("view package information") + .multiple_values(true), + ), + ); + assert!(utils::compare_output( + app, + "pacman query --help", + FLAG_SUBCOMMAND_NO_LONG_HELP, + false + )); +} From a3f8ddb0b07aa057d1c82043600d541811a599aa Mon Sep 17 00:00:00 2001 From: NickHackman Date: Wed, 8 Jul 2020 21:46:00 -0400 Subject: [PATCH 21/30] doc: removed '[``]' from documentation --- src/build/app/mod.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 5abdfba7f6f..1b6d11c67eb 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -872,7 +872,7 @@ impl<'b> App<'b> { self } - /// Allows adding a [``] alias, which function as "hidden" short flag subcommands that + /// Allows adding an alias, which function as "hidden" short flag subcommands that /// automatically dispatch as if this subcommand was used. This is more efficient, and easier /// than creating multiple hidden subcommands as one only needs to check for the existence of /// this command, and not all variants. @@ -887,7 +887,6 @@ impl<'b> App<'b> { /// .get_matches_from(vec!["myprog", "d"]); /// assert_eq!(m.subcommand_name(), Some("test")); /// ``` - /// [``]: ./struct..html pub fn short_flag_alias(mut self, name: char) -> Self { if name == '-' { panic!("short alias name cannot be `-`"); @@ -921,7 +920,7 @@ impl<'b> App<'b> { self } - /// Allows adding [``] aliases, which function as "hidden" short flag subcommands that + /// Allows adding aliases, which function as "hidden" short flag subcommands that /// automatically dispatch as if this subcommand was used. This is more efficient, and easier /// than creating multiple hidden subcommands as one only needs to check for the existence of /// this command, and not all variants. @@ -940,7 +939,6 @@ impl<'b> App<'b> { /// .get_matches_from(vec!["myprog", "-a"]); /// assert_eq!(m.subcommand_name(), Some("test")); /// ``` - /// [``]: ./struct..html pub fn short_flag_aliases(mut self, names: &[char]) -> Self { for s in names { if s == &'-' { @@ -971,7 +969,7 @@ impl<'b> App<'b> { self } - /// Allows adding a [``] alias that functions exactly like those defined with + /// Allows adding an alias that functions exactly like those defined with /// [`App::short_flag_alias`], except that they are visible inside the help message. /// /// # Examples @@ -984,7 +982,6 @@ impl<'b> App<'b> { /// .get_matches_from(vec!["myprog", "-d"]); /// assert_eq!(m.subcommand_name(), Some("test")); /// ``` - /// [``]: ./struct..html /// [`App::short_flag_alias`]: ./struct.App.html#method.short_flag_alias pub fn visible_short_flag_alias(mut self, name: char) -> Self { if name == '-' { @@ -1014,7 +1011,7 @@ impl<'b> App<'b> { self } - /// Allows adding multiple [``] short flag aliases that functions exactly like those defined + /// Allows adding multiple short flag aliases that functions exactly like those defined /// with [`App::short_flag_aliases`], except that they are visible inside the help message. /// /// # Examples @@ -1027,7 +1024,6 @@ impl<'b> App<'b> { /// .get_matches_from(vec!["myprog", "-t"]); /// assert_eq!(m.subcommand_name(), Some("test")); /// ``` - /// [``]: ./struct..html /// [`App::short_flag_aliases`]: ./struct.App.html#method.short_flag_aliases pub fn visible_short_flag_aliases(mut self, names: &[char]) -> Self { for s in names { From 0a266f8c8c63bbd44a19d92b234b8c61e0e980c0 Mon Sep 17 00:00:00 2001 From: NickHackman Date: Thu, 9 Jul 2020 22:15:48 -0400 Subject: [PATCH 22/30] fix: merge related issues get_subcommands() now returns an impl Iterator rather than a slice, match_alias! removed --- src/build/app/mod.rs | 4 +--- src/parse/parser.rs | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 1f61a166982..b06fbb01d60 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -2401,7 +2401,6 @@ impl<'b> App<'b> { /// Find a flag subcommand name by short flag or an alias pub(crate) fn find_short_subcmd(&self, c: char) -> Option<&str> { self.get_subcommands() - .iter() .find(|sc| sc.short == Some(c) || sc.get_all_short_aliases().any(|alias| alias == c)) .map(|sc| sc.get_name()) } @@ -2409,10 +2408,9 @@ impl<'b> App<'b> { /// Find a flag subcommand name by long flag or an alias pub(crate) fn find_long_subcmd(&self, long: &ArgStr<'_>) -> Option<&str> { self.get_subcommands() - .iter() .find(|sc| { if let Some(sc_long) = sc.long { - match_alias!(sc, long, sc_long) + *long == sc_long || sc.aliases_to(long) } else { false } diff --git a/src/parse/parser.rs b/src/parse/parser.rs index e8d9e8204ad..fe659a4a3d2 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -887,7 +887,6 @@ where let options = self .app .get_subcommands() - .iter() .fold(Vec::new(), |mut options, sc| { if let Some(long) = sc.long { if arg_os.is_prefix_of(long) { From f5aabfa48207ca03d72e8e81fec5fb4d3570696f Mon Sep 17 00:00:00 2001 From: NickHackman Date: Fri, 10 Jul 2020 08:52:13 -0400 Subject: [PATCH 23/30] style: rename short, long -> short_flag, long_flag These names are more explicit about what short and long flag subcommands are. --- src/build/app/mod.rs | 34 ++++++++++++++++++---------------- src/output/help.rs | 6 +++--- src/parse/parser.rs | 6 +++--- 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index b06fbb01d60..9f15936831c 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -72,8 +72,8 @@ pub(crate) enum Propagation { pub struct App<'b> { pub(crate) id: Id, pub(crate) name: String, - pub(crate) long: Option<&'b str>, - pub(crate) short: Option, + pub(crate) long_flag: Option<&'b str>, + pub(crate) short_flag: Option, pub(crate) bin_name: Option, pub(crate) author: Option<&'b str>, pub(crate) version: Option<&'b str>, @@ -110,13 +110,13 @@ impl<'b> App<'b> { /// Get the short flag of the subcommand #[inline] pub fn get_short_flag(&self) -> Option { - self.short + self.short_flag } /// Get the long flag of the subcommand #[inline] pub fn get_long_flag(&self) -> Option<&str> { - self.long + self.long_flag } /// Get the name of the binary @@ -144,7 +144,7 @@ impl<'b> App<'b> { /// Iterate through the *visible* short aliases for this subcommand. #[inline] - pub fn get_visible_short_aliases(&self) -> impl Iterator + '_ { + pub fn get_visible_short_flag_aliases(&self) -> impl Iterator + '_ { self.short_flag_aliases .iter() .filter(|(_, vis)| *vis) @@ -159,7 +159,7 @@ impl<'b> App<'b> { /// Iterate through the set of *all* the short aliases for this subcommand, both visible and hidden. #[inline] - pub fn get_all_short_aliases(&self) -> impl Iterator + '_ { + pub fn get_all_short_flag_aliases(&self) -> impl Iterator + '_ { self.short_flag_aliases.iter().map(|a| a.0) } @@ -449,7 +449,7 @@ impl<'b> App<'b> { /// assert!(sync_matches.is_present("search")); /// ``` pub fn short_flag(mut self, short: char) -> Self { - self.short = Some(short); + self.short_flag = Some(short); self } @@ -492,7 +492,7 @@ impl<'b> App<'b> { /// assert!(sync_matches.is_present("search")); /// ``` pub fn long_flag(mut self, long: &'b str) -> Self { - self.long = Some(long.trim_start_matches(|c| c == '-')); + self.long_flag = Some(long.trim_start_matches(|c| c == '-')); self } @@ -1697,14 +1697,14 @@ enum Flag<'a> { impl<'a> Flag<'_> { fn short(&self) -> Option { match self { - Self::App(app) => app.short, + Self::App(app) => app.short_flag, Self::Arg(arg) => arg.short, } } fn long(&self) -> Option<&str> { match self { - Self::App(app) => app.long, + Self::App(app) => app.long_flag, Self::Arg(arg) => arg.long, } } @@ -1857,7 +1857,7 @@ impl<'b> App<'b> { for sc in &self.subcommands { // Conflicts between flag subcommands and long args - if let Some(l) = sc.long { + if let Some(l) = sc.long_flag { if let Some((first, second)) = self.two_flags_of(|f| f.long() == Some(l)) { panic!( "Long option names must be unique for each argument, \ @@ -1868,7 +1868,7 @@ impl<'b> App<'b> { } // Conflicts between flag subcommands and short args - if let Some(s) = sc.short { + if let Some(s) = sc.short_flag { if let Some((first, second)) = self.two_flags_of(|f| f.short() == Some(s)) { panic!( "Short option names must be unique for each argument, \ @@ -2082,7 +2082,7 @@ impl<'b> App<'b> { || self .subcommands .iter() - .any(|sc| sc.short == Some('h') || sc.long == Some("help"))) + .any(|sc| sc.short_flag == Some('h') || sc.long_flag == Some("help"))) { debug!("App::_create_help_and_version: Building --help"); let mut help = Arg::new("help") @@ -2103,7 +2103,7 @@ impl<'b> App<'b> { || self .subcommands .iter() - .any(|sc| sc.short == Some('V') || sc.long == Some("version"))) + .any(|sc| sc.short_flag == Some('V') || sc.long_flag == Some("version"))) { debug!("App::_create_help_and_version: Building --version"); let mut version = Arg::new("version") @@ -2401,7 +2401,9 @@ impl<'b> App<'b> { /// Find a flag subcommand name by short flag or an alias pub(crate) fn find_short_subcmd(&self, c: char) -> Option<&str> { self.get_subcommands() - .find(|sc| sc.short == Some(c) || sc.get_all_short_aliases().any(|alias| alias == c)) + .find(|sc| { + sc.short_flag == Some(c) || sc.get_all_short_flag_aliases().any(|alias| alias == c) + }) .map(|sc| sc.get_name()) } @@ -2409,7 +2411,7 @@ impl<'b> App<'b> { pub(crate) fn find_long_subcmd(&self, long: &ArgStr<'_>) -> Option<&str> { self.get_subcommands() .find(|sc| { - if let Some(sc_long) = sc.long { + if let Some(sc_long) = sc.long_flag { *long == sc_long || sc.aliases_to(long) } else { false diff --git a/src/output/help.rs b/src/output/help.rs index ef656ffff07..5108444ad13 100644 --- a/src/output/help.rs +++ b/src/output/help.rs @@ -585,7 +585,7 @@ impl<'b, 'c, 'd, 'w> Help<'b, 'c, 'd, 'w> { ); let mut short_als = a - .get_visible_short_aliases() + .get_visible_short_flag_aliases() .map(|a| format!("-{}", a)) .collect::>(); @@ -781,8 +781,8 @@ impl<'b, 'c, 'd, 'w> Help<'b, 'c, 'd, 'w> { { let btm = ord_m.entry(sc.disp_ord).or_insert(BTreeMap::new()); let mut sc_str = String::new(); - sc_str.push_str(&sc.short.map_or(String::new(), |c| format!("-{}, ", c))); - sc_str.push_str(&sc.long.map_or(String::new(), |c| format!("--{}, ", c))); + sc_str.push_str(&sc.short_flag.map_or(String::new(), |c| format!("-{}, ", c))); + sc_str.push_str(&sc.long_flag.map_or(String::new(), |c| format!("--{}, ", c))); sc_str.push_str(&sc.name); self.longest = cmp::max(self.longest, str_width(&sc_str)); btm.insert(sc_str, sc.clone()); diff --git a/src/parse/parser.rs b/src/parse/parser.rs index fe659a4a3d2..a2204238afb 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -888,7 +888,7 @@ where .app .get_subcommands() .fold(Vec::new(), |mut options, sc| { - if let Some(long) = sc.long { + if let Some(long) = sc.long_flag { if arg_os.is_prefix_of(long) { options.push(long); } @@ -1072,11 +1072,11 @@ where // Display subcommand name, short and long in usage let mut sc_names = sc.name.clone(); let mut flag_subcmd = false; - if let Some(l) = sc.long { + if let Some(l) = sc.long_flag { sc_names.push_str(&format!(", --{}", l)); flag_subcmd = true; } - if let Some(s) = sc.short { + if let Some(s) = sc.short_flag { sc_names.push_str(&format!(", -{}", s)); flag_subcmd = true; } From 5118cec1b12824e18d03948bfab6c3cf5be026ff Mon Sep 17 00:00:00 2001 From: NickHackman Date: Fri, 10 Jul 2020 09:27:14 -0400 Subject: [PATCH 24/30] fix: long_flag_aliases instead of using alias Previously long_flag alias checks were against normal aliases instead of specifically designated long_flag aliases, this is more clear and explicit. --- src/build/app/mod.rs | 139 +++++++++++++++++++++++++++++++++++--- tests/flag_subcommands.rs | 22 +++++- 2 files changed, 149 insertions(+), 12 deletions(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 9f15936831c..e3b76c15163 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -84,6 +84,7 @@ pub struct App<'b> { pub(crate) after_help: Option<&'b str>, pub(crate) aliases: Vec<(&'b str, bool)>, // (name, visible) pub(crate) short_flag_aliases: Vec<(char, bool)>, // (name, visible) + pub(crate) long_flag_aliases: Vec<(&'b str, bool)>, // (name, visible) pub(crate) usage_str: Option<&'b str>, pub(crate) usage: Option, pub(crate) help_str: Option<&'b str>, @@ -151,6 +152,15 @@ impl<'b> App<'b> { .map(|a| a.0) } + /// Iterate through the *visible* short aliases for this subcommand. + #[inline] + pub fn get_visible_long_flag_aliases(&self) -> impl Iterator + '_ { + self.long_flag_aliases + .iter() + .filter(|(_, vis)| *vis) + .map(|a| a.0) + } + /// Iterate through the set of *all* the aliases for this subcommand, both visible and hidden. #[inline] pub fn get_all_aliases(&self) -> impl Iterator { @@ -163,6 +173,12 @@ impl<'b> App<'b> { self.short_flag_aliases.iter().map(|a| a.0) } + /// Iterate through the set of *all* the long aliases for this subcommand, both visible and hidden. + #[inline] + pub fn get_all_long_flag_aliases(&self) -> impl Iterator + '_ { + self.long_flag_aliases.iter().map(|a| a.0) + } + /// Get the list of subcommands #[inline] pub fn get_subcommands(&self) -> impl Iterator> { @@ -913,7 +929,7 @@ impl<'b> App<'b> { /// let m = App::new("myprog") /// .subcommand(App::new("test").short_flag('t') /// .short_flag_alias('d')) - /// .get_matches_from(vec!["myprog", "d"]); + /// .get_matches_from(vec!["myprog", "-d"]); /// assert_eq!(m.subcommand_name(), Some("test")); /// ``` pub fn short_flag_alias(mut self, name: char) -> Self { @@ -924,6 +940,26 @@ impl<'b> App<'b> { self } + /// Allows adding an alias, which function as "hidden" long flag subcommands that + /// automatically dispatch as if this subcommand was used. This is more efficient, and easier + /// than creating multiple hidden subcommands as one only needs to check for the existence of + /// this command, and not all variants. + /// + /// # Examples + /// + /// ```no_run + /// # use clap::{App, Arg, }; + /// let m = App::new("myprog") + /// .subcommand(App::new("test").long_flag('t') + /// .long_flag_alias("testing")) + /// .get_matches_from(vec!["myprog", "--testing"]); + /// assert_eq!(m.subcommand_name(), Some("test")); + /// ``` + pub fn long_flag_alias(mut self, name: &'b str) -> Self { + self.long_flag_aliases.push((name, false)); + self + } + /// Allows adding [``] aliases, which function as "hidden" subcommands that /// automatically dispatch as if this subcommand was used. This is more efficient, and easier /// than creating multiple hidden subcommands as one only needs to check for the existence of @@ -978,6 +1014,32 @@ impl<'b> App<'b> { self } + /// Allows adding aliases, which function as "hidden" long flag subcommands that + /// automatically dispatch as if this subcommand was used. This is more efficient, and easier + /// than creating multiple hidden subcommands as one only needs to check for the existence of + /// this command, and not all variants. + /// + /// # Examples + /// + /// ```rust + /// # use clap::{App, Arg, }; + /// let m = App::new("myprog") + /// .subcommand(App::new("test").long_flag("test") + /// .long_flag_aliases(&["testing", "testall", "test_all"])) + /// .arg(Arg::new("input") + /// .about("the file to add") + /// .index(1) + /// .required(false)) + /// .get_matches_from(vec!["myprog", "--testing"]); + /// assert_eq!(m.subcommand_name(), Some("test")); + /// ``` + pub fn long_flag_aliases(mut self, names: &[&'b str]) -> Self { + for s in names { + self.long_flag_aliases.push((s, false)); + } + self + } + /// Allows adding a [``] alias that functions exactly like those defined with /// [`App::alias`], except that they are visible inside the help message. /// @@ -1020,6 +1082,25 @@ impl<'b> App<'b> { self } + /// Allows adding an alias that functions exactly like those defined with + /// [`App::long_flag_alias`], except that they are visible inside the help message. + /// + /// # Examples + /// + /// ```no_run + /// # use clap::{App, Arg, }; + /// let m = App::new("myprog") + /// .subcommand(App::new("test").long_flag("test") + /// .visible_long_flag_alias("testing")) + /// .get_matches_from(vec!["myprog", "--testing"]); + /// assert_eq!(m.subcommand_name(), Some("test")); + /// ``` + /// [`App::long_flag_alias`]: ./struct.App.html#method.long_flag_alias + pub fn visible_long_flag_alias(mut self, name: &'b str) -> Self { + self.long_flag_aliases.push((name, true)); + self + } + /// Allows adding multiple [``] aliases that functions exactly like those defined /// with [`App::aliases`], except that they are visible inside the help message. /// @@ -1064,6 +1145,27 @@ impl<'b> App<'b> { self } + /// Allows adding multiple long flag aliases that functions exactly like those defined + /// with [`App::long_flag_aliases`], except that they are visible inside the help message. + /// + /// # Examples + /// + /// ```no_run + /// # use clap::{App, Arg, }; + /// let m = App::new("myprog") + /// .subcommand(App::new("test").long_flag("test") + /// .visible_long_flag_aliases(&["testing", "testall", "test_all"])) + /// .get_matches_from(vec!["myprog", "--testing"]); + /// assert_eq!(m.subcommand_name(), Some("test")); + /// ``` + /// [`App::short_flag_aliases`]: ./struct.App.html#method.short_flag_aliases + pub fn visible_long_flag_aliases(mut self, names: &[&'b str]) -> Self { + for s in names { + self.long_flag_aliases.push((s, true)); + } + self + } + /// Replaces an argument to this application with other arguments. /// /// Below, when the given args are `app install`, they will be changed to `app module install`. @@ -2301,6 +2403,29 @@ impl<'b> App<'b> { *name == *self.get_name() || self.get_all_aliases().any(|alias| *name == *alias) } + /// Check if this subcommand can be referred to as `name`. In other words, + /// check if `name` is the name of this short flag subcommand or is one of its short flag aliases. + #[inline] + pub(crate) fn short_flag_aliases_to(&self, flag: char) -> bool { + Some(flag) == self.short_flag + || self.get_all_short_flag_aliases().any(|alias| flag == alias) + } + + /// Check if this subcommand can be referred to as `name`. In other words, + /// check if `name` is the name of this long flag subcommand or is one of its long flag aliases. + #[inline] + pub(crate) fn long_flag_aliases_to(&self, flag: &T) -> bool + where + T: PartialEq + ?Sized, + { + match self.long_flag { + Some(long_flag) => { + flag == long_flag || self.get_all_long_flag_aliases().any(|alias| flag == alias) + } + None => self.get_all_long_flag_aliases().any(|alias| flag == alias), + } + } + #[cfg(debug_assertions)] pub(crate) fn id_exists(&self, id: &Id) -> bool { self.args.args.iter().any(|x| x.id == *id) || self.groups.iter().any(|x| x.id == *id) @@ -2401,22 +2526,14 @@ impl<'b> App<'b> { /// Find a flag subcommand name by short flag or an alias pub(crate) fn find_short_subcmd(&self, c: char) -> Option<&str> { self.get_subcommands() - .find(|sc| { - sc.short_flag == Some(c) || sc.get_all_short_flag_aliases().any(|alias| alias == c) - }) + .find(|sc| sc.short_flag_aliases_to(c)) .map(|sc| sc.get_name()) } /// Find a flag subcommand name by long flag or an alias pub(crate) fn find_long_subcmd(&self, long: &ArgStr<'_>) -> Option<&str> { self.get_subcommands() - .find(|sc| { - if let Some(sc_long) = sc.long_flag { - *long == sc_long || sc.aliases_to(long) - } else { - false - } - }) + .find(|sc| sc.long_flag_aliases_to(long)) .map(|sc| sc.get_name()) } } diff --git a/tests/flag_subcommands.rs b/tests/flag_subcommands.rs index ac0a0338ffc..b014e370633 100644 --- a/tests/flag_subcommands.rs +++ b/tests/flag_subcommands.rs @@ -207,7 +207,27 @@ fn flag_subcommand_long_with_alias() { .long("test") .about("testing testing"), ) - .alias("result"), + .long_flag_alias("result"), + ) + .get_matches_from(vec!["myprog", "--result", "--test"]); + assert_eq!(matches.subcommand_name().unwrap(), "some"); + let sub_matches = matches.subcommand_matches("some").unwrap(); + assert!(sub_matches.is_present("test")); +} + +#[test] +fn flag_subcommand_long_with_aliases() { + let matches = App::new("test") + .subcommand( + App::new("some") + .long_flag("some") + .arg( + Arg::new("test") + .short('t') + .long("test") + .about("testing testing"), + ) + .long_flag_aliases(&["result", "someall"]), ) .get_matches_from(vec!["myprog", "--result", "--test"]); assert_eq!(matches.subcommand_name().unwrap(), "some"); From cf9265d660e47f6544bfe4e19e3fd755f41b86eb Mon Sep 17 00:00:00 2001 From: NickHackman Date: Fri, 10 Jul 2020 09:36:39 -0400 Subject: [PATCH 25/30] fix: failing doc test --- src/build/app/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index e3b76c15163..5b30d9e0304 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -950,7 +950,7 @@ impl<'b> App<'b> { /// ```no_run /// # use clap::{App, Arg, }; /// let m = App::new("myprog") - /// .subcommand(App::new("test").long_flag('t') + /// .subcommand(App::new("test").long_flag("test") /// .long_flag_alias("testing")) /// .get_matches_from(vec!["myprog", "--testing"]); /// assert_eq!(m.subcommand_name(), Some("test")); From 432be5bc301d18761802ecd5f5173bf9bf09e99d Mon Sep 17 00:00:00 2001 From: NickHackman Date: Sat, 11 Jul 2020 14:25:47 -0400 Subject: [PATCH 26/30] tests: conflicts with flag sc and arg aliases Tests to check for conflicts between flag subcommands long and short and their aliases and args both long and short and their aliases. Tests to handle self conflicts, where a flag subcommand short or long with have a corresponding alias with the same value. --- tests/flag_subcommands.rs | 56 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/flag_subcommands.rs b/tests/flag_subcommands.rs index b014e370633..d3d6be314a7 100644 --- a/tests/flag_subcommands.rs +++ b/tests/flag_subcommands.rs @@ -95,6 +95,22 @@ fn flag_subcommand_short_with_alias() { assert!(sub_matches.is_present("test")); } +#[test] +fn flag_subcommand_short_with_alias_same_as_short_flag() { + let matches = App::new("test") + .subcommand(App::new("some").short_flag('S').short_flag_alias('S')) + .get_matches_from(vec!["myprog", "-S"]); + assert_eq!(matches.subcommand_name().unwrap(), "some"); +} + +#[test] +fn flag_subcommand_long_with_alias_same_as_long_flag() { + let matches = App::new("test") + .subcommand(App::new("some").long_flag("sync").long_flag_alias("sync")) + .get_matches_from(vec!["myprog", "--sync"]); + assert_eq!(matches.subcommand_name().unwrap(), "some"); +} + #[test] fn flag_subcommand_short_with_aliases_vis_and_hidden() { let app = App::new("test").subcommand( @@ -273,6 +289,46 @@ fn flag_subcommand_short_conflict_with_arg() { .get_matches_from(vec!["myprog", "-f"]); } +#[cfg(debug_assertions)] +#[test] +#[should_panic = "Short option names must be unique for each argument, but \'-f\' is used by both an App named \'some\' and an App named \'result\'"] +fn flag_subcommand_short_conflict_with_alias() { + let _ = App::new("test") + .subcommand(App::new("some").short_flag('f').long_flag("some")) + .subcommand(App::new("result").short_flag('t').short_flag_alias('f')) + .get_matches_from(vec!["myprog", "-f"]); +} + +#[cfg(debug_assertions)] +#[test] +#[should_panic = "Long option names must be unique for each argument, but \'--flag\' is used by both an App named \'some\' and an App named \'result\'"] +fn flag_subcommand_long_conflict_with_alias() { + let _ = App::new("test") + .subcommand(App::new("some").long_flag("flag")) + .subcommand(App::new("result").long_flag("test").long_flag_alias("flag")) + .get_matches_from(vec!["myprog", "--flag"]); +} + +#[cfg(debug_assertions)] +#[test] +#[should_panic = "Short option names must be unique for each argument, but \'-f\' is used by both an App named \'some\' and an Arg named \'test\'"] +fn flag_subcommand_short_conflict_with_arg_alias() { + let _ = App::new("test") + .subcommand(App::new("some").short_flag('f').long_flag("some")) + .arg(Arg::new("test").short('t').short_alias('f')) + .get_matches_from(vec!["myprog", "-f"]); +} + +#[cfg(debug_assertions)] +#[test] +#[should_panic = "Long option names must be unique for each argument, but \'--some\' is used by both an App named \'some\' and an Arg named \'test\'"] +fn flag_subcommand_long_conflict_with_arg_alias() { + let _ = App::new("test") + .subcommand(App::new("some").short_flag('f').long_flag("some")) + .arg(Arg::new("test").long("test").alias("some")) + .get_matches_from(vec!["myprog", "--some"]); +} + #[cfg(debug_assertions)] #[test] #[should_panic = "Long option names must be unique for each argument, but \'--flag\' is used by both an App named \'some\' and an Arg named \'flag\'"] From 6ddf940ac39899246863e8d5438b722af2e005a2 Mon Sep 17 00:00:00 2001 From: NickHackman Date: Sat, 11 Jul 2020 14:30:13 -0400 Subject: [PATCH 27/30] fix: conflicts between flag scs and args alias When debug_assertions flag is active, properly handles conflicts between flag subcommands short and long their aliases and args and their aliases prevents self conflicts where the alias and the flag subcomand were the same. --- src/build/app/mod.rs | 111 +++++++++++++++++++++++++++++++++---------- 1 file changed, 85 insertions(+), 26 deletions(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 5b30d9e0304..fbbfa514ad8 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -1791,33 +1791,33 @@ impl<'b> App<'b> { #[cfg(debug_assertions)] #[derive(Debug)] enum Flag<'a> { - App(&'a App<'a>), - Arg(&'a Arg<'a>), + App(&'a App<'a>, String), + Arg(&'a Arg<'a>, String), } #[cfg(debug_assertions)] impl<'a> Flag<'_> { - fn short(&self) -> Option { + pub fn value(&self) -> &str { match self { - Self::App(app) => app.short_flag, - Self::Arg(arg) => arg.short, + Self::App(_, value) => value, + Self::Arg(_, value) => value, } } - fn long(&self) -> Option<&str> { + pub fn id(&self) -> &Id { match self { - Self::App(app) => app.long_flag, - Self::Arg(arg) => arg.long, + Self::App(app, _) => &app.id, + Self::Arg(arg, _) => &arg.id, } } } #[cfg(debug_assertions)] -impl<'a> fmt::Display for Flag<'_> { +impl<'a> fmt::Display for Flag<'a> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - Self::App(app) => write!(f, "App named '{}'", app.get_name()), - Self::Arg(arg) => write!(f, "Arg named '{}'", arg.get_name()), + Self::App(app, _) => write!(f, "App named '{}'", app.name), + Self::Arg(arg, _) => write!(f, "Arg named '{}'", arg.name), } } } @@ -1923,14 +1923,65 @@ impl<'b> App<'b> { } } - // Checks for conflicts in `Arg::long`/`App::long` or `Arg::short`/`App::short` #[cfg(debug_assertions)] - fn two_flags_of(&self, condition: F) -> Option<(Flag, Flag)> + fn two_long_flags_of(&self, condition: F) -> Option<(Flag, Flag)> where F: Fn(&Flag<'_>) -> bool, { - let mut flags: Vec<_> = self.subcommands.iter().map(|a| Flag::App(a)).collect(); - flags.extend(self.args.args.iter().map(|a| Flag::Arg(a))); + let mut flags: Vec = Vec::new(); + for sc in &self.subcommands { + if let Some(long) = sc.long_flag { + flags.push(Flag::App(&sc, long.to_string())); + } + flags.extend( + sc.get_all_long_flag_aliases() + .map(|alias| Flag::App(&sc, alias.to_string())), + ); + self.args.args.iter().for_each(|arg| { + flags.extend( + arg.aliases + .iter() + .map(|(alias, _)| Flag::Arg(arg, alias.to_string())), + ) + }); + flags.extend( + self.args + .args + .iter() + .filter_map(|arg| arg.long.map(|l| Flag::Arg(arg, l.to_string()))), + ); + } + two_elements_of(flags.into_iter().filter(|f| condition(f))) + } + + #[cfg(debug_assertions)] + fn two_short_flags_of(&self, condition: F) -> Option<(Flag, Flag)> + where + F: Fn(&Flag<'_>) -> bool, + { + let mut flags: Vec = Vec::new(); + for sc in &self.subcommands { + if let Some(short) = sc.short_flag { + flags.push(Flag::App(&sc, short.to_string())); + } + flags.extend( + sc.get_all_short_flag_aliases() + .map(|alias| Flag::App(&sc, alias.to_string())), + ); + self.args.args.iter().for_each(|arg| { + flags.extend( + arg.short_aliases + .iter() + .map(|(alias, _)| Flag::Arg(arg, alias.to_string())), + ) + }); + flags.extend( + self.args + .args + .iter() + .filter_map(|arg| arg.short.map(|l| Flag::Arg(arg, l.to_string()))), + ); + } two_elements_of(flags.into_iter().filter(|f| condition(f))) } @@ -1960,23 +2011,31 @@ impl<'b> App<'b> { for sc in &self.subcommands { // Conflicts between flag subcommands and long args if let Some(l) = sc.long_flag { - if let Some((first, second)) = self.two_flags_of(|f| f.long() == Some(l)) { - panic!( - "Long option names must be unique for each argument, \ + if let Some((first, second)) = self.two_long_flags_of(|f| f.value() == l) { + // Prevent conflicts with itself + if first.id() != second.id() { + panic!( + "Long option names must be unique for each argument, \ but '--{}' is used by both an {} and an {}", - l, first, second - ); + l, first, second + ); + } } } - // Conflicts between flag subcommands and short args + // Conflicts between flag subcommands and long args if let Some(s) = sc.short_flag { - if let Some((first, second)) = self.two_flags_of(|f| f.short() == Some(s)) { - panic!( - "Short option names must be unique for each argument, \ + if let Some((first, second)) = + self.two_short_flags_of(|f| f.value() == &s.to_string()) + { + // Prevent conflicts with itself + if first.id() != second.id() { + panic!( + "Short option names must be unique for each argument, \ but '-{}' is used by both an {} and an {}", - s, first, second - ); + s, first, second + ); + } } } } From 0ddd58c9351bf19f5b8621510b5b4948647f96d1 Mon Sep 17 00:00:00 2001 From: NickHackman Date: Sat, 11 Jul 2020 15:21:53 -0400 Subject: [PATCH 28/30] fix: clippy lint warning Not necessary to borrow the to_string for comparison --- src/build/app/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index fbbfa514ad8..ac20caf3968 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -2026,7 +2026,7 @@ impl<'b> App<'b> { // Conflicts between flag subcommands and long args if let Some(s) = sc.short_flag { if let Some((first, second)) = - self.two_short_flags_of(|f| f.value() == &s.to_string()) + self.two_short_flags_of(|f| f.value() == s.to_string()) { // Prevent conflicts with itself if first.id() != second.id() { From 21436398a20c55a2982fa8331a289596245ad696 Mon Sep 17 00:00:00 2001 From: NickHackman Date: Thu, 16 Jul 2020 17:48:00 -0400 Subject: [PATCH 29/30] docs: improved flag subcommand documentation Improved documentation in flag subcommand example and in `App::short_flag` and `App::long_flag` method documentation. --- examples/23_flag_subcommands_pacman.rs | 65 ++++++++------------------ src/build/app/mod.rs | 22 +-------- 2 files changed, 21 insertions(+), 66 deletions(-) diff --git a/examples/23_flag_subcommands_pacman.rs b/examples/23_flag_subcommands_pacman.rs index 343865ef0cb..cd186e9ad9b 100644 --- a/examples/23_flag_subcommands_pacman.rs +++ b/examples/23_flag_subcommands_pacman.rs @@ -1,50 +1,23 @@ -// Working with flag subcommands allows behavior similar to the popular Archlinux package manager Pacman. -// Man page: https://jlk.fjfi.cvut.cz/arch/manpages/man/pacman.8 -// -// It's suggested that you read examples/20_subcommands.rs prior to learning about `FlagSubCommand`s -// -// This differs from normal subcommands because it allows passing subcommands in the same fashion as `clap::Arg` in short or long args. -// -// Top Level App (pacman) TOP -// | -// --------------------------------------------------- -// / | | | | \ \ -// sync database remove files query deptest upgrade LEVEL 1 -// -// Given the above hierachy, valid runtime uses would be (not an all inclusive list): -// +// This feature allows users of the app to pass subcommands in the fashion of short or long flags. +// You may be familiar with it if you ever used [`pacman`](https://wiki.archlinux.org/index.php/pacman). +// Some made up examples of what flag subcommands are: +// +// ```shell +// $ pacman -S +// ^--- short flag subcommand. +// $ pacman --sync +// ^--- long flag subcommand. // $ pacman -Ss -// ^--- subcommand followed by an arg in its scope. -// -// $ pacman -Qs -// -// $ pacman -Rns -// -// NOTE: Subcommands short flags can be uppercase or lowercase. -// -// $ pacman --sync --search -// ^--- subcommand -// -// $ pacman sync -s -// ^--- subcommand -// -// NOTE: this isn't valid for pacman, but is done implicitly by Clap which -// adds support for both flags and standard subcommands out of the box. -// Allowing your users to make the choice of what feels more intuitive for them. -// -// Notice only one command per "level" may be used. You could not, for example, do: -// -// $ pacman -SQR -// -// It's also important to know that subcommands each have their own set of matches and may have args -// with the same name as other subcommands in a different part of the tree heirachy (i.e. the arg -// names aren't in a flat namespace). -// -// In order to use subcommands in clap, you only need to know which subcommand you're at in your -// tree, and which args are defined on that subcommand. -// -// Let's make a quick program to illustrate. We'll be using the same example as above but for -// brevity sake we won't implement all of the subcommands, only a few. +// ^--- short flag subcommand followed by a short flag +// (users can "stack" short subcommands with short flags or with other short flag subcommands) +// $ pacman -S -s +// ^--- same as above +// $ pacman -S --sync +// ^--- short flag subcommand followed by a long flag +// ``` +// NOTE: Keep in mind that subcommands, flags, and long flags are *case sensitive*: `-Q` and `-q` are different flags/subcommands. For example, you can have both `-Q` subcommand and `-q` flag, and they will be properly disambiguated. +// Let's make a quick program to illustrate. + use clap::{App, AppSettings, Arg}; fn main() { diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index ac20caf3968..ed0ebd8a73c 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -440,15 +440,6 @@ impl<'b> App<'b> { /// /// ``` /// # use clap::{App, Arg}; - /// App::new("pacman").subcommand( - /// App::new("sync") - /// .short_flag('S'), - /// ) - /// # ; - /// ``` - /// - /// ``` - /// # use clap::{App, Arg}; /// let matches = App::new("pacman") /// .subcommand( /// App::new("sync").short_flag('S').arg( @@ -483,25 +474,16 @@ impl<'b> App<'b> { /// /// ``` /// # use clap::{App, Arg}; - /// App::new("pacman").subcommand( - /// App::new("sync") - /// .long_flag("sync"), - /// ) - /// # ; - /// ``` - /// - /// ``` - /// # use clap::{App, Arg}; /// let matches = App::new("pacman") /// .subcommand( - /// App::new("sync").short_flag('S').arg( + /// App::new("sync").long_flag("sync").arg( /// Arg::new("search") /// .short('s') /// .long("search") /// .about("search remote repositories for matching strings"), /// ), /// ) - /// .get_matches_from(vec!["pacman", "-Ss"]); + /// .get_matches_from(vec!["pacman", "--sync", "--search"]); /// /// assert_eq!(matches.subcommand_name().unwrap(), "sync"); /// let sync_matches = matches.subcommand_matches("sync").unwrap(); From 6c6b9db45cd85f4463dc3f95f123f8de772e56ed Mon Sep 17 00:00:00 2001 From: NickHackman Date: Thu, 16 Jul 2020 18:05:25 -0400 Subject: [PATCH 30/30] fix: clippy lint warning - name change Lint name changed clippy::block_in_if_condition_stmt -> clippy::block_in_if_conditions --- src/parse/parser.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parse/parser.rs b/src/parse/parser.rs index a2204238afb..a390e3f67ca 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -308,7 +308,7 @@ where true } - #[allow(clippy::block_in_if_condition_stmt)] + #[allow(clippy::blocks_in_if_conditions)] // Does all the initializing and prepares the parser pub(crate) fn _build(&mut self) { debug!("Parser::_build");