Repository: pacak/bpaf
Pull request #96 (2022-09-30 21:14:44)

deduplicate help items
Contains items from https://github.com/pacak/bpaf/pull/96


Patch:
diff --git a/Changelog.md b/Changelog.md
index 07e3fe9..a473f73 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -1,5 +1,10 @@
 # Change Log
 
+## bpaf [0.6.2] - unreleased
+- cosmetic improvements
+- pure_with implementation
+  thanks to @xitep
+
 ## bpaf [0.6.1] - 2022-09-30
 - cosmetic improvements
 - completion info in sensors example
diff --git a/examples/flatten.rs b/examples/flatten.rs
index e19d89c..5847cb3 100644
--- a/examples/flatten.rs
+++ b/examples/flatten.rs
@@ -1,5 +1,5 @@
 //! All the flags don't have to live in the same structure, this example uses non derive version.
-//! with derive API you would use `extern` annotation
+//! with derive API you would use `external` annotation
 
 use bpaf::*;
 
diff --git a/src/lib.rs b/src/lib.rs
index 724afe1..e598aac 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1702,6 +1702,11 @@ pub enum CompleteDecor {
 /// This parser produces `T` without consuming anything from the command line, can be useful
 /// with [`construct!`]. As with any parsers `T` should be `Clone` and `Debug`.
 ///
+/// Both `pure` and [`pure_with`] are designed to put values into structures, to generate fallback
+/// you should be using [`fallback`](Parser::fallback) and [`fallback_with`](Parser::fallback_with).
+///
+/// See also [`pure_with`] for a pure computation that can fail.
+///
 /// # Combinatoric usage
 /// ```rust
 /// # use bpaf::*;
@@ -1716,19 +1721,27 @@ pub fn pure<T>(val: T) -> ParsePure<T> {
     ParsePure(val)
 }
 
+/// Wrap a calculated value into a `Parser`
 ///
-/// Wrap a lazily calculated value into a `Parser`
-///
-/// This parser represents an equivalent to [`pure`] with the function of [`Parser::fallback_with`].
-/// It produces `T`  by invoking the provided callback without consuming anything from the command
+/// This parser represents a possibly failing equivalent to [`pure`].
+/// It produces `T` by invoking the provided callback without consuming anything from the command
 /// line, can be useful with [`construct!`]. As with any parsers `T` should be `Clone` and `Debug`.
 ///
+/// Both [`pure`] and `pure_with` are designed to put values into structures, to generate fallback
+/// you should be using [`fallback`](Parser::fallback) and [`fallback_with`](Parser::fallback_with).
+///
+/// See also [`pure`] for a pure computation that can't fail.
+
 /// # Combinatoric usage
 /// ```rust
 /// # use bpaf::*;
 /// fn pair() -> impl Parser<bool> {
 ///     let a = long("flag-a").switch();
-///     let b = pure_with::<_, _, String>(|| {/* search for history file and try to fish out the last used value ...*/ Ok(false)});
+///     let b = pure_with::<_, _, String>(|| {
+///         // search for history file and try to fish out the last used value ...
+///         // if this computation fails - user will see it
+///         Ok(false)
+///     });
 ///     construct!([a, b])
 /// }
 /// ```
diff --git a/src/meta_help.rs b/src/meta_help.rs
index 94d4957..5b57d8f 100644
--- a/src/meta_help.rs
+++ b/src/meta_help.rs
@@ -6,7 +6,7 @@ use crate::{
     Meta,
 };
 
-#[derive(Debug)]
+#[derive(Debug, Ord, PartialEq, PartialOrd, Eq, Copy, Clone)]
 pub(crate) enum HelpItem<'a> {
     Decor {
         help: &'a str,
@@ -44,12 +44,17 @@ pub(crate) struct HelpItems<'a> {
 impl HelpItem<'_> {
     pub fn is_decor(&self) -> bool {
         match self {
-            HelpItem::Decor { .. } => true,
+            HelpItem::Decor { .. } | HelpItem::BlankDecor => true,
             _ => false,
         }
     }
 }
 
+fn dedup(items: &mut Vec<HelpItem>) {
+    let mut cur = std::collections::BTreeSet::new();
+    items.retain(move |i| i.is_decor() || cur.insert(*i));
+}
+
 impl<'a> HelpItems<'a> {
     #[inline(never)]
     pub(crate) fn classify_item(&mut self, item: &'a Item) {
@@ -141,7 +146,7 @@ impl<'a> HelpItems<'a> {
     }
 }
 
-#[derive(Debug)]
+#[derive(Debug, Ord, PartialEq, PartialOrd, Eq, Copy, Clone)]
 pub(crate) struct ShortLongHelp(ShortLong);
 
 impl ShortLongHelp {
@@ -344,6 +349,9 @@ pub(crate) fn render_help(
     let mut items = HelpItems::default();
     items.classify(parser_meta);
     items.classify(help_meta);
+    dedup(&mut items.psns);
+    dedup(&mut items.flgs);
+    dedup(&mut items.cmds);
     if !items.psns.is_empty() {
         let max_width = items
             .psns


Test patch:
diff --git a/tests/help_format.rs b/tests/help_format.rs
index a18ad02..31feb87 100644
--- a/tests/help_format.rs
+++ b/tests/help_format.rs
@@ -38,3 +38,121 @@ footer
 
     assert_eq!(r, expected);
 }
+
+#[test]
+fn duplicate_items_same_help() {
+    let a = short('a').req_flag(());
+    let b = short('b').req_flag(());
+    let c1 = short('c').help("c").switch();
+    let c2 = short('c').help("c").switch();
+    let ac = construct!(a, c1);
+    let bc = construct!(b, c2);
+    let parser = construct!([ac, bc]).to_options();
+
+    let r = parser
+        .run_inner(Args::from(&["--help"]))
+        .unwrap_err()
+        .unwrap_stdout();
+
+    let expected = "\
+Usage: (-a [-c] | -b [-c])
+
+Available options:
+    -a
+    -c          c
+    -b
+    -h, --help  Prints help information
+";
+
+    assert_eq!(r, expected);
+}
+
+#[test]
+fn duplicate_items_dif_help() {
+    let a = short('a').req_flag(());
+    let b = short('b').req_flag(());
+    let c1 = short('c').help("c1").switch();
+    let c2 = short('c').help("c2").switch();
+    let ac = construct!(a, c1);
+    let bc = construct!(b, c2);
+    let parser = construct!([ac, bc]).to_options();
+
+    let r = parser
+        .run_inner(Args::from(&["--help"]))
+        .unwrap_err()
+        .unwrap_stdout();
+
+    let expected = "\
+Usage: (-a [-c] | -b [-c])
+
+Available options:
+    -a
+    -c          c1
+    -b
+    -c          c2
+    -h, --help  Prints help information
+";
+
+    assert_eq!(r, expected);
+}
+
+#[test]
+fn duplicate_pos_items_same_help() {
+    let a = short('a').req_flag(());
+    let b = short('b').req_flag(());
+    let c1 = positional::<String>("C").help("C");
+    let c2 = positional::<String>("C").help("C");
+    let ac = construct!(a, c1);
+    let bc = construct!(b, c2);
+    let parser = construct!([ac, bc]).to_options();
+
+    let r = parser
+        .run_inner(Args::from(&["--help"]))
+        .unwrap_err()
+        .unwrap_stdout();
+
+    let expected = "\
+Usage: (-a <C> | -b <C>)
+
+Available positional items:
+    <C>  C
+
+Available options:
+    -a
+    -b
+    -h, --help  Prints help information
+";
+
+    assert_eq!(r, expected);
+}
+
+#[test]
+fn duplicate_pos_items_diff_help() {
+    let a = short('a').req_flag(());
+    let b = short('b').req_flag(());
+    let c1 = positional::<String>("C").help("C1");
+    let c2 = positional::<String>("C").help("C2");
+    let ac = construct!(a, c1);
+    let bc = construct!(b, c2);
+    let parser = construct!([ac, bc]).to_options();
+
+    let r = parser
+        .run_inner(Args::from(&["--help"]))
+        .unwrap_err()
+        .unwrap_stdout();
+
+    let expected = "\
+Usage: (-a <C> | -b <C>)
+
+Available positional items:
+    <C>  C1
+    <C>  C2
+
+Available options:
+    -a
+    -b
+    -h, --help  Prints help information
+";
+
+    assert_eq!(r, expected);
+}
diff --git a/tests/pure_with.rs b/tests/pure_with.rs
new file mode 100644
index 0000000..90d587a
--- /dev/null
+++ b/tests/pure_with.rs
@@ -0,0 +1,60 @@
+use bpaf::*;
+
+#[test]
+fn default_value_using_pure_with_ok() {
+    // ~ this is a bit artifactial (we'd use fallback_with instead actually), but it demonstrates the concept
+    let a = short('t').argument::<u32>("N");
+    let b = pure_with::<_, _, String>(|| Ok(42u32));
+
+    let opts = construct!([a, b]).to_options();
+
+    let r = opts.run_inner(Args::from(&["-t", "87"]));
+    assert_eq!(87, r.unwrap());
+
+    let r = opts.run_inner(Args::from(&[]));
+    assert_eq!(42, r.unwrap());
+}
+
+#[test]
+fn default_value_using_pure_with_err() {
+    let a = short('t').argument::<u32>("N");
+    let b = pure_with::<_, _, String>(|| Err(String::from("some-err")));
+
+    let opts = construct!([a, b]).to_options();
+    let r = opts.run_inner(Args::from(&[]));
+    let e = r.unwrap_err().unwrap_stderr();
+    assert_eq!("some-err", e);
+}
+
+#[test]
+fn default_value_using_pure_with_ok_for_some() {
+    let user_seeds = positional::<u32>("SEED").some("at least one required");
+    let last_seeds = pure_with::<_, _, String>(|| {
+        // ~ trying to lookup the last used seeds
+        Ok(vec![3, 5, 7, 11])
+    });
+    let seeds = construct!([user_seeds, last_seeds]).to_options();
+
+    let r = seeds.run_inner(Args::from(&["23", "59"]));
+    assert_eq!(vec![23, 59], r.unwrap());
+
+    let r = seeds.run_inner(Args::from(&[]));
+    assert_eq!(vec![3, 5, 7, 11], r.unwrap());
+}
+
+#[test]
+fn default_value_using_pure_with_err_for_some() {
+    let user_seeds = positional::<u32>("SEED").some("at least one required");
+    let last_seeds = pure_with(|| {
+        // ~ trying to lookup the last used seeds but we're failing
+        Err("oh, no!")
+    });
+    let default_seeds = pure(vec![1, 2]);
+    let seeds = construct!([user_seeds, last_seeds, default_seeds]).to_options();
+
+    let r = seeds.run_inner(Args::from(&["23", "59"]));
+    assert_eq!(vec![23, 59], r.unwrap());
+
+    let r = seeds.run_inner(Args::from(&[]));
+    assert_eq!(r.unwrap(), vec![1, 2]);
+}
diff --git a/tests/tests.rs b/tests/tests.rs
index 45786fb..883603a 100644
--- a/tests/tests.rs
+++ b/tests/tests.rs
@@ -1628,62 +1628,3 @@ fn default_for_many() {
     let r = parser.run_inner(Args::from(&[])).unwrap();
     assert_eq!(r, vec![String::from(".")]);
 }
-
-#[test]
-fn default_value_using_pure_with_ok() {
-    // ~ this is a bit artifactial (we'd use fallback_with instead actually), but it demonstrates the concept
-    let a = short('t').argument::<u32>("N");
-    let b = pure_with::<_, _, String>(|| Ok(42u32));
-
-    let opts = construct!([a, b]).to_options();
-
-    let r = opts.run_inner(Args::from(&["-t", "87"]));
-    assert_eq!(87, r.unwrap());
-
-    let r = opts.run_inner(Args::from(&[]));
-    assert_eq!(42, r.unwrap());
-}
-
-#[test]
-fn default_value_using_pure_with_err() {
-    let a = short('t').argument::<u32>("N");
-    let b = pure_with::<_, _, String>(|| Err(String::from("some-err")));
-
-    let opts = construct!([a, b]).to_options();
-    let r = opts.run_inner(Args::from(&[]));
-    let e = r.unwrap_err().unwrap_stderr();
-    assert_eq!("some-err", e);
-}
-
-#[test]
-fn default_value_using_pure_with_ok_for_some() {
-    let user_seeds = positional::<u32>("SEED").some("at least one required");
-    let last_seeds = pure_with::<_, _, String>(|| {
-        // ~ trying to lookup the last used seeds
-        Ok(vec![3, 5, 7, 11])
-    });
-    let seeds = construct!([user_seeds, last_seeds]).to_options();
-
-    let r = seeds.run_inner(Args::from(&["23", "59"]));
-    assert_eq!(vec![23, 59], r.unwrap());
-
-    let r = seeds.run_inner(Args::from(&[]));
-    assert_eq!(vec![3, 5, 7, 11], r.unwrap());
-}
-
-#[test]
-fn default_value_using_pure_with_err_for_some() {
-    let user_seeds = positional::<u32>("SEED").some("at least one required");
-    let last_seeds = pure_with(|| {
-        // ~ trying to lookup the last used seeds but we're failing
-        Err("oh, no!")
-    });
-    let default_seeds = pure(vec![1, 2]);
-    let seeds = construct!([user_seeds, last_seeds, default_seeds]).to_options();
-
-    let r = seeds.run_inner(Args::from(&["23", "59"]));
-    assert_eq!(vec![23, 59], r.unwrap());
-
-    let r = seeds.run_inner(Args::from(&[]));
-    assert_eq!(r.unwrap(), vec![1, 2]);
-}