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

Fixes #737: Twig validation does not recognize Drupal filters/functions. #743

Merged
merged 1 commit into from
Dec 5, 2016

Conversation

grasmash
Copy link
Contributor

@grasmash grasmash commented Dec 5, 2016

No description provided.

@grasmash grasmash added in progress Bug Something isn't working labels Dec 5, 2016
@grasmash
Copy link
Contributor Author

grasmash commented Dec 5, 2016

Unfortunately, we've got to downgrade this feature to a degree. We're switching to pure syntax checking and ignoring things like filter/function checks.

The previous approach required a real compilation of twig into php, which would have required a full Drupal bootstrap and real implementation of Drupal core's twig extension. That was just a bit too much of a rabbit hole for me to chase.

However, this could be a good feature for Drupal Console, which is container aware. @jmolivas you might be interested in adding twig validation.

protected function execute(InputInterface $input, OutputInterface $output)
{
$loader = new \Twig_Loader_Filesystem($input->getArgument('environment-dir'));
$this->setTwigEnvironment(new \Twig_Environment($loader));
Copy link
Contributor Author

@grasmash grasmash Dec 5, 2016

Choose a reason for hiding this comment

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

If you wanted to go all the way here, you'd actually add Drupal core's Twig extension:

$twig = new \Twig_Environment(NULL, array(
  'debug' => TRUE,
  'cache' => FALSE,
  'autoescape' => 'html',
  'optimizations' => 0
));
$twig->addExtension(new TwigExtension($renderer);

You need a real Drupal container to instantiate $renderer.

@grasmash grasmash merged commit 10b70d3 into acquia:8.x Dec 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant