Skip to content

Commit

Permalink
merged branch Seldaek/router_def (PR #3437)
Browse files Browse the repository at this point in the history
Commits
-------

09b1bd5 [HttpKernel] Remove the _controller since it is not a route parameter part of the url

Discussion
----------

[HttpKernel] Remove the _controller since it is not a route parameter part of the URL

There is no reason for the _controller to be there, the whole idea behind this _route_params thing was to help re-generating the current page's URL, you can easily grab the _route + _route_params and reconstruct it without having lots of garbage as query parameters like `?_controller=Foo::..`

---------------------------------------------------------------------------

by fabpot at 2012-02-24T10:29:01Z

I agree but isn't it a BC break? I mean, someone may rely on `_controller` in his code.

---------------------------------------------------------------------------

by Seldaek at 2012-02-24T11:45:46Z

This is a new 2.1 feature AFAIK so no it's not breaking anything. If _controller is deemed necessary then we should add it on the attributes, but not in the _route_params IMO.

---------------------------------------------------------------------------

by stof at 2012-02-24T13:32:41Z

indeed, ``_route_params`` is new in 2.1
  • Loading branch information
fabpot committed Feb 27, 2012
2 parents 3f71012 + 09b1bd5 commit b3da94d
Showing 1 changed file with 1 addition and 0 deletions.
Expand Up @@ -61,6 +61,7 @@ public function onKernelRequest(GetResponseEvent $event)

$request->attributes->add($parameters);
unset($parameters['_route']);
unset($parameters['_controller']);
$request->attributes->set('_route_params', $parameters);
} catch (ResourceNotFoundException $e) {
$message = sprintf('No route found for "%s %s"', $request->getMethod(), $request->getPathInfo());
Expand Down

2 comments on commit b3da94d

@powah
Copy link

@powah powah commented on b3da94d Mar 8, 2012

Choose a reason for hiding this comment

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

There is no tests for checking default request attributes?

@Seldaek
Copy link
Member

@Seldaek Seldaek commented on b3da94d Mar 8, 2012

Choose a reason for hiding this comment

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

I guess not, if you'd like to submit some it would be great.

Please sign in to comment.