Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 'remainder': greedy positional arguments #143

Merged
merged 1 commit into from
Sep 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 43 additions & 9 deletions argh/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(()) })
}
Expand Down
84 changes: 84 additions & 0 deletions argh/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()],
},
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you add a check that this still plays nicely with --? With something like:

        assert_output(
            &["5", "foo", "bar", "--", "hi"],
            LastRepeatingGreedy {
                a: 5,
                b: false,
                c: None,
                d: vec!["foo".into(), "bar".into(), "--".into(), "hi".into()],
            },
        );

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For sure.

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 {
Expand Down
13 changes: 13 additions & 0 deletions argh/tests/ui/conflicting-tails/positional-and-greedy.rs
Original file line number Diff line number Diff line change
@@ -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() {}
11 changes: 11 additions & 0 deletions argh/tests/ui/conflicting-tails/positional-and-greedy.stderr
Original file line number Diff line number Diff line change
@@ -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)]
| ^
18 changes: 15 additions & 3 deletions argh_derive/src/help.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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() {
Expand Down Expand Up @@ -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(']');
}
Expand Down
12 changes: 11 additions & 1 deletion argh_derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
17 changes: 15 additions & 2 deletions argh_derive/src/parse_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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`",
),
);
}
Expand All @@ -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());
}
Expand Down