[Twig] added subpath function #3965

Tobion · 2012-04-17T14:37:00Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#4849, partly #6857
License	MIT

The new function subpath merges the variables of the current route request with the ones you optionally passed to the function. On top of that you can also add the query params (disabled by default).

It will generate a path-relative URL once #3958 gets merged but this PR is not necessarily depending on it. Since the target URL, that inherits some params from the current request, is probably a sub-page in the URL hierarchy (which is why it's called subpath), it makes sense to default it to a relative path, which makes the links shorter.

Use cases:

Generating the same route but changing a few parameters while keeping others of the current request (e.g. typical locale switcher).
Generating a different route while removing/overriding/adding some parameters on top of the current params. This is very useful when you have hierarchically structured pages, which is very common.
Example: We are at /user-slug/article-slug/ (article_show) and want to generate the link to comments of the article (article_comments). At the moment we would have to always repeat the current variables like
{{ path('article_comments', {'user': 'user-slug', 'article': 'article-slug'}) }} and would for example get /user-slug/article-slug/comments.
With subpath the advantage is you don't need to specify the variables of the current request again.
So simply {{ subpath('article_comments') }} is enough for the relative URL comments in this case. But you can optionally overwrite some request variables with {{ subpath('article_comments', {'article': 'different-article-slug'} ) }} which would generate ../different-article-slug/comments (i.e. same user, but different article).
Merging the query params can be used for filtering links, e.g. a facetted navigation page that filters items and you want to generate a link to add/overwrite filters based on the current filters. This is also one reason why removing params is needed which can be achieved by passing null as value for the unwanted param.
Generating a route with the same host params. See [Routing] Generating URLs with host matching #6857.
~~Generating the same URL.~~ I really thought much about this use-case. And I came to the conclusion it's not necessary for it to have a specific twig function. Of course you can also generate the same URL with path, url and subpath functions. But with path and url, you need to pass all variables again. And with subpath it will just generate an empty string '' because that's the relative path for the current URL. So you don't need to call subpath at all. And if you need an absolute path or url, then you can just access the request information directly (Request::getRequestUri, i.e. in twig {{ app.request.requestUri }}. It's also faster because it doesn't go through the routing url generation at all. So all in all, it's already easy to get the same url. It would also not have much to do with subpath but instead it would be more appropriate to expose a new function like {{ self() }}. But as I said, it's not really needed and not part of this PR.

stof · 2012-04-17T17:38:35Z

This is indeed the wrong way as it makes Twig unusable from the CLI

Tobion · 2012-04-17T18:07:39Z

So injecting the container?

alexandresalome · 2012-04-18T20:58:02Z

Magic was killed in 2010, and I don't think it's a good thing to generate URLs like this. Probably you should integrate this to your project, maybe a bundle or an article. But I don't think it should be part of the framework.

Tobion · 2012-04-19T09:25:05Z

It's not magic. It just exposes the information (_route_params) that are already stored for these use cases in a well defined method.

sstok · 2012-05-04T09:17:12Z

src/Symfony/Bridge/Twig/Extension/RoutingExtension.php

-    public function getUrl($name, $parameters = array())
+    /**
+     * Returns the path to a route (defaults to current route) with all current route parameters merged
+     * with the passed params. Optionally one can also include the params of the query string.


You should separate the short and long description.

Tobion · 2012-05-17T18:56:04Z

FYI, Zend Framework also has the possibility to merge the current request params as this PR proposes.
See https://github.com/zendframework/zf2/blob/master/library/Zend/View/Helper/Url.php#L83

kix · 2012-09-25T14:00:04Z

+1 here, we do need to have a way to generate links to current path while being able to alter a single parameter and keeping others the same as they were. I18n locale switching is just one of the cases here.
And this should really be achieved by using the Symfony core, not by third-party bundles.

stof · 2012-09-25T14:07:28Z

@kix Symfony 2.1 introduced an attribute making it quite easy to do this locale switching:

{% set route_params = app.request.attributes.get('_route_params') %}

{# merge the query string params if you want to keep them when switching the locale #}
{% set route_params = route_params|merge(app.request.query.all) %}

{# merge the additional params you want to change #}
{% set route_params = route_params|merge({'_locale': 'fr'}) %}

{{ path(app.request.attributes.get('_route'), route_params) }}

kix · 2012-09-25T14:23:33Z

I know this should be working well, though it seems like it's way too much
template code for such a primitive task. Also we seem to be putting
extraneous logic into the template, and that seems like breaking the
separation of concerns principle.

Yet, thanks for the snippet, I assume I'll stick to it till there's a
better way.

stof · 2012-10-13T16:08:25Z

src/Symfony/Bridge/Twig/Extension/RoutingExtension.php

+     * @param ContainerInterface    $container A ContainerInterface instance
+     * @param UrlGeneratorInterface $generator A UrlGeneratorInterface instance
+     */
+    public function __construct(ContainerInterface $container, UrlGeneratorInterface $generator)


