feat(spec): make unknown flags configurable, and keep them as values - #810
Conversation
📝 WalkthroughWalkthroughThe PR adds per-command ChangesUnknown flag handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant DeriveCodegen
participant CommandTable
participant Parser
participant PositionalArguments
participant UsageError
CLI->>DeriveCodegen: configure unknown_flags
DeriveCodegen->>CommandTable: emit effective policy
CommandTable->>Parser: parse flag-like token
Parser->>PositionalArguments: forward token in Value mode
Parser->>UsageError: reject token in Error mode
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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThe PR makes unknown-flag handling configurable and consistently propagates the effective mode through the spec model, generated parser tables, derive output, and serialization.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (6): Last reviewed commit: "fix(argv): write a subcommand's unknown_..." | 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: 3
🧹 Nitpick comments (1)
docs/spec/argv.md (1)
158-161: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify that the current command participates in precedence.
The phrase “nearest enclosing command” can be read as excluding the command that is currently parsing the token. State that the current command or its nearest ancestor wins before the spec-level setting and the
valuedefault.Suggested wording
- The nearest enclosing command that states a preference wins, then the spec, then `value`. + The current command or nearest ancestor that states a preference wins, then the spec, then `value`.🤖 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 158 - 161, Update the precedence description in the argv documentation to explicitly include the current command alongside its nearest ancestor: the current command or nearest enclosing ancestor preference wins first, followed by the spec-level setting, then the value default. Preserve the existing explanation of inherited option forwarding.
🤖 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 `@docs/spec/reference/cmd.md`:
- Around line 97-100: Update the argv grammar link in the surrounding command
documentation to use the source-relative target ../argv.md#unrecognized-flags,
or the documentation builder’s validated equivalent, so markdownlint MD051
passes while preserving the intended fragment.
In `@lib/src/parse.rs`:
- Line 883: Update the short-bundle parsing flow around
reject_unknown_flag_if_asked to pre-scan the entire bundle before applying any
flags, stopping at an unknown letter or value-taking flag. In Error mode, return
InvalidFlag before mutating state; in Value mode, route the unchanged original
token to positional parsing without setting earlier flags. Add regression
coverage for -a, -az, and a positional argument in both strict and lenient
modes.
In `@lib/src/spec/mod.rs`:
- Around line 82-84: Update Spec::merge to merge the spec-level unknown_flags
option using merge_opt!(unknown_flags), preserving an included specification’s
policy. Add a regression test covering an include that sets unknown_flags to
"error" and verifies the merged spec retains that setting.
---
Nitpick comments:
In `@docs/spec/argv.md`:
- Around line 158-161: Update the precedence description in the argv
documentation to explicitly include the current command alongside its nearest
ancestor: the current command or nearest enclosing ancestor preference wins
first, followed by the spec-level setting, then the value default. Preserve the
existing explanation of inherited option forwarding.
🪄 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: 550a71cb-c39e-4234-825e-d293ac2f3b7f
📒 Files selected for processing (20)
argv/src/lib.rsargv/src/spec.rsconformance/src/argv.rsconformance/tests/argv.rsconformance/tests/derive.rscorpus/01-long-flags.jsoncorpus/02-short-flags.jsoncorpus/04-subcommands.jsoncorpus/05-globals.jsonderive/src/codegen.rsderive/src/model.rsdocs/cli/reference/commands.jsondocs/spec/argv.mddocs/spec/reference/cmd.mdlib/src/docs/models.rslib/src/lib.rslib/src/parse.rslib/src/spec/cmd.rslib/src/spec/mod.rslib/src/spec/unknown_flags.rs
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.
|
4088564 to
8ed7529
Compare
|
Both fixed, and the first one is the best catch on this PR. The two parsers disagreed about One rule now, spelled out identically on both sides: digits, at most one
The grammar page states the rule rather than leaving "a negative number" to interpretation.
One note on CI: the remaining AI-assisted — Tool: Claude Code; model: anthropic/claude-fable-5; version: unavailable. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 10f765f. Configure here.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@argv/src/spec.rs`:
- Around line 353-379: Remove the duplicate unknown_flags serialization branch
in the spec-writing function, keeping a single comparison against
inherited_unknown_flags and one write! call using meta.cmd.unknown_flags. Update
the relevant test assertion to verify that an overriding command emits
unknown_flags exactly once.
In `@conformance/tests/argv.rs`:
- Line 16: Replace the hardcoded IN_SCOPE constant in argv.rs with the
exempt-vector snapshot from PR `#813`, so the expected count tracks the intended
exempt corpus rather than ordinary vector additions. Preserve the existing type
and usage of IN_SCOPE.
In `@docs/spec/argv.md`:
- Around line 171-172: Update the wording near “add the number case back
afterwards” to use the American English form “afterward,” preserving the rest of
the sentence.
- Around line 34-37: Update the numeric-argument description around the
“optional exponent” wording to define the grammar precisely: specify the
exponent marker, optional sign, and required exponent digits, and list the
accepted decimal forms, including the behavior for values such as -1e-3, -1e,
and -1. Keep the documented grammar aligned with the parser’s strict
classification rules.
🪄 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: 88e0c6ed-b060-454c-8072-8f47bc381581
📒 Files selected for processing (11)
argv/src/lib.rsargv/src/spec.rsconformance/tests/argv.rsconformance/tests/derive.rscorpus/01-long-flags.jsoncorpus/02-short-flags.jsondocs/spec/argv.mddocs/spec/reference/cmd.mdlib/src/parse.rslib/src/spec/cmd.rslib/src/spec/mod.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- corpus/01-long-flags.json
- conformance/tests/derive.rs
- lib/src/spec/cmd.rs
- lib/src/spec/mod.rs
- argv/src/lib.rs
| // Written only where it changes, since the spec inherits it. The tables hold the | ||
| // effective value per command, so repeating the enclosing command's answer would | ||
| // say nothing — but a command that differs has to say so, or the setting is lost | ||
| // on the way out. | ||
| if meta.cmd.unknown_flags != inherited_unknown_flags { | ||
| write!( | ||
| out, | ||
| " unknown_flags={}", | ||
| quoted(match meta.cmd.unknown_flags { | ||
| UnknownFlags::Value => "value", | ||
| UnknownFlags::Error => "error", | ||
| }) | ||
| )?; | ||
| } | ||
| // Written only where it changes, since the spec inherits it: the tables hold the | ||
| // effective value per command, so the same value as the enclosing one says nothing. | ||
| let unknown_flags = meta.cmd.unknown_flags; | ||
| if unknown_flags != inherited_unknown_flags { | ||
| write!( | ||
| out, | ||
| " unknown_flags={}", | ||
| quoted(match unknown_flags { | ||
| UnknownFlags::Value => "value", | ||
| UnknownFlags::Error => "error", | ||
| }) | ||
| )?; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Remove the duplicate unknown_flags property write.
Lines 357-379 append the same property twice when a command overrides its inherited policy. This emits unknown_flags="value" unknown_flags="value" for exec. Keep one branch. Update the test to assert one occurrence.
Proposed fix
- // Written only where it changes, since the spec inherits it: the tables hold the
- // effective value per command, so the same value as the enclosing one says nothing.
- let unknown_flags = meta.cmd.unknown_flags;
- if unknown_flags != inherited_unknown_flags {
- write!(
- out,
- " unknown_flags={}",
- quoted(match unknown_flags {
- UnknownFlags::Value => "value",
- UnknownFlags::Error => "error",
- })
- )?;
- }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Written only where it changes, since the spec inherits it. The tables hold the | |
| // effective value per command, so repeating the enclosing command's answer would | |
| // say nothing — but a command that differs has to say so, or the setting is lost | |
| // on the way out. | |
| if meta.cmd.unknown_flags != inherited_unknown_flags { | |
| write!( | |
| out, | |
| " unknown_flags={}", | |
| quoted(match meta.cmd.unknown_flags { | |
| UnknownFlags::Value => "value", | |
| UnknownFlags::Error => "error", | |
| }) | |
| )?; | |
| } | |
| // Written only where it changes, since the spec inherits it: the tables hold the | |
| // effective value per command, so the same value as the enclosing one says nothing. | |
| let unknown_flags = meta.cmd.unknown_flags; | |
| if unknown_flags != inherited_unknown_flags { | |
| write!( | |
| out, | |
| " unknown_flags={}", | |
| quoted(match unknown_flags { | |
| UnknownFlags::Value => "value", | |
| UnknownFlags::Error => "error", | |
| }) | |
| )?; | |
| } | |
| // Written only where it changes, since the spec inherits it. The tables hold the | |
| // effective value per command, so repeating the enclosing command's answer would | |
| // say nothing — but a command that differs has to say so, or the setting is lost | |
| // on the way out. | |
| if meta.cmd.unknown_flags != inherited_unknown_flags { | |
| write!( | |
| out, | |
| " unknown_flags={}", | |
| quoted(match meta.cmd.unknown_flags { | |
| UnknownFlags::Value => "value", | |
| UnknownFlags::Error => "error", | |
| }) | |
| )?; | |
| } |
🤖 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 `@argv/src/spec.rs` around lines 353 - 379, Remove the duplicate unknown_flags
serialization branch in the spec-writing function, keeping a single comparison
against inherited_unknown_flags and one write! call using
meta.cmd.unknown_flags. Update the relevant test assertion to verify that an
overriding command emits unknown_flags exactly once.
|
|
||
| /// Vectors that exercise binding, which is what usage-argv implements. | ||
| const IN_SCOPE: usize = 64; | ||
| const IN_SCOPE: usize = 75; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target file ---'
cat -n conformance/tests/argv.rs | sed -n '1,140p'
printf '%s\n' '--- repository references to IN_SCOPE and PR `#813` ---'
rg -n -C 3 'IN_SCOPE|`#813`|813' . --glob '!target' --glob '!node_modules' || true
printf '%s\n' '--- concise repository status and recent commits ---'
git status --short
git log --oneline -12Repository: jdx/usage
Length of output: 11554
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- conformance files ---'
git ls-files conformance
printf '%s\n' '--- count-update commit ---'
git show --stat --oneline 8ed75291
git show --format=fuller --no-ext-diff 8ed75291 -- conformance/tests/argv.rs conformance 2>/dev/null | sed -n '1,220p'
printf '%s\n' '--- corpus loading and argv outcome logic ---'
rg -n -C 5 'fn corpus_dir|pub fn corpus_dir|enum Outcome|OutOfScope|fn run|pub fn run|load\(' conformance --glob '*.rs'
printf '%s\n' '--- corpus manifests and vector files ---'
find conformance -maxdepth 3 -type f | sort | sed -n '1,220p'Repository: jdx/usage
Length of output: 10705
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- repository refs and remotes ---'
git remote -v || true
git branch --all --contains HEAD
git log --all --oneline --decorate -20
printf '%s\n' '--- workspace and corpus configuration ---'
cat -n Cargo.toml | sed -n '1,180p'
cat -n conformance/Cargo.toml | sed -n '1,180p'
find . -maxdepth 2 -type d -print | sort
find . -maxdepth 2 -type f \( -name '*.json' -o -name '.gitmodules' \) -print | sort
printf '%s\n' '--- pull request 813 metadata ---'
curl -fsSL https://api.github.com/repos/jdx/usage/pulls/813 |
jq '{number,title,state,merged_at,base:.base.ref,head:.head.ref,head_sha:.head.sha,merge_commit_sha,body}'Repository: jdx/usage
Length of output: 7854
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- PR `#813` changed files ---'
curl -fsSL https://api.github.com/repos/jdx/usage/pulls/813/files?per_page=100 |
jq -r '.[] | [.filename, .status, (.additions|tostring), (.deletions|tostring)] | `@tsv`'
printf '%s\n' '--- PR `#813` patch for the test ---'
curl -fsSL https://api.github.com/repos/jdx/usage/pulls/813/files?per_page=100 |
jq -r '.[] | select(.filename == "conformance/tests/argv.rs") | .patch'
printf '%s\n' '--- changes from main to the reviewed commit ---'
git diff --stat origin/main...HEAD
git diff -- conformance/tests/argv.rs corpus | sed -n '1,260p'
printf '%s\n' '--- corpus vector counts by file and current hardcoded count history ---'
python3 - <<'PY'
import json
from pathlib import Path
total = 0
for path in sorted(Path("corpus").glob("*.json")):
data = json.loads(path.read_text())
vectors = data.get("vectors", [])
print(f"{path}: {len(vectors)}")
total += len(vectors)
print(f"total vectors: {total}")
PY
git log --oneline --all -- corpus conformance/tests/argv.rs | sed -n '1,30p'Repository: jdx/usage
Length of output: 8029
Replace the hardcoded IN_SCOPE count with the exempt-vector snapshot from PR #813.
A count changes when ordinary corpus vectors are added and can make concurrent changes fail CI.
🤖 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/tests/argv.rs` at line 16, Replace the hardcoded IN_SCOPE
constant in argv.rs with the exempt-vector snapshot from PR `#813`, so the
expected count tracks the intended exempt corpus rather than ordinary vector
additions. Preserve the existing type and usage of IN_SCOPE.
| A number here means digits, at most one `.`, and an optional exponent — | ||
| deliberately narrower than what a float parser accepts, since `-inf` is far | ||
| likelier to be a misspelled flag than a number somebody meant to pass. `-1x` and | ||
| `-1e` are not numbers either, and so name flags that do not exist. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Define the numeric grammar precisely.
The text says “optional exponent”, but strict classification depends on the signed-exponent form. State the exponent marker, optional sign, required digits, and accepted decimal forms. This prevents documentation consumers from classifying -1e-3, -1e, and -1. differently from the parser.
Suggested clarification
-A number here means digits, at most one `.`, and an optional exponent —
+A number here means digits, at most one `.`, and an optional signed exponent.
+Define the exponent marker, required digits, and accepted decimal forms.🤖 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 34 - 37, Update the numeric-argument
description around the “optional exponent” wording to define the grammar
precisely: specify the exponent marker, optional sign, and required exponent
digits, and list the accepted decimal forms, including the behavior for values
such as -1e-3, -1e, and -1. Keep the documented grammar aligned with the
parser’s strict classification rules.
| add the number case back afterwards. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use afterward in American English.
Replace afterwards with afterward to satisfy the locale check.
🧰 Tools
🪛 LanguageTool
[locale-violation] ~171-~171: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...gs, and had to add the number case back afterwards. ## Positional arguments A word that ...
(AFTERWARDS_US)
🤖 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 171 - 172, Update the wording near “add the
number case back afterwards” to use the American English form “afterward,”
preserving the rest of the sentence.
Source: Linters/SAST tools
The mount page describes only the behavior of the first commit in #806 and never picked up what the later ones changed: it still claims flags never trigger discovery, which is untrue of completions and help, and `overrides_default` shipped with no documentation at all. My fault, and worth writing down how: the edits were made by string replacement against text prettier had already rewrapped and turned `*own*` into `_own_`, so both replacements silently matched nothing. I said in the pull request that the page had been corrected, and it had not. Diff verified this time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A flag-like token that names no flag stays what it has always been here: a word, offered to the positionals like any other. That is deliberate, and now documented as a decision rather than left as an accident — the grammar said one thing and the parser did another, which was the only indefensible state. Every comparable parser refuses the token, and they are right for what they do: parse their own argv, where a dash-word is a flag or a typo. A usage spec also parses command lines whose flags it does not own — a script forwarding options through `usage exec`, a task whose script is the authority on what it accepts, a completion asked about half-typed input. In those, an unrecognized token is data in transit far more often than a mistake, and refusing it breaks the wrapper for anyone who did not enumerate the flags of the program behind it. The cost is a misspelled `--hekp` becoming an argument, so a CLI that owns all of its flags can say `unknown_flags "error"` and get typo detection. It is settable per CLI and per command, and inherited — a CLI can be strict everywhere except the one command that forwards. `#[usage(unknown_flags = "error")]` says it from a Rust struct, which is the case that usually wants it. Even when refusing, a lone `-` and a negative number stay values. oclif made exactly this mistake on the same switch and had to add the number back. Six corpus vectors move from expecting an error to expecting a value, and five new ones cover the strict mode, the inherited override, and the negative-number carve-out. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Four review findings, and the first is the interesting one. `-az` with only `-a` declared was read as a bundle: `-a` was applied and `z` left over as a value. A token containing an unrecognized letter is not a bundle at all, so none of its letters apply — the grammar said so and usage-argv did it, but usage-lib did not. The check had to go in phase 1, not just phase 2. Phase 1 keys a short token on its first letter, so `-az` was recorded as a binding for `-a`, and phase 2 trusting that binding is what defeated a check placed only there. It now decides whether the whole token is a bundle where the token is first read. The corpus could not see this: the vector for it declares no argument, so both readings produce `unexpected_arg`. A second vector gives it a free argument, where the difference is the whole point — `-a` unset and `-az` bound as one word. `is_flag_like` also treated anything after a digit as a number, so `-1x` slipped past a CLI that asked for unknown flags to be refused. It now asks whether the token *is* a number. usage-argv gets the same rule, spelled out by hand rather than deferred to a float parse, since it runs on the hot path. And `Spec::merge` dropped `unknown_flags`, so an `include` lost the policy it declared. Plus a docs link that markdownlint could not resolve. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The two implementations disagreed about `-1e5`. usage-lib asked `f64::from_str`, which accepts an exponent; usage-argv used a hand-written scanner that stopped at `e` and called the token a flag. So a CLI refusing unknown flags would accept `--offset -1e5` under one parser and reject it under the other — exactly the drift the corpus exists to prevent, in the one place the corpus had no vector. One rule now, spelled out identically in both: digits, at most one `.`, and an optional signed exponent. Narrower than a float parse on purpose, since `-inf` is far likelier to be a misspelled flag than a number somebody meant to pass. Four vectors pin the edges — `-1e5`, `-1.5e-3`, `-inf`, `-1e` — so the two cannot drift again without a test saying so. Also emits `unknown_flags` for a subcommand that differs from the command enclosing it. The tables carry the effective value per command, so repeating an inherited answer says nothing, but a command that differs has to say so or the setting never reaches the spec. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
10f765f to
938f8f9
Compare
The property was written twice, so an overriding subcommand emitted `unknown_flags="value" unknown_flags="value"` — a KDL node carrying the same property twice, which keeps only the last. Exactly the bug this same pull request fixes for `overrides` and `required_unless`, reintroduced two commits later by a botched relocation of the block. The test did not catch it because it asked whether the output *contained* `unknown_flags="value"`, which a doubled write also does. It now finds the line and counts, and I checked it fails on the duplicate before removing it. Also states the numeric grammar exactly — digits, at most one `.`, then optionally `e`/`E`, an optional sign, and at least one digit — rather than leaving "an optional exponent" to be guessed at, and uses the American spelling the locale check wants. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Two fixes, and the first is embarrassing in a useful way. The property was written twice. An overriding subcommand emitted The test did not catch it because it asked whether the output contained Worth naming the pattern, since it is the third time in this stack: The numeric grammar is now stated exactly — digits, at most one The AI-assisted — Tool: Claude Code; model: anthropic/claude-fable-5; version: unavailable. |
1 participant
Your call, implemented. An unrecognized flag-like token stays what it has always been here — a word, offered to the positionals — and that is now a documented decision rather than an accident. The grammar said one thing and the parser did another, which was the only indefensible state.
Why the outlier position is the right one
Every comparable parser refuses the token, and they are right for what they do: parse their own argv, where a dash-word is a flag or a typo. A usage spec also parses command lines whose flags it does not own:
usage exec, forwarding options to a tool it wrapsIn those, an unrecognized token is data in transit far more often than a mistake, and refusing it breaks the wrapper for anyone who did not enumerate the flags of the program behind it. The grammar page now says this, with the cost stated plainly: a misspelled
--hekpbecomes an argument, and whether it does depends on whether a positional happens to be free.Configurable at both levels
Nearest command that states a preference wins, then the spec, then
value. Unlikeeffectthis is inherited — it describes how a command line is read rather than what a command does, and a CLI that forwards options tends to forward them everywhere.From Rust:
#[usage(unknown_flags = "error")], which is the case that usually wants it — a compiled binary generally owns every flag it accepts.The carve-out oclif had to learn
Even when refusing, a lone
-and a negative number stay values. oclif/core#600 is that exact regression: they switched to "anything starting with a hyphen is a flag", broke negative-number arguments, and had to special-case them back. The check here skips non-flag-like tokens before it ever consults the setting, and a test pins-1underunknown_flags "error".One subtlety worth flagging: the rejection had to go inside the flag branches, where the lookup has just failed and nothing from the token has been applied. Placed downstream,
-azset-abefore discovering thatznames nothing — a partly-applied bundle from a rejected token.Scope
lib: the setting onSpecandSpecCommand, KDL parse and emit, inherited resolution, refusal in both flag branches.argv: lenient by default, with the effective mode stored per command — inheritance is resolved by whoever builds the tables, keeping it out of the parse.derive: the struct attribute, emitted into both the tables and the spec./spec/argv#unrecognized-flagsexplains the reasoning;/spec/reference/cmddocuments the knob.This closes the last of the big recorded divergences. 91 vectors, 10 remaining — and those are now narrow.
AI-assisted — Tool: Claude Code; model: anthropic/claude-fable-5; version: unavailable.
Note
Medium Risk
Changes default argv parsing for unrecognized flags (reject → treat as values) across both parsers, which can alter existing CLI behavior. Mitigated by an explicit strict mode and broad corpus coverage.
Overview
Unrecognized flag-like tokens are now values by default, offered to positionals like any other word. Specs that wrap or forward argv (scripts, tasks, completions) no longer reject tokens they do not own. CLIs that want typo detection opt in with
unknown_flags "error".The setting lives on both the root spec and individual commands, inherits (nearest enclosing preference wins), and is overridable per command — e.g. strict overall except
cmd "exec" unknown_flags="value". Derive exposes the same knob via#[usage(unknown_flags = "error")].Also tightens number detection so
-1e5/-1.5e-3stay values while-infand-1xremain flag-shaped, and ensures unknown short bundles like-azapply none of their letters before falling through or erroring.Reviewed by Cursor Bugbot for commit 2dd7590. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation