Skip to content

fix(cli): parse inline values for optional short flags - #31077

Closed
sjh9714 wants to merge 1 commit into
oven-sh:mainfrom
sjh9714:fix-config-short-flag-value
Closed

fix(cli): parse inline values for optional short flags#31077
sjh9714 wants to merge 1 commit into
oven-sh:mainfrom
sjh9714:fix-config-short-flag-value

Conversation

@sjh9714

@sjh9714 sjh9714 commented May 19, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes #21431.

Short optional CLI flags such as -c=custom.toml were parsed as bare flags before the short-option parser checked the inline = value. This makes the Values::OneOptional short-flag path return the value after = while preserving the existing DoesntTakeValue error for no-value flags like -a=value.

The regression test covers the reported config forms:

  • bun -c=custom.toml index.ts
  • bun -c=custom.toml run index.ts
  • bun run -c=custom.toml index.ts

How did you verify your code works?

  • USE_SYSTEM_BUN=1 bun test test/cli/bun.test.ts -t "-c=path loads bunfig" (fails on system Bun 1.3.14 with missing, confirming the regression)
  • git diff --check
  • PATH="$HOME/.cargo/bin:$PATH" SDKROOT="$(xcrun --show-sdk-path)" DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer RUSTFLAGS="-C link-arg=-isysroot -C link-arg=$(xcrun --show-sdk-path)" cargo test -p bun_clap short_params
  • build/debug/bun-debug test test/cli/bun.test.ts -t "-c=path loads bunfig"

Local caveat: my Homebrew LLVM 21 config points at a missing CommandLineTools MacOSX26.sdk, so the full bun bd test ... command needed local generated-build env fixes to use the installed Xcode SDK/Apple clang for host/Rust build edges. No generated files are included in this PR.

Note: I used Codex to help inspect the issue/code path and draft the tests, then reviewed the final diff and ran the listed verification commands locally.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c4c2a630-f0bc-4f85-983a-169ff906ac29

📥 Commits

Reviewing files that changed from the base of the PR and between 3dfca30 and 34cc82c.

📒 Files selected for processing (2)
  • src/clap/streaming.rs
  • test/cli/bun.test.ts

Walkthrough

The PR fixes short option argument parsing to handle optional values with = syntax. The parser is updated so that -c=path extracts path as the option value, and unit and integration tests verify both the parser change and its real-world behavior for the config flag.

Changes

Short option parsing with optional values

Layer / File(s) Summary
Parser logic and unit tests
src/clap/streaming.rs
When parsing a chained short option with OneOptional value semantics, the parser now extracts the substring after = and returns it as the argument value. Unit test fixture and assertions are extended to exercise -e=1 (parsed value) and -e (no value) cases.
Integration test for short config flag
test/cli/bun.test.ts
New test verifies -c=path works equivalently to --config=path by running Bun with a temporary config that preloads a side effect, checking both argument orderings produce correct output and exit code.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(cli): parse inline values for optional short flags' directly and clearly describes the main code change in the PR, which modifies the short option parser to handle inline values with = for optional flags.
Description check ✅ Passed The description follows the template with both required sections completed: 'What does this PR do?' explains the fix and links the issue, and 'How did you verify your code works?' lists comprehensive verification steps including tests and cargo commands.
Linked Issues check ✅ Passed The PR successfully addresses the linked issue #21431 by fixing the short flag -c= parsing to work with optional values, enabling forms like -c=custom.toml to properly set the config file as documented.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the reported issue: the streaming.rs modification handles the inline value parsing, and the test addition verifies the fix covers the reported use cases.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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 and usage tips.

robobun added a commit that referenced this pull request Aug 16, 2026
Adds the `bun run <flag> app.ts` and `bun <flag> run app.ts` argv shapes
from #31077 to the config-flag matrix, on top of the `bun <flag> app.ts`
shape that was already covered.

Co-authored-by: jinhyuk9714 <jinhyuk9714@gmail.com>
@robobun

robobun commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator

Thanks for tracking this down, and sorry for the slow response here.

This overlaps with #34983, which fixes the same -c=path case in StreamingClap::chainging and also covers the attached form (-cpath) and the space-separated forms (-c path, --config path) by making -c, --config a required-value flag in both the runtime and install param tables, so the path is no longer treated as a script to run or a package to install. Since it is the superset, we are keeping that one and closing this PR.

The argv shapes from your test (-c=... index.ts, -c=... run index.ts, run -c=... index.ts) are now part of the test matrix over there, with you credited as co-author on that commit.

@robobun robobun closed this Aug 16, 2026
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.

Bug: -c flag does not set config file for bun run should work like --config

2 participants