Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Deprecate _os variants #4141

Merged
merged 1 commit into from Aug 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -59,6 +59,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

MSRV is now 1.60.0

Deprecated
- `default_value_os`, `default_values_os`, `default_value_if_os`, and `default_value_ifs_os` as the non `_os` variants now accept either a `str` or an `OsStr`

### Features

- `Arg::num_args` now accepts ranges, allowing setting both the minimum and maximum number of values per occurrence
Expand Down
4 changes: 2 additions & 2 deletions clap_derive/src/attrs.rs
Expand Up @@ -655,7 +655,7 @@ impl Attrs {
})
};

let raw_ident = Ident::new("default_value_os", attr.name.clone().span());
let raw_ident = Ident::new("default_value", attr.name.clone().span());
self.methods.push(Method::new(raw_ident, val));
}

Expand Down Expand Up @@ -723,7 +723,7 @@ impl Attrs {
};

self.methods.push(Method::new(
Ident::new("default_values_os", attr.name.clone().span()),
Ident::new("default_values", attr.name.clone().span()),
val,
));
}
Expand Down
83 changes: 39 additions & 44 deletions src/builder/arg.rs
Expand Up @@ -1515,19 +1515,18 @@ impl Arg {
#[inline]
#[must_use]
pub fn default_value(self, val: impl Into<OsStr>) -> Self {
self.default_values_os([val])
self.default_values([val])
}

/// Value for the argument when not present.
///
/// See [`Arg::default_value`].
///
/// [`Arg::default_value`]: Arg::default_value()
/// [`OsStr`]: std::ffi::OsStr
#[inline]
#[must_use]
#[doc(hidden)]
#[cfg_attr(
feature = "deprecated",
deprecated(since = "4.0.0", note = "Replaced with `Arg::default_value`")
)]
pub fn default_value_os(self, val: impl Into<OsStr>) -> Self {
self.default_values_os([val])
self.default_values([val])
}

/// Value for the argument when not present.
Expand All @@ -1537,21 +1536,20 @@ impl Arg {
/// [`Arg::default_value`]: Arg::default_value()
#[inline]
#[must_use]
pub fn default_values(self, vals: impl IntoIterator<Item = impl Into<OsStr>>) -> Self {
self.default_values_os(vals)
pub fn default_values(mut self, vals: impl IntoIterator<Item = impl Into<OsStr>>) -> Self {
self.default_vals = vals.into_iter().map(|s| s.into()).collect();
self
}

/// Value for the argument when not present.
///
/// See [`Arg::default_values`].
///
/// [`Arg::default_values`]: Arg::default_values()
/// [`OsStr`]: std::ffi::OsStr
#[inline]
#[must_use]
pub fn default_values_os(mut self, vals: impl IntoIterator<Item = impl Into<OsStr>>) -> Self {
self.default_vals = vals.into_iter().map(|s| s.into()).collect();
self
#[doc(hidden)]
#[cfg_attr(
feature = "deprecated",
deprecated(since = "4.0.0", note = "Replaced with `Arg::default_values`")
)]
pub fn default_values_os(self, vals: impl IntoIterator<Item = impl Into<OsStr>>) -> Self {
self.default_values(vals)
}

/// Value for the argument when the flag is present but no value is specified.
Expand Down Expand Up @@ -2603,32 +2601,32 @@ impl Arg {
/// [`Arg::default_value`]: Arg::default_value()
#[must_use]
pub fn default_value_if(
self,
mut self,
arg_id: impl Into<Id>,
val: impl Into<ArgPredicate>,
default: impl IntoResettable<OsStr>,
) -> Self {
self.default_value_if_os(arg_id, val, default)
self.default_vals_ifs.push((
arg_id.into(),
val.into(),
default.into_resettable().into_option(),
));
self
}

/// Provides a conditional default value in the exact same manner as [`Arg::default_value_if`]
/// only using [`OsStr`]s instead.
///
/// [`Arg::default_value_if`]: Arg::default_value_if()
/// [`OsStr`]: std::ffi::OsStr
#[must_use]
#[doc(hidden)]
#[cfg_attr(
feature = "deprecated",
deprecated(since = "4.0.0", note = "Replaced with `Arg::default_value_if`")
)]
pub fn default_value_if_os(
mut self,
self,
arg_id: impl Into<Id>,
val: impl Into<ArgPredicate>,
default: impl IntoResettable<OsStr>,
) -> Self {
self.default_vals_ifs.push((
arg_id.into(),
val.into(),
default.into_resettable().into_option(),
));
self
self.default_value_if(arg_id, val, default)
}

/// Specifies multiple values and conditions in the same manner as [`Arg::default_value_if`].
Expand Down Expand Up @@ -2724,19 +2722,19 @@ impl Arg {
>,
) -> Self {
for (arg, val, default) in ifs {
self = self.default_value_if_os(arg, val, default);
self = self.default_value_if(arg, val, default);
}
self
}

/// Provides multiple conditional default values in the exact same manner as
/// [`Arg::default_value_ifs`] only using [`OsStr`]s instead.
///
/// [`Arg::default_value_ifs`]: Arg::default_value_ifs()
/// [`OsStr`]: std::ffi::OsStr
#[must_use]
#[doc(hidden)]
#[cfg_attr(
feature = "deprecated",
deprecated(since = "4.0.0", note = "Replaced with `Arg::default_value_ifs`")
)]
pub fn default_value_ifs_os(
mut self,
self,
ifs: impl IntoIterator<
Item = (
impl Into<Id>,
Expand All @@ -2745,10 +2743,7 @@ impl Arg {
),
>,
) -> Self {
for (arg, val, default) in ifs {
self = self.default_value_if_os(arg, val, default);
}
self
self.default_value_ifs(ifs)
}

/// Set this arg as [required] as long as the specified argument is not present at runtime.
Expand Down
10 changes: 5 additions & 5 deletions tests/builder/default_vals.rs
Expand Up @@ -112,7 +112,7 @@ fn osstr_opts() {
.arg(
arg!(o: -o <opt> "some opt")
.required(false)
.default_value_os(expected),
.default_value(expected),
)
.try_get_matches_from(vec![""]);
assert!(r.is_ok(), "{}", r.unwrap_err());
Expand All @@ -133,7 +133,7 @@ fn osstr_opt_user_override() {
.arg(
arg!(--opt <FILE> "some arg")
.required(false)
.default_value_os(default),
.default_value(default),
)
.try_get_matches_from(vec!["", "--opt", "value"]);
assert!(r.is_ok(), "{}", r.unwrap_err());
Expand All @@ -151,7 +151,7 @@ fn osstr_positionals() {
let expected = OsStr::new("default");

let r = Command::new("df")
.arg(arg!([arg] "some opt").default_value_os(expected))
.arg(arg!([arg] "some opt").default_value(expected))
.try_get_matches_from(vec![""]);
assert!(r.is_ok(), "{}", r.unwrap_err());
let m = r.unwrap();
Expand All @@ -168,7 +168,7 @@ fn osstr_positional_user_override() {
let default = OsStr::new("default");

let r = Command::new("df")
.arg(arg!([arg] "some arg").default_value_os(default))
.arg(arg!([arg] "some arg").default_value(default))
.try_get_matches_from(vec!["", "value"]);
assert!(r.is_ok(), "{}", r.unwrap_err());
let m = r.unwrap();
Expand Down Expand Up @@ -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", OsStr::new("flag=标记2"))]),
.default_value_ifs([("flag", "标记2", OsStr::new("flag=标记2"))]),
);
let result = cmd.try_get_matches_from(["my_cargo", "--flag", "标记2"]);
assert!(result.is_ok(), "{}", result.unwrap_err());
Expand Down
2 changes: 2 additions & 0 deletions tests/derive/default_value.rs
Expand Up @@ -166,6 +166,8 @@ fn default_values_os_t() {

#[test]
fn detect_os_variant() {
#![allow(deprecated)]

#[derive(clap::Parser)]
pub struct Options {
#[clap(default_value_os = "123")]
Expand Down