Skip to content

Commit 5b5f2c1

Browse files
epageepage
committed
fix!: Track original Ids, rather than a hash
This is a step towards clap-rs#1041 - `ArgGroup` no longer takes a lifetime - One less field type needs a lifetime For now, we are using a more brute force type (`String`) so we can establish performance base lines. I was torn on whether to use `&str` everywhere or make an `IdRef`. The latter would add a lot of noise that I'm concerned about, so i left it simple for now. `IdRef` would help to communicate the types involved though. Speaking of communicating types, I'm also torn on whether we should use `Id` for all strings or if we should have `Id`, `Name`, etc types to avoid people mixing and matching. This added 18.7 KB. Compared to `HEAD~` on `06_rustup`: - build: 6.23us -> 7.41us - parse: 8.17us -> 9.36us - parse_sc: 7.65us -> 9.29us
1 parent f84e38a commit 5b5f2c1

File tree

17 files changed

+222
-320
lines changed

17 files changed

+222
-320
lines changed

Diff for: ‎clap_mangen/src/render.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ pub(crate) fn synopsis(roff: &mut Roff, cmd: &clap::Command) {
6363
if let Some(value) = arg.get_value_names() {
6464
line.push(italic(value.join(" ")));
6565
} else {
66-
line.push(italic(arg.get_id()));
66+
line.push(italic(arg.get_id().as_str()));
6767
}
6868
line.push(roman(rhs));
6969
line.push(roman(" "));
@@ -129,7 +129,7 @@ pub(crate) fn options(roff: &mut Roff, cmd: &clap::Command) {
129129
if let Some(value) = pos.get_value_names() {
130130
header.push(italic(value.join(" ")));
131131
} else {
132-
header.push(italic(pos.get_id()));
132+
header.push(italic(pos.get_id().as_str()));
133133
};
134134
header.push(roman(rhs));
135135

Diff for: ‎src/builder/arg.rs

+45-73
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ use crate::INTERNAL_ERROR_MSG;
5050
#[derive(Default, Clone)]
5151
pub struct Arg<'help> {
5252
pub(crate) id: Id,
53-
name: &'help str,
5453
pub(crate) help: Option<&'help str>,
5554
pub(crate) long_help: Option<&'help str>,
5655
pub(crate) action: Option<ArgAction>,
@@ -102,18 +101,16 @@ impl<'help> Arg<'help> {
102101
/// # ;
103102
/// ```
104103
/// [`Arg::action(ArgAction::Set)`]: Arg::action()
105-
pub fn new(id: impl Into<&'help str>) -> Self {
104+
pub fn new(id: impl Into<Id>) -> Self {
106105
Arg::default().id(id)
107106
}
108107

109108
/// Set the identifier used for referencing this argument in the clap API.
110109
///
111110
/// See [`Arg::new`] for more details.
112111
#[must_use]
113-
pub fn id(mut self, id: impl Into<&'help str>) -> Self {
114-
let id = id.into();
115-
self.id = Id::from(id);
116-
self.name = id;
112+
pub fn id(mut self, id: impl Into<Id>) -> Self {
113+
self.id = id.into();
117114
self
118115
}
119116

@@ -660,9 +657,8 @@ impl<'help> Arg<'help> {
660657
/// [Conflicting]: Arg::conflicts_with()
661658
/// [override]: Arg::overrides_with()
662659
#[must_use]
663-
pub fn requires(mut self, arg_id: impl Into<&'help str>) -> Self {
664-
self.requires
665-
.push((ArgPredicate::IsPresent, arg_id.into().into()));
660+
pub fn requires(mut self, arg_id: impl Into<Id>) -> Self {
661+
self.requires.push((ArgPredicate::IsPresent, arg_id.into()));
666662
self
667663
}
668664

@@ -2443,8 +2439,8 @@ impl<'help> Arg<'help> {
24432439
///
24442440
/// [`ArgGroup`]: crate::ArgGroup
24452441
#[must_use]
2446-
pub fn group(mut self, group_id: impl Into<&'help str>) -> Self {
2447-
self.groups.push(group_id.into().into());
2442+
pub fn group(mut self, group_id: impl Into<Id>) -> Self {
2443+
self.groups.push(group_id.into());
24482444
self
24492445
}
24502446

@@ -2484,9 +2480,8 @@ impl<'help> Arg<'help> {
24842480
///
24852481
/// [`ArgGroup`]: crate::ArgGroup
24862482
#[must_use]
2487-
pub fn groups(mut self, group_ids: impl IntoIterator<Item = impl Into<&'help str>>) -> Self {
2488-
self.groups
2489-
.extend(group_ids.into_iter().map(|n| n.into().into()));
2483+
pub fn groups(mut self, group_ids: impl IntoIterator<Item = impl Into<Id>>) -> Self {
2484+
self.groups.extend(group_ids.into_iter().map(Into::into));
24902485
self
24912486
}
24922487

@@ -2605,7 +2600,7 @@ impl<'help> Arg<'help> {
26052600
#[must_use]
26062601
pub fn default_value_if(
26072602
self,
2608-
arg_id: impl Into<&'help str>,
2603+
arg_id: impl Into<Id>,
26092604
val: Option<&'help str>,
26102605
default: Option<&'help str>,
26112606
) -> Self {
@@ -2620,12 +2615,12 @@ impl<'help> Arg<'help> {
26202615
#[must_use]
26212616
pub fn default_value_if_os(
26222617
mut self,
2623-
arg_id: impl Into<&'help str>,
2618+
arg_id: impl Into<Id>,
26242619
val: Option<&'help OsStr>,
26252620
default: Option<&'help OsStr>,
26262621
) -> Self {
26272622
self.default_vals_ifs
2628-
.push((arg_id.into().into(), val.into(), default));
2623+
.push((arg_id.into(), val.into(), default));
26292624
self
26302625
}
26312626

@@ -2712,13 +2707,7 @@ impl<'help> Arg<'help> {
27122707
#[must_use]
27132708
pub fn default_value_ifs(
27142709
mut self,
2715-
ifs: impl IntoIterator<
2716-
Item = (
2717-
impl Into<&'help str>,
2718-
Option<&'help str>,
2719-
Option<&'help str>,
2720-
),
2721-
>,
2710+
ifs: impl IntoIterator<Item = (impl Into<Id>, Option<&'help str>, Option<&'help str>)>,
27222711
) -> Self {
27232712
for (arg, val, default) in ifs {
27242713
self = self.default_value_if_os(arg, val.map(OsStr::new), default.map(OsStr::new));
@@ -2734,13 +2723,7 @@ impl<'help> Arg<'help> {
27342723
#[must_use]
27352724
pub fn default_value_ifs_os(
27362725
mut self,
2737-
ifs: impl IntoIterator<
2738-
Item = (
2739-
impl Into<&'help str>,
2740-
Option<&'help OsStr>,
2741-
Option<&'help OsStr>,
2742-
),
2743-
>,
2726+
ifs: impl IntoIterator<Item = (impl Into<Id>, Option<&'help OsStr>, Option<&'help OsStr>)>,
27442727
) -> Self {
27452728
for (arg, val, default) in ifs {
27462729
self = self.default_value_if_os(arg.into(), val, default);
@@ -2802,8 +2785,8 @@ impl<'help> Arg<'help> {
28022785
/// ```
28032786
/// [required]: Arg::required()
28042787
#[must_use]
2805-
pub fn required_unless_present(mut self, arg_id: impl Into<&'help str>) -> Self {
2806-
self.r_unless.push(arg_id.into().into());
2788+
pub fn required_unless_present(mut self, arg_id: impl Into<Id>) -> Self {
2789+
self.r_unless.push(arg_id.into());
28072790
self
28082791
}
28092792

@@ -2877,10 +2860,9 @@ impl<'help> Arg<'help> {
28772860
#[must_use]
28782861
pub fn required_unless_present_all(
28792862
mut self,
2880-
names: impl IntoIterator<Item = impl Into<&'help str>>,
2863+
names: impl IntoIterator<Item = impl Into<Id>>,
28812864
) -> Self {
2882-
self.r_unless_all
2883-
.extend(names.into_iter().map(|n| n.into().into()));
2865+
self.r_unless_all.extend(names.into_iter().map(Into::into));
28842866
self
28852867
}
28862868

@@ -2956,10 +2938,9 @@ impl<'help> Arg<'help> {
29562938
#[must_use]
29572939
pub fn required_unless_present_any(
29582940
mut self,
2959-
names: impl IntoIterator<Item = impl Into<&'help str>>,
2941+
names: impl IntoIterator<Item = impl Into<Id>>,
29602942
) -> Self {
2961-
self.r_unless
2962-
.extend(names.into_iter().map(|n| n.into().into()));
2943+
self.r_unless.extend(names.into_iter().map(Into::into));
29632944
self
29642945
}
29652946

@@ -3043,8 +3024,8 @@ impl<'help> Arg<'help> {
30433024
/// [Conflicting]: Arg::conflicts_with()
30443025
/// [required]: Arg::required()
30453026
#[must_use]
3046-
pub fn required_if_eq(mut self, arg_id: impl Into<&'help str>, val: &'help str) -> Self {
3047-
self.r_ifs.push((arg_id.into().into(), val));
3027+
pub fn required_if_eq(mut self, arg_id: impl Into<Id>, val: &'help str) -> Self {
3028+
self.r_ifs.push((arg_id.into(), val));
30483029
self
30493030
}
30503031

@@ -3124,10 +3105,10 @@ impl<'help> Arg<'help> {
31243105
#[must_use]
31253106
pub fn required_if_eq_any(
31263107
mut self,
3127-
ifs: impl IntoIterator<Item = (impl Into<&'help str>, &'help str)>,
3108+
ifs: impl IntoIterator<Item = (impl Into<Id>, &'help str)>,
31283109
) -> Self {
31293110
self.r_ifs
3130-
.extend(ifs.into_iter().map(|(id, val)| (id.into().into(), val)));
3111+
.extend(ifs.into_iter().map(|(id, val)| (id.into(), val)));
31313112
self
31323113
}
31333114

@@ -3205,10 +3186,10 @@ impl<'help> Arg<'help> {
32053186
#[must_use]
32063187
pub fn required_if_eq_all(
32073188
mut self,
3208-
ifs: impl IntoIterator<Item = (impl Into<&'help str>, &'help str)>,
3189+
ifs: impl IntoIterator<Item = (impl Into<Id>, &'help str)>,
32093190
) -> Self {
32103191
self.r_ifs_all
3211-
.extend(ifs.into_iter().map(|(id, val)| (id.into().into(), val)));
3192+
.extend(ifs.into_iter().map(|(id, val)| (id.into(), val)));
32123193
self
32133194
}
32143195

@@ -3268,9 +3249,9 @@ impl<'help> Arg<'help> {
32683249
/// [Conflicting]: Arg::conflicts_with()
32693250
/// [override]: Arg::overrides_with()
32703251
#[must_use]
3271-
pub fn requires_if(mut self, val: &'help str, arg_id: impl Into<&'help str>) -> Self {
3252+
pub fn requires_if(mut self, val: &'help str, arg_id: impl Into<Id>) -> Self {
32723253
self.requires
3273-
.push((ArgPredicate::Equals(OsStr::new(val)), arg_id.into().into()));
3254+
.push((ArgPredicate::Equals(OsStr::new(val)), arg_id.into()));
32743255
self
32753256
}
32763257

@@ -3321,11 +3302,11 @@ impl<'help> Arg<'help> {
33213302
#[must_use]
33223303
pub fn requires_ifs(
33233304
mut self,
3324-
ifs: impl IntoIterator<Item = (&'help str, impl Into<&'help str>)>,
3305+
ifs: impl IntoIterator<Item = (&'help str, impl Into<Id>)>,
33253306
) -> Self {
33263307
self.requires.extend(
33273308
ifs.into_iter()
3328-
.map(|(val, arg)| (ArgPredicate::Equals(OsStr::new(val)), arg.into().into())),
3309+
.map(|(val, arg)| (ArgPredicate::Equals(OsStr::new(val)), arg.into())),
33293310
);
33303311
self
33313312
}
@@ -3389,11 +3370,11 @@ impl<'help> Arg<'help> {
33893370
/// [Conflicting]: Arg::conflicts_with()
33903371
/// [override]: Arg::overrides_with()
33913372
#[must_use]
3392-
pub fn requires_all(mut self, names: impl IntoIterator<Item = impl Into<&'help str>>) -> Self {
3373+
pub fn requires_all(mut self, names: impl IntoIterator<Item = impl Into<Id>>) -> Self {
33933374
self.requires.extend(
33943375
names
33953376
.into_iter()
3396-
.map(|s| (ArgPredicate::IsPresent, s.into().into())),
3377+
.map(|s| (ArgPredicate::IsPresent, s.into())),
33973378
);
33983379
self
33993380
}
@@ -3443,8 +3424,8 @@ impl<'help> Arg<'help> {
34433424
/// [`Arg::conflicts_with_all(names)`]: Arg::conflicts_with_all()
34443425
/// [`Arg::exclusive(true)`]: Arg::exclusive()
34453426
#[must_use]
3446-
pub fn conflicts_with(mut self, arg_id: impl Into<&'help str>) -> Self {
3447-
self.blacklist.push(arg_id.into().into());
3427+
pub fn conflicts_with(mut self, arg_id: impl Into<Id>) -> Self {
3428+
self.blacklist.push(arg_id.into());
34483429
self
34493430
}
34503431

@@ -3493,12 +3474,8 @@ impl<'help> Arg<'help> {
34933474
/// [`Arg::conflicts_with`]: Arg::conflicts_with()
34943475
/// [`Arg::exclusive(true)`]: Arg::exclusive()
34953476
#[must_use]
3496-
pub fn conflicts_with_all(
3497-
mut self,
3498-
names: impl IntoIterator<Item = impl Into<&'help str>>,
3499-
) -> Self {
3500-
self.blacklist
3501-
.extend(names.into_iter().map(|n| n.into().into()));
3477+
pub fn conflicts_with_all(mut self, names: impl IntoIterator<Item = impl Into<Id>>) -> Self {
3478+
self.blacklist.extend(names.into_iter().map(Into::into));
35023479
self
35033480
}
35043481

@@ -3535,8 +3512,8 @@ impl<'help> Arg<'help> {
35353512
/// assert!(!*m.get_one::<bool>("flag").unwrap());
35363513
/// ```
35373514
#[must_use]
3538-
pub fn overrides_with(mut self, arg_id: impl Into<&'help str>) -> Self {
3539-
self.overrides.push(arg_id.into().into());
3515+
pub fn overrides_with(mut self, arg_id: impl Into<Id>) -> Self {
3516+
self.overrides.push(arg_id.into());
35403517
self
35413518
}
35423519

@@ -3571,12 +3548,8 @@ impl<'help> Arg<'help> {
35713548
/// assert!(!*m.get_one::<bool>("flag").unwrap());
35723549
/// ```
35733550
#[must_use]
3574-
pub fn overrides_with_all(
3575-
mut self,
3576-
names: impl IntoIterator<Item = impl Into<&'help str>>,
3577-
) -> Self {
3578-
self.overrides
3579-
.extend(names.into_iter().map(|n| n.into().into()));
3551+
pub fn overrides_with_all(mut self, names: impl IntoIterator<Item = impl Into<Id>>) -> Self {
3552+
self.overrides.extend(names.into_iter().map(Into::into));
35803553
self
35813554
}
35823555
}
@@ -3585,8 +3558,8 @@ impl<'help> Arg<'help> {
35853558
impl<'help> Arg<'help> {
35863559
/// Get the name of the argument
35873560
#[inline]
3588-
pub fn get_id(&self) -> &'help str {
3589-
self.name
3561+
pub fn get_id(&self) -> &Id {
3562+
&self.id
35903563
}
35913564

35923565
/// Get the help specified for this argument, if any
@@ -4017,7 +3990,7 @@ impl<'help> Arg<'help> {
40173990
}
40183991
} else {
40193992
debug!("Arg::name_no_brackets: just name");
4020-
Cow::Borrowed(self.get_id())
3993+
Cow::Borrowed(self.get_id().as_str())
40213994
}
40223995
}
40233996

@@ -4051,7 +4024,7 @@ impl<'help> PartialOrd for Arg<'help> {
40514024

40524025
impl<'help> Ord for Arg<'help> {
40534026
fn cmp(&self, other: &Arg) -> Ordering {
4054-
self.get_id().cmp(other.get_id())
4027+
self.id.cmp(&other.id)
40554028
}
40564029
}
40574030

@@ -4110,7 +4083,6 @@ impl<'help> fmt::Debug for Arg<'help> {
41104083
#[allow(unused_mut)]
41114084
let mut ds = ds
41124085
.field("id", &self.id)
4113-
.field("name", &self.name)
41144086
.field("help", &self.help)
41154087
.field("long_help", &self.long_help)
41164088
.field("action", &self.action)
@@ -4155,7 +4127,7 @@ pub(crate) fn render_arg_val(arg: &Arg) -> String {
41554127

41564128
let arg_name_storage;
41574129
let val_names = if arg.val_names.is_empty() {
4158-
arg_name_storage = [arg.get_id()];
4130+
arg_name_storage = [arg.get_id().as_str()];
41594131
&arg_name_storage
41604132
} else {
41614133
arg.val_names.as_slice()

0 commit comments

Comments
 (0)