[DI][DX] Allow exclude to be an array of patterns #24428
Conversation
| $loader->registerClasses( | ||
| new Definition(), | ||
| 'Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\\', | ||
| 'Prototype/*', array( |
There was a problem hiding this comment.
FileLoader::registerClasses docblock must be updated, right?
There was a problem hiding this comment.
You're absolutely right, I completely missed this one. My bad
| * @param Definition $prototype A definition to use as template | ||
| * @param string $namespace The namespace prefix of classes in the scanned directory | ||
| * @param string $resource The directory to look for classes, glob-patterns allowed | ||
| * @param string|array $exclude A globed path of files to exclude or an array of globed paths of files to exclude |
There was a problem hiding this comment.
should it be string[] instad of array ?
| if (null === $excludePrefix) { | ||
| $excludePrefix = $resource->getPrefix(); | ||
| if ($excludePatterns) { | ||
| $excludePatterns = (array) $parameterBag->unescapeValue($parameterBag->resolveValue($excludePatterns)); |
There was a problem hiding this comment.
I'd rather do the cast in registerClasses and add the array $excludePatterns typehint in this method
|
This might be enough for Yaml, but this doesn't benefit Xml nor the PHP configurators yet, do we want to make this available for these formats also? |
|
Moving to 4.1. |
|
@nicolas-grekas I'm sorry I could not finish this earlier, I'll rebase and edit base branch on github |
|
@apetitpa let's continue this one? |
|
@nicolas-grekas I'm back on it ! Would you mind giving me a hint on what's left to do please ? |
|
I think we should make this work somehow for other config formats (xml and php-dsl) |
|
@nicolas-grekas Thanks for your help I have rebased on master and started to work something out, so this means I should make this piece of xml configuration work, right ? <prototype namespace="AppBundle\" resource="../../src/AppBundle/*">
<exclude>../../src/AppBundle/{Entity,Repository}</exclude>
</prototype>About the php based configuration it should already work since you can now pass an array as the third argument of $this->registerClasses($definition, 'AppBundle\\', '../../src/AppBundle/*', array('../../src/AppBundle/{Entity,Repository}')); |
|
I was more specifically referring to the Configurator in the Loader namespace. Look for a call to this method there, you should find something to adjust. |
|
ping @apetitpa |
|
@apetitpa do you think you can finish this PR before the feature freeze by the end of the month? |
|
@nicolas-grekas I'll do my best but I'm very short in time these days. I'll give it a try probably on next saturday evening, is it too late ? |
| * @param Definition $prototype A definition to use as template | ||
| * @param string $namespace The namespace prefix of classes in the scanned directory | ||
| * @param string $resource The directory to look for classes, glob-patterns allowed | ||
| * @param string|string[] $exclude A globed path of files to exclude or an array of globed paths of files to exclude |
There was a problem hiding this comment.
right type should actually be string|string[]|null (the phpdoc should be string|null in 3.4)
| foreach ($this->glob($excludePattern, true, $resource) as $path => $info) { | ||
| if (null === $excludePrefix) { | ||
| $excludePrefix = $resource->getPrefix(); | ||
| if ($excludePatterns) { |
There was a problem hiding this comment.
you could remove this diff, handling the empty array like any other array (the string|null type was needing a dedicated handling of null before)
|
It's now too late for 4.1. If you miss time to finish @apetitpa I can take it over if it's good for you. |
|
I've created a pull request (just above) to finish it :) |
|
I'm sorry I couldn't make it guys and thank you @magnetik for taking over. |
…netik) This PR was merged into the 4.2-dev branch. Discussion ---------- [DI][DX] Allow exclude to be an array of patterns | Q | A | ------------- | --- | Branch? | 4.2 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #23956 | License | MIT This is basically continuing #24428. In YAML before: ```yaml AppBundle\: resource: '../../src/AppBundle/*' exclude: '../../src/AppBundle/{Entity,Payload,Repository}' ``` in YAML after: ```yaml AppBundle\: resource: '../../src/AppBundle/*' exclude: - '../../src/AppBundle/{Entity,Payload,Repository}' - '../../src/AppBundle/Event/*Event.php' ``` In XML before: ```xml <prototype namespace="App\" resource="../src/*" exclude="../src/{Entity,Migrations,Tests}" /> ``` in XML after: ```xml <prototype namespace="App\" resource="../src/*"> <exclude>../src/{Entity,Migrations,Tests}</exclude> <exclude>../src/Yolo</exclude> </prototype> ``` In PHP before: ```php $di->load(Prototype::class.'\\', '../Prototype') ->autoconfigure() ->exclude('../Prototype/{OtherDir,BadClasses}') ``` In PHP after: ```php $di->load(Prototype::class.'\\', '../Prototype') ->autoconfigure() ->exclude(['../Prototype/OtherDir', '../Prototype/BadClasses']) ``` Everything is backward compatible. Maybe a decision about handling both attribute exclude and element exclude in XML should be taken. Commits ------- 3ae3a03 [DI][DX] Allow exclude to be an array of patterns (taking #24428 over)
Before:
After: