Provide annoted command support for Drush 8. #2139
Conversation
|
consolidation/annotated-commands currently requires php 5.5. It inherits this requirement from Symfony 3. Note, however, that this PR does not use Symfony/Console, and the components it does use work fine all the way back to php 5.3. |
|
So, there are three potential routes forward vis-a-vis avoiding the Symfony/Console requirement here:
The first option is what might be considered the "technically correct" solution; however, there's a maintenance cost to splitting out code & unit tests, and lots of very small repositories are harder to maintain. We need to keep the right balance between separability and maintainability / overhead work. That's why I think that the last option might just be best; just orphan the classes, maintain them to a level of functionality sufficient for the reduced functionality in this PR, and let the main project move forward. There is a cost to ensuring that any needed bug fixes are backported, but I don't anticipate many changes to command info and command discovery once we finish with #2103, so perhaps this is okay. |
| } | ||
| // Check to see if this is the Drush searchpath for instances where we are | ||
| // NOT going to do a full bootstrap (e.g. when running a help command) | ||
| if (($phase == DRUSH_BOOTSTRAP_DRUPAL_SITE) && ($phase_max < DRUSH_BOOTSTRAP_DRUPAL_FULL)) { |
There was a problem hiding this comment.
This bootstrap test has been buggy many times in the past few years. I rather hate to copy the logic here. Would rather have it centralized if feasible.
There was a problem hiding this comment.
I am reluctant to rework how bootstrapping works in the Drush 8 branch; that is what would be necessary to centralize it. Move the code here that finds all of the specific module folders into the bootstrap code. This is not necessary for the existing use case, which searches deeply for .drush.inc files.
|
Tentatively agreeing that third option makes sense. |
| if (($phase == DRUSH_BOOTSTRAP_DRUPAL_SITE) && ($phase_max < DRUSH_BOOTSTRAP_DRUPAL_FULL)) { | ||
| $searchpath = annotationcommand_adapter_refine_searchpaths($searchpath); | ||
| } | ||
| $commands = drush_get_context('DRUSH_ANNOTATED_COMMANDS'); |
There was a problem hiding this comment.
$commands starts out with the set of commands discovered the last time this function was called.
Since command hooks are somewhat different in consolidation/annotated-command than in Drush 8, this PR simplifies things and only handles the primary hook for annotated command files. While it would be possible to adapt the new hooks in the old system, that would be more effort; it seems better to let people continue to write their personal policy files using the old hook system until they are ready to upgrade to Drush 9. Supporing only the primary hook in Drush 8 should meet the use case of Drupal 8 module writers who just want to write a simple command line interface that they also want to work in Drush 9. This code is still in progress; more work is needed to make help commands and validation work right. However, in its current form, this will run the example in the "Modern CLI tools" patch at https://www.drupal.org/node/2703131.
…st site's database.
…avoid dependency on Symfony/Console.
greg-1-anderson
force-pushed
the
annotation-drush8
branch
from
64fafbf to
8acb695
Compare
…te to use actual consolidation/annotated-command project.
|
Actually, php 5.4 is not an issue, as it is supported by Symfony 2. Now, both consolidation/annotated-command and consolidation/output-formatters support php 5.4. The latest commit here uses both of these projects directly, so that annotated commands are now dispatched, processed and formatted using the same functions as the master branch. The main difference here is that we still are not using Symfony/Console in this PR. |
…esults from annotated commands.
…of annotationcommand_adapter_get_commands.
…ice commands are found.
… do not require it for internal command handlers.
| $this->assertContains('Aliases: try-formatters', $output); | ||
|
|
||
| // Disable the woot module to avoid cross-contamination of the Drupal | ||
| // test site's database. (not necessary?) |
There was a problem hiding this comment.
The Drupal site is wiped after each test class. No need to clean up.
…o, use the provided createFromRequest method.
…rectly; we do not use an Application object.
… it causes problems.
…nel->invalidateContainer() will do the trick.
Since command hooks are somewhat different in consolidation/annotated-command than in Drush 8, this PR simplifies things and only handles the primary hook for annotated command files. While it would be possible to adapt the new hooks in the old system, that would be more effort; it seems better to let people continue to write their personal policy files using the old hook system until they are ready to upgrade to Drush 9. Supporing only the primary hook in Drush 8 should meet the use case of Drupal 8 module writers who just want to write a simple command line interface that they also want to work in Drush 9.
This code is still in progress; more work is needed to make help commands and validation work right. However, in its current form, this will run the example in the "Modern CLI tools" patch at https://www.drupal.org/node/2703131.