wp plugin update all doesnt display info which plugin is being updated

[kkmuffme]

The output for all plugins updated is always this:

Unpacking the package...
Installing the plugin...
Removing the old version of the plugin...
Plugin updated successfully.
Success: Installed 1 of 1 plugins.

Would be much more helpful if it at least said once which plugin it is updating in a separate line first.

[danielbachhuber]

Would be much more helpful if it at least said once which plugin it is updating in a separate line first.

I agree!

[i-am-chitti]

@kkmuffme / @danielbachhuber, The wp plugin update --all is already displaying the plugins being updated one by one and another table view for all updates. Refer - https://github.com/wp-cli/extension-command#wp-plugin-update

So, is it still a valid ticket here or any extension is required?

[danielbachhuber]

I think it would still be nice to indicate the plugin in the log output, e.g.:

$ wp plugin update --all
Enabling Maintenance mode...
Updating akismet
Downloading update from https://downloads.wordpress.org/plugin/akismet.3.1.11.zip...
Unpacking the update...
Installing the latest version...
Removing the old version of the plugin...
Plugin updated successfully.
Updating nginx-champuru
Downloading update from https://downloads.wordpress.org/plugin/nginx-champuru.3.2.0.zip...
Unpacking the update...
Installing the latest version...
Removing the old version of the plugin...
Plugin updated successfully.
Disabling Maintenance mode...

[i-am-chitti]

Got it. I inspected the codebase and found that this change need to be made in WP core codebase here https://github.com/WordPress/WordPress/blob/master/wp-admin/includes/class-plugin-upgrader.php#L378-L379 as messages are broadcasted from the core not the cli.

[swissspidy]

We should be able to override those strings simply by extending Plugin_Upgrader and overriding the upgrade_strings() / install_strings() methods.

[baizmandesign]

Hey there,

I've recently been trying to make some small contributions to the WP CLI project and stumbled upon this older issue.

I've created a rough work-in-progress and before going further wanted to pose a few questions and get feedback to determine how best to move forward:

• The previous comments say that we need to extend Plugin_Upgrader and over-ride upgrade_strings() and install_strings(). It was also necessary for me to over-ride the entire run() method of the Plugin_Upgrader class located in wp-admin/includes/class-wp-upgrader.php. While this worked, it introduced quite a bit of redundant code. Is there an approach that doesn't require so much duplication (~220 lines)?

• I'm unsure about where to store certain files and what to call them. I created a file named Verbose_Plugin_Upgrader.php and stored it in extension-command/src.

  • I'm pretty certain that filename isn't one we want to keep. Is there a naming convention that might help in this instance?
  • I'm uncertain whether the file ought to be stored in extension-command/src or extension-command/src/WP_CLI. What determines the storage location?

• A question: it appears to be the case that the $bulk property of Plugin_Upgrader is set to true when upgrading multiple plugins but not when installing multiple plugins. Why is that the case?

• Another question: the original GitHub issue was restricted to adding more verbose output to the upgrade command. Shouldn't we add it to the install command, too? My code takes a stab at this, but since the plugin slug isn't easily available when installing a plugin, the code outputs the ZIP filename, which may be confusing or undesirable ("Adding akismet.5.3.1.zip..."). (Of course, a slug might be "guessable" from a filename like my-plugin.1.2.3.zip, but is that format more or less consistent across all plugins?)

• In my WIP, I have not tested this for compatability with themes but intend to, provided we agree that I should continue moving forward. I have also not yet added tests or cleaned up the code formatting.

I can submit a pull request if it's easier to review.

Thanks!

Saul

[danielbachhuber]

@baizmandesign Thanks for diving into it!

The previous comments say that we need to extend Plugin_Upgrader and over-ride upgrade_strings() and install_strings(). It was also necessary for me to over-ride the entire run() method of the Plugin_Upgrader class located in wp-admin/includes/class-wp-upgrader.php. While this worked, it introduced quite a bit of redundant code. Is there an approach that doesn't require so much duplication (~220 lines)?

Oh wow, that's a lot of code. We don't want to fork core that substantially for something relatively inconsequential like this.

What made it necessary to override the entire run() method?

[baizmandesign]

Hi @danielbachhuber. The short answer regarding why it was necessary to override the run() method is that I didn't see a more "surgical" (narrowly targeted) approach. Hence my question: is there a better approach? I didn't see an action or filter to hook into.

[danielbachhuber]

@baizmandesign It looks like this before() method is called roughly when we'd like it to:

https://github.com/WordPress/wordpress-develop/blob/ee5142efbca11450e7befd202d37eba4660173c1/src/wp-admin/includes/class-wp-upgrader.php#L812

We can render whatever output we'd like in our custom UpgraderSkin:

https://github.com/wp-cli/wp-cli/blob/52751b1d1f8bdf0b47d38942ec199c7840ee1d85/php/WP_CLI/UpgraderSkin.php#L13

If that doesn't work for whatever reason, we could instead render some output on this filter:

https://github.com/WordPress/wordpress-develop/blob/ee5142efbca11450e7befd202d37eba4660173c1/src/wp-admin/includes/class-wp-upgrader.php#L796

I think both of these would be better than forking run().

[baizmandesign]

Hi @danielbachhuber,

I'm sorry I missed WP CLI Hack Day yesterday! But I haven't forgotten about this issue or the other one I messaged you about a few weeks ago.

Regarding this issue, it appears the path of least resistance is modifying the before() method in the UpgraderSkin class. After implementing some rudimentary code to get this working—please refer to this WIP—there are two issues with this approach:

