feat(spec): say that one flag needs another, which nothing here could by jdx · Pull Request #925 · jdx/usage

jdx · 2026-08-16T23:23:51Z

Stacked on #919 — review that first; this PR's diff is the second commit.

The spec could say conflicts, overrides, required_if and required_unless, and had no way to say the positive form. requires is the last blocker in PLAN.md's derive-v1 list.

flag "--out <path>" requires="--format"
flag "--sign" {
  requires "--key" "--identity"   // several, and all of them are needed
}

#[usage(long, requires("--key", "--identity"))]
sign: bool,

Why not just `required_if`

It states the same rule from the other end, and both stay. --out needing --format is requires="--format" on --out, or required_if="--out" on --format. Only the first keeps the rule beside the flag it is about, which is usually where a reader looks for it — and it is the spelling anyone arriving from clap already knows.

Wired the whole way through

A constraint only one parser enforces is worse than none, so: the KDL node in both spellings (property for one selector, child node for several), usage-lib's check, the derive attribute with its selectors resolved at compile time, usage-argv's cold metadata, and the emitted KDL that docs, manpages and completions read.

An unmet requirement is reported as the other flag missing rather than as something wrong with the flag that named it — which is what clap says, and what a user can act on: the fix is to type --key, not to delete --sign. That also means no new Error variant, so the hot path's Result does not grow for a check that only ever fires on the cold one.

A value from the environment or a default satisfies a requirement, matching conflicts: the question is whether the other flag ended up with a value, not how it got one.

The bridge does not carry it

clap_usage reads a clap::Command back through public getters, and clap 4.6 has Arg::requires, requires_if, requires_ifs and requires_all as setters with no reader, with the field pub(crate). So there is nothing to read, whatever the Arg was built with.

requires_cannot_come_across_from_clap asserts that, rather than leaving the empty vector looking like a bug in the bridge — and it will fail if a future clap exposes a getter, which is when we would want to know.

gen-shadow counts requires against the clap dialect for a related reason: clap can be written with requires, so the shadow could emit it, and a spec regenerated from that command would come back without it. Counting it keeps the clap side from looking more faithful than it is.

docs/spec/integrations/clap.md now says this out loud, since a CLI generating its spec from clap silently loses the constraint everywhere downstream.

Tests

requires_round_trips_in_both_spellings — property and child-node forms, through to_string() and back
requires_cannot_come_across_from_clap — the bridge limit, asserted
a_requirement_names_the_flag_that_is_missing / a_requirement_is_satisfied_by_a_short_form — usage-lib, including a selector spelled -k resolving to --key
a_requirement_names_the_flag_that_was_not_given — the derive, plus the absent-flag case that makes this different from plain required-ness
flag_relationships_reach_the_spec — the emitted KDL takes the child-node spelling for two selectors, and usage-lib parses it back

cargo test --all --all-features and cargo clippy --all --all-features -- -D warnings are clean.

🤖 Generated with Claude Code

Note

Medium Risk
Touches post-parse validation in usage-lib and derive-generated checks; behavior is well-tested but wrong satisfaction rules (defaults/env/overrides) would change CLI acceptance for derived binaries.

Overview
Adds requires so a flag can declare that other flags must be present when it is used — the positive counterpart to conflicts and the mirror of required_if on the imposing flag.

Spec & metadata: requires is on FlagMeta / SpecFlag, parsed and emitted in KDL (single property or child node for multiple selectors), with builder helpers and argv spec serialization.

Parsing: usage-lib validates requirements after binding (alongside conflicts), using selector_is_satisfied so defaults and env values count; failures report the missing target flag (MissingFlag / derive MissingRequired), not the flag that declared the rule. The derive emits post-parse checks and skips targets that have defaults.

Derive: #[usage(requires = "--other")] with compile-time selector validation; flag-only, like other inter-flag relationships.

Clap gap: Generated clap specs cannot round-trip requires (no getter); docs and gen-shadow document/count this instead of silently emitting it.

^{Reviewed by Cursor Bugbot for commit 89866b8. Bugbot is set up for automated code reviews on this repo. Configure here.}

coderabbitai · 2026-08-16T23:24:04Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: bf9e02c4-1fc0-4c11-a7d3-a36b680a85fe

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

greptile-apps · 2026-08-16T23:25:54Z

Greptile Summary

The PR adds the requires flag relationship across KDL specs, the reference parser, derive-generated parsers, emitted metadata, documentation, and shadow generation.

