Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
12 changes: 10 additions & 2 deletions PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ manpages, and SDKs — never a runtime dependency of somebody else's program.
`env`, defaults, delimiters, the `double_dash` modes, global flags, flatten,
boxed subcommand variants, headings, `cfg`-gated variants.
- [x] **The post-binding layer** — `required`, `choices`, `env` fallback, defaults,
`var_min`/`var_max`, `conflicts`, `required_if`, `required_unless`, and
`overrides`. All of them need a value's type, so they belong with the derive
`var_min`, `conflicts`, `required_if`, `required_unless`, and `overrides` —
`var_max` moved to the binder, see the decision below. All of them need a value's type, so they belong with the derive
rather than in the parser. `overrides` is the one that happens _during_ the
parse instead of after it: it asks which of two flags came last, which only the
arriving token knows.
Expand All @@ -151,6 +151,14 @@ carrying them rather than being worked around.
shape mise uses on `run`, `exec` and `git`: `[ARGS]…` for the words before the
separator and `[-- ARGS_LAST]…` for the ones after. Both parsers already bound it;
only the derive's validation disagreed. Found by compiling the mise shadow.
- [x] **Is `var_max` a limit or a check?** — usage-argv let a variadic take every word and
judged the count afterwards; usage-lib stopped the variadic at the bound and gave the
rest to the next argument. Two coherent readings, and nothing had recorded that the
two implementations disagreed. Settled as a **limit**, matching usage-lib and clap's
`num_args`, since every spec in the fleet is generated from a clap command and it is
the only reading under which `[a]… [b]` can be filled. `var_max` therefore moved into
the table binding reads; `var_min` stays a check, because no single word tells you a
variadic will end up short. Costs 55 instructions per parse, or 0.1%.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
- [ ] **A mount on the root command** — the spec accepts `mount` only inside a
`cmd` block, so a CLI whose _top-level_ subcommands are discovered by running
something cannot say so. Worth deciding whether that is a gap or a deliberate
Expand Down
71 changes: 66 additions & 5 deletions argv/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,13 @@ pub struct Flag<'a> {
/// parser, since it already reports every occurrence separately. Conflating
/// the two makes a merely repeatable flag greedy enough to eat a positional.
pub variadic: bool,
/// How many values one variadic occurrence may take, after which the next word
/// belongs to whatever comes next.
///
/// Only for [`variadic`](Self::variadic). A merely repeatable flag — the spec's
/// `var=#true` — is bounded on how many times it was *given*, which no single token
/// can decide, so that bound stays with the metadata and is checked after the parse.
pub var_max: ::core::option::Option<u32>,
/// Whether the flag is recognized by every command beneath the one that
/// declares it.
pub global: bool,
Expand All @@ -170,6 +177,7 @@ impl Flag<'_> {
negate: None,
takes_value: false,
variadic: false,
var_max: ::core::option::Option::None,
global: false,
};

Expand All @@ -188,6 +196,14 @@ pub struct Arg<'a> {
pub key: u64,
/// Whether this argument keeps taking values once it has one.
pub var: bool,
/// How many words a variadic may take before the next argument gets the rest.
///
/// A bound belongs here, in the table binding reads, rather than with the metadata:
/// it decides *where* a word lands, not whether what landed is acceptable. clap's
/// `num_args` works the same way, and every spec in the wild is generated from a clap
/// command. `u32` rather than `usize` because a CLI that bounds a variadic above four
/// billion has other problems, and this table is read on the hot path.
pub var_max: ::core::option::Option<u32>,
/// This argument's relationship to the `--` separator.
pub double_dash: DoubleDash,
/// Unused by binding, kept so a table entry can carry its own name for
Expand All @@ -200,6 +216,7 @@ impl Arg<'_> {
pub const REQUIRED: Arg<'static> = Arg {
key: 0,
var: false,
var_max: ::core::option::Option::None,
double_dash: DoubleDash::Optional,
name: "",
};
Expand Down Expand Up @@ -386,8 +403,12 @@ pub struct Parser<'t, 'v> {
bundle_token: &'v [u8],
/// A variadic flag that is still collecting values.
collecting: Option<&'t Flag<'t>>,
/// How many values it has taken, so a bound can stop it.
collected: u32,
/// Which of `cmd.args` is next to fill.
arg_pos: usize,
/// How many words the variadic at `arg_pos` has taken, for the same reason.
arg_taken: u32,
/// Whether any word has been bound to a positional of `cmd`. Once one has,
/// no further word can select a subcommand.
arg_filled: bool,
Expand Down Expand Up @@ -420,7 +441,9 @@ impl<'t, 'v> Parser<'t, 'v> {
bundle: &[],
bundle_token: &[],
collecting: None,
collected: 0,
arg_pos: 0,
arg_taken: 0,
arg_filled: false,
flags_stopped: false,
separator_seen: false,
Expand Down Expand Up @@ -478,6 +501,12 @@ impl<'t, 'v> Parser<'t, 'v> {
match self.argv.get(self.pos) {
Some(next) if !is_flag_like(bytes(next)) && bytes(next) != b"--" => {
self.pos += 1;
self.collected += 1;
// Same rule as a positional: a bounded occurrence takes that many and
// leaves the rest to whatever follows.
if flag.var_max.is_some_and(|max| self.collected >= max) {
self.collecting = None;
}
return Some(Ok(Event::Flag {
flag,
value: Some(bytes(next)),
Expand Down Expand Up @@ -512,7 +541,12 @@ impl<'t, 'v> Parser<'t, 'v> {
.iter()
.position(|a| a.double_dash == DoubleDash::Required)
{
// The count belongs to the argument at `arg_pos`, so jumping past it has
// to leave the count behind: a bounded variadic before the separator would
// otherwise lend its total to the argument after it, which then stops
// early or at once.
self.arg_pos += idx;
self.arg_taken = 0;
}
return self.step();
}
Expand Down Expand Up @@ -558,7 +592,7 @@ impl<'t, 'v> Parser<'t, 'v> {
None
};
if flag.variadic {
self.collecting = Some(flag);
Comment thread
cursor[bot] marked this conversation as resolved.
Resolved
self.start_collecting(flag);
}
return Ok(Event::Flag {
flag,
Expand Down Expand Up @@ -633,7 +667,7 @@ impl<'t, 'v> Parser<'t, 'v> {
rest
};
if flag.variadic {
self.collecting = Some(flag);
self.start_collecting(flag);
}
Ok(Event::Flag {
flag,
Expand Down Expand Up @@ -682,9 +716,16 @@ impl<'t, 'v> Parser<'t, 'v> {
if arg.double_dash == DoubleDash::Automatic {
self.flags_stopped = true;
}
// A variadic keeps taking values, so the cursor stays put.
if !arg.var {
self.arg_pos += 1;
// A variadic keeps taking values, so the cursor stays put — until it reaches its
// bound, at which point the words after it belong to whatever comes next. That is
// what makes `[a]… [b]` expressible at all.
if arg.var {
self.arg_taken += 1;
if arg.var_max.is_some_and(|max| self.arg_taken >= max) {
self.advance_arg();
}
} else {
self.advance_arg();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}
Ok(Event::Arg { arg, value: token })
}
Expand All @@ -697,10 +738,30 @@ impl<'t, 'v> Parser<'t, 'v> {
self.depth += 1;
self.cmd = sub;
self.arg_pos = 0;
self.arg_taken = 0;
self.arg_filled = false;
Ok(())
}

/// Move to the next positional, forgetting what the last one took.
fn advance_arg(&mut self) {
self.arg_pos += 1;
self.arg_taken = 0;
}

/// A variadic flag occurrence begins, counting from zero.
///
/// The value it was given on the same token counts, which is why this starts at one:
/// `--include a b` with `var_max=2` takes `a` and `b`, not three words.
fn start_collecting(&mut self, flag: &'t Flag<'t>) {
self.collected = 1;
self.collecting = if flag.var_max.is_some_and(|max| max <= 1) {
None
} else {
Some(flag)
};
}

fn next_arg(&self) -> Option<&'t Arg<'t>> {
self.cmd.args.get(self.arg_pos).copied()
}
Expand Down
15 changes: 15 additions & 0 deletions conformance/src/argv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,17 @@ fn build(
// single-value argument is repeatable instead: one value per
// occurrence, which the parser gets by not collecting.
variadic: f.arg.as_ref().is_some_and(|a| a.var),
// The bound on one occurrence's values, which is the argument's. A
// repeatable flag's own `var_max` counts occurrences and is checked after
// the parse, so it does not belong in this table.
var_max: f
.arg
.as_ref()
.filter(|a| a.var)
.and_then(|a| a.var_max)
// Saturating rather than truncating: `4294967296 as u32` is zero,
// which would read as "stop at once" rather than "no real limit".
.map(|max| u32::try_from(max).unwrap_or(u32::MAX)),
global: f.global,
}))
})
Expand All @@ -249,6 +260,10 @@ fn build(
key: 0,
name: leak(&a.name),
var: a.var,
var_max: a
.var_max
.filter(|_| a.var)
.map(|max| u32::try_from(max).unwrap_or(u32::MAX)),
double_dash: match a.double_dash {
usage::SpecDoubleDashChoices::Required => DoubleDash::Required,
usage::SpecDoubleDashChoices::Preserve => DoubleDash::Preserve,
Expand Down
37 changes: 37 additions & 0 deletions conformance/tests/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -669,3 +669,40 @@ fn a_double_dash_argument_can_follow_a_variadic() {
assert_eq!(last.double_dash, usage::SpecDoubleDashChoices::Required);
assert!(last.var);
}

/// A bounded variadic, with an argument after it.
///
/// The bound is what makes the second one reachable: `[first]…` takes two words and hands
/// the rest over. An unbounded variadic there would be a compile error, because nothing
/// after it could ever be filled.
#[derive(Cli, Debug)]
#[usage(bin = "bounded")]
struct Bounded {
/// The first two words
#[usage(arg, name = "FIRST", var_max = 2)]
first: Vec<String>,
/// Whatever follows them
#[usage(arg, name = "REST")]
rest: Option<String>,
}

#[test]
fn a_bound_hands_the_rest_to_the_next_argument() {
let a = argv(["x", "y", "z"]);
let b = Bounded::parse_from(&a).expect("should parse");
assert_eq!(b.first, ["x", "y"]);
assert_eq!(b.rest.as_deref(), Some("z"));

// Fewer words than the bound: the variadic simply takes what there is.
let a = argv(["x"]);
let b = Bounded::parse_from(&a).expect("should parse");
assert_eq!(b.first, ["x"]);
assert!(b.rest.is_none());

// And the spec says the bound, so docs and completions agree with the parse.
assert!(
Bounded::to_kdl().contains("var_max=2"),
"{}",
Bounded::to_kdl()
);
}
Loading