Skip to content
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.

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

Add a method Robo::getCommandInstance. #675

Closed
wants to merge 1 commit into from

Conversation

greg-1-anderson
Copy link
Member

Overview

This pull request:

  • Fixes a bug
  • Adds a feature
  • Breaks backwards compatibility
  • Has tests that cover changes (indirectly)

Summary

Adds static method Robo::getCommandInstance

Description

It is frequently a request by command authors, who, in order to implement some command my:a which in turn should execute other commands my:b and my:c, the desire of the author is to recover the commandfile instance for the subcommands so that the command's implementation methods may be called directly.

In general, this technique has been discouraged, because calling an implementation method directly omits any hook evaluation, so the result might not be the same as running my:b and my:c e.g. via exec. However, the advantage of having a reference to the command object is that the direct command result is available, whereas using exec or $application->run() all you would get is stdout.

Copy link
Member

@DavertMik DavertMik left a comment

Choose a reason for hiding this comment

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

Looks pretty clean to me

@dafeder
Copy link

dafeder commented Aug 8, 2020

Any chance to get this merged? Still interested in this!

@greg-1-anderson
Copy link
Member Author

Did you test it? Does it satisfy your use case? What is your use case?

@pfrenssen
Copy link
Contributor

I tested this out. My use case is to have a single command that sets up a development environment for a Drupal website. This command is intended to run multiple other Robo commands, to set up config files like settings.php, services.yml, etc.

In my test I tried to create a command that calls 2 other commands directly: behat:generate-config (which generates a behat.yml configuration file) and drush:generate-config (which generates a drush.yml file).

My commands live in a Robo extension and are set up using the instructions at https://robo.li/extending/#register-command-files-via-psr-4-autoloading. I am using Robo 2.2.1, and this PR applies cleanly on it. The commit that tests this PR is code-lab-eu/robo-drupal-setup@1c549ad

I am getting an error, it appears that in my setup the command file I am trying to instantiate is not registered in the container.

$ ./vendor/bin/robo dev:setup
 [error]  Alias (ConfigFileCommands) is not being managed by the container
➜                                                                                                                                                              
➜   Requested RoboFile `/home/pieter/robo-drupal-setup/RoboFile.php` is invalid, please provide valid absolute path to load Robofile.
➜                                                                                                                                                              
➜
➜   Robo is not initialized here. Please run `robo init` to create a new RoboFile.

Also I am getting complaints about a missing RoboFile but since I expose commands in an extension I am expecting that these shouldn't require a RoboFile?

@greg-1-anderson
Copy link
Member Author

@pfrenssen: Could you compare with $application->run() in Robo 3? Capturing the output should work better there. Seems like perhaps you do not need the output for your use-case anyway, so perhaps this would also work well for you in Robo 2.

@pfrenssen
Copy link
Contributor

I tried with Robo 3 but this gives a similar error:

$ ./vendor/bin/robo dev:setup
 [error]  Alias (ConfigFileCommands) is not being managed by the container or delegates
Requested RoboFile `/home/pieter/v/event-venue/vendor/code-lab-eu/robo-drupal-setup/RoboFile.php` is invalid, please provide valid absolute path to load Robofile.
Robo is not initialized here. Please run `robo init` to create a new RoboFile.

I'm not clear what you mean by comparing with $application->run(), I am just getting started with Robo and I am trying out to build a standalone robo extension which I can include in my Drupal projects using Composer, so that I don't need to have a RoboFile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants