test(parse): add normative argv grammar and conformance corpus - #797
Merged
Conversation
How argv binds against a spec was defined only by lib/src/parse.rs, so a second implementation had no way to know whether it agreed and no way to prove it. This writes the grammar down and makes it executable. docs/spec/argv.md is the normative description: token classification, long and short flag forms, positional filling, subcommand routing, flag scope, the `--` separator, and the error classes. corpus/ is the same thing as 83 JSON vectors, deliberately language-neutral so a Go, JS, or Python implementation can run them without reimplementing a test format. Each vector records whether usage-lib agrees, measured rather than assumed, and the test suite fails if a label is wrong in either direction — so a divergence that gets fixed reports itself instead of rotting into folklore. 15 of 83 currently diverge, from four causes documented in the doc: unrecognized flags fall through to positionals, a flag missing its value is silently dropped, `=` is kept in attached short values, and a repeated `--` is eaten. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe workspace adds an argv conformance crate. It defines corpus schemas, executes vectors through ChangesARGV conformance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Oracle
participant CorpusLoader
participant ReferenceAdapter
participant UsageLib
Oracle->>CorpusLoader: load JSON vectors
Oracle->>ReferenceAdapter: run each vector
ReferenceAdapter->>UsageLib: parse spec, argv, and environment
UsageLib-->>ReferenceAdapter: parsed values or diagnostic
ReferenceAdapter-->>Oracle: observed result
Oracle-->>Oracle: compare expectation and reference label
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Instruction counts
No instruction-count regression above 1%. Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run. Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.
|
Contributor
Greptile SummaryThe PR defines a normative argv grammar and adds a language-neutral conformance corpus backed by a Rust reference harness.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (2): Last reviewed commit: "test(parse): close corpus gaps and harde..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@conformance/src/bin/oracle.rs`:
- Around line 75-78: Update the diagnostic condition in the vector-reporting
logic around agrees and declared so expected and observed values are printed for
every mislabeled vector, including the agrees=true and declared=false case that
emits MISM. Preserve the existing output formatting and only broaden the
condition governing those two println! calls.
In `@conformance/src/lib.rs`:
- Around line 19-26: Add #[serde(deny_unknown_fields)] to the VectorFile,
Vector, and Parsed structs so deserialization rejects unrecognized JSON fields
instead of silently applying defaults. Keep the existing serde derives and field
definitions unchanged.
- Around line 143-147: Update the directory iteration that builds paths to
propagate individual read_dir entry failures instead of silently discarding them
via filter_map(Result::ok). Convert each entry error into the existing
descriptive error format, while preserving JSON extension filtering and the
surrounding error-return behavior.
In `@conformance/src/reference.rs`:
- Around line 97-137: Update the unexpected-argument branch in classify to match
only msg.contains("unexpected word:"). Remove the broader "Unexpected argument"
and "unexpected" checks while preserving the existing UnexpectedArg
classification for usage-lib’s unexpected word diagnostics.
In `@corpus/01-long-flags.json`:
- Line 4: Reconcile the corpus total for the eight corpus files: either add the
six intended vectors so the collection contains 83 vectors, or update the stated
objective/count to accurately reflect the existing 77 vectors before publishing.
In `@corpus/07-env-and-defaults.json`:
- Around line 60-65: Rename the test vector ID from
env-does-not-satisfy-required-arg-position to
env-satisfies-required-arg-position, leaving its specification, environment,
arguments, and expected result unchanged.
In `@docs/spec/argv.md`:
- Around line 42-52: Update the token-reading algorithm and related
subcommand-routing rules to explicitly define when descent into a matching
subcommand remains available. State that routing is allowed only before any
positional binding consumes a word, and that positional bindings—especially
variadic or otherwise consuming bindings—disable further subcommand descent,
preserving the intended behavior for inputs such as “ex other install.”
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 6df69cd6-015f-4e49-8d91-529e6839c9a3
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
Cargo.tomlconformance/Cargo.tomlconformance/src/bin/oracle.rsconformance/src/lib.rsconformance/src/reference.rsconformance/tests/reference.rscorpus/01-long-flags.jsoncorpus/02-short-flags.jsoncorpus/03-positionals.jsoncorpus/04-subcommands.jsoncorpus/05-globals.jsoncorpus/06-double-dash.jsoncorpus/07-env-and-defaults.jsoncorpus/08-choices-and-required.jsoncorpus/README.mddocs/.vitepress/config.mtsdocs/spec/argv.md
Comment on lines
+97
to
+137
| /// Map a usage-lib error onto one of the grammar's error codes. | ||
| /// | ||
| /// Matching on the rendered message rather than on `UsageErr` variants: parsing | ||
| /// returns a `miette::Error`, so the concrete type is erased by the time it gets | ||
| /// here, and `InvalidFlag` would need its `reason` inspected anyway. The strings | ||
| /// below are load-bearing and will need revisiting whenever usage-lib rewords a | ||
| /// diagnostic — which is why an unrecognized message becomes | ||
| /// [`Observed::Unclassified`] and fails loudly instead of quietly matching | ||
| /// whatever the vector expected. | ||
| fn classify(msg: &str) -> Observed { | ||
| let code = if msg.contains("Missing required flag") { | ||
| ErrorCode::MissingRequiredFlag | ||
| } else if msg.contains("Missing required arg") { | ||
| ErrorCode::MissingRequiredArg | ||
| } else if msg.contains("can only be set after a `--` separator") { | ||
| ErrorCode::ArgRequiresDoubleDash | ||
| } else if msg.contains("requires at least") { | ||
| // Both the arg and flag forms of "too few" say this; they differ only in | ||
| // whether the subject is rendered as <name> or --name, and the corpus | ||
| // does not distinguish subject. | ||
| ErrorCode::VarTooFew | ||
| } else if msg.contains("accepts at most") { | ||
| ErrorCode::VarTooMany | ||
| } else if msg.contains("Invalid flag") { | ||
| // usage-lib funnels several distinct situations through InvalidFlag, so | ||
| // the reason has to be read to tell them apart. | ||
| if msg.contains("requires an argument") || msg.contains("missing value") { | ||
| ErrorCode::MissingFlagValue | ||
| } else if msg.contains("Invalid choice") || msg.contains("expected one of") { | ||
| ErrorCode::InvalidChoice | ||
| } else { | ||
| ErrorCode::UnknownFlag | ||
| } | ||
| } else if msg.contains("Invalid choice") || msg.contains("expected one of") { | ||
| ErrorCode::InvalidChoice | ||
| } else if msg.contains("Unexpected argument") || msg.contains("unexpected") { | ||
| ErrorCode::UnexpectedArg | ||
| } else { | ||
| return Observed::Unclassified(msg.to_string()); | ||
| }; | ||
| Observed::Failed(code) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify the canonical usage-lib diagnostic before narrowing this matcher.
rg -n -C 4 'Unexpected argument|unexpected|UsageErr' libRepository: jdx/usage
Length of output: 45794
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- conformance/src/reference.rs ---'
cat -n conformance/src/reference.rs | sed -n '80,150p'
printf '%s\n' '--- UsageErr display definitions ---'
cat -n lib/src/error.rs | sed -n '1,100p'
printf '%s\n' '--- parser diagnostic construction ---'
cat -n lib/src/parse.rs | sed -n '880,960p'
cat -n lib/src/parse.rs | sed -n '1220,1310p'
printf '%s\n' '--- conformance references and dependency metadata ---'
rg -n -C 3 'Unexpected argument|UnexpectedArg|unclassified|classify\(|usage-lib|path =|git =|version' conformance Cargo.toml Cargo.lock 2>/dev/null || trueRepository: jdx/usage
Length of output: 50366
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- parse return path ---'
rg -n -C 5 'pub fn parse|fn parse|bail!\("unexpected word|miette::Result|anyhow' lib/src/parse.rs lib/src/lib.rs
printf '%s\n' '--- exact diagnostic strings ---'
rg -n -C 2 'Unexpected argument|unexpected word|unexpected' conformance lib/src --glob '!**/bash-completion/**'
printf '%s\n' '--- corpus vectors involving unexpected arguments ---'
rg -n -C 8 -i 'unexpected[_ ]arg|unexpected argument|too many|extra argument' conformance corpus . --glob '*.json' --glob '*.yaml' --glob '*.yml' 2>/dev/null | head -n 300
printf '%s\n' '--- source-level matcher probe ---'
python3 - <<'PY'
from pathlib import Path
import re
reference = Path("conformance/src/reference.rs").read_text()
library = Path("lib/src/parse.rs").read_text()
m = re.search(r'bail!\("unexpected word: \{w\}"\)', library)
print("usage-lib emits canonical unexpected diagnostic:", bool(m))
print("classifier matches `Unexpected argument`:", "msg.contains(\"Unexpected argument\")" in reference)
print("classifier matches generic `unexpected`:", "msg.contains(\"unexpected\")" in reference)
for msg in ["unexpected word: value", "Unexpected argument: value", "unexpected number of arguments"]:
broad = "unexpected" in msg
title_only = "Unexpected argument" in msg
print(f"{msg!r}: broad={broad}, title_only={title_only}")
PYRepository: jdx/usage
Length of output: 30275
Narrow the unexpected-argument matcher to unexpected word:.
usage-lib emits unexpected-argument errors as unexpected word: {w}, not Unexpected argument. Match msg.contains("unexpected word:") to avoid classifying unrelated diagnostics as UnexpectedArg.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@conformance/src/reference.rs` around lines 97 - 137, Update the
unexpected-argument branch in classify to match only msg.contains("unexpected
word:"). Remove the broader "Unexpected argument" and "unexpected" checks while
preserving the existing UnexpectedArg classification for usage-lib’s unexpected
word diagnostics.
| { | ||
| "section": "long-flags", | ||
| "about": "How `--name` tokens bind. Long names match exactly — there is no abbreviation inference — and a value may be attached with `=` or given as the following word.", | ||
| "vectors": [ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reconcile the corpus count.
The eight corpus files in this PR contain 77 vectors, but the PR objective states 83 vectors. Add the six intended vectors or correct the stated count before publishing the corpus.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@corpus/01-long-flags.json` at line 4, Reconcile the corpus total for the
eight corpus files: either add the six intended vectors so the collection
contains 83 vectors, or update the stated objective/count to accurately reflect
the existing 77 vectors before publishing.
Comment on lines
+60
to
+65
| "id": "env-does-not-satisfy-required-arg-position", | ||
| "doc": "An env-backed required positional is satisfied by the environment, so the command line need not supply it.", | ||
| "spec": "name \"ex\"\nbin \"ex\"\narg \"<file>\" env=\"EX_FILE\"\n", | ||
| "argv": [], | ||
| "env": { "EX_FILE": "x.txt" }, | ||
| "expect": { "ok": { "args": { "file": "x.txt" } } } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rename the vector to state its behavior.
env-does-not-satisfy-required-arg-position says that the environment does not satisfy the required argument. The documentation and expectation show that it does. Rename it to env-satisfies-required-arg-position.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@corpus/07-env-and-defaults.json` around lines 60 - 65, Rename the test vector
ID from env-does-not-satisfy-required-arg-position to
env-satisfies-required-arg-position, leaving its specification, environment,
arguments, and expected result unchanged.
Comment on lines
+42
to
+52
| At each token, in order: | ||
|
|
||
| 1. If flag interpretation has stopped (a `--` was consumed), the token is a | ||
| value. | ||
| 2. If the token is exactly `--`, flag interpretation stops. The token is consumed | ||
| and is not itself a value. | ||
| 3. If the token is flag-like, it is matched as a flag ([long](#long-flags) or | ||
| [short](#short-flags)). No match is an error. | ||
| 4. Otherwise the token is a word: it selects a [subcommand](#subcommands) if one | ||
| matches, and is otherwise offered to the command's | ||
| [positional arguments](#positional-arguments). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Define the subcommand routing condition in the algorithm.
Step 4 says that every matching word selects a subcommand. Lines 131-133 later prohibit routing after a positional consumes a word. The grammar does not define this state in the token-reading algorithm.
State when descent remains available, and state which positional bindings disable it. This removes ambiguity for inputs such as ex other install.
Also applies to: 121-133
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/spec/argv.md` around lines 42 - 52, Update the token-reading algorithm
and related subcommand-routing rules to explicitly define when descent into a
matching subcommand remains available. State that routing is allowed only before
any positional binding consumes a word, and that positional bindings—especially
variadic or otherwise consuming bindings—disable further subcommand descent,
preserving the intended behavior for inputs such as “ex other install.”
Review feedback on the corpus contract and its coverage. The loader accepted unknown fields, so a misspelled `reference` would default to `agrees` and a misspelled `flags` would become an empty expectation — a malformed vector could load and pass while testing nothing. All three corpus types now deny unknown fields. A failed directory entry was also being skipped rather than reported, which would let CI validate a partial corpus and still pass. Coverage: the grammar described `double_dash="automatic"` and variadic bounds with nothing exercising the first and only positional cases for the second, so an implementation could ignore both and still pass. Adds a vector for automatic mode — which usage-lib does not enforce, as the arg reference already says — and two for flag-level var_min and var_max, which it does. 86 vectors, 16 recorded divergences. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jdx
added a commit
that referenced
this pull request
Aug 11, 2026
Stacked on #797 — review that one first; this PR's diff against `main` will include it until it merges. ## What this is `argv/` — a new crate, `usage-argv`, implementing the binding half of [the grammar from #797](https://usage.jdx.dev/spec/argv). No command tree, no allocation, one pass over `argv`. Not published (name is reserved on crates.io at 0.0.0), and deliberately outside the shared-version release cycle, the way `clap_usage` already is. ## Two design decisions worth a look **Events, not a map.** Parsing yields `Event::{Command, Flag, Arg}` rather than returning a structure. A map would have to allocate and then be read back out; an event can be assigned straight into a struct field by generated code. Same reasoning as serde deserializing into your type instead of into a `Value`. **Values are `&[u8]`.** Borrowed from `argv`, converted by the caller with `as_str`. Slicing an `OsStr` into `&str` pieces needs either an allocation or `unsafe`, and this crate forbids `unsafe`. The upside is that a non-UTF-8 command line still *parses* — flags match, subcommands route — and only the values actually looked at can fail to convert, which is where that failure belongs. Tables are borrowed slices (`&'a [&'a Flag<'a>]`) so a derive can emit the whole tree as `static` data. Tables and `argv` carry separate lifetimes; a single lifetime compiled but forced `argv` to be `'static`, which a doctest caught. ## Scope Binding only. `required`, `choices`, `env` fallback, defaults, `var_min`/`var_max`, and `overrides` all happen after the last token is read and need to know a value's type, so they belong to the layer that owns the target struct. Keeping them out is what keeps this loop small. ## How it's verified **The corpus.** `conformance/` gained a second runner, so the same vectors now exercise both parsers. usage-argv answers **61 of 83**; the remaining 22 are post-binding and report *why* they're exempt, with the count asserted so the exempt set can't quietly grow. A separate test asserts that **all 15 vectors where usage-lib diverges from the grammar are ones this parser gets right** — that was the point of writing the grammar down. Since nothing emits tables yet, the harness builds them from a `Spec` and leaks them. A test process building a few small tables is the one place where leaking is the simplest correct answer; generated code has no such problem. **Zero allocation.** `argv/tests/no_alloc.rs` arms a counting global allocator around parses of ten realistic command lines and four failing ones, and asserts zero. It also asserts the counter observes a deliberate allocation, so the test can't pass vacuously. One wrinkle worth knowing if you write similar tests: the file holds exactly one `#[test]`, because the counter is global and a sibling test building a `Vec` on another thread showed up as four phantom allocations here. ## One grammar change Implementing this surfaced a rule the doc didn't state: a flag with a variadic argument (`--include <pattern>...`) consumes following tokens until one is flag-like. I've added it to `docs/spec/argv.md`, including the note that it is greedy and will eat positionals — inherent to the feature, ended explicitly with `--`. ## Next A derive that emits these tables, and then a benchmark against a clap-shaped equivalent at mise's scale, which is the gate for whether this is worth continuing. *AI-assisted — Tool: Claude Code; model: anthropic/claude-fable-5; version: unavailable.* <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > New public parser surface and grammar-aligned behavior that differs from usage-lib on recorded divergences; release script changes affect publishing, but no changes to existing usage-lib parse paths in this diff. > > **Overview** > Introduces **`usage-argv`**, a dependency-free crate that implements the argv **binding** half of the usage grammar in one pass with no heap allocation. Parsing is driven by static `Command` / `Flag` / `Arg` tables (intended for future derive output) and streams **`Event`** values with **`&[u8]`** payloads instead of building a map. > > **Conformance** gains a second runner (`conformance/src/argv.rs` + `conformance/tests/argv.rs`) that builds leaked tables from corpus specs and asserts **62** in-scope binding vectors pass, including cases where usage-lib diverges; post-binding features (`required`, `choices`, `env`, defaults, etc.) are explicitly **out of scope** with a fixed count. **`argv/tests/no_alloc.rs`** enforces zero allocations during parse via a counting global allocator. > > **`docs/spec/argv.md`** documents greedy variadic flag value consumption and notes dual-parser corpus testing. **`tasks/release-plz`** publishes `usage-argv` with the shared version and includes `argv/**` in changelog path filters. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 180aec7. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added a zero-allocation command-line argument parser supporting flags, arguments, subcommands, aliases, negation, short flag bundles, attached values, variadic values, and `--` handling. - Added structured parsing events and detailed errors for invalid command-line input. - **Documentation** - Documented variadic flag behavior and updated conformance coverage details. - **Tests** - Added comprehensive parser, allocation, error-handling, and conformance tests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
jdx
added a commit
that referenced
this pull request
Aug 11, 2026
`PLAN.md` — the plan for this work written down end to end, including the parts that do not exist yet. Three PRs in (#797, #798, #799), the plan lived in PR descriptions and in my head. That is fine for one PR and not for a dozen, especially for the config layer, where the shape is worth arguing about *before* it gets built. Checkboxes rather than prose, so the file doubles as status: **an unchecked box means the thing does not exist.** Ticking them as things land keeps it honest, and makes it obvious when a branch of the plan has stalled. ## What it covers - **Why** — mise's measured numbers, and the fact that mise already hand-maintains two argv scanners to avoid building its clap tree. That workaround existing is the argument for the project. - **How it is arranged** — the four rules that hold it together: code authors and the spec defines; usage-lib is the reference implementation; the hot path stays small; end users never need a second binary. - **Milestones** — what is done, the derive work next, the table stakes after it (help, self-contained completions, docs, diagnostics). - **The gate** — the perf targets, measured with `tak` against a shadow CLI generated from mise's own committed spec. Explicitly: if the targets miss by a wide margin, write that down and stop. Nothing touches mise before this. - **Known usage-lib divergences** — as a to-do list, since each is a small change to `lib/src/parse.rs` and the corpus already knows how to verify a fix. - **Config** — the v2 design, from reading all four CLIs. ## The config section is the part worth reviewing mise, hk, pitchfork, and fnox have each independently built the same settings model — a TOML registry, `build.rs` codegen, a typed `Settings` plus a meta map, project-over-global-over-defaults layering — and agree on ~80% of the vocabulary. The differences are mostly *drift* rather than intent: - Every one hand-writes the CLI-to-settings binding, and every one has a hole in it: hk declares `sources.cli` entries nothing reads, pitchfork's `--help` documents a CLI layer it does not have (copied by hand into its committed spec), and fnox resolves `age_key_file` through a hardcoded five-way chain because its settings and config files are separate systems. - Only hk can say where a value came from, and it needed a second parallel merge to do it. - Docs/schema generation is three separate reimplementations, and fnox has none. The proposal is to declare props in code, lower them into the spec's `config { prop ... }` block — which **exists today and no CLI emits or consumes** — and generate the CLI binding instead of hand-writing it. That block needs extending first (`deprecated`, `enum`, `optional`, `aliases`, `merge`, scope, per-source lists), which is spec-first per the canonicality rule. Three open questions are listed rather than decided, including whether config belongs in this repo at all. *AI-assisted — Tool: Claude Code; model: anthropic/claude-fable-5; version: unavailable.* <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only addition with no runtime, build, or API changes. > > **Overview** > Introduces **`PLAN.md`** as the canonical, in-repo plan for the compiled argv parser work and the later shared config layer—replacing plan text that lived only in PR descriptions. > > The doc uses **unchecked checkboxes as status** (unchecked = not built yet) and covers motivation (mise/clap cost), architecture (spec → usage-derive / usage-argv / usage-lib), milestones (done vs derive vs gate vs adoption), perf gate targets, corpus gaps, known **usage-lib** divergences as a fix list, and a **config** design sketch (unify mise/hk/pitchfork/fnox settings) with open questions—not implementation. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 96b60f9. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
How
argvbinds against a spec is currently defined only bylib/src/parse.rs. That is fine while usage-lib is the only parser, and a problem as soon as it is not: a second implementation — a Rust parser, a Go port, a completion script doing its own matching — has no way to know whether it agrees, and no way to prove it.This writes the grammar down and makes it executable, so "compatible with usage" becomes a thing you can run rather than a thing you assert.
What's here
docs/spec/argv.md— the normative grammar, published at/spec/argv. Token classification, the single left-to-right pass, long and short flag forms (attached,=-attached, detached), positional filling, subcommand routing and flag scope, the--separator and thedouble_dashmodes, where values come from when argv doesn't supply them, and the error classes.corpus/— 83 JSON vectors covering the same ground. Deliberately language-neutral: a Go, JS, or Python implementation can run these without reimplementing a test format.corpus/README.mddocuments the format for exactly that audience.conformance/— the harness (publish = false).cargo test -p usage-conformancechecks the corpus is well formed and that every vector's reference label is accurate.cargo run -p usage-conformance --bin oraclereports what usage-lib actually does with each vector, which is how the labels got filled in.The interesting part: 15 of 83 vectors diverge
Every vector records whether usage-lib agrees, as a measurement, and the suite fails if a label is wrong in either direction — so a divergence that gets fixed reports itself as "delete this label" rather than rotting into folklore.
Four causes, all in
lib/src/parse.rs:ex --watbinds--watto a free argument, or reportsunexpected_argif there isn't one. This accounts for most of the divergences, including the cases where the grammar and usage-lib agree a flag is out of scope but disagree on which error to report.ex --jobsparses successfully with nothing bound, so a forgotten value looks like a working command.ex --jobs --forcelikewise bindsforceand leavesjobsunset.=is kept in attached short values.-j=8binds=8.--is eaten, so a forwarded command line containing its own separator is altered in transit.Plus two smaller gaps:
--jobs=binds nothing rather than the empty string, and a flag with a variadic argument (--include <pattern>..., which/spec/reference/flagdocuments) rejects its second value.I've written the grammar as the intent and left usage-lib's behavior labeled, rather than describing current behavior as correct — several of these look like bugs worth fixing, and the corpus is how a fix would be verified. Happy to flip any of them the other way if you'd rather the grammar match the implementation; that's a one-line change per vector.
Not covered yet
restart_token(mise's:::) needs a multi-invocation expectation shape,mountneeds process stubbing, andparse_partialis a different contract that deserves its own corpus. All three are noted in the doc.Context
Groundwork for a compiled Rust parser (
usage-rs), but it stands on its own: it makes usage-lib a verified reference implementation rather than an assumed one, and it is the artifact any other-language port would be built against.AI-assisted — Tool: Claude Code; model: anthropic/claude-fable-5; version: unavailable.
Note
Low Risk
New test/documentation and a non-published conformance crate; production parsing in
libis unchanged, with divergences recorded rather than fixed here.Overview
Introduces a normative argv grammar (
docs/spec/argv.md, linked from the spec nav) so parsing behavior is defined outsideusage-libalone, and backs it with an executable conformance corpus (corpus/*.json) of spec/argv/expected-result vectors across flags, positionals, subcommands, globals,--, env/defaults, and post-bind constraints.Adds workspace crate
usage-conformance: loads corpus JSON with strict typing, runs each vector through usage-lib via a reference adapter (including error-class mapping from diagnostics),cargo test -p usage-conformancefor corpus integrity and accurate per-vectorreferencelabels (agreesvsdiverges), and anoraclebinary to report observed vs expected when authoring vectors. Vectors that intentionally disagree with current usage-lib behavior are explicitly labeled so CI catches fixed divergences; the doc summarizes known gaps (e.g. unknown flags falling through to positionals) without changing the parser in this PR.Reviewed by Cursor Bugbot for commit 38b4b75. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
--, environment variables, defaults, choices, and required values.Documentation
Tests