This project is archived and is in readonly mode.
Add optional :format argument to named routes
-
DHH
I like this. Seems that http://gist.github.com/23712 has the pointers for making this so. Just needs to be wrapped up in a real patch with tests.
-
Michael Koziarski
At the same time it would be great to continue jeremy's work to cut down on the memory usage from the generated recognition code.
-
aaronbatalion
I was working on a real patch with tests already. Will update soon. There are a couple known bugs in the above gist.
-
aaronbatalion
- Tag changed from actionpack, options, resources to actionpack, options, patch, resources
Attached is the patch for optional .:format in routes, which decreases the number of routes by 50%, saving up to 100M of RAM on larger rails apps.
Notes: Found one side effect when PageCachingTest.
In RouteSet#routes_for_controller_and_action_and_keys, routes are sorted by significant keys, subtracting the keys that are passed in. Therefore, are UrlRewriter.rewrite(:controller => "foo", :format => nil) and UrlRewriter.rewrite(:controller => "foo") are not the same.
I've added a test to UrlRewriterTest, and modified the RouteSet#routes_for_controller_and_action_and_keys to remove pairs with nil values, and left the PageCachingTest alone.
-
Lourens Naudé
Aaron,
Attached is an updated diff, compatible with Tom's changes from http://github.com/rails/rails/co...
All tests passing and just piped it through the test suite of a large app with a huge number of nested routes.
All seems well.Great track of thought with this !
- Lourens
-
Michael Koziarski
Really nice work so far guys, if we do this we need to think about a few things:
generation optimisations
They assume that all segments are mandatory. This changes that and will cause them to fail to kick in even when they should.
Deprecating the formatted_... routes nicely
the formatted routes need to warn you, and continue to work with positional arguments
formatted_person_url(1, :xml)
-
DHH
- Milestone cleared.
I'd really like to see this make it into 2.3. The formatted_ stuff was a hack anyway. Would be great to get rid of it. Anyone working on this have some comments for koz's concerns?
-
aaronbatalion
Attached is a new patch that gets rid of :format's, and answers Koz's concerns.
1) Included a deprecation warn for each use of formatted_url* methods. If someone would like to suggest the proper warning text, I'm sure it can be improved.
2) Fixed the generation optimisations implementation to still work for the previously route.optimise?-able routes.
3) Started to rip out all direct calls to formatted_*, but that might need another pass to completely deprecate it.
-
aaronbatalion
correct patch attached.
-
Repository
- State changed from new to committed
(from [fef6c32afe2276dffa0347e25808a86e7a101af1]) Added optimal formatted routes to rails, deprecating the formatted_* methods, and reducing routes creation by 50% [#1359 Add optional :format argument to named routes state:committed]
Signed-off-by: David Heinemeier Hansson david@loudthinking.com http://github.com/rails/rails/co...