Supports single-selector property and multi-selector child-node KDL forms.
Resolves derive selectors at compile time and reports the required flag that is missing.
Treats explicit, environment-provided, and defaulted values as satisfying requirements.
Documents the clap bridge’s inability to recover this relationship.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously reported default-ordering issue is addressed by requirement satisfaction logic that recognizes non-overridden declared defaults before those defaults are bound.

Important Files Changed

Filename	Overview
lib/src/parse.rs	Adds reference-parser requirement validation and now recognizes declared defaults when deciding whether the required flag is satisfied, resolving the prior ordering defect.
derive/src/codegen.rs	Emits requirement metadata and post-binding checks while omitting checks whose targets always receive defaults.
derive/src/model.rs	Parses and validates derive-level `requires` selectors alongside the existing flag relationships.
lib/src/spec/flag.rs	Adds the `requires` model field with both KDL spellings, serialization, and round-trip coverage.
argv/src/spec.rs	Carries `requires` through cold metadata and emits it into generated KDL.
conformance/tests/post_binding.rs	Covers missing, satisfied, absent-declaring-flag, and default-satisfied derive behavior.

_{Reviews (2): Last reviewed commit: "fix(spec): a defaulted flag is not a mis..." | Re-trigger Greptile}

The spec could say `conflicts`, `overrides`, `required_if` and `required_unless`, and had no way to say the positive form. `requires` is the last blocker in the derive's v1 list, and the only one of the four relationships mise's clap declarations cannot round-trip at all. Declared on the flag that imposes the rule, which is where clap puts it and where a reader looks for it. `required_if` states the same rule from the other end and both stay: `--out` needing `--format` is `requires="--format"` on `--out` or `required_if="--out"` on `--format`, and only the first keeps the rule beside the flag it is about. Wired the whole way through, since a constraint that only one parser enforces is worse than none: the KDL node in both spellings, usage-lib's check, the derive attribute with its selectors resolved at compile time, usage-argv's metadata, and the emitted KDL. An unmet requirement is reported as the *other* flag missing, which is what clap says and what a user can act on — the fix is to type `--key`, not to delete `--sign`. That also means no new `Error` variant, so the hot path's `Result` does not grow for a check that only fires on the cold one. A value from the environment or a default satisfies a requirement, matching what `conflicts` already does: the question is whether the other flag ended up with a value, not how it got one. The bridge does not carry it, and cannot. clap 4.6 has `Arg::requires` and its variants as setters with no getter and keeps the field `pub(crate)`, so a `Command` cannot be asked what it requires. `requires_cannot_come_across_from_clap` asserts that rather than leaving the empty vector looking like a bug, and will fail if a future clap exposes it. `gen-shadow` counts it against the clap dialect for the same reason: clap can be *written* with `requires`, and a spec regenerated from that command comes back without it, so emitting it there would make the clap side look more faithful than it is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…its fields Two from review. A requirement whose target had a default was reported as unmet, which contradicted this PR's own documentation and, worse, contradicted the file it lives in: plain `required`, `required_if` and `required_unless` all read a default as filling a flag, so the same flag was missing in one check and present ten lines below. The rule, stated once so the next relationship inherits it: a *negative* rule — `conflicts` — counts only what was given, because a flag with a default would otherwise conflict with everything and nothing would parse. A *positive* rule asks whether the flag it names has a value, and a default is a value. usage-lib gets `selector_is_satisfied` beside `selector_is_explicit`; the derive skips emitting the check altogether when the target has a default, since it could never fail. And the conformance fixture declared `--key` and `--identity` without reading them, which `clippy --all-targets` fails on. They are asserted now, which is what they were for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

jdx · 2026-08-17T00:11:04Z

Pushed fixes for both findings.

Defaults failing requirement checks (@greptile-apps P1) — correct, and worse than a doc mismatch: required, required_if and required_unless in the same file all read a default as filling a flag, so the same flag was missing in this check and present ten lines below it.

The rule, written down so the next relationship inherits it rather than picking again:

A negative rule — conflicts — counts only what was given. It has to: a flag with a default would otherwise conflict with everything and no command line would parse.
A positive rule asks whether the flag it names has a value, and a default is a value.

usage-lib gets selector_is_satisfied beside selector_is_explicit. The derive skips emitting the check entirely when the target has a default, since it could never fail — decided at compile time, the same way required_checks already skips such a field.

The lint failure — the conformance fixture declared --key and --identity and never read them, which clippy --all-targets fails on. They are asserted now, which is what they were for.

This comment was generated by Claude Code.

greptile-apps · 2026-08-17T00:12:04Z

Both fixes land correctly.

selector_is_satisfied is the right shape: it delegates to selector_is_explicit first, then falls through to check flag.default and arg.default — so the positive/negative asymmetry is expressed once in one place rather than scattered across callers. The rule you've written down (negative counts only what was given, positive asks whether the flag has a value) is exactly what the code does, and the three new tests (a_default_satisfies_a_requirement, an_environment_value_satisfies_a_requirement, a_default_on_the_required_flag_satisfies_the_requirement) each pin a different entry path into that value.

The derive side is the better call too: skipping the check entirely at compile time when the target has a default is cleaner than emitting if partial.__given_sign && !partial.__given_jobs that can never be true — same reasoning as required_checks already uses, and the comment says so.

The clap requires_cannot_come_across_from_clap assertion is a nice guard: an empty vec that's a deliberate design decision and an empty vec that's a bug look identical until the test names the difference. If a future clap exposes the getter, the failure is the right signal.

Everything looks good — no further concerns.

`usage` is now its own first adopter. The ten command structs, the root and the two command enums are declared with `usage-derive` instead of clap, and `--usage-spec` prints `Cli::to_kdl()` — the same tables that parsed the command line, rather than a transcription of a clap `Command` through `clap_usage`. Smaller than mise and far less forgiving of a lossy spec, because this CLI's spec is what generates its own docs, manpage and completions: anything the derive cannot say shows up in the checked-in output. ## What went away - `clap`, `clap_usage` and the `clap-sort` dev-dependency, plus `tests/clap_sort.rs` — declaration order is held by the spec since #915. - `command_effects.rs`'s two tables, 60 lines that existed because "clap has no way to express this". Each command declares `#[usage(effect = "…")]` where it is defined; the file keeps `UNCLASSIFIED` and the coverage tests, which now read the derived metadata. A stale entry is no longer possible for the effects themselves — an effect moves with the command it is written on. ## What the conversion needed - The four shell commands shared one `Shell` struct, which the derive refuses: a command collects into the struct that declares it. They are four structs flattening a shared group now, written by a macro so the paragraph of long help is not copied four times. Their docs improve as a side effect — all four used to say "Execute a shell script with the specified shell". - `sponsors` is a bare variant (#923), so its empty struct is gone. - `requires` has no positive form in the spec, so the two constraints that used it are stated as `required_if` on the other flag. `--out-dir requires --multi` is a positive requirement on a `bool` and has no spelling at all; #925 adds `requires`, and it belongs here when it lands. ## What the emitted spec gains and loses Gains `JDX_USAGE_BIN` on `--usage-bin` (the bridge dropped `env`), long help that keeps its line breaks, an `about` for `generate manpage`, and `name "usage"` rather than `name "usage-cli"`. Loses `subcommand_required`, which the spec can hold and the derive knows from a bare `T` subcommand field but does not emit, and strictness on subcommands: `unknown_flags` is accepted on an `Args` and ignored, and the root's is not inherited, so only the root is strict. Both are derive gaps worth their own fix rather than a workaround here. Workspace suite green, clippy clean, docs and assets re-rendered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…ccepts Follow-up to the conversion, now stacked on the two gaps it found rather than describing them as open. `unknown_flags` reaches the whole tree. The root declares `error` once and every subcommand inherits it, so `usage lint --nope f.kdl` names the flag again instead of making `--nope` the file and calling the real file unexpected. The five commands that hand a command line to somebody else's script declare `value` for themselves, which is the half that needs a subcommand to be able to say something. ## From review `--version` says `usage 5.1.0`, not `usage-cli 5.1.0`. The crate is `usage-cli` and the binary is `usage`; everything else this CLI says about itself now comes from the spec, where the name is `usage`, so the version line was contradicting the help banner printed directly above it. Read from the spec by `version()`, which `-v` shares, so the two cannot drift. `--out-dir` without `--multi` is refused. clap said this as a `requires` in both directions; the spec can only state the direction that makes `--multi` need a destination, so without a check the other way `usage g markdown --out-dir docs` quietly wrote a single file somewhere else. Enforced in `run()` until #925's `requires` can carry it. `--completions` is a flag, which is how it is typed. The clap declaration made it a *positional*, so the spec, the docs, the manpage and the generated completions all described a `[COMPLETIONS]` argument that nothing accepts, while the flag that does work went undocumented. The conversion carried that over faithfully, wrong included. The point of emitting the spec from the declaration is that the two cannot disagree, so the declaration is what changes. Declined one: CodeRabbit read the `man` alias on `generate manpage` as removed. It is declared, in the emitted spec, and on the rendered page. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>