-
Notifications
You must be signed in to change notification settings - Fork 34
feat: add preparse hook for --flags-dir #1536
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
QA notes: bumped oclif/core and plugins-core in plugin-data => 3.2.1, create a file called ✅ with --json gives good json results ✅ empty boolean |
|
Fantastic 🤩 ! I'm not sure I understand what you mean here though:
Does this mean that it will be possible to create a json file in which the keys will be the flags, and the associated values will be their state? |
@Alfystar Unfortunately, no - the idea here is that you can have a directory that contains files, each of which map to a flag. For instance, you could run a deploy like this: Or with And the And each of those files contains the value for the flag (e.g. the None of those files require extensions EXCEPT if the contents of the file are going to be JSON. In that case you need to add the |
…e hook
The preparse hook was using ?? (nullish coalescing) instead of || (logical OR)
when checking if a CLI flag was already present. This caused a bug where CLI
arguments would not override flags-dir values when:
1. The flag has a short char defined (e.g., -l for --test-level)
2. The user specifies the long form on CLI (--test-level)
3. The same flag exists in flags-dir
With ??: (char && argv.includes('-l')) returns false, false ?? next returns false
With ||: (char && argv.includes('-l')) returns false, false || next evaluates next
This fix ensures CLI arguments properly override flags-dir values as documented
in PR salesforcecli#1536: "Flags/values provided by the user will override any flag/values
found by --flags-dir"
The eslint-disable is intentional because the lint rule incorrectly suggests ??
when || is actually required for proper boolean short-circuit evaluation.
…e hook The preparse hook was using ?? (nullish coalescing) instead of || (logical OR) when checking if a CLI flag was already present. This caused a bug where CLI arguments would not override flags-dir values in certain scenarios. Bug introduced in: c83f81a (2025-04-22, SF CLI 2.100.0) The change added a necessary guard (flagOptions.char &&) to prevent checking for undefined short chars, but incorrectly used ?? instead of || to chain the conditions. Since ?? only evaluates the right side when left is null/undefined, and false is neither, the subsequent checks were skipped. Affected scenarios (5 of 14 systematic test cases): - String flag with char defined, CLI uses long form (--test-level vs -l) - Boolean flag with char defined, CLI uses long form (--dry-run vs -d) - Boolean allowNo flag, CLI uses --no-<flag> form to override <flag> file - Boolean allowNo with char, CLI uses --<flag> to override no-<flag> file - Boolean allowNo with char, CLI uses --no-<flag> to override <flag> file Example: Flag defined as Flags.string({ char: 'l' }) with flags-dir containing a 'test-level' file. Running `sf cmd --test-level NoTestRun` should override the file value, but with ?? it was erroneously combined (conflict error). This fix restores the documented behavior from PR salesforcecli#1536: "Flags/values provided by the user will override any flag/values found by --flags-dir -- unless the flag supports `multiple`, in which case they will all be combined." The eslint-disable is intentional because the lint rule incorrectly suggests ?? when || is actually required for proper boolean short-circuit evaluation.
…e hook The preparse hook was using ?? (nullish coalescing) instead of || (logical OR) when checking if a CLI flag was already present. This caused a bug where CLI arguments would not override flags-dir values in certain scenarios. Bug introduced in: c83f81a (2025-04-22, SF CLI 2.100.0) The change added a necessary guard (flagOptions.char &&) to prevent checking for undefined short chars, but incorrectly used ?? instead of || to chain the conditions. Since ?? only evaluates the right side when left is null/undefined, and false is neither, the subsequent checks were skipped. Affected scenarios (5 of 14 systematic test cases): - String flag with char defined, CLI uses long form (--test-level vs -l) - Boolean flag with char defined, CLI uses long form (--dry-run vs -d) - Boolean allowNo flag, CLI uses --no-<flag> form to override <flag> file - Boolean allowNo with char, CLI uses --<flag> to override no-<flag> file - Boolean allowNo with char, CLI uses --no-<flag> to override <flag> file Example: Flag defined as Flags.string({ char: 'l' }) with flags-dir containing a 'test-level' file. Running `sf cmd --test-level NoTestRun` should override the file value, but with ?? it was erroneously combined (conflict error). This fix restores the documented behavior from PR salesforcecli#1536: "Flags/values provided by the user will override any flag/values found by --flags-dir -- unless the flag supports `multiple`, in which case they will all be combined." The eslint-disable is intentional because the lint rule incorrectly suggests ?? when || is actually required for proper boolean short-circuit evaluation.
What does this PR do?
Adds
preparsehook (depends on oclif/core#1005) to handle--flags-dirflag--flags-dirwill read files from the provided directory and insert them in the argv before the command's flags are parsed..jsonin order for it to correctly render the json in the argvno-<flag>, if the boolean flag supports it.--flag val1 --flag --val2 --flag --val3--flags-dir- unless the flag supportsmultiple, in which case they will all be combined to the final argv.--flags-diris not present--flags-dirdefinitionWhat issues does this PR fix or reference?
@W-15285609@
forcedotcom/cli#2346
forcedotcom/cli#2670
solves forcedotcom/cli#2260 (when PDR gets updated)