Merge pull request #4092 from epage/flag

fix: Allow non-bool value_parsers for SetTrue

CHANGELOG.md

@@ -70,6 +70,7 @@ MSRV is now 1.60.0
 - `Arg::default_missing_value` now applies per occurrence rather than if a value is missing across all occurrences
 - `arg!(--long [value])` to accept `0..=1` per occurrence rather than across all occurrences, making it safe to use with `ArgAction::Append`
 - Allow `OsStr`s for `Arg::{required_if_eq,required_if_eq_any,required_if_eq_all}`
+- Allow non-bool `value_parser`s for `ArgAction::SetTrue` / `ArgAction::SetFalse`
 - *(assert)* Always enforce that version is specified when the `ArgAction::Version` is used
 - *(assert)* Add missing `#[track_caller]`s to make it easier to debug asserts
 - *(assert)* Ensure `overrides_with` IDs are valid

src/builder/action.rs

@@ -283,8 +283,8 @@ impl ArgAction {
         match self {
             Self::Set => None,
             Self::Append => None,
-            Self::SetTrue => Some(AnyValueId::of::<bool>()),
-            Self::SetFalse => Some(AnyValueId::of::<bool>()),
+            Self::SetTrue => None,
+            Self::SetFalse => None,
             Self::Count => Some(AnyValueId::of::<CountType>()),
             Self::Help => None,
             Self::Version => None,

tests/derive/flags.rs

@@ -12,6 +12,7 @@
 // commit#ea76fa1b1b273e65e3b0b1046643715b49bec51f which is licensed under the
 // MIT/Apache 2.0 license.
 
+use clap::ArgAction;
 use clap::CommandFactory;
 use clap::Parser;
 
@@ -43,6 +44,39 @@ fn bool_type_is_flag() {
     assert!(Opt::try_parse_from(&["test", "-a", "foo"]).is_err());
 }
 
+#[test]
+fn non_bool_type_flag() {
+    fn parse_from_flag(b: &str) -> Result<usize, String> {
+        b.parse::<bool>()
+            .map(|b| if b { 10 } else { 5 })
+            .map_err(|e| e.to_string())
+    }
+
+    #[derive(Parser, Debug)]
+    struct Opt {
+        #[clap(short, long, action = ArgAction::SetTrue, value_parser = parse_from_flag)]
+        alice: usize,
+        #[clap(short, long, action = ArgAction::SetTrue, value_parser = parse_from_flag)]
+        bob: usize,
+    }
+
+    let opt = Opt::try_parse_from(&["test"]).unwrap();
+    assert_eq!(opt.alice, 5);
+    assert_eq!(opt.bob, 5);
+
+    let opt = Opt::try_parse_from(&["test", "-a"]).unwrap();
+    assert_eq!(opt.alice, 10);
+    assert_eq!(opt.bob, 5);
+
+    let opt = Opt::try_parse_from(&["test", "-b"]).unwrap();
+    assert_eq!(opt.alice, 5);
+    assert_eq!(opt.bob, 10);
+
+    let opt = Opt::try_parse_from(&["test", "-b", "-a"]).unwrap();
+    assert_eq!(opt.alice, 10);
+    assert_eq!(opt.bob, 10);
+}
+
 #[test]
 #[ignore] // Not a good path for supporting this atm
 fn inferred_help() {