Skip to content

Added app.token to the list of global variables #7191

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 1 commit into from

Conversation

javiereguiluz
Copy link
Member

@javiereguiluz javiereguiluz commented Nov 28, 2016

This fixes #7167.


Two questions:

  1. do you agree adding a short description to each global variable? request and sessionare so obvious ... but others may not be so obvious to the reader.

  2. The paragraph above this states that:

The ``app`` variable is available everywhere and gives access to many commonly
 needed objects and values. It is an instance of
 :class:`Symfony\\Bundle\\FrameworkBundle\\Templating\\GlobalVariables`.

Are we sure about that class? Shouldn't it be Symfony\Bridge\Twig\AppVariable instead?

Thanks!

@javiereguiluz javiereguiluz changed the base branch from master to 3.2 November 28, 2016 15:25
@HeahDude
Copy link
Contributor

@javiereguiluz thanks for documenting this feature!

For your point 2, both are used :) The one from the framework bundle is used in php templates.

So maybe I should have added it there too...

@xabbuh
Copy link
Member

xabbuh commented Dec 8, 2016

I like adding the explanations. But I think we should merge them into the 2.7 branch first. What do you think?

fabpot added a commit to symfony/symfony that referenced this pull request Dec 13, 2016
…eahDude)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[FrameworkBundle] Added GlobalVariables::getToken()

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony/symfony-docs#7191 comments
| License       | MIT
| Doc PR        | symfony/symfony-docs#7191

I propose this feature as bug fix in 3.2, since I don't use the PHP templating I forgot to add the method in the `FrameworkBundle`, to keep it align with the `TwigBridge` in #19991.

Is this acceptable or should it go in master?

Commits
-------

099b848 Added GlobalVariables::getToken()
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this pull request Dec 13, 2016
…eahDude)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[FrameworkBundle] Added GlobalVariables::getToken()

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony/symfony-docs#7191 comments
| License       | MIT
| Doc PR        | symfony/symfony-docs#7191

I propose this feature as bug fix in 3.2, since I don't use the PHP templating I forgot to add the method in the `FrameworkBundle`, to keep it align with the `TwigBridge` in symfony/symfony#19991.

Is this acceptable or should it go in master?

Commits
-------

099b848 Added GlobalVariables::getToken()
@HeahDude
Copy link
Contributor

HeahDude commented Apr 1, 2017

I agree with @xabbuh that this should go in 2.7 first.

@javiereguiluz
Copy link
Member Author

Closing it in favor of #7808.

xabbuh added a commit that referenced this pull request May 8, 2017
…viereguiluz)

This PR was merged into the 2.7 branch.

Discussion
----------

Improved the description of the global Twig variables

This finished #7191. When merged, don't forget to add this description in 3.2 and master:

```
* ``app.token`` (a :class:`Symfony\\Component\\Security\\Core\\Authentication\\Token\\TokenInterface`
  object representing the security token)
```

Commits
-------

5dafdf6 Improved the description of the global Twig variables
@javiereguiluz javiereguiluz deleted the fix_7167 branch May 24, 2018 16:04
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.

4 participants