Repository: pacak/bpaf
Pull request #238 (2023-07-14 19:44:42)

Parser::collect`
allows collecting multiple items into an arbitrary `FromIterator` collection


Patch:
diff --git a/Changelog.md b/Changelog.md
index 6120bdc..9ba8645 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -1,5 +1,8 @@
 # Change Log
 
+## bpaf [0.9.3], bpaf_derive [0.5.3] - Unreleased
+- `Parser::collect` allows to collect multiple items into an arbitrary `FromIterator` collection
+
 ## bpaf [0.9.2], bpaf_derive [0.5.2] - 2023-07-13
 - with `docgen` feature you can render documentation as markdown
 - cosmetic changes to error messages
diff --git a/bpaf_derive/src/attrs.rs b/bpaf_derive/src/attrs.rs
index df9d724..9ee23ef 100644
--- a/bpaf_derive/src/attrs.rs
+++ b/bpaf_derive/src/attrs.rs
@@ -210,6 +210,7 @@ impl ToTokens for PostParse {
             PostParse::Adjacent { .. } => quote!(adjacent()),
             PostParse::Catch { .. } => quote!(catch()),
             PostParse::Many { .. } => quote!(many()),
+            PostParse::Collect { .. } => quote!(collect()),
             PostParse::Count { .. } => quote!(count()),
             PostParse::Some_ { msg, .. } => quote!(some(#msg)),
             PostParse::Map { f, .. } => quote!(map(#f)),
@@ -247,6 +248,7 @@ pub(crate) enum PostParse {
     Adjacent { span: Span },
     Catch { span: Span },
     Many { span: Span },
+    Collect { span: Span },
     Count { span: Span },
     Some_ { span: Span, msg: Box<Expr> },
     Map { span: Span, f: Box<Expr> },
@@ -260,6 +262,7 @@ impl PostParse {
             Self::Adjacent { span }
             | Self::Catch { span }
             | Self::Many { span }
+            | Self::Collect { span }
             | Self::Count { span }
             | Self::Some_ { span, .. }
             | Self::Map { span, .. }
@@ -458,6 +461,8 @@ impl PostParse {
             Self::Catch { span }
         } else if kw == "many" {
             Self::Many { span }
+        } else if kw == "collect" {
+            Self::Collect { span }
         } else if kw == "count" {
             Self::Count { span }
         } else if kw == "map" {
diff --git a/docs2/src/collect/cases.md b/docs2/src/collect/cases.md
new file mode 100644
index 0000000..d452261
--- /dev/null
+++ b/docs2/src/collect/cases.md
@@ -0,0 +1,19 @@
+In usage lines `collect` items are indicated with `...`
+> --help
+
+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 --argument 20
+
+If there's no matching parameters - it would produce an empty set. Note, in case of
+[`switch`](NamedArg::switch) parser or other parsers that can succeed without consuming anything
+it would capture that value so `many` captures the first one of those.
+You can use [`req_flag`](NamedArg::req_flag) to avoid that.
+
+>
+
+For parsers that can succeed without consuming anything such as `flag` or `switch` - `many`
+only collects values as long as they produce something
+
+> --switch --switch
diff --git a/docs2/src/collect/combine.rs b/docs2/src/collect/combine.rs
new file mode 100644
index 0000000..523b346
--- /dev/null
+++ b/docs2/src/collect/combine.rs
@@ -0,0 +1,18 @@
+//
+use bpaf::*;
+use std::collections::BTreeSet;
+
+#[derive(Debug, Clone)]
+pub struct Options {
+    argument: BTreeSet<u32>,
+    switches: BTreeSet<bool>,
+}
+
+pub fn options() -> OptionParser<Options> {
+    let argument = long("argument")
+        .help("important argument")
+        .argument("ARG")
+        .collect();
+    let switches = long("switch").help("some switch").switch().collect();
+    construct!(Options { argument, switches }).to_options()
+}
diff --git a/docs2/src/collect/derive.rs b/docs2/src/collect/derive.rs
new file mode 100644
index 0000000..f82cdc0
--- /dev/null
+++ b/docs2/src/collect/derive.rs
@@ -0,0 +1,14 @@
+//
+use bpaf::*;
+use std::collections::BTreeSet;
+
+#[derive(Debug, Clone, Bpaf)]
+#[bpaf(options)]
+pub struct Options {
+    /// important argument
+    #[bpaf(argument::<u32>("ARG"), collect)]
+    argument: BTreeSet<u32>,
+    /// some switch
+    #[bpaf(long("switch"), switch, collect)]
+    switches: BTreeSet<bool>,
+}
diff --git a/src/docs2/collect.md b/src/docs2/collect.md
new file mode 100644
index 0000000..3e238d5
--- /dev/null
+++ b/src/docs2/collect.md
@@ -0,0 +1,102 @@
+<details><summary>Combinatoric example</summary>
+
+```no_run
+use std::collections::BTreeSet;
+
+#[derive(Debug, Clone)]
+pub struct Options {
+    argument: BTreeSet<u32>,
+    switches: BTreeSet<bool>,
+}
+
+pub fn options() -> OptionParser<Options> {
+    let argument = long("argument")
+        .help("important argument")
+        .argument("ARG")
+        .collect();
+    let switches = long("switch").help("some switch").switch().collect();
+    construct!(Options { argument, switches }).to_options()
+}
+```
+
+</details>
+<details><summary>Derive example</summary>
+
+```no_run
+use std::collections::BTreeSet;
+
+#[derive(Debug, Clone, Bpaf)]
+#[bpaf(options)]
+pub struct Options {
+    /// important argument
+    #[bpaf(argument::<u32>("ARG"), collect)]
+    argument: BTreeSet<u32>,
+    /// some switch
+    #[bpaf(long("switch"), switch, collect)]
+    switches: BTreeSet<bool>,
+}
+```
+
+</details>
+<details><summary>Output</summary>
+
+In usage lines `collect` items are indicated with `...`
+
+<div class='bpaf-doc'>
+$ app --help<br>
+<p><b>Usage</b>: <tt><b>app</b></tt> <tt><b>--argument</b></tt>=<tt><i>ARG</i></tt>... [<tt><b>--switch</b></tt>]...</p><p><div>
+<b>Available options:</b></div><dl><dt><tt><b>    --argument</b></tt>=<tt><i>ARG</i></tt></dt>
+<dd>important argument</dd>
+<dt><tt><b>    --switch</b></tt></dt>
+<dd>some switch</dd>
+<dt><tt><b>-h</b></tt>, <tt><b>--help</b></tt></dt>
+<dd>Prints help information</dd>
+</dl>
+</p>
+<style>
+div.bpaf-doc {
+    padding: 14px;
+    background-color:var(--code-block-background-color);
+    font-family: "Source Code Pro", monospace;
+    margin-bottom: 0.75em;
+}
+div.bpaf-doc dt { margin-left: 1em; }
+div.bpaf-doc dd { margin-left: 3em; }
+div.bpaf-doc dl { margin-top: 0; padding-left: 1em; }
+div.bpaf-doc  { padding-left: 1em; }
+</style>
+</div>
+
+
+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
+
+
+<div class='bpaf-doc'>
+$ app --argument 10 --argument 20 --argument 20<br>
+Options { argument: {10, 20}, switches: {false} }
+</div>
+
+
+If there's no matching parameters - it would produce an empty set. Note, in case of
+[`switch`](NamedArg::switch) parser or other parsers that can succeed without consuming anything
+it would capture that value so `many` captures the first one of those.
+You can use [`req_flag`](NamedArg::req_flag) to avoid that.
+
+
+<div class='bpaf-doc'>
+$ app <br>
+Options { argument: {}, switches: {false} }
+</div>
+
+
+For parsers that can succeed without consuming anything such as `flag` or `switch` - `many`
+only collects values as long as they produce something
+
+
+<div class='bpaf-doc'>
+$ app --switch --switch<br>
+Options { argument: {}, switches: {true} }
+</div>
+
+</details>
\ No newline at end of file
diff --git a/src/lib.rs b/src/lib.rs
index e76a2e7..399ea2e 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -317,7 +317,7 @@ pub mod parsers {
     pub use crate::params::{NamedArg, ParseAny, ParseArgument, ParseCommand, ParsePositional};
     #[doc(inline)]
     pub use crate::structs::{
-        ParseCon, ParseCount, ParseFallback, ParseFallbackWith, ParseLast, ParseMany,
+        ParseCollect, ParseCon, ParseCount, ParseFallback, ParseFallbackWith, ParseLast, ParseMany,
         ParseOptional, ParseSome,
     };
 }
@@ -339,9 +339,9 @@ use crate::{
     params::build_positional,
     parsers::{NamedArg, ParseAny, ParseCommand, ParsePositional},
     structs::{
-        ParseCount, ParseFail, ParseFallback, ParseFallbackWith, ParseGroupHelp, ParseGuard,
-        ParseHide, ParseLast, ParseMany, ParseMap, ParseOptional, ParseOrElse, ParsePure,
-        ParsePureWith, ParseSome, ParseUsage, ParseWith, ParseWithGroupHelp,
+        ParseCollect, ParseCount, ParseFail, ParseFallback, ParseFallbackWith, ParseGroupHelp,
+        ParseGuard, ParseHide, ParseLast, ParseMany, ParseMap, ParseOptional, ParseOrElse,
+        ParsePure, ParsePureWith, ParseSome, ParseUsage, ParseWith, ParseWithGroupHelp,
     },
 };
 
@@ -764,7 +764,8 @@ pub trait Parser<T> {
     ///
     /// # See also
     /// [`some`](Parser::some) also collects results to a vector but requires at least one
-    /// element to succeed
+    /// element to succeed, [`collect`](Parser::collect) collects results into a [`FromIterator`]
+    /// structure
     fn many(self) -> ParseMany<Self>
     where
         Self: Sized,
@@ -776,6 +777,34 @@ pub trait Parser<T> {
     }
     // }}}
 
+    // {{{ collect
+    /// Transform parser into a collection parser
+    ///
+    /// A generic variant of [`many`](Parser::many), instead of collecting into a vector
+    /// it collects into any collection that implements [`FromIterator`] trait
+    ///
+    /// `collect` preserves any parsing falures and propagates them outwards, with extra
+    /// [`catch`](ParseCollect::catch) statement you can instead stop at the first value
+    /// that failed to parse and ignore it and all the subsequent ones.
+    ///
+    #[cfg_attr(not(doctest), doc = include_str!("docs2/collect.md"))]
+    ///
+    /// `collect` 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.
+    fn collect<C>(self) -> ParseCollect<Self, C, T>
+    where
+        C: FromIterator<T>,
+        Self: Sized,
+    {
+        ParseCollect {
+            inner: self,
+            catch: false,
+            ctx: PhantomData,
+        }
+    }
+    // }}}
+
     // {{{ some
     /// Consume one or more items from a command line and collect them into a [`Vec`]
     ///
@@ -793,7 +822,9 @@ pub trait Parser<T> {
     ///
     /// # See also
     /// [`many`](Parser::many) also collects results to a vector but succeeds with
-    /// no matching values
+    /// no matching values. [`collect`](Parser::collect) collects results into a [`FromIterator`]
+    /// structure
+
     #[must_use]
     fn some(self, message: &'static str) -> ParseSome<Self>
     where
diff --git a/src/structs.rs b/src/structs.rs
index 01aa0bd..8ca99d1 100644
--- a/src/structs.rs
+++ b/src/structs.rs
@@ -144,17 +144,10 @@ where
 {
     fn eval(&self, args: &mut State) -> Result<Vec<T>, Error> {
         let mut res = Vec::new();
-        let mut len = args.len();
-
-        while let Some(val) = parse_option(&self.inner, args, self.catch)? {
-            // 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 {
-                break;
-            }
+        let mut len = usize::MAX;
+
+        while let Some(val) = parse_option(&self.inner, &mut len, args, self.catch)? {
+            res.push(val);
         }
 
         if res.is_empty() {
@@ -169,6 +162,48 @@ where
     }
 }
 
+/// Apply inner parser several times and collect results into `FromIterator`, created with
+/// [`collect`](Parser::collect),
+/// Implements [`catch`](ParseCollect::catch)
+pub struct ParseCollect<P, C, T> {
+    pub(crate) inner: P,
+    pub(crate) catch: bool,
+    pub(crate) ctx: PhantomData<(C, T)>,
+}
+
+impl<T, C, P> ParseCollect<P, C, T> {
+    #[must_use]
+    /// Handle parse failures
+    ///
+    /// Can be useful to decide to skip parsing of some items on a command line
+    /// When parser succeeds - `catch` version would return a value as usual
+    /// if it fails - `catch` would restore all the consumed values and return None.
+    ///
+    /// There's several structures that implement this attribute: [`ParseOptional`], [`ParseMany`]
+    /// and [`ParseSome`], behavior should be identical for all of them.
+    #[cfg_attr(not(doctest), doc = include_str!("docs2/some_catch.md"))]
+    pub fn catch(mut self) -> Self {
+        self.catch = true;
+        self
+    }
+}
+
+impl<T, C, P> Parser<C> for ParseCollect<P, C, T>
+where
+    P: Parser<T>,
+    C: FromIterator<T>,
+{
+    fn eval(&self, args: &mut State) -> Result<C, Error> {
+        let mut len = usize::MAX;
+        std::iter::from_fn(|| parse_option(&self.inner, &mut len, args, self.catch).transpose())
+            .collect::<Result<C, Error>>()
+    }
+
+    fn meta(&self) -> Meta {
+        Meta::Many(Box::new(Meta::Required(Box::new(self.inner.meta()))))
+    }
+}
+
 /// Parser that returns results as usual but not shown in `--help` output, created with
 /// [`Parser::hide`]
 pub struct ParseHide<P> {
@@ -558,7 +593,8 @@ where
     fn eval(&self, args: &mut State) -> Result<usize, Error> {
         let mut res = 0;
         let mut current = args.len();
-        while (parse_option(&self.inner, args, false)?).is_some() {
+        let mut len = usize::MAX;
+        while (parse_option(&self.inner, &mut len, args, false)?).is_some() {
             res += 1;
             if current == args.len() {
                 break;
@@ -586,7 +622,8 @@ where
     fn eval(&self, args: &mut State) -> Result<T, Error> {
         let mut last = None;
         let mut current = args.len();
-        while let Some(val) = parse_option(&self.inner, args, false)? {
+        let mut len = usize::MAX;
+        while let Some(val) = parse_option(&self.inner, &mut len, args, false)? {
             last = Some(val);
             if current == args.len() {
                 break;
@@ -618,7 +655,8 @@ where
     P: Parser<T>,
 {
     fn eval(&self, args: &mut State) -> Result<Option<T>, Error> {
-        parse_option(&self.inner, args, self.catch)
+        let mut len = usize::MAX;
+        parse_option(&self.inner, &mut len, args, self.catch)
     }
 
     fn meta(&self) -> Meta {
@@ -672,13 +710,25 @@ impl<P> ParseMany<P> {
 }
 
 /// try to parse
-fn parse_option<P, T>(parser: &P, args: &mut State, catch: bool) -> Result<Option<T>, Error>
+fn parse_option<P, T>(
+    parser: &P,
+    len: &mut usize,
+    args: &mut State,
+    catch: bool,
+) -> Result<Option<T>, Error>
 where
     P: Parser<T>,
 {
     let mut orig_args = args.clone();
     match parser.eval(args) {
-        Ok(val) => Ok(Some(val)),
+        // we keep including values for as long as we consume values from the argument
+        // list or at least one value
+        Ok(val) => Ok(if args.len() < *len {
+            *len = args.len();
+            Some(val)
+        } else {
+            None
+        }),
         Err(Error(err)) => {
             // this is safe to return Ok(None) in following scenarios
             // when inner parser never consumed anything and
@@ -713,19 +763,9 @@ where
     P: Parser<T>,
 {
     fn eval(&self, args: &mut State) -> Result<Vec<T>, Error> {
-        let mut res = Vec::new();
-        let mut len = args.len();
-        while let Some(val) = parse_option(&self.inner, args, self.catch)? {
-            // 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 {
-                break;
-            }
-        }
-        Ok(res)
+        let mut len = usize::MAX;
+        std::iter::from_fn(|| parse_option(&self.inner, &mut len, args, self.catch).transpose())
+            .collect::<Result<Vec<T>, Error>>()
     }
 
     fn meta(&self) -> Meta {


Test patch:
diff --git a/bpaf_derive/src/field_tests.rs b/bpaf_derive/src/field_tests.rs
index 2d1e8d0..e81165f 100644
--- a/bpaf_derive/src/field_tests.rs
+++ b/bpaf_derive/src/field_tests.rs
@@ -320,6 +320,18 @@ fn many_catch() {
     assert_eq!(input.to_token_stream().to_string(), output.to_string());
 }
 
+#[test]
+fn collect_catch() {
+    let input: NamedField = parse_quote! {
+        #[bpaf(argument("FILE"), collect, catch)]
+        files: Vec<std::path::PathBuf>
+    };
+    let output = quote! {
+        ::bpaf::long("files").argument::<std::path::PathBuf>("FILE").collect().catch()
+    };
+    assert_eq!(input.to_token_stream().to_string(), output.to_string());
+}
+
 #[test]
 fn option_catch() {
     let input: NamedField = parse_quote! {
diff --git a/tests/tests.rs b/tests/tests.rs
index ca529cd..f4fc5ca 100644
--- a/tests/tests.rs
+++ b/tests/tests.rs
@@ -1370,6 +1370,23 @@ fn parse_many_errors_positional() {
     assert_eq!(r, "couldn't parse `x`: invalid digit found in string");
 }
 
+#[test]
+fn parse_collect_flag() {
+    let p = short('p')
+        .argument::<u32>("N")
+        .collect::<Vec<_>>()
+        .to_options();
+
+    let r = p.run_inner(&["-p", "1", "-p", "2"]).unwrap();
+    assert_eq!(r, vec![1, 2]);
+
+    let r = p
+        .run_inner(&["-p", "1", "-p", "x"])
+        .unwrap_err()
+        .unwrap_stderr();
+    assert_eq!(r, "couldn't parse `x`: invalid digit found in string");
+}
+
 #[test]
 fn parse_many_errors_flag() {
     let p = short('p').argument::<u32>("N").many().to_options();