Skip to content

fix(spec): split a clap default by the delimiter clap splits it by - #901

Merged
jdx merged 1 commit into
agent/double-dashfrom
agent/delimited-defaults
Aug 16, 2026
Merged

fix(spec): split a clap default by the delimiter clap splits it by#901
jdx merged 1 commit into
agent/double-dashfrom
agent/delimited-defaults

Conversation

@jdx

@jdx jdx commented Aug 15, 2026

Copy link
Copy Markdown
Owner

value_delimiter = ',' means clap splits every value before anyone sees it, defaults included — so default_value = "a,b,c" is three values and never the joined string. The conversion recorded the string.

mise's --fs-events is where it shows:

#[arg(long = "fs-events", default_value = "create,remove,rename,modify,metadata",
      value_delimiter = ',', value_name = "EVENTS")]
pub filter_fs_events: Vec<FsEvent>,

and the spec that came out:

flag --fs-events var=#true default=create,remove,rename,modify,metadata {
    arg <EVENTS> { choices access create remove rename modify metadata }
}

A default its own choices node forbids. Nothing read it strictly enough to complain, which is the only reason it went unnoticed; a reader that does check — and the derive on top of this stack now does — refuses the CLI outright.

The spec has no delimiter of its own. It has a list, which says the same thing, so the fix is to write the list. Only where clap declares a delimiter: "a,b" without one is a value containing a comma, and splitting on speculation would corrupt every path list.

Verification

Built mise against this branch and regenerated its spec; --fs-events comes out as

default {
    create
    remove
    rename
    modify
    metadata
}

every word one of the declared choices. benches/mise.usage.kdl is updated to that, in the commit above it, since that is where the shadow reads it.

Mutation: ignoring the delimiter fails a_delimited_default_becomes_the_values_clap_would_split_it_into. 404 lib tests green.

AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable.


Note

Low Risk
Narrow change to clap-to-spec metadata conversion with a targeted test; no runtime CLI parsing behavior in this PR.

Overview
Clap → spec conversion now records defaults the way clap actually applies them: when an arg has a value_delimiter, default strings are split into multiple spec default entries instead of one joined string.

A shared default_values helper drives both SpecArg and SpecFlag clap bridges, fixing cases like delimited multi-value flags whose recorded default violated their own choices (e.g. mise --fs-events). Without a delimiter, defaults stay a single string (commas preserved).

A regression test covers delimited vs non-delimited behavior.

Reviewed by Cursor Bugbot for commit 5cd2095. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Aug 15, 2026

Copy link
Copy Markdown

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: 83224fe1-fce8-4696-8460-50be1119c861

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

greptile-apps Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR aligns generated usage-spec defaults with clap’s declared value delimiter.

  • Adds a shared helper that splits clap defaults only when the argument declares a delimiter.
  • Uses the normalized defaults for both positional arguments and flags.
  • Adds regression coverage for delimited defaults and literal comma-containing defaults.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
lib/src/spec/arg.rs Adds shared delimiter-aware clap default conversion and applies it to positional arguments.
lib/src/spec/flag.rs Reuses the shared default conversion for flags.
lib/src/spec/mod.rs Adds regression coverage for delimited and non-delimited comma-containing defaults.

Reviews (4): Last reviewed commit: "fix(spec): split a clap default by the d..." | Re-trigger Greptile

@github-actions

github-actions Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Instruction counts

benchmark trend instructions Δ wall (min) Δ
markdown ▁▁▁▁▁▂▂▇█ 177,370,938 → 177,568,833 +0.11% 16.94 → 16.56ms -2.24%
startup ▁▁▇▁▁▇▁▂█ 1,221,836 → 1,221,988 +0.01% 1.04 → 1.09ms +4.57%

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.

Shadow comparison

Parsing mise use -g node@20 against a shadow of mise's committed spec.
Reported, not gated: the shadow grows as the derive learns to express more, so
what to watch is the ratio rather than either column.

usage clap ratio
instructions, cold parse 72182 5893587 81x
usage: argv -> struct                            1277 ns      1.28 µs
clap: build tree + parse -> struct             497855 ns    497.85 µs
clap: parse -> struct, tree reused              23046 ns     23.05 µs
clap: build tree only                          306996 ns    307.00 µs

5cd20957094d vs f68fef4b25ee · measured on the runner, not pushed to the history.

@jdx
jdx force-pushed the agent/delimited-defaults branch from 815779e to 0385fa6 Compare August 16, 2026 17:31
@jdx
jdx force-pushed the agent/delimited-defaults branch from 0385fa6 to ebd28db Compare August 16, 2026 17:56
`value_delimiter = ','` means clap splits every value before anyone sees it, defaults
included — so `default_value = "a,b,c"` is three values and never the joined string.
The conversion recorded the string.

mise's `--fs-events` is where it shows: `default_value = "create,remove,rename,modify,
metadata"`, a `value_parser` listing exactly those words as its choices, and a spec that
came out declaring a default its own `choices` node forbids. Nothing read it strictly
enough to complain, which is the only reason it went unnoticed; a reader that does check
— and the derive now does — refuses the CLI outright.

The spec has no delimiter of its own. It has a list, which says the same thing, so the
fix is to write the list. Only where clap declares a delimiter: `"a,b"` without one is a
value containing a comma, and splitting on speculation would corrupt every path list.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jdx
jdx force-pushed the agent/delimited-defaults branch from ebd28db to 5cd2095 Compare August 16, 2026 18:47
@jdx
jdx merged commit 5afbdf6 into main Aug 16, 2026
8 checks passed
@jdx
jdx deleted the agent/delimited-defaults branch August 16, 2026 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant