8000 [TwigBundle] Bundle inheritance is not supported for @ notation · Issue #6919 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[TwigBundle] Bundle inheritance is not supported for @ notation #6919

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.

Sign up for GitHub

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

Jump to bottom
Closed
vicb opened this issue Jan 30, 2013 · 14 comments
Closed

[TwigBundle] Bundle inheritance is not supported for @ notation #6919

vicb opened this issue Jan 30, 2013 · 14 comments
Labels
Bug TwigBundle

Comments

@vicb
Copy link
Contributor
vicb commented Jan 30, 2013

By looking at the code, I am under the impression that the Bundle inheritance is not supported by the current code.
@vicb
Copy link
Contributor Author
vicb commented Feb 9, 2013

Thinking more about this, I think it was a mistake to make Twig parse the template names. There is now two separate paths (Twig & the Framework Bundle) to parse templates and this will lead to issues (ie this one).

To me the best way to solve this problem would be to implement the @ notation parsing in the Template Parser and delegate all parsing to this (ie no more parsing in Twig). The work has been started in #6918.

@stof
Copy link
Member
stof commented Feb 11, 2013

but this would revert the fact that templates are usable outside Symfony when using Twig (for instance using the profiler in Silex)

@stof
Copy link
Member
stof commented Feb 11, 2013

and I think Twig can support the bundle inheritance for paths as it supports registering several paths for each namespace. It simply need to be handled when registering the paths which is not the case currently.

@vicb
Copy link
Contributor Author
vicb commented Feb 11, 2013

@stof yes it can support them, which is the meaning of my first message.

The meaning of my second message is that in spite of the fact that Twig NS could be should in the context of Sf, they should not because they do not have the same meaning than Sf template names. We could use array to emulate this feature but still we would have 2 paths (template parser & twig) to do the same thing. Which will lead to issues in the future.

If someone extends the template name parser, it would work for some templates but not for others. Which is bad !

@vicb vicb mentioned this issue Feb 11, 2013
Closed
5 tasks
@stof
Copy link
Member
stof commented Feb 11, 2013

@vicb The fact that the @ notation is the Twig native notation is documented AFAIK.

@vicb
Copy link
Contributor Author
vicb commented Feb 11, 2013

I don't get what you mean here ? You want to have 2 different notations ? (ie @ not supported in controllers)

@stof
Copy link
Member
stof commented Feb 11, 2013

not supported in controllers ? You mean to reference them ? We already have 2 different (similar) notations as one is referencing a class while the other is referencing a file

@vicb
Copy link
Contributor Author
vicb commented Feb 11, 2013

??? (Maybe it's becoming too late for me today)

I mean @Template("@//") annotation (/yml /xml) and return $this->render("@//")

@stof
Copy link
Member
stof commented Feb 12, 2013

why isn't it supported ? $this->render("@//") will ask Twig to render the template, and it supports the namespaced paths of course

@vicb
Copy link
Contributor Author
vicb commented Feb 12, 2013

I am afraid you're wrong which is why we had trouble to understand ourselves, see the linked PR.

@vicb vicb mentioned this issue Mar 24, 2013
Closed
@webmozart webmozart mentioned this issue Apr 23, 2013
Merged
@sstok sstok mentioned this issue Aug 30, 2013
Closed
@Koc
Copy link
Contributor
Koc commented Apr 9, 2014

Any news on this issue?

@Tobion Tobion mentioned this issue Jun 6, 2014
Closed
@sstok sstok mentioned this issue Nov 15, 2014
Closed
@Tobion Tobion mentioned this issue Feb 17, 2016
Closed
@wesleylancel
Copy link
Contributor

@Tobion: sorry for duping. Is this something that might still be fixed or does it need to discussed first?

@veloxy
Copy link
veloxy commented Aug 9, 2016

Is this going to be fixed/what is the status on this? It has been open since 2013 ...

I'm running into this issue in several places, one of those places is the TwigBundle error views. I'm not able to override them with my own.

The docs mention I can override the error page views the same way you override other bundle views. But this method only works with the "old" notation (AcmeDemoBundle:Default:index.html.twig), not with the @ notation.

I can't find much about the @ notation in the docs either, it's not even mentioned in the best practices.

Note that it works fine when the files are located in the app/Resources/views/ directory, but not within a bundle that has TwigBundle as a parent.

Example:

src\
|_ SymfonyResources\
|__ TwigBundle\
|___ Resources\
|____ views\
|_____ Exception
|______ error.html.twig
|______ error404.html.twig
|______ ...

<?php

namespace SymfonyResources\TwigBundle;

use Symfony\Component\HttpKernel\Bundle\Bundle;

class SymfonyResourcesTwigBundle extends Bundle
{
    public function getParent()
    {
        return 'TwigBundle';
    }
}

I believe this should work, but even non-symfony bundles are having the same issue. Nothing that uses the @ notation for templating is inheritable via another bundle.

@wesleylancel wesleylancel mentioned this issue Aug 10, 2016
Closed
@mvrhov
Copy link
mvrhov commented Aug 10, 2016

What's more interesting is that when @ notation was introduced was said that his will be the recommended way of accessing the templates and the old one will be deprecated.

@ro0NL ro0NL mentioned this issue Aug 12, 2016
Closed
4 tasks
@VasekPurchart VasekPurchart mentioned this issue Nov 21, 2016
Closed
@xabbuh xabbuh mentioned this issue Dec 28, 2016
Merged
fabpot added a commit that referenced this issue Jan 11, 2017
@fabpot
bug #19586 [TwigBundle] Fix bug where namespaced paths don't take par…
17ce5f5 
…ent bundles in account (wesleylancel)

This PR was squashed before being merged into the 2.7 branch (closes #19586).

Discussion
----------

[TwigBundle] Fix bug where namespaced paths don't take parent bundles in account

| Q | A |
| --- | --- |
| Branch? | 2.7 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #6919 |
| License | MIT |
| Doc PR |  |

Currently namespaced paths for templates such as `{% extends '@App/Layout/layout.html.twig' %}` do not work with bundles that have overruled templates using the `getParent()` method in another bundle. See attached ticket. This change prepends the path of the bundle implementing `getParent()` to the paths of the namespace of bundle returned as a parent.

Commits
-------

0c77ce2 [TwigBundle] Fix bug where namespaced paths don't take parent bundles in account
@fabpot fabpot closed this as completed Jan 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug TwigBundle
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[2.3] [WIP][FrameworkBundle] Add support for the '@Bundle/Path/template.format.engi... vicb/symfony
7 participants
@fabpot @Koc @vicb @stof @mvrhov @veloxy @wesleylancel
0