[TwigBridge][TwigBundle][HttpKernel] prefer getSourceContext() over getSource() #20440
Conversation
| Q | A |
|---|---|
| Branch? | 2.7 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #20435 |
| License | MIT |
| Doc PR | n/a |
| @@ -75,14 +75,14 @@ public function exists($name) | |||
|
|
|||
| $loader = $this->environment->getLoader(); | |||
|
|
|||
| if ($loader instanceof \Twig_ExistsLoaderInterface) { | |||
| if ($loader instanceof \Twig_ExistsLoaderInterface || method_exists($loader, 'exists')) { | |||
There was a problem hiding this comment.
In Twig 2.0, the exists() method will be present in the Twig_LoaderInterface.
There was a problem hiding this comment.
This is not needed as Twig_ExistsLoaderInterface still exist in 2.0. It will be removed in 3.0 only.
There was a problem hiding this comment.
The call to exists() will never be made though if the loader does not implement Twig_ExistsLoaderInterface, but is just an instance of Twig_LoaderInterface.
There was a problem hiding this comment.
And this seems to happen. Otherwise, I don't see how #20435 would ever reach the getSource() call.
| $loader->getSourceContext($template); | ||
| } else { | ||
| $loader->getSource($template); | ||
| } |
There was a problem hiding this comment.
I would suggest: ... } else if (method_exists($loader, 'getSource')) { ... instead of line 150
| return $loader->exists($template); | ||
| } | ||
|
|
||
| try { | ||
| $loader->getSource($template); | ||
| if (method_exists($loader, 'getSourceContext')) { |
There was a problem hiding this comment.
👍 as HttpKernel does not constrain Twig version.
|
fabbot failures are unrelated. |
|
Thank you @xabbuh. |
…xt() over getSource() (xabbuh) This PR was merged into the 2.7 branch. Discussion ---------- [TwigBridge][TwigBundle][HttpKernel] prefer getSourceContext() over getSource() | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #20435 | License | MIT | Doc PR | n/a Commits ------- adbc529 prefer getSourceContext() over getSource()
This PR was merged into the 1.x branch. Discussion ---------- add machine-readable version constants This will make it easier to implement version depending features (see symfony/symfony#20440 (comment) for an example). Commits ------- bf07db4 add machine-readable version constants
|
good job @xabbuh |