Accept plaintext as alias for var_export format#6286
Accept plaintext as alias for var_export format#6286dilipom13 wants to merge 16 commits intowp-cli:mainfrom
Conversation
|
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. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances WP-CLI's output formatting capabilities by introducing "plaintext" as a convenient alias for the "var_export" format. This change streamlines the user experience by providing a more intuitive option for developers who prefer a plain text representation of data that is still valid PHP. The implementation includes a new utility function for format detection, integration into core output mechanisms, updated command documentation, and comprehensive test coverage to ensure reliability. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces plaintext as an alias for the var_export output format, enhancing user experience by providing a more intuitive format name. The changes are consistently applied across various commands (cli alias list, cli param-dump) and the Formatter class. A new utility function Utils\is_var_export_format() centralizes the logic, and the new functionality is well-covered by Behat and PHPUnit tests.
My review includes a couple of suggestions to improve code clarity and maintainability.
I am having trouble creating individual review comments. Click here to see my feedback.
php/class-wp-cli.php (1189-1190)
This new elseif block has the same implementation as the one that follows on lines 1191-1192. This creates redundancy. To improve maintainability, you could combine both conditions into a single block.
For example, you could replace lines 1189-1192 with:
} elseif ( Utils\is_var_export_format( Utils\get_flag_value( $assoc_args, 'format' ) ) || is_array( $value ) || is_object( $value ) ) {
$_value = var_export( $value, true );
}php/utils.php (494)
The type hint for $format is mixed, which is too general. Based on its usage with Utils\get_flag_value(), this parameter will typically be a string or null. Using a more specific type hint like @param string|null $format would improve code clarity and help static analysis tools.
* @param string|null $format Format flag value from command arguments.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
No idea why there are so many test failures 🤔 |
|
Hello @swissspidy, again, I updated from my side. |
There was a problem hiding this comment.
Pull request overview
This PR standardizes output-format naming by treating plaintext as an alias for var_export across WP-CLI surfaces that already support var_export, and updates command synopses + Behat coverage accordingly.
Changes:
- Document
plaintextas a supported--formatoption forwp cli param-dumpandwp cli alias list. - Extend
WP_CLI::print_value()andWP_CLI\Formatterto acceptplaintext(and explicitly handlevar_exportas a format). - Add Behat scenarios verifying
plaintextworks forparam-dump,alias list, andFormatteroutput.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| php/utils.php | Updates format_items() docblock to include var_export/plaintext formats. |
| php/commands/src/CLI_Command.php | Adds plaintext to param-dump synopsis and treats it as var_export in the implementation. |
| php/commands/src/CLI_Alias_Command.php | Adds plaintext to wp cli alias list format options documentation. |
| php/class-wp-cli.php | Updates print_value() to recognize var_export/plaintext explicitly and refactors scalar printing into a helper. |
| php/WP_CLI/Formatter.php | Adds var_export/plaintext format handling for item output and single-item field output. |
| features/formatter.feature | Adds scenario coverage for --format=var_export and --format=plaintext via WP_CLI\Formatter. |
| features/cli.feature | Adds scenario coverage for wp cli param-dump --format=plaintext. |
| features/aliases.feature | Adds scenario coverage for wp cli alias list --format=plaintext. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| */ | ||
| $_value = Spyc::YAMLDump( $value, 2, 0 ); | ||
| $output = Spyc::YAMLDump( $value, 2, 0 ); | ||
| } elseif ( in_array( $format, [ 'var_export', 'plaintext' ], true ) ) { |
There was a problem hiding this comment.
The PR description mentions adding Utils\is_var_export_format() and using it in print_value(), but there is no such helper in the codebase and the format check is duplicated inline here. Consider adding the helper in WP_CLI\Utils (e.g., returning true for var_export/plaintext) and using it here (and in other updated call sites), or update the PR description if the helper was intentionally dropped.
|
Hello @swissspidy One file is failing. |
|
You can run |
Okay @swissspidy |
|
Hello @swissspidy Please review it. |
Fixes #4774
plaintextas an alias forvar_exportwherever that output format is used (per maintainer note).Utils\is_var_export_format()and use it inparam_dump,print_value, andFormatter.plaintextinwp cli param-dumpandwp cli alias listsynopses.Please review it.
Thanks, @schlessera, for the guidance.