Fix uses of 'Plugin not found' string #1651
Conversation
westonruter
added
[Type] Bug
westonruter
requested review from
mukeshpanchal27 and
felixarntz
as code owners
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
| $transient_key = 'perflab_plugins_info-v2'; | ||
| $transient_key = 'perflab_plugins_info'; |
There was a problem hiding this comment.
This was changed in #1613 to account for the format change. Since the transient only lasts for 1 day anyway, we can now restore the original transient key.
| @@ -359,8 +369,13 @@ function perflab_install_and_activate_plugin( string $plugin_slug, array &$proce | |||
| } | |||
|
|
|||
| $plugins = get_plugins( '/' . $plugin_slug ); | |||
| if ( empty( $plugins ) ) { | |||
| return new WP_Error( 'plugin_not_found', __( 'Plugin not found.', 'default' ) ); | |||
| if ( count( $plugins ) === 0 ) { | |||
There was a problem hiding this comment.
The replacement of empty() with count() is part of #1219
| 'plugin_not_found', | ||
| __( 'Plugin not found.', 'default' ) . ' ' . | ||
| /* translators: %s is error code */ | ||
| sprintf( '(Error code: %d)', __LINE__ ) |
There was a problem hiding this comment.
I find the "error code" terminology confusing. It makes me think this is either an HTTP response code or an actual error code I can look up in some docs. Why not say "line"?
There was a problem hiding this comment.
Sure. Done in 49cb889
Co-authored-by: felixarntz <flixos90@git.wordpress.org>
There was a problem hiding this comment.
@westonruter On second thought, I think it would be better to provide contextual error messages. Providing which line an error was triggered seems a bit too low-level and points to the messages themselves not being helpful enough. Error lines should come from stack traces etc., it doesn't feel right to include them in the messages arbitrarily.
This means we can't use a Core translation string anymore, but that's not necessarily a bad thing. @swissspidy had mentioned somewhere else before that that's a bit fragile to do anyway (e.g. because Core strings can change, outside of our control).
I left suggestions for ways to make the messages more helpful below.
Co-authored-by: Felix Arntz <felixarntz@google.com>
|
Build for testing (3.6.0-alpha): performance-lab.zip |
I replied to a comment from @gordonrjones (maybe the same GitHub username?) on the WordPress support forum:
This PR seeks to improve the usage of the strings, including their disambiguation.
The error message now includes an "error code" which is the line number from where the error originated: