Repository: pacak/bpaf (rust)

Issue:
cauwugo add: `--rev` has the wrong value name (TAG instead of REV)

Pull request:
Cauwugo add fixes
Mostly making the command line options closer to one of `cargo`'s

Fixes #91

Patch:
diff --git a/bpaf_cauwugo/README.md b/bpaf_cauwugo/README.md
index aafe9e2..8aec43a 100644
--- a/bpaf_cauwugo/README.md
+++ b/bpaf_cauwugo/README.md
@@ -37,7 +37,7 @@ continuations is mostly fixed, but can include things like file names or parts o
 dynamic completions set of possible continuations is computed on runtime by a shell function or
 a program.
 
-Cargo comes with static completion for `zsh` you can find its contents by typing
+Cargo comes with a completion file for `zsh` you can find its contents by typing
 ```console
 % cat "$(rustc --print sysroot)"/share/zsh/site-functions/_cargo
 ```
diff --git a/bpaf_cauwugo/src/add.rs b/bpaf_cauwugo/src/add.rs
index 42899b3..83dc98c 100644
--- a/bpaf_cauwugo/src/add.rs
+++ b/bpaf_cauwugo/src/add.rs
@@ -9,16 +9,29 @@ pub struct Add {
     #[bpaf(external)]
     pub package: Option<&'static str>,
 
-    /// Add as dev dependency
-    pub dev: bool,
+    #[bpaf(external)]
+    pub dest: Dest,
 
-    /// Add as build dependency
-    pub build: bool,
+    /// Don't actually write the manifest
+    pub dry_run: bool,
 
     #[bpaf(external)]
     pub source: Source,
 }
 
+#[derive(Debug, Clone, Bpaf)]
+#[bpaf(fallback(Dest::Normal))]
+pub enum Dest {
+    /// Add as dev dependency
+    Dev,
+
+    /// Add as build dependency
+    Build,
+
+    #[bpaf(hide)]
+    Normal,
+}
+
 #[derive(Debug, Clone, Bpaf)]
 pub enum Source {
     Git {
@@ -34,13 +47,23 @@ pub enum Source {
         /// Git tag to download the crate from
         tag: Option<String>,
 
-        #[bpaf(long, argument("TAG"))]
+        #[bpaf(long, argument("REV"))]
         /// Git reference to download the crate from
         rev: Option<String>,
+
+        /// Package name
+        #[bpaf(positional("DEP"))]
+        // we can go to the URL and check what the names are available there...
+        name: Option<String>,
     },
     Local {
         /// Filesystem path to local crate to add
         path: PathBuf,
+
+        /// Package name
+        #[bpaf(positional("DEP"))]
+        // and ask cargo for names available there...
+        name: Option<String>,
     },
     Crates {
         /// Package registry to use instead of crates.io
@@ -85,22 +108,33 @@ impl Add {
     pub fn pass_to_cmd(&self, cmd: &mut Command) {
         cmd.arg("add");
         pass_arg!(cmd, self.package, "--package");
-        pass_flag!(cmd, self.dev, "--dev");
-        pass_flag!(cmd, self.build, "--build");
+        match self.dest {
+            Dest::Dev => {
+                cmd.arg("--dev");
+            }
+            Dest::Build => {
+                cmd.arg("--build");
+            }
+            Dest::Normal => {}
+        }
+        pass_flag!(cmd, self.dry_run, "--dry-run");
         match &self.source {
             Source::Git {
                 git,
                 branch,
                 tag,
                 rev,
+                name,
             } => {
                 pass_req_arg!(cmd, git, "--git");
                 pass_arg!(cmd, branch, "--branch");
                 pass_arg!(cmd, tag, "--tag");
                 pass_arg!(cmd, rev, "--rev");
+                pass_pos!(cmd, name);
             }
-            Source::Local { path } => {
+            Source::Local { path, name } => {
                 pass_req_arg!(cmd, path, "--path");
+                pass_pos!(cmd, name);
             }
             Source::Crates { registry, name } => {
                 cmd.arg(name);
diff --git a/bpaf_cauwugo/src/lib.rs b/bpaf_cauwugo/src/lib.rs
index 8bf0a9f..b3cdb1d 100644
--- a/bpaf_cauwugo/src/lib.rs
+++ b/bpaf_cauwugo/src/lib.rs
@@ -14,6 +14,14 @@ macro_rules! pass_arg {
     };
 }
 
+macro_rules! pass_pos {
+    ($cmd:ident, $field:expr) => {
+        for v in $field {
+            $cmd.arg(v);
+        }
+    };
+}
+
 macro_rules! pass_req_arg {
     ($cmd:ident, $field:expr, $value:literal) => {
         $cmd.arg($value).arg($field);
diff --git a/src/meta_usage.rs b/src/meta_usage.rs
index a9607b9..81cc67f 100644
--- a/src/meta_usage.rs
+++ b/src/meta_usage.rs
@@ -79,10 +79,13 @@ fn collect_usage_meta(meta: &Meta, is_pos: &mut bool) -> Option<UsageMeta> {
 
         Meta::Or(xs) => {
             let mut saw_command = false;
+            let mut any_pos = false;
             let mut items = xs
                 .iter()
                 .filter_map(|x| {
-                    let usage_meta = collect_usage_meta(x, is_pos)?;
+                    let mut top_pos = *is_pos;
+                    let usage_meta = collect_usage_meta(x, &mut top_pos)?;
+                    any_pos |= top_pos;
                     if let UsageMeta::Command = &usage_meta {
                         if saw_command {
                             None
@@ -95,7 +98,7 @@ fn collect_usage_meta(meta: &Meta, is_pos: &mut bool) -> Option<UsageMeta> {
                     }
                 })
                 .collect::<Vec<_>>();
-
+            *is_pos |= any_pos;
             match items.len() {
                 0 => return None,
                 1 => items.remove(0),


Test patch:
diff --git a/tests/meta_usage.rs b/tests/meta_usage.rs
index ae9c8fc..ba88c38 100644
--- a/tests/meta_usage.rs
+++ b/tests/meta_usage.rs
@@ -223,3 +223,16 @@ fn a_or_b_opt_and_c() {
     let parser = construct!(ab, c).to_options();
     assert_usage(parser, "[-a A | -b B] <C>");
 }
+
+#[test]
+fn positionals_in_branches_are_okay() {
+    let a = short('a').argument::<String>("A");
+    let b = short('b').argument::<String>("B");
+    let c = positional::<String>("C");
+    let d = positional::<String>("D");
+
+    let ac = construct!(a, c);
+    let bd = construct!(b, d);
+    let parser = construct!([ac, bd]).to_options();
+    assert_usage(parser, "(-a A <C> | -b B <D>)");
+}