Merge pull request #4141 from epage/deprecate

fix: Deprecate _os variants

CHANGELOG.md

@@ -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

clap_derive/src/attrs.rs

@@ -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));
                 }
 
@@ -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,
                     ));
                 }

src/builder/arg.rs

@@ -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.
@@ -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.
@@ -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`].
@@ -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>,
@@ -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.

tests/builder/default_vals.rs

@@ -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());
@@ -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());
@@ -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();
@@ -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();
@@ -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());

tests/derive/default_value.rs

@@ -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")]