Depending on the container makes it impossible to use this extension in a project using Twig and Routing without using the DI component (Silex projects for instance)

So should i make it __construct(UrlGeneratorInterface $generator, ContainerInterface $container = null)
and then assert that the container is set in getSubPath?

@stof would that be better? Silex would only not be able to use the subPath function then, or?

@stof / @fabpot ping

Yeah but when using on-invalid="ignored" the setter will not be called when request is null. Should work just as fine, doesnt it?

The setter must be called so that your object knows that it is acting outside the handling of a request. If not, you will still have access to an older Request object.

Hm, actually we don't really want the subrequest info. Only the master request matters. When generating a relative path in a subrequest (e.g. a partial with ESI), it would be wrong from a browser perspective. Because the browser would resolve the relative link based on the master/main/user-entered URL. So the relative link in a subrequest probably goes to the wrong target.

But this is just a documentation thing and it should be clear that relative paths in a subrequest don't make much sense. This is nothing special for the new subpath function; the same applies for the url and path methods.

this is then also fixed by the request stack

Tobion · 2012-10-27T12:03:30Z

There is an issue that makes it impossible to really re-generate the current URL:
The _route_params attribute in the request also contains the merged default values. So when you use these params to generate the URL, it might not really be the current URL. Example: new Route('/{page}.{_format}', array('_format' => 'html') which matches the URL /page.html. So _route_params = array('page' => 'page', '_format' => 'html').
When regeneting the URL it will produce /page without the format because the default it left out (see #5180), so it's not the current URL. With #5410 it would be the other way round, so still a problem.

So the only way to fix it would be that _route_params only contains the matched params without default and a new attribute like _route_defaults contains the defaults. But the current Listener and UrlMatcher does not allow to distinguish these values.

Tobion · 2012-12-15T14:15:46Z

As there is no way to really regenerate the current URL with the UrlGenerator (see above), we cannot use it in this case. But I just figured we don't need to. We can simply use Request::getUri or Request::getRequestUri to achieve that. It's also more straight-forward because why would we need to regenerate the Url that we already have available...

Tobion · 2013-03-14T14:59:05Z

~~I added an url generation mode that returns the shortest path.~~ This is not required for this PR. But I think it's a good idea because only the UrlGenerator already has this information available so why not expose this possibility.

stof · 2013-03-14T15:13:16Z

src/Symfony/Component/Routing/Generator/UrlGeneratorInterface.php

+     * Generates a relative or absolute path depending on which is shorter,
+     * e.g. "/dir/file" vs "../dir/file".
+     */
+    const SHORTEST_PATH = 'short';


This should be a separate PR as it is a separate feature (and I don't really see a need for it)

Yeah I will remove it. Was just experimental.

fabpot · 2013-09-17T09:42:06Z

src/Symfony/Bridge/Twig/Extension/RoutingExtension.php

    public function getPath($name, $parameters = array(), $relative = false)
    {
        return $this->generator->generate($name, $parameters, $relative ? UrlGeneratorInterface::RELATIVE_PATH : UrlGeneratorInterface::ABSOLUTE_PATH);
    }

-    public function getUrl($name, $parameters = array(), $schemeRelative = false)
+    /**
+     * Sets the current request that is needed for the subpath function.


You should use the request stack instead.

I don't understand what u mean

Instead of relying on a setRequest method, you can take the request stack as an argument and call ->getMasterRequest() whenever you need the request. That way, you always have the right master request. The current way won't work if you handle two master requests in the same PHP process (without rebooting the Kernel of course.)

ah i see. it's about #8904

Tobion · 2013-09-17T14:09:01Z

@fabpot this is ready. I updated the main description.

Tobion · 2013-09-17T17:38:14Z

src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml

@@ -81,6 +81,7 @@

        <service id="twig.extension.routing" class="%twig.extension.routing.class%" public="false">
            <argument type="service" id="router" />
+            <argument type="service" id="request_stack" />


twig bundle doesnt have the framework bundle as dependency yet. but since the twig bundle relies on service definitions in framework bundle, shouldn't it be the case?!

no, the Twig bridge is used by Silex without the definition as we are using Pimple there, so this is not a hard dependency.

this is the twig bundle, not the bridge.

use the current route by default add possibility to remove existing params by passing null for it added tests for the twig routing extension use synchronized request

Tobion · 2013-09-30T08:22:00Z

@fabpot ping ready

fabpot · 2013-10-01T09:31:39Z

@Tobion Documentation is missing

fabpot · 2013-10-01T09:36:32Z

The more I think about it, the more I find the feature confusing. The probability that a route takes the same arguments as the current one is rather thin. I know that you can remove arguments by setting them to null, but then, I think it's better to just pass the arguments you need. And if you use subpath in a partial that is included in more than one page (so more than one URL), the behavior is not predictable anymore.

I can definitely see the use case for variation of the current route, but then, we won't need the route name as an argument.

So, I'm still -1 on adding this in core as is.

fabpot · 2013-10-01T09:49:16Z

Let me explain my feeling a bit more.

Let's say that my current page is /articles/Foo/12.

This URL has been generated with {{ path('article', { id: 12, category: 'Foo' }) }}

Now, on the page, I want to generate a link to the next article, 13, using {{ subpath('article', { id: 14 }) }} looks wrong as this is not a sub path of the current page.

I do understand the usefulness of the feature but the implementation does not convey the right meaning.

What about doing something like: {{ path('article', { id: 14 }|merge_current_path_attributes) }} instead (replace merge_current_path_attributes with whatever makes more sense and is shorter)?

Let's take another example, a list: {{ path('articles', { sort: 'desc', q: 'foo', page: 4 }) }}

I have several link to change the different values of the route attributes. I can do {{ subpath('articles', { page: 5 }) }} to go to page 5 or {{ path('articles', { page: 5 }|merge_current_path_attributes }}. Again, I prefer the second one as the page is not a sub path of the current page.

Of course, we could also be able to remove some attributes: {{ path('articles', { page: 5 }|merge_current_path_attributes({ q: null }) }}.

Tobion · 2013-10-01T11:22:52Z

So you are just not satisfied with the name subpath. The idea behind the name is that is expresses that the current request attributes are reused and that the resulting reference is relative. That the resulting URL must not be a "subpage" depends on the route defintions of the developer which we cannot influence. The name should just intend what it is designed for. The possibilily to also generate "siblings" or "parents" is just the flexibility. So I'm open to other name suggestions. But the argument that does not necessarily fit the resulting URL would also be true for path itself. Because the path function can also generate full URLs (when host/scheme is different). So you could say it's just as confusing. So again depends on the route definitions that path() does not generate a path, but a URL.

{{ path('articles', { page: 5 }|merge_current_path_attributes }}

Could work as well. But stilll some problems

What's the best name?
There is still no way to reuse the current route name (except app.request.attributes.get(_route) which is not really friendly).
How to optionally include the query params?

stof · 2013-10-01T11:30:38Z

but the only feature added by this PR is the easy reuse of attributes. Generating a relative url is already supported since 2.2 through path(..., ..., true), which will call the url generator with UrlGeneratorInterface::RELATIVE_PATH

Tobion · 2013-10-01T11:32:01Z

@stof I think nobody said something different.

stof · 2013-10-01T11:35:35Z

quoting you: The idea behind the name is that is expresses that the current request attributes are reused and that the resulting reference is relative.. Making the url relative is not a feature added by this method.

thus, the filter-based implementation proposed by @fabpot above would be compatible with the 4 url generation styles, not only with the relative path.

Tobion · 2013-10-01T11:41:31Z

Huh? I didnt say its a feature added here. I just said why I chose the name subpath because that is what fabpot does not agree with. But I'm open to any other name if you find one.

fabpot · 2013-10-01T12:39:03Z

I think you are both right ;) Indeed I don't like the name, but @stof is right as the feature you want to add can be more easily done with a filter and would work with all generator strategies.

Tobion · 2013-10-01T12:43:53Z

I still don't see how to use the current route with a filter without bc break. So if the 3 problems are solved above, I'm fine using a filter. Although I find a seperate function more clean.

Tobion · 2013-10-01T12:51:07Z

If we don't provide a shortcut, we could say, we don't actually need to do anything. People can already use something like {{ path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')|merge({ page: 5 })) }}. Looks quite strange, but it's using ur filter suggestion.

dewos · 2013-10-01T13:27:06Z

A shortcut like {{ path(app.request.route, app.request.params|merge({ page: 5 })) }}? Will not be BC?

stof · 2013-10-01T13:32:09Z

@dewos There is no reason to have a getRoute() getter on the Request object

ErichHartmann · 2013-10-27T15:33:31Z

From a neophye's perspective, I see a lot of value in easily generating new paths in the response from the request's route's attributes. I agree, however, with @stof and @fabpot that it is not a "sub" path. It is not a hierarchy, but instead the request's attributes used to generate a whole new path in the response (atomic?).

I do think the @Tobion idea for a much more simplified filter is important, because as a new implementer, I would never conceive of something as intricate as:

{{ path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')|merge({ page: 5 })) }}

but...

{{ path('articles', { page: 5 }|merge_request_path_attributes }}

or...

{{ path('articles', { page: 5, q: null, _locale: de-DE }|merge_request_path_attributes }}

... readily makes sense to me and conveys the intent of generating a new path in the response using the request's path's attributes.

(notice the small edit I made to the @fabpot example above -- it's not the "current" path, but belongs to the request's path/route in my mind. The is no temporal or hierarchical component to the function.)

I hope this can yet make it into 2.4.

bierdok · 2013-10-28T11:09:27Z

Why not make the use of the underscore generic in the router ?

For example,

host: {_subdomain}.acme.org
pattern: {_category_id}/{article_id}.html

As we do today, I think about the variable "_locale" that does not need to be redeclared each url generation.

I found a solution to do this properly.

Just add a route listener

namespace Acme\CoreBundle\EventListener;

use Symfony\Bundle\FrameworkBundle\Routing\Router;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;

class RouteListener implements EventSubscriberInterface
{
    private $router;

    public function __construct(Router $router)
    {
        $this->router = $router;
    }

    public function onKernelRequest(GetResponseEvent $event)
    {
        $request = $event->getRequest();

        $parameters = $request->attributes->get('_route_params');

        if($parameters) {
            foreach($parameters as $name => $value) {
                if(substr($name, 0, 1) == '_') {
                    $this->router->getContext()->setParameter($name, $value);
                }
            }
        }
    }

    public static function getSubscribedEvents()
    {
        return array(
            KernelEvents::REQUEST => array(array('onKernelRequest', 16)),
        );
    }
}

and instantiate it as a service

services:
    core.route_listener:
        class: Acme\CoreBundle\EventListener\RouteListener
        arguments:
            - "@router"
        tags:
            - { name: kernel.event_subscriber }

I hope this can help you. ;-)

Lumbendil · 2013-10-29T14:23:43Z

The problem with {{ path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')|merge({ page: 5 })) }} is that it's not user friendly... then how about {{ path(app.currentRoute, app.currentRouteParams|merge({ page: 5 })) }} ? This could be done by adding the GlobalVariables::getCurrentRoute() and GlobalVariables::getCurrentRouteParams() methods.

Koc · 2014-07-30T11:47:06Z

Very useful function. Is it possible ship it with 2.6?

Tobion · 2014-07-30T12:31:36Z

We cannot agree how to provide this feature. So I don't think it will make it to 2.6

I'm summing up the proposals and arguments against it:

Using a new function like {{ subpath('articles', { 'page': 5 } ) }} as this PR suggests.
- better name?
- not clear enough what it does?
Using a new filter like {{ path('articles', { 'page': 5 }|merge_current_path_attributes }}
- this would inverse the logic of merge as normally the second params overwrite the first. but here it's the other way round
- better name?
- how to reuse the current route name
- how to optionally include the query params
Not provide a shortcut at all and let users do somthing like {{ path(app.request.attributes.get('_route'), app.request.attributes.get('_route_params')|merge({ page: 5 })) }}
- not user-friendly
- no way to remove params
- things like query params merging has to be implemented again and again
- manually merging query params can lead to wrong implementations that break the site, e.g. giving query params more priority than path placeholders allows hackers to break all links
Provide twig globals to ease accessing stuff like {{ path(app.currentRoute, app.currentRouteParams|merge({ page: 5 })) }}
- more user-friendlyl but same problems as in 3)

jakzal · 2014-07-31T19:39:24Z

How about app.request.attributes.merge('_route_params', {page: 5})?

jakzal · 2014-07-31T19:40:24Z

On a second thought it's not a good idea, since _route_params might not be an array in case of a request attribute.

wouterj · 2015-07-27T22:10:59Z

ping @xabbuh @jakzal @stof @fabpot @Tobion can we please decide on the solutions mentioned in #3965 (comment) ?

xabbuh · 2015-07-29T07:27:58Z

In my opinion, option four is a step in the right direction. The app.currentRoute and app.currentRouteParams globals could also be useful outside of creating URLs/paths.

Some things we then still need to solve are:

removing params
merging query params

For the first issue, I could imagine that this is something that could be added to Twig itself (like it is done the other way around with the merge filter).

For the second issue, I don't have a concrete idea right now. Maybe adding another Twig global or function could be the solution.

fabpot · 2015-07-29T08:12:58Z

It really depends on which use cases we are talking about. From the issue description, I think that only the first one is relevant.

fabpot · 2016-06-09T07:03:05Z

I'm closing this very old PR as there is no activity, and no consensus on what to do.

sstok reviewed May 4, 2012
View reviewed changes

Tobion mentioned this pull request Jul 11, 2012

[2.2] [Routing] Re-generating the current URL #4849

Closed

stof reviewed Oct 13, 2012
View reviewed changes

stof reviewed Mar 14, 2013
View reviewed changes

Tobion mentioned this pull request Mar 18, 2013

[Routing] allow URLs to be generated that include the default param #5410 A3E2

Closed

fabpot reviewed Sep 17, 2013
View reviewed changes

Tobion reviewed Sep 17, 2013
View reviewed changes

Tobion added 2 commits September 18, 2013 14:05

[Twig] added subpath function

b3e68d0

use the current route by default add possibility to remove existing params by passing null for it added tests for the twig routing extension use synchronized request

use the request stack instead of synchronized request

e8fda4d

Tobion mentioned this pull request Oct 25, 2013

Conditional routing by multiple hosts #9373

Closed

bierdok mentioned this pull request Oct 29, 2013

[Routing] 'host' routes using 'requirements' does not pass through mandatory parameters #7538

Closed

sintanial mentioned this pull request Mar 1, 2014

How to generate url with host ? FriendsOfSymfony/FOSJsRoutingBundle#134

Closed

Tobion changed the title ~~[2.4][Twig] added subpath function~~ [Twig] added subpath function Dec 9, 2014

Tobion mentioned this pull request Dec 9, 2014

All route variables from the current route... #12901

Closed

fabpot closed this Jun 9, 2016

Tobion deleted the subpath branch September 10, 2019 19:51

Uh oh!

[Twig] added subpath function #3965

[Twig] added subpath function #3965

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!