Skip to content

Documented the Twig extension priority #9135

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

Closed
wants to merge 2 commits into from

Conversation

javiereguiluz
Copy link
Member

This fixes #9096.

Copy link
Contributor

@linaori linaori left a comment

Choose a reason for hiding this comment

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

LGTM

@Xymanek
Copy link

Xymanek commented Jan 26, 2018

How will this work if the service is also auto configured?

@javiereguiluz
Copy link
Member Author

@Xymanek same as the other tags that allow priority. When using autoconfiguration you get a priority of 0 and can't change it.

@Xymanek
Copy link

Xymanek commented Jan 26, 2018

Maybe it makes sense to mention that in docs? Because someone can think that to change priority they need to add a new tag, ending up with the service being tagged twice.

@javiereguiluz
Copy link
Member Author

Not sure I follow you. You say Because someone can think that to change priority they need to add a new tag ... but that's exactly what you need to do. If you want a priority, you need to add the tag explicitly yourself.

@Xymanek
Copy link

Xymanek commented Jan 26, 2018

If you add a tag to auto configured service, won't that service end up with 2 twig.extension tags? E.g.:

  • Auto configured tag with default priority
  • Manual tag with custom priority

javiereguiluz pushed a commit that referenced this pull request Jan 29, 2018
javiereguiluz added a commit that referenced this pull request Jan 29, 2018
This PR was merged into the master branch.

Discussion
----------

[#9135] some minor tweaks

Commits
-------

825d5f2 [#9135] some minor tweaks
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.

[TwigBundle] Added priority to twig extensions
7 participants