diff --git a/src/builder/arg.rs b/src/builder/arg.rs index ffebc3d10739..648f495ed8ea 100644 --- a/src/builder/arg.rs +++ b/src/builder/arg.rs @@ -1,11 +1,11 @@ // Std #[cfg(feature = "env")] use std::env; +#[cfg(feature = "env")] +use std::ffi::OsString; use std::{ borrow::Cow, cmp::{Ord, Ordering}, - ffi::OsStr, - ffi::OsString, fmt::{self, Display, Formatter}, str, }; @@ -13,9 +13,11 @@ use std::{ // Internal use super::{ArgFlags, ArgSettings}; use crate::builder::ArgPredicate; +use crate::builder::IntoResettable; use crate::builder::PossibleValue; use crate::builder::ValueRange; use crate::util::Id; +use crate::util::OsStr; use crate::ArgAction; use crate::ValueHint; use crate::INTERNAL_ERROR_MSG; @@ -60,8 +62,8 @@ pub struct Arg<'help> { pub(crate) overrides: Vec, pub(crate) groups: Vec, pub(crate) requires: Vec<(ArgPredicate, Id)>, - pub(crate) r_ifs: Vec<(Id, OsString)>, - pub(crate) r_ifs_all: Vec<(Id, OsString)>, + pub(crate) r_ifs: Vec<(Id, OsStr)>, + pub(crate) r_ifs_all: Vec<(Id, OsStr)>, pub(crate) r_unless: Vec, pub(crate) r_unless_all: Vec, pub(crate) short: Option, @@ -72,11 +74,11 @@ pub struct Arg<'help> { pub(crate) val_names: Vec<&'help str>, pub(crate) num_vals: Option, pub(crate) val_delim: Option, - pub(crate) default_vals: Vec<&'help OsStr>, - pub(crate) default_vals_ifs: Vec<(Id, ArgPredicate, Option<&'help OsStr>)>, - pub(crate) default_missing_vals: Vec<&'help OsStr>, + pub(crate) default_vals: Vec, + pub(crate) default_vals_ifs: Vec<(Id, ArgPredicate, Option)>, + pub(crate) default_missing_vals: Vec, #[cfg(feature = "env")] - pub(crate) env: Option<(&'help OsStr, Option)>, + pub(crate) env: Option<(OsStr, Option)>, pub(crate) terminator: Option<&'help str>, pub(crate) index: Option, pub(crate) help_heading: Option>, @@ -1514,8 +1516,8 @@ impl<'help> Arg<'help> { /// [`Arg::default_value_if`]: Arg::default_value_if() #[inline] #[must_use] - pub fn default_value(self, val: &'help str) -> Self { - self.default_values_os([OsStr::new(val)]) + pub fn default_value(self, val: impl Into) -> Self { + self.default_values_os([val]) } /// Value for the argument when not present. @@ -1526,7 +1528,7 @@ impl<'help> Arg<'help> { /// [`OsStr`]: std::ffi::OsStr #[inline] #[must_use] - pub fn default_value_os(self, val: &'help OsStr) -> Self { + pub fn default_value_os(self, val: impl Into) -> Self { self.default_values_os([val]) } @@ -1537,9 +1539,8 @@ impl<'help> Arg<'help> { /// [`Arg::default_value`]: Arg::default_value() #[inline] #[must_use] - pub fn default_values(self, vals: impl IntoIterator>) -> Self { - let vals_vec = vals.into_iter().map(|val| OsStr::new(val.into())); - self.default_values_os(vals_vec) + pub fn default_values(self, vals: impl IntoIterator>) -> Self { + self.default_values_os(vals) } /// Value for the argument when not present. @@ -1550,10 +1551,7 @@ impl<'help> Arg<'help> { /// [`OsStr`]: std::ffi::OsStr #[inline] #[must_use] - pub fn default_values_os( - mut self, - vals: impl IntoIterator>, - ) -> Self { + pub fn default_values_os(mut self, vals: impl IntoIterator>) -> Self { self.default_vals = vals.into_iter().map(|s| s.into()).collect(); self } @@ -1649,8 +1647,8 @@ impl<'help> Arg<'help> { /// [`Arg::default_value`]: Arg::default_value() #[inline] #[must_use] - pub fn default_missing_value(self, val: &'help str) -> Self { - self.default_missing_values_os([OsStr::new(val)]) + pub fn default_missing_value(self, val: impl Into) -> Self { + self.default_missing_values_os([val]) } /// Value for the argument when the flag is present but no value is specified. @@ -1661,7 +1659,7 @@ impl<'help> Arg<'help> { /// [`OsStr`]: std::ffi::OsStr #[inline] #[must_use] - pub fn default_missing_value_os(self, val: &'help OsStr) -> Self { + pub fn default_missing_value_os(self, val: impl Into) -> Self { self.default_missing_values_os([val]) } @@ -1672,12 +1670,8 @@ impl<'help> Arg<'help> { /// [`Arg::default_missing_value`]: Arg::default_missing_value() #[inline] #[must_use] - pub fn default_missing_values( - self, - vals: impl IntoIterator>, - ) -> Self { - let vals_vec = vals.into_iter().map(|val| OsStr::new(val.into())); - self.default_missing_values_os(vals_vec) + pub fn default_missing_values(self, vals: impl IntoIterator>) -> Self { + self.default_missing_values_os(vals) } /// Value for the argument when the flag is present but no value is specified. @@ -1690,7 +1684,7 @@ impl<'help> Arg<'help> { #[must_use] pub fn default_missing_values_os( mut self, - vals: impl IntoIterator>, + vals: impl IntoIterator>, ) -> Self { self.default_missing_vals = vals.into_iter().map(|s| s.into()).collect(); self @@ -1844,8 +1838,8 @@ impl<'help> Arg<'help> { #[cfg(feature = "env")] #[inline] #[must_use] - pub fn env(self, name: &'help str) -> Self { - self.env_os(OsStr::new(name)) + pub fn env(self, name: impl Into) -> Self { + self.env_os(name) } /// Read from `name` environment variable when argument is not present. @@ -1854,8 +1848,10 @@ impl<'help> Arg<'help> { #[cfg(feature = "env")] #[inline] #[must_use] - pub fn env_os(mut self, name: &'help OsStr) -> Self { - self.env = Some((name, env::var_os(name))); + pub fn env_os(mut self, name: impl Into) -> Self { + let name = name.into(); + let value = env::var_os(&name); + self.env = Some((name, value)); self } } @@ -2615,9 +2611,9 @@ impl<'help> Arg<'help> { self, arg_id: impl Into, val: impl Into, - default: Option<&'help str>, + default: impl IntoResettable, ) -> Self { - self.default_value_if_os(arg_id, val.into(), default.map(OsStr::new)) + self.default_value_if_os(arg_id, val, default) } /// Provides a conditional default value in the exact same manner as [`Arg::default_value_if`] @@ -2630,10 +2626,13 @@ impl<'help> Arg<'help> { mut self, arg_id: impl Into, val: impl Into, - default: Option<&'help OsStr>, + default: impl IntoResettable, ) -> Self { - self.default_vals_ifs - .push((arg_id.into(), val.into(), default)); + self.default_vals_ifs.push(( + arg_id.into(), + val.into(), + default.into_resettable().into_option(), + )); self } @@ -2721,10 +2720,16 @@ impl<'help> Arg<'help> { #[must_use] pub fn default_value_ifs( mut self, - ifs: impl IntoIterator, impl Into, Option<&'help str>)>, + ifs: impl IntoIterator< + Item = ( + impl Into, + impl Into, + impl IntoResettable, + ), + >, ) -> Self { for (arg, val, default) in ifs { - self = self.default_value_if_os(arg, val, default.map(OsStr::new)); + self = self.default_value_if_os(arg, val, default); } self } @@ -2737,10 +2742,16 @@ impl<'help> Arg<'help> { #[must_use] pub fn default_value_ifs_os( mut self, - ifs: impl IntoIterator, impl Into, Option<&'help OsStr>)>, + ifs: impl IntoIterator< + Item = ( + impl Into, + impl Into, + impl IntoResettable, + ), + >, ) -> Self { for (arg, val, default) in ifs { - self = self.default_value_if_os(arg.into(), val, default); + self = self.default_value_if_os(arg, val, default); } self } @@ -3038,7 +3049,7 @@ impl<'help> Arg<'help> { /// [Conflicting]: Arg::conflicts_with() /// [required]: Arg::required() #[must_use] - pub fn required_if_eq(mut self, arg_id: impl Into, val: impl Into) -> Self { + pub fn required_if_eq(mut self, arg_id: impl Into, val: impl Into) -> Self { self.r_ifs.push((arg_id.into(), val.into())); self } @@ -3119,7 +3130,7 @@ impl<'help> Arg<'help> { #[must_use] pub fn required_if_eq_any( mut self, - ifs: impl IntoIterator, impl Into)>, + ifs: impl IntoIterator, impl Into)>, ) -> Self { self.r_ifs .extend(ifs.into_iter().map(|(id, val)| (id.into(), val.into()))); @@ -3200,7 +3211,7 @@ impl<'help> Arg<'help> { #[must_use] pub fn required_if_eq_all( mut self, - ifs: impl IntoIterator, impl Into)>, + ifs: impl IntoIterator, impl Into)>, ) -> Self { self.r_ifs_all .extend(ifs.into_iter().map(|(id, val)| (id.into(), val.into()))); @@ -3713,14 +3724,14 @@ impl<'help> Arg<'help> { /// # Examples /// /// ```rust - /// # use std::ffi::OsStr; + /// # use clap::OsStr; /// # use clap::Arg; /// let arg = Arg::new("foo").env("ENVIRONMENT"); - /// assert_eq!(Some(OsStr::new("ENVIRONMENT")), arg.get_env()); + /// assert_eq!(arg.get_env(), Some(&OsStr::from("ENVIRONMENT"))); /// ``` #[cfg(feature = "env")] pub fn get_env(&self) -> Option<&OsStr> { - self.env.as_ref().map(|x| x.0) + self.env.as_ref().map(|x| &x.0) } /// Get the default values specified for this argument, if any @@ -3730,9 +3741,9 @@ impl<'help> Arg<'help> { /// ```rust /// # use clap::Arg; /// let arg = Arg::new("foo").default_value("default value"); - /// assert_eq!(&["default value"], arg.get_default_values()); + /// assert_eq!(arg.get_default_values(), &["default value"]); /// ``` - pub fn get_default_values(&self) -> &[&OsStr] { + pub fn get_default_values(&self) -> &[OsStr] { &self.default_vals } @@ -3743,10 +3754,10 @@ impl<'help> Arg<'help> { /// ``` /// # use clap::Arg; /// let arg = Arg::new("foo"); - /// assert_eq!(true, arg.is_positional()); + /// assert_eq!(arg.is_positional(), true); /// /// let arg = Arg::new("foo").long("foo"); - /// assert_eq!(false, arg.is_positional()); + /// assert_eq!(arg.is_positional(), false); /// ``` pub fn is_positional(&self) -> bool { self.long.is_none() && self.short.is_none() @@ -3892,12 +3903,12 @@ impl<'help> Arg<'help> { if let Some(action) = self.action.as_ref() { if let Some(default_value) = action.default_value() { if self.default_vals.is_empty() { - self.default_vals = vec![default_value]; + self.default_vals = vec![default_value.into()]; } } if let Some(default_value) = action.default_missing_value() { if self.default_missing_vals.is_empty() { - self.default_missing_vals = vec![default_value]; + self.default_missing_vals = vec![default_value.into()]; } } } diff --git a/src/builder/debug_asserts.rs b/src/builder/debug_asserts.rs index 77b0dbb5993e..f52a63834e2b 100644 --- a/src/builder/debug_asserts.rs +++ b/src/builder/debug_asserts.rs @@ -7,6 +7,7 @@ use crate::mkeymap::KeyType; use crate::util::FlatSet; use crate::util::Id; use crate::ArgAction; +use crate::OsStr; use crate::INTERNAL_ERROR_MSG; use crate::{Arg, Command, ValueHint}; @@ -766,18 +767,18 @@ fn assert_arg(arg: &Arg) { assert_arg_flags(arg); - assert_defaults(arg, "default_value", arg.default_vals.iter().copied()); + assert_defaults(arg, "default_value", arg.default_vals.iter()); assert_defaults( arg, "default_missing_value", - arg.default_missing_vals.iter().copied(), + arg.default_missing_vals.iter(), ); assert_defaults( arg, "default_value_if", arg.default_vals_ifs .iter() - .filter_map(|(_, _, default)| *default), + .filter_map(|(_, _, default)| default.as_ref()), ); } @@ -813,7 +814,7 @@ fn assert_arg_flags(arg: &Arg) { fn assert_defaults<'d>( arg: &Arg, field: &'static str, - defaults: impl IntoIterator, + defaults: impl IntoIterator, ) { for default_os in defaults { let value_parser = arg.get_value_parser(); diff --git a/src/builder/mod.rs b/src/builder/mod.rs index a147f1eb2597..504e6fd56490 100644 --- a/src/builder/mod.rs +++ b/src/builder/mod.rs @@ -9,6 +9,7 @@ mod arg_settings; mod command; mod possible_value; mod range; +mod resettable; mod value_hint; mod value_parser; @@ -25,6 +26,8 @@ pub use arg_predicate::ArgPredicate; pub use command::Command; pub use possible_value::PossibleValue; pub use range::ValueRange; +pub use resettable::IntoResettable; +pub use resettable::Resettable; pub use value_hint::ValueHint; pub use value_parser::_AutoValueParser; pub use value_parser::via_prelude; diff --git a/src/builder/resettable.rs b/src/builder/resettable.rs new file mode 100644 index 000000000000..0f269d7e4662 --- /dev/null +++ b/src/builder/resettable.rs @@ -0,0 +1,53 @@ +// Unlike `impl Into>` or `Option>`, this isn't ambiguous for the `None` +// case. + +/// Clearable builder value +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub enum Resettable { + /// Overwrite builder value + Value(T), + /// Reset builder value + Reset, +} + +impl Resettable { + pub(crate) fn into_option(self) -> Option { + match self { + Self::Value(t) => Some(t), + Self::Reset => None, + } + } +} + +/// Convert to the intended resettable type +pub trait IntoResettable { + /// Convert to the intended resettable type + fn into_resettable(self) -> Resettable; +} + +impl IntoResettable for Option<&'static str> { + fn into_resettable(self) -> Resettable { + match self { + Some(s) => Resettable::Value(s.into()), + None => Resettable::Reset, + } + } +} + +impl> IntoResettable for I { + fn into_resettable(self) -> Resettable { + Resettable::Value(self.into()) + } +} + +impl> IntoResettable for I { + fn into_resettable(self) -> Resettable { + Resettable::Value(self.into()) + } +} + +impl> IntoResettable for I { + fn into_resettable(self) -> Resettable { + Resettable::Value(self.into()) + } +} diff --git a/src/lib.rs b/src/lib.rs index cf87ddd0b39e..d15b969814df 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -109,7 +109,7 @@ pub use crate::util::color::ColorChoice; #[allow(unused_imports)] pub(crate) use crate::util::color::ColorChoice; pub use crate::util::Id; -pub(crate) use crate::util::OsStr; +pub use crate::util::OsStr; pub(crate) use crate::util::Str; pub use crate::derive::{Args, CommandFactory, FromArgMatches, Parser, Subcommand, ValueEnum}; diff --git a/src/output/help.rs b/src/output/help.rs index 26620a05e861..932e5b098b77 100644 --- a/src/output/help.rs +++ b/src/output/help.rs @@ -597,7 +597,7 @@ impl<'help, 'cmd, 'writer> Help<'help, 'cmd, 'writer> { let pvs = a .default_vals .iter() - .map(|&pvs| pvs.to_string_lossy()) + .map(|pvs| pvs.to_string_lossy()) .map(|pvs| { if pvs.contains(char::is_whitespace) { Cow::from(format!("{:?}", pvs)) diff --git a/src/parser/parser.rs b/src/parser/parser.rs index 1eea0d17e114..444b822f0fbb 100644 --- a/src/parser/parser.rs +++ b/src/parser/parser.rs @@ -1123,8 +1123,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { raw_vals.extend( arg.default_missing_vals .iter() - .copied() - .map(ToOwned::to_owned), + .map(|s| s.as_os_str().to_owned()), ); } } @@ -1388,8 +1387,8 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { }; if add { - if let Some(default) = *default { - let arg_values = vec![default.to_owned()]; + if let Some(default) = default { + let arg_values = vec![default.to_os_string()]; let trailing_idx = None; let _ = self.react( None, @@ -1424,8 +1423,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { let arg_values: Vec<_> = arg .default_vals .iter() - .copied() - .map(ToOwned::to_owned) + .map(crate::OsStr::to_os_string) .collect(); let trailing_idx = None; let _ = self.react( diff --git a/src/util/os_str.rs b/src/util/os_str.rs index e9d7767a2ffb..592e2b69b1b0 100644 --- a/src/util/os_str.rs +++ b/src/util/os_str.rs @@ -23,10 +23,15 @@ impl OsStr { } } - /// Get the raw string of the `OsStr` + /// Get the raw string as an `std::ffi::OsStr` pub fn as_os_str(&self) -> &std::ffi::OsStr { self.name.as_os_str() } + + /// Get the raw string as an `OsString` + pub fn to_os_string(&self) -> std::ffi::OsString { + self.as_os_str().to_owned() + } } impl From<&'_ OsStr> for OsStr { diff --git a/tests/builder/default_vals.rs b/tests/builder/default_vals.rs index fc01816aea6f..d2f2fc08a1d8 100644 --- a/tests/builder/default_vals.rs +++ b/tests/builder/default_vals.rs @@ -624,7 +624,7 @@ fn default_value_ifs_os() { Arg::new("other") .long("other") .value_parser(value_parser!(OsString)) - .default_value_ifs_os([("flag", "标记2", Some(OsStr::new("flag=标记2")))]), + .default_value_ifs_os([("flag", "标记2", OsStr::new("flag=标记2"))]), ); let result = cmd.try_get_matches_from(["my_cargo", "--flag", "标记2"]); assert!(result.is_ok(), "{}", result.unwrap_err()); diff --git a/tests/derive/default_value.rs b/tests/derive/default_value.rs index 70141836fc5f..8f0fdf1424b8 100644 --- a/tests/derive/default_value.rs +++ b/tests/derive/default_value.rs @@ -166,11 +166,9 @@ fn default_values_os_t() { #[test] fn detect_os_variant() { - #![allow(unused_parens)] // needed for `as_ref` call - #[derive(clap::Parser)] pub struct Options { - #[clap(default_value_os = ("123".as_ref()))] + #[clap(default_value_os = "123")] x: String, } Options::command().debug_assert();