Repository navigation
Accept plaintext as an alias of var_export in cli param-dump - #6339
Conversation
`plaintext` and `var_export` are the same plain-text output format under two names across WP-CLI commands. `cli param-dump` only honored `var_export`, so `--format=plaintext` silently fell through to JSON. Accept both names (and list `plaintext` in the documented options) so the format is consistent with the single-value formatters, which already alias the two. Adds a functional test asserting `--format=plaintext` emits the var_export representation. Refs wp-cli#4774
|
Hello! 👋 Thanks for opening this pull request! Please check out our contributing guidelines. We appreciate you taking the initiative to contribute to this project. Contributing isn't limited to just code. We encourage you to contribute in the way that best fits your abilities, by writing tutorials, giving a demo at your local meetup, helping other users with their support questions, or revising our documentation. Here are some useful Composer commands to get you started:
To run a single Behat test, you can use the following command: # Run all tests in a single file
composer behat features/some-feature.feature
# Run only a specific scenario (where 123 is the line number of the "Scenario:" title)
composer behat features/some-feature.feature:123You can find a list of all available Behat steps in our handbook. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesCLI parameter dump formatting
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.2.7)PHPStan was skipped because the config uses disallowed 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.
Pull request overview
This PR aligns wp cli param-dump with the rest of WP-CLI’s format naming by accepting plaintext as an alias of var_export, preventing --format=plaintext from falling back to JSON and documenting the alias.
Changes:
- Accept both
var_exportandplaintextforwp cli param-dump --format=.... - Document
plaintextas a valid--formatoption forcli param-dump. - Add a Behat scenario verifying
--format=plaintextemitsvar_export-style output.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| php/commands/src/CLI_Command.php | Accepts plaintext alongside var_export and documents the new option in the command help. |
| features/cli.feature | Adds an acceptance test ensuring --format=plaintext outputs the var_export representation (e.g. 'path' =>). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
schlessera
left a comment
There was a problem hiding this comment.
Approve. The alias is right: Formatter.php:367 and :370 already register var_export and plaintext on the same handler, and param-dump was the one place that knew only one name. Adding plaintext to the options: list (CLI_Command.php:996) and accepting both in the check (:1042) is the whole fix, and the scenario covers it.
One correction for the description: on main --format=plaintext does not "silently fall through to JSON". The options: list in the docblock makes the argument validator reject it before the command runs (Subcommand.php:552-586): Error: Parameter errors: Invalid value specified for 'format'. So this turns a hard error into working output, which is additive and safe. Please reword that sentence so the release notes do not describe a fallback that never happened.
Optional, not blocking: CLI_Command.php:983 still says "as JSON or in var_export format"; naming both aliases there keeps wp help cli param-dump in step with the options list.
Milestone 3.0.0. #4774 stays open: this covers param-dump only.
|
Good catch, and you're right, that's not what happens. I traced it through Appreciate you naming the exact lines. I'll leave the |
plaintextandvar_exportare two names for the same plain-text output format across WP-CLI. The single-value formatters already register both names to the same handler, butcli param-dumpdoes its ownvar_export( $spec )and only listedvar_exportin its documented--formatoptions.That meant
--format=plaintextwasn't silently doing anything wrong, it was rejected outright: WP-CLI validates a flag's value against itsoptions:list before the command ever runs, so it errored withInvalid value specified for 'format'instead of running at all.This accepts both names in
param-dumpand listsplaintextin the command's documented--formatoptions, matching the suggestion on #4774 to accept both wherever that format is used. It is backward compatible:var_exportkeeps working andplaintextnow works too.To verify:
wp cli param-dump --format=plaintextnow emits the PHPvar_exportrepresentation (e.g.'path' =>) instead of erroring. A functional test infeatures/cli.featurecovers it.Refs #4774
(full disclosure: AI helped me identify the issue and verify my work)