Output routes in :html format #8499

Merged
merged 1 commit into from Dec 13, 2012

Conversation

Projects
None yet
10 participants
Member

schneems commented Dec 12, 2012

By formatting routes for different media (txt/html) we can apply optimizations based on the format. We can include meta-data in the HTML to allow a rich experience while rendering and viewing the routes. This PR shows route helpers as they are used with the _path extension, it also has a javascript toggle on the top to switch to _url. This way the developer can see the exact named route helper they can use instead of having to modify a base.

This is one example of an optimization that could be applied. Eventually we can link out to guides for the different columns to better explain what helper, HTTP Verb, Path, and Controller#action indicate. We could even add a route search box that could allow developers to input a given route and see all of the routes that match it. These are stand alone features and should be delivered separately.


@schneems schneems Output routes in :html format
By formatting routes for different media (txt/html) we can apply optimizations based on the format. We can include meta-data in the HTML to allow a rich experience while rendering and viewing the routes. This PR shows route helpers as they are used with the `_path` extension, it also has a javascript toggle on the top to switch to `_url`. This way the developer can see the exact named route helper they can use instead of having to modify a base. 

This is one example of an optimization that could be applied. Eventually we can link out to guides for the different columns to better explain what helper, HTTP Verb, Path, and Controller#action indicate. We could even add a route search box that could allow developers to input a given route and see all of the routes that match it. These are stand alone features and should be delivered separately.
08d7b18
Member

steveklabnik commented Dec 12, 2012

👍

Contributor

sheerun commented Dec 12, 2012

sextant on steroids 👍

Owner

rafaelfranca commented Dec 13, 2012

I like it.

@spastorino @jeremy thoughts?

Owner

jeremy commented Dec 13, 2012

Dig the idea and end result. The hard-coded format conditionals seem funky, though.

It could have separate formatters that are passed to the format method. So instead of calling

format(..., :html)

You could then inject the formatter:

format(... HTMLFormatter.new)

Or use the other way around:

HTMLFormatter.new(inspector.collect_all_routes(_routes.routes)).format.
Owner

rafaelfranca commented Dec 13, 2012

Yes, I'd propose this refactoring after we accept the change.

Member

steveklabnik commented Dec 13, 2012

Well, my vote is :shipit: then.

Contributor

gaurish commented Dec 13, 2012

good idea 👍

@rafaelfranca right, seems 👍 to me so.

steveklabnik merged commit ae68fc3 into rails:master Dec 13, 2012

Member

steveklabnik commented Dec 13, 2012

Okay then!

Member

steveklabnik commented Dec 13, 2012

Oh! @schneems I forgot to bug you about a CHANGELOG. Let's do that with the refactoring, eh?

Contributor

coreyhaines commented Dec 13, 2012

Just a thought on an improvement. One confusion is the implicit "look at the route helper name above" if there isn't anything in the column. Could we have this repeat each route helper name in the left column?

Pretty neat to see this, though.

Contributor

jacortinas commented Dec 13, 2012

It would be incredible if the first occurrence of the route helper name was bold or black and every duplicate occurrence was a lighter shade of gray. /cc @coreyhaines

Owner

dhh commented Dec 13, 2012

How and where is this output used? It feels like half a PR.

Member

steveklabnik commented Dec 13, 2012

It is in the new 'routes in your browser' functionality.

That said, it is half of one, as I misunderstood that the refactoring was
before this got merged. I talked to @schneems and he's doing that soon, so
I didn't bother reverting.

Owner

dhh commented Dec 14, 2012

I'm a huge fan of getting more of this stuff into the browser. That's what the Conductor project was about. I'm not so big a fan of having all this HTML generation shoved into the routing code itself, though. IMO this should be a separate engine that follows a regular mvc setup.

If there are people here who are interested in working on this, lets revive the conductor project and let's make routes display the first use case.

But let's remove this HTML generation from the routing code and treat it as a model.

On Dec 14, 2012, at 0:09, Steve Klabnik notifications@github.com wrote:

It is in the new 'routes in your browser' functionality.

That said, it is half of one, as I misunderstood that the refactoring was
before this got merged. I talked to @schneems and he's doing that soon, so
I didn't bother reverting.

Reply to this email directly or view it on GitHub.

Member

steveklabnik commented Dec 14, 2012

I'm not so big a fan of having all this HTML generation shoved into the routing code itself, though.

@dhh I agree. This is what @carlosantoniodasilva suggested here: #8499 (comment)

@schneems is gonna grab this, and if he doesn't in a few days, then I am, since it's my fault for merging early.

IMO this should be a separate engine that follows a regular mvc setup.

Check out #6696. Routes are now shown in the browser upon routing errors. It's not quite an Engine, exactly...

Owner

dhh commented Dec 14, 2012

That's a nice use case for it. But if we're going to expose it elsewhere, I say we get started on that Conductor approach.

In the mean time it seems like we should revert this as this is never going to be the implementation of it?

Member

schneems commented Dec 14, 2012

@dhh the original functionality was put in a Gem https://rubygems.org/gems/sextant that has had over thirty-three thousand downloads in the last few months, it was added into Rails 4 after some on the core team reached out to me. I wrote a blog post on the ideals behind this and my other work in Raise Hell: Better programming through Error Messages.

I teach Rails at the University of Texas and this functionality came straight from user research with my students, as well as requests from seasoned Rails devs. The Engine is very limited in capacity, i'll get to that but first a quick recap on reasoning behind adding this functionality:

  1. Moving the routing into the Rails app, eliminates the need to boot the app to spit out the routes (such as when running rake routes). For some larger apps i've worked on this cuts down the time to see the routes from 30 seconds + to nearly instantaneously. This insanely reduced time allows for quick iteration, and less hairpulling. The Engine can give us this, but that is about it.
  2. When you get a routing error, it's because you either mis-typed something or you misinterpreted something in your routes. I can't help with the first, but we can show you the result of your routes on the error page. This info is presented just-in-time and gives you everything you need to fix your mistake. An engine doesn't have the hooks to add information to one of the built in error pages without totally over-writing it.
  3. The intent of this yet to be completed PR as @steveklabnik mentioned, is to add semantics and additional info to the rendering of the information which is not possible while outputting to the console.
  4. While these three pieces are great for seasoned devs, where they really shines is for beginners. When someone is first learning Rails, Routing is one of the hardest things to grok, making this information harder to find, and longer to get to only adds to the pain and confusion. Having it baked into Rails means it is available out of the box, with no configuration needed means developers of all skill levels have this information at their fingertips.

If you want to revert this PR, I'm all for it... it is still a WIP, I ask that you don't revert the other functionality without the pieces in place to provide an adequate replacement. I would be interested in helping out more with adding in some of those hooks if you're willing to give me some guidance.

@steveklabnik steveklabnik added a commit to steveklabnik/rails that referenced this pull request Dec 14, 2012

@steveklabnik steveklabnik Revert "Merge pull request #8499 from schneems/schneems/html-route-in…
…spector"

This reverts commit ae68fc3, reversing
changes made to 0262a18.

See here: rails#8499 (comment)
8554537
Member

steveklabnik commented Dec 14, 2012

Reverted in 8554537 as requested.

Member

schneems commented Dec 14, 2012

Thanks @steveklabnik

Owner

dhh commented Dec 14, 2012

@schneems It's a great feature. What we are discussing here is the implementation. Having the routing code know how to format HTML doesn't feel like its the right level of abstraction. Just like your model classes don't format HTML either.

I could see returning a json representation that can be transformed into HTML where that makes sense.

Member

schneems commented Dec 14, 2012

@dhh I agree completely that the implementation is flawed, returning a collection and passing that to a partial or moving to some kind of presenter would even be a big step forwards. I'll iterate on the implementation a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment