Skip to content

[FrameworkBundle] Add flag to sort debug:router output #57651

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

Closed
wants to merge 2 commits into from

Conversation

HMRDevil
Copy link
Contributor

@HMRDevil HMRDevil commented Jul 4, 2024

Q A
Branch? 7.2
Bug fix? no
New feature? yes
Deprecations? no
Issues Closes #54847
License MIT

See #54847

Copy link
Member

@derrabus derrabus left a comment

Choose a reason for hiding this comment

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

Thank you for your proposal. Please add tests.

@HMRDevil
Copy link
Contributor Author

HMRDevil commented Jul 4, 2024

@derrabus This command was not initially considered in the tests. Of course, I can try to write them, but I'm not sure I can do it

@derrabus
Copy link
Member

derrabus commented Jul 4, 2024

@HMRDevil
Copy link
Contributor Author

HMRDevil commented Jul 4, 2024

@derrabus Oh, I only looked at Tests\Command :-) Thanks for the tip

Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

Tested with Symfony Demo, this creates a TypeError.

\ksort($routesArray);
foreach ($routesArray as $name => $route) {
$sortedRoutes->add($name, $route);
$sortedRoutes->addAlias($route->getDefault('_controller'), $name);
Copy link
Member

Choose a reason for hiding this comment

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

I don't get why you add an alias to the controller for every routes.

Not all routes have a controller. Example: _logout_main.

Symfony\Component\Routing\RouteCollection::addAlias(): Argument #1 ($name) must be of type string, null given, called in src/Symfony/Bundle/FrameworkBundle/Command/RouterDebugCommand.php on line 192


if ('method' === $column) {
foreach ($routesArray as $name => $route) {
$methods = empty($route->getMethods()) ? 'ANY' : \implode('|', $route->getMethods());
Copy link
Member

Choose a reason for hiding this comment

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

Since the logic for formatting a list of methods is in the descriptors, it would make sense to move the sorting method into Symfony\Bundle\FrameworkBundle\Console\Descriptor\Descriptor. The method describeRouteCollection has an $options array that could be used to pass the desired sort key.
That would make unit testing a lot easier.

}
}

\ksort($helperArray);
Copy link
Member

Choose a reason for hiding this comment

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

Instead of creating an array and using ksort, you can use usort with a callback that compare the sort key of routes 2-by-2. It think there would be less array variables involved.

@@ -55,6 +55,7 @@ protected function configure(): void
new InputOption('show-aliases', null, InputOption::VALUE_NONE, 'Show aliases in overview'),
new InputOption('format', null, InputOption::VALUE_REQUIRED, \sprintf('The output format ("%s")', implode('", "', $this->getAvailableFormatOptions())), 'txt'),
new InputOption('raw', null, InputOption::VALUE_NONE, 'To output raw route(s)'),
new InputOption('sort', null, InputOption::VALUE_REQUIRED, 'Sort by column name'),
Copy link
Member

Choose a reason for hiding this comment

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

Please add the list of possible values as last parameter, for bash completion.

Suggested change
new InputOption('sort', null, InputOption::VALUE_REQUIRED, 'Sort by column name'),
new InputOption('sort', null, InputOption::VALUE_REQUIRED, 'Sort by column name', null, ['name', 'method', 'scheme', 'host', 'path']),

@OskarStark OskarStark changed the title [FrameworkBundle] Add flag to sort debug:router [FrameworkBundle] Add flag to sort debug:router output Jul 4, 2024
@ghost

This comment has been minimized.

@ghost

This comment has been minimized.

@fabpot
Copy link
Member

fabpot commented Aug 3, 2024

@HMRDevil Are you still interested in finishing this PR? Ask for help if needed.

@HMRDevil
Copy link
Contributor Author

HMRDevil commented Aug 13, 2024

@fabpot Yes, I will continue what I started as soon as I have a little more free time.

@javiereguiluz javiereguiluz added the ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" label Oct 3, 2024
@fabpot fabpot modified the milestones: 7.2, 7.3 Nov 20, 2024
@fabpot
Copy link
Member

fabpot commented Mar 29, 2025

Closing as there is no more activity and some unadressed comments.
Feel free to reopen when you have time.

@fabpot fabpot closed this Mar 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature FrameworkBundle ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" Status: Needs Work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a flag to sort debug:router output by alphabetical order, according to a column name
7 participants