Repository: clap-rs/clap
Pull request #5912 (2025-02-17 18:30:42)

feat(builder): Allow flags to take `num_args=0..=1`
Inspired by #5909


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 4edac955..afc47c82 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1464,7 +1464,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "e310b3a6b5907f99202fcdb4960ff45b93735d7c7d96b760fcff8db2dc0e103d"
 dependencies = [
  "cfg-if",
- "windows-targets 0.48.5",
+ "windows-targets 0.52.6",
 ]
 
 [[package]]
@@ -3354,9 +3354,9 @@ dependencies = [
 
 [[package]]
 name = "trybuild"
-version = "1.0.103"
+version = "1.0.102"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b812699e0c4f813b872b373a4471717d9eb550da14b311058a4d9cf4173cbca6"
+checksum = "9f14b5c02a137632f68194ec657ecb92304138948e8957c932127eb1b58c23be"
 dependencies = [
  "glob",
  "serde",
@@ -3369,9 +3369,9 @@ dependencies = [
 
 [[package]]
 name = "trycmd"
-version = "0.15.9"
+version = "0.15.8"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a8b5cf29388862aac065d6597ac9c8e842d1cc827cb50f7c32f11d29442eaae4"
+checksum = "656073a28690a4b8dfd578d1df087826cf8fa53c8161dbd90130d092570a21e1"
 dependencies = [
  "anstream",
  "automod",
diff --git a/clap_builder/src/builder/action.rs b/clap_builder/src/builder/action.rs
index 2e8221d8..0d2ca7be 100644
--- a/clap_builder/src/builder/action.rs
+++ b/clap_builder/src/builder/action.rs
@@ -1,6 +1,8 @@
 #[cfg(debug_assertions)]
 use crate::util::AnyValueId;
 
+use crate::builder::ValueRange;
+
 /// Behavior of arguments when they are encountered while parsing
 ///
 /// # Examples
@@ -369,6 +371,35 @@ impl ArgAction {
         }
     }
 
+    #[cfg(debug_assertions)]
+    pub(crate) fn max_num_args(&self) -> ValueRange {
+        match self {
+            Self::Set => ValueRange::FULL,
+            Self::Append => ValueRange::FULL,
+            Self::SetTrue => ValueRange::OPTIONAL,
+            Self::SetFalse => ValueRange::OPTIONAL,
+            Self::Count => ValueRange::EMPTY,
+            Self::Help => ValueRange::EMPTY,
+            Self::HelpShort => ValueRange::EMPTY,
+            Self::HelpLong => ValueRange::EMPTY,
+            Self::Version => ValueRange::EMPTY,
+        }
+    }
+
+    pub(crate) fn default_num_args(&self) -> ValueRange {
+        match self {
+            Self::Set => ValueRange::SINGLE,
+            Self::Append => ValueRange::SINGLE,
+            Self::SetTrue => ValueRange::EMPTY,
+            Self::SetFalse => ValueRange::EMPTY,
+            Self::Count => ValueRange::EMPTY,
+            Self::Help => ValueRange::EMPTY,
+            Self::HelpShort => ValueRange::EMPTY,
+            Self::HelpLong => ValueRange::EMPTY,
+            Self::Version => ValueRange::EMPTY,
+        }
+    }
+
     pub(crate) fn default_value(&self) -> Option<&'static std::ffi::OsStr> {
         match self {
             Self::Set => None,
diff --git a/clap_builder/src/builder/arg.rs b/clap_builder/src/builder/arg.rs
index 79181497..9ed75ece 100644
--- a/clap_builder/src/builder/arg.rs
+++ b/clap_builder/src/builder/arg.rs
@@ -4546,11 +4546,7 @@ impl Arg {
         if val_names_len > 1 {
             self.num_vals.get_or_insert(val_names_len.into());
         } else {
-            let nargs = if self.get_action().takes_values() {
-                ValueRange::SINGLE
-            } else {
-                ValueRange::EMPTY
-            };
+            let nargs = self.get_action().default_num_args();
             self.num_vals.get_or_insert(nargs);
         }
     }
diff --git a/clap_builder/src/builder/debug_asserts.rs b/clap_builder/src/builder/debug_asserts.rs
index 001ef8a1..28ef70eb 100644
--- a/clap_builder/src/builder/debug_asserts.rs
+++ b/clap_builder/src/builder/debug_asserts.rs
@@ -690,14 +690,14 @@ fn assert_arg(arg: &Arg) {
         arg.get_id(),
     );
 
-    if arg.is_takes_value_set() {
-        assert!(
-            arg.get_action().takes_values(),
-            "Argument `{}`'s selected action {:?} contradicts `takes_value`",
-            arg.get_id(),
-            arg.get_action()
-        );
-    }
+    assert!(
+        arg.get_num_args().unwrap_or(1.into()).max_values()
+            <= arg.get_action().max_num_args().max_values(),
+        "Argument `{}`'s action {:?} is incompatible with `num_args({:?})`",
+        arg.get_id(),
+        arg.get_action(),
+        arg.get_num_args().unwrap_or(1.into())
+    );
     if let Some(action_type_id) = arg.get_action().value_type_id() {
         assert_eq!(
             action_type_id,
diff --git a/clap_builder/src/builder/range.rs b/clap_builder/src/builder/range.rs
index 1676ed32..be5695de 100644
--- a/clap_builder/src/builder/range.rs
+++ b/clap_builder/src/builder/range.rs
@@ -18,6 +18,17 @@ impl ValueRange {
         end_inclusive: 1,
     };
 
+    #[cfg(debug_assertions)]
+    pub(crate) const OPTIONAL: Self = Self {
+        start_inclusive: 0,
+        end_inclusive: 1,
+    };
+
+    pub(crate) const FULL: Self = Self {
+        start_inclusive: 0,
+        end_inclusive: usize::MAX,
+    };
+
     /// Create a range
     ///
     /// # Panics
@@ -135,9 +146,7 @@ impl From<std::ops::Range<usize>> for ValueRange {
 
 impl From<std::ops::RangeFull> for ValueRange {
     fn from(_: std::ops::RangeFull) -> Self {
-        let start_inclusive = 0;
-        let end_inclusive = usize::MAX;
-        Self::raw(start_inclusive, end_inclusive)
+        Self::FULL
     }
 }
 
@@ -176,7 +185,10 @@ impl From<std::ops::RangeToInclusive<usize>> for ValueRange {
 impl std::fmt::Display for ValueRange {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         ok!(self.start_inclusive.fmt(f));
-        if !self.is_fixed() {
+        if self.is_fixed() {
+        } else if self.end_inclusive == usize::MAX {
+            ok!("..".fmt(f));
+        } else {
             ok!("..=".fmt(f));
             ok!(self.end_inclusive.fmt(f));
         }
diff --git a/clap_complete/src/engine/complete.rs b/clap_complete/src/engine/complete.rs
index 99077499..7b01f55e 100644
--- a/clap_complete/src/engine/complete.rs
+++ b/clap_complete/src/engine/complete.rs
@@ -81,7 +81,7 @@ pub fn complete(
                 });
 
                 if let Some(opt) = opt {
-                    if opt.get_action().takes_values() && value.is_none() {
+                    if opt.get_num_args().expect("built").takes_values() && value.is_none() {
                         next_state = ParseState::Opt((opt, 1));
                     };
                 } else if pos_allows_hyphen(current_cmd, pos_index) {
@@ -581,7 +581,10 @@ fn parse_shortflags<'c, 's>(
                     });
                     is_find.unwrap_or(false)
                 });
-                if opt.map(|o| o.get_action().takes_values()).unwrap_or(false) {
+                if opt
+                    .map(|o| o.get_num_args().expect("built").takes_values())
+                    .unwrap_or(false)
+                {
                     takes_value_opt = opt;
                     break;
                 }


Test patch:
diff --git a/tests/builder/action.rs b/tests/builder/action.rs
index b0a0abf2..e2af19dc 100644
--- a/tests/builder/action.rs
+++ b/tests/builder/action.rs
@@ -219,6 +219,19 @@ fn set_true_with_required_if_eq() {
     assert_eq!(matches.get_flag("mammal"), true);
 }
 
+#[test]
+#[should_panic = "Argument `mammal`'s action SetTrue is incompatible with `num_args(1..)`"]
+fn set_true_with_incompatible_num_args() {
+    Command::new("test")
+        .arg(
+            Arg::new("mammal")
+                .long("mammal")
+                .action(ArgAction::SetTrue)
+                .num_args(1..),
+        )
+        .build();
+}
+
 #[test]
 fn set_false() {
     let cmd = Command::new("test").arg(
@@ -333,6 +346,19 @@ fn set_false_with_default_value_if_value() {
     assert_eq!(matches.get_flag("mammal"), false);
 }
 
+#[test]
+#[should_panic = "Argument `mammal`'s action SetFalse is incompatible with `num_args(1..)`"]
+fn set_false_with_incompatible_num_args() {
+    Command::new("test")
+        .arg(
+            Arg::new("mammal")
+                .long("mammal")
+                .action(ArgAction::SetFalse)
+                .num_args(1..),
+        )
+        .build();
+}
+
 #[test]
 fn count() {
     let cmd = Command::new("test").arg(Arg::new("mammal").long("mammal").action(ArgAction::Count));
@@ -442,3 +468,69 @@ fn count_with_default_value_if_value() {
     assert_eq!(*matches.get_one::<u8>("dog").unwrap(), 0);
     assert_eq!(*matches.get_one::<u8>("mammal").unwrap(), 1);
 }
+
+#[test]
+#[should_panic = "Argument `mammal`'s action Count is incompatible with `num_args(1..)`"]
+fn count_with_incompatible_num_args() {
+    Command::new("test")
+        .arg(
+            Arg::new("mammal")
+                .long("mammal")
+                .action(ArgAction::Count)
+                .num_args(1..),
+        )
+        .build();
+}
+
+#[test]
+#[should_panic = "Argument `mammal`'s action Help is incompatible with `num_args(1..)`"]
+fn help_with_incompatible_num_args() {
+    Command::new("test")
+        .arg(
+            Arg::new("mammal")
+                .long("mammal")
+                .action(ArgAction::Help)
+                .num_args(1..),
+        )
+        .build();
+}
+
+#[test]
+#[should_panic = "Argument `mammal`'s action HelpShort is incompatible with `num_args(1..)`"]
+fn help_short_with_incompatible_num_args() {
+    Command::new("test")
+        .arg(
+            Arg::new("mammal")
+                .long("mammal")
+                .action(ArgAction::HelpShort)
+                .num_args(1..),
+        )
+        .build();
+}
+
+#[test]
+#[should_panic = "Argument `mammal`'s action HelpLong is incompatible with `num_args(1..)`"]
+fn help_long_with_incompatible_num_args() {
+    Command::new("test")
+        .arg(
+            Arg::new("mammal")
+                .long("mammal")
+                .action(ArgAction::HelpLong)
+                .num_args(1..),
+        )
+        .build();
+}
+
+#[test]
+#[should_panic = "Argument `mammal`'s action Version is incompatible with `num_args(1..)`"]
+fn version_with_incompatible_num_args() {
+    Command::new("test")
+        .version("1.0.0")
+        .arg(
+            Arg::new("mammal")
+                .long("mammal")
+                .action(ArgAction::Version)
+                .num_args(1..),
+        )
+        .build();
+}
diff --git a/tests/builder/flags.rs b/tests/builder/flags.rs
index 27a89e07..0eb1f4e6 100644
--- a/tests/builder/flags.rs
+++ b/tests/builder/flags.rs
@@ -222,3 +222,28 @@ fn leading_dash_stripped() {
     let cmd = Command::new("mycat").arg(Arg::new("filename").long("--filename"));
     cmd.debug_assert();
 }
+
+#[test]
+fn optional_value() {
+    let cmd = Command::new("flag").args([arg!(-f --flag "some flag")
+        .action(ArgAction::SetTrue)
+        .num_args(0..=1)]);
+
+    let m = cmd.clone().try_get_matches_from(vec![""]).unwrap();
+    assert!(!*m.get_one::<bool>("flag").expect("defaulted by clap"));
+
+    let m = cmd.clone().try_get_matches_from(vec!["", "-f"]).unwrap();
+    assert!(*m.get_one::<bool>("flag").expect("defaulted by clap"));
+
+    let m = cmd
+        .clone()
+        .try_get_matches_from(vec!["", "-f", "false"])
+        .unwrap();
+    assert!(!*m.get_one::<bool>("flag").expect("defaulted by clap"));
+
+    let m = cmd
+        .clone()
+        .try_get_matches_from(vec!["", "-f", "true"])
+        .unwrap();
+    assert!(*m.get_one::<bool>("flag").expect("defaulted by clap"));
+}
diff --git a/tests/derive/flags.rs b/tests/derive/flags.rs
index 3b5dd67a..edc09daa 100644
--- a/tests/derive/flags.rs
+++ b/tests/derive/flags.rs
@@ -329,3 +329,26 @@ fn unit_for_negation() {
         Opt::try_parse_from(["test", "--arg", "--no-arg"]).unwrap()
     );
 }
+
+#[test]
+fn optional_value_flag() {
+    #[derive(Parser, PartialEq, Eq, Debug)]
+    struct Opt {
+        #[arg(short, long, num_args=0..=1)]
+        alice: bool,
+    }
+
+    assert_eq!(Opt { alice: false }, Opt::try_parse_from(["test"]).unwrap());
+    assert_eq!(
+        Opt { alice: true },
+        Opt::try_parse_from(["test", "-a"]).unwrap()
+    );
+    assert_eq!(
+        Opt { alice: true },
+        Opt::try_parse_from(["test", "-a", "true"]).unwrap()
+    );
+    assert_eq!(
+        Opt { alice: false },
+        Opt::try_parse_from(["test", "-a", "false"]).unwrap()
+    );
+}