Skip to content

Conversation

@killua99
Copy link
Contributor

@killua99 killua99 commented Feb 7, 2019

--debug showing a message to differentiate the namespace from the command.

Resolve: #4937

if ( ! empty( $parent ) ) {
$sub_command = trim( str_replace( $parent, '', $name ) );
self::debug( "Adding command: {$sub_command} in {$parent} Namespace", 'commands' );
self::debug( "Adding namespace: {$parent}", 'commands' );
Copy link
Member

Choose a reason for hiding this comment

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

This message needs to be displayed when the actual namespace object is being added, otherwise it serves no purpose.

To check whether a command to be added is a namespace, you can use:

if ( $leaf_command instanceof Dispatcher\CommandNamespace ) { }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, I thought this was week like make none sense what I did, but perhaps with the condition of instanceof perhaps makes more sense. I'll correct it for today.

@schlessera
Copy link
Member

@killua99 Still interested in picking this up? Is something unclear about what the intended outcome should be?

@killua99
Copy link
Contributor Author

killua99 commented Apr 3, 2019

Hi @schlessera sorry for delay this issue for so long, I've been committing patches in other place.

I would say, if anyone want to bring a patch for it, then it should be submit the path. I would do it just idk when if this week or the next one.

Is clear to me the scope of the issue and what the outcome should be.

Let's see if I've time for it soon™

@killua99 killua99 requested a review from a team as a code owner April 16, 2019 15:49
@killua99
Copy link
Contributor Author

I'm not quite sure how to really debug or test the debug flag here. Which situation would have such of thing? @schlessera

@schlessera
Copy link
Member

@killua99 An example of where this is being used is the core verify-checksums command.

  • wp-cli/checksum-command gets loaded first, before wp-cli/core-command(alphabetical order)
  • wp-cli/checksum-command contains core verify-checksums, however, when it wants to add this, the parent core command has not been registered yet. So it first registers the namespace core and attaches the core verify-checksums to that namespace. The namespace is a placeholder in that case.
  • When wp-cli/core-command loads and wants to register the core command, it sees that there is already a core namespace registered. So it creates the core command, moves all the subcommands from the core namespace to the actual core command, and then replaces the core namespace with that new command.

@schlessera
Copy link
Member

This was fixed via #5563

@schlessera schlessera closed this Jan 6, 2022
@schlessera
Copy link
Member

Thanks for the effort in creating this PR, @killua99 !

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.

In --debug output, differentiate between commands and namespace

2 participants