Repository: google/argh
Pull request #143 (2022-09-22 17:40:26)

Add 'remainder': greedy positional arguments
This adds a new variant of positional arguments that, once they start processing, consume all arguments after as part of the positional argument, as if they always had '--' in front or behind them.

We need this for deferring subcommand parsing in ffx until after we've read the main command's arguments.


Patch:
diff --git a/argh/src/lib.rs b/argh/src/lib.rs
index 7ca94a3..c507be0 100644
--- a/argh/src/lib.rs
+++ b/argh/src/lib.rs
@@ -127,6 +127,34 @@
 //! The last positional argument may include a default, or be wrapped in
 //! `Option` or `Vec` to indicate an optional or repeating positional argument.
 //!
+//! If your final positional argument has the `greedy` option on it, it will consume
+//! any arguments after it as if a `--` were placed before the first argument to
+//! match the greedy positional:
+//!
+//! ```rust
+//! use argh::FromArgs;
+//! #[derive(FromArgs, PartialEq, Debug)]
+//! /// A command with a greedy positional argument at the end.
+//! struct WithGreedyPositional {
+//!     /// some stuff
+//!     #[argh(option)]
+//!     stuff: Option<String>,
+//!     #[argh(positional, greedy)]
+//!     all_the_rest: Vec<String>,
+//! }
+//! ```
+//!
+//! Now if you pass `--stuff Something` after a positional argument, it will
+//! be consumed by `all_the_rest` instead of setting the `stuff` field.
+//!
+//! Note that `all_the_rest` won't be listed as a positional argument in the
+//! long text part of help output (and it will be listed at the end of the usage
+//! line as `[all_the_rest...]`), and it's up to the caller to append any
+//! extra help output for the meaning of the captured arguments. This is to
+//! enable situations where some amount of argument processing needs to happen
+//! before the rest of the arguments can be interpreted, and shouldn't be used
+//! for regular use as it might be confusing.
+//!
 //! Subcommands are also supported. To use a subcommand, declare a separate
 //! `FromArgs` type for each subcommand as well as an enum that cases
 //! over each command:
@@ -839,7 +867,7 @@ pub fn parse_struct_args(
             }
         }
 
-        parse_positionals.parse(&mut positional_index, next_arg)?;
+        options_ended |= parse_positionals.parse(&mut positional_index, next_arg)?;
     }
 
     if help {
@@ -910,25 +938,31 @@ pub enum ParseStructOption<'a> {
 pub struct ParseStructPositionals<'a> {
     pub positionals: &'a mut [ParseStructPositional<'a>],
     pub last_is_repeating: bool,