1. While a previous comment you made suggested printing the plugin's slug, this information isn't available in the UpgraderSkin or Plugin_Upgrader objects. The UpgraderSkin object contains a property named $plugin_info, which contains the plugin header data as an array. The "Name" field is the closest thing to the slug I could find. Here's what some sample output looks like when updating two plugins:

$ wp plugin update accessibility-checker akismet
Updating Accessibility Checker...
Downloading update from https://downloads.wordpress.org/plugin/accessibility-checker.1.10.2.zip...
Unpacking the update...
Installing the latest version...
Removing the old version of the plugin...
Plugin updated successfully.
Updating Akismet...
Downloading update from https://downloads.wordpress.org/plugin/akismet.5.3.2.zip...
Unpacking the update...
Installing the latest version...
Removing the old version of the plugin...
Plugin updated successfully.
+-----------------------+-------------+-------------+---------+
| name                  | old_version | new_version | status  |
+-----------------------+-------------+-------------+---------+
| accessibility-checker | 1.8.1       | 1.10.2      | Updated |
| akismet               | 3.1.11      | 5.3.2       | Updated |
+-----------------------+-------------+-------------+---------+

2. It seems to be the case that all of the status messages that appear during the upgrade process are stored in the $strings property of the Plugin_Upgrader object. The text I'm outputting only lives in the before() method of the UpgraderSkin object. Should we add this text to the $strings property?

Saul

[danielbachhuber]

The "Name" field is the closest thing to the slug I could find. Here's what some sample output looks like when updating two plugins:

I think this could be fine. Can you share all of the data that's in the array?

It seems to be the case that all of the status messages that appear during the upgrade process are stored in the $strings property of the Plugin_Upgrader object. The text I'm outputting only lives in the before() method of the UpgraderSkin object. Should we add this text to the $strings property?

I don't fully grok the implications of this. Can you explain further?

[baizmandesign]

Can you share all of the data that's in the array?

Here is the contents of the $plugin_info array for a plugin being updated:

array(15) {
  ["Name"]=>
  string(21) "Accessibility Checker"
  ["PluginURI"]=>
  string(23) "https://a11ychecker.com"
  ["Version"]=>
  string(5) "1.8.1"
  ["Description"]=>
  string(114) "Audit and check your website for accessibility before you hit publish. In-post accessibility scanner and guidance."
  ["Author"]=>
  string(16) "Equalize Digital"
  ["AuthorURI"]=>
  string(27) "https://equalizedigital.com"
  ["TextDomain"]=>
  string(21) "accessibility-checker"
  ["DomainPath"]=>
  string(10) "/languages"
  ["Network"]=>
  bool(false)
  ["RequiresWP"]=>
  string(0) ""
  ["RequiresPHP"]=>
  string(0) ""
  ["UpdateURI"]=>
  string(0) ""
  ["RequiresPlugins"]=>
  string(0) ""
  ["Title"]=>
  string(21) "Accessibility Checker"
  ["AuthorName"]=>
  string(16) "Equalize Digital"
}

I don't fully grok the implications of this. Can you explain further?

My point was simply about organization. Strings seem to be stored all in one place (the $strings property), but we're not storing this string there. I was wondering whether, from an architectural perspective, that was wise, or slightly inconsistent. It also makes it harder to over-ride this string, though I suppose someone could just over-ride the before() method instead.

[baizmandesign]

Hi, @danielbachhuber. Any update on this issue? Did we still want to move forward implementing this idea? Thanks.

[BrianHenryIE]

There's a few options here.

1. Your existing WIP, wp-cli/wp-cli@main...baizmandesign:wp-cli:feature/plugin-update-info-261, needs to check isset( $this->plugin_info ) because that array doesn't seem to be available during a plain wp plugin install plugin-slug, and decode the special characters from the string
2. That array will, for .org plugins, have a PluginURI field where the slug can be found with preg_match('/https:\/\/wordpress.org\/plugins\/(.*?)\//', $input_line, $output_array);
3. Open a patch for Core to pass the Plugin_Upgrader::bulk_upgrade() $options array to WP_Upgrader_Skin::before() which you are using

I think option 1 is the best. Using the plugin Name is better than the slug since, as mentioned above, the slug is still present in the output in the filename.

wp-cli/wp-cli/php/WP_CLI/UpgraderSkin.php:88

public function before() {
    if( isset( $this->plugin_info ) ) {
        WP_CLI::log( sprintf( 'Updating %s...', htmlspecialchars_decode( $this->plugin_info['Name'] ) ) );
    }
}

wp-cli/extension-command/features/plugin-update.feature:267

  @require-wp-5.2
  Scenario: Updating all plugins should show the name of each plugin as it is updated
    Given a WP install
    And I run `wp plugin delete akismet`

    When I run `wp plugin install health-check --version=1.5.0`
    Then STDOUT should not be empty

    When I run `wp plugin install wordpress-importer --version=0.5`
    Then STDOUT should not be empty

    When I try `wp plugin update --all`
    Then STDOUT should contain:
      """
      Updating Health Check & Troubleshooting...
      """

    And STDOUT should contain:
      """
      Success: Updated 2 of 2 plugins.
      """

[baizmandesign]

@BrianHenryIE Thanks for your comment. My original question was related to the proper place to store strings, but it seems you're saying the install command should function similarly to the upgrade command. I suppose that's… fine. I can revise the WIP in the manner you recommend.

[BrianHenryIE]

I'm not saying it should, I was just noting that when testing that array was not available, so needs to be guarded against.