fix(lib): validate every variadic fallback - #1049
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughFallback values now split by configured delimiters before validation. Argument and flag fallbacks preserve scalar or variadic output types. Tests cover environment, default, and conditional fallback values. ChangesFallback validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This localized validation change is merge-ready after normal checks and review; no actionable merge-blocking risk remains. Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8f02fed. Configure here.
Instruction counts
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 comparisonParsing
|

Summary
MultiStringContext
Follow-up to #1037 for the final incremental CodeRabbit finding that arrived as the original PR merged.
Test plan
cargo test --workspace --all-featurescargo clippy --workspace --all-features -- -D warningsdefault_ifvaluesThis PR description was AI-generated.
Note
Medium Risk
Changes core CLI fallback parsing and validation semantics; incorrect edge cases could affect apps using delimited env/defaults, though behavior is intentionally stricter with added tests.
Overview
Fixes variadic fallbacks (env, defaults,
default_if) so delimited strings are split and each piece is validated—not only the first value or one combined string.Fallback binding now uses
split_fallback_valueswith the arg’s delimiter, runsvalidate_choice_valueson the full list, enforcesvar_min/var_maxvia newvalidate_*_fallback_counthelpers, and storesMultiStringwhenvarapplies (including flag env paths whereflag.varorarg.varis set).bind_flag_fallbackis aligned so option flags with variadic args split and validate the same way; boolean-only variadic flags still map toMultiBool.Regression tests cover delimiter-split env/defaults, per-value expression errors, and
var_maxoverflow messages for positionals and flags.Reviewed by Cursor Bugbot for commit 44d5cd5. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit