Problem/Motivation

grep parameters core/**/RouteProvider.php|grep -vF '  * '
  public function getRouteByName($name, $parameters = array()) {
    $routes = $this->getRoutesByNames(array($name), $parameters);
  public function getRoutesByNames($names, $parameters = array()) {

Proposed resolution

Remove parameters? Make it do something?

Remaining tasks

User interface changes

API changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Crell’s picture

Category: Bug report » Support request
Status: Active » Fixed

From RouteProviderInterface, in the Routing CMF vendor package:

    /**
     * Find many routes by their names using the provided list of names.
     *
     * Note that this method may not throw an exception if some of the routes
     * are not found or are not actually Route instances. It will just return the
     * list of those Route instances it found.
     *
     * This method exists in order to allow performance optimizations. The
     * simple implementation could be to just repeatedly call
     * $this->getRouteByName() while catching and ignoring eventual exceptions.
     *
     * @param array $names      the list of names to retrieve
     * @param array $parameters DEPRECATED the parameters as they are passed to
     *      the UrlGeneratorInterface::generate call. (Only one array, not one
     *      for each entry in $names.
     *
     * @return \Symfony\Component\Routing\Route[] iterable thing with the keys
     *      the names of the $names argument.
     */

It's a deprecated and not useful part of the interface. We can/should just ignore it.

chx’s picture

Category: Support request » Bug report
Status: Fixed » Active
     * @param array $parameters DEPRECATED the parameters as they are passed to
     *      the UrlGeneratorInterface::generate call. 

They are not passed anywhere. Fix the doxygen or fix the code. You can't just ignore this, deprecated or not as long as it is there. The proper doxygen might be "DEPRECATED and doesn't do anything." silly as it is.

Crell’s picture

Category: Bug report » Support request
Status: Active » Fixed

The interface and therefore the docblock are defined in Routing CMF. If you want to change the language of the docblock file a Pull Request here:

https://github.com/symfony-cmf/Routing

dawehner’s picture

Title: parameters is not doing anything in RouteProvider » Upgrade symfony CMF to 1.2 to no longer have paramaters in the interface, because it is pointless
Category: Support request » Bug report
Status: Fixed » Needs review
FileSize
85.19 KB

Here comes the thing ...

dawehner’s picture

Category: Bug report » Task
Priority: Normal » Major

meh. it is probably rather that status.

Status: Needs review » Needs work

The last submitted patch, 4: 2316537.patch, failed testing.

lsmith77’s picture

FYI .. Routing 1.3 will likely be released in September but this time there are not major changes expected:
https://github.com/symfony-cmf/Routing/compare/1.2.0...master

However there is an open unfinished PR to add a RangedRouteProviderInterface by Daniel Wehner:
https://github.com/symfony-cmf/Routing/pull/101

chx’s picture

To understand why this is important: because this leads to the understanding that routs are 'bare' objects. If you pass in parameters you'd think (I wasn't sure!) that somehow the route has those parameters.

dawehner’s picture

Status: Needs work » Needs review
Issue tags: +WSCCI
FileSize
838 bytes
86.01 KB

To understand why this is important: because this leads to the understanding that routs are 'bare' objects. If you pass in parameters you'd think (I wasn't sure!) that somehow the route has those parameters.

Well, you sadly got moved to the wrong assumptions by that parameters. Technically having that combination (route name, parameters (and route))
is something which does not exist at a concept yet, neither in symfony/Routing nor in CMF. I could imagine though that it could
be really useful for those systems as well. Here is an issue in symfony to discuss it: https://github.com/symfony/symfony/issues/11642

We needed to fix some of our unit tests as well.

@chx
Given that I can understand that you might feel that I hijacked the issue, sorry in case.

@lsmith77
Ha, now you really want to force me getting that PR in :)

chx’s picture

Title: Upgrade symfony CMF to 1.2 to no longer have paramaters in the interface, because it is pointless » Upgrade symfony CMF to 1.2 to remove confusing parameters from getRouteByName*

No, you didn't hijack the issue, I appreciate any solution to this.

> Technically having that combination (route name, parameters (and route)) is something which does not exist at a concept yet, neither in symfony/Routing nor in CMF.

But it does in Drupal, right? Url.

dawehner’s picture

But it does in Drupal, right? Url.

Exactly, this is why I proposed that in the symfony issue as it is. The RouteMatch is a separate issue but certainly related.

Crell’s picture

Title: Upgrade symfony CMF to 1.2 to remove confusing parameters from getRouteByName* » Upgrade symfony CMF to 1.2, remove deprecated parameters from getRouteByName*
Status: Needs review » Reviewed & tested by the community

Keeping up with our upstream dependencies is always appropriate, IMO, regardless of parameters or not. Thanks, dawehner!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 9: 2316537-9.patch, failed testing.

dawehner’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
84.51 KB

Reroll

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed c8bd0b4 and pushed to 8.0.x. Thanks!

  • alexpott committed c8bd0b4 on 8.0.x
    Issue #2316537 by dawehner | chx: Upgrade symfony CMF to 1.2, remove...
chx’s picture

Title: Upgrade symfony CMF to 1.2, remove deprecated parameters from getRouteByName* » Upgrade symfony CMF to 1.2, remove confusing parameters from getRouteByName*

*sigh*

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.