drush
drush copied to clipboard
Deprecate Annotated Commands in favor of native Symfony Console commands
Status
- We will present this work at Drupalcon Vienna and hope to gain consensus for moving most of our commands into Drupal core or Drupal CMS.
- Console recently merged https://github.com/symfony/symfony/pull/59340 (blog post). This PR has not yet been adjusted.
Open issues
- [x] Depends on https://github.com/consolidation/output-formatters/pull/113
- [ ] Keep dependency on
consolidation/filter-via-dot-access-data, move that into output-formatters, or drop the feature. See \Drush\Formatters\FormatterTrait::alterResult - [ ] Update: we will do this when the PR is ready for merge. Move FormatterTrait and related Attributes into consolidation/output-formatters.
- [x] Update: Moshe hacked image:flush into Dex and it worked!. Attempt to re-use the new style commands and listeners in a separate vanilla Console app, or use Dex. The goal is to verify how consumable these commands are.
- [x] Decide between two methods of delivering OptionSets. SqlDumpCommand demonstrates both approaches:
- The sql optionset is added by calling a static method from configure(). No Attribute is involved. This is less magical (good) but perhaps slightly harder to discover as a command author.
- The tableSelection optionset is added by adding #[OptionsetTableSelection] on the class. This signals OptionsetTableSelectionListener to add the options.
- [x] Same as above, for validators. ImageFlushCommand demonstrates the two approaches.
- The top of execute() calls the static Validators::entityLoad()
- The
#[ValidateModulesEnabled]Attribute signals to ValidateModulesEnabledListener to do its validation work
Description
It is our hope that Drush commands will be consumed by a future CLI in Drupal core. To make our commands more consumable, this PR changes Drush commands to directly extend Symfony's Command class.
Converted commands are sql:dump, twig:unused and image:flush. Some highlights:
- Each command gets its own file. Attributes are placed on the class instead of the method.
- Use Symfony's #[AsCommand] Attribute to declare command name and alias. The Drush #[Command] Attribute is deprecated.
- A Drush command extends Symfony's Command (not DrushCommands) 💪.
- Use configure() and interact() methods as per usual with Symfony commands. The #[Argument] and #[Option] PHP Attributes are deprecated.
- Drush and Drupal services may still be autowired. This is how you access the logger. Build own $io as needed.
- Commands that wish to offer multiple output formats (yes please!) should:
- inject FormatterManager in __construct()
- use FormatterTrait
- call $this->configureFormatter() in configure() in order to automatically add the needed options. Example.
- execute() is pretty boilerplate. do your work in doExecute() instead.
We have more plans for simplifying Drush's Preflight and removing the consolidation/annotated-command and consolidation/robo dependencies - this is just a start!
We had talked about moving FormatterTrait to consolidation/output-formatters. I would like to do that. I think this implies that src/Attributes/FieldLabels.php and the other format-related attributes would also need to move over. We can maintain compatibility with Drush 13 if we make Drush\Attributes\FieldLables & c. extend the consolidation/output-formatters attribute.
Should I go ahead and do that, and update this PR to use the output-formatters copy?
I'm undecided about moving FormatterTrait and the related Attributes. I do agree that both are coupled and probably should live together. When authoring a command, its helpful for all Attributes to be in same namespace so you can browse them in your IDE. If we move the Formatter attributes, we bifurcate them, or we thinly extend the ones provided by output-formatters. We do that thin extension today for those provided by annotated-command, but I always found that slightly awkward. Lets mull it.
Update: the implementation now complete. Maybe @greg-1-anderson has a suggestion about how to implement --filter in the new paradigm. See DrushCommandAlterer and FilterHooks. In this PR I've added #[CLI\FilterDefaultField(field: 'template')] to twig:unused, and [\Drush\Formatters\FormatterTrait::addFormatterOptions] auto-adds the --filter option when needed. Lets discuss how to implement result altering. I'm hoping that command authors don't have to worry about. Maybe we add result altering to \Drush\Formatters\FormatterTrait::execute just before formatting?
--filter is a seldom used feature of Drush and perhaps we should drop it.
Yes, I was thinking that filter could happen in the format trait.
I think that consolidation/output-formatters should have the trait and format attributes, so that they are available when that library is used outside of Drush. I'll probably duplicate the code there regardless, even if Drush doesn't use it. Perhaps thin extensions would be a workable solution, where necessary.
OK, when this PR is ready to go in, lets move the FormatterTrait and format Attributes, and Drush may thinly extend the Attributes.
The formatter thing looks complex and too magic. Will it save much time for command developers?
I think FormatterTrait should have just one simple getter for the formatter instance.
Something like this.
private static function getOutputFormatter(): FormatterManager {
return \Drupal::service(FormatterManager::class);
}
It seems quite easy to apply it manually.
protected function configure(): void
{
$this
->addArgument('some-arg', InputArgument::REQUIRED, 'Description');
self::getOutputFormatter()->configureCommand($this);
}
public function execute(InputInterface $input, OutputInterface $output): int
{
$data = [];
self::getOutputFormatter()->write($output, $input->getOption('format'), new RowsOfFields($data));
return self::SUCCESS;
}
I would actually inject the formatter through constructor rather than through trait.
Given that Command::addOption() is a public method. We could potentially add format option automatically to commands that indicate a need for it through some PHP attribute.
Also curious if it is possible to integrate the formatter with DrushStyle. For instance SymfonyStyle has methods like table(), listing(), etc. Why can't DrushStyle have something like writeFormatted()?
- The output formatter system is opt-in. Commands who dont care can ignore it.
- The doExecute and getFormatterOptions() are a bit of a dance, I agree. I'm open to suggestions. Note that your example didn't pass FormatterOptions which is where most of the complexity resides. Also note that it needs to calculated during configure (for automaticOptions - only valid formats are listed in the description and autocomplete), and execute (for the actual filtering and formatting)
I attempted to remove a bit of magic for commands that format - see SqlDumpCommand. The command no longer has to add PHP Attributes like #[DefaultTableFields]. Instead, the command directly implements a getFormatterOptions() method to specify its configurationData. This is discussed a bit more in Open Issues above.
I like the requirement to call some method in configure() to set up the formatter options. I am not very fond of using a static method for this, though. Perhaps calling a method of some injectable class would be better. Unsure, though.
I am also wondering if formatter options can somehow be set up by calling methods in configure(), just as args and options are set up, rather than having folks provide a getFormatterOptions() method, which is a different pattern. Maybe the FormatterTrait could provide something.
The key bits that only the command knows the values for stuff like DefaultTableFields, FieldLabels, etc. Everything else is machinery.
I am also wondering if formatter options can somehow be set up by calling methods in configure()
We could add methods if we extended Command, like AnnotatedCommand does. Laravel extends it too. I'm not sure how loathsome that would be to Drupal.
Maybe those methods could be in the FormatterTrait.
I posted a status update to the issue description here. It says
This PR is on long term hold. Since Drush is already bundled with Drupal CMS, most newcomers to Drupal are already getting Drush's benefits. If a member of Drupal Leadership expresses intent to use these commands directly in core or CMS, I (moshe) will restart work on this.
makes sense to link Ideas issue which probably is the next to decide https://www.drupal.org/project/ideas/issues/3159647
Symfony Console recently released 7.3 which includes https://github.com/symfony/symfony/pull/59340 (blog post). This PR should probably be adjusted accordingly. Thoughts are welcome.