Skip to content

Fix: handle valueless boolean CLI flags at compile time#1842

Merged
kickster97 merged 2 commits intomainfrom
config-fixes
Apr 8, 2026
Merged

Fix: handle valueless boolean CLI flags at compile time#1842
kickster97 merged 2 commits intomainfrom
config-fixes

Conversation

@kickster97
Copy link
Copy Markdown
Member

@kickster97 kickster97 commented Apr 7, 2026

WHAT is this pull request doing?

Valueless boolean CLI flags like --clustering and --raise-gc-warn are broken. OptionParser passes an empty string for flags without =VALUE, and parse_value("", Bool) evaluates to false instead of true.

Fixed by detecting valueless boolean flags at compile time in the parse_cli macro and assigning the correct literal (true, or false for --no- prefixed flags) instead of always going through parse_value.

maybe this code is a bit clunky? I'd love to discuss if there are more elegant solutions

HOW can this pull request be tested?

run updated specs

@kickster97 kickster97 requested a review from a team as a code owner April 7, 2026 07:30
@kickster97 kickster97 added this to the 2.7.0 milestone Apr 7, 2026
@spuun
Copy link
Copy Markdown
Member

spuun commented Apr 7, 2026

Try

      @[CliOpt("", "--clustering", "Enable clustering", ->(String) { true }, section: "clustering")]

@claude
Copy link
Copy Markdown

claude bot commented Apr 7, 2026

No issues found.

The fix correctly uses explicit Proc value parsers for the three valueless boolean CLI flags (--no-data-dir-lock, --raise-gc-warn, --clustering). The approach is sound: when OptionParser passes an empty string for valueless flags, the lambda returns the correct literal value instead of going through parse_value("", Bool) which would evaluate to false. Specs cover the new behavior.

@kickster97
Copy link
Copy Markdown
Member Author

Try

      @[CliOpt("", "--clustering", "Enable clustering", ->(String) { true }, section: "clustering")]

Think that makes for a better solution!

@kickster97 kickster97 merged commit 561c103 into main Apr 8, 2026
17 of 18 checks passed
@kickster97 kickster97 deleted the config-fixes branch April 8, 2026 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants