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

Point that route parameters are also Request attributes #6219

Closed
wants to merge 2 commits into from

Conversation

sfdumi
Copy link
Contributor

@sfdumi sfdumi commented Feb 2, 2016

Point that route parameters are also Request attributes

Point that route parameters are also Request attributes
@javiereguiluz
Copy link
Member

@sfdumi 👍 I like your proposal because a lot of people are not aware of this. Thank you!

{
// ...
$titleAttribute = $request->attributes->get('title'); // same as $title
Copy link
Contributor

Choose a reason for hiding this comment

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

$titleAttribute is confusing imo, to show this, its enough to write $request->attributes->get('title'); // same as $title

what do you thing @javiereguiluz

Copy link
Member

Choose a reason for hiding this comment

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

I agree with you. We could even split the example in two parts:

// Getting the title via controller arguments

public function indexAction($page, $title)
{
    // ...
}

// Getting the title via request attributes

use Symfony\Component\HttpFoundation\Request;

public function indexAction(Request $request)
{
    $title = $request->attributes->get('title');
    // ...
}

@sfdumi
Copy link
Contributor Author

sfdumi commented Feb 4, 2016

This sounds like a good idea. Should I update the PR?

@@ -7,7 +7,7 @@ How to Pass Extra Information from a Route to a Controller
Parameters inside the ``defaults`` collection don't necessarily have to
match a placeholder in the route ``path``. In fact, you can use the
``defaults`` array to specify extra parameters that will then be accessible as
arguments to your controller:
arguments to your controller, and as attributes of the ``Request`` object:
Copy link
Member

Choose a reason for hiding this comment

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

We change this a bit and only tell the reader that we can access the parameters in the controller:

[...] that will then be accessible in your controller::

@wouterj
Copy link
Member

wouterj commented Feb 6, 2016

I like this addition! If you can and agree with them, please update this PR to add the proposed changes.

Separated the Request attributes example from the method argument one.
@sfdumi
Copy link
Contributor Author

sfdumi commented Feb 9, 2016

Updated the PR.

@javiereguiluz
Copy link
Member

👍 this PR looks ready to be merged. Our doc managers can fix some long lines when merging this.

Thanks @sfdumi.

@wouterj
Copy link
Member

wouterj commented Mar 10, 2016

👍 (I'm very sorry, the update comment completely slipped under my radar)

xabbuh added a commit that referenced this pull request Mar 11, 2016
…sfdumi)

This PR was submitted for the 3.0 branch but it was merged into the 2.3 branch instead (closes #6219).

Discussion
----------

Point that route parameters are also Request attributes

Point that route parameters are also Request attributes

Commits
-------

08aae4e Point that route parameters are also Request attributes
xabbuh added a commit that referenced this pull request Mar 11, 2016
@xabbuh
Copy link
Member

xabbuh commented Mar 11, 2016

Thank you for these improvements @sfdumi! I have merged them into the 2.3 branch.

@xabbuh xabbuh closed this Mar 11, 2016
xabbuh added a commit that referenced this pull request Mar 11, 2016
* 2.3:
  [#6219] some tweaks
  Point that route parameters are also Request attributes
  [#6348] some minor tweaks
  [best practices] mostly typos
  [#6275] some minor tweaks
  [quick tour] mostly typos
  remove link-local IPv6 address (fe80::1)
  [#6305] move link reference to the bottom
  Mention IvoryCKEditorBundle in the Symfony Forms doc
  [#6328] minor tweak
  Update extension.rst - added caution box for people trying to remove the default file with services definitions
  Altered single / multiple inheritance sentence
  Replace XLIFF number ids by strings
xabbuh added a commit that referenced this pull request Mar 11, 2016
* 2.7:
  [#6219] some tweaks
  Point that route parameters are also Request attributes
  [#6348] some minor tweaks
  [best practices] mostly typos
  [#6275] some minor tweaks
  [quick tour] mostly typos
  remove link-local IPv6 address (fe80::1)
  [#6305] move link reference to the bottom
  Mention IvoryCKEditorBundle in the Symfony Forms doc
  [#6328] minor tweak
  Update extension.rst - added caution box for people trying to remove the default file with services definitions
  Altered single / multiple inheritance sentence
  Replace XLIFF number ids by strings
  Rename DunglasApiBundle to ApiPlatform
xabbuh added a commit that referenced this pull request Mar 11, 2016
* 2.8:
  [#6219] some tweaks
  Point that route parameters are also Request attributes
  [#6348] some minor tweaks
  [best practices] mostly typos
  [#6275] some minor tweaks
  [quick tour] mostly typos
  remove link-local IPv6 address (fe80::1)
  [#6305] move link reference to the bottom
  Mention IvoryCKEditorBundle in the Symfony Forms doc
  [#6328] minor tweak
  Update extension.rst - added caution box for people trying to remove the default file with services definitions
  Altered single / multiple inheritance sentence
  Replace XLIFF number ids by strings
  Rename DunglasApiBundle to ApiPlatform
xabbuh added a commit that referenced this pull request Mar 11, 2016
* 3.0:
  [#6219] some tweaks
  Point that route parameters are also Request attributes
  [#6348] some minor tweaks
  [best practices] mostly typos
  Fix reference to app folder
  [#6275] some minor tweaks
  [quick tour] mostly typos
  remove link-local IPv6 address (fe80::1)
  [#6305] move link reference to the bottom
  Mention IvoryCKEditorBundle in the Symfony Forms doc
  [#6328] minor tweak
  Update extension.rst - added caution box for people trying to remove the default file with services definitions
  Altered single / multiple inheritance sentence
  Replace XLIFF number ids by strings
  Rename DunglasApiBundle to ApiPlatform
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.

5 participants