Skip to content

fix(parse): align negative values across parse phases - #1318

Merged
jdx merged 1 commit into
mainfrom
fix/external-negative-flag-values
Aug 25, 2026
Merged

jdx merged 1 commit into
mainfrom
fix/external-negative-flag-values

Conversation

@jdx

@jdx jdx commented Aug 25, 2026

Copy link
Copy Markdown
Owner

Summary

  • use one detached-value eligibility rule during subcommand discovery and argument binding
  • prevent external_subcommand discovery from consuming a required flag's negative value
  • preserve attached short-bundle values and optional-value ambiguity rules
  • add regression coverage for --apps -1 with external_subcommand

Follow-up to #1317 and its final review feedback.

Testing

  • cargo test -p usage-lib --all-features
  • cargo fmt --all -- --check

This pull request was generated by Codex.


Note

Medium Risk
Changes core CLI argument parsing semantics for flags, optional values, and external subcommand discovery; incorrect rules would mis-bind arguments or break clap-aligned edge cases.

Overview
Unifies when a flag may consume the next token by introducing accepts_detached_flag_value and using it everywhere detached values are decided (prefix forwarding, allow_hyphen_values, optional/default-missing binding, and pending-flag binding) instead of duplicated is_flag_like / negative-number checks.

Tightens negative detached values for optional flags: tokens like -1 are only taken as a detached value when allow_negative_numbers is set, or when the flag has a required value (no default_missing and not value_optional). That keeps optional/default_missing flags from swallowing -1 during subcommand discovery while still allowing required flags and explicit allow_negative_numbers.

Skips the early hyphen-value path when attached_continuation is set, so attached short-bundle continuations are not mis-handled.

Adds a regression test for external_subcommand with --apps -1 so -1 binds to the flag instead of being treated as external input.

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

@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 23 minutes.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

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

Review profile: CHILL

Plan: Pro Plus

Run ID: 056597c0-5f9b-4b09-b47c-bf0caf481178

📥 Commits

Reviewing files that changed from the base of the PR and between 3279571 and e1a7e82.

📒 Files selected for processing (1)
  • lib/src/parse.rs

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.

@github-actions

Copy link
Copy Markdown
Contributor

Instruction counts

benchmark trend instructions Δ wall (min) Δ
markdown ▆▆▇█▅▆▆▁▂▁▂▃▃▂▃▃▃▃ 337,269,574 → 338,201,461 +0.28% 29.95 → 29.89ms -0.19%
startup ▃▃▃▄▁▁▁▃▃▃▃███████ 910,425 → 908,091 -0.26% 0.96 → 0.91ms -4.68%

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 stripped binary, bytes
usage 1321264
bpaf 2493280
clap 3101832
framework instructions, cold parse vs usage
usage 8309
clap 6315556 760x
bpaf 21908997 2636x
                                              min       p01       p10    median
usage-rs: argv -> struct                      431       436       451       468  ns
clap: build tree + parse -> struct         519309    519927    525722    538662  ns
bpaf: build parser + parse -> struct      1589180   1589180   1601509   1639823  ns

usage: argv -> struct                             437 ns      0.44 µs
clap: build tree + parse -> struct             535935 ns    535.93 µs
clap: parse -> struct, tree reused              24642 ns     24.64 µs
clap: build tree only                          323902 ns    323.90 µs

e1a7e8277c01 vs 32795712b5b2 · measured on the runner, not pushed to the history.

@jdx
jdx merged commit 680e11e into main Aug 25, 2026
10 checks passed
@jdx
jdx deleted the fix/external-negative-flag-values branch August 25, 2026 16:58
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