[TwigBundle] removed usage of Templating classes #15970
Conversation
fabpot
commented
Sep 28, 2015
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | yes (but frankly, I don't see how that would break anything out there) |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | n/a |
| License | MIT |
| Doc PR | n/a |
| @@ -63,7 +61,7 @@ public function showAction(Request $request, FlattenException $exception, DebugL | |||
| $code = $exception->getStatusCode(); | |||
|
|
|||
| return new Response($this->twig->render( | |||
| (string) $this->findTemplate($request, $request->getRequestFormat(), $code, $showException), | |||
| $this->findTemplate($request, $request->getRequestFormat(), $code, $showException), | |||
There was a problem hiding this comment.
That's kind of useless to use a TemplateReference everywhere as Twig only know about plain string templates anyway. It was needed before we added support for proper Twig namespace paths, which we did earlier this year. This is a leftover that forbids to disable the templating component in 2.8/3.0.
There was a problem hiding this comment.
this is a protected method, meaning it is an extension point. I think we should keep the string casting for now, for cases where a child class returns a TemplateReferenceInterface (which is likely as it was the expected output).
Casting the TemplateReferenceInterface will give you a template name, and so will work.
There was a problem hiding this comment.
I can indeed do that even if I doubt anyone has ever used this "extension point".
There was a problem hiding this comment.
@fabpot FOSRestBundle is using it (it was the reason it was introduced at first btw)
There was a problem hiding this comment.
we could add a deprecation warning when it doesn't return a string
15b8cdf to
81cdf2f
Compare
| @@ -1,4 +1,4 @@ | |||
| {% extends 'TwigBundle::layout.html.twig' %} | |||
| {% extends '@TwigBundle/layout.html.twig' %} | |||
There was a problem hiding this comment.
@Twig instead of @TwigBundle ?
81cdf2f to
2f62cfc
Compare
|
For the record, this notation has been available since Symfony 2.2 and Twig 1.10. |
2f62cfc to
ff3c107
Compare
|
👍 Status: Reviewed |
This PR was merged into the 2.8 branch. Discussion ---------- [TwigBundle] removed usage of Templating classes | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | yes (but frankly, I don't see how that would break anything out there) | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a Commits ------- ff3c107 [TwigBundle] removed usage of Templating classes 7f13f95 [WebProfilerBundle] fixed a template reference
…s (fabpot) This PR was merged into the 2.8 branch. Discussion ---------- [TwigBundle] removed usage of Templating classes | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | yes (but frankly, I don't see how that would break anything out there) | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a Commits ------- ff3c107 [TwigBundle] removed usage of Templating classes 7f13f95 [WebProfilerBundle] fixed a template reference