Accept plaintext as alias for var_export format#6286
Accept plaintext as alias for var_export format#6286dilipom13 wants to merge 5 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.
php/utils.php
Outdated
| * @param mixed $format Format flag value from command arguments. | ||
| * @return bool | ||
| */ | ||
| function is_var_export_format( $format ) { |
There was a problem hiding this comment.
We don't really need an extra function for this one line check. Let's just inline this everywhere
There was a problem hiding this comment.
Okay @swissspidy, but still I am confused.
Why are so many files failing
I think I need to check more.
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. |
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.