Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow turning all optimizations off #4051

Closed
wants to merge 1 commit into from

Conversation

Lumaraf
Copy link
Contributor

@Lumaraf Lumaraf commented Apr 24, 2024

Currently setting the TwigEnvironment option optimizers to 0 (OptimizerNodeVisitor::OPTIMIZE_NONE) does not turn of the PrintNode optimization.

@@ -81,7 +82,9 @@ public function leaveNode(Node $node, Environment $env): ?Node
$node = $this->optimizeRawFilter($node);
}

$node = $this->optimizePrintNode($node);
if (self::OPTIMIZE_PRINT === (self::OPTIMIZE_PRINT & $this->optimizers)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would we want to be able to disable this optimization. I didn't provide a way to disable it on purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran into problems with one of my own extension because of missing SourceContexts on nodes created by the PrintNode and TextNode optimizations. The obvious solution to me would be to turn off those or all optimizers, but setting the optimizers property on TwigEnvironment to 0/OptimizerNodeVisitor::OPTIMIZE_NONE still leaves the PrintNode optimization active.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we are fixing the source context, would you agree that there is not need to make this optimization configurable?

Copy link
Contributor Author

@Lumaraf Lumaraf Apr 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The source context fix is sufficient to solve my current problems. I still think this should be fixed though as the optimizers option currently doesn't behave in the way it is described in the doc comment in

Twig/src/Environment.php

Lines 100 to 102 in 4a7de4a

* * optimizations: A flag that indicates which optimizations to apply
* (default to -1 which means that all optimizations are enabled;
* set it to 0 to disable).
as setting it to 0 does not disable all optimizations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's close then.

@fabpot fabpot closed this Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants