rails icon indicating copy to clipboard operation
rails copied to clipboard

Add Colorized, a new routes formatter

Open faqndo97 opened this issue 2 years ago • 11 comments

Summary

I added ActionDispatch::Routing::ConsoleFormatter::Colorized that is a new routes formatter.

Why a new formatter?

The intention of this one is to add a different structure and colors to the actual rails route command. This formatter can be used by a new flag (-C) added on the route command.

Before

Screen Shot 2022-06-01 at 6 10 17 PM

After

Screen Shot 2022-06-01 at 6 10 02 PM

faqndo97 avatar Jun 01 '22 21:06 faqndo97

I would love to see this merged 🙌 Such a nice detail. ❤️

Diogomartf avatar Jun 02 '22 14:06 Diogomartf

Shared this on Twitter as well, but I think it'd be cool if we could separate PUT/PATCH from the actual POST requests for ease of readability.

This is already doing it with DELETE, so it does make sense to me to have it as different categories.

Great addition, it makes things better to easily parse 🎉 .

alessandrapereyra avatar Jun 02 '22 14:06 alessandrapereyra

Shared this on Twitter as well, but I think it'd be cool if we could separate PUT/PATCH from the actual POST requests for ease of readability.

This is already doing it with DELETE, so it does make sense to me to have it as different categories.

Great addition, it makes things better to easily parse 🎉 .

Yeah, I've commonly seen orange used for the PUT/PATCH in other REST clients. For example: https://swagger.io/tools/swagger-ui/

natematykiewicz avatar Jun 02 '22 15:06 natematykiewicz

I think instead of --colorized I would love to see a more general --format option, and an API to register formatters. Then applications/gems could define new formatters and it wouldn't require patches to Rails to use them. I think this would also prevent unexpected results if someone were to pass both --expanded and --colorized, since expanded is checked first in the formatter method.

skipkayhil avatar Jun 02 '22 22:06 skipkayhil

@skipkayhil By implementing what you're saying, --expanded --colorized would actually colorize the "expanded" format, right?

The current --colorized really looks like both "colorized" AND "justified".

So, is "colorized" an option, but then "sheet", "expanded" and "justified" are formats? Perhaps these screenshots should actually be --colorized --justified?

natematykiewicz avatar Jun 02 '22 22:06 natematykiewicz

FWIW, most CLI things I've seen that have a "colorized" option do not change any other formatting. They just add color codes when you colorize the output. Having a colorized flag that also rearranges the output is a bit nonstandard. That's why I'm thinking that "colorized" should apply to all formatters, and then if you want a new formatter for this justified output, that makes sense to me too.

natematykiewicz avatar Jun 02 '22 22:06 natematykiewicz

@skipkayhil By implementing what you're saying, --expanded --colorized would actually colorize the "expanded" format, right?

The current --colorized really looks like both "colorized" AND "justified".

So, is "colorized" an option, but then "sheet", "expanded" and "justified" are formats? Perhaps these screenshots should actually be --colorized --justified?

Yep, that all seems reasonable to me. If the implementation was changed to just colorize the existing format I think that would provide justification for a --colorized flag.

skipkayhil avatar Jun 02 '22 23:06 skipkayhil

Can you perhaps expand on what's better about this layout, colours aside?

At first look, to me, it seems to put all the emphasis on the path patterns, and makes it hard to match them up with their corresponding helpers. Most of the time I would expect users to want the opposite: for most purposes, one should be thinking in and using path helpers, and only occasionally need to deal with exactly which URLs they generate/match.

matthewd avatar Jun 05 '22 12:06 matthewd

Ok, I have been thinking about how to continue with this based on all the feedback received here and on Twitter.

@matthewd I think this new layout is more natural to find what we're looking for in the output. From my experience, when we run the command, most of the time, the information we have is the path, and we're looking for the helper or controller/action or both. So, how we read from left to right, display the information we tend to have on the left side, and put the information we're looking for on the right side matches that. Again, this is from my POV and experience.

Related to what @skipkayhil and @natematykiewicz were talking about, I agree that maybe the "colorize" concept should be a modifier of all formatters and not a formatter itself.

I didn't want to jump directly and replace the default one (Sheet) because probably some people will continue using it.

So maybe the best approach to continue with this, as @natematykiewicz proposed, is to rename this formatter to Justified and refactor the remind formatters to accept the colorized modifier. What do you think?

Related to other comments for improvements that I received, the notes I took are:

  • Use a different color for post.
  • Use a grayish color for (.:format).
  • Use a different color for the path helper instead of gray to highlight it more.
  • Improve the separator between helper name and formatter.

faqndo97 avatar Jun 08 '22 02:06 faqndo97

Hey @faqndo97 this looks awesome! As mentioned above, i've opened #45330 which allows for registering custom formatters; maybe there's a combination of these two PR's we can work on?

phendrick avatar Jun 11 '22 19:06 phendrick

Is this branch still alive?

Would love to see this feature shipped. 🚀

Happy to help.

Diogomartf avatar Sep 13 '22 23:09 Diogomartf

would be great to see this PR merged on the main branch, what does it take to get there? Or there is no interest on the community to have this merged?

Either way would be good to know where we stand.

I think rails routes command needs a lift up and this is a step in the right direction.

Diogomartf avatar Feb 14 '23 18:02 Diogomartf

@Diogomartf Thanks for being interested on this. I couldn't come back here for some time, but I'll think what can be the best next steps here and continue working + communicating next week hopefully

faqndo97 avatar Feb 14 '23 19:02 faqndo97

Really dig this! So much so that I think it should be the default and we use the flag to turn it off, when needed.

dhh avatar Jan 01 '24 15:01 dhh

Any reason to not use https://github.com/fazibear/colorize logic instead of defining this inline in routing inspector? Alternatively would it make sense to make "colorize" functionality into Active Support and use in here? IMHO it could be used for more outputs later.

simi avatar Jan 01 '24 16:01 simi

This doesn't warrant another gem dependency. If we can make it happen inline, great, otherwise, no.

dhh avatar Jan 01 '24 17:01 dhh

@dhh Thanks for the feedback, will solve conflicts, and make the changes this week.

faqndo97 avatar Jan 02 '24 00:01 faqndo97