Skip to content
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

[WIP] Add routes rake task to display annotated routes #68

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dNitza
Copy link
Member

@dNitza dNitza commented Jan 21, 2017

This PR adds in a rake task to render out a list of annotated routes using roda-route_list.

The task itself currently does a number of things and I have left it as a single procedure so we can discuss what is happening without having to look in multiple places.

Example annotation:
# route[optional_name]: GET|POST /segment/:param

Example output:

              Name Methods   Path
             admin GET, POST /admin
     example_route GET       /example
        blog_posts GET       /posts
         blog_post GET       /posts/:id
blog_post_comments GET       /posts/:id/comments

@dNitza dNitza changed the title [WIP] Add routes rake task to display annotated routes [WIP] Add routes rake task to display annotated routes Jan 21, 2017
@app.plugin :route_list
routes = @app.route_list

# determine the max lengths of our output strings so we can display a nice list of routes
Copy link
Member Author

Choose a reason for hiding this comment

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

This part (including the width method) is inspired by ActionDispatch:: Routing:: ConsoleFormatter and I wonder if it's worth moving this out to something like that, though I doubt this code will need to be reused in a different context yet.

Copy link
Member

Choose a reason for hiding this comment

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

I think this is fine to leave inline now until we need to use it in a 2nd place 👍

@timriley
Copy link
Member

timriley commented Jan 23, 2017

Nice work getting this in place, @dNitza! Solving this has been something that's been nagging at me for a while (look at the timestamp on #5!).

I remember trying roda-route_list way back in the day and feeling dissatisfied with how it had to use a file to store its data. I felt like this would let things get out of sync. But I guess Jeremy Evans has probably thought this through a little more than me and since this plugin exists, we should actually commit to trying it for a while 😄

If we're going to do this, then I think there are a few other things we should handle:

  • Actually put the route list plugin into our sub-apps, so we can use the listed_route helper in our routes for redirecting, etc.
  • Allow access to this listed_route helper through a dry-view context object (you'll need dry-view master for this right now), so that we can generate routes from inside our templates (which is where we need it half the time).
  • Ensure the task to generate the route files is run during our standard deployments.

Then, as a bonus, something like this would be nice:

  • Make a smarter, more convenient route builder that allows us to pass whole objects to it, so we can e.g. pass a "product" to a product route helper, and it'd generate a URL using both product.id and product.slug as required

@timriley
Copy link
Member

I should note that this actually lines up with one of the things I suggested @solnic tackle as he gets started this week. @solnic, this issue might be worth checking in on :)

@dNitza
Copy link
Member Author

dNitza commented Jan 23, 2017

Oh neat! Yeah the file thing was a bit weird to me too which is why I didn't look into listed_route much more. I think if we wanted to use that helper for navigation within our apps, we would need to regenerate the routes list .json each time we boot / deploy / make a request in dev mode. But, I think if we can tackle the syncing issue (especially in dev), then yeah, I think that taking advantage of listed_route in our routes is a great way to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants