Skip to content

Add runtime_args to options array for runcommand#5722

Merged
schlessera merged 5 commits intowp-cli:mainfrom
mrsdizzie:main
Jan 18, 2023
Merged

Add runtime_args to options array for runcommand#5722
schlessera merged 5 commits intowp-cli:mainfrom
mrsdizzie:main

Conversation

@mrsdizzie
Copy link
Member

@mrsdizzie mrsdizzie commented Jan 10, 2023

Per https://wordpress.slack.com/archives/C02RP4T41/p1673268015148999?thread_ts=1672934225.683679&cid=C02RP4T41

I have a command that calls WP_CLI::runcommand a lot and I want to set some default global options for most of these (particularly --skip-themes --skip-plugins). I also have default global options for WP_CLI::runcommand itself.

Currently, this ends up looking something like:

private $default_cli_opts = array(
	'return'       => 'all',
	'exit_error'   => false,
);

private $default_command_opts = '--skip-themes --skip-plugins';

WP_CLI::runcommand( "config has set WPCACHEHOME --path=$path " . $this->default_command_opts, $this->default_cli_opts );
etc...

It would be nice to be able to keep these options in a single place like:

private $default_cli_opts = array(
	'return'       => 'all', // returns an OBJECT
	'exit_error'   => false,
	'runtime_args' => array( '--skip-themes', '--skip-plugins' ),
);

WP_CLI::runcommand( "config has set WPCACHEHOME --path=$path", $this->default_cli_opts );

This PR just appends the options to the command as if the caller included them in the command directly.

I didn't see any place that runcommand was tested as if being called from a script so I wasn't sure where/how to add a test for this as it isn't accessible when using wp-cli directly. Happy to add one though

@mrsdizzie mrsdizzie requested a review from a team as a code owner January 10, 2023 15:49
danielbachhuber
danielbachhuber reviewed Jan 10, 2023
View reviewed changes
Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

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

Tests should be great, thanks! features/runcommand.feature would be a good place for them.

@mrsdizzie
Copy link
Member Author

mrsdizzie commented Jan 11, 2023

Tests should be great, thanks! features/runcommand.feature would be a good place for them.

I added a test. I wasn't familiar with behat before this so any suggestions welcome.

@mrsdizzie
Copy link
Member Author

mrsdizzie commented Jan 11, 2023

While working on this I was thinking another helpful feature might be to include the full command run as part of the object returned here:

wp-cli/php/class-wp-cli.php

Lines 1402 to 1405 in 187eda2

$retval = (object) [
'stdout' => trim( $stdout ),
'stderr' => trim( $stderr ),
'return_code' => $return_code,

So there would be a way to see what this was:

wp-cli/php/class-wp-cli.php

Line 1335 in 187eda2

$runcommand = "{$php_bin} {$script_path} {$runtime_config} {$command}";

Right now I don't think there is a way to see the full command that is run?

schlessera
schlessera requested changes Jan 17, 2023
View reviewed changes
Copy link
Member

@schlessera schlessera left a comment

Choose a reason for hiding this comment

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

I'm not very fond of the name runtime_args. This would imply we need to distinguish them from compile-time arguments, which we don't have. Everything that happens here is runtime anyway.

How about command_args to distinguish them from the method arguments?

@mrsdizzie
Copy link
Member Author

mrsdizzie commented Jan 18, 2023

I'm not very fond of the name runtime_args. This would imply we need to distinguish them from compile-time arguments, which we don't have. Everything that happens here is runtime anyway.

How about command_args to distinguish them from the method arguments?

Sounds good, updated!

mrsdizzie and others added 4 commits January 17, 2023 23:54
@mrsdizzie
Add runtime_args to options array for runcommand
01797e8 
Allow a caller to pass runtime arguments to WP_CLI::runcommand so that it is possible to store all 'default' options in one place.

ex if you want to avoid plugins and themes per default in a script that calls runcommand a lot:

```
$default_cli_opts  = array(
	'return'       => 'all',
	'exit_error'   => false,
	'runtime_args' => array( '--skip-themes', '--skip-plugins' ),
);

$output = WP_CLI::runcommand( 'option get siteurl', $default_cli_opts);
```
@mrsdizzie
Add tests
7eea904
@mrsdizzie @schlessera
Update php/class-wp-cli.php
56fa28b 
Co-authored-by: Alain Schlesser <alain.schlesser@gmail.com>
@mrsdizzie
Rename runtime_args => command_args
fa04fa4
schlessera
schlessera requested changes Jan 18, 2023
View reviewed changes
Copy link
Member

@schlessera schlessera left a comment

Choose a reason for hiding this comment

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

Minor cosmetic changes.

@schlessera schlessera added this to the 2.8.0 milestone Jan 18, 2023
@schlessera schlessera merged commit a584b19 into wp-cli:main Jan 18, 2023
@schlessera
Copy link
Member

schlessera commented Jan 18, 2023

Thanks for the PR, @mrsdizzie !

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

Reviewers

@danielbachhuber danielbachhuber danielbachhuber left review comments

@schlessera schlessera schlessera approved these changes

Assignees

No one assigned

Labels

internal-api:wp-cli-run-command

Projects

None yet

Milestone

2.8.0

Development

Successfully merging this pull request may close these issues.

3 participants

@mrsdizzie @schlessera @danielbachhuber