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

use the include() Twig function instead of the tag #5548

Merged
merged 1 commit into from
Jul 25, 2015

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Jul 22, 2015

Q A
Doc fix? yes
New docs? no
Applies to all
Fixed tickets

section that you want to direct the user to. #}
{% include '@WebProfiler/Profiler/toolbar_item.html.twig' with { 'link': true } %}
{{ include('@WebProfiler/Profiler/toolbar_item.html.twig', variables = { 'link': true }) }}
Copy link
Member

Choose a reason for hiding this comment

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

The variables name is completely optional and very verbose. What about dispalying the short version first and mention that, as usual in Twig functions/filters, you can name the variable if you prefer?

{{ include('@WebProfiler/Profiler/toolbar_item.html.twig', { 'link': true }) }}

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, Javier. I don't think we should talk about named arguments here at all as it's a too common concept in Twig to be mentioned in every Twig function we use.

The `include() function`_ is a new Twig feature that's available in Symfony
2.2. Prior, the `{% include %} tag`_ tag was used.
2.3. Prior, the `{% include %} tag`_ tag was used.
Copy link
Member

Choose a reason for hiding this comment

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

the function was introduced in Twig 1.12, supported by all Symfony 2.x releases. Not sure if we need this versionadded at all...

Copy link
Member Author

Choose a reason for hiding this comment

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

In the contribution docs we have it with 2.3. Since Symfony 2.3 was the first version requiring Twig 1.12 or above (Symfony 2.2 could have been used with Twig 1.11) I decided to make the change here. But I don't have any hard feelings about this. It should just be consistent.

@wouterj
Copy link
Member

wouterj commented Jul 22, 2015

👍

@weaverryan
Copy link
Member

Looks good to me - thanks for updating these!

@weaverryan weaverryan merged commit f0d4ea4 into symfony:2.3 Jul 25, 2015
weaverryan added a commit that referenced this pull request Jul 25, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

use the include() Twig function instead of the tag

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | all
| Fixed tickets |

Commits
-------

f0d4ea4 use the include() Twig function instead of the tag
@xabbuh xabbuh deleted the twig-include-function branch July 25, 2015 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants