Repository: pacak/bpaf (rust)

Issue:
many + env = problem
```rust
#[test]
fn many_env() {
    std::env::set_var("USER", "top s3cr3t");
    let parser = short('v')
        .env("USER")
        .argument::<String>("USER")
        .many()
        .to_options();
    let r = parser.run_inner(Args::from(&[])).unwrap();
    todo!("{:?}", r);
}
```

Pull request description:
make sure `env` works with combination of `many`
Fixes #169 

Patch:
diff --git a/Changelog.md b/Changelog.md
index 01e5184..3e3d324 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -1,5 +1,14 @@
 # Change Log
 
+## bpaf [0.8.0], bpaf_derive [0.3.6] - Unreleased
+- `many` and `some` will now collect one result from a parser
+  that does not consume anything from an argument list allowing
+  for easier composition with parsers that consume from both
+  command line and environment variables. If your code depends on
+  the original behavior you should replace non failing parsers under
+  `many` with failing parsers: `req_flag` instead of `switch`.
+- support deriving `req_flag` consumers
+
 ## bpaf [0.7.10], bpaf_derive [0.3.5] - 2013-03-19
 - improve error messages for typos like `-llvm` instead of `--llvm`
 - improve error messages when a flag is accepted by a command but not directly
diff --git a/bpaf_derive/src/field/named_field.rs b/bpaf_derive/src/field/named_field.rs
index 1068c85..39f2d27 100644
--- a/bpaf_derive/src/field/named_field.rs
+++ b/bpaf_derive/src/field/named_field.rs
@@ -223,6 +223,12 @@ impl Field {
                         content.parse::<token::Comma>()?;
                         let b = content.parse()?;
                         res.consumer = Some(ConsumerAttr::Flag(Box::new(a), Box::new(b)));
+                    } else if keyword == "req_flag" {
+                        check_stage(&mut stage, 2, &keyword)?;
+                        let _ = parenthesized!(content in input);
+                        let a = content.parse()?;
+                        res.consumer = Some(ConsumerAttr::ReqFlag(Box::new(a)));
+
                     //
                     // external
                     } else if keyword == "external" {
diff --git a/docs/src/many/cases.txt b/docs/src/many/cases.txt
index c57d0d6..18b0286 100644
--- a/docs/src/many/cases.txt
+++ b/docs/src/many/cases.txt
@@ -1,12 +1,13 @@
 ? Run inner parser as many times as possible collecting all the new results
+? First `false` is collected from a switch even if it is not consuming anything
 > --argument 10 --argument 20
 OK
-Options { argument: [10, 20], switches: [] }
+Options { argument: [10, 20], switches: [false] }
 
 ? If there's no matching parsers - it would produce an empty vector
 >
 OK
-Options { argument: [], switches: [] }
+Options { argument: [], switches: [false] }
 
 ? For parsers that can succeed without consuming anything such as `flag` or `switch` - `many`
 ? only collects values as long as they produce something
diff --git a/docs/src/some/cases.txt b/docs/src/some/cases.txt
index 8fde0f2..219a478 100644
--- a/docs/src/some/cases.txt
+++ b/docs/src/some/cases.txt
@@ -23,7 +23,7 @@ Options { argument: [10], switches: [true] }
 ? In usage lines `some` items are indicated with `...`
 > --help
 Stdout
-Usage: --argument ARG... [--switch]...
+Usage: --argument ARG... --switch...
 
 Available options:
         --argument <ARG>  important argument
diff --git a/docs/src/some/combine.rs b/docs/src/some/combine.rs
index 9b4e4a2..6ce659c 100644
--- a/docs/src/some/combine.rs
+++ b/docs/src/some/combine.rs
@@ -15,7 +15,7 @@ pub fn options() -> OptionParser<Options> {
         .some("want at least one argument");
     let switches = long("switch")
         .help("some switch")
-        .switch()
+        .req_flag(true)
         .some("want at least one switch");
     construct!(Options { argument, switches }).to_options()
 }
diff --git a/docs/src/some/derive.rs b/docs/src/some/derive.rs
index 22fef9c..e4b9c27 100644
--- a/docs/src/some/derive.rs
+++ b/docs/src/some/derive.rs
@@ -9,6 +9,6 @@ pub struct Options {
     #[bpaf(argument("ARG"), some("want at least one argument"))]
     argument: Vec<u32>,
     /// some switch
-    #[bpaf(long("switch"), switch, some("want at least one switch"))]
+    #[bpaf(long("switch"), req_flag(true), some("want at least one switch"))]
     switches: Vec<bool>,
 }
diff --git a/src/docs/many.md b/src/docs/many.md
index a5f67c3..936204a 100644
--- a/src/docs/many.md
+++ b/src/docs/many.md
@@ -44,15 +44,16 @@ pub struct Options {
 
 
 Run inner parser as many times as possible collecting all the new results
+First `false` is collected from a switch even if it is not consuming anything
 ```console
 % app --argument 10 --argument 20
-Options { argument: [10, 20], switches: [] }
+Options { argument: [10, 20], switches: [false] }
 ```
 
 If there's no matching parsers - it would produce an empty vector
 ```console
 % app 
-Options { argument: [], switches: [] }
+Options { argument: [], switches: [false] }
 ```
 
 For parsers that can succeed without consuming anything such as `flag` or `switch` - `many`
diff --git a/src/docs/some.md b/src/docs/some.md
index 4c17971..7a68ebc 100644
--- a/src/docs/some.md
+++ b/src/docs/some.md
@@ -17,7 +17,7 @@ pub fn options() -> OptionParser<Options> {
         .some("want at least one argument");
     let switches = long("switch")
         .help("some switch")
-        .switch()
+        .req_flag(true)
         .some("want at least one switch");
     construct!(Options { argument, switches }).to_options()
 }
@@ -37,7 +37,7 @@ pub struct Options {
     #[bpaf(argument("ARG"), some("want at least one argument"))]
     argument: Vec<u32>,
     /// some switch
-    #[bpaf(long("switch"), switch, some("want at least one switch"))]
+    #[bpaf(long("switch"), req_flag(true), some("want at least one switch"))]
     switches: Vec<bool>,
 }
 ```
@@ -76,7 +76,7 @@ Options { argument: [10], switches: [true] }
 In usage lines `some` items are indicated with `...`
 ```console
 % app --help
-Usage: --argument ARG... [--switch]...
+Usage: --argument ARG... --switch...
 
 Available options:
         --argument <ARG>  important argument
diff --git a/src/lib.rs b/src/lib.rs
index 2f9a804..321caa9 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -826,7 +826,10 @@ pub trait Parser<T> {
     /// [`catch`](ParseMany::catch) statement you can instead stop at the first value
     /// that failed to parse and ignore it and all the subsequent ones.
     ///
-    /// `many` only collects elements that only consume something from the argument list.
+    /// `many` will collect at most one result that does not consume anything from the argument
+    /// list allowing using it in combination of any parsers with a fallback. After the first one
+    /// it will keep collecting the results as long as they consume something.
+    ///
     /// For derive usage `bpaf_derive` would insert implicit `many` when resulting type is a
     /// vector.
     ///
@@ -855,7 +858,9 @@ pub trait Parser<T> {
     /// [`catch`](ParseSome::catch) statement you can instead stop at the first value
     /// that failed to parse and ignore it and all the subsequent ones.
     ///
-    /// `some` only collects elements that only consume something from the argument list.
+    /// `some` will collect at most one result that does not consume anything from the argument
+    /// list allowing using it in combination of any parsers with a fallback. After the first one
+    /// it will keep collecting the results as long as they consume something.
     ///
     #[doc = include_str!("docs/some.md")]
     ///
diff --git a/src/structs.rs b/src/structs.rs
index 380a8e3..f80ac9a 100644
--- a/src/structs.rs
+++ b/src/structs.rs
@@ -90,7 +90,9 @@ where
         let mut len = args.len();
 
         while let Some(val) = parse_option(&self.inner, args, self.catch)? {
-            if args.len() < len {
+            // we keep including values for as long as we consume values from the argument
+            // list or at least one value
+            if args.len() < len || res.is_empty() {
                 len = args.len();
                 res.push(val);
             } else {
@@ -515,7 +517,9 @@ where
         let mut res = Vec::new();
         let mut len = args.len();
         while let Some(val) = parse_option(&self.inner, args, self.catch)? {
-            if args.len() < len {
+            // we keep including values for as long as we consume values from the argument
+            // list or at least one value
+            if args.len() < len || res.is_empty() {
                 len = args.len();
                 res.push(val);
             } else {


Test patch:
diff --git a/bpaf_derive/src/field_tests.rs b/bpaf_derive/src/field_tests.rs
index 0f7fe6f..69b6b33 100644
--- a/bpaf_derive/src/field_tests.rs
+++ b/bpaf_derive/src/field_tests.rs
@@ -477,6 +477,18 @@ fn explicit_switch_argument() {
     assert_eq!(input.to_token_stream().to_string(), output.to_string());
 }
 
+#[test]
+fn explicit_req_flag_argument() {
+    let input: NamedField = parse_quote! {
+        #[bpaf(req_flag(true))]
+        item: bool
+    };
+    let output = quote! {
+        ::bpaf::long("item").req_flag(true)
+    };
+    assert_eq!(input.to_token_stream().to_string(), output.to_string());
+}
+
 #[test]
 fn implicit_switch_argument() {
     let input: NamedField = parse_quote! {
diff --git a/tests/tests.rs b/tests/tests.rs
index 82c6493..c3ff372 100644
--- a/tests/tests.rs
+++ b/tests/tests.rs
@@ -1840,3 +1840,27 @@ Available options:
 ";
     assert_eq!(r, expected);
 }
+
+#[test]
+fn many_env() {
+    std::env::set_var("USER1", "top s3cr3t");
+    let parser = short('v')
+        .env("USER1")
+        .argument::<String>("USER")
+        .many()
+        .to_options();
+    let r = parser.run_inner(Args::from(&[])).unwrap();
+    assert_eq!(r, vec!["top s3cr3t".to_owned()]);
+}
+
+#[test]
+fn some_env() {
+    std::env::set_var("USER1", "top s3cr3t");
+    let parser = short('v')
+        .env("USER1")
+        .argument::<String>("USER")
+        .some("a")
+        .to_options();
+    let r = parser.run_inner(Args::from(&[])).unwrap();
+    assert_eq!(r, vec!["top s3cr3t".to_owned()]);
+}