Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Pass in args to before_run_command hook#5554

Merged
schlessera merged 4 commits into
wp-cli:masterwp-cli/wp-cli:masterfrom
CodeProKid:feature/add-before-run-command-argsCodeProKid/wp-cli:feature/add-before-run-command-argsCopy head branch name to clipboard
Sep 3, 2021
Merged

Pass in args to before_run_command hook#5554
schlessera merged 4 commits into
wp-cli:masterwp-cli/wp-cli:masterfrom
CodeProKid:feature/add-before-run-command-argsCodeProKid/wp-cli:feature/add-before-run-command-argsCopy head branch name to clipboard

Conversation

@CodeProKid

@CodeProKid CodeProKid commented Aug 11, 2021

Copy link
Copy Markdown

This PR passes in some args to the before_run_command hook so callbacks have some more context to base their logic off of.

The reason for proposing this change is because of a particular use case I have where I'm implementing some logging to track CLI commands that are being run in our codebase. I currently have an implementation that looks something like:

WP_CLI::add_hook( 'before_run_command', function() {
	$runner  = WP_CLI::get_runner();
	$command = $runner->find_command_to_run( $runner->arguments )[0];
	log( $command->get_name() );
} );

This works for commands invoked from the command line but doesn't work for commands invoked via WP_CLI::run_command with launch => false set. This is because run_command will call the Runner->run_command() method directly so the Runner instance doesn't get bootstrapped again with the proper context.

To get around this, I'm proposing we pass the params from the run_command method into the before_run_command hook.

@CodeProKid CodeProKid requested a review from a team as a code owner August 11, 2021 18:21
Comment thread features/hook.feature
"""
And the return code should be 0

Scenario: Add callback to the `before_run_command` with args

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'm definitely open to a better testing approach here to make sure the hook args are flowing through to the callback properly. 😆

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Testing looks good to me.

@janw-me

janw-me commented Aug 17, 2021

Copy link
Copy Markdown
Member

Love the fix, I'm always up for better hooks.

I'm no expert on the behat tests, but now you only test the happy path not the 2 failure states.
I don't know what the best way to test those is.

@CodeProKid

Copy link
Copy Markdown
Author

@schlessera curious if you have any thoughts/feedback on this change?

@schlessera

Copy link
Copy Markdown
Member

I'm no expert on the behat tests, but now you only test the happy path not the 2 failure states.

Those are already being tested by checking for the return code. Those 2 failure states use WP_CLI::error(), which produces a non-zero return code.

Comment thread features/hook.feature Outdated
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.