extended twig-related things instead of forcing them #3
Conversation
EmanueleMinotto
force-pushed
the
extended-twig
branch
from
ccabeaa to
9798db0
Compare
| $app['twig.loader'] = $app->share(function (Application $app) { | ||
| return new PuliTemplateLoader($app['puli.repository']); | ||
| $app->extend('twig.loader', function ($twigLoader, $app) { | ||
| $twigLoader->addLoader(new PuliTemplateLoader($app['puli.repository'])); |
There was a problem hiding this comment.
This depends on the fact that we get a chain loader here, no? What if that's not the case? Should we handle that?
There was a problem hiding this comment.
Yes, by default there's a Twig_Loader_Chain, but it could be overridden.
I see some possibilities here:
- an aggresive strategy, so overriding always the old loader
- wrap the current loader with a new Twig_Loader_Chain and than add the Puli loader (:+1:)
- don't consider other possibilities, so considering only the Silex default loader
What do you think?
There was a problem hiding this comment.
I would do an instanceof check to see whether you receive an chain loader. If it is not a chain, just create a chain loader with both in it. If it is already a chain loader, just add your own loader (faster than nesting chain loaders into each other)
EmanueleMinotto
force-pushed
the
extended-twig
branch
from
9798db0 to
4edd12b
Compare
|
ping @webmozart |
| @@ -55,11 +55,17 @@ public function register(Application $app) | |||
| public function boot(Application $app) | |||
| { | |||
| if (isset($app['twig']) && $app['puli.enable_twig']) { | |||
| $app['twig.loader'] = $app->share(function (Application $app) { | |||
| return new PuliTemplateLoader($app['puli.repository']); | |||
| $app->extend('twig.loader', function ($twigLoader, $app) { | |||
There was a problem hiding this comment.
extending things should not be done on boot time though. Services may have been instantiated already (this was already an issue previously btw)
EmanueleMinotto
force-pushed
the
extended-twig
branch
5 times, most recently
from
94b9bd1 to
a83e83f
Compare
|
@stof I've added a check for the chain loader, but this part below isn't still fully clear to me, can you give me an example of what you mean, please?
|
EmanueleMinotto
force-pushed
the
extended-twig
branch
from
a83e83f to
a683ba3
Compare
|
Just rediscovered this PR. I guess this is still relevant? Could you rebase? |
As reported by @stof in #1