Run boolean flag fallbacks when the flag is absent - #7229
Open
WikiRik wants to merge 1 commit into
Open
Conversation
`Param.withFallbackConfig` and `Param.withFallbackPrompt` both trigger on the `MissingOption` / `MissingArgument` errors, which a boolean flag never produces: an absent boolean flag parses as `false`. Applying either combinator to a `Flag.boolean` was therefore a silent no-op, including in the example documented on `Flag.withFallbackConfig`. Both combinators now detect the boolean flag they read and consult the fallback when that flag is absent from the parsed flags, before parsing resolves it to `false`. The parser records aliases and `--no-` negations under the canonical flag name, so an explicit `--flag` or `--no-flag` still wins. Params wrapped by `optional` or `withDefault`, and those with `orElse` alternatives, keep supplying their own value. A missing config still falls back to `false`; a cancelled prompt fails with `MissingOption`, as it does for other flags. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: ea3d2f1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Quick second PR of the day, this one has more tests and does contradict with an earlier commit although I think this is for the better. Feel free to close/adjust if you don't agree.
The following is from the clanker again.
Flag.withFallbackConfigandFlag.withFallbackPromptare silent no-ops when applied to a boolean flag. Both fallbacks are driven by theMissingOption/MissingArgumenterrors, and an absent boolean flag never produces one — the parser resolves it tofalse, which is correct CLI behaviour on its own. The consequence is that the fallback simply never runs.The JSDoc on
Flag.withFallbackConfiguses a boolean flag as its only example, so this is the shape people copy:Nothing fails — a config value is just ignored, which is easy to ship and hard to notice.
Reproduction
One environment variable, one combinator, two outcomes depending only on the flag's primitive type.
The fix
Both combinators now detect the boolean flag a param reads and consult the fallback when that flag is absent from the parsed flags, before parsing resolves it to
false.No parser change was needed. The parser already canonicalizes aliases and records
--no-flagas"false"under the canonical flag name, so "no entry inParsedArgs.flags" is exactly "absent from the command line" — the same checkparseFlagalready makes before defaulting tofalse.Precedence is unchanged everywhere else:
--flagand--no-flagboth win over the fallback.optionalandwithDefaultsupply their own value first, so the fallback is not consulted — matching how they already shadow the fallback for non-boolean flags.orElsealternatives is left alone, since the alternative may supply the value.false; a cancelled prompt still fails withMissingOption.Open question for maintainers: should
withFallbackPromptchange too?The
withFallbackConfighalf is an unambiguous bug fix. ThewithFallbackPrompthalf is not, and I'd like your call on it.It reverses a deliberate decision from
2e3e4b246("Add Flag.withFallbackPrompt"), whose spec said:There was a passing test asserting exactly that, which this PR inverts.
The case for changing it: it is the identical no-op, for the identical reason. If
withFallbackConfigon a boolean flag should do something, it is hard to arguewithFallbackPromptshould not.The case against: a boolean flag is inherently optional, so "absent" is not really a missing value the way it is for a required string flag. Prompting on every invocation where
--flagwas omitted is a much more visible behaviour change than reading an environment variable.Happy to drop that half and keep this to
withFallbackConfig(the commit is easy to split), or to keep both. If they stay split, the alternative for the prompt side is to document the limitation instead — the JSDoc examples currently promise behaviour that does not happen.Testing
packages/effect/test/unstable/cli/Param.test.ts— 9 new cases across both combinators: the fallback fires when the flag is absent,--no-flagbeats the fallback, missing config yieldsfalse,withDefaultbeats the fallback, an unparseable config value becomesInvalidValue, and a cancelled prompt fails withMissingOption.With the source change reverted, 4 of them fail and 26 pass. The other 5 pass either way by design — they pin the precedence rules the fix must not disturb.
pnpm lintandpnpm checkare clean.🤖 Generated with Claude Code