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

update `wp cli info` #4613

Merged
merged 4 commits into from Feb 15, 2018

Conversation

3 participants
@thrijith
Contributor

thrijith commented Jan 14, 2018

to update #4599
change output format to table : #4604 (comment)

not sure if this is what you expected @gitlost, please provide your inputs for any changes.

thrijith added some commits Jan 14, 2018

update 'info'
change output format to table
update 'info'
fix tests
@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera Jan 15, 2018

Member

I wonder if people use the output from wp cli info for scripting purposes. If so, the above is a breaking change. I'll ask in #cli to get some (limited) feedback.

Member

schlessera commented Jan 15, 2018

I wonder if people use the output from wp cli info for scripting purposes. If so, the above is a breaking change. I'll ask in #cli to get some (limited) feedback.

@gitlost

This comment has been minimized.

Show comment
Hide comment
@gitlost

gitlost Jan 15, 2018

Contributor

Yes, apologies for the misleading comment @thrijith, although the output does look a lot better (though could do without the header). Please note this isn't a high priority at the moment so won't be reviewed immediately and may not make 1.5.0.

Contributor

gitlost commented Jan 15, 2018

Yes, apologies for the misleading comment @thrijith, although the output does look a lot better (though could do without the header). Please note this isn't a high priority at the moment so won't be reviewed immediately and may not make 1.5.0.

@thrijith

This comment has been minimized.

Show comment
Hide comment
@thrijith
Contributor

thrijith commented Jan 15, 2018

@thrijith

This comment has been minimized.

Show comment
Hide comment
@thrijith

thrijith Jan 15, 2018

Contributor

no problem @gitlost will remove the header.

Contributor

thrijith commented Jan 15, 2018

no problem @gitlost will remove the header.

@gitlost

This comment has been minimized.

Show comment
Hide comment
@gitlost

gitlost Jan 15, 2018

Contributor

will remove the header.

@thrijith if instead you have time to investigate wp-cli/scaffold-command#109 and whether the output is compliant in other cases apart from the taxonomy one you fixed that would very much be appreciated!

Contributor

gitlost commented Jan 15, 2018

will remove the header.

@thrijith if instead you have time to investigate wp-cli/scaffold-command#109 and whether the output is compliant in other cases apart from the taxonomy one you fixed that would very much be appreciated!

@thrijith

This comment has been minimized.

Show comment
Hide comment
@thrijith

thrijith Jan 15, 2018

Contributor

ok @gitlost will work on it then

Contributor

thrijith commented Jan 15, 2018

ok @gitlost will work on it then

@gitlost

This comment has been minimized.

Show comment
Hide comment
@gitlost

gitlost Jan 25, 2018

Contributor

@thrijith am going to leave this PR out of 1.5.0 but will revisit shortly afterwards, ta!

Contributor

gitlost commented Jan 25, 2018

@thrijith am going to leave this PR out of 1.5.0 but will revisit shortly afterwards, ta!

@gitlost

This comment has been minimized.

Show comment
Hide comment
@gitlost

gitlost Feb 9, 2018

Contributor

@thrijith re headerless table, at the moment need to workaround limitations of Utils\format_items(), WP_CLI\Formatter and cli\Table so maybe do

			$table = new \cli\Table;
			$table->setRows( $info );
			$table->setRenderer( new \cli\table\Ascii );
			$lines = array_slice( $table->getDisplayLines(), 2 ); // Remove header and border.
			foreach ( $lines as $line ) {
				\WP_CLI::line( $line );
			}
Contributor

gitlost commented Feb 9, 2018

@thrijith re headerless table, at the moment need to workaround limitations of Utils\format_items(), WP_CLI\Formatter and cli\Table so maybe do

			$table = new \cli\Table;
			$table->setRows( $info );
			$table->setRenderer( new \cli\table\Ascii );
			$lines = array_slice( $table->getDisplayLines(), 2 ); // Remove header and border.
			foreach ( $lines as $line ) {
				\WP_CLI::line( $line );
			}

@schlessera schlessera requested a review from wp-cli/committers Feb 9, 2018

@gitlost

Thanks for sticking with this @thrijith ! Although technically a BC break I think anyone who is parsing this is hopefully using the json format.

(One would have to agree with code climate though that this function could do with a bit of a refactor - the json and table paths should share and return the same data (with different keys) for instance. Also getting the shell should probably be put in a new Utils function. But these are for another day!)

@gitlost gitlost merged commit fa2ba92 into wp-cli:master Feb 15, 2018

2 checks passed

codeclimate Approved by Alain Schlesser.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@gitlost gitlost added this to the 2.0.0 milestone Feb 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment