We currently use yaml discovery for both routing and info files, yet they do not share code past the symfony yaml parser. This makes it more difficult for other subsystems that want to use yaml discovery as well as coupling the code that handles routing and info files to yaml unnecessarily.
The need for a generic component has come up in other issues (#2005716: Promote EntityType to a domain object, #2050227: Add local action plugin deriver to use YAML discovery for static definitions), so I think it's time we added this.
The attached patch is extracted from what I wrote for #2005716: Promote EntityType to a domain object.
Here's an example of how it'd be used:
// Finds and parses all modulename.somename.yml files
$directories = $this->moduleHandler->getModuleDirectories();
$discovery = new YamlDiscovery("somename", $this->directories());
$discoverable_things = $discovery->findAll();
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | 2050289-15.patch | 13.44 KB | damiankloip |
| #15 | interdiff-2050289-15.txt | 1.06 KB | damiankloip |
| #13 | 2050289-13.patch | 13.53 KB | damiankloip |
| #11 | 2050289-11.patch | 9.77 KB | damiankloip |
| #8 | 2050289-8.patch | 8.97 KB | damiankloip |
Comments
Comment #1
fubhy commentedLet's move this to Drupal\Components, add some docblocks, and write a UnitTest!
Comment #2
damiankloip commentedon it.
Comment #3
damiankloip commentedOk new patch:
- Moved the class to Component
- Added a fileBaseName method so it can easily be overridden if the pattern of dir_basename.something.yml needs to be changed
- Added a unit test with some test yml files in a Fixtures (change the name? based it on Symfony test files) folder
Comment #4
msonnabaum commentedLooks great to me.
Comment #5
dawehnerIt would be cool if there would be a single place where this functionality is actually used.
Comment #6
damiankloip commentedI'll maybe convert RouteBuilder to use this, as the other issues in the summary aren't in yet
Comment #7
msonnabaum commentedDrupalKernel would be the other option for implementation.
Comment #8
damiankloip commentedI decided to just go with the Routing example, I already had that one in my head :)
I have changed the YamlDiscovery slightly, as the return from findAll() needs to return its discovered data my the module key that was passed in with the directories array, so essentially the directory path gets replaced by an array of discovered data.
Comment #9
damiankloip commentedOh, maybe I should add back in the !empty() check on routes? or, we could add checking in the YamlDisocvery class to default to array() is nothing is returned from a file?
Comment #11
damiankloip commentedLooks like layout_test module is the only module in core to not be inside it's own folder. Moved from modules/layout/tests/layout_test.* to modules/layout/tests/layout_test/layout_test.*
Comment #13
damiankloip commentedSilly, I forgot to move all the other sub dirs for layout_test.
Comment #14
dawehnerCan't we just do $route_info += array(
'defaults' => array(),
'requirements' => array(),
'options' => array()
);
Comment #15
damiankloip commentedSure, why not. Then we just need to pass each array element into Route() params.
Also this just seems like more of a refactoring task than a feature request?
Comment #16
dawehnerThank you!
Comment #17
Crell commentedAs noted in IRC, a side-effect of this change is that the routing alter event is only called for modules that define routes, not for all modules. I consider that a bonus as it's a slight performance boost. +1 from me.
Comment #18
alexpottCommitted 03e535c and pushed to 8.x. Thanks!
Comment #20
berdirThis introduced a regression, see #2067809: YamlDiscovery uses basename of directory instead of module name