Skip to content

Conversation

@schlessera
Copy link
Member

@schlessera schlessera commented Jan 25, 2018

wp config list now produces a list of all config variables, constants and included files.

wp config get <key> now retrieves the value of the requested config key.

Note: Default behavior is to retrieve whatever key happens to fit, across both constants and variables. In case there's an ambiguous key that happens to exist as both a variable and a constant (which should be pretty rare), it will throw an error and ask for an explicit --type=<type> to disambiguate.

@schlessera
Copy link
Member Author

Fixes #41

README.md Outdated
### wp config get

Gets variables, constants, and file includes defined in wp-config.php file.
Get the value of a specific variable or constant defined in wp-config.php
Copy link
Contributor

Choose a reason for hiding this comment

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

Just first glance notice lack of third-person singular here and in list!

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed.

*/
public function get( $_, $assoc_args ) {
public function list_( $args, $assoc_args ) {
$path = Utils\locate_wp_config();
Copy link
Contributor

Choose a reason for hiding this comment

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

Another trivial one, there's a space at the beginning of the line here before the tab, and in 4 other places /^ \t/

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed. Copy/paste issue because I copied from a previous implementation I had already previously done.

Copy link
Contributor

@gitlost gitlost left a comment

Choose a reason for hiding this comment

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

Fab stuff. Made a few points but 2 are ignorable and the one about the test can be done later.

Edit: I'll leave you to merge. Have to go off now for a while.

public function list_( $args, $assoc_args ) {
$path = Utils\locate_wp_config();
if ( ! $path ) {
WP_CLI::error( "'wp-config.php' not found." );
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe here and for get, add a similar spiel as in Runner::check_wp_version() about Pass --path=path/to/wordpress

Copy link
Member Author

Choose a reason for hiding this comment

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


$strict = Utils\get_flag_value( $assoc_args, 'strict' );
if ( $strict && empty( $args ) ) {
WP_CLI::error( 'The --strict option can only be used in combination with a filter.' );
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a test to config-list.feature to invoke this error?

Copy link
Member Author

Choose a reason for hiding this comment

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

continue;
}

if ( false === strpos( $value['key'], $filter ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I see you have tests for case but was wondering should this case-insensitive as not strict? (Or maybe add a case-insensitive flag).

Copy link
Member Author

Choose a reason for hiding this comment

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

Case-insensitivity will probably open up a huge can of worms. I'd prefer to keep everything case-sensitive for now.

@schlessera schlessera merged commit e3d6749 into master Jan 25, 2018
@schlessera schlessera deleted the 41-restructure-config-retrieval branch January 25, 2018 22:36
schlessera added a commit that referenced this pull request Jan 6, 2022
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.

3 participants