+    pub last_is_greedy: bool,
 }
 
 impl<'a> ParseStructPositionals<'a> {
     /// Parse the next positional argument.
     ///
     /// `arg`: the argument supplied by the user.
-    fn parse(&mut self, index: &mut usize, arg: &str) -> Result<(), EarlyExit> {
+    ///
+    /// Returns true if non-positional argument parsing should stop
+    /// after this one.
+    fn parse(&mut self, index: &mut usize, arg: &str) -> Result<bool, EarlyExit> {
         if *index < self.positionals.len() {
             self.positionals[*index].parse(arg)?;
 
-            // Don't increment position if we're at the last arg
-            // *and* the last arg is repeating.
-            let skip_increment = self.last_is_repeating && *index == self.positionals.len() - 1;
-
-            if !skip_increment {
+            if self.last_is_repeating && *index == self.positionals.len() - 1 {
+                // Don't increment position if we're at the last arg
+                // *and* the last arg is repeating. If it's also remainder,
+                // halt non-option processing after this.
+                Ok(self.last_is_greedy)
+            } else {
+                // If it is repeating, though, increment the index and continue
+                // processing options.
                 *index += 1;
+                Ok(false)
             }
-
-            Ok(())
         } else {
             Err(EarlyExit { output: unrecognized_arg(arg), status: Err(()) })
         }
diff --git a/argh_derive/src/help.rs b/argh_derive/src/help.rs
index 276b706..c9daad5 100644
--- a/argh_derive/src/help.rs
+++ b/argh_derive/src/help.rs
@@ -30,7 +30,8 @@ pub(crate) fn help(
     #![allow(clippy::format_push_string)]
     let mut format_lit = "Usage: {command_name}".to_string();
 
-    let positional = fields.iter().filter(|f| f.kind == FieldKind::Positional);
+    let positional =
+        fields.iter().filter(|f| f.kind == FieldKind::Positional && f.attrs.greedy.is_none());
     let mut has_positional = false;
     for arg in positional.clone() {
         has_positional = true;
@@ -44,6 +45,13 @@ pub(crate) fn help(
         option_usage(&mut format_lit, option);
     }
 
+    let remain =
+        fields.iter().filter(|f| f.kind == FieldKind::Positional && f.attrs.greedy.is_some());
+    for arg in remain {
+        format_lit.push(' ');
+        positional_usage(&mut format_lit, arg);
+    }
+
     if let Some(subcommand) = subcommand {
         format_lit.push(' ');
         if !subcommand.optionality.is_required() {
@@ -143,13 +151,17 @@ fn positional_usage(out: &mut String, field: &StructField<'_>) {
     if !field.optionality.is_required() {
         out.push('[');
     }
-    out.push('<');
+    if field.attrs.greedy.is_none() {
+        out.push('<');
+    }
     let name = field.arg_name();
     out.push_str(&name);
     if field.optionality == Optionality::Repeating {
         out.push_str("...");
     }
-    out.push('>');
+    if field.attrs.greedy.is_none() {
+        out.push('>');
+    }
     if !field.optionality.is_required() {
         out.push(']');
     }
diff --git a/argh_derive/src/lib.rs b/argh_derive/src/lib.rs
index 0a190f5..49a6c17 100644
--- a/argh_derive/src/lib.rs
+++ b/argh_derive/src/lib.rs
@@ -117,7 +117,7 @@ impl<'a> StructField<'a> {
                 field,
                 concat!(
                     "Missing `argh` field kind attribute.\n",
-                    "Expected one of: `switch`, `option`, `subcommand`, `positional`",
+                    "Expected one of: `switch`, `option`, `remaining`, `subcommand`, `positional`",
                 ),
             );
             return None;
@@ -275,6 +275,10 @@ fn impl_from_args_struct_from_args<'a>(
         .last()
         .map(|field| field.optionality == Optionality::Repeating)
         .unwrap_or(false);
+    let last_positional_is_greedy = positional_fields
+        .last()
+        .map(|field| field.kind == FieldKind::Positional && field.attrs.greedy.is_some())
+        .unwrap_or(false);
 
     let flag_output_table = fields.iter().filter_map(|field| {
         let field_name = &field.field.ident;
@@ -348,6 +352,7 @@ fn impl_from_args_struct_from_args<'a>(
                         )*
                     ],
                     last_is_repeating: #last_positional_is_repeating,
+                    last_is_greedy: #last_positional_is_greedy,
                 },
                 #parse_subcommands,
                 &|| #help,
@@ -384,6 +389,10 @@ fn impl_from_args_struct_redact_arg_values<'a>(
         .last()
         .map(|field| field.optionality == Optionality::Repeating)
         .unwrap_or(false);
+    let last_positional_is_greedy = positional_fields
+        .last()
+        .map(|field| field.kind == FieldKind::Positional && field.attrs.greedy.is_some())
+        .unwrap_or(false);
 
     let flag_output_table = fields.iter().filter_map(|field| {
         let field_name = &field.field.ident;
@@ -459,6 +468,7 @@ fn impl_from_args_struct_redact_arg_values<'a>(
                         )*
                     ],
                     last_is_repeating: #last_positional_is_repeating,
+                    last_is_greedy: #last_positional_is_greedy,
                 },
                 #redact_subcommands,
                 &|| #help,
diff --git a/argh_derive/src/parse_attrs.rs b/argh_derive/src/parse_attrs.rs
index 212b184..f5fa841 100644
--- a/argh_derive/src/parse_attrs.rs
+++ b/argh_derive/src/parse_attrs.rs
@@ -18,6 +18,7 @@ pub struct FieldAttrs {
     pub long: Option<syn::LitStr>,
     pub short: Option<syn::LitChar>,
     pub arg_name: Option<syn::LitStr>,
+    pub greedy: Option<syn::Path>,
 }
 
 /// The purpose of a particular field on a `#![derive(FromArgs)]` struct.
@@ -120,13 +121,15 @@ impl FieldAttrs {
                         FieldKind::Positional,
                         &mut this.field_type,
                     );
+                } else if name.is_ident("greedy") {
+                    this.greedy = Some(name.clone());
                 } else {
                     errors.err(
                         &meta,
                         concat!(
                             "Invalid field-level `argh` attribute\n",
-                            "Expected one of: `arg_name`, `default`, `description`, `from_str_fn`, `long`, ",
-                            "`option`, `short`, `subcommand`, `switch`",
+                            "Expected one of: `arg_name`, `default`, `description`, `from_str_fn`, `greedy`, ",
+                            "`long`, `option`, `short`, `subcommand`, `switch`",
                         ),
                     );
                 }
@@ -144,6 +147,16 @@ impl FieldAttrs {
             }
         }
 
+        match (&this.greedy, this.field_type.as_ref().map(|f| f.kind)) {
+            (Some(_), Some(FieldKind::Positional)) => {}
+            (Some(greedy), Some(_)) => errors.err(
+                &greedy,
+                "`greedy` may only be specified on `#[argh(positional)]` \
+                    fields",
+            ),
+            _ => {}
+        }
+
         if let Some(d) = &this.description {
             check_option_description(errors, d.content.value().trim(), d.content.span());
         }


Test patch:
diff --git a/argh/tests/lib.rs b/argh/tests/lib.rs
index b3bc5ed..584897c 100644
--- a/argh/tests/lib.rs
+++ b/argh/tests/lib.rs
@@ -458,6 +458,90 @@ Options:
         );
     }
 
+    #[derive(FromArgs, Debug, PartialEq)]
+    /// Woot
+    struct LastRepeatingGreedy {
+        #[argh(positional)]
+        /// fooey
+        a: u32,
+        #[argh(switch)]
+        /// woo
+        b: bool,
+        #[argh(option)]
+        /// stuff
+        c: Option<String>,
+        #[argh(positional, greedy)]
+        /// fooey
+        d: Vec<String>,
+    }
+
+    #[test]
+    fn positional_greedy() {
+        assert_output(&["5"], LastRepeatingGreedy { a: 5, b: false, c: None, d: vec![] });
+        assert_output(
+            &["5", "foo"],
+            LastRepeatingGreedy { a: 5, b: false, c: None, d: vec!["foo".into()] },
+        );
+        assert_output(
+            &["5", "foo", "bar"],
+            LastRepeatingGreedy { a: 5, b: false, c: None, d: vec!["foo".into(), "bar".into()] },
+        );
+        assert_output(
+            &["5", "--b", "foo", "bar"],
+            LastRepeatingGreedy { a: 5, b: true, c: None, d: vec!["foo".into(), "bar".into()] },
+        );
+        assert_output(
+            &["5", "foo", "bar", "--b"],
+            LastRepeatingGreedy {
+                a: 5,
+                b: false,
+                c: None,
+                d: vec!["foo".into(), "bar".into(), "--b".into()],
+            },
+        );
+        assert_output(
+            &["5", "--c", "hi", "foo", "bar"],
+            LastRepeatingGreedy {
+                a: 5,
+                b: false,
+                c: Some("hi".into()),
+                d: vec!["foo".into(), "bar".into()],
+            },
+        );
+        assert_output(
+            &["5", "foo", "bar", "--c", "hi"],
+            LastRepeatingGreedy {
+                a: 5,
+                b: false,
+                c: None,
+                d: vec!["foo".into(), "bar".into(), "--c".into(), "hi".into()],
+            },
+        );
+        assert_output(
+            &["5", "foo", "bar", "--", "hi"],
+            LastRepeatingGreedy {
+                a: 5,
+                b: false,
+                c: None,
+                d: vec!["foo".into(), "bar".into(), "--".into(), "hi".into()],
+            },
+        );
+        assert_help_string::<LastRepeatingGreedy>(
+            r###"Usage: test_arg_0 <a> [--b] [--c <c>] [d...]
+
+Woot
+
+Positional Arguments:
+  a                 fooey
+
+Options:
+  --b               woo
+  --c               stuff
+  --help            display usage information
+"###,
+        );
+    }
+
     #[derive(FromArgs, Debug, PartialEq)]
     /// Woot
     struct LastOptional {
diff --git a/argh/tests/ui/conflicting-tails/positional-and-greedy.rs b/argh/tests/ui/conflicting-tails/positional-and-greedy.rs
new file mode 100644
index 0000000..18c039a
--- /dev/null
+++ b/argh/tests/ui/conflicting-tails/positional-and-greedy.rs
@@ -0,0 +1,13 @@
+/// Command
+#[derive(argh::FromArgs)]
+struct Cmd {
+    #[argh(positional)]
+    /// positional
+    positional: Vec<String>,
+
+    #[argh(positional, greedy)]
+    /// remainder
+    remainder: Vec<String>,
+}
+
+fn main() {}
diff --git a/argh/tests/ui/conflicting-tails/positional-and-greedy.stderr b/argh/tests/ui/conflicting-tails/positional-and-greedy.stderr
new file mode 100644
index 0000000..f6a6399
--- /dev/null
+++ b/argh/tests/ui/conflicting-tails/positional-and-greedy.stderr
@@ -0,0 +1,11 @@
+error: Only the last positional argument may be `Option`, `Vec`, or defaulted.
+ --> tests/ui/conflicting-tails/positional-and-greedy.rs:4:5
+  |
+4 |     #[argh(positional)]
+  |     ^
+
+error: Later positional argument declared here.
+ --> tests/ui/conflicting-tails/positional-and-greedy.rs:8:5
+  |
+8 |     #[argh(positional, greedy)]
+  |     ^