fix(parse): allow hyphen-prefixed flag values

[jdx]

Summary

• add allow_hyphen_values to flag specs and builders
• let opted-in value flags consume pending values before short/long flag parsing
• add regression coverage for -a -destroy, --args=-destroy, and the unchanged default behavior

Closes #713.

Testing

• cargo test -p usage-lib hyphen_values
• cargo test -p usage-lib --all-features
• cargo clippy --all --all-features -- -D warnings

cargo test --all --all-features was also run; the Rust suites passed until existing Node-backed CLI example tests failed because node is not installed on PATH in this shell.

---

Note

Medium Risk
Touches core CLI parsing order for all specs, but behavior changes only when allow_hyphen_values is enabled; regression tests cover the main cases.

Overview
Opt-in flag values that look like flags — specs can set allow_hyphen_values=#true on value-taking flags (KDL, builders, and clap-derived specs). The parser treats the next -… token as that flag’s value before short/long flag parsing, including embedded forms like --args=-destroy and repeated variadic values.

Pending value handling is centralized in drain_pending_flag_values. complete-word’s --cword is regenerated with this setting so negative indices parse correctly. Flags without the option keep today’s behavior (e.g. -a -destroy still splits -destroy into other short flags).

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

Summary by CodeRabbit

• New Features
  • Flags and options can now accept values that start with a hyphen, improving support for inputs like -value and --args=-value.
  • Added a configurable setting to enable this behavior for specific options.
• Bug Fixes
  • Prevented hyphen-prefixed values from being misread as additional flags when this setting is enabled.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds an allow_hyphen_values boolean property to SpecFlag, exposed via a new builder method and supported in KDL property/child-node parsing, KDL serialization, and clap-based conversion. The parser now consumes hyphen-prefixed tokens as values for pending flags marked with this property, with corresponding tests.

Changes

allow_hyphen_values support

Layer / File(s)	Summary
SpecFlag schema, parsing, serialization, and builder lib/src/spec/flag.rs, lib/src/spec/builder.rs	Adds allow_hyphen_values: bool field to SpecFlag, parses it from KDL top-level properties and child nodes, emits it during KDL serialization, sets it from clap::Arg::is_allow_hyphen_values_set(), and adds a chainable SpecFlagBuilder::allow_hyphen_values method.
Parser handling of hyphen-leading values lib/src/parse.rs	parse_partial_with_env now consumes the next token as the value for pending flags with allow_hyphen_values set, even if it starts with -, validates choices, and stores the value in out.flags; adds a flag_string_value test helper and three tests validating the new behavior and prior fallback behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Parser as parse_partial_with_env
    participant Spec as SpecFlag

    User->>Parser: pass token starting with "-"
    Parser->>Parser: check flag_awaiting_value queue
    Parser->>Spec: check allow_hyphen_values
    alt allow_hyphen_values is true
        Spec-->>Parser: true
        Parser->>Parser: validate choices for token
        Parser->>Parser: store value in out.flags (String/MultiString)
    else allow_hyphen_values is false
        Spec-->>Parser: false
        Parser->>Parser: reinterpret token as flag(s)
    end
    Parser-->>User: return parsed flags

Loading

Poem

A hyphen hopped in, looking quite sly,
"Am I a flag?" it asked with a sigh.
The rabbit said "No, allow_hyphen says true,
You're just a value, straight through!"
🐇✨ Hop, parse, and carry on!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes add allow_hyphen_values support, handle hyphen-leading values in parsing, and cover the requested collision and = forms.
Out of Scope Changes check	✅ Passed	All code changes are directly related to the requested opt-in hyphen-value parsing and its tests; no unrelated scope is evident.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: parsing opt-in hyphen-prefixed flag values.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Greptile Summary

This PR introduces an opt-in allow_hyphen_values attribute for value-taking flags so that tokens like -destroy or --args=-destroy are consumed as the flag's value rather than being re-parsed as short or long flags. The parser change inserts a new early-exit block that fires before short/long flag handling when the awaiting flag has the attribute set; default behaviour is unchanged.

• lib/src/parse.rs: The old inline "drain pending flag values" body is extracted to a shared helper drain_pending_flag_values; a new block (guarded by w.starts_with('-') and flag.allow_hyphen_values()) runs it before flag parsing to short-circuit hyphen-prefixed tokens as values. Four regression tests cover the short-flag collision, embedded =-style long flag, variadic repetition, and unchanged default behaviour.
• lib/src/spec/flag.rs / builder.rs: allow_hyphen_values() is backed by double_dash == Automatic on the flag's inner arg; KDL serialisation, KDL parsing, the builder API, and clap conversion all set/read this consistently. The --cword flag in usage.usage.kdl is updated to opt in.

Confidence Score: 5/5

Safe to merge; the feature is opt-in and four regression tests cover all advertised scenarios without touching the default parsing path.

The core parsing change is a clean early-exit block that only fires when both the leading - and allow_hyphen_values conditions are met, leaving all non-opted-in flags on their original code path. The extracted drain_pending_flag_values helper is semantically equivalent to the inlined code it replaced. The two observations are design-level concerns that do not affect current callers.

lib/src/spec/flag.rs — the allow_hyphen_values() / double_dash == Automatic coupling is worth revisiting if SpecDoubleDashChoices grows additional variants or flag-arg double_dash semantics are used elsewhere.

Important Files Changed

Filename	Overview
lib/src/parse.rs	Extracts drain_pending_flag_values helper and adds a new hyphen-value early-exit block (guarded by starts_with('-') and allow_hyphen_values()) that fires before short/long flag parsing; four regression tests added covering all announced scenarios
lib/src/spec/flag.rs	Adds allow_hyphen_values() / set_allow_hyphen_values() using double_dash == Automatic as the backing store; KDL serialisation clones arg and resets double_dash to Optional to avoid emitting redundant attribute; clap conversion honours is_allow_hyphen_values_set()
lib/src/spec/builder.rs	Adds allow_hyphen_values bool to SpecFlagBuilder; arg() and build() both reconcile the double_dash state; allow_hyphen_values(true) without a subsequent .arg() call silently has no effect
cli/usage.usage.kdl	Opts --cword into allow_hyphen_values so negative-integer-like cword values are not mis-parsed as flags
docs/cli/reference/commands.json	Generated JSON updated to reflect --cword arg's double_dash changing from Optional to Automatic after allow_hyphen_values is applied

_{Reviews (3): Last reviewed commit: "fix(spec): preserve flag struct compatib..." | Re-trigger Greptile}

[coderabbitai]

🧹 Nitpick comments (1)

lib/src/parse.rs (1)

564-597: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consumption block duplicates the existing flag_awaiting_value drain logic.

Lines 569-594 are nearly identical to the existing drain at Lines 670-695 (choice validation, var vs scalar insert, w = ""). Only the guard differs. Consider extracting a shared helper (e.g. drain_pending_flag_values(&mut out, spec, w, custom_env)?) to keep the two paths from diverging.

Note the guard only inspects flag_awaiting_value.last(), yet the while loop drains every pending entry, assigning w to the last-pushed flag and "" to any older ones. This matches the existing block's semantics, so behavior is consistent, but it's worth confirming that's intended when multiple flags are pending (e.g. grouped shorts that each take a value).

🤖 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 `@lib/src/parse.rs` around lines 564 - 597, The `parse` function in
`lib/src/parse.rs` has duplicated `flag_awaiting_value` drain logic in the
consumption block, which should be consolidated to avoid drift between paths.
Extract the shared choice-validation and value-assignment sequence into a helper
(for example, a private `drain_pending_flag_values` used by both branches) and
keep the existing `var` versus scalar insertion behavior and `w = ""` handling
identical. Also review the `last()` guard versus draining the full stack in that
block to ensure the intended ordering remains correct for multiple pending
flags.

🤖 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.

Nitpick comments:
In `@lib/src/parse.rs`:
- Around line 564-597: The `parse` function in `lib/src/parse.rs` has duplicated
`flag_awaiting_value` drain logic in the consumption block, which should be
consolidated to avoid drift between paths. Extract the shared choice-validation
and value-assignment sequence into a helper (for example, a private
`drain_pending_flag_values` used by both branches) and keep the existing `var`
versus scalar insertion behavior and `w = ""` handling identical. Also review
the `last()` guard versus draining the full stack in that block to ensure the
intended ordering remains correct for multiple pending flags.

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: CHILL

Plan: Pro Plus

Run ID: 172b4b65-b7da-4609-a0fd-e752e717e077

📥 Commits

Reviewing files that changed from the base of the PR and between 67e05ff and 23c7e33.

📒 Files selected for processing (3)

• lib/src/parse.rs
• lib/src/spec/builder.rs
• lib/src/spec/flag.rs