-
Notifications
You must be signed in to change notification settings - Fork 10
Extend wp sidebar command with additional subcommands #69
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
Extend wp sidebar command with additional subcommands #69
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. |
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request extends the wp sidebar command with get, exists, and widgets subcommands, which is a great addition for more complete sidebar management. The implementation is generally solid, but I've found a recurring bug in all new subcommands where wp_register_unused_sidebar() is not called. This will lead to failures when operating on the inactive widgets sidebar. I've also suggested restoring some documentation for the list command that was removed and a minor readability improvement in the widgets command. Overall, great work on expanding the command's functionality.
|
Thanks for your PR. Haven't looked at the code or the failing test yet, but a quick question: How is "wp sidebar widgets" different from the already existing "wp widget list" command? We don't want to duplicate functionality. |
|
|
I'm afraid that's incorrect. Have you tried using & comparing the two commands?
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
This is still lacking test coverage. If you could add Behat tests for these new commands I'll happily take a look. |
Description
This PR extends the
wp sidebarcommand with additional subcommands to make sidebar management more complete and consistent with other WP-CLI commands.New subcommands added
wp sidebar get <id>Retrieve details for a specific registered sidebar.
wp sidebar exists <id>Check whether a sidebar exists (returns appropriate exit codes).
wp sidebar widgets <id>List widgets assigned to a given sidebar.