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 a --pretty flag to WP-CLI get-mapping + docs #2653

Merged
merged 6 commits into from
Mar 28, 2022

Conversation

felipeelia
Copy link
Member

@felipeelia felipeelia commented Mar 14, 2022

Description of the Change

Currently, the get-mapping WP-CLI command just outputs the raw response from Elasticsearch, an unformatted JSON string. This PR introduces a --pretty flag, where the JSON is broken into several lines and receives some indentation, making things easier to read.

As this is a new feature, I'm adding it to the 4.1.0 milestone.

Changelog Entry

Added: new --pretty flag to the WP-CLI commands that output a JSON.

Credits

Props @felipeelia and @oscarssanchez

@oscarssanchez oscarssanchez self-requested a review March 21, 2022 21:26
oscarssanchez
oscarssanchez previously approved these changes Mar 21, 2022
Copy link
Contributor

@oscarssanchez oscarssanchez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, it would be good to add that flag to other methods as well.

@mckdemps mckdemps assigned felipeelia and unassigned oscarssanchez Mar 22, 2022
@felipeelia
Copy link
Member Author

@oscarssanchez added the flag to the other commands and assigning back to you for the final review. Thanks for the suggestion!

@@ -1023,8 +1039,9 @@ public function get_last_cli_index( $args, $assoc_args ) {
delete_site_option( 'ep_last_cli_index' );
}

WP_CLI::line( wp_json_encode( $last_sync ) );
$flag = ( ! empty( $assoc_args['pretty'] ) ) ? JSON_PRETTY_PRINT : null;
Copy link
Contributor

@oscarssanchez oscarssanchez Mar 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1042-1044 are similar to 1013-1015 and somewhat similar to 1471 and 1474 is there a way we could make it more DRY? maybe splitting print_json_response() into two functions, one that prints the JSON based on the pretty flag, and other that processes the response/option data ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about that and wondered if it wouldn't be an overengineering but you are right. I've pushed a new commit addressing that (and also passing arguments to get_indexing_status() as that wasn't working properly). Do you mind checking it again, @oscarssanchez ? Thanks!

@felipeelia felipeelia merged commit b1e62e8 into develop Mar 28, 2022
@felipeelia felipeelia deleted the feature/get-pretty-mapping branch March 28, 2022 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants