Add filter for plugins list status text #8294
Conversation
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
@Sukhendu2002 could you merge the latest trunk changes into this to see if that fixes the test issues? |
|
Test Report added to the Trac ticket: https://core.trac.wordpress.org/ticket/60495#comment:26 |
| * @param string $type The status slug being filtered. | ||
| */ | ||
| $text = apply_filters( 'plugins_list_status_text', '', $count, $type ); | ||
| if ( empty( $text ) ) { |
There was a problem hiding this comment.
| if ( empty( $text ) ) { | |
| if ( empty( $text ) || ! is_string( $text ) ) { |
| if ( empty( $text ) ) { | ||
| $text = $type; | ||
| } | ||
| $text .= ' ' . sprintf( |
There was a problem hiding this comment.
Maybe not strictly necessary but for sanity's sake:
| $text .= ' ' . sprintf( | |
| $text = esc_html( $text ) . ' ' . sprintf( |
| break; | ||
| default: | ||
| /** | ||
| * Filter the status text of default switch case in the Plugins list table. |
There was a problem hiding this comment.
Does this need to be capitalized?
| * Filter the status text of default switch case in the Plugins list table. | |
| * Filters the status text of default switch case in the plugins list table. |
| * @param string $text Plugins list status text. Default empty string. | ||
| * @param int $count Count the Number of plugins. | ||
| * @param string $type The status slug being filtered. |
There was a problem hiding this comment.
| * @param string $text Plugins list status text. Default empty string. | |
| * @param int $count Count the Number of plugins. | |
| * @param string $type The status slug being filtered. | |
| * @param string $text Plugins list status text. Default empty string. | |
| * @param int $count Count of the number of plugins. | |
| * @param string $type The status slug being filtered. |
| * @param int $count Count the Number of plugins. | ||
| * @param string $type The status slug being filtered. | ||
| */ | ||
| $text = apply_filters( 'plugins_list_status_text', '', $count, $type ); |
There was a problem hiding this comment.
Instead of checking for a string as suggested below, should we force a string value from the filter?
| $text = apply_filters( 'plugins_list_status_text', '', $count, $type ); | |
| $text = (string) apply_filters( 'plugins_list_status_text', '', $count, $type ); |
There was a problem hiding this comment.
That could result in a PHP warning if the filter returns an array: https://3v4l.org/XAR5P
There was a problem hiding this comment.
True. My thought was that, since WP core does this kind of casting already in numerous places, it would be safe to do since a warning doesn't stop execution, and it provides clues about incorrect code that doesn't provide expected results, whereas without it we kinda hide the problem. But as your example shows, having a string that says "Array" isn't very helpful either. Happy to withdraw my suggestion.
There was a problem hiding this comment.
I don't feel strongly about it. If someone uses this filter to return anything other than a string then they are doing it wrong anyway. There's probably a literal billion other places on WP where this kind of thing would also happen currently.
|
@Sukhendu2002 are you available to update your PR to address the review from Weston? |
|
Patch Refreshed in this PR #10133 |
4 participants
Trac ticket: https://core.trac.wordpress.org/ticket/60495
Patch refresh for: #6637
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.