Skip to content

[FrameworkBundle] Only show relevant columns in debug:router call and adding colors #59780

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 23, 2025
Merged

[FrameworkBundle] Only show relevant columns in debug:router call and adding colors #59780

merged 1 commit into from
Aug 23, 2025
+221 −30

Conversation

mamazu
Copy link
Contributor

@mamazu mamazu commented Feb 14, 2025 •
edited
Loading

Q A
Branch? 7.3
Bug fix? no
New feature? yes
Deprecations? yes
Issues Fix #59767
License MIT

What I did

  • Adding more tests for the descriptor (empty route collection)
  • Always showing the controllers in the list view (we but not removing the --show-controllers so it's not a BC break)
  • Only show the "Host" and "Schema" columns when the values are != "ANY"
  • Adding colors to the HTTP methods (it's the same colors Swagger uses)

Why

  • Color coding the HTTP methods gives you more info at a quick glance
  • Hiding columns that don't provide values reduces the noise of the table (for more info there is still detail view)

Screenshots

Before:
image

After:
image

@carsonbot carsonbot added this to the 7.3 milestone Feb 14, 2025
@carsonbot

This comment was marked as outdated.

Sign in to view
@mamazu mamazu changed the title Only show relevant columns in debug:router Only show relevant columns in debug:router call Feb 14, 2025
@mamazu mamazu force-pushed the console_clean_ups branch 2 times, most recently from 821481d to 44577d0 Compare February 14, 2025 18:34
@mamazu
Copy link
Contributor Author

mamazu commented Feb 14, 2025

Oddly enough the tests work locally. Any idea why? Or is there a way to generate the expected output from the input?

@fabpot
Copy link
Member

fabpot commented Feb 15, 2025

Can you share some before/after screenshots?

@mamazu
Copy link
Contributor Author

mamazu commented Feb 15, 2025

Added screenshots oddly enough it does not show the deprecation notice I added.

@carsonbot carsonbot changed the title Only show relevant columns in debug:router call [FrameworkBundle] Only show relevant columns in debug:router call Feb 15, 2025
@mamazu
Copy link
Contributor Author

mamazu commented Feb 17, 2025

Okay the last remaining error I presume is because the CI tests do not use a true color terminal. Question: should I set the CI colors to use COLORTERM=truecolor or should I update the test output to expect the smaller color set?

@ro0NL ro0NL mentioned this pull request Mar 25, 2025
Open
@javiereguiluz
Copy link
Member

@mamazu I like this a lot. Thanks!

Two quick comments:

(1): I'd use the HTTP method colors from the OpenAPI standard (and keep ANY without color). See e.g. Swagger:

http-colors

(2): I'm divided about outputting the controller. It's useful, but it will break the table design for most people.

See your screenshot. Even if you use very short controllers (most folks will display long FQCN) the resulting table is pretty wide. In practice, each row could be displayed in two rows instead. So, why not display each route in two lines since the beginning to avoid breaking the design?

@mamazu
Copy link
Contributor Author

mamazu commented Mar 27, 2025

@javiereguiluz Thanks for the feedback.

  1. Updated. There wasn't much of a difference.

  2. For me the main reason to use the debug:router is to see which controller belongs to which route. But I didn't want to remove the route name name. And the problem with multiple lines per entry destroys the command's grepability.

@mamazu mamazu force-pushed the console_clean_ups branch 2 times, most recently from af774f0 to fe882b3 Compare April 9, 2025 07:02
@OskarStark
Copy link
Contributor

OskarStark commented Apr 9, 2025 •
edited
Loading

Because you mentioned "grepability", another approach can also be to use --format=json and then use jq binary

@mamazu
Copy link
Contributor Author

mamazu commented Apr 9, 2025

You are completely correct, the text representation contains the least amount of infos anyways, so any other format for searching would also work.

nicolas-grekas
nicolas-grekas reviewed Apr 16, 2025
View reviewed changes
@mamazu mamazu force-pushed the console_clean_ups branch from d5d400c to b6f285a Compare April 16, 2025 17:25
@mamazu
Copy link
Contributor Author

mamazu commented Apr 22, 2025

The test failures aren't related to the feature.

@mamazu
Copy link
Contributor Author

mamazu commented May 1, 2025

@nicolas-grekas So I've removed the deprecation. (I don't know if I can remove the deprecation tag from the PR) Secondly I don't know how the pipeline failures are related to the feature. Could you please restart it or is there anything else I could do?

@fabpot fabpot modified the milestones: 7.3, 7.4 May 26, 2025
@mamazu mamazu force-pushed the console_clean_ups branch 4 times, most recently from 9fad15b to bd7a1d2 Compare July 6, 2025 11:55
stloyd
stloyd reviewed Jul 6, 2025
View reviewed changes
@mamazu
Copy link
Contributor Author

mamazu commented Jul 6, 2025

Rebased to 7.4 and fixed the conflicts.

@mamazu mamazu changed the title [FrameworkBundle] Only show relevant columns in debug:router call [FrameworkBundle] Only show relevant columns in debug:router call and adding colors Aug 2, 2025
@mamazu
Copy link
Contributor Author

mamazu commented Aug 14, 2025

@nicolas-grekas Can you have a look at this again please. It should be ready to merge.

@mamazu mamazu force-pushed the console_clean_ups branch 3 times, most recently from b053d5d to 6eb665a Compare August 18, 2025 10:21
@fabpot fabpot force-pushed the console_clean_ups branch from 6eb665a to 75728d2 Compare August 23, 2025 06:33
@fabpot
Copy link
Member

fabpot commented Aug 23, 2025

Thank you @mamazu.

@fabpot fabpot merged commit cd7cfec into symfony:7.4 Aug 23, 2025
10 of 12 checks passed
@fabpot fabpot mentioned this pull request Aug 23, 2025
Open
@mamazu mamazu deleted the console_clean_ups branch August 23, 2025 08:37
@mamazu mamazu mentioned this pull request Aug 23, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

@fabpot fabpot fabpot approved these changes

+3 more reviewers

@stloyd stloyd stloyd left review comments

@alexander-schranz alexander-schranz alexander-schranz left review comments

@MrYamous MrYamous MrYamous left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Feature FrameworkBundle Status: Reviewed
Projects
None yet
Milestone
7.4
Development

Successfully merging this pull request may close these issues.

Improved output of debug route collection
9 participants
@mamazu @carsonbot @fabpot @javiereguiluz @OskarStark @stloyd @nicolas-grekas @alexander-schranz @MrYamous