From e86194dea0143f8e78504a6acb8faa13550f9639 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 12 Oct 2021 10:29:05 -0500 Subject: [PATCH] fix(builder)!: Accept a more liberal Iterator This replaces the most common uses of `&[]` with `IntoIterator`. This is based on #2414. Unlike that, this stays consistent with using `Into<&'help str>`. There are some remaining functions not converted over, like defaults and requires. BREAKING CHANGE: Instances like `aliases(&[])` need to switch to `aliases([])`. --- benches/03_complex.rs | 4 +- clap_derive/examples/subcommand_aliases.rs | 2 +- clap_derive/tests/non_literal_attributes.rs | 2 +- examples/21_aliases.rs | 2 +- src/build/app/mod.rs | 64 ++++++++++++------ src/build/arg/mod.rs | 72 +++++++++++++-------- src/build/macros.rs | 2 +- tests/arg_aliases.rs | 12 ++-- tests/arg_aliases_short.rs | 12 ++-- tests/flag_subcommands.rs | 8 +-- tests/multiple_values.rs | 6 +- tests/subcommands.rs | 2 +- 12 files changed, 116 insertions(+), 72 deletions(-) diff --git a/benches/03_complex.rs b/benches/03_complex.rs index efb79e3da37..e599b652a7b 100644 --- a/benches/03_complex.rs +++ b/benches/03_complex.rs @@ -111,7 +111,7 @@ pub fn build_from_builder(c: &mut Criterion) { Arg::new("multvals") .long("multvals") .about("Tests multiple values, not mult occs") - .value_names(&["one", "two"]), + .value_names(["one", "two"]), ) .arg( Arg::new("multvalsmo") @@ -120,7 +120,7 @@ pub fn build_from_builder(c: &mut Criterion) { .setting(ArgSettings::MultipleValues) .setting(ArgSettings::MultipleOccurrences) .about("Tests multiple values, not mult occs") - .value_names(&["one", "two"]), + .value_names(["one", "two"]), ) .arg( Arg::new("minvals") diff --git a/clap_derive/examples/subcommand_aliases.rs b/clap_derive/examples/subcommand_aliases.rs index cddd5094da4..0e7007c7a04 100644 --- a/clap_derive/examples/subcommand_aliases.rs +++ b/clap_derive/examples/subcommand_aliases.rs @@ -10,7 +10,7 @@ enum Opt { #[clap(alias = "foobar")] Foo, // https://docs.rs/clap/2/clap/struct.App.html#method.aliases - #[clap(aliases = &["baz", "fizz"])] + #[clap(aliases = ["baz", "fizz"])] Bar, } diff --git a/clap_derive/tests/non_literal_attributes.rs b/clap_derive/tests/non_literal_attributes.rs index 51d00331514..e139568c6a7 100644 --- a/clap_derive/tests/non_literal_attributes.rs +++ b/clap_derive/tests/non_literal_attributes.rs @@ -30,7 +30,7 @@ struct Opt { )] x: i32, - #[clap(short = 'l', long = "level", aliases = &["set-level", "lvl"])] + #[clap(short = 'l', long = "level", aliases = ["set-level", "lvl"])] level: String, #[clap(long("values"))] diff --git a/examples/21_aliases.rs b/examples/21_aliases.rs index e0442339eb3..d0f9caae3c1 100644 --- a/examples/21_aliases.rs +++ b/examples/21_aliases.rs @@ -4,7 +4,7 @@ fn main() { let matches = App::new("MyApp") .subcommand( App::new("ls") - .aliases(&["list", "dir"]) + .aliases(["list", "dir"]) .about("Adds files to myapp") .license("MIT OR Apache-2.0") .version("0.1") diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 9409d7327bb..19ba02d0e98 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -1232,7 +1232,7 @@ impl<'help> App<'help> { /// # use clap::{App, Arg}; /// let m = App::new("myprog") /// .subcommand(App::new("test") - /// .aliases(&["do-stuff", "do-tests", "tests"])) + /// .aliases(["do-stuff", "do-tests", "tests"])) /// .arg(Arg::new("input") /// .about("the file to add") /// .index(1) @@ -1241,8 +1241,13 @@ impl<'help> App<'help> { /// assert_eq!(m.subcommand_name(), Some("test")); /// ``` /// [`App::visible_aliases`]: App::visible_aliases() - pub fn aliases(mut self, names: &[&'help str]) -> Self { - self.aliases.extend(names.iter().map(|n| (*n, false))); + pub fn aliases(mut self, names: I) -> Self + where + I: IntoIterator, + T: Into<&'help str>, + { + self.aliases + .extend(names.into_iter().map(|n| (n.into(), false))); self } @@ -1257,7 +1262,7 @@ impl<'help> App<'help> { /// # use clap::{App, Arg, }; /// let m = App::new("myprog") /// .subcommand(App::new("test").short_flag('t') - /// .short_flag_aliases(&['a', 'b', 'c'])) + /// .short_flag_aliases(['a', 'b', 'c'])) /// .arg(Arg::new("input") /// .about("the file to add") /// .index(1) @@ -1265,10 +1270,13 @@ impl<'help> App<'help> { /// .get_matches_from(vec!["myprog", "-a"]); /// assert_eq!(m.subcommand_name(), Some("test")); /// ``` - pub fn short_flag_aliases(mut self, names: &[char]) -> Self { + pub fn short_flag_aliases(mut self, names: I) -> Self + where + I: IntoIterator, + { for s in names { - assert!(s != &'-', "short alias name cannot be `-`"); - self.short_flag_aliases.push((*s, false)); + assert!(s != '-', "short alias name cannot be `-`"); + self.short_flag_aliases.push((s, false)); } self } @@ -1284,7 +1292,7 @@ impl<'help> App<'help> { /// # use clap::{App, Arg, }; /// let m = App::new("myprog") /// .subcommand(App::new("test").long_flag("test") - /// .long_flag_aliases(&["testing", "testall", "test_all"])) + /// .long_flag_aliases(["testing", "testall", "test_all"])) /// .arg(Arg::new("input") /// .about("the file to add") /// .index(1) @@ -1292,9 +1300,13 @@ impl<'help> App<'help> { /// .get_matches_from(vec!["myprog", "--testing"]); /// assert_eq!(m.subcommand_name(), Some("test")); /// ``` - pub fn long_flag_aliases(mut self, names: &[&'help str]) -> Self { + pub fn long_flag_aliases(mut self, names: I) -> Self + where + I: IntoIterator, + T: Into<&'help str>, + { for s in names { - self.long_flag_aliases.push((s, false)); + self.long_flag_aliases.push((s.into(), false)); } self } @@ -1390,13 +1402,18 @@ impl<'help> App<'help> { /// # use clap::{App, Arg, }; /// let m = App::new("myprog") /// .subcommand(App::new("test") - /// .visible_aliases(&["do-stuff", "tests"])) + /// .visible_aliases(["do-stuff", "tests"])) /// .get_matches_from(vec!["myprog", "do-stuff"]); /// assert_eq!(m.subcommand_name(), Some("test")); /// ``` /// [`App::alias`]: App::alias() - pub fn visible_aliases(mut self, names: &[&'help str]) -> Self { - self.aliases.extend(names.iter().map(|n| (*n, true))); + pub fn visible_aliases(mut self, names: I) -> Self + where + I: IntoIterator, + T: Into<&'help str>, + { + self.aliases + .extend(names.into_iter().map(|n| (n.into(), true))); self } @@ -1409,15 +1426,18 @@ impl<'help> App<'help> { /// # use clap::{App, Arg, }; /// let m = App::new("myprog") /// .subcommand(App::new("test").short_flag('b') - /// .visible_short_flag_aliases(&['t'])) + /// .visible_short_flag_aliases(['t'])) /// .get_matches_from(vec!["myprog", "-t"]); /// assert_eq!(m.subcommand_name(), Some("test")); /// ``` /// [`App::short_flag_aliases`]: App::short_flag_aliases() - pub fn visible_short_flag_aliases(mut self, names: &[char]) -> Self { + pub fn visible_short_flag_aliases(mut self, names: I) -> Self + where + I: IntoIterator, + { for s in names { - assert!(!(s == &'-'), "short alias name cannot be `-`"); - self.short_flag_aliases.push((*s, true)); + assert!(!(s == '-'), "short alias name cannot be `-`"); + self.short_flag_aliases.push((s, true)); } self } @@ -1431,14 +1451,18 @@ impl<'help> App<'help> { /// # use clap::{App, Arg, }; /// let m = App::new("myprog") /// .subcommand(App::new("test").long_flag("test") - /// .visible_long_flag_aliases(&["testing", "testall", "test_all"])) + /// .visible_long_flag_aliases(["testing", "testall", "test_all"])) /// .get_matches_from(vec!["myprog", "--testing"]); /// assert_eq!(m.subcommand_name(), Some("test")); /// ``` /// [`App::long_flag_aliases`]: App::long_flag_aliases() - pub fn visible_long_flag_aliases(mut self, names: &[&'help str]) -> Self { + pub fn visible_long_flag_aliases(mut self, names: I) -> Self + where + I: IntoIterator, + T: Into<&'help str>, + { for s in names { - self.long_flag_aliases.push((s, true)); + self.long_flag_aliases.push((s.into(), true)); } self } diff --git a/src/build/arg/mod.rs b/src/build/arg/mod.rs index 1142182962a..516963ee41c 100644 --- a/src/build/arg/mod.rs +++ b/src/build/arg/mod.rs @@ -479,7 +479,7 @@ impl<'help> Arg<'help> { /// let m = App::new("prog") /// .arg(Arg::new("test") /// .long("test") - /// .aliases(&["do-stuff", "do-tests", "tests"]) + /// .aliases(["do-stuff", "do-tests", "tests"]) /// .about("the file to add") /// .required(false)) /// .get_matches_from(vec![ @@ -487,8 +487,13 @@ impl<'help> Arg<'help> { /// ]); /// assert!(m.is_present("test")); /// ``` - pub fn aliases(mut self, names: &[&'help str]) -> Self { - self.aliases.extend(names.iter().map(|&x| (x, false))); + pub fn aliases(mut self, names: I) -> Self + where + I: IntoIterator, + T: Into<&'help str>, + { + self.aliases + .extend(names.into_iter().map(|x| (x.into(), false))); self } @@ -504,7 +509,7 @@ impl<'help> Arg<'help> { /// let m = App::new("prog") /// .arg(Arg::new("test") /// .short('t') - /// .short_aliases(&['e', 's']) + /// .short_aliases(['e', 's']) /// .about("the file to add") /// .required(false)) /// .get_matches_from(vec![ @@ -512,10 +517,13 @@ impl<'help> Arg<'help> { /// ]); /// assert!(m.is_present("test")); /// ``` - pub fn short_aliases(mut self, names: &[char]) -> Self { + pub fn short_aliases(mut self, names: I) -> Self + where + I: IntoIterator, + { for s in names { - assert!(s != &'-', "short alias name cannot be `-`"); - self.short_aliases.push((*s, false)); + assert!(s != '-', "short alias name cannot be `-`"); + self.short_aliases.push((s, false)); } self } @@ -580,15 +588,20 @@ impl<'help> Arg<'help> { /// let m = App::new("prog") /// .arg(Arg::new("test") /// .long("test") - /// .visible_aliases(&["something", "awesome", "cool"])) + /// .visible_aliases(["something", "awesome", "cool"])) /// .get_matches_from(vec![ /// "prog", "--awesome" /// ]); /// assert!(m.is_present("test")); /// ``` /// [`App::aliases`]: Arg::aliases() - pub fn visible_aliases(mut self, names: &[&'help str]) -> Self { - self.aliases.extend(names.iter().map(|n| (*n, true))); + pub fn visible_aliases(mut self, names: I) -> Self + where + I: IntoIterator, + T: Into<&'help str>, + { + self.aliases + .extend(names.into_iter().map(|n| (n.into(), true))); self } @@ -602,17 +615,20 @@ impl<'help> Arg<'help> { /// let m = App::new("prog") /// .arg(Arg::new("test") /// .long("test") - /// .visible_short_aliases(&['t', 'e'])) + /// .visible_short_aliases(['t', 'e'])) /// .get_matches_from(vec![ /// "prog", "-t" /// ]); /// assert!(m.is_present("test")); /// ``` /// [`App::aliases`]: Arg::short_aliases() - pub fn visible_short_aliases(mut self, names: &[char]) -> Self { + pub fn visible_short_aliases(mut self, names: I) -> Self + where + I: IntoIterator, + { for n in names { - assert!(n != &'-', "short alias name cannot be `-`"); - self.short_aliases.push((*n, true)); + assert!(n != '-', "short alias name cannot be `-`"); + self.short_aliases.push((n, true)); } self } @@ -2555,7 +2571,7 @@ impl<'help> Arg<'help> { /// # use clap::{App, Arg}; /// Arg::new("speed") /// .short('s') - /// .value_names(&["fast", "slow"]); + /// .value_names(["fast", "slow"]); /// ``` /// /// ```rust @@ -2563,7 +2579,7 @@ impl<'help> Arg<'help> { /// let m = App::new("prog") /// .arg(Arg::new("io") /// .long("io-files") - /// .value_names(&["INFILE", "OUTFILE"])) + /// .value_names(["INFILE", "OUTFILE"])) /// .get_matches_from(vec![ /// "prog", "--help" /// ]); @@ -2588,8 +2604,12 @@ impl<'help> Arg<'help> { /// [`Arg::number_of_values`]: Arg::number_of_values() /// [`Arg::takes_value(true)`]: Arg::takes_value() /// [`Arg::multiple_values(true)`]: Arg::multiple_values() - pub fn value_names(mut self, names: &[&'help str]) -> Self { - self.val_names = names.to_vec(); + pub fn value_names(mut self, names: I) -> Self + where + I: IntoIterator, + { + self.val_names.clear(); + self.val_names.extend(names); self.takes_value(true) } @@ -2642,7 +2662,7 @@ impl<'help> Arg<'help> { /// [`Arg::takes_value(true)`]: Arg::takes_value() #[inline] pub fn value_name(self, name: &'help str) -> Self { - self.value_names(&[name]) + self.value_names([name]) } /// Specifies the value of the argument when *not* specified at runtime. @@ -4099,7 +4119,7 @@ impl<'help> Arg<'help> { /// .short('o') /// .setting(ArgSettings::TakesValue) /// .setting(ArgSettings::NextLineHelp) - /// .value_names(&["value1", "value2"]) + /// .value_names(["value1", "value2"]) /// .about("Some really long help and complex\n\ /// help that makes more sense to be\n\ /// on a line after the option")) @@ -5204,7 +5224,7 @@ mod test { #[test] fn option_display2() { - let o2 = Arg::new("opt").short('o').value_names(&["file", "name"]); + let o2 = Arg::new("opt").short('o').value_names(["file", "name"]); assert_eq!(&*format!("{}", o2), "-o "); } @@ -5215,7 +5235,7 @@ mod test { .short('o') .takes_value(true) .multiple_values(true) - .value_names(&["file", "name"]); + .value_names(["file", "name"]); assert_eq!(&*format!("{}", o2), "-o "); } @@ -5235,7 +5255,7 @@ mod test { let o = Arg::new("opt") .long("option") .takes_value(true) - .visible_aliases(&["als2", "als3", "als4"]) + .visible_aliases(["als2", "als3", "als4"]) .alias("als_not_visible"); assert_eq!(&*format!("{}", o), "--option "); @@ -5256,7 +5276,7 @@ mod test { let o = Arg::new("opt") .short('a') .takes_value(true) - .visible_short_aliases(&['b', 'c', 'd']) + .visible_short_aliases(['b', 'c', 'd']) .short_alias('e'); assert_eq!(&*format!("{}", o), "-a "); @@ -5293,7 +5313,7 @@ mod test { #[test] fn positional_display_val_names() { - let p2 = Arg::new("pos").index(1).value_names(&["file1", "file2"]); + let p2 = Arg::new("pos").index(1).value_names(["file1", "file2"]); assert_eq!(&*format!("{}", p2), " "); } @@ -5303,7 +5323,7 @@ mod test { let p2 = Arg::new("pos") .index(1) .setting(ArgSettings::Required) - .value_names(&["file1", "file2"]); + .value_names(["file1", "file2"]); assert_eq!(&*format!("{}", p2), " "); } diff --git a/src/build/macros.rs b/src/build/macros.rs index 7ac3ea4f2d2..2a7df6cf7ff 100644 --- a/src/build/macros.rs +++ b/src/build/macros.rs @@ -133,7 +133,7 @@ macro_rules! yaml_char { #[cfg(feature = "yaml")] macro_rules! yaml_chars { ($v:expr) => {{ - &$v.as_vec() + $v.as_vec() .unwrap_or_else(|| panic!("failed to convert YAML {:?} value to a list", $v)) .into_iter() .map(|s| { diff --git a/tests/arg_aliases.rs b/tests/arg_aliases.rs index ab0fb78db01..55f70b8b69c 100644 --- a/tests/arg_aliases.rs +++ b/tests/arg_aliases.rs @@ -58,7 +58,7 @@ fn multiple_aliases_of_option() { .long("aliases") .takes_value(true) .about("multiple aliases") - .aliases(&["alias1", "alias2", "alias3"]), + .aliases(["alias1", "alias2", "alias3"]), ); let long = a .clone() @@ -106,7 +106,7 @@ fn single_alias_of_flag() { #[test] fn multiple_aliases_of_flag() { - let a = App::new("test").arg(Arg::new("flag").long("flag").aliases(&[ + let a = App::new("test").arg(Arg::new("flag").long("flag").aliases([ "invisible", "set", "of", @@ -149,7 +149,7 @@ fn alias_on_a_subcommand_option() { .about("testing testing"), ), ) - .arg(Arg::new("other").long("other").aliases(&["o1", "o2", "o3"])) + .arg(Arg::new("other").long("other").aliases(["o1", "o2", "o3"])) .get_matches_from(vec!["test", "some", "--opt", "awesome"]); assert!(m.subcommand_matches("some").is_some()); @@ -169,9 +169,9 @@ fn invisible_arg_aliases_help_output() { .long("opt") .short('o') .takes_value(true) - .aliases(&["invisible", "als1", "more"]), + .aliases(["invisible", "als1", "more"]), ) - .arg(Arg::from("-f, --flag").aliases(&["unseeable", "flg1", "anyway"])), + .arg(Arg::from("-f, --flag").aliases(["unseeable", "flg1", "anyway"])), ); assert!(utils::compare_output( app, @@ -199,7 +199,7 @@ fn visible_arg_aliases_help_output() { Arg::new("flg") .long("flag") .short('f') - .visible_aliases(&["v_flg", "flag2", "flg3"]), + .visible_aliases(["v_flg", "flag2", "flg3"]), ), ); assert!(utils::compare_output( diff --git a/tests/arg_aliases_short.rs b/tests/arg_aliases_short.rs index 7aa9c2b2853..519e74070e1 100644 --- a/tests/arg_aliases_short.rs +++ b/tests/arg_aliases_short.rs @@ -58,7 +58,7 @@ fn multiple_short_aliases_of_option() { .long("aliases") .takes_value(true) .about("multiple aliases") - .short_aliases(&['1', '2', '3']), + .short_aliases(['1', '2', '3']), ); let long = a .clone() @@ -103,7 +103,7 @@ fn multiple_short_aliases_of_flag() { let a = App::new("test").arg( Arg::new("flag") .long("flag") - .short_aliases(&['a', 'b', 'c', 'd', 'e']), + .short_aliases(['a', 'b', 'c', 'd', 'e']), ); let flag = a.clone().try_get_matches_from(vec!["", "--flag"]); @@ -144,7 +144,7 @@ fn short_alias_on_a_subcommand_option() { .arg( Arg::new("other") .long("other") - .short_aliases(&['1', '2', '3']), + .short_aliases(['1', '2', '3']), ) .get_matches_from(vec!["test", "some", "-o", "awesome"]); @@ -165,9 +165,9 @@ fn invisible_short_arg_aliases_help_output() { .long("opt") .short('o') .takes_value(true) - .short_aliases(&['a', 'b', 'c']), + .short_aliases(['a', 'b', 'c']), ) - .arg(Arg::from("-f, --flag").short_aliases(&['x', 'y', 'z'])), + .arg(Arg::from("-f, --flag").short_aliases(['x', 'y', 'z'])), ); assert!(utils::compare_output( app, @@ -196,7 +196,7 @@ fn visible_short_arg_aliases_help_output() { .long("flag") .short('f') .visible_alias("flag1") - .visible_short_aliases(&['a', 'b', '🦆']), + .visible_short_aliases(['a', 'b', '🦆']), ), ); assert!(utils::compare_output( diff --git a/tests/flag_subcommands.rs b/tests/flag_subcommands.rs index 8c6b694f573..bb243455584 100644 --- a/tests/flag_subcommands.rs +++ b/tests/flag_subcommands.rs @@ -122,7 +122,7 @@ fn flag_subcommand_short_with_aliases_vis_and_hidden() { .long("test") .about("testing testing"), ) - .visible_short_flag_aliases(&['M', 'B']) + .visible_short_flag_aliases(['M', 'B']) .short_flag_alias('C'), ); let app1 = app.clone(); @@ -150,7 +150,7 @@ fn flag_subcommand_short_with_aliases() { .long("test") .about("testing testing"), ) - .short_flag_aliases(&['M', 'B']), + .short_flag_aliases(['M', 'B']), ) .get_matches_from(vec!["myprog", "-Bt"]); assert_eq!(matches.subcommand_name().unwrap(), "some"); @@ -189,7 +189,7 @@ fn flag_subcommand_short_with_aliases_hyphen() { .long("test") .about("testing testing"), ) - .short_flag_aliases(&['-', '-', '-']), + .short_flag_aliases(['-', '-', '-']), ) .get_matches_from(vec!["myprog", "-Bt"]); } @@ -259,7 +259,7 @@ fn flag_subcommand_long_with_aliases() { .long("test") .about("testing testing"), ) - .long_flag_aliases(&["result", "someall"]), + .long_flag_aliases(["result", "someall"]), ) .get_matches_from(vec!["myprog", "--result", "--test"]); assert_eq!(matches.subcommand_name().unwrap(), "some"); diff --git a/tests/multiple_values.rs b/tests/multiple_values.rs index d8584bc68d5..3df5e0f8305 100644 --- a/tests/multiple_values.rs +++ b/tests/multiple_values.rs @@ -1257,7 +1257,7 @@ fn value_names_building_num_vals() { .arg( Arg::new("pos") .long("pos") - .value_names(&["who", "what", "why"]), + .value_names(["who", "what", "why"]), ) .try_get_matches_from(vec!["myprog", "--pos", "val1", "val2", "val3"]); @@ -1288,7 +1288,7 @@ fn value_names_building_num_vals_from_usage() { #[test] fn value_names_building_num_vals_for_positional() { let m = App::new("test") - .arg(Arg::new("pos").value_names(&["who", "what", "why"])) + .arg(Arg::new("pos").value_names(["who", "what", "why"])) .try_get_matches_from(vec!["myprog", "val1", "val2", "val3"]); assert!(m.is_ok(), "{:?}", m.unwrap_err().kind); @@ -1307,7 +1307,7 @@ fn number_of_values_preferred_over_value_names() { Arg::new("pos") .long("pos") .number_of_values(4) - .value_names(&["who", "what", "why"]), + .value_names(["who", "what", "why"]), ) .try_get_matches_from(vec!["myprog", "--pos", "val1", "val2", "val3", "val4"]); diff --git a/tests/subcommands.rs b/tests/subcommands.rs index 9d6f41d9978..41620815bd8 100644 --- a/tests/subcommands.rs +++ b/tests/subcommands.rs @@ -209,7 +209,7 @@ fn single_alias() { #[test] fn multiple_aliases() { let m = App::new("myprog") - .subcommand(App::new("test").aliases(&["do-stuff", "test-stuff"])) + .subcommand(App::new("test").aliases(["do-stuff", "test-stuff"])) .get_matches_from(vec!["myprog", "test-stuff"]); assert_eq!(m.subcommand_name(), Some("test")); }