[Twig][Form] Moved twig.form.resources to a higher level #11343
Conversation
stefanosala
commented
Jul 7, 2014
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | yes |
| Tests pass? | yes |
| Fixed tickets | #11296 |
| License | MIT |
| Doc PR | symfony/symfony-docs#4003 |
| @@ -55,7 +55,12 @@ public function load(array $configs, ContainerBuilder $container) | ||
|
|
||
| $container->setParameter('twig.exception_listener.controller', $config['exception_controller']); | ||
|
|
||
| $container->setParameter('twig.form.resources', $config['form']['resources']); | ||
| // To prevent BC, load the configuration from both branches | ||
| $formThemes = array_unique(array_merge($config['form']['resources'], $config['form_themes'])); |
There was a problem hiding this comment.
This should be handled in the Configuration class instead, by rewriting the old path to the new one
There was a problem hiding this comment.
You can add a normalizer (on the $rootNode) which checks if ['form']['resources'] is set, and call a closure which merges the configurations into form_theme.
->beforeNormalization()
->ifTrue(function ($v) { return count($value['form']['resources']) > 0; })
->then(function($v) {
$v['form_themes'] = array_unique(array_merge($v['form']['resources'], $v['form_themes']));
return $v;
})
->end()There was a problem hiding this comment.
Thanks for the hint, I just had to do that in validate() method because it has to happen after xml normalization.
|
Not sure if this PR is fully BC. Does it will still support to merge extra resources in a DI extension as done here? |
|
@egeloen No, you're right, it doesn't support the merge of extra resources at the moment. |
|
the name of the DIC parameter should indeed be preserved |
|
Fixed. 👍 |
|
Doc PR created. |
|
@stefanosala There is a slight code standards issue - check out fabbot on the link near the bottom. And awesome job! |
|
@weaverryan 👍 thank you for the review! |
|
👍 |
| @@ -31,6 +31,8 @@ public function testLoadEmptyConfiguration() | ||
| $this->compileContainer($container); | ||
|
|
||
| $this->assertEquals('Twig_Environment', $container->getParameter('twig.class'), '->load() loads the twig.xml file'); | ||
|
|
||
| // @deprecated since 2.6, to be removed in 3.0 |
There was a problem hiding this comment.
this is wrong. The DIC parameter is not deprecated
There was a problem hiding this comment.
it took me a while to understand why, but you're right, merci!
twig.form_themes
c3e6060 to
ab0b5e6
Compare
|
Thanks Stefano for working on this feature, this is much appreciated. |
…el (stefanosala) This PR was merged into the 2.6-dev branch. Discussion ---------- [Twig][Form] Moved twig.form.resources to a higher level | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #11296 | License | MIT | Doc PR | symfony/symfony-docs#4003 Commits ------- ab0b5e6 [Twig][Form] Moved configuration key twig.form.resources to twig.form_themes
…l (stefanosala) This PR was merged into the master branch. Discussion ---------- [Twig][Form] Moved twig.form.resources to a higher level | Q | A | ------------- | --- | Doc fix? | no | New docs? | no, symfony/symfony#11343 | Applies to | >= 2.6 | Fixed tickets | - Commits ------- 63d8657 [Twig][Form] Moved twig.form.resources to a higher level