From 3c3a0b71d1314c5f10bcca6d5052e9deb8203a5b Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 23 Nov 2021 09:53:02 -0600 Subject: [PATCH] fix: Deprecate runtime usage parser Fixes #8 --- benches/03_complex.rs | 46 ------------- src/build/app/mod.rs | 45 ++----------- src/build/arg/mod.rs | 133 +------------------------------------- src/build/usage_parser.rs | 34 ++++++++++ tests/multiple_values.rs | 15 ----- tests/opts.rs | 15 ----- 6 files changed, 42 insertions(+), 246 deletions(-) diff --git a/benches/03_complex.rs b/benches/03_complex.rs index 536bd1b8..aff631c2 100644 --- a/benches/03_complex.rs +++ b/benches/03_complex.rs @@ -45,51 +45,6 @@ macro_rules! create_app { }}; } -macro_rules! create_app_from_usage { - () => {{ - App::new("claptests") - .version("0.1") - .about("tests clap library") - .author("Kevin K. ") - .arg(Arg::from_usage("-o --option=[opt]... 'tests options'")) - .arg(Arg::from_usage("[positional] 'tests positionals'")) - .arg(Arg::from_usage("-f --flag... 'tests flags'").global(true)) - .args(&[ - Arg::from_usage("[flag2] -F 'tests flags with exclusions'") - .conflicts_with("flag") - .requires("option2"), - Arg::from_usage( - "[option2] --long-option-2 [option2] 'tests long options with exclusions'", - ) - .conflicts_with("option") - .requires("positional2"), - Arg::from_usage("[positional2] 'tests positionals with exclusions'"), - Arg::from_usage("-O --Option [option3] 'tests options with specific value sets'") - .possible_values(OPT3_VALS), - Arg::from_usage("[positional3]... 'tests positionals with specific values'") - .possible_values(POS3_VALS), - Arg::from_usage("--multvals [one] [two] 'Tests multiple values, not mult occs'"), - Arg::from_usage( - "--multvalsmo... [one] [two] 'Tests multiple values, not mult occs'", - ), - Arg::from_usage("--minvals2 [minvals]... 'Tests 2 min vals'").min_values(2), - Arg::from_usage("--maxvals3 [maxvals]... 'Tests 3 max vals'").max_values(3), - ]) - .subcommand( - App::new("subcmd") - .about("tests subcommands") - .version("0.1") - .author("Kevin K. ") - .arg(Arg::from_usage("-o --option [scoption]... 'tests options'")) - .arg(Arg::from_usage("[scpositional] 'tests positionals'")), - ) - }}; -} - -pub fn build_from_usage(c: &mut Criterion) { - c.bench_function("build_from_usage", |b| b.iter(|| create_app_from_usage!())); -} - pub fn build_from_builder(c: &mut Criterion) { c.bench_function("build_from_builder", |b| { b.iter(|| { @@ -334,7 +289,6 @@ pub fn parse_complex_with_sc_complex(c: &mut Criterion) { criterion_group!( benches, - build_from_usage, build_from_builder, parse_complex, parse_complex_with_flag, diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index d7fcb751..8bd5d9ee 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -1271,50 +1271,17 @@ impl<'help> App<'help> { self } - /// A convenience method for adding a single [argument] from a usage type string. The string - /// used follows the same rules and syntax as [`Arg::from_usage`] - /// - /// **NOTE:** The downside to using this method is that you can not set any additional - /// properties of the [`Arg`] other than what [`Arg::from_usage`] supports. - /// - /// # Examples - /// - /// ```no_run - /// # use clap::{App, Arg}; - /// App::new("myprog") - /// .arg_from_usage("-c --config= 'Sets a configuration file to use'") - /// # ; - /// ``` - /// [argument]: ./struct.Arg.html - /// [`Arg`]: ./struct.Arg.html - /// [`Arg::from_usage`]: ./struct.Arg.html#method.from_usage + /// Deprecated in Issue #8, see [`arg!`][crate::arg!]. + #[deprecated(since = "3.0.0", note = "Replaced with `arg!`")] pub fn arg_from_usage(self, usage: &'help str) -> Self { + #![allow(deprecated)] self.arg(Arg::from_usage(usage)) } - /// Adds multiple [arguments] at once from a usage string, one per line. See - /// [`Arg::from_usage`] for details on the syntax and rules supported. - /// - /// **NOTE:** Like [`App::arg_from_usage`] the downside is you only set properties for the - /// [`Arg`]s which [`Arg::from_usage`] supports. - /// - /// # Examples - /// - /// ```no_run - /// # use clap::{App, Arg}; - /// App::new("myprog") - /// .args_from_usage( - /// "-c --config=[FILE] 'Sets a configuration file to use' - /// [debug]... -d 'Sets the debugging level' - /// 'The input file to use'" - /// ) - /// # ; - /// ``` - /// [arguments]: ./struct.Arg.html - /// [`Arg::from_usage`]: ./struct.Arg.html#method.from_usage - /// [`App::arg_from_usage`]: ./struct.App.html#method.arg_from_usage - /// [`Arg`]: ./struct.Arg.html + /// Deprecated in Issue #8, see [`arg!`][crate::arg!]. + #[deprecated(since = "3.0.0", note = "Replaced with `arg!`")] pub fn args_from_usage(mut self, usage: &'help str) -> Self { + #![allow(deprecated)] for line in usage.lines() { let l = line.trim(); if l.is_empty() { diff --git a/src/build/arg/mod.rs b/src/build/arg/mod.rs index e1018776..4b7b5216 100644 --- a/src/build/arg/mod.rs +++ b/src/build/arg/mod.rs @@ -482,137 +482,8 @@ impl<'help> Arg<'help> { a } - /// Creates a new instance of [`Arg`] from a usage string. Allows creation of basic settings - /// for the [`Arg`]. The syntax is flexible, but there are some rules to follow. - /// - /// **NOTE**: Not all settings may be set using the usage string method. Some properties are - /// only available via the builder pattern. - /// - /// **NOTE**: Only ASCII values are officially supported in [`Arg::from_usage`] strings. Some - /// UTF-8 codepoints may work just fine, but this is not guaranteed. - /// - /// # Syntax - /// - /// Usage strings typically following the form: - /// - /// ```notrust - /// [explicit name] [short] [long] [value names] [help string] - /// ``` - /// - /// This is not a hard rule as the attributes can appear in other orders. There are also - /// several additional sigils which denote additional settings. Below are the details of each - /// portion of the string. - /// - /// ### Explicit Name - /// - /// This is an optional field, if it's omitted the argument will use one of the additional - /// fields as the name using the following priority order: - /// - /// * Explicit Name (This always takes precedence when present) - /// * Long - /// * Short - /// * Value Name - /// - /// `clap` determines explicit names as the first string of characters between either `[]` or - /// `<>` where `[]` has the dual notation of meaning the argument is optional, and `<>` meaning - /// the argument is required. - /// - /// Explicit names may be followed by: - /// * The multiple denotation `...` - /// - /// Example explicit names as follows (`ename` for an optional argument, and `rname` for a - /// required argument): - /// - /// ```notrust - /// [ename] -s, --long 'some flag' - /// -r, --longer 'some other flag' - /// ``` - /// - /// ### Short - /// - /// This is set by placing a single character after a leading `-`. - /// - /// Shorts may be followed by - /// * The multiple denotation `...` - /// * An optional comma `,` which is cosmetic only - /// * Value notation - /// - /// Example shorts are as follows (`-s`, and `-r`): - /// - /// ```notrust - /// -s, --long 'some flag' - /// -r [val], --longer 'some option' - /// ``` - /// - /// ### Long - /// - /// This is set by placing a word (no spaces) after a leading `--`. - /// - /// Shorts may be followed by - /// * The multiple denotation `...` - /// * Value notation - /// - /// Example longs are as follows (`--some`, and `--rapid`): - /// - /// ```notrust - /// -s, --some 'some flag' - /// --rapid=[FILE] 'some option' - /// ``` - /// - /// ### Values (Value Notation) - /// - /// This is set by placing a word(s) between `[]` or `<>` optionally after `=` (although this - /// is cosmetic only and does not affect functionality). If an explicit name has **not** been - /// set, using `<>` will denote a required argument, and `[]` will denote an optional argument - /// - /// Values may be followed by - /// * The multiple denotation `...` - /// * More Value notation - /// - /// More than one value will also implicitly set the arguments number of values, i.e. having - /// two values, `--option [val1] [val2]` specifies that in order for option to be satisified it - /// must receive exactly two values - /// - /// Example values are as follows (`FILE`, and `SPEED`): - /// - /// ```notrust - /// -s, --some [FILE] 'some option' - /// --rapid=... 'some required multiple option' - /// ``` - /// - /// ### Help String - /// - /// The help string is denoted between a pair of single quotes `''` and may contain any - /// characters. - /// - /// Example help strings are as follows: - /// - /// ```notrust - /// -s, --some [FILE] 'some option' - /// --rapid=... 'some required multiple option' - /// ``` - /// - /// ### Additional Sigils - /// - /// Multiple notation `...` (three consecutive dots/periods) specifies that this argument may - /// be used multiple times. Do not confuse multiple occurrences (`...`) with multiple values. - /// `--option val1 val2` is a single occurrence with multiple values. `--flag --flag` is - /// multiple occurrences (and then you can obviously have instances of both as well) - /// - /// # Examples - /// - /// ```rust - /// # use clap::{App, Arg}; - /// App::new("prog") - /// .args(&[ - /// Arg::from_usage("--config 'a required file for the configuration and no short'"), - /// Arg::from_usage("-d, --debug... 'turns on debugging information and allows multiples'"), - /// Arg::from_usage("[input] 'an optional input file to use'") - /// ]) - /// # ; - /// ``` - /// [`Arg`]: ./struct.Arg.html - /// [`Arg::from_usage`]: ./struct.Arg.html#method.from_usage + /// Deprecated in Issue #8, see [`arg!`][crate::arg!]. + #[deprecated(since = "3.0.0", note = "Replaced with `arg!`")] pub fn from_usage(u: &'help str) -> Self { UsageParser::from_usage(u).parse() } diff --git a/src/build/usage_parser.rs b/src/build/usage_parser.rs index 19fc5de9..ec868be3 100644 --- a/src/build/usage_parser.rs +++ b/src/build/usage_parser.rs @@ -241,6 +241,8 @@ fn default_value_end(b: u8) -> bool { #[cfg(test)] mod test { + #![allow(deprecated)] + use crate::build::{Arg, ArgSettings}; #[allow(clippy::cognitive_complexity)] @@ -1239,4 +1241,36 @@ mod test { assert_eq!(a.val_names.iter().collect::>(), [&"üñíčöĐ€"]); assert_eq!(a.help, Some("hælp")); } + + #[test] + fn value_names_building_num_vals_from_usage() { + use crate::App; + let m = App::new("test") + .arg(Arg::from_usage("--pos ")) + .try_get_matches_from(vec!["myprog", "--pos", "val1", "val2", "val3"]); + + assert!(m.is_ok(), "{:?}", m.unwrap_err().kind); + let m = m.unwrap(); + + assert_eq!( + m.values_of("pos").unwrap().collect::>(), + ["val1", "val2", "val3"] + ); + } + + #[test] + fn issue_665() { + use crate::{App, ErrorKind}; + // Verify fix for "arg_from_usage(): required values not being enforced when followed by another option" + let res = App::new("tester") + .arg(Arg::from_usage("-v, --reroll-count=[N] 'Mark the patch series as PATCH vN'")) + .arg( + Arg::from_usage("--subject-prefix [Subject-Prefix] 'Use [Subject-Prefix] instead of the standard [PATCH] prefix'") + .setting(ArgSettings::ForbidEmptyValues) + ) + .try_get_matches_from(vec!["test", "--subject-prefix", "-v", "2"]); + + assert!(res.is_err()); + assert_eq!(res.unwrap_err().kind, ErrorKind::EmptyValue); + } } diff --git a/tests/multiple_values.rs b/tests/multiple_values.rs index aaed2531..01836c3b 100644 --- a/tests/multiple_values.rs +++ b/tests/multiple_values.rs @@ -1270,21 +1270,6 @@ fn value_names_building_num_vals() { ); } -#[test] -fn value_names_building_num_vals_from_usage() { - let m = App::new("test") - .arg(Arg::from_usage("--pos ")) - .try_get_matches_from(vec!["myprog", "--pos", "val1", "val2", "val3"]); - - assert!(m.is_ok(), "{:?}", m.unwrap_err().kind); - let m = m.unwrap(); - - assert_eq!( - m.values_of("pos").unwrap().collect::>(), - ["val1", "val2", "val3"] - ); -} - #[test] fn value_names_building_num_vals_for_positional() { let m = App::new("test") diff --git a/tests/opts.rs b/tests/opts.rs index 38a9d463..45a715ae 100644 --- a/tests/opts.rs +++ b/tests/opts.rs @@ -435,21 +435,6 @@ fn did_you_mean() { )); } -#[test] -fn issue_665() { - // Verify fix for "arg_from_usage(): required values not being enforced when followed by another option" - let res = App::new("tester") - .arg(Arg::from_usage("-v, --reroll-count=[N] 'Mark the patch series as PATCH vN'")) - .arg( - Arg::from_usage("--subject-prefix [Subject-Prefix] 'Use [Subject-Prefix] instead of the standard [PATCH] prefix'") - .setting(ArgSettings::ForbidEmptyValues) - ) - .try_get_matches_from(vec!["test", "--subject-prefix", "-v", "2"]); - - assert!(res.is_err()); - assert_eq!(res.unwrap_err().kind, ErrorKind::EmptyValue); -} - #[test] fn issue_1047_min_zero_vals_default_val() { let m = App::new("foo")