Skip to content

feat(spec): bind a default when another flag is given - #1023

Merged
jdx merged 5 commits into
cursor/external-subcommand-66dbfrom
cursor/default-if-66db
Aug 18, 2026
Merged

jdx merged 5 commits into
cursor/external-subcommand-66dbfrom
cursor/default-if-66db

Conversation

@jdx

@jdx jdx commented Aug 18, 2026 •

Copy link
Copy Markdown
Owner

Stacked on #1021 (external_subcommand). Adds clap's default_value_if / default_value_ifs as default_if on the target flag.

Semantics

  • Two KDL/attribute arguments are ArgPredicate::IsPresent; three are Equals.
  • First matching condition wins.
  • Applied only when the target was not on the command line and has no env value.
  • Env is applied to every flag before any default_if, so a sibling env can activate the condition regardless of declaration order.
  • An applied default_if is a default: it satisfies requires and does not activate requires_if.
  • An Equals condition on a bool reads the negate form as "false" (--no-json), matching requires_if.
  • clap 4 has the setter and no getter, so the clap bridge leaves default_if empty (same hole as requires).
flag "--bin-names" {
  default_if "--json" "true"
  default_if "--output" "json" "pretty"
}
#[usage(long, default_if("--json", "true"))]
bin_names: bool,

Surfaces

  • Spec parse/emit, builder, usage-lib parse
  • Derive (default_if / default_ifs) and usage-argv FlagMeta
  • Go Meta.DefaultIf, Fill (skips unconditional default when DefaultIf is set), ApplyDefaultIf (takes the parser's negation map)
  • Corpus 11-default-if.json (layer: "post-binding")

value_parser ranges remain deferred. Multicall is the next stacked PR.

Open in Web Open in Cursor 

@coderabbitai

coderabbitai Bot commented Aug 18, 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: ddc58bfb-6a17-4b69-a579-d64a25cc0747

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.

Comment thread go/argv/post.go
Comment thread derive/src/codegen.rs Outdated
@github-actions

github-actions Bot commented Aug 18, 2026 •

Copy link
Copy Markdown
Contributor

Instruction counts

benchmark trend instructions Δ wall (min) Δ
markdown ▁▁▂▂▃▁▁▂▂█ 196,141,583 → 196,953,672 +0.41% 18.22 → 17.57ms -3.56%
startup █▄███████▁ 829,332 → 825,019 -0.52% 0.84 → 0.83ms -1.44%

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.

framework instructions, cold parse vs usage
usage 4220 —
argh 6292 1.5x
clap 5895248 1396x
bpaf 21917796 5193x
                                              min       p01       p10    median
usage-rs: argv -> struct                      198       201       204       207  ns
argh: argv -> struct                          278       283       292       301  ns
clap: build tree + parse -> struct         508644    512046    515812    521179  ns
bpaf: build parser + parse -> struct      1666119   1666119   1684545   1699706  ns

usage: argv -> struct                             219 ns      0.22 µs
clap: build tree + parse -> struct             506551 ns    506.55 µs
clap: parse -> struct, tree reused              24312 ns     24.31 µs
clap: build tree only                          308074 ns    308.07 µs

70eadff1198b vs df69276f650a · measured on the runner, not pushed to the history.

@cursor
cursor Bot marked this pull request as ready for review August 18, 2026 13:26
cursoragent and others added 3 commits August 18, 2026 13:35
Add default_if on the target flag, first-match-wins, matching clap's
default_value_if. Two arguments are IsPresent; three are Equals. Env is
applied to every flag first so a sibling env can activate the condition;
argv and env on the target suppress it. An applied default_if is a
default: it satisfies requires and does not activate requires_if.

Co-authored-by: jeff <jeff@jdx.dev>
ApplyDefaultIf always passed negated=false into RelationshipValues, so
an Equals condition of "false" never matched --no-json. Thread the
parser's negation map through, the same way requires_if already does.

Co-authored-by: jeff <jeff@jdx.dev>
Equals for a bool accepted 1/True/0/False, while requires_if and
usage-lib's argv path only compare canonical true/false. Tighten so the
same KDL binds in every implementation.

Co-authored-by: jeff <jeff@jdx.dev>
@cursor
cursor Bot force-pushed the cursor/default-if-66db branch from e7f5779 to c3bd092 Compare August 18, 2026 13:39

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c3bd092. Configure here.

Comment thread lib/src/parse.rs
cursoragent and others added 2 commits August 18, 2026 13:48
CI lint uses --all-targets. The Equals default_if fixture never
asserted `output`, and the new parse tests used get().is_none()
where contains_key is the form clippy wants.

Co-authored-by: jeff <jeff@jdx.dev>
Decide every default_if against argv and env first, then bind, then
apply unconditional defaults. Binding as we went put a default into
out.flags so the next flag treated it as explicit. Go Given() and the
derive __given_* flags already ignore defaults here.

Co-authored-by: jeff <jeff@jdx.dev>
@jdx
jdx merged commit b029b45 into main Aug 18, 2026
10 checks passed
@jdx
jdx deleted the cursor/default-if-66db branch August 18, 2026 15:05
jdx added a commit that referenced this pull request Aug 19, 2026
<!-- CURSOR_AGENT_PR_BODY_BEGIN -->
`external_subcommand` (#1021) and `default_if` (#1023) are in the
parser, the derive, and the corpus. PLAN.md still listed them as open
clap gaps.

This ticks those items, drops them from the fleet table, and splits
`multicall` from `no_binary_name` so the next PR can close one without
pretending the other landed. `no_binary_name` stays out of scope until a
fleet CLI needs it.

Also stops quoting a corpus vector count in PLAN.md and `go/README.md`,
matching the argv grammar page (#1024): agreement is measured on each
run rather than asserted as "154".

Stacked next: #1028 (`multicall`).

_This comment was generated by Claude Code._
<!-- CURSOR_AGENT_PR_BODY_END -->

<div><a
href="https://cursor.com/agents/bc-4de7e59e-aa2e-4ad0-a1d0-1c2e69c166db?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/open-in-web-light.png"><img
alt="Open in Web" width="114" height="28"
src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a>&nbsp;<a
href="https://cursor.com/background-agent?bcId=bc-4de7e59e-aa2e-4ad0-a1d0-1c2e69c166db&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img
alt="Open in Cursor" width="131" height="28"
src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a>&nbsp;</div>



<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added BusyBox-style multicall support, selecting subcommands from the
executable name.
* Added support for paths, Windows `.exe` suffixes, dispatcher names,
symlink-style invocations, and external subcommands.
* Added aliases, hidden choices, and optional case-insensitive value
matching.
* Added multicall configuration across supported integrations and
generated specifications.

* **Bug Fixes**
* Added validation for invalid multicall configurations and clearer
unknown-applet behavior.

* **Documentation**
* Documented multicall behavior, value choices, supported invocation
patterns, and conformance status.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
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.

2 participants