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

Use `--constant=<constant>` or `--global=<global>` to get the value of a specific constant or global #16

Merged
merged 4 commits into from Jun 19, 2017

Conversation

3 participants
@lucatume
Contributor

lucatume commented Jun 15, 2017

Implements the feature of #15.

I've put in place a way to know if the user mistype a legit constant or global: in that case an error will be shown; I could use some feedback about that.

Show outdated Hide outdated features/config-get-field.feature
Show outdated Hide outdated features/config-get-field.feature
Show outdated Hide outdated features/config-get-field.feature
Show outdated Hide outdated features/config-get-field.feature
Show outdated Hide outdated features/config-get-field.feature
Show outdated Hide outdated features/config-get-field.feature
Show outdated Hide outdated features/config-get-field.feature
Show outdated Hide outdated features/config-get-field.feature
Show outdated Hide outdated features/config-get-field.feature
Show outdated Hide outdated features/config-get-field.feature
@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera Jun 15, 2017

Member

Thanks for the pull request. As discussed, I only have minor nitpicks, and then it is ready to be merged.

Member

schlessera commented Jun 15, 2017

Thanks for the pull request. As discussed, I only have minor nitpicks, and then it is ready to be merged.

@danielbachhuber

Thanks for the pull request, @lucatume ! There are a few more things to clean up.

Show outdated Hide outdated src/Config_Command.php
Show outdated Hide outdated src/Config_Command.php
@@ -224,6 +230,14 @@ public function get( $_, $assoc_args ) {
$wp_config_vars = self::get_wp_config_vars( get_defined_vars(), $wp_cli_original_defined_vars, 'variable', array( 'wp_cli_original_defined_vars' ) );
$wp_config_constants = self::get_wp_config_vars( get_defined_constants(), $wp_cli_original_defined_constants, 'constant' );
$get_constant = ! empty( $assoc_args['constant'] );
$get_global = ! empty( $assoc_args['global'] );

This comment has been minimized.

@danielbachhuber

danielbachhuber Jun 15, 2017

Member

If both $get_constant and $get_global are provided, we should return an error. This error condition should also have a test.

@danielbachhuber

danielbachhuber Jun 15, 2017

Member

If both $get_constant and $get_global are provided, we should return an error. This error condition should also have a test.

Show outdated Hide outdated src/Config_Command.php
Show outdated Hide outdated src/Config_Command.php
Show outdated Hide outdated src/Config_Command.php
@lucatume

This comment has been minimized.

Show comment
Hide comment
@lucatume

lucatume Jun 19, 2017

Contributor

I've updated the code as requested, guess it's good for another review.

Contributor

lucatume commented Jun 19, 2017

I've updated the code as requested, guess it's good for another review.

@danielbachhuber

Looking good! Just a few more small tweaks.

Show outdated Hide outdated src/Config_Command.php
Show outdated Hide outdated src/Config_Command.php
Show outdated Hide outdated src/Config_Command.php
@lucatume

This comment has been minimized.

Show comment
Hide comment
@lucatume

lucatume Jun 19, 2017

Contributor

Ok, more fixes in.
I've added quotes (') around the constants/globals too; the idea is to stick with (what I get as) the standard. Let me know if it's ok or there is more work to do.

Contributor

lucatume commented Jun 19, 2017

Ok, more fixes in.
I've added quotes (') around the constants/globals too; the idea is to stick with (what I get as) the standard. Let me know if it's ok or there is more work to do.

Review is no longer applicable; outdated code

@danielbachhuber danielbachhuber merged commit cb6a4a5 into wp-cli:master Jun 19, 2017

1 check passed

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

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Jun 19, 2017

Member

Great work, @lucatume — thanks for your persistence!

Member

danielbachhuber commented Jun 19, 2017

Great work, @lucatume — thanks for your persistence!

@danielbachhuber danielbachhuber changed the title from add first version of constant and global getting code to Use `--constant=<constant>` or `--global=<global>` to get the value of a specific constant or global Jun 19, 2017

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