Skip to content
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

Add json and csv formats to upgrade output #2452

Merged
merged 7 commits into from Feb 5, 2016

Conversation

@gilbitron
Copy link
Contributor

commented Feb 5, 2016

Resolves #2415

@gilbitron

This comment has been minimized.

Copy link
Contributor Author

commented Feb 5, 2016

@danielbachhuber While putting together this PR I realised that the output of --format=json and --format=csv should probably not include the progress updates. For example:

Enabling Maintenance mode...
Downloading update from https://downloads.wordpress.org/theme/twentyfifteen.1.4.zip...
Using cached file '/Users/gilbitron/.wp-cli/cache/theme/twentyfifteen-1.4.zip'...
Unpacking the update...
Installing the latest version...
Removing the old version of the theme...
Theme updated successfully.
Disabling Maintenance mode...
Success: Updated 1/1 themes.

Any ideas how we would get around this?

@danielbachhuber

This comment has been minimized.

Copy link
Member

commented Feb 5, 2016

should probably not include the progress updates

Correct.

Any ideas how we would get around this?

Most of the output is from WP_CLI\UpgraderSkin. Given we can't pass much context to the callbacks, our next best bet would be to assign a class-level variable to the object.

This worked well in my testing:

diff --git a/php/WP_CLI/CommandWithUpgrade.php b/php/WP_CLI/CommandWithUpgrade.php
index f7da951..6c27d65 100755
--- a/php/WP_CLI/CommandWithUpgrade.php
+++ b/php/WP_CLI/CommandWithUpgrade.php
@@ -249,6 +249,7 @@ abstract class CommandWithUpgrade extends \WP_CLI_Command {
                                $cache_manager->whitelist_package($item['update_package'], $this->item_type, $item['name'], $item['update_version']);
                        }
                        $upgrader = $this->get_upgrader( $assoc_args );
+                       $upgrader->cli_output_format = ! empty( $assoc_args['format'] ) ? $assoc_args['format'] : '';
                        $result = $upgrader->bulk_upgrade( wp_list_pluck( $items_to_update, 'update_id' ) );
                }

diff --git a/php/WP_CLI/UpgraderSkin.php b/php/WP_CLI/UpgraderSkin.php
index aa56b43..b2b3d0f 100644
--- a/php/WP_CLI/UpgraderSkin.php
+++ b/php/WP_CLI/UpgraderSkin.php
@@ -41,7 +41,9 @@ class UpgraderSkin extends \WP_Upgrader_Skin {

                $string = str_replace( '…', '...', strip_tags( $string ) );

-               \WP_CLI::line( $string );
+               if ( ! empty( $this->cli_output_format ) && ! in_array( $this->cli_output_format, array( 'csv', 'json' ) ) ) {
+                       \WP_CLI::line( $string );
+               }
        }
 }
@gilbitron

This comment has been minimized.

Copy link
Contributor Author

commented Feb 5, 2016

@danielbachhuber Almost there. However when using --format=json and --format=csv I'm still getting this single line of output before the json/csv:

Using cached file '/Users/gilbitron/.wp-cli/cache/theme/twentyfifteen-1.4.zip'...

Any ideas how to mute that?

@gilbitron

This comment has been minimized.

Copy link
Contributor Author

commented Feb 5, 2016

Also my tests are looking for incomplete data because of the fact they don't know what the "upgrading to" version will be. Any ideas how to get around that? Is there a way we can find the "upgrade to" version and do something like And save STDOUT as {UPGRADE_VERSION}?

@danielbachhuber

This comment has been minimized.

Copy link
Member

commented Feb 5, 2016

However when using --format=json and --format=csv I'm still getting this single line of output before the json/csv:

Ok. Let's go with a different approach. Early in the update() method, let's do:

if ( ! empty( $assoc_args['format'] ) && in_array( $assoc_args['format'], array( 'csv', 'json' ) ) ) {
    $logger = new \WP_CLI\Loggers\Quiet;
    WP_CLI::set_logger( $logger )
}

We'll need to make sure all logging lines use WP_CLI::log() instead of WP_CLI::line()

Any ideas how to get around that? Is there a way we can find the "upgrade to" version and do something like And save STDOUT as {UPGRADE_VERSION}?

When I run `wp theme list --name=twentyfifteen --field=update_version`
Then save STDOUT as {UPDATE_VERSION}`
Then STDOUT should not be empty

When I run `wp theme update twentyfifteen --format=json --dry-run`
Then STDOUT should contain:

This comment has been minimized.

Copy link
@danielbachhuber

danielbachhuber Feb 5, 2016

Member

Let's use Then STDOUT should be JSON containing:, which will ensure it's valid JSON.

Scenario: When updating a theme --format should be the same when using --dry-run
Given a WP install

When I run `wp theme update twentyfifteen --version=1.0`

This comment has been minimized.

Copy link
@danielbachhuber

danielbachhuber Feb 5, 2016

Member

Can we use wp theme install --force twentyfifteen --version=1.0, for versions of WordPress where twentyfifteen isn't installed by default? Also, we may want to use twentytwelve to ensure compatibility down to WP 3.7

Then STDOUT should not be empty

When I run `wp theme update twentyfifteen --format=csv --dry-run`
Then STDOUT should contain:

This comment has been minimized.

Copy link
@danielbachhuber

danielbachhuber Feb 5, 2016

Member

Same as above: Then STDOUT should be CSV containing:

Then STDOUT should not be empty

When I run `wp theme update twentyfifteen --format=json`
Then STDOUT should contain:

This comment has been minimized.

Copy link
@danielbachhuber

danielbachhuber Feb 5, 2016

Member

Same as above.

Then STDOUT should not be empty

When I run `wp theme update twentyfifteen --format=csv`
Then STDOUT should contain:

This comment has been minimized.

Copy link
@danielbachhuber

danielbachhuber Feb 5, 2016

Member

Same as above.

@@ -41,7 +41,9 @@ function feedback( $string ) {
$string = str_replace( '…', '...', strip_tags( $string ) );
\WP_CLI::line( $string );
if ( empty( $this->upgrader->cli_output_format ) || ! in_array( $this->upgrader->cli_output_format, array( 'csv', 'json' ) ) ) {
\WP_CLI::line( $string );

This comment has been minimized.

Copy link
@danielbachhuber

danielbachhuber Feb 5, 2016

Member

If we use \WP_CLI::log() here, we won't need to assign $this->upgrader->cli_output_format

\WP_CLI::warning( $line );
} else {
\WP_CLI::error( $line );
if ( empty( $assoc_args['format'] ) || ! in_array( $assoc_args['format'], array( 'json', 'csv' ) ) ) {

This comment has been minimized.

Copy link
@danielbachhuber

danielbachhuber Feb 5, 2016

Member

Shouldn't need the format check here if we're using the Quiet logger.

gilbitron added 2 commits Feb 5, 2016
@gilbitron

This comment has been minimized.

Copy link
Contributor Author

commented Feb 5, 2016

@danielbachhuber Done. Only issue I had was using variables like {UPDATE_VERSION} in tables (STDOUT should be CSV containing) didn't seem to work.

@danielbachhuber

This comment has been minimized.

Copy link
Member

commented Feb 5, 2016

🍑 Thanks!

danielbachhuber added a commit that referenced this pull request Feb 5, 2016
Merge pull request #2452 from gilbitron/theme-update-format
Add json and csv formats to upgrade output

@danielbachhuber danielbachhuber merged commit a8a1939 into wp-cli:master Feb 5, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.