From d3f5d7ce342cf9da10a68090fe45bfbc30ea1945 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 11 Feb 2022 14:10:29 -0600 Subject: [PATCH] fix: Clarify Arg/ArgGroup id's role This adjusts names. Adjusting the derive naming (and re-naming) is left to #2475. Fixes #3335 --- clap_complete/src/shells/zsh.rs | 4 ++-- clap_complete_fig/src/fig.rs | 2 +- clap_mangen/src/render.rs | 4 ++-- src/build/arg.rs | 16 ++++++++++++++-- src/build/arg_group.rs | 12 +++++++++--- src/build/debug_asserts.rs | 2 +- src/macros.rs | 18 ++++++++--------- tests/builder/help.rs | 7 ++----- tests/builder/tests.rs | 2 +- tests/derive/help.rs | 22 ++++++++++----------- tests/macros.rs | 34 ++++++++++++++++----------------- 11 files changed, 69 insertions(+), 54 deletions(-) diff --git a/clap_complete/src/shells/zsh.rs b/clap_complete/src/shells/zsh.rs index 3861bf6f120..0b6f7eda1ae 100644 --- a/clap_complete/src/shells/zsh.rs +++ b/clap_complete/src/shells/zsh.rs @@ -545,7 +545,7 @@ fn write_flags_of(p: &App, p_global: Option<&App>) -> String { let mut ret = vec![]; for f in utils::flags(p) { - debug!("write_flags_of:iter: f={}", f.get_name()); + debug!("write_flags_of:iter: f={}", f.get_id()); let help = f.get_help().map_or(String::new(), escape_help); let conflicts = arg_conflicts(p, &f, p_global); @@ -639,7 +639,7 @@ fn write_positionals_of(p: &App) -> String { let a = format!( "'{cardinality}:{name}{help}:{value_completion}' \\", cardinality = cardinality, - name = arg.get_name(), + name = arg.get_id(), help = arg .get_help() .map_or("".to_owned(), |v| " -- ".to_owned() + v) diff --git a/clap_complete_fig/src/fig.rs b/clap_complete_fig/src/fig.rs index 8dec8b892c4..95c3ef467f5 100644 --- a/clap_complete_fig/src/fig.rs +++ b/clap_complete_fig/src/fig.rs @@ -221,7 +221,7 @@ fn gen_args(arg: &Arg, indent: usize) -> String { buffer.push_str(&format!( "{{\n{:indent$} name: \"{}\",\n", "", - arg.get_name(), + arg.get_id(), indent = indent )); diff --git a/clap_mangen/src/render.rs b/clap_mangen/src/render.rs index df753ae302c..ced17bcdeea 100644 --- a/clap_mangen/src/render.rs +++ b/clap_mangen/src/render.rs @@ -61,7 +61,7 @@ pub(crate) fn synopsis(roff: &mut Roff, app: &clap::App) { for arg in app.get_positionals() { let (lhs, rhs) = option_markers(arg); line.push(roman(lhs)); - line.push(italic(arg.get_name())); + line.push(italic(arg.get_id())); line.push(roman(rhs)); line.push(roman(" ")); } @@ -120,7 +120,7 @@ pub(crate) fn options(roff: &mut Roff, app: &clap::App) { for pos in items.iter().filter(|a| a.is_positional()) { let (lhs, rhs) = option_markers(pos); - let name = format!("{}{}{}", lhs, pos.get_name(), rhs); + let name = format!("{}{}{}", lhs, pos.get_id(), rhs); let mut header = vec![bold(&name)]; diff --git a/src/build/arg.rs b/src/build/arg.rs index 499eade3f2a..4975ca4ae9b 100644 --- a/src/build/arg.rs +++ b/src/build/arg.rs @@ -117,13 +117,19 @@ impl<'help> Arg<'help> { /// /// See [`Arg::new`] for more details. #[must_use] - pub fn name>(mut self, n: S) -> Self { + pub fn id>(mut self, n: S) -> Self { let name = n.into(); self.id = Id::from(&*name); self.name = name; self } + /// Deprecated, replaced with [`Arg::id`] + #[deprecated(since = "3.1.0", note = "Replaced with `Arg::id`")] + pub fn name>(self, n: S) -> Self { + self.id(n) + } + /// Sets the short version of the argument without the preceding `-`. /// /// By default `V` and `h` are used by the auto-generated `version` and `help` arguments, @@ -4489,10 +4495,16 @@ impl<'help> Arg<'help> { impl<'help> Arg<'help> { /// Get the name of the argument #[inline] - pub fn get_name(&self) -> &'help str { + pub fn get_id(&self) -> &'help str { self.name } + /// Deprecated, replaced with [`Arg::get_id`] + #[deprecated(since = "3.1.0", note = "Replaced with `Arg::get_id`")] + pub fn get_name(&self) -> &'help str { + self.get_id() + } + /// Get the help specified for this argument, if any #[inline] pub fn get_help(&self) -> Option<&'help str> { diff --git a/src/build/arg_group.rs b/src/build/arg_group.rs index 0c4a0b85cab..20588578b49 100644 --- a/src/build/arg_group.rs +++ b/src/build/arg_group.rs @@ -109,7 +109,7 @@ impl<'help> ArgGroup<'help> { /// # ; /// ``` pub fn new>(n: S) -> Self { - ArgGroup::default().name(n) + ArgGroup::default().id(n) } /// Sets the group name. @@ -122,12 +122,18 @@ impl<'help> ArgGroup<'help> { /// # ; /// ``` #[must_use] - pub fn name>(mut self, n: S) -> Self { + pub fn id>(mut self, n: S) -> Self { self.name = n.into(); self.id = Id::from(self.name); self } + /// Deprecated, replaced with [`ArgGroup::id`] + #[deprecated(since = "3.1.0", note = "Replaced with `ArgGroup::id`")] + pub fn name>(self, n: S) -> Self { + self.id(n) + } + /// Adds an [argument] to this group by name /// /// # Examples @@ -495,7 +501,7 @@ impl<'help> From<&'help Yaml> for ArgGroup<'help> { "conflicts_with" => yaml_vec_or_str!(a, v, conflicts_with), "name" => { if let Some(ys) = v.as_str() { - a = a.name(ys); + a = a.id(ys); } a } diff --git a/src/build/debug_asserts.rs b/src/build/debug_asserts.rs index 85874b7f7e0..d4f51f2f782 100644 --- a/src/build/debug_asserts.rs +++ b/src/build/debug_asserts.rs @@ -648,7 +648,7 @@ fn assert_arg_flags(arg: &Arg) { )+ if !s.is_empty() { - panic!("Argument {:?}\n{}", arg.get_name(), s) + panic!("Argument {:?}\n{}", arg.get_id(), s) } } } diff --git a/src/macros.rs b/src/macros.rs index f0ade76381e..701a8ef152f 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -382,8 +382,8 @@ macro_rules! arg_impl { let mut arg = $arg; let long = $crate::arg_impl! { @string $long }; - if arg.get_name().is_empty() { - arg = arg.name(long); + if arg.get_id().is_empty() { + arg = arg.id(long); } arg.long(long) }) @@ -404,8 +404,8 @@ macro_rules! arg_impl { let mut arg = $arg; let long = $crate::arg_impl! { @string $long }; - if arg.get_name().is_empty() { - arg = arg.name(long); + if arg.get_id().is_empty() { + arg = arg.id(long); } arg.long(long) }) @@ -466,8 +466,8 @@ macro_rules! arg_impl { arg = arg.takes_value(true); let value_name = $crate::arg_impl! { @string $value_name }; - if arg.get_name().is_empty() { - arg = arg.name(value_name); + if arg.get_id().is_empty() { + arg = arg.id(value_name); } arg.value_name(value_name) }) @@ -496,8 +496,8 @@ macro_rules! arg_impl { arg = arg.takes_value(true); let value_name = $crate::arg_impl! { @string $value_name }; - if arg.get_name().is_empty() { - arg = arg.name(value_name); + if arg.get_id().is_empty() { + arg = arg.id(value_name); } arg.value_name(value_name) }) @@ -622,7 +622,7 @@ macro_rules! arg { let arg = $crate::arg_impl! { @arg ($crate::Arg::default()) $($tail)+ }; - debug_assert!(!arg.get_name().is_empty(), "Without a value or long flag, the `name:` prefix is required"); + debug_assert!(!arg.get_id().is_empty(), "Without a value or long flag, the `name:` prefix is required"); arg }}; } diff --git a/tests/builder/help.rs b/tests/builder/help.rs index a09e8152831..4c41af6ce89 100644 --- a/tests/builder/help.rs +++ b/tests/builder/help.rs @@ -2727,7 +2727,7 @@ fn disable_help_flag_affects_help_subcommand() { .find_subcommand("help") .unwrap() .get_arguments() - .map(|a| a.get_name()) + .map(|a| a.get_id()) .collect::>(); assert!( !args.contains(&"help"), @@ -2768,10 +2768,7 @@ fn help_without_short() { .arg(arg!(--help)); app._build_all(); - let help = app - .get_arguments() - .find(|a| a.get_name() == "help") - .unwrap(); + let help = app.get_arguments().find(|a| a.get_id() == "help").unwrap(); assert_eq!(help.get_short(), None); let m = app.try_get_matches_from(["test", "-h", "0x100"]).unwrap(); diff --git a/tests/builder/tests.rs b/tests/builder/tests.rs index 1f01341c9c3..4e2b334a861 100644 --- a/tests/builder/tests.rs +++ b/tests/builder/tests.rs @@ -400,7 +400,7 @@ fn mut_arg_all() { let mut app = utils::complex_app(); let arg_names = app .get_arguments() - .map(|a| a.get_name()) + .map(|a| a.get_id()) .filter(|a| *a != "version" && *a != "help") .collect::>(); diff --git a/tests/derive/help.rs b/tests/derive/help.rs index 54fca8a07f1..2f26b9e357d 100644 --- a/tests/derive/help.rs +++ b/tests/derive/help.rs @@ -16,13 +16,13 @@ fn arg_help_heading_applied() { let should_be_in_section_a = app .get_arguments() - .find(|a| a.get_name() == "should-be-in-section-a") + .find(|a| a.get_id() == "should-be-in-section-a") .unwrap(); assert_eq!(should_be_in_section_a.get_help_heading(), Some("HEADING A")); let should_be_in_section_b = app .get_arguments() - .find(|a| a.get_name() == "no-section") + .find(|a| a.get_id() == "no-section") .unwrap(); assert_eq!(should_be_in_section_b.get_help_heading(), None); } @@ -44,13 +44,13 @@ fn app_help_heading_applied() { let should_be_in_section_a = app .get_arguments() - .find(|a| a.get_name() == "should-be-in-section-a") + .find(|a| a.get_id() == "should-be-in-section-a") .unwrap(); assert_eq!(should_be_in_section_a.get_help_heading(), Some("HEADING A")); let should_be_in_default_section = app .get_arguments() - .find(|a| a.get_name() == "should-be-in-default-section") + .find(|a| a.get_id() == "should-be-in-default-section") .unwrap(); assert_eq!( should_be_in_default_section.get_help_heading(), @@ -121,19 +121,19 @@ fn app_help_heading_flattened() { let should_be_in_section_a = app .get_arguments() - .find(|a| a.get_name() == "should-be-in-section-a") + .find(|a| a.get_id() == "should-be-in-section-a") .unwrap(); assert_eq!(should_be_in_section_a.get_help_heading(), Some("HEADING A")); let should_be_in_section_b = app .get_arguments() - .find(|a| a.get_name() == "should-be-in-section-b") + .find(|a| a.get_id() == "should-be-in-section-b") .unwrap(); assert_eq!(should_be_in_section_b.get_help_heading(), Some("HEADING B")); let should_be_in_default_section = app .get_arguments() - .find(|a| a.get_name() == "should-be-in-default-section") + .find(|a| a.get_id() == "should-be-in-default-section") .unwrap(); assert_eq!(should_be_in_default_section.get_help_heading(), None); @@ -141,7 +141,7 @@ fn app_help_heading_flattened() { let should_be_in_sub_a = sub_a_two .get_arguments() - .find(|a| a.get_name() == "should-be-in-sub-a") + .find(|a| a.get_id() == "should-be-in-sub-a") .unwrap(); assert_eq!(should_be_in_sub_a.get_help_heading(), Some("SUB A")); @@ -149,7 +149,7 @@ fn app_help_heading_flattened() { let should_be_in_sub_b = sub_b_one .get_arguments() - .find(|a| a.get_name() == "should-be-in-sub-b") + .find(|a| a.get_id() == "should-be-in-sub-b") .unwrap(); assert_eq!(should_be_in_sub_b.get_help_heading(), Some("SUB B")); @@ -158,7 +158,7 @@ fn app_help_heading_flattened() { let should_be_in_sub_c = sub_c_one .get_arguments() - .find(|a| a.get_name() == "should-be-in-sub-c") + .find(|a| a.get_id() == "should-be-in-sub-c") .unwrap(); assert_eq!(should_be_in_sub_c.get_help_heading(), Some("SUB C")); } @@ -182,7 +182,7 @@ fn flatten_field_with_help_heading() { let should_be_in_section_a = app .get_arguments() - .find(|a| a.get_name() == "should-be-in-section-a") + .find(|a| a.get_id() == "should-be-in-section-a") .unwrap(); assert_eq!(should_be_in_section_a.get_help_heading(), Some("HEADING A")); } diff --git a/tests/macros.rs b/tests/macros.rs index 5b975435e39..f53ddc52573 100644 --- a/tests/macros.rs +++ b/tests/macros.rs @@ -293,7 +293,7 @@ mod arg { #[test] fn name_explicit() { let arg = clap::arg!(foo: --bar ); - assert_eq!(arg.get_name(), "foo"); + assert_eq!(arg.get_id(), "foo"); assert_eq!(arg.get_long(), Some("bar")); assert_eq!(arg.get_value_names(), Some(vec!["NUM"].as_slice())); assert!(arg.is_required_set()); @@ -302,7 +302,7 @@ mod arg { #[test] fn name_from_long() { let arg = clap::arg!(--bar ); - assert_eq!(arg.get_name(), "bar"); + assert_eq!(arg.get_id(), "bar"); assert_eq!(arg.get_long(), Some("bar")); assert_eq!(arg.get_value_names(), Some(vec!["NUM"].as_slice())); assert!(arg.is_required_set()); @@ -311,7 +311,7 @@ mod arg { #[test] fn name_from_value() { let arg = clap::arg!(); - assert_eq!(arg.get_name(), "NUM"); + assert_eq!(arg.get_id(), "NUM"); assert_eq!(arg.get_long(), None); assert_eq!(arg.get_value_names(), Some(vec!["NUM"].as_slice())); assert!(arg.is_required_set()); @@ -332,28 +332,28 @@ mod arg { #[test] fn short() { let arg = clap::arg!(foo: -b); - assert_eq!(arg.get_name(), "foo"); + assert_eq!(arg.get_id(), "foo"); assert_eq!(arg.get_short(), Some('b')); assert!(!arg.is_multiple_occurrences_set()); assert!(!arg.is_required_set()); assert_eq!(arg.get_help(), None); let arg = clap::arg!(foo: -'b'); - assert_eq!(arg.get_name(), "foo"); + assert_eq!(arg.get_id(), "foo"); assert_eq!(arg.get_short(), Some('b')); assert!(!arg.is_multiple_occurrences_set()); assert!(!arg.is_required_set()); assert_eq!(arg.get_help(), None); let arg = clap::arg!(foo: -b ...); - assert_eq!(arg.get_name(), "foo"); + assert_eq!(arg.get_id(), "foo"); assert_eq!(arg.get_short(), Some('b')); assert!(arg.is_multiple_occurrences_set()); assert!(!arg.is_required_set()); assert_eq!(arg.get_help(), None); let arg = clap::arg!(foo: -b "How to use it"); - assert_eq!(arg.get_name(), "foo"); + assert_eq!(arg.get_id(), "foo"); assert_eq!(arg.get_short(), Some('b')); assert!(!arg.is_multiple_occurrences_set()); assert!(!arg.is_required_set()); @@ -363,7 +363,7 @@ mod arg { #[test] fn short_and_long() { let arg = clap::arg!(foo: -b --hello); - assert_eq!(arg.get_name(), "foo"); + assert_eq!(arg.get_id(), "foo"); assert_eq!(arg.get_long(), Some("hello")); assert_eq!(arg.get_short(), Some('b')); assert!(!arg.is_multiple_occurrences_set()); @@ -371,7 +371,7 @@ mod arg { assert_eq!(arg.get_help(), None); let arg = clap::arg!(foo: -'b' --hello); - assert_eq!(arg.get_name(), "foo"); + assert_eq!(arg.get_id(), "foo"); assert_eq!(arg.get_long(), Some("hello")); assert_eq!(arg.get_short(), Some('b')); assert!(!arg.is_multiple_occurrences_set()); @@ -379,7 +379,7 @@ mod arg { assert_eq!(arg.get_help(), None); let arg = clap::arg!(foo: -b --hello ...); - assert_eq!(arg.get_name(), "foo"); + assert_eq!(arg.get_id(), "foo"); assert_eq!(arg.get_long(), Some("hello")); assert_eq!(arg.get_short(), Some('b')); assert!(arg.is_multiple_occurrences_set()); @@ -387,7 +387,7 @@ mod arg { assert_eq!(arg.get_help(), None); let arg = clap::arg!(foo: -b --hello "How to use it"); - assert_eq!(arg.get_name(), "foo"); + assert_eq!(arg.get_id(), "foo"); assert_eq!(arg.get_long(), Some("hello")); assert_eq!(arg.get_short(), Some('b')); assert!(!arg.is_multiple_occurrences_set()); @@ -398,42 +398,42 @@ mod arg { #[test] fn positional() { let arg = clap::arg!(); - assert_eq!(arg.get_name(), "NUM"); + assert_eq!(arg.get_id(), "NUM"); assert_eq!(arg.get_value_names(), Some(vec!["NUM"].as_slice())); assert!(!arg.is_multiple_occurrences_set()); assert!(arg.is_required_set()); assert_eq!(arg.get_help(), None); let arg = clap::arg!([NUM]); - assert_eq!(arg.get_name(), "NUM"); + assert_eq!(arg.get_id(), "NUM"); assert_eq!(arg.get_value_names(), Some(vec!["NUM"].as_slice())); assert!(!arg.is_multiple_occurrences_set()); assert!(!arg.is_required_set()); assert_eq!(arg.get_help(), None); let arg = clap::arg!(); - assert_eq!(arg.get_name(), "NUM"); + assert_eq!(arg.get_id(), "NUM"); assert_eq!(arg.get_value_names(), Some(vec!["NUM"].as_slice())); assert!(!arg.is_multiple_occurrences_set()); assert!(arg.is_required_set()); assert_eq!(arg.get_help(), None); let arg = clap::arg!(foo: ); - assert_eq!(arg.get_name(), "foo"); + assert_eq!(arg.get_id(), "foo"); assert_eq!(arg.get_value_names(), Some(vec!["NUM"].as_slice())); assert!(!arg.is_multiple_occurrences_set()); assert!(arg.is_required_set()); assert_eq!(arg.get_help(), None); let arg = clap::arg!( ...); - assert_eq!(arg.get_name(), "NUM"); + assert_eq!(arg.get_id(), "NUM"); assert_eq!(arg.get_value_names(), Some(vec!["NUM"].as_slice())); assert!(arg.is_multiple_occurrences_set()); assert!(arg.is_required_set()); assert_eq!(arg.get_help(), None); let arg = clap::arg!( "How to use it"); - assert_eq!(arg.get_name(), "NUM"); + assert_eq!(arg.get_id(), "NUM"); assert_eq!(arg.get_value_names(), Some(vec!["NUM"].as_slice())); assert!(!arg.is_multiple_occurrences_set()); assert!(arg.is_required_set());