diff --git a/clap_mangen/src/render.rs b/clap_mangen/src/render.rs index 1655af38a2a..b437125bd50 100644 --- a/clap_mangen/src/render.rs +++ b/clap_mangen/src/render.rs @@ -63,7 +63,7 @@ pub(crate) fn synopsis(roff: &mut Roff, cmd: &clap::Command) { if let Some(value) = arg.get_value_names() { line.push(italic(value.join(" "))); } else { - line.push(italic(arg.get_id())); + line.push(italic(arg.get_id().as_str())); } line.push(roman(rhs)); line.push(roman(" ")); @@ -129,7 +129,7 @@ pub(crate) fn options(roff: &mut Roff, cmd: &clap::Command) { if let Some(value) = pos.get_value_names() { header.push(italic(value.join(" "))); } else { - header.push(italic(pos.get_id())); + header.push(italic(pos.get_id().as_str())); }; header.push(roman(rhs)); diff --git a/src/builder/arg.rs b/src/builder/arg.rs index 7f023cd9e65..dd14a95e0bd 100644 --- a/src/builder/arg.rs +++ b/src/builder/arg.rs @@ -50,7 +50,6 @@ use crate::INTERNAL_ERROR_MSG; #[derive(Default, Clone)] pub struct Arg<'help> { pub(crate) id: Id, - name: &'help str, pub(crate) help: Option<&'help str>, pub(crate) long_help: Option<&'help str>, pub(crate) action: Option, @@ -102,7 +101,7 @@ impl<'help> Arg<'help> { /// # ; /// ``` /// [`Arg::action(ArgAction::Set)`]: Arg::action() - pub fn new(id: impl Into<&'help str>) -> Self { + pub fn new(id: impl Into) -> Self { Arg::default().id(id) } @@ -110,10 +109,8 @@ impl<'help> Arg<'help> { /// /// See [`Arg::new`] for more details. #[must_use] - pub fn id(mut self, id: impl Into<&'help str>) -> Self { - let id = id.into(); - self.id = Id::from(id); - self.name = id; + pub fn id(mut self, id: impl Into) -> Self { + self.id = id.into(); self } @@ -660,9 +657,8 @@ impl<'help> Arg<'help> { /// [Conflicting]: Arg::conflicts_with() /// [override]: Arg::overrides_with() #[must_use] - pub fn requires(mut self, arg_id: impl Into<&'help str>) -> Self { - self.requires - .push((ArgPredicate::IsPresent, arg_id.into().into())); + pub fn requires(mut self, arg_id: impl Into) -> Self { + self.requires.push((ArgPredicate::IsPresent, arg_id.into())); self } @@ -2443,8 +2439,8 @@ impl<'help> Arg<'help> { /// /// [`ArgGroup`]: crate::ArgGroup #[must_use] - pub fn group(mut self, group_id: impl Into<&'help str>) -> Self { - self.groups.push(group_id.into().into()); + pub fn group(mut self, group_id: impl Into) -> Self { + self.groups.push(group_id.into()); self } @@ -2484,9 +2480,8 @@ impl<'help> Arg<'help> { /// /// [`ArgGroup`]: crate::ArgGroup #[must_use] - pub fn groups(mut self, group_ids: impl IntoIterator>) -> Self { - self.groups - .extend(group_ids.into_iter().map(|n| n.into().into())); + pub fn groups(mut self, group_ids: impl IntoIterator>) -> Self { + self.groups.extend(group_ids.into_iter().map(Into::into)); self } @@ -2605,7 +2600,7 @@ impl<'help> Arg<'help> { #[must_use] pub fn default_value_if( self, - arg_id: impl Into<&'help str>, + arg_id: impl Into, val: Option<&'help str>, default: Option<&'help str>, ) -> Self { @@ -2620,12 +2615,12 @@ impl<'help> Arg<'help> { #[must_use] pub fn default_value_if_os( mut self, - arg_id: impl Into<&'help str>, + arg_id: impl Into, val: Option<&'help OsStr>, default: Option<&'help OsStr>, ) -> Self { self.default_vals_ifs - .push((arg_id.into().into(), val.into(), default)); + .push((arg_id.into(), val.into(), default)); self } @@ -2712,13 +2707,7 @@ impl<'help> Arg<'help> { #[must_use] pub fn default_value_ifs( mut self, - ifs: impl IntoIterator< - Item = ( - impl Into<&'help str>, - Option<&'help str>, - Option<&'help str>, - ), - >, + ifs: impl IntoIterator, Option<&'help str>, Option<&'help str>)>, ) -> Self { for (arg, val, default) in ifs { self = self.default_value_if_os(arg, val.map(OsStr::new), default.map(OsStr::new)); @@ -2734,13 +2723,7 @@ impl<'help> Arg<'help> { #[must_use] pub fn default_value_ifs_os( mut self, - ifs: impl IntoIterator< - Item = ( - impl Into<&'help str>, - Option<&'help OsStr>, - Option<&'help OsStr>, - ), - >, + ifs: impl IntoIterator, Option<&'help OsStr>, Option<&'help OsStr>)>, ) -> Self { for (arg, val, default) in ifs { self = self.default_value_if_os(arg.into(), val, default); @@ -2802,8 +2785,8 @@ impl<'help> Arg<'help> { /// ``` /// [required]: Arg::required() #[must_use] - pub fn required_unless_present(mut self, arg_id: impl Into<&'help str>) -> Self { - self.r_unless.push(arg_id.into().into()); + pub fn required_unless_present(mut self, arg_id: impl Into) -> Self { + self.r_unless.push(arg_id.into()); self } @@ -2877,10 +2860,9 @@ impl<'help> Arg<'help> { #[must_use] pub fn required_unless_present_all( mut self, - names: impl IntoIterator>, + names: impl IntoIterator>, ) -> Self { - self.r_unless_all - .extend(names.into_iter().map(|n| n.into().into())); + self.r_unless_all.extend(names.into_iter().map(Into::into)); self } @@ -2956,10 +2938,9 @@ impl<'help> Arg<'help> { #[must_use] pub fn required_unless_present_any( mut self, - names: impl IntoIterator>, + names: impl IntoIterator>, ) -> Self { - self.r_unless - .extend(names.into_iter().map(|n| n.into().into())); + self.r_unless.extend(names.into_iter().map(Into::into)); self } @@ -3043,8 +3024,8 @@ impl<'help> Arg<'help> { /// [Conflicting]: Arg::conflicts_with() /// [required]: Arg::required() #[must_use] - pub fn required_if_eq(mut self, arg_id: impl Into<&'help str>, val: &'help str) -> Self { - self.r_ifs.push((arg_id.into().into(), val)); + pub fn required_if_eq(mut self, arg_id: impl Into, val: &'help str) -> Self { + self.r_ifs.push((arg_id.into(), val)); self } @@ -3124,10 +3105,10 @@ impl<'help> Arg<'help> { #[must_use] pub fn required_if_eq_any( mut self, - ifs: impl IntoIterator, &'help str)>, + ifs: impl IntoIterator, &'help str)>, ) -> Self { self.r_ifs - .extend(ifs.into_iter().map(|(id, val)| (id.into().into(), val))); + .extend(ifs.into_iter().map(|(id, val)| (id.into(), val))); self } @@ -3205,10 +3186,10 @@ impl<'help> Arg<'help> { #[must_use] pub fn required_if_eq_all( mut self, - ifs: impl IntoIterator, &'help str)>, + ifs: impl IntoIterator, &'help str)>, ) -> Self { self.r_ifs_all - .extend(ifs.into_iter().map(|(id, val)| (id.into().into(), val))); + .extend(ifs.into_iter().map(|(id, val)| (id.into(), val))); self } @@ -3268,9 +3249,9 @@ impl<'help> Arg<'help> { /// [Conflicting]: Arg::conflicts_with() /// [override]: Arg::overrides_with() #[must_use] - pub fn requires_if(mut self, val: &'help str, arg_id: impl Into<&'help str>) -> Self { + pub fn requires_if(mut self, val: &'help str, arg_id: impl Into) -> Self { self.requires - .push((ArgPredicate::Equals(OsStr::new(val)), arg_id.into().into())); + .push((ArgPredicate::Equals(OsStr::new(val)), arg_id.into())); self } @@ -3321,11 +3302,11 @@ impl<'help> Arg<'help> { #[must_use] pub fn requires_ifs( mut self, - ifs: impl IntoIterator)>, + ifs: impl IntoIterator)>, ) -> Self { self.requires.extend( ifs.into_iter() - .map(|(val, arg)| (ArgPredicate::Equals(OsStr::new(val)), arg.into().into())), + .map(|(val, arg)| (ArgPredicate::Equals(OsStr::new(val)), arg.into())), ); self } @@ -3389,11 +3370,11 @@ impl<'help> Arg<'help> { /// [Conflicting]: Arg::conflicts_with() /// [override]: Arg::overrides_with() #[must_use] - pub fn requires_all(mut self, names: impl IntoIterator>) -> Self { + pub fn requires_all(mut self, names: impl IntoIterator>) -> Self { self.requires.extend( names .into_iter() - .map(|s| (ArgPredicate::IsPresent, s.into().into())), + .map(|s| (ArgPredicate::IsPresent, s.into())), ); self } @@ -3443,8 +3424,8 @@ impl<'help> Arg<'help> { /// [`Arg::conflicts_with_all(names)`]: Arg::conflicts_with_all() /// [`Arg::exclusive(true)`]: Arg::exclusive() #[must_use] - pub fn conflicts_with(mut self, arg_id: impl Into<&'help str>) -> Self { - self.blacklist.push(arg_id.into().into()); + pub fn conflicts_with(mut self, arg_id: impl Into) -> Self { + self.blacklist.push(arg_id.into()); self } @@ -3493,12 +3474,8 @@ impl<'help> Arg<'help> { /// [`Arg::conflicts_with`]: Arg::conflicts_with() /// [`Arg::exclusive(true)`]: Arg::exclusive() #[must_use] - pub fn conflicts_with_all( - mut self, - names: impl IntoIterator>, - ) -> Self { - self.blacklist - .extend(names.into_iter().map(|n| n.into().into())); + pub fn conflicts_with_all(mut self, names: impl IntoIterator>) -> Self { + self.blacklist.extend(names.into_iter().map(Into::into)); self } @@ -3535,8 +3512,8 @@ impl<'help> Arg<'help> { /// assert!(!*m.get_one::("flag").unwrap()); /// ``` #[must_use] - pub fn overrides_with(mut self, arg_id: impl Into<&'help str>) -> Self { - self.overrides.push(arg_id.into().into()); + pub fn overrides_with(mut self, arg_id: impl Into) -> Self { + self.overrides.push(arg_id.into()); self } @@ -3571,12 +3548,8 @@ impl<'help> Arg<'help> { /// assert!(!*m.get_one::("flag").unwrap()); /// ``` #[must_use] - pub fn overrides_with_all( - mut self, - names: impl IntoIterator>, - ) -> Self { - self.overrides - .extend(names.into_iter().map(|n| n.into().into())); + pub fn overrides_with_all(mut self, names: impl IntoIterator>) -> Self { + self.overrides.extend(names.into_iter().map(Into::into)); self } } @@ -3585,8 +3558,8 @@ impl<'help> Arg<'help> { impl<'help> Arg<'help> { /// Get the name of the argument #[inline] - pub fn get_id(&self) -> &'help str { - self.name + pub fn get_id(&self) -> &Id { + &self.id } /// Get the help specified for this argument, if any @@ -4017,7 +3990,7 @@ impl<'help> Arg<'help> { } } else { debug!("Arg::name_no_brackets: just name"); - Cow::Borrowed(self.get_id()) + Cow::Borrowed(self.get_id().as_str()) } } @@ -4051,7 +4024,7 @@ impl<'help> PartialOrd for Arg<'help> { impl<'help> Ord for Arg<'help> { fn cmp(&self, other: &Arg) -> Ordering { - self.get_id().cmp(other.get_id()) + self.id.cmp(&other.id) } } @@ -4110,7 +4083,6 @@ impl<'help> fmt::Debug for Arg<'help> { #[allow(unused_mut)] let mut ds = ds .field("id", &self.id) - .field("name", &self.name) .field("help", &self.help) .field("long_help", &self.long_help) .field("action", &self.action) @@ -4155,7 +4127,7 @@ pub(crate) fn render_arg_val(arg: &Arg) -> String { let arg_name_storage; let val_names = if arg.val_names.is_empty() { - arg_name_storage = [arg.get_id()]; + arg_name_storage = [arg.get_id().as_str()]; &arg_name_storage } else { arg.val_names.as_slice() diff --git a/src/builder/arg_group.rs b/src/builder/arg_group.rs index a376e131c4b..ffc88c2c1ca 100644 --- a/src/builder/arg_group.rs +++ b/src/builder/arg_group.rs @@ -75,9 +75,8 @@ use crate::util::Id; /// [conflict]: crate::Arg::conflicts_with() /// [requirement]: crate::Arg::requires() #[derive(Default, Clone, Debug, PartialEq, Eq)] -pub struct ArgGroup<'help> { +pub struct ArgGroup { pub(crate) id: Id, - name: &'help str, pub(crate) args: Vec, pub(crate) required: bool, pub(crate) requires: Vec, @@ -86,14 +85,7 @@ pub struct ArgGroup<'help> { } /// # Builder -impl<'help> ArgGroup<'help> { - pub(crate) fn with_id(id: Id) -> Self { - ArgGroup { - id, - ..ArgGroup::default() - } - } - +impl ArgGroup { /// Create a `ArgGroup` using a unique name. /// /// The name will be used to get values from the group or refer to the group inside of conflict @@ -106,7 +98,7 @@ impl<'help> ArgGroup<'help> { /// ArgGroup::new("config") /// # ; /// ``` - pub fn new(id: impl Into<&'help str>) -> Self { + pub fn new(id: impl Into) -> Self { ArgGroup::default().id(id) } @@ -120,10 +112,8 @@ impl<'help> ArgGroup<'help> { /// # ; /// ``` #[must_use] - pub fn id(mut self, id: impl Into<&'help str>) -> Self { - let id = id.into(); + pub fn id(mut self, id: impl Into) -> Self { self.id = id.into(); - self.name = id; self } @@ -151,8 +141,8 @@ impl<'help> ArgGroup<'help> { /// ``` /// [argument]: crate::Arg #[must_use] - pub fn arg(mut self, arg_id: impl Into<&'help str>) -> Self { - self.args.push(arg_id.into().into()); + pub fn arg(mut self, arg_id: impl Into) -> Self { + self.args.push(arg_id.into()); self } @@ -179,7 +169,7 @@ impl<'help> ArgGroup<'help> { /// ``` /// [arguments]: crate::Arg #[must_use] - pub fn args(mut self, ns: impl IntoIterator>) -> Self { + pub fn args(mut self, ns: impl IntoIterator>) -> Self { for n in ns { self = self.arg(n); } @@ -317,8 +307,8 @@ impl<'help> ArgGroup<'help> { /// [required group]: ArgGroup::required() /// [argument requirement rules]: crate::Arg::requires() #[must_use] - pub fn requires(mut self, id: impl Into<&'help str>) -> Self { - self.requires.push(id.into().into()); + pub fn requires(mut self, id: impl Into) -> Self { + self.requires.push(id.into()); self } @@ -360,7 +350,7 @@ impl<'help> ArgGroup<'help> { /// [required group]: ArgGroup::required() /// [argument requirement rules]: crate::Arg::requires_all() #[must_use] - pub fn requires_all(mut self, ns: impl IntoIterator>) -> Self { + pub fn requires_all(mut self, ns: impl IntoIterator>) -> Self { for n in ns { self = self.requires(n); } @@ -400,8 +390,8 @@ impl<'help> ArgGroup<'help> { /// ``` /// [argument exclusion rules]: crate::Arg::conflicts_with() #[must_use] - pub fn conflicts_with(mut self, id: impl Into<&'help str>) -> Self { - self.conflicts.push(id.into().into()); + pub fn conflicts_with(mut self, id: impl Into) -> Self { + self.conflicts.push(id.into()); self } @@ -442,10 +432,7 @@ impl<'help> ArgGroup<'help> { /// /// [argument exclusion rules]: crate::Arg::conflicts_with_all() #[must_use] - pub fn conflicts_with_all( - mut self, - ns: impl IntoIterator>, - ) -> Self { + pub fn conflicts_with_all(mut self, ns: impl IntoIterator>) -> Self { for n in ns { self = self.conflicts_with(n); } @@ -454,16 +441,16 @@ impl<'help> ArgGroup<'help> { } /// # Reflection -impl<'help> ArgGroup<'help> { +impl ArgGroup { /// Get the name of the group #[inline] - pub fn get_id(&self) -> &'help str { - self.name + pub fn get_id(&self) -> &Id { + &self.id } } -impl<'help> From<&'_ ArgGroup<'help>> for ArgGroup<'help> { - fn from(g: &Self) -> Self { +impl From<&'_ ArgGroup> for ArgGroup { + fn from(g: &ArgGroup) -> Self { g.clone() } } diff --git a/src/builder/command.rs b/src/builder/command.rs index 32f75762d01..393ce3dd6e3 100644 --- a/src/builder/command.rs +++ b/src/builder/command.rs @@ -93,7 +93,7 @@ pub struct Command<'help> { args: MKeyMap<'help>, subcommands: Vec>, replacers: FlatMap<&'help str, &'help [&'help str]>, - groups: Vec>, + groups: Vec, current_help_heading: Option<&'help str>, current_disp_ord: Option, subcommand_value_name: Option<&'help str>, @@ -230,17 +230,15 @@ impl<'help> Command<'help> { /// assert!(res.is_ok()); /// ``` #[must_use] - pub fn mut_arg(mut self, arg_id: impl Into<&'help str>, f: F) -> Self + pub fn mut_arg(mut self, arg_id: impl Into, f: F) -> Self where F: FnOnce(Arg<'help>) -> Arg<'help>, { - let arg_id: &str = arg_id.into(); - let id = Id::from(arg_id); - + let id = arg_id.into(); let a = self .args - .remove_by_name(&id) - .unwrap_or_else(|| Arg::new(arg_id)); + .remove_by_name(id.as_str()) + .unwrap_or_else(|| Arg::new(id)); self.args.push(f(a)); self @@ -320,7 +318,7 @@ impl<'help> Command<'help> { /// ``` #[inline] #[must_use] - pub fn group>>(mut self, group: G) -> Self { + pub fn group>(mut self, group: G) -> Self { self.groups.push(group.into()); self } @@ -351,7 +349,7 @@ impl<'help> Command<'help> { pub fn groups(mut self, groups: I) -> Self where I: IntoIterator, - T: Into>, + T: Into, { for g in groups.into_iter() { self = self.group(g.into()); @@ -3383,7 +3381,7 @@ impl<'help> Command<'help> { /// Iterate through the set of groups. #[inline] - pub fn get_groups(&self) -> impl Iterator> { + pub fn get_groups(&self) -> impl Iterator { self.groups.iter() } @@ -3763,10 +3761,10 @@ impl<'help> Command<'help> { // Fill in the groups for g in &a.groups { if let Some(ag) = self.groups.iter_mut().find(|grp| grp.id == *g) { - ag.args.push(a.id.clone()); + ag.args.push(a.get_id().clone()); } else { - let mut ag = ArgGroup::with_id(g.clone()); - ag.args.push(a.id.clone()); + let mut ag = ArgGroup::new(g); + ag.args.push(a.get_id().clone()); self.groups.push(ag); } } @@ -3992,11 +3990,11 @@ impl<'help> Command<'help> { pub(crate) fn _panic_on_missing_help(&self, help_required_globally: bool) { if self.is_set(AppSettings::HelpExpected) || help_required_globally { - let args_missing_help: Vec = self + let args_missing_help: Vec = self .args .args() .filter(|arg| arg.help.is_none() && arg.long_help.is_none()) - .map(|arg| arg.get_id().to_owned()) + .map(|arg| arg.get_id().clone()) .collect(); assert!(args_missing_help.is_empty(), @@ -4021,9 +4019,9 @@ impl<'help> Command<'help> { // just in case #[allow(unused)] - fn two_groups_of(&self, condition: F) -> Option<(&ArgGroup<'help>, &ArgGroup<'help>)> + fn two_groups_of(&self, condition: F) -> Option<(&ArgGroup, &ArgGroup)> where - F: Fn(&ArgGroup<'help>) -> bool, + F: Fn(&ArgGroup) -> bool, { two_elements_of(self.groups.iter().filter(|a| condition(a))) } @@ -4088,7 +4086,7 @@ impl<'help> Command<'help> { if !self.is_disable_help_flag_set() { debug!("Command::_check_help_and_version: Building default --help"); - let arg = Arg::new("help") + let arg = Arg::new(Id::HELP) .short('h') .long("help") .action(ArgAction::Help) @@ -4099,7 +4097,7 @@ impl<'help> Command<'help> { } if !self.is_disable_version_flag_set() { debug!("Command::_check_help_and_version: Building default --version"); - let arg = Arg::new("version") + let arg = Arg::new(Id::VERSION) .short('V') .long("version") .action(ArgAction::Version) @@ -4273,7 +4271,7 @@ impl<'help> Command<'help> { .map(|grp| grp.id.clone()) } - pub(crate) fn find_group(&self, group_id: &Id) -> Option<&ArgGroup<'help>> { + pub(crate) fn find_group(&self, group_id: &Id) -> Option<&ArgGroup> { self.groups.iter().find(|g| g.id == *group_id) } diff --git a/src/builder/debug_asserts.rs b/src/builder/debug_asserts.rs index 5c181db5c49..77b0dbb5993 100644 --- a/src/builder/debug_asserts.rs +++ b/src/builder/debug_asserts.rs @@ -67,20 +67,26 @@ pub(crate) fn assert_app(cmd: &Command) { ); if let Some(s) = arg.short.as_ref() { - short_flags.push(Flag::Arg(format!("-{}", s), arg.get_id())); + short_flags.push(Flag::Arg(format!("-{}", s), arg.get_id().as_str())); } for (short_alias, _) in &arg.short_aliases { - short_flags.push(Flag::Arg(format!("-{}", short_alias), arg.get_id())); + short_flags.push(Flag::Arg( + format!("-{}", short_alias), + arg.get_id().as_str(), + )); } if let Some(l) = arg.long.as_ref() { assert!(!l.starts_with('-'), "Argument {}: long {:?} must not start with a `-`, that will be handled by the parser", arg.get_id(), l); - long_flags.push(Flag::Arg(format!("--{}", l), arg.get_id())); + long_flags.push(Flag::Arg(format!("--{}", l), arg.get_id().as_str())); } for (long_alias, _) in &arg.aliases { - long_flags.push(Flag::Arg(format!("--{}", long_alias), arg.get_id())); + long_flags.push(Flag::Arg( + format!("--{}", long_alias), + arg.get_id().as_str(), + )); } // Name conflicts @@ -144,7 +150,7 @@ pub(crate) fn assert_app(cmd: &Command) { for req in &arg.requires { assert!( cmd.id_exists(&req.1), - "Command {}: Argument or group '{:?}' specified in 'requires*' for '{}' does not exist", + "Command {}: Argument or group '{}' specified in 'requires*' for '{}' does not exist", cmd.get_name(), req.1, arg.get_id(), @@ -159,7 +165,7 @@ pub(crate) fn assert_app(cmd: &Command) { ); assert!( cmd.id_exists(&req.0), - "Command {}: Argument or group '{:?}' specified in 'required_if_eq*' for '{}' does not exist", + "Command {}: Argument or group '{}' specified in 'required_if_eq*' for '{}' does not exist", cmd.get_name(), req.0, arg.get_id() @@ -174,7 +180,7 @@ pub(crate) fn assert_app(cmd: &Command) { ); assert!( cmd.id_exists(&req.0), - "Command {}: Argument or group '{:?}' specified in 'required_if_eq_all' for '{}' does not exist", + "Command {}: Argument or group '{}' specified in 'required_if_eq_all' for '{}' does not exist", cmd.get_name(), req.0, arg.get_id() @@ -189,7 +195,7 @@ pub(crate) fn assert_app(cmd: &Command) { ); assert!( cmd.id_exists(req), - "Command {}: Argument or group '{:?}' specified in 'required_unless*' for '{}' does not exist", + "Command {}: Argument or group '{}' specified in 'required_unless*' for '{}' does not exist", cmd.get_name(), req, arg.get_id(), @@ -204,7 +210,7 @@ pub(crate) fn assert_app(cmd: &Command) { ); assert!( cmd.id_exists(req), - "Command {}: Argument or group '{:?}' specified in 'required_unless*' for '{}' does not exist", + "Command {}: Argument or group '{}' specified in 'required_unless*' for '{}' does not exist", cmd.get_name(), req, arg.get_id(), @@ -215,7 +221,7 @@ pub(crate) fn assert_app(cmd: &Command) { for req in &arg.blacklist { assert!( cmd.id_exists(req), - "Command {}: Argument or group '{:?}' specified in 'conflicts_with*' for '{}' does not exist", + "Command {}: Argument or group '{}' specified in 'conflicts_with*' for '{}' does not exist", cmd.get_name(), req, arg.get_id(), @@ -226,7 +232,7 @@ pub(crate) fn assert_app(cmd: &Command) { for req in &arg.overrides { assert!( cmd.id_exists(req), - "Command {}: Argument or group '{:?}' specified in 'overrides_with*' for '{}' does not exist", + "Command {}: Argument or group '{}' specified in 'overrides_with*' for '{}' does not exist", cmd.get_name(), req, arg.get_id(), @@ -283,7 +289,7 @@ pub(crate) fn assert_app(cmd: &Command) { // Groups should not have naming conflicts with Args assert!( - !cmd.get_arguments().any(|x| x.id == group.id), + !cmd.get_arguments().any(|x| x.get_id() == group.get_id()), "Command {}: Argument group name '{}' must not conflict with argument name", cmd.get_name(), group.get_id(), @@ -292,8 +298,8 @@ pub(crate) fn assert_app(cmd: &Command) { for arg in &group.args { // Args listed inside groups should exist assert!( - cmd.get_arguments().any(|x| x.id == *arg), - "Command {}: Argument group '{}' contains non-existent argument '{:?}'", + cmd.get_arguments().any(|x| x.get_id() == arg), + "Command {}: Argument group '{}' contains non-existent argument '{}'", cmd.get_name(), group.get_id(), arg @@ -350,12 +356,10 @@ pub(crate) fn assert_app(cmd: &Command) { } fn duplicate_tip(cmd: &Command<'_>, first: &Arg<'_>, second: &Arg<'_>) -> &'static str { - if !cmd.is_disable_help_flag_set() - && (first.id == Id::help_hash() || second.id == Id::help_hash()) - { + if !cmd.is_disable_help_flag_set() && (first.id == Id::HELP || second.id == Id::HELP) { " (call `cmd.disable_help_flag(true)` to remove the auto-generated `--help`)" } else if !cmd.is_disable_version_flag_set() - && (first.id == Id::version_hash() || second.id == Id::version_hash()) + && (first.id == Id::VERSION || second.id == Id::VERSION) { " (call `cmd.disable_version_flag(true)` to remove the auto-generated `--version`)" } else { @@ -703,9 +707,9 @@ fn assert_arg(arg: &Arg) { arg.is_takes_value_set(), "Argument '{}` is positional, it must take a value{}", arg.get_id(), - if arg.id == Id::help_hash() { + if arg.id == Id::HELP { " (`mut_arg` no longer works with implicit `--help`)" - } else if arg.id == Id::version_hash() { + } else if arg.id == Id::VERSION { " (`mut_arg` no longer works with implicit `--version`)" } else { "" diff --git a/src/mkeymap.rs b/src/mkeymap.rs index 56f22b6977f..28eb268bc61 100644 --- a/src/mkeymap.rs +++ b/src/mkeymap.rs @@ -2,7 +2,6 @@ use std::iter::Iterator; use std::ops::Index; use std::{ffi::OsStr, ffi::OsString}; -use crate::util::Id; use crate::Arg; use crate::INTERNAL_ERROR_MSG; @@ -144,10 +143,10 @@ impl<'help> MKeyMap<'help> { /// Remove an arg in the graph by Id, usually used by `mut_arg`. Return /// `Some(arg)` if removed. - pub(crate) fn remove_by_name(&mut self, name: &Id) -> Option> { + pub(crate) fn remove_by_name(&mut self, name: &str) -> Option> { self.args .iter() - .position(|arg| &arg.id == name) + .position(|arg| arg.id == name) // since it's a cold function, using this wouldn't hurt much .map(|i| self.args.remove(i)) } diff --git a/src/output/help.rs b/src/output/help.rs index b6a867bfce9..26620a05e86 100644 --- a/src/output/help.rs +++ b/src/output/help.rs @@ -217,7 +217,7 @@ impl<'help, 'cmd, 'writer> Help<'help, 'cmd, 'writer> { x.to_string() } else { let mut s = '{'.to_string(); - s.push_str(arg.get_id()); + s.push_str(arg.id.as_str()); s }; ord_v.push((arg.get_display_order(), key, arg)); diff --git a/src/parser/arg_matcher.rs b/src/parser/arg_matcher.rs index c872f45fb96..ed03e35d76f 100644 --- a/src/parser/arg_matcher.rs +++ b/src/parser/arg_matcher.rs @@ -170,9 +170,11 @@ impl ArgMatcher { } pub(crate) fn start_occurrence_of_external(&mut self, cmd: &crate::Command) { - let id = Id::empty_hash(); + let id = Id::EXTERNAL; debug!("ArgMatcher::start_occurrence_of_external: id={:?}", id,); - let ma = self.entry(id).or_insert(MatchedArg::new_external(cmd)); + let ma = self + .entry(id.into()) + .or_insert(MatchedArg::new_external(cmd)); debug_assert_eq!( ma.type_id(), Some( diff --git a/src/parser/error.rs b/src/parser/error.rs index caeba4b8fee..fb143f88df6 100644 --- a/src/parser/error.rs +++ b/src/parser/error.rs @@ -1,5 +1,3 @@ -use crate::util::Id; - /// Violation of [`ArgMatches`][crate::ArgMatches] assumptions #[derive(Clone, Debug)] #[allow(missing_copy_implementations)] // We might add non-Copy types in the future @@ -22,7 +20,7 @@ pub enum MatchesError { impl MatchesError { #[track_caller] - pub(crate) fn unwrap(id: &Id, r: Result) -> T { + pub(crate) fn unwrap(id: &str, r: Result) -> T { let err = match r { Ok(t) => { return t; @@ -30,7 +28,7 @@ impl MatchesError { Err(err) => err, }; panic!( - "Mismatch between definition and access of `{:?}`. {}", + "Mismatch between definition and access of `{}`. {}", id, err ) } diff --git a/src/parser/matches/arg_matches.rs b/src/parser/matches/arg_matches.rs index 5655c26eb0f..7b77a54500e 100644 --- a/src/parser/matches/arg_matches.rs +++ b/src/parser/matches/arg_matches.rs @@ -12,7 +12,7 @@ use crate::parser::MatchedArg; use crate::parser::MatchesError; use crate::parser::ValueSource; use crate::util::FlatMap; -use crate::util::{Id, Key}; +use crate::util::Id; use crate::INTERNAL_ERROR_MSG; /// Container for parse results. @@ -108,8 +108,7 @@ impl ArgMatches { /// [`default_value`]: crate::Arg::default_value() #[track_caller] pub fn get_one(&self, id: &str) -> Option<&T> { - let internal_id = Id::from(id); - MatchesError::unwrap(&internal_id, self.try_get_one(id)) + MatchesError::unwrap(id, self.try_get_one(id)) } /// Iterate over values of a specific option or positional argument. @@ -149,8 +148,7 @@ impl ArgMatches { &self, id: &str, ) -> Option> { - let internal_id = Id::from(id); - MatchesError::unwrap(&internal_id, self.try_get_many(id)) + MatchesError::unwrap(id, self.try_get_many(id)) } /// Iterate over the original argument values. @@ -196,8 +194,7 @@ impl ArgMatches { /// [`String`]: std::string::String #[track_caller] pub fn get_raw(&self, id: &str) -> Option> { - let internal_id = Id::from(id); - MatchesError::unwrap(&internal_id, self.try_get_raw(id)) + MatchesError::unwrap(id, self.try_get_raw(id)) } /// Returns the value of a specific option or positional argument. @@ -235,8 +232,7 @@ impl ArgMatches { /// [`default_value`]: crate::Arg::default_value() #[track_caller] pub fn remove_one(&mut self, id: &str) -> Option { - let internal_id = Id::from(id); - MatchesError::unwrap(&internal_id, self.try_remove_one(id)) + MatchesError::unwrap(id, self.try_remove_one(id)) } /// Return values of a specific option or positional argument. @@ -274,8 +270,7 @@ impl ArgMatches { &mut self, id: &str, ) -> Option> { - let internal_id = Id::from(id); - MatchesError::unwrap(&internal_id, self.try_remove_many(id)) + MatchesError::unwrap(id, self.try_remove_many(id)) } /// Check if values are present for the argument or group id @@ -305,8 +300,7 @@ impl ArgMatches { /// /// [`default_value`]: crate::Arg::default_value() pub fn contains_id(&self, id: &str) -> bool { - let internal_id = Id::from(id); - MatchesError::unwrap(&internal_id, self.try_contains_id(id)) + MatchesError::unwrap(id, self.try_contains_id(id)) } /// Check if any args were present on the command line @@ -367,9 +361,8 @@ impl ArgMatches { /// [`Iterator`]: std::iter::Iterator #[cfg(feature = "unstable-grouped")] #[cfg_attr(debug_assertions, track_caller)] - pub fn grouped_values_of(&self, id: T) -> Option { - let id = Id::from(id); - let arg = self.get_arg(&id)?; + pub fn grouped_values_of(&self, id: &str) -> Option { + let arg = self.get_arg(id)?; let v = GroupedValues { iter: arg.vals().map(|g| g.iter().map(unwrap_string).collect()), len: arg.vals().len(), @@ -401,10 +394,8 @@ impl ArgMatches { /// /// [`default_value`]: crate::Arg::default_value() #[cfg_attr(debug_assertions, track_caller)] - pub fn value_source(&self, id: T) -> Option { - let id = Id::from(id); - - let value = self.get_arg(&id); + pub fn value_source(&self, id: &str) -> Option { + let value = self.get_arg(id); value.and_then(MatchedArg::source) } @@ -552,8 +543,8 @@ impl ArgMatches { /// ``` /// [delimiter]: crate::Arg::value_delimiter() #[cfg_attr(debug_assertions, track_caller)] - pub fn index_of(&self, id: T) -> Option { - let arg = self.get_arg(&Id::from(id))?; + pub fn index_of(&self, id: &str) -> Option { + let arg = self.get_arg(id)?; let i = arg.get_index(0)?; Some(i) } @@ -634,8 +625,8 @@ impl ArgMatches { /// [`ArgMatches::index_of`]: ArgMatches::index_of() /// [delimiter]: Arg::value_delimiter() #[cfg_attr(debug_assertions, track_caller)] - pub fn indices_of(&self, id: T) -> Option> { - let arg = self.get_arg(&Id::from(id))?; + pub fn indices_of(&self, id: &str) -> Option> { + let arg = self.get_arg(id)?; let i = Indices { iter: arg.indices(), len: arg.num_vals(), @@ -843,7 +834,7 @@ impl ArgMatches { #[cfg(debug_assertions)] { let _name = _name.to_owned(); - _name.is_empty() || self.valid_subcommands.contains(&_name) + _name == String::default() || self.valid_subcommands.contains(&_name) } #[cfg(not(debug_assertions))] { @@ -859,8 +850,7 @@ impl ArgMatches { &self, id: &str, ) -> Result, MatchesError> { - let id = Id::from(id); - let arg = self.try_get_arg_t::(&id)?; + let arg = self.try_get_arg_t::(id)?; let value = match arg.and_then(|a| a.first()) { Some(value) => value, None => { @@ -878,8 +868,7 @@ impl ArgMatches { &self, id: &str, ) -> Result>, MatchesError> { - let id = Id::from(id); - let arg = match self.try_get_arg_t::(&id)? { + let arg = match self.try_get_arg_t::(id)? { Some(arg) => arg, None => return Ok(None), }; @@ -895,8 +884,7 @@ impl ArgMatches { /// Non-panicking version of [`ArgMatches::get_raw`] pub fn try_get_raw(&self, id: &str) -> Result>, MatchesError> { - let id = Id::from(id); - let arg = match self.try_get_arg(&id)? { + let arg = match self.try_get_arg(id)? { Some(arg) => arg, None => return Ok(None), }; @@ -914,8 +902,7 @@ impl ArgMatches { &mut self, id: &str, ) -> Result, MatchesError> { - let id = Id::from(id); - match self.try_remove_arg_t::(&id)? { + match self.try_remove_arg_t::(id)? { Some(values) => Ok(values .into_vals_flatten() // enforced by `try_get_arg_t` @@ -930,8 +917,7 @@ impl ArgMatches { &mut self, id: &str, ) -> Result>, MatchesError> { - let id = Id::from(id); - let arg = match self.try_remove_arg_t::(&id)? { + let arg = match self.try_remove_arg_t::(id)? { Some(arg) => arg, None => return Ok(None), }; @@ -947,11 +933,9 @@ impl ArgMatches { /// Non-panicking version of [`ArgMatches::contains_id`] pub fn try_contains_id(&self, id: &str) -> Result { - let id = Id::from(id); - - self.verify_arg(&id)?; + self.verify_arg(id)?; - let presence = self.args.contains_key(&id); + let presence = self.args.contains_key(id); Ok(presence) } } @@ -959,7 +943,7 @@ impl ArgMatches { // Private methods impl ArgMatches { #[inline] - fn try_get_arg(&self, arg: &Id) -> Result, MatchesError> { + fn try_get_arg(&self, arg: &str) -> Result, MatchesError> { self.verify_arg(arg)?; Ok(self.args.get(arg)) } @@ -967,7 +951,7 @@ impl ArgMatches { #[inline] fn try_get_arg_t( &self, - arg: &Id, + arg: &str, ) -> Result, MatchesError> { let arg = match self.try_get_arg(arg)? { Some(arg) => arg, @@ -982,7 +966,7 @@ impl ArgMatches { #[inline] fn try_remove_arg_t( &mut self, - arg: &Id, + arg: &str, ) -> Result, MatchesError> { self.verify_arg(arg)?; let matched = match self.args.remove(arg) { @@ -997,7 +981,7 @@ impl ArgMatches { if actual == expected { Ok(Some(matched)) } else { - self.args.insert(arg.clone(), matched); + self.args.insert(Id::from(arg.to_owned()), matched); Err(MatchesError::Downcast { actual, expected }) } } @@ -1016,10 +1000,11 @@ impl ArgMatches { } #[inline] - fn verify_arg(&self, _arg: &Id) -> Result<(), MatchesError> { + fn verify_arg(&self, _arg: &str) -> Result<(), MatchesError> { #[cfg(debug_assertions)] { - if *_arg == Id::empty_hash() || self.valid_args.contains(_arg) { + let _arg = Id::from(_arg.to_owned()); + if _arg == Id::EXTERNAL || self.valid_args.contains(&_arg) { } else { debug!( "`{:?}` is not an id of an argument or a group.\n\ @@ -1035,10 +1020,11 @@ impl ArgMatches { #[inline] #[cfg_attr(debug_assertions, track_caller)] - fn get_arg(&self, arg: &Id) -> Option<&MatchedArg> { + fn get_arg<'s>(&'s self, arg: &str) -> Option<&'s MatchedArg> { #[cfg(debug_assertions)] { - if *arg == Id::empty_hash() || self.valid_args.contains(arg) { + let arg = Id::from(arg.to_owned()); + if arg == Id::EXTERNAL || self.valid_args.contains(&arg) { } else { panic!( "`{:?}` is not an id of an argument or a group.\n\ @@ -1058,7 +1044,7 @@ impl ArgMatches { #[cfg(debug_assertions)] { let name = name.to_owned(); - if name.is_empty() || self.valid_subcommands.contains(&name) { + if name == String::default() || self.valid_subcommands.contains(&name) { } else { panic!("`{}` is not a name of a subcommand.", name); } diff --git a/src/parser/parser.rs b/src/parser/parser.rs index 9ed587ee983..27c20fdcbc4 100644 --- a/src/parser/parser.rs +++ b/src/parser/parser.rs @@ -433,8 +433,8 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { for raw_val in raw_args.remaining(&mut args_cursor) { let val = external_parser.parse_ref(self.cmd, None, raw_val)?; - let external_id = &Id::empty_hash(); - sc_m.add_val_to(external_id, val, raw_val.to_os_string()); + let external_id = Id::from(Id::EXTERNAL); + sc_m.add_val_to(&external_id, val, raw_val.to_os_string()); } matcher.subcommand(SubCommand { @@ -1208,7 +1208,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { ArgAction::Count => { let raw_vals = if raw_vals.is_empty() { let existing_value = *matcher - .get_one::(arg.get_id()) + .get_one::(arg.get_id().as_str()) .unwrap_or(&0); let next_value = existing_value.saturating_add(1); vec![OsString::from(next_value.to_string())] diff --git a/src/util/fnv.rs b/src/util/fnv.rs deleted file mode 100644 index 4602300a498..00000000000 --- a/src/util/fnv.rs +++ /dev/null @@ -1,46 +0,0 @@ -use std::{ - fmt::Display, - hash::{Hash, Hasher}, -}; - -const MAGIC_INIT: u64 = 0x811C_9DC5; - -// TODO: Docs -pub trait Key: Hash + Display { - fn key(&self) -> u64; -} - -impl Key for T -where - T: Hash + Display, -{ - fn key(&self) -> u64 { - let mut hasher = FnvHasher::new(); - self.hash(&mut hasher); - hasher.finish() - } -} - -pub(crate) struct FnvHasher(u64); - -impl FnvHasher { - pub(crate) fn new() -> Self { - FnvHasher(MAGIC_INIT) - } -} - -impl Hasher for FnvHasher { - fn finish(&self) -> u64 { - self.0 - } - fn write(&mut self, bytes: &[u8]) { - let FnvHasher(mut hash) = *self; - - for byte in bytes.iter() { - hash ^= u64::from(*byte); - hash = hash.wrapping_mul(0x0100_0000_01b3); - } - - *self = FnvHasher(hash); - } -} diff --git a/src/util/id.rs b/src/util/id.rs index f481dd12a17..193bce0325f 100644 --- a/src/util/id.rs +++ b/src/util/id.rs @@ -1,83 +1,88 @@ -use crate::util::fnv::Key; +#[derive(Default, Clone, Eq, PartialEq, PartialOrd, Ord, Hash)] +#[repr(transparent)] +pub struct Id { + name: String, +} -use std::{ - fmt::{Debug, Formatter, Result}, - hash::{Hash, Hasher}, - ops::Deref, -}; +impl Id { + pub(crate) const HELP: &'static str = "help"; + pub(crate) const VERSION: &'static str = "version"; + pub(crate) const EXTERNAL: &'static str = ""; -#[derive(Clone, Eq, Default)] -#[cfg_attr(not(debug_assertions), repr(transparent))] -pub(crate) struct Id { - #[cfg(debug_assertions)] - name: String, - id: u64, + pub fn as_str(&self) -> &str { + self.name.as_str() + } +} + +impl<'s> From<&'s Id> for Id { + fn from(id: &'s Id) -> Self { + id.clone() + } +} + +impl From for Id { + fn from(name: String) -> Self { + Self { name } + } } -macro_rules! precomputed_hashes { - ($($fn_name:ident, $const:expr, $name:expr;)*) => { - impl Id { - $( - #[allow(dead_code)] - pub(crate) fn $fn_name() -> Self { - Id { - #[cfg(debug_assertions)] - name: $name.into(), - id: $const, - } - } - )* - } - }; +impl<'s> From<&'s String> for Id { + fn from(name: &'s String) -> Self { + name.to_owned().into() + } } -// precompute some common values -precomputed_hashes! { - empty_hash, 0x1C9D_3ADB_639F_298E, ""; - help_hash, 0x5963_6393_CFFB_FE5F, "help"; - version_hash, 0x30FF_0B7C_4D07_9478, "version"; +impl From<&'static str> for Id { + fn from(name: &'static str) -> Self { + name.to_owned().into() + } } -impl Debug for Id { - fn fmt(&self, f: &mut Formatter) -> Result { - #[cfg(debug_assertions)] - write!(f, "{}", self.name)?; - #[cfg(not(debug_assertions))] - write!(f, "[hash: {:X}]", self.id)?; +impl std::fmt::Display for Id { + #[inline] + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + std::fmt::Display::fmt(self.as_str(), f) + } +} - Ok(()) +impl std::fmt::Debug for Id { + #[inline] + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + std::fmt::Debug::fmt(self.as_str(), f) } } -impl Deref for Id { - type Target = u64; +impl AsRef for Id { + #[inline] + fn as_ref(&self) -> &str { + self.as_str() + } +} - fn deref(&self) -> &Self::Target { - &self.id +impl std::borrow::Borrow for Id { + #[inline] + fn borrow(&self) -> &str { + self.as_str() } } -impl From for Id { - fn from(val: T) -> Self { - Id { - #[cfg(debug_assertions)] - name: val.to_string(), - id: val.key(), - } +impl PartialEq for Id { + #[inline] + fn eq(&self, other: &str) -> bool { + PartialEq::eq(self.as_str(), other) } } -impl Hash for Id { - fn hash(&self, state: &mut H) - where - H: Hasher, - { - self.id.hash(state) +impl<'s> PartialEq<&'s str> for Id { + #[inline] + fn eq(&self, other: &&str) -> bool { + PartialEq::eq(self.as_str(), *other) } } -impl PartialEq for Id { - fn eq(&self, other: &Id) -> bool { - self.id == other.id +impl PartialEq for Id { + #[inline] + fn eq(&self, other: &String) -> bool { + PartialEq::eq(self.as_str(), other.as_str()) } } diff --git a/src/util/mod.rs b/src/util/mod.rs index 73b7ad81a97..d80df9656d5 100644 --- a/src/util/mod.rs +++ b/src/util/mod.rs @@ -2,13 +2,10 @@ mod flat_map; mod flat_set; -mod fnv; mod graph; mod id; mod str_to_bool; -pub use self::fnv::Key; - pub(crate) use self::flat_map::Entry; pub(crate) use self::flat_map::FlatMap; pub(crate) use self::flat_set::FlatSet; diff --git a/tests/builder/groups.rs b/tests/builder/groups.rs index edd43063064..4ff054b8e44 100644 --- a/tests/builder/groups.rs +++ b/tests/builder/groups.rs @@ -43,7 +43,7 @@ fn required_group_missing_arg() { #[cfg(debug_assertions)] #[test] -#[should_panic = "Argument group 'req' contains non-existent argument"] +#[should_panic = "Command group: Argument group 'req' contains non-existent argument"] fn non_existing_arg() { let _ = Command::new("group") .arg(arg!(-f --flag "some flag")) @@ -54,7 +54,7 @@ fn non_existing_arg() { #[cfg(debug_assertions)] #[test] -#[should_panic = "Argument group name must be unique\n\n\t'req' is already in use"] +#[should_panic = "Command group: Argument group name must be unique\n\n\t'req' is already in use"] fn unique_group_name() { let _ = Command::new("group") .arg(arg!(-f --flag "some flag")) @@ -66,7 +66,7 @@ fn unique_group_name() { #[cfg(debug_assertions)] #[test] -#[should_panic = "Command group: Argument group name '' must not conflict with argument name"] +#[should_panic = "Command group: Argument group name 'a' must not conflict with argument name"] fn groups_new_of_arg_name() { let _ = Command::new("group") .arg(Arg::new("a").long("a").group("a")) diff --git a/tests/builder/help.rs b/tests/builder/help.rs index ba07139563b..f84a47839c8 100644 --- a/tests/builder/help.rs +++ b/tests/builder/help.rs @@ -2586,7 +2586,7 @@ fn disable_help_flag_affects_help_subcommand() { .find_subcommand("help") .unwrap() .get_arguments() - .map(|a| a.get_id()) + .map(|a| a.get_id().as_str()) .collect::>(); assert!( !args.contains(&"help"), diff --git a/tests/builder/tests.rs b/tests/builder/tests.rs index b93a4d601cb..ea976c3ee8f 100644 --- a/tests/builder/tests.rs +++ b/tests/builder/tests.rs @@ -418,7 +418,7 @@ fn mut_arg_all() { let arg_names = cmd .get_arguments() .map(|a| a.get_id().clone()) - .filter(|a| *a != "version" && *a != "help") + .filter(|a| a != "version" && a != "help") .collect::>(); for arg_name in arg_names {