Skip to content

Add runtime_args to options array for runcommand #5722

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jan 18, 2023
Merged

Add runtime_args to options array for runcommand #5722

merged 5 commits into from
Jan 18, 2023

Conversation

mrsdizzie
Copy link
Member

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

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

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

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

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