symfony-cmf / Routing

Routing component building on the Symfony Routing component
Other
288 stars 69 forks source link

New cmf_routing.pre_dynamic_generate overrides the route name #134

Closed luishdez closed 9 years ago

luishdez commented 9 years ago

Now all my {{ paths(...) }} try to load 'cmf_routing.pre_dynamic_generate' instead the original name of the route.

I found that the route is being override by the event name

https://github.com/symfony-cmf/Routing/blob/master/DynamicRouter.php#L174

    public function generate($name, $parameters = array(), $referenceType = false)
    {
        if ($this->eventDispatcher) {
            $event = new RouterGenerateEvent($name, $parameters, $referenceType);
            $this->eventDispatcher->dispatch(Events::PRE_DYNAMIC_GENERATE, $event);
            $name = $event->getName(); <------ 
            $parameters = $event->getParameters();
            $referenceType = $event->getReferenceType();
        }
        return $this->getGenerator()->generate($name, $parameters, $referenceType);
    }

Note: That getName() method in event for Symfony 3.0 is deprecated so I guess this will work with 3.0

dbu commented 9 years ago

oh f... sorry for that one. very stupid, we run into a name clash about "name"

i think the only sane solution is to call our thing routeName and setRouteName / getRouteName. luckily there is no release yet of this version.

can you do a pull request to adjust RouterGenerateEvent and the DynamicRouter accordingly? if not, i can do that soonish.

luishdez commented 9 years ago

Sure no problem!

Checking the event I see that this is the name of the route or an instance of Route. Shouldn't we call this just route ? getRouteName() and getting a route instance doesn't seem very logic.

    /**
     * The name of the route or the Route instance to generate.
     *
     * @var string|Route
     */
    private $name;
dbu commented 9 years ago

indeed, forgot about that. yes, very good point. lets call this getRoute then.