Actually, it would be more accurate to say move the drush commands related to typed data out of D8 Rules Essentials (where they have been developed and tested) and into the Typed Data API module, which is a more logical place.
These commands are:
$ drush --filter=typed-data
Drush Commandline Tool 10.3.1
Run `drush help [command]` to view command-specific help. Run `drush topic` to read even more documentation.
Available commands:
typed-data:
typed-data:contexts (cl, context-list) Show a list of available contexts.
typed-data:datafilters (fl, datafilter-list) Show a list of available TypedDataFilter plugins.
typed-data:datatypes (tl, datatype-list) Show a list of available Typed Data datatypes.
typed-data:entities (el, entity-list) Show a list of available entities.
typed-data:formwidgets (wl, formwidget-list) Show a list of available TypedDataFormWidget plugins.
Comments
Comment #2
tr commentedComment #3
jonathan1055 commentedNice work. Just a few points/questions:
$categories = TRUEand$short = FALSEparameters in formatOutput() but no way to alter or set thm. Is it worth having command options to allow these to be changed?$plugin['provider']not$plugin['category']?Thanks for this, it will be useful.
Comment #4
tr commentedOK, so the history of this code is that every one of these commands started out as a quick-and-dirty script to fetch information I found useful, then I turned them into individual drush commands for the tr_rulez module. Then I refactored the code because all those commands were doing essentially the same thing - getting the plugin definition from the plugin manager and printing out some of the array elements from the plugin definition. The only real difference was which plugin manager was used and which array elements were available - that code went into formatOutput() and the list...() functions became thin wrappers around formatOutput().
These commands have been in tr_rulez for more than 2 years now. And I have documented them all in the D8RE documentation. I have always meant to improve the output control "eventually", like I did with the Rules
drush rules:listcommand, but since it seems that it's only me who has been using them (no feedback ever in the issue queue or documentation for any of these features) I have put off working on that - I have what I currently need; further work on the output formatting is a low priority for me when there are more important things that need to be done. But of course I'm totally for it if someone else wanted to contribute.Now, as part of porting these drush commands to Drupal 9+, I decided that a better place for the commands related to typed data is in the Typed Data module. To split these commands out of tr_rulez I had to duplicate formatOutput(), so it looks like some of the options aren't needed (and they're not, at least not for the typed_data module). Specifically, Data Filter and Form Widget plugin managers don't implement CategorizingPluginManagerInterface, so those plugins do not have categories thus the category parameter to formatOutput() really is not needed in Typed Data. But that parameter IS needed for the copy of formatOutput() that still exists in tr_rulez. Categories are used for Rules events, conditions and actions - these are the "Content", "System", "User" etc. divisions you see in the UI when selecting an event, condition or action - the category is a parameter set in the annotation.
I left the commented code in because sometimes I want to know the class that implements the plugin (useful for when there's a deriver involved, for instance) but I never took the time to make this into a proper option - it was easier just to uncomment the line when I needed it.
So yes, I think there's a lot of room for improvement. My goals for this issue are primarily:
Comment #5
jonathan1055 commentedThanks for explaining the background. Yes you are right that we have more pressing tasks, so RTBC for patch #2. At some time I could look at catering for the display options with drush arguments and adding a switch to show the full class, if that would be helpful.
Comment #7
tr commentedCommitted.
Comment #9
jonathan1055 commentedThe commit above introduced four coding standards warnings.
Is this something we want to fix? How much effort is it? We have plenty of other things to do.
Comment #10
tr commentedI deliberately did not inject services into the Drupal command class for several reasons. First, it's a Drush command class - we will not be writing tests for these Drush commands because they are just thin wrappers around the core and module services that the commands are using (which are already tested elsewhere), and testing would be the primary motivation for injecting the services here. Second, we would have to inject at least 5 services, each for a one-time use, and that would add ~80 lines of boilerplate code which would obscure and overwhelm the ~125 lines of actual functional code in there. Plus every new command would require a new service to be injected.
The coding standard is "should be avoided", not "MUST be avoided", so instead of treating this as a commandment we should use our discretion to determine when this coding standard provides a benefit and when it doesn't.
Comment #11
jonathan1055 commentedYes I thought the overhead would be detrimental, and your decisions were deliberate, but good to check.
Here's a patch which uses
// @phpcs:ignore DrupalPractice.Objects.GlobalDrupal.GlobalDrupalto mark these four warnings as known and accepted. It's worth keeping the codebase clean, so that when a new coding standard message is shown it is more apparent.I cannot re-open this issue, so won't be able to set this patch for testing on drupal.org. On local testing phpcs is clear, we get no messages.
Comment #12
tr commentedThanks. Committed.