Repository: clap-rs/clap
Pull request #4092 (2022-08-19 02:36:49)

fix: Allow non-bool value_parsers for SetTrue
Not sure if we could have originally made this work but it definitely
does now that we use `default_missing_value` for this (#4000)

<!--
Thanks for helping out!

Please link the appropriate issue from your PR.

If you don't have an issue, we'd recommend starting with one first so the PR can focus on the
implementation (unless its an obvious bug or documentation fix that will have
little conversation).
-->



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index e8d09428..a6775519 100644
--- a/CHANGELOG.md
+++ b/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
@@ -103,7 +104,7 @@ MSRV is now 1.60.0
 
 ### Fixes
 
-- A `multiple_values` positional followed by another positional now works with multiple flags
+- A `multple_values` positional followed by another positional now works with multiple flags
 
 ## [3.2.13] - 2022-07-19
 
diff --git a/src/builder/action.rs b/src/builder/action.rs
index ff8f92bf..1de512a4 100644
--- a/src/builder/action.rs
+++ b/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,


Test patch:
diff --git a/tests/derive/flags.rs b/tests/derive/flags.rs
index e83ee7b8..5a792081 100644
--- a/tests/derive/flags.rs
+++ b/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() {