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

wp-cli-comform command #705

Merged
merged 5 commits into from Apr 2, 2015

Conversation

Projects
None yet
2 participants
@szepeviktor
Copy link
Contributor

commented Feb 25, 2015

Adding line comments...

@@ -1,6 +1,6 @@
<?php
/**
* Stream commands for WP-CLI
* Stream command for WP-CLI

This comment has been minimized.

Copy link
@szepeviktor

szepeviktor Feb 25, 2015

Author Contributor

only one

*
* @see WP_Stream_Query
* @subcommand query

This comment has been minimized.

Copy link
@szepeviktor

szepeviktor Feb 25, 2015

Author Contributor

redundant

*/
public function query( $args, $assoc_args ) {
self::connection();

This comment has been minimized.

Copy link
@szepeviktor

szepeviktor Feb 25, 2015

Author Contributor

no need to be static

$start = microtime( true );
$query_args = array();

This comment has been minimized.

Copy link
@szepeviktor

szepeviktor Feb 25, 2015

Author Contributor

removed timer, a wp-cli user is advanced enough to know what is happening, and why it takes time

This comment has been minimized.

Copy link
@fjarrett

fjarrett Apr 2, 2015

Contributor

@szepeviktor Fair enough. Better results can be seen with the time command anyway.

$ time wp stream query

foreach ( $assoc_args as $key => $value ) {
$query_args[ $key ] = $value;
if ( empty( $assoc_args['fields'] ) ) {
$fields = array( 'created', 'ip', 'author', 'author_meta.user_login', 'author_role', 'summary' );

This comment has been minimized.

Copy link
@szepeviktor

szepeviktor Feb 25, 2015

Author Contributor

Where to put the YAML how-to?
A separate wiki page for wp-cli?

This comment has been minimized.

Copy link
@fjarrett

fjarrett Mar 12, 2015

Contributor

Hey @szepeviktor, sorry for my absence - I just returned from paternity leave. Yeah, I think a separate wiki page for WP-CLI makes sense for that. Just created this placeholder: https://github.com/wp-stream/stream/wiki/WP-CLI-Command

This comment has been minimized.

This comment has been minimized.

Copy link
@fjarrett

This comment has been minimized.

Copy link
@szepeviktor

szepeviktor Apr 2, 2015

Author Contributor

Clear to merge?

*
* @return array $defaults
*/
$defaults = apply_filters( 'wp_stream_wp_cli_default_fields', (array) $defaults );

This comment has been minimized.

Copy link
@szepeviktor

szepeviktor Feb 25, 2015

Author Contributor

removed filtering

$records = wp_stream_query( $query_args );
if ( empty( $records ) ) {
WP_CLI::success( __( 'No results found.', 'stream' ) );
return;

This comment has been minimized.

Copy link
@szepeviktor

szepeviktor Feb 25, 2015

Author Contributor

An empty table is the usual wp-cli output.

}
$fields = array_map( 'trim', explode( ',', $query_args['fields'] ) );

This comment has been minimized.

Copy link
@szepeviktor

szepeviktor Feb 25, 2015

Author Contributor

on the comand line, it is very hard to append a SPACE character

$value = is_array( $record->$field ) ? implode( ', ', $record->$field ) : $record->$field;
$output .= $value;
$diff = absint( $longest - strlen( $value ) );
$formatter = new \WP_CLI\Formatter(

This comment has been minimized.

Copy link
@szepeviktor

szepeviktor Feb 25, 2015

Author Contributor

wp-cli way of displaying output

$found = count( $records );
$stop = microtime( true ) - $start;
$message = sprintf( _n( '1 result found in %s seconds.', '%s results found in %s seconds.', $found, 'stream' ), number_format( $found ), number_format( $stop, 4 ) );

This comment has been minimized.

Copy link
@szepeviktor

szepeviktor Feb 25, 2015

Author Contributor

the timer distracts the user and prevents processing of the output

This comment has been minimized.

Copy link
@fjarrett

fjarrett Apr 2, 2015

Contributor

I added the results count back in 1377185, it's helpful meta to know when dealing with large result sets.

$stop = microtime( true ) - $start;
$message = sprintf( _n( '1 result found in %s seconds.', '%s results found in %s seconds.', $found, 'stream' ), number_format( $found ), number_format( $stop, 4 ) );
WP_CLI::success( $message );

This comment has been minimized.

Copy link
@szepeviktor

szepeviktor Feb 25, 2015

Author Contributor

success should be detected by the return value as echo $? - we are on the command line,
a lot of shell prompts include this value or turn red when not zero

*
* @return void
*/
private static function connection() {
WP_CLI::line( __( 'Establishing secure connection with WP Stream...', 'stream' ) );

This comment has been minimized.

Copy link
@szepeviktor

szepeviktor Feb 25, 2015

Author Contributor

this prevents processing of the output, e.g. by grep

private static function connection() {
WP_CLI::line( __( 'Establishing secure connection with WP Stream...', 'stream' ) );
private function connection() {
$query = wp_stream_query( array( 'records_per_page' => 1, 'fields' => 'created' ) );
if ( ! $query ) {
WP_CLI::error( __( 'SITE IS DISCONNECTED', 'stream' ) );

This comment has been minimized.

Copy link
@szepeviktor

szepeviktor Feb 25, 2015

Author Contributor

this is necessary

@szepeviktor

This comment has been minimized.

Copy link
Contributor Author

commented Feb 25, 2015

This must have an least one error. Please review.

@szepeviktor

This comment has been minimized.

Copy link
Contributor Author

commented Feb 25, 2015

Has a very nice output. Do you need csv, json and count output?

+------------------------------+----------------+--------+------------------------+---------------+------------------------------------+
| created                      | ip             | author | author_meta.user_login | author_role   | summary                            |
+------------------------------+----------------+--------+------------------------+---------------+------------------------------------+
| 2015-02-25T18:07:20.709+0000 | 192.168.12.146 | 1      | viktor                 | administrator | "1" post published                 |
| 2015-02-25T18:07:18.045+0000 | 192.168.12.146 | 1      | viktor                 | administrator | "1" post drafted                   |
| 2015-02-25T18:04:51.063+0000 | 192.168.12.146 | 1      | viktor                 | administrator | "What's running" plugin activated  |
| 2015-02-25T18:04:01.200+0000 | 192.168.12.146 | 1      | viktor                 | administrator | Site connected to Stream           |
+------------------------------+----------------+--------+------------------------+---------------+------------------------------------+

*included

@szepeviktor

This comment has been minimized.

Copy link
Contributor Author

commented Feb 25, 2015

I am OK if you reject it. It could also be a separate wp-cli community command.
I've done these modifications based on the wp-cli contributor experience.
I hope it will not hurt any of you.

@szepeviktor szepeviktor referenced this pull request Feb 25, 2015

Closed

No filter for wp-cli #704

szepeviktor added some commits Feb 25, 2015

@fjarrett

This comment has been minimized.

Copy link
Contributor

commented Mar 12, 2015

@szepeviktor

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2015

Was it CodeSniffer?

@szepeviktor

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2015

I've done it for free. Could you give me a trial PRO licence?

@szepeviktor

This comment has been minimized.

Copy link
Contributor Author

commented Mar 12, 2015

All green.

@fjarrett

This comment has been minimized.

Copy link
Contributor

commented Mar 12, 2015

Thanks @szepeviktor. We are using the WordPress Coding Standards project to enforce coding style, which uses PHP_CodeSniffer.

Absolutely, please e-mail me your Stream Site ID frankie@wp-stream.com

fjarrett added a commit that referenced this pull request Apr 2, 2015

Merge pull request #705 from szepeviktor/develop
wp-cli-comform command

@fjarrett fjarrett merged commit 147029d into xwp:develop Apr 2, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@fjarrett

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2015

@szepeviktor Nicely done! Thanks for these many improvements. Please do shoot me an email when you can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.