Make sure subcommand @alias definition respects @when definition by mrsdizzie · Pull Request #5730 · wp-cli/wp-cli
Currently subcommand @alias value is not passed to register_early_invoke. This fixes #5724
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, @mrsdizzie — thank you!
Following up on your comment, I think the best place for @alias and @when parsing is inside of add_command():
| public static function add_command( $name, $callable, $args = [] ) { |
In fact, CompositeCommand probably shouldn't call register_early_invoke() directly:
| WP_CLI::get_runner()->register_early_invoke( $when_to_invoke, $this ); |
@alias is a very old feature introduced for backcompat when renaming commands f467e3c #186
The broken behavior you're experiencing is simply because no one ever thought of this particular scenario.
Thanks!
There is a lot of abstraction im not familiar with here so I'm a bit lost, but it doesn't look like add_command() processes any of the DocBlock directly. It looks like it calls CommandFactory::create which calls create_subcommand or create_composite_command where that can happen.
You can pass $args to add_command() which can contain those same DocBlock definitions, but most uses of add::command don't do that they just pass a callable which has a DocBlock (like the example test).
I wouldn't know where to move that functionally into add_command
@mrsdizzie No worries, sounds good. I'll dive into this when I have some time, but can't make any commitments to when it might be. I've assigned the issue to myself though.
Here's some code that works, but I don't think is correct:
diff --git a/features/command.feature b/features/command.feature index 23b1a255..3bc320f4 100644 --- a/features/command.feature +++ b/features/command.feature @@ -1519,6 +1519,7 @@ Feature: WP-CLI Commands wp custom """ + @daniel Scenario: subcommand alias should respect @when definition Given an empty directory And a custom-cmd.php file: diff --git a/php/WP_CLI/Dispatcher/CompositeCommand.php b/php/WP_CLI/Dispatcher/CompositeCommand.php index fc711a9e..912270ca 100644 --- a/php/WP_CLI/Dispatcher/CompositeCommand.php +++ b/php/WP_CLI/Dispatcher/CompositeCommand.php @@ -41,7 +41,7 @@ class CompositeCommand { $when_to_invoke = $docparser->get_tag( 'when' ); if ( $when_to_invoke ) { - WP_CLI::get_runner()->register_early_invoke( $when_to_invoke, $this ); + WP_CLI::get_runner()->register_early_invoke( $when_to_invoke, $this, $docparser->get_tag( 'alias' ) ); } } diff --git a/php/WP_CLI/Dispatcher/Subcommand.php b/php/WP_CLI/Dispatcher/Subcommand.php index 837a0071..f4dd9c10 100644 --- a/php/WP_CLI/Dispatcher/Subcommand.php +++ b/php/WP_CLI/Dispatcher/Subcommand.php @@ -25,6 +25,10 @@ class Subcommand extends CompositeCommand { $this->when_invoked = $when_invoked; $this->alias = $docparser->get_tag( 'alias' ); + if ( 'foo' === $name ) { + var_dump( $docparser ); + var_dump( $docparser->get_tag( 'alias' ) ); + } $this->synopsis = $docparser->get_synopsis(); if ( ! $this->synopsis && $this->longdesc ) { diff --git a/php/WP_CLI/Runner.php b/php/WP_CLI/Runner.php index afa9bc51..2c29b17c 100644 --- a/php/WP_CLI/Runner.php +++ b/php/WP_CLI/Runner.php @@ -89,9 +89,19 @@ class Runner { * * @param string $when Named execution hook * @param Subcommand $command + * @param string $alias */ - public function register_early_invoke( $when, $command ) { - $this->early_invoke[ $when ][] = array_slice( Dispatcher\get_path( $command ), 1 ); + public function register_early_invoke( $when, $command, $alias = '' ) { + $cmd_path = array_slice( Dispatcher\get_path( $command ), 1 ); + $this->early_invoke[ $when ][] = $cmd_path; + if ( 'test' === $cmd_path[0] ) { + var_dump( $command ); + } + if ( $alias ) { + array_pop( $cmd_path ); + $cmd_path[] = $alias; + $this->early_invoke[ $when ][] = $cmd_path; + } } /**
$ wp --require=test-cmd.php test bar object(WP_CLI\Dispatcher\Subcommand)#18 (9) { ["alias":"WP_CLI\Dispatcher\Subcommand":private]=> NULL ["when_invoked":"WP_CLI\Dispatcher\Subcommand":private]=> NULL ["name":protected]=> string(3) "foo" ["shortdesc":protected]=> string(4) "test" ["longdesc":protected]=> string(0) "" ["synopsis":protected]=> NULL ["docparser":protected]=> object(WP_CLI\DocParser)#16 (1) { ["doc_comment":protected]=> string(39) "test @alias bar @when before_wp_load " } ["parent":protected]=> object(WP_CLI\Dispatcher\CompositeCommand)#14 (7) { ["name":protected]=> string(4) "test" ["shortdesc":protected]=> string(0) "" ["longdesc":protected]=> string(0) "" ["synopsis":protected]=> NULL ["docparser":protected]=> object(WP_CLI\DocParser)#13 (1) { ["doc_comment":protected]=> string(0) "" } ["parent":protected]=> object(WP_CLI\Dispatcher\RootCommand)#10 (7) { ["name":protected]=> string(2) "wp" ["shortdesc":protected]=> string(42) "Manage WordPress through the command-line." ["longdesc":protected]=> NULL ["synopsis":protected]=> NULL ["docparser":protected]=> NULL ["parent":protected]=> bool(false) ["subcommands":protected]=> array(0) { } } ["subcommands":protected]=> array(0) { } } ["subcommands":protected]=> array(0) { } } object(WP_CLI\DocParser)#16 (1) { ["doc_comment":protected]=> string(39) "test @alias bar @when before_wp_load " } string(3) "bar" Hello
The alias the subcommand object is somehow empty when it's passed into register_early_invoke:
Ah, this does the trick:
diff --git a/features/command.feature b/features/command.feature index 23b1a255..3bc320f4 100644 --- a/features/command.feature +++ b/features/command.feature @@ -1519,6 +1519,7 @@ Feature: WP-CLI Commands wp custom """ + @daniel Scenario: subcommand alias should respect @when definition Given an empty directory And a custom-cmd.php file: diff --git a/php/WP_CLI/Dispatcher/Subcommand.php b/php/WP_CLI/Dispatcher/Subcommand.php index 837a0071..9ad47de3 100644 --- a/php/WP_CLI/Dispatcher/Subcommand.php +++ b/php/WP_CLI/Dispatcher/Subcommand.php @@ -20,12 +20,13 @@ class Subcommand extends CompositeCommand { private $when_invoked; public function __construct( $parent, $name, $docparser, $when_invoked ) { + + $this->alias = $docparser->get_tag( 'alias' ); + parent::__construct( $parent, $name, $docparser ); $this->when_invoked = $when_invoked; - $this->alias = $docparser->get_tag( 'alias' ); - $this->synopsis = $docparser->get_synopsis(); if ( ! $this->synopsis && $this->longdesc ) { $this->synopsis = self::extract_synopsis( $this->longdesc ); diff --git a/php/WP_CLI/Runner.php b/php/WP_CLI/Runner.php index afa9bc51..c2e3f455 100644 --- a/php/WP_CLI/Runner.php +++ b/php/WP_CLI/Runner.php @@ -91,7 +91,13 @@ class Runner { * @param Subcommand $command */ public function register_early_invoke( $when, $command ) { - $this->early_invoke[ $when ][] = array_slice( Dispatcher\get_path( $command ), 1 ); + $cmd_path = array_slice( Dispatcher\get_path( $command ), 1 ); + $this->early_invoke[ $when ][] = $cmd_path; + if ( $command->get_alias() ) { + array_pop( $cmd_path ); + $cmd_path[] = $command->get_alias(); + $this->early_invoke[ $when ][] = $cmd_path; + } } /**
The problem was that register_early_invoke() is called in parent::__construct(), and $this->alias` hadn't been defined yet.
Move the docparser call before parent::__construct() so we detect and later handle any alias properly.
@danielbachhuber cool! that is pretty similar to what I had started with ( moving the docparser call above the parent::__construct() call). Looks good to me thanks for looking into it
@danielbachhuber could this be considered for 2.8.0 milestone since it is coming up?
@mrsdizzie Yeah, I'm fine to land this. There's probably a much broader refactor that could happen to make this more "correct", but I don't think it's worth the effort if the existing tests pass just fine.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters