Skip to content

Commit 429333f

Browse files
epageepage
committed
fix: Switch OsStr's in builder to owned/borrowed type
This is a part of clap-rs#1041 Because `Option<Into<T>>` is ambiguous for `None`, we had to create `Resettable` to workaround it.
1 parent 5950c4b commit 429333f

File tree

10 files changed

+139
-70
lines changed

10 files changed

+139
-70
lines changed

Diff for: ‎src/builder/arg.rs

+64-53
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,23 @@
11
// Std
22
#[cfg(feature = "env")]
33
use std::env;
4+
#[cfg(feature = "env")]
5+
use std::ffi::OsString;
46
use std::{
57
borrow::Cow,
68
cmp::{Ord, Ordering},
7-
ffi::OsStr,
8-
ffi::OsString,
99
fmt::{self, Display, Formatter},
1010
str,
1111
};
1212

1313
// Internal
1414
use super::{ArgFlags, ArgSettings};
1515
use crate::builder::ArgPredicate;
16+
use crate::builder::IntoResettable;
1617
use crate::builder::PossibleValue;
1718
use crate::builder::ValueRange;
1819
use crate::util::Id;
20+
use crate::util::OsStr;
1921
use crate::ArgAction;
2022
use crate::ValueHint;
2123
use crate::INTERNAL_ERROR_MSG;
@@ -60,8 +62,8 @@ pub struct Arg<'help> {
6062
pub(crate) overrides: Vec<Id>,
6163
pub(crate) groups: Vec<Id>,
6264
pub(crate) requires: Vec<(ArgPredicate, Id)>,
63-
pub(crate) r_ifs: Vec<(Id, OsString)>,
64-
pub(crate) r_ifs_all: Vec<(Id, OsString)>,
65+
pub(crate) r_ifs: Vec<(Id, OsStr)>,
66+
pub(crate) r_ifs_all: Vec<(Id, OsStr)>,
6567
pub(crate) r_unless: Vec<Id>,
6668
pub(crate) r_unless_all: Vec<Id>,
6769
pub(crate) short: Option<char>,
@@ -72,11 +74,11 @@ pub struct Arg<'help> {
7274
pub(crate) val_names: Vec<&'help str>,
7375
pub(crate) num_vals: Option<ValueRange>,
7476
pub(crate) val_delim: Option<char>,
75-
pub(crate) default_vals: Vec<&'help OsStr>,
76-
pub(crate) default_vals_ifs: Vec<(Id, ArgPredicate, Option<&'help OsStr>)>,
77-
pub(crate) default_missing_vals: Vec<&'help OsStr>,
77+
pub(crate) default_vals: Vec<OsStr>,
78+
pub(crate) default_vals_ifs: Vec<(Id, ArgPredicate, Option<OsStr>)>,
79+
pub(crate) default_missing_vals: Vec<OsStr>,
7880
#[cfg(feature = "env")]
79-
pub(crate) env: Option<(&'help OsStr, Option<OsString>)>,
81+
pub(crate) env: Option<(OsStr, Option<OsString>)>,
8082
pub(crate) terminator: Option<&'help str>,
8183
pub(crate) index: Option<usize>,
8284
pub(crate) help_heading: Option<Option<&'help str>>,
@@ -1514,8 +1516,8 @@ impl<'help> Arg<'help> {
15141516
/// [`Arg::default_value_if`]: Arg::default_value_if()
15151517
#[inline]
15161518
#[must_use]
1517-
pub fn default_value(self, val: &'help str) -> Self {
1518-
self.default_values_os([OsStr::new(val)])
1519+
pub fn default_value(self, val: impl Into<OsStr>) -> Self {
1520+
self.default_values_os([val])
15191521
}
15201522

15211523
/// Value for the argument when not present.
@@ -1526,7 +1528,7 @@ impl<'help> Arg<'help> {
15261528
/// [`OsStr`]: std::ffi::OsStr
15271529
#[inline]
15281530
#[must_use]
1529-
pub fn default_value_os(self, val: &'help OsStr) -> Self {
1531+
pub fn default_value_os(self, val: impl Into<OsStr>) -> Self {
15301532
self.default_values_os([val])
15311533
}
15321534

@@ -1537,9 +1539,8 @@ impl<'help> Arg<'help> {
15371539
/// [`Arg::default_value`]: Arg::default_value()
15381540
#[inline]
15391541
#[must_use]
1540-
pub fn default_values(self, vals: impl IntoIterator<Item = impl Into<&'help str>>) -> Self {
1541-
let vals_vec = vals.into_iter().map(|val| OsStr::new(val.into()));
1542-
self.default_values_os(vals_vec)
1542+
pub fn default_values(self, vals: impl IntoIterator<Item = impl Into<OsStr>>) -> Self {
1543+
self.default_values_os(vals)
15431544
}
15441545

15451546
/// Value for the argument when not present.
@@ -1550,10 +1551,7 @@ impl<'help> Arg<'help> {
15501551
/// [`OsStr`]: std::ffi::OsStr
15511552
#[inline]
15521553
#[must_use]
1553-
pub fn default_values_os(
1554-
mut self,
1555-
vals: impl IntoIterator<Item = impl Into<&'help OsStr>>,
1556-
) -> Self {
1554+
pub fn default_values_os(mut self, vals: impl IntoIterator<Item = impl Into<OsStr>>) -> Self {
15571555
self.default_vals = vals.into_iter().map(|s| s.into()).collect();
15581556
self
15591557
}
@@ -1649,8 +1647,8 @@ impl<'help> Arg<'help> {
16491647
/// [`Arg::default_value`]: Arg::default_value()
16501648
#[inline]
16511649
#[must_use]
1652-
pub fn default_missing_value(self, val: &'help str) -> Self {
1653-
self.default_missing_values_os([OsStr::new(val)])
1650+
pub fn default_missing_value(self, val: impl Into<OsStr>) -> Self {
1651+
self.default_missing_values_os([val])
16541652
}
16551653

16561654
/// Value for the argument when the flag is present but no value is specified.
@@ -1661,7 +1659,7 @@ impl<'help> Arg<'help> {
16611659
/// [`OsStr`]: std::ffi::OsStr
16621660
#[inline]
16631661
#[must_use]
1664-
pub fn default_missing_value_os(self, val: &'help OsStr) -> Self {
1662+
pub fn default_missing_value_os(self, val: impl Into<OsStr>) -> Self {
16651663
self.default_missing_values_os([val])
16661664
}
16671665

@@ -1672,12 +1670,8 @@ impl<'help> Arg<'help> {
16721670
/// [`Arg::default_missing_value`]: Arg::default_missing_value()
16731671
#[inline]
16741672
#[must_use]
1675-
pub fn default_missing_values(
1676-
self,
1677-
vals: impl IntoIterator<Item = impl Into<&'help str>>,
1678-
) -> Self {
1679-
let vals_vec = vals.into_iter().map(|val| OsStr::new(val.into()));
1680-
self.default_missing_values_os(vals_vec)
1673+
pub fn default_missing_values(self, vals: impl IntoIterator<Item = impl Into<OsStr>>) -> Self {
1674+
self.default_missing_values_os(vals)
16811675
}
16821676

16831677
/// Value for the argument when the flag is present but no value is specified.
@@ -1690,7 +1684,7 @@ impl<'help> Arg<'help> {
16901684
#[must_use]
16911685
pub fn default_missing_values_os(
16921686
mut self,
1693-
vals: impl IntoIterator<Item = impl Into<&'help OsStr>>,
1687+
vals: impl IntoIterator<Item = impl Into<OsStr>>,
16941688
) -> Self {
16951689
self.default_missing_vals = vals.into_iter().map(|s| s.into()).collect();
16961690
self
@@ -1844,8 +1838,8 @@ impl<'help> Arg<'help> {
18441838
#[cfg(feature = "env")]
18451839
#[inline]
18461840
#[must_use]
1847-
pub fn env(self, name: &'help str) -> Self {
1848-
self.env_os(OsStr::new(name))
1841+
pub fn env(self, name: impl Into<OsStr>) -> Self {
1842+
self.env_os(name)
18491843
}
18501844

18511845
/// Read from `name` environment variable when argument is not present.
@@ -1854,8 +1848,10 @@ impl<'help> Arg<'help> {
18541848
#[cfg(feature = "env")]
18551849
#[inline]
18561850
#[must_use]
1857-
pub fn env_os(mut self, name: &'help OsStr) -> Self {
1858-
self.env = Some((name, env::var_os(name)));
1851+
pub fn env_os(mut self, name: impl Into<OsStr>) -> Self {
1852+
let name = name.into();
1853+
let value = env::var_os(&name);
1854+
self.env = Some((name, value));
18591855
self
18601856
}
18611857
}
@@ -2615,9 +2611,9 @@ impl<'help> Arg<'help> {
26152611
self,
26162612
arg_id: impl Into<Id>,
26172613
val: impl Into<ArgPredicate>,
2618-
default: Option<&'help str>,
2614+
default: impl IntoResettable<OsStr>,
26192615
) -> Self {
2620-
self.default_value_if_os(arg_id, val.into(), default.map(OsStr::new))
2616+
self.default_value_if_os(arg_id, val, default)
26212617
}
26222618

26232619
/// Provides a conditional default value in the exact same manner as [`Arg::default_value_if`]
@@ -2630,10 +2626,13 @@ impl<'help> Arg<'help> {
26302626
mut self,
26312627
arg_id: impl Into<Id>,
26322628
val: impl Into<ArgPredicate>,
2633-
default: Option<&'help OsStr>,
2629+
default: impl IntoResettable<OsStr>,
26342630
) -> Self {
2635-
self.default_vals_ifs
2636-
.push((arg_id.into(), val.into(), default));
2631+
self.default_vals_ifs.push((
2632+
arg_id.into(),
2633+
val.into(),
2634+
default.into_resettable().into_option(),
2635+
));
26372636
self
26382637
}
26392638

@@ -2721,10 +2720,16 @@ impl<'help> Arg<'help> {
27212720
#[must_use]
27222721
pub fn default_value_ifs(
27232722
mut self,
2724-
ifs: impl IntoIterator<Item = (impl Into<Id>, impl Into<ArgPredicate>, Option<&'help str>)>,
2723+
ifs: impl IntoIterator<
2724+
Item = (
2725+
impl Into<Id>,
2726+
impl Into<ArgPredicate>,
2727+
impl IntoResettable<OsStr>,
2728+
),
2729+
>,
27252730
) -> Self {
27262731
for (arg, val, default) in ifs {
2727-
self = self.default_value_if_os(arg, val, default.map(OsStr::new));
2732+
self = self.default_value_if_os(arg, val, default);
27282733
}
27292734
self
27302735
}
@@ -2737,10 +2742,16 @@ impl<'help> Arg<'help> {
27372742
#[must_use]
27382743
pub fn default_value_ifs_os(
27392744
mut self,
2740-
ifs: impl IntoIterator<Item = (impl Into<Id>, impl Into<ArgPredicate>, Option<&'help OsStr>)>,
2745+
ifs: impl IntoIterator<
2746+
Item = (
2747+
impl Into<Id>,
2748+
impl Into<ArgPredicate>,
2749+
impl IntoResettable<OsStr>,
2750+
),
2751+
>,
27412752
) -> Self {
27422753
for (arg, val, default) in ifs {
2743-
self = self.default_value_if_os(arg.into(), val, default);
2754+
self = self.default_value_if_os(arg, val, default);
27442755
}
27452756
self
27462757
}
@@ -3038,7 +3049,7 @@ impl<'help> Arg<'help> {
30383049
/// [Conflicting]: Arg::conflicts_with()
30393050
/// [required]: Arg::required()
30403051
#[must_use]
3041-
pub fn required_if_eq(mut self, arg_id: impl Into<Id>, val: impl Into<OsString>) -> Self {
3052+
pub fn required_if_eq(mut self, arg_id: impl Into<Id>, val: impl Into<OsStr>) -> Self {
30423053
self.r_ifs.push((arg_id.into(), val.into()));
30433054
self
30443055
}
@@ -3119,7 +3130,7 @@ impl<'help> Arg<'help> {
31193130
#[must_use]
31203131
pub fn required_if_eq_any(
31213132
mut self,
3122-
ifs: impl IntoIterator<Item = (impl Into<Id>, impl Into<OsString>)>,
3133+
ifs: impl IntoIterator<Item = (impl Into<Id>, impl Into<OsStr>)>,
31233134
) -> Self {
31243135
self.r_ifs
31253136
.extend(ifs.into_iter().map(|(id, val)| (id.into(), val.into())));
@@ -3200,7 +3211,7 @@ impl<'help> Arg<'help> {
32003211
#[must_use]
32013212
pub fn required_if_eq_all(
32023213
mut self,
3203-
ifs: impl IntoIterator<Item = (impl Into<Id>, impl Into<OsString>)>,
3214+
ifs: impl IntoIterator<Item = (impl Into<Id>, impl Into<OsStr>)>,
32043215
) -> Self {
32053216
self.r_ifs_all
32063217
.extend(ifs.into_iter().map(|(id, val)| (id.into(), val.into())));
@@ -3713,14 +3724,14 @@ impl<'help> Arg<'help> {
37133724
/// # Examples
37143725
///
37153726
/// ```rust
3716-
/// # use std::ffi::OsStr;
3727+
/// # use clap::OsStr;
37173728
/// # use clap::Arg;
37183729
/// let arg = Arg::new("foo").env("ENVIRONMENT");
3719-
/// assert_eq!(Some(OsStr::new("ENVIRONMENT")), arg.get_env());
3730+
/// assert_eq!(arg.get_env(), Some(&OsStr::from("ENVIRONMENT")));
37203731
/// ```
37213732
#[cfg(feature = "env")]
37223733
pub fn get_env(&self) -> Option<&OsStr> {
3723-
self.env.as_ref().map(|x| x.0)
3734+
self.env.as_ref().map(|x| &x.0)
37243735
}
37253736

37263737
/// Get the default values specified for this argument, if any
@@ -3730,9 +3741,9 @@ impl<'help> Arg<'help> {
37303741
/// ```rust
37313742
/// # use clap::Arg;
37323743
/// let arg = Arg::new("foo").default_value("default value");
3733-
/// assert_eq!(&["default value"], arg.get_default_values());
3744+
/// assert_eq!(arg.get_default_values(), &["default value"]);
37343745
/// ```
3735-
pub fn get_default_values(&self) -> &[&OsStr] {
3746+
pub fn get_default_values(&self) -> &[OsStr] {
37363747
&self.default_vals
37373748
}
37383749

@@ -3743,10 +3754,10 @@ impl<'help> Arg<'help> {
37433754
/// ```
37443755
/// # use clap::Arg;
37453756
/// let arg = Arg::new("foo");
3746-
/// assert_eq!(true, arg.is_positional());
3757+
/// assert_eq!(arg.is_positional(), true);
37473758
///
37483759
/// let arg = Arg::new("foo").long("foo");
3749-
/// assert_eq!(false, arg.is_positional());
3760+
/// assert_eq!(arg.is_positional(), false);
37503761
/// ```
37513762
pub fn is_positional(&self) -> bool {
37523763
self.long.is_none() && self.short.is_none()
@@ -3892,12 +3903,12 @@ impl<'help> Arg<'help> {
38923903
if let Some(action) = self.action.as_ref() {
38933904
if let Some(default_value) = action.default_value() {
38943905
if self.default_vals.is_empty() {
3895-
self.default_vals = vec![default_value];
3906+
self.default_vals = vec![default_value.into()];
38963907
}
38973908
}
38983909
if let Some(default_value) = action.default_missing_value() {
38993910
if self.default_missing_vals.is_empty() {
3900-
self.default_missing_vals = vec![default_value];
3911+
self.default_missing_vals = vec![default_value.into()];
39013912
}
39023913
}
39033914
}

Diff for: ‎src/builder/debug_asserts.rs

+5-4
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use crate::mkeymap::KeyType;
77
use crate::util::FlatSet;
88
use crate::util::Id;
99
use crate::ArgAction;
10+
use crate::OsStr;
1011
use crate::INTERNAL_ERROR_MSG;
1112
use crate::{Arg, Command, ValueHint};
1213

@@ -766,18 +767,18 @@ fn assert_arg(arg: &Arg) {
766767

767768
assert_arg_flags(arg);
768769

769-
assert_defaults(arg, "default_value", arg.default_vals.iter().copied());
770+
assert_defaults(arg, "default_value", arg.default_vals.iter());
770771
assert_defaults(
771772
arg,
772773
"default_missing_value",
773-
arg.default_missing_vals.iter().copied(),
774+
arg.default_missing_vals.iter(),
774775
);
775776
assert_defaults(
776777
arg,
777778
"default_value_if",
778779
arg.default_vals_ifs
779780
.iter()
780-
.filter_map(|(_, _, default)| *default),
781+
.filter_map(|(_, _, default)| default.as_ref()),
781782
);
782783
}
783784

@@ -813,7 +814,7 @@ fn assert_arg_flags(arg: &Arg) {
813814
fn assert_defaults<'d>(
814815
arg: &Arg,
815816
field: &'static str,
816-
defaults: impl IntoIterator<Item = &'d std::ffi::OsStr>,
817+
defaults: impl IntoIterator<Item = &'d OsStr>,
817818
) {
818819
for default_os in defaults {
819820
let value_parser = arg.get_value_parser();

Diff for: ‎src/builder/mod.rs

+3
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ mod arg_settings;
99
mod command;
1010
mod possible_value;
1111
mod range;
12+
mod resettable;
1213
mod value_hint;
1314
mod value_parser;
1415

@@ -25,6 +26,8 @@ pub use arg_predicate::ArgPredicate;
2526
pub use command::Command;
2627
pub use possible_value::PossibleValue;
2728
pub use range::ValueRange;
29+
pub use resettable::IntoResettable;
30+
pub use resettable::Resettable;
2831
pub use value_hint::ValueHint;
2932
pub use value_parser::_AutoValueParser;
3033
pub use value_parser::via_prelude;

0 commit comments

Comments
 (0)