drush-ops / drush

Drush is a command-line shell and scripting interface for Drupal, a veritable Swiss Army knife designed to make life easier for those who spend their working hours hacking away at the command prompt.
https://www.drush.org
2.34k stars 1.08k forks source link

docs for custom commands lack detail on annotation-based commands #2905

Closed joachim-n closed 6 years ago

joachim-n commented 7 years ago

Docs need to say, among other things:

weitzman commented 7 years ago

Thanks for the list. Added docs in 1d55077. Please open new issues or PRs as needed.

joachim-n commented 7 years ago

Thanks for the prompt update!

There are a few mistakes in the commit though I think:

+1. The file's namespace should be \Drush.

I don't think that's correct. My commandfile is getting picked up with this namespace:

namespace Drush\Commands\CodeBuilder;

where CodeBuilder is the name of the folder my command file is in.

+1. The enclosing directory must be named Commands

Not correct either.

As far as I could tell, there's no restriction on the enclosing folder name when the Symfony Finder is invoked.

My command file's path in full is:

/Users/joachim/Library/Application\ Support/Drush/Drush\ 9/Commands/CodeBuilder/CodeBuilderCommands.php

where '/Users/joachim/Library/Application\ Support/Drush/Drush\ 9/Commands/' is in the Drush 'include' option. So the structure inside the include is just:

CodeBuilder/CodeBuilderCommands.php

weitzman commented 7 years ago

Thanks. Maybe @greg-1-anderson can improve on the text I just wrote.

greg-1-anderson commented 7 years ago

I fixed https://github.com/consolidation/annotated-command/issues/107, but Drush will need to explicitly call followLinks() if we want it to follow Symlinks when searching for commandfiles.

weitzman commented 7 years ago

I added that followLinks() call in #2915.

danepowell commented 6 years ago

I can't seem to get my ported command file to be picked up by Drush no matter how I namespace it. Any tips? You can see my work here: https://github.com/acquia/blt/pull/2210

greg-1-anderson commented 6 years ago

Change your namespace to namespace Drush\Commands, or change your filename path to drush/Commands/Blt/BltCommands.php and it should work.

If you do that and it still doesn't work, let me know.

danepowell commented 6 years ago

Thanks, both of those suggestions worked.

It turns out my include path was tripping me up as well. It should be: drush blt-doctor --include=vendor/acquia/blt/drush. Slight variations like vendor/acquia/blt or vendor/acquia/blt/drush/Commands don't work.

greg-1-anderson commented 6 years ago

If you put your drush component in a separate Composer project and give it a type of drupal-drush, then composer installers will place it in __ROOT__/drush/contrib, and Drush will pick it up.

greg-1-anderson commented 6 years ago

See also https://github.com/consolidation/Robo/pull/604, which is a proposal for a plugin system that would allow you to put command files in your vendor directory without specifying a --include option.

danepowell commented 6 years ago

In #2885, you mentioned that DI was not possible when you use global command files, can you provide an example of how to work around that?

For instance, I am trying to use StatusCommands, which needs to have a site alias manager set. It fails when called from the context of a global command because the site alias manager isn't set via DI: https://github.com/acquia/blt/pull/2210#issuecomment-340203700

danepowell commented 6 years ago

Oh, I think I figured it out! You just have to instantiate the class and set the manager yourself...

With DI:

$status_table = StatusCommands::getPropertyList([]);

Without DI:

$status_commands = new StatusCommands();
$status_commands->setSiteAliasManager(new SiteAliasManager());
$status_table = $status_commands->getPropertyList([]);

Sorry, this is my first time working so heavily with this sort of pattern...

greg-1-anderson commented 6 years ago

Drupal DI does not work for global commands. Drush DI via SiteAliasManagerAwareInterface / SiteAliasManagerAwareTrait should work just fine.

Neither of the examples above are correct.

If you are in some non-command context where you cannot do dependency injection, you can get a reference to anything in the DI container via:

$aliasManager = \Drush\Drush::service('site.alias.manager');

We should perhaps provide an API so folks can get reference to the command objects (e.g. StatusCommands) in case folks want to call them directly. However, doing this wouldn't always produce the right results, as hooks would not run in this instance, and some commands might do necessary setup in an init hook or something like that.

Another option is to get a reference to the $application object and just call run() manually. This would require that you build your own commandline options with an ArrayInput, which isn't ideal; however, drush_invoke_process essentially works the same way, so maybe that's not horrible. Really, though, I think we should keep using drush_invoke_process for now, and provide a more modern replacement later -- maybe something as simple as \Drush\Drush::invoke()

weitzman commented 6 years ago

A few notes adding to Greg's answer:

I think this issue is resolved. I can reopen if thats mistaken.

danepowell commented 6 years ago

Thanks for correcting me, I switched to using drush_invoke_process(): https://github.com/acquia/blt/pull/2218