LiipThemeBundle icon indicating copy to clipboard operation
LiipThemeBundle copied to clipboard

Issue when using "traditional" path notation

Open emulienfou opened this issue 6 years ago • 3 comments

Preconditions

When using the traditional notation for template path, the file is not found.

Steps to reproduce

  • Template not found when using traditional notation of template path, like CmfTreeBrowserBundle:Base:scripts.html.twig.
  • If I change the path to the new notation, like: @CmfTreeBrowser/Base/scripts.html.twig this solve the issue.
  • However, some bundles still use the traditional notation and I think this need to be fixed directly in this bundle.

Possible fix

Here is a fix/test I did to not have this issue. I would loved to have some feedback and if you can think about a better handling about that.

<?php
// ...
class FilesystemLoader extends \Twig_Loader_Filesystem
{
//...
protected function findTemplate($template, $throw = true)
    {
        $logicalName = (string) $template;

        if ($this->activeTheme) {
            $logicalName .= '|'.$this->activeTheme->getName();
        }

        if (isset($this->cache[$logicalName])) {
            return $this->cache[$logicalName];
        }

        $file = null;
        $previous = null;

        try {
            $templateReference = $this->parser->parse($template);
            $file = $this->locator->locate($templateReference);

        } catch (\Exception $e) {
            $previous = $e;

            // for BC
            try {
                $file = parent::findTemplate((string) $template);
            } catch (\Twig_Error_Loader $e) {
                $previous = $e;

               // Traditional notation need to be replaced by the new notation
                try {
                    $template = (string) $template;
                    $template = str_replace('Bundle:', '/', $template);
                    $template = str_replace(':', '/', $template);
                    $file = parent::findTemplate('@' . $template);
                } catch(\Twig_Error_Loader $e){
                    $previous = $e;
                }
            }
        }

        if (false === $file || null === $file) {
            if ($throw) {
                throw new \Twig_Error_Loader(sprintf('Unable to find template "%s".', $logicalName), -1, null, $previous);
            }
            
            return false;
        }

        return $this->cache[$logicalName] = $file;
    }
}

emulienfou avatar Oct 05 '18 03:10 emulienfou

I think that's related to https://github.com/liip/LiipThemeBundle/pull/193#discussion_r222908895.

xabbuh avatar Oct 05 '18 07:10 xabbuh

Tracked it even further down: https://github.com/liip/LiipThemeBundle/pull/193#discussion_r222939731

The TemplateNameParser class seems to be still available in Symfony 4.1, any reason not to use that one?

danrot avatar Oct 05 '18 09:10 danrot

Hello @danrot you're right the next code is the issue even in SF4.1.6:

$twigFilesystemLoaderDefinition->setArguments(array(
    $container->getDefinition('liip_theme.templating_locator'),
    $container->getDefinition('templating.filename_parser')
));

I will need to do some other tests on the main application where I have the issue and had to add this code before confirming if removing this code fix the problem in all cases.

So, the "fix/update" in did above can be removed/ignored because it works without it

emulienfou avatar Oct 05 '18 12:10 emulienfou