feat(argv): emit a usage spec from static metadata - #801
Conversation
A CLI knows more about itself than a parse needs: help text, choices, defaults, what a command does to the world. Putting that in the parse tables would make them sparse for no benefit, since a successful parse never reads any of it, so it goes in a parallel tree behind the `spec` feature. Each entry borrows the table entry it describes, so a flag's long and short forms have one definition and cannot drift from what the parser matches. `Spec::to_kdl` writes the spec out. Hand-written rather than through the kdl crate, because this crate has no dependencies and the output only has to be correct — which is checked by parsing it back with usage-lib and comparing the resulting spec field by field, over a fixture chosen to be awkward: text needing escapes, a hidden alias, a negated flag, choices on both a flag and an argument, every double_dash mode, an effect, a mount, a restart token, and two levels of nesting. It then renders through the markdown and manpage generators an adopter's docs build actually uses. One thing is deliberately missing. `help_heading` groups flags in help output, clap has it, and mise uses it — but the spec has no node for it, so carrying it here would make the emitted KDL a summary rather than a definition. It is recorded in PLAN.md as a spec extension to propose instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR adds an optional ChangesCLI specification emission
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SpecToKdl
participant UsageLib
participant MarkdownGenerator
participant ManpageGenerator
SpecToKdl->>UsageLib: Emit and parse specification KDL
UsageLib->>MarkdownGenerator: Render parsed metadata
UsageLib->>ManpageGenerator: Render parsed metadata
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 adds feature-gated static CLI metadata and KDL emission for
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported root-example omission is fixed by writing every root example as a top-level KDL node, and the conformance test verifies the parsed fields. Important Files Changed
Reviews (4): Last reviewed commit: "fix(spec): emit strings that can be read..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Three ways the writer was lossy, all found in review of the claim that it is not. A flag's `effect` was never written, though commands' were. Several defaults were written as repeated properties, and KDL properties are unique per node, so all but the last were discarded — they need a child block. And the root command's examples and mount went nowhere, because the root's nodes belong at the top level of the document rather than inside a `cmd` block, so the shared writer never saw them. The fixture now includes a destructive flag, a flag with two defaults, and a root-level example, so each stays covered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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 5094b6a. Configure here.
The previous commit started writing the root command's own nodes at the top level of the document, and included its mount. The spec accepts `mount` only inside a `cmd` block, so that produced a document that does not parse. Dropping it quietly would be the lossiness this module claims not to have, so it is a debug assertion and a doc comment on the field, and PLAN.md now asks whether a root-level mount should be expressible at all. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
conformance/Cargo.toml (1)
17-17: 📐 Maintainability & Code Quality | 🔵 TrivialFeature unification can hide the default-off build of
usage-argv.Cargo unifies features across workspace members within one build invocation for the same target. A
cargo test --workspacetherefore compilesusage-argvwithspecenabled for every member, because this crate requests it.The feature is additive, so nothing breaks. But the configuration the crate documents as its default —
specoff, no metadata compiled — is then never built in that invocation.Add a job step that checks the crate on its own, so the default-off path stays compiling.
cargo check -p usage-argv --no-default-features cargo test -p usage-argv --features spec🤖 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/Cargo.toml` at line 17, Add a CI job step for the usage-argv crate that separately runs cargo check -p usage-argv --no-default-features to validate its default-off configuration, alongside cargo test -p usage-argv --features spec to cover the feature-enabled path. Keep the existing workspace configuration unchanged.
🤖 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 262-288: Update write_body to assert equal lengths between the
parse-table and metadata collections before iterating: meta.cmd.flags with
meta.flags, meta.cmd.args with meta.args, and meta.cmd.subcommands with
meta.subcommands. Keep the existing per-entry ordering assertions and writing
behavior unchanged.
- Around line 388-393: Update the metadata serialization around has_children and
the overrides and required_unless emission loops so multi-value fields are
represented as child nodes rather than repeated properties; retain property
output for single values as appropriate. Ensure the reader can preserve both
entries and add a round-trip test covering two values for each field.
- Around line 238-254: Update write_kdl’s root metadata handling to prevent
loss: extend the existing debug_assert guard for self.root to reject root
effect, hide, restart_token, and aliases, and ensure root about/long_about are
either emitted at the top level or validated as unset so callers use Spec.about
and Spec.long_about.
- Around line 586-601: Update quoted to escape every KDL-disallowed code point,
including control characters and U+200E–U+200F, U+202A–U+202E, U+2066–U+2069,
and U+FEFF, using \u{...} notation while preserving existing quote, slash, and
common whitespace escaping. Add coverage for U+0000, U+007F, U+202E, U+2069, and
U+FEFF.
In `@conformance/tests/spec_roundtrip.rs`:
- Around line 571-582: Update the comment above
reparsing_our_own_output_is_a_fixed_point to accurately describe that it checks
usage-lib serializer idempotence, not fidelity of the emitted KDL or detection
of dropped fields. Leave the test logic unchanged.
- Around line 606-609: Update the manpage assertion in the relevant test to
check for the specific title header ".TH EX 1" instead of the broad "ex"
substring, ensuring the program name is validated in the manpage header.
---
Nitpick comments:
In `@conformance/Cargo.toml`:
- Line 17: Add a CI job step for the usage-argv crate that separately runs cargo
check -p usage-argv --no-default-features to validate its default-off
configuration, alongside cargo test -p usage-argv --features spec to cover the
feature-enabled path. Keep the existing workspace configuration unchanged.
🪄 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: 9a1ebf16-59bd-497a-9054-855c61b3e65b
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockconformance/tests/snapshots/spec_roundtrip__the_emitted_spec_is_stable.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
PLAN.mdargv/Cargo.tomlargv/src/lib.rsargv/src/spec.rsconformance/Cargo.tomlconformance/tests/spec_roundtrip.rs
| // The root's own nodes sit at the top level rather than inside a `cmd` | ||
| // block, so they are written here instead of by write_command. | ||
| // | ||
| // A mount is the exception: the spec only accepts one inside a `cmd` | ||
| // block, so a root mount is not expressible. Emitting it anyway would | ||
| // produce a document that does not parse, and dropping it quietly is the | ||
| // lossiness this module claims not to have — so it fails loudly in debug | ||
| // builds instead, and PLAN.md carries it as a possible spec extension. | ||
| debug_assert!( | ||
| self.root.mount.is_none(), | ||
| "a mount on the root command cannot be written: the spec accepts \ | ||
| `mount` only inside a `cmd` block" | ||
| ); | ||
| for example in self.root.examples { | ||
| write_example(out, example, 0)?; | ||
| } | ||
| write_body(out, self.root, 0) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
usage.jdx.dev spec KDL top-level nodes alias effect restart_token
💡 Result:
The usage.jdx.dev specification, used for defining CLI tools, utilizes KDL (a human-readable document language) to declare CLI metadata, commands, flags, and arguments [1][2]. Below are the details for the requested nodes and properties: alias Aliases are used to define alternative names for commands or flags [3][4]. - For commands, aliases are defined within the command block [3]. You can pass multiple aliases at once and optionally hide them from documentation/completions using the hide=#true property [3]. - For flags, an alias can be defined as a child node or property within the flag definition [4]. effect The effect property is used on commands to classify the side effects the command has on the system [5]. - It can be set as a property (effect="read") or a child node (effect "read") [5]. - Recognized, optional values include "read", "write", and "destructive" [5]. - If missing, the effect is considered "unknown" [5]. These values are not inherited by subcommands [5]. restart_token The restart_token property is used to support command chaining by resetting the argument parsing state [6]. - It allows users to invoke multiple commands on a single line (e.g., cmd1::: cmd2) by specifying the token that triggers a parser reset [6]. - It is defined at the command level within the KDL spec [6]. Top-Level Nodes Top-level metadata defines the CLI itself [7]. Common top-level nodes include [7][8]: - min_usage_version: The minimum supported version of the Usage spec. - name: The friendly name of the CLI. - bin: The name of the binary. - version: The version of the CLI. - author: The author of the CLI. - license: The SPDX license. - repository: The URL of the source repository. - about / long_about: Short and long descriptions for help outputs. - before_help / after_help / before_long_help / after_long_help: Text displayed in relation to help screens. - include: Allows merging and overriding values from another usage spec file. - example: Demonstrates CLI usage at the spec level [7].
Citations:
- 1: https://usage.jdx.dev/spec/
- 2: https://usage.jdx.dev/
- 3: https://usage.jdx.dev/spec/reference/cmd
- 4: https://usage.jdx.dev/spec/reference/flag
- 5: feat(spec): add effect= to declare what a command does to the world #739
- 6: https://github.com/jdx/usage/releases/tag/v2.11.0
- 7: https://usage.jdx.dev/spec/reference/
- 8: Unsupported top-level metadata
license,(before|after)_help,(before|after)_long_help#537
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- relevant files ---'
git ls-files 'argv/src/spec.rs' 'argv/src/**' | head -80
printf '%s\n' '--- spec.rs outline ---'
ast-grep outline argv/src/spec.rs || true
printf '%s\n' '--- writer and metadata definitions/usages ---'
rg -n -C 3 'struct (Spec|CommandMeta)|enum Effect|restart_token|hidden_aliases|aliases|fn write_(kdl|body|command)|Spec::about|long_about' argv/srcRepository: jdx/usage
Length of output: 10071
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- writer implementation ---'
sed -n '213,335p' argv/src/spec.rs
printf '%s\n' '--- module documentation and fixture references ---'
sed -n '1,80p' argv/src/spec.rs
rg -n -C 4 'ROOT_META|CommandMeta|effect|restart_token|hidden_aliases|default_subcommand|long_about' . \
-g '!target' -g '!node_modules' | head -240Repository: jdx/usage
Length of output: 22837
🌐 Web query:
site:usage.jdx.dev/spec/reference effect restart_token hide alias top-level Usage spec
💡 Result:
In the context of the --usage specification (a tool for defining CLI interfaces in KDL format), the terms you mentioned relate to controlling command, flag, and alias visibility and behavior [1][2]. Note that there is no standard feature named "restart_token" in the --usage specification; it is possible this refers to a specific implementation detail or a misinterpretation of another configuration option. The hide and alias directives are used as follows: Alias: Aliases are used to provide alternative names for commands or flags [1][2]. - For commands: You can define multiple aliases within a command block [2]. - For flags: You can define an alias for a flag (e.g., -u for --user) [1]. - For config keys: You can use config_alias to map one configuration key to another for backwards compatibility [3]. Hide: The hide directive is used to prevent an item from appearing in generated documentation or CLI completion lists [1][2]. - You can hide specific aliases by setting hide=#true inside an alias definition [1][2]. - You can hide entire commands by setting hide=#true at the command level [2]. Top-level Usage Spec: The --usage specification allows for defining metadata at the top level of the KDL file, such as the CLI name, version, author, and license [4]. This top-level block acts as the entry point for configuring the CLI's behavior and generating its documentation [4].
Citations:
- 1: https://usage.jdx.dev/spec/reference/flag
- 2: https://usage.jdx.dev/spec/reference/cmd
- 3: https://usage.jdx.dev/spec/reference/config
- 4: https://usage.jdx.dev/spec/reference/
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
src = Path("argv/src/spec.rs").read_text()
write_kdl = src[src.index("fn write_kdl"):src.index("/// Write a command's contents")]
write_command = src[src.index("fn write_command"):src.index("fn write_example")]
root_only = ["about", "long_about", "hide", "effect", "restart_token", "aliases", "hidden_aliases"]
print("root writer references:")
for name in root_only:
print(f"{name}: {name in write_kdl}")
print("\ncommand writer references:")
for name in root_only:
print(f"{name}: {name in write_command}")
assert "self.root.mount" in write_kdl
assert "self.root.about" not in write_kdl
assert "self.root.long_about" not in write_kdl
assert "self.root.effect" not in write_kdl
assert "self.root.restart_token" not in write_kdl
assert "self.root.cmd.aliases" not in write_kdl
assert "self.root.hidden_aliases" not in write_kdl
PYRepository: jdx/usage
Length of output: 420
Guard or emit root command metadata
write_kdl emits Spec.about and Spec.long_about, but it drops the corresponding root CommandMeta fields. effect, hide, restart_token, and aliases are valid only inside cmd blocks, so extend the debug guard to reject them on the root. Either emit root about and long_about at the top level, or assert that callers set them only on Spec.
🤖 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 238 - 254, Update write_kdl’s root metadata
handling to prevent loss: extend the existing debug_assert guard for self.root
to reject root effect, hide, restart_token, and aliases, and ensure root
about/long_about are either emitted at the top level or validated as unset so
callers use Spec.about and Spec.long_about.
| fn quoted(value: &str) -> String { | ||
| let mut out = String::with_capacity(value.len() + 2); | ||
| out.push('"'); | ||
| for ch in value.chars() { | ||
| match ch { | ||
| '"' => out.push_str("\\\""), | ||
| '\\' => out.push_str("\\\\"), | ||
| '\n' => out.push_str("\\n"), | ||
| '\r' => out.push_str("\\r"), | ||
| '\t' => out.push_str("\\t"), | ||
| c => out.push(c), | ||
| } | ||
| } | ||
| out.push('"'); | ||
| out | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
KDL v2 quoted string escape rules disallowed literal control characters
💡 Result:
In KDL v2, quoted strings are not permitted to contain certain literal control characters directly [1]. The specification requires that these disallowed literal code points be represented instead as Unicode escape sequences (e.g., \u{...}) within the string [1]. Specifically, the KDL v2 specification mandates that strings must not include disallowed literal code points directly [1]. While the spec defines a standard set of escape sequences for common control characters (such as \n, \r, \t, \b, and \f), any control characters not covered by these specific escape sequences must be handled using the \u{...} Unicode escape syntax to be included as values in a quoted string [2][3][1]. This rule is enforced to ensure consistency and prevent issues with invisible or potentially harmful control characters in the document language [1]. If a character is not explicitly listed as a permitted literal or a supported escape in the specification's table, it cannot appear as a literal within the quoted string [2][3][1].
Citations:
- 1: https://github.com/kdl-org/kdl/blob/fa204cec62abef085e33af65f849994846ae68a6/SPEC.md
- 2: https://github.com/kdl-org/kdl/blob/main/draft-marchan-kdl2.md
- 3: https://kdl.dev/spec/
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(argv/src/spec\.rs|.*kdl.*|.*spec.*)$' | head -80
printf '%s\n' '--- quoted and related tests ---'
rg -n -C 8 'fn quoted|quoting_escapes_what_would_break_a_document|help_heading|is_control|direction|U\+00|unicode' argv/src/spec.rs
printf '%s\n' '--- KDL string escape references ---'
rg -n -i -C 3 'quoted string|unicode escape|control character|direction|bidi|literal' . -g '*.rs' -g '*.md' -g '*.kdl' | head -240Repository: jdx/usage
Length of output: 20593
🌐 Web query:
KDL v2 specification quoted string forbidden literal code points C0 DEL bidi direction-control U+202A U+202B U+202C U+202D U+202E U+2066 U+2067 U+2068 U+2069
💡 Result:
In the KDL v2 specification, quoted strings are forbidden from including certain literal code points directly [1][2][3]. Specifically, they must not contain characters classified as disallowed literal code points within their body [1][2][3]. These disallowed literal code points include: - C0 control characters (U+0000 through U+001F) [1][2][3]. - The Delete (DEL) control character (U+007F) [1][2][3]. - Bidi direction-control characters, specifically: - Left-to-Right Embedding (U+202A) [1][2][3] - Right-to-Left Embedding (U+202B) [1][2][3] - Pop Directional Formatting (U+202C) [1][2][3] - Left-to-Right Override (U+202D) [1][2][3] - Right-to-Left Override (U+202E) [1][2][3] - Left-to-Right Isolate (U+2066) [1][2][3] - Right-to-Left Isolate (U+2067) [1][2][3] - First Strong Isolate (U+2068) [1][2][3] - Pop Directional Isolate (U+2069) [1][2][3] While these code points cannot be included as literal characters in the body of a quoted string, they can be included as string values by using their corresponding Unicode escape sequence (e.g., \u{...}) [1][3]. Raw strings, by contrast, do not support escape sequences and therefore cannot represent these characters [1][2].
Citations:
- 1: https://kdl.dev/spec/
- 2: https://github.com/kdl-org/kdl/blob/main/draft-marchan-kdl2.md
- 3: https://github.com/kdl-org/kdl/blob/fa204cec62abef085e33af65f849994846ae68a6/SPEC.md
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
p = Path("argv/src/spec.rs")
lines = p.read_text().splitlines()
for start, end in ((570, 630),):
print(f"--- {p}:{start}-{end} ---")
for n in range(start, min(end, len(lines)) + 1):
print(f"{n:4}: {lines[n-1]}")
PY
printf '%s\n' '--- repository metadata and dependency declarations ---'
rg -n -i 'kdl|usage-lib|usage_lib' Cargo.toml Cargo.lock argv -g '*.toml' -g '*.rs' | head -200Repository: jdx/usage
Length of output: 3409
🌐 Web query:
site:kdl.dev/spec KDL v2 string grammar forbidden control characters directional formatting characters
💡 Result:
According to the KDL specification, certain code points are disallowed from appearing literally anywhere in a KDL document, including within strings. These characters must be represented using Unicode escapes (\u{...}) if they are to be included in quoted or multi-line strings, though they cannot be represented in raw strings. [1] The disallowed literal code points include: [1] Control characters: U+0000 through U+0008, U+000E through U+001F, and U+007F (Delete). Unicode direction control characters: U+200E through U+200F (Left-to-Right Mark, Right-to-Left Mark), U+202A through U+202E (Embedding/Override characters), and U+2066 through U+2069 (Isolate characters). Other disallowed code points: Any code point that is not a Unicode scalar value (U+D800 through U+DFFF) and U+FEFF (Byte Order Mark, or Zero-width Non-Breaking Space), unless it appears as the very first character in a document. [1]
Citations:
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import urllib.request
urls = [
"https://kdl.dev/spec/",
"https://raw.githubusercontent.com/kdl-org/kdl/main/draft-marchan-kdl2.md",
"https://raw.githubusercontent.com/kdl-org/kdl/fa204cec62abef085e33af65f849994846ae68a6/SPEC.md",
]
for url in urls:
print(f"--- {url} ---")
try:
text = urllib.request.urlopen(url, timeout=10).read().decode()
except Exception as e:
print(type(e).__name__, e)
continue
terms = ("disallowed", "control character", "direction", "U+0000", "U+FEFF", "quoted string")
lines = text.splitlines()
for i, line in enumerate(lines):
if any(t.lower() in line.lower() for t in terms):
lo, hi = max(0, i - 2), min(len(lines), i + 4)
print("\n".join(f"{n+1}: {lines[n]}" for n in range(lo, hi)))
print()
PYRepository: jdx/usage
Length of output: 793
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
for cp in [*range(0x00, 0x20), *range(0x7f, 0xa0),
0x200e, 0x200f, *range(0x202a, 0x202f),
*range(0x2066, 0x206a), 0xfeff]:
ch = chr(cp)
print(f"U+{cp:04X} is_control={ch.isascii() and ch.isprintable() is False or cp >= 0x7f and cp <= 0x9f}")
PYRepository: jdx/usage
Length of output: 1930
🌐 Web query:
KDL v2 specification "U+000E" "U+200E" "U+FEFF" disallowed literal code points
💡 Result:
In the KDL specification (including v2), the code points U+000E, U+200E, and U+FEFF are classified as disallowed literal code points [1][2][3]. According to the specification, these characters must not appear literally anywhere in a KDL document [1][3]. They can, however, be represented within Quoted Strings (but not Raw Strings) using the KDL Unicode escape syntax \u{...} [1][2][3]. The specific treatment for these characters is as follows: 1. U+000E: This is grouped with U+0000–U+0008 and U+000F–U+001F as part of the disallowed C0 control characters [1][3]. 2. U+200E: This is grouped with U+200F, U+202A–U+202E, and U+2066–U+2069 as part of the disallowed Unicode direction control characters [1][3]. 3. U+FEFF: This is the Zero-width Non-breaking Space (ZWNBSP), also known as the Byte Order Mark (BOM). It is disallowed as a literal character anywhere in the document, with the explicit exception that it is permitted as the first code point of a document [1][2][3]. These constraints are maintained to ensure document integrity and avoid ambiguity in parsing or processing text [1][3].
Citations:
- 1: https://kdl.dev/spec/
- 2: https://github.com/kdl-org/kdl/blob/fa204cec62abef085e33af65f849994846ae68a6/SPEC.md
- 3: https://github.com/kdl-org/kdl/blob/main/draft-marchan-kdl2.md
🌐 Web query:
KDL specification "disallowed literal code points" "U+202A"
💡 Result:
In the KDL specification, the code point U+202A (along with other related direction control characters) is explicitly disallowed as a literal character anywhere in a KDL document [1][2]. These characters are categorized as Unicode "direction control" characters, which include the following ranges: - U+200E–U+200F - U+202A–U+202E - U+2066–U+2069 Because they are disallowed as literals, they cannot appear directly in the document [2]. However, they may be represented within standard strings (but not raw strings) using Unicode escape sequences (e.g., \u{202A}) [2]. This restriction is in place to prevent potential security and display issues associated with invisible directional formatting characters [1][2].
Citations:
- 1: https://github.com/kdl-org/kdl/blob/fa204cec62abef085e33af65f849994846ae68a6/SPEC.md
- 2: https://github.com/kdl-org/kdl/blob/main/draft-marchan-kdl2.md
🏁 Script executed:
#!/bin/bash
set -eu
curl -L --fail --silent --show-error \
https://raw.githubusercontent.com/kdl-org/kdl/main/draft-marchan-kdl2.md |
rg -n -i -C 8 'disallowed literal|U\+0000|200E|202A|2066|FEFF|control character'Repository: jdx/usage
Length of output: 5514
Escape the complete KDL disallowed-code-point set
KDL v2 also forbids literal U+200E–U+200F, U+202A–U+202E, U+2066–U+2069, and U+FEFF. char::is_control() does not match these characters. Escape the complete set with \u{...} and add tests for U+0000, U+007F, U+202E, U+2069, and U+FEFF.
🤖 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 586 - 601, Update quoted to escape every
KDL-disallowed code point, including control characters and U+200E–U+200F,
U+202A–U+202E, U+2066–U+2069, and U+FEFF, using \u{...} notation while
preserving existing quote, slash, and common whitespace escaping. Add coverage
for U+0000, U+007F, U+202E, U+2069, and U+FEFF.
| assert!( | ||
| manpage.contains("ex"), | ||
| "the manpage should name the program" | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find the manpage template header so the assertion matches the real output.
fd -H -t f . lib/src/docs/manpage
rg -n -C3 '\.TH|title|section' lib/src/docs/manpageRepository: jdx/usage
Length of output: 8809
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- conformance assertion context ---'
sed -n '580,620p' conformance/tests/spec_roundtrip.rs
printf '%s\n' '--- renderer header implementation and tests ---'
sed -n '34,47p' lib/src/docs/manpage/renderer.rs
sed -n '425,462p' lib/src/docs/manpage/renderer.rs
printf '%s\n' '--- all manpage header assertions ---'
rg -n 'contains\("\.TH|control\("TH"' lib conformanceRepository: jdx/usage
Length of output: 3112
Assert the manpage title header
manpage.contains("ex") can match unrelated generated text. Assert manpage.contains(".TH EX 1") to verify the program name.
🤖 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/spec_roundtrip.rs` around lines 606 - 609, Update the
manpage assertion in the relevant test to check for the specific title header
".TH EX 1" instead of the broad "ex" substring, ensuring the program name is
validated in the manpage header.
|
All four bot findings on this PR are fixed in the two commits since — the comments above are re-posts against the cumulative diff rather than the current state.
16 round-trip tests, and the whole workspace is green. AI-assisted — Tool: Claude Code; model: anthropic/claude-fable-5; version: unavailable. |
The round-trip test found two ways this crate serialized a spec it could not reparse, both of them older than the argv work. A node argument beginning with a dash was rendered bare, so a flag with several `overrides` or `required_unless` values produced `overrides --keep --dry-run` — which KDL rejects, since a bare word that starts with two dashes is not a value. Properties are left alone: `negate=--no-color` renders and parses today, and quoting it would rewrite every committed spec for nothing. A control character was rendered literally, and KDL requires an escape. That is not hypothetical: any CLI that colors its help text has an escape character in the middle of it, so `mise usage` output could contain one. Also in this commit, four fixes to the argv writer from the same review: a flag's several `overrides` and `required_unless` values now use a child node like defaults do, rather than repeated properties where only the last survives; control characters are escaped on the way out; the root command's `about` and `long_about` are written rather than dropped, and the things a root cannot express — effect, hide, restart token, aliases — fail loudly instead; and the table-versus-metadata check compares lengths, since indexing by metadata position could not see a table entry that had none. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Stacked on #801 — review that first; this PR's diff will include it until it merges. Closes the gap #801 turned up: writing the spec emitter, I reached for `help_heading` and found the spec has no such field. ## Why it matters more than it looks A CLI with dozens of flags is unreadable without sections, and clap has had `help_heading` for years. Because the spec could not record one, **`From<&clap::Arg>` was silently dropping it** — so every CLI in the fleet that groups its flags has been losing the grouping on the way into its spec. mise groups its entire `watch` passthrough set that way (31 uses, all in one file). That is the part worth noting: this was not a missing feature so much as a leak that was invisible because nothing downstream could have shown it. ## The spec side - `help_heading` on `SpecFlag` and `SpecArg`, as a property (`help_heading="Filtering"`) or a child node for longer text, written back out. - Mapped from clap's `get_help_heading()` for both flags and positionals. - Builder setters, docs in the flag and arg references, tests for the round trip and the clap conversion. - `usage-argv` carries it in both `FlagMeta` and `ArgMeta`, which is what prompted this: the derive cannot emit what the spec cannot express, so per the canonicality rule the spec went first. ## The rendering side Help output and generated markdown both group by heading now — a field nothing displays is half a feature. Grouping happens in the docs models, not in a template: Tera can filter on an attribute's *value* but cannot partition on one, and "everything without a heading" is not expressible as a filter. Behaviour: - Unheaded entries keep the default section title (`Flags:` / `Arguments:`) and come **first**. - Each heading gets its own section, in the order the headings first appear. - A heading with nothing *visible* in it produces no section, and a CLI that heads every flag gets no empty `Flags:`. - Positionals group too, since the spec field is on both. One thing worth knowing if you touch this: the groups hold clones, and `render_md` mutates the flag and arg lists *after* the model is built — so grouping at construction published copies without their rendered markdown. That cost me a debugging round; groups are rebuilt at the end of `render_md` and the method that does it says why. **Every existing snapshot is unchanged** — output is byte-identical when nothing has a heading — and four new ones cover the grouped case across both renderers, including hidden entries. `mise run render` produces no diff. *AI-assisted — Tool: Claude Code; model: anthropic/claude-fable-5; version: unavailable.* <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Presentational metadata and docs/help rendering only; parsing behavior is unchanged and output without headings remains identical. > > **Overview** > Adds **`help_heading`** to flags and positionals in the usage spec (KDL parse/serialize, builders, reference docs) and threads it through **`usage-argv`** metadata and KDL emission so derive output can record section titles losslessly. > > **Clap bridge fix:** `From<&clap::Arg>` now maps `get_help_heading()`, so grouped flags are no longer dropped when converting to a spec. > > **Rendering:** CLI help (short/long templates) and generated markdown partition flags and args by heading via `flag_groups` / `arg_groups` in the docs models. Unheaded items stay under default **Arguments** / **Flags** and appear first; custom headings follow in first-seen order; sections with only hidden entries are omitted. Markdown regroups after `render_md` so grouped clones keep rendered help text. > > Conformance roundtrip tests and new snapshot tests cover grouped help and global flags; existing output stays byte-identical when no headings are set. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit da412c7. 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 configurable help headings for flags and arguments. * Help and Markdown output now groups entries under their headings while preserving declaration order. * Empty sections and hidden entries are omitted from generated documentation. * Global and local flags are displayed in separate grouped sections. * **Documentation** * Added specification examples and reference documentation for configuring help headings. * **Bug Fixes** * Improved consistency between generated help output and Markdown documentation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Stacked on #802 (which is stacked on #801) — review those first; this diff includes them until they merge. The piece the last three PRs were building toward: one Rust type in, a parser and a spec out. ```rust /// A tool that does things #[derive(usage::Cli)] #[usage(bin = "ex", version = "1.0")] struct Cli { /// How many jobs to run at once #[usage(short = 'j', long, env = "EX_JOBS", default = "4")] jobs: Option<String>, /// Colorize output #[usage(long, negate = "--no-color", default = "true")] color: bool, /// Files to process files: Vec<String>, } ``` That gives you `Cli::parse_from(argv)`, `Cli::command()`, `Cli::spec()`, and `Cli::to_kdl()` — so the same declaration feeds `usage g markdown|manpage`, the completion generators, and grouped help output from #802. ## What's generated Three things, and the split is the design: - **`static` parse tables** — all a successful parse reads. - **`static` metadata** — what spec emission and help need, which a parse never touches. - **a parse function** — a `match` on table keys assigning straight into the struct's fields. No map to build and read back, nothing allocated that does not end up in the result. `command()` returns a `&'static`, so there is no command tree to construct before parsing starts. A test asserts the pointer is the same every call, which is the property the whole project exists for. A field with `long` or `short` is a flag; anything else is positional. Help comes from the doc comment — first paragraph short, whole comment long. ## Scope, stated plainly **One command per struct.** Subcommands need an enum of variants, cross-type table references, and a nested path through the parse function; that is its own PR and a box in `PLAN.md`. **Values are text** — `bool`, `String`, `Option<String>`, `Vec<String>`, or an unsigned integer with `count`. Converting to other types is also where `env`, required-ness, and `choices` get enforced, and that layer does not exist yet. So `Option<u32>` is a *compile error* explaining exactly that, rather than something that silently half-works. ## The error messages got real attention They are the surface an author actually interacts with, so: - `short = "j"` → *a short flag is a character: write `short = 'j'`* - a duplicate `--flag` → points at both declarations, second first - an argument after a variadic one → *can never be filled, because the variadic takes every remaining word* - an unknown option → lists the ones that exist - `count` on a `String` → says it has to be an unsigned integer ## Tests Twelve, over a deliberately awkward CLI: attached and bundled shorts, `--flag=value`, repeated flags, a negation turning off a default, a hidden flag that still parses, `--` passthrough, and a typo reported rather than bound. Then the same declaration is checked to emit a spec usage-lib accepts field by field, render as markdown and a manpage, and group by heading. Two more CLIs cover the empty cases — no flags, no positionals — which is where generated code tends to break on an unused variable or an empty `match`. Not published: a CLI framework that cannot express subcommands is not one to depend on by accident. The version tracks the workspace so it is ready the moment it can. *AI-assisted — Tool: Claude Code; model: anthropic/claude-fable-5; version: unavailable.* <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Large new proc-macro surface that defines CLI parsing behavior for adopters; mitigated by extensive conformance tests but still pre-v1 (no subcommands, limited types, env not enforced at parse time). > > **Overview** > Adds the **`usage-derive`** workspace crate and **`#[derive(Cli)]`**, so a single struct with `#[usage(...)]` attributes becomes a **`usage-argv` parser**, **static spec metadata**, and **KDL** for docs/completions. > > Generated code exposes **`parse_from` / `parse`**, **`command()`** and **`spec()`** as `&'static` tables, and **`to_kdl()`**. Parsing is a direct `match` on flag/arg keys into prefixed locals (avoids field-name clashes). The model layer rejects invalid declarations at compile time (duplicate flags, `var` vs `variadic`, unsupported types, dashed long/name normalization, etc.) with targeted errors. > > **v0 scope:** one command per struct; text-ish field types only (`bool`, `String`, `Option<String>`, `Vec<String>`, counting integers). Subcommands and typed value conversion are explicitly deferred in **PLAN.md**. > > **Conformance** gains **`conformance/tests/derive.rs`** end-to-end tests (parsing, spec round-trip, markdown/manpage/help). Release tooling includes **`usage-derive`** in publish and git-cliff paths. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 973a60a. 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 `usage-derive`, enabling CLI definitions through a `#[derive(Cli)]` macro. * Supports flags, positional arguments, defaults, aliases, repeatable values, negation, help text, environment settings, and generated command specifications. * Added parsing, help/documentation rendering, and KDL serialization for derived CLI types. * Added compile-time diagnostics for unsupported or invalid declarations. * **Documentation** * Documented supported attributes, value types, limitations, and the current roadmap. * **Tests** * Added comprehensive end-to-end coverage for parsing, help output, specifications, errors, and CLI configurations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 participant
Next step toward the derive: the cold half of what it will emit. Ticks two boxes in
PLAN.md.The split
A CLI knows more about itself than a parse needs.
usage-argv's tables hold what a token is matched against; everything else — help text, choices, defaults, effects, mounts — now lives in a parallel tree behind a newspecfeature.Two reasons, and the second is the one that matters:
FlagMeta.flag: &Flagmeans a flag's long and short forms have exactly one definition, so what the docs say and what the parser matches cannot drift apart. Nothing is duplicated between the two trees.A
debug_assertalso checks the two are in step, since they are written in the same order by construction.Emission
Spec::to_kdlwrites the spec by hand rather than through thekdlcrate — this crate has no dependencies and I would rather keep it that way for a cold path that only has to be correct.Correct is verified rather than eyeballed: 13 tests parse the output back with usage-lib and check the resulting
Specfield by field, over a fixture chosen to be awkward — help text containing a quote, multi-line long help, a hidden alias next to a visible one, a negated flag, choices on both a flag and an argument, everydouble_dashmode, both effect levels, a mount, a restart token, and two levels of nesting. Then it renders through usage-lib's markdown and manpage generators, which is the same code an adopter'susage gbuild runs, so a spec that parses but renders to nothing fails here instead of in someone's docs.There is also an insta snapshot of the emitted text, so a change to the writer has to be looked at rather than inferred from assertions still passing.
Writing the test found a real bug immediately: I had invented a
help_headingchild node that the spec does not have, and the whole document failed to parse.One thing deliberately missing
help_headinggroups flags under a heading in help output. clap has it, mise uses it (inwatch, for its vendored watchexec arguments), and the spec has no node for it. Rather than carry the field and silently drop it on the way out — which would make the emitted KDL a summary rather than a definition — it is absent, andPLAN.mdgains a "spec gaps found on the way" section proposing it. Per the canonicality rule, the spec gets extended first.Next
The derive itself, which generates both trees from a Rust type. This PR is what it will emit; the macro is the part that writes it for you.
AI-assisted — Tool: Claude Code; model: anthropic/claude-fable-5; version: unavailable.
Note
Low Risk
Large additive surface behind an off-by-default feature; usage-lib serialization changes are targeted at round-trip correctness for existing spec shapes.
Overview
Adds an optional
specfeature onusage-argvwith a cold metadata tree (Spec,CommandMeta,FlagMeta,ArgMeta) that borrows the existing parse tables (no duplicated flag names) andSpec::to_kdl()— dependency-free, hand-written KDL for docs/completions downstream.Conformance turns on
specand adds round-trip tests: parse emitted KDL with usage-lib, assert fields on an awkward fixture, insta snapshot stability, and markdown/manpage rendering.PLAN.mdmarks metadata + KDL emission done and documents deliberate omissions (help_heading, rootmount).usage-lib fixes KDL re-serialization so node arguments starting with
-and help text with control characters (e.g. ANSI) stay quoted/escaped and reparse correctly — aligned with the argv writer’s quoting rules.Reviewed by Cursor Bugbot for commit ce12211. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes