search

zendframework / zend-expressive-twigrenderer

Twig integration for Expressive
BSD 3-Clause "New" or "Revised" License
25 stars 14 forks source link

Split Twig_Environment from TwigRendererFactory #15

Closed geerteltink closed 7 years ago

geerteltink commented 8 years ago

This PR splits the Twig_Environment from the TwigRendererFactory. It makes the Twig_Environment reusable where needed as discussed in zendframework/zend-expressive-twigrenderer#14.

The Twig_Environment must be registered with its TwigEnvironmentFactory.

return [
    'dependencies' => [
        'factories' => [
            Twig_Environment::class                                   => Zend\Expressive\Twig\TwigEnvironmentFactory::class,
            Zend\Expressive\Template\TemplateRendererInterface::class => Zend\Expressive\Twig\TwigRendererFactory::class,
        ],
    ],

    // ...
];
geerteltink commented 8 years ago

@weierophinney I've tested this on a existing app and it seems to work.

Just wondering if config can be added for these: cache, debug (under the twig namespace), strict_variables and auto_reload. Or should they go into the a separate pr?

geerteltink commented 8 years ago

@weierophinney I'm actually waiting for a code review. Since you were talking about using delegators, I want to make sure this is a good approach before I start rewriting the tests. I thought I already mentioned this but I didn't.

glen-84 commented 8 years ago

@xtreamwayz @weierophinney -- Any news here? I really need a clean way of accessing the Twig_Environment.

geerteltink commented 8 years ago

Sorry, I'm waiting on a code review before I start rewriting all the tests.

In the meantime you could write your own factories or use the ones in this PR to get you started.

glen-84 commented 7 years ago

@michaelmoussa Are you able to review this?

michaelmoussa commented 7 years ago

@glen-84 @xtreamwayz this looks fine to me - pretty straightforward extraction.

geerteltink commented 7 years ago

Ok, I'll update the tests probably this weekend.

glen-84 commented 7 years ago

Will this make it into the next round of releases (with the query string stuff, etc.)?

weierophinney commented 7 years ago

Thanks, @xtreamwayz! I have incorporated the changes I suggested in my review, and merged to develop at this time.