Skip to content
Browse files

Use custom visitor class for optimized url helpers

Rather than trying to use gsub to remove the optional route segments,
which will fail with nested optional segments, use a custom visitor
class that returns a empty string for group nodes.

Closes #9524
  • Loading branch information...
1 parent 48c21e3 commit 86cf7a2d166430fac1611aa7593b52b46eeb9f70 @pixeltrix pixeltrix committed Mar 3, 2013
View
4 actionpack/lib/action_dispatch/journey/route.rb
@@ -71,6 +71,10 @@ def format(path_options)
Visitors::Formatter.new(path_options).accept(path.spec)
end
+ def optimized_path
+ Visitors::OptimizedPath.new.accept(path.spec)
+ end
+
def optional_parts
path.optional_names.map { |n| n.to_sym }
end
View
8 actionpack/lib/action_dispatch/journey/visitors.rb
@@ -74,6 +74,14 @@ def visit_GROUP(node)
end
end
+ class OptimizedPath < String # :nodoc:
+ private
+
+ def visit_GROUP(node)
+ ""
+ end
+ end
+
# Used for formatting urls (url_for)
class Formatter < Visitor # :nodoc:
attr_reader :options, :consumed
View
10 actionpack/lib/action_dispatch/routing/route_set.rb
@@ -165,7 +165,7 @@ def initialize(route, options)
super
@path_parts = @route.required_parts
@arg_size = @path_parts.size
- @string_route = string_route(route)
+ @string_route = @route.optimized_path
end
def call(t, args)
@@ -180,14 +180,6 @@ def call(t, args)
private
- def string_route(route)
- string_route = route.ast.to_s.dup
- while string_route.gsub!(/\([^\)]*\)/, "")
- true
- end
- string_route
- end
-
def optimized_helper(args)
path = @string_route.dup
klass = Journey::Router::Utils
View
6 actionpack/test/dispatch/routing_test.rb
@@ -3198,6 +3198,7 @@ class TestOptimizedNamedRoutes < ActionDispatch::IntegrationTest
app.draw do
ok = lambda { |env| [200, { 'Content-Type' => 'text/plain' }, []] }
get '/foo' => ok, as: :foo
+ get '/post(/:action(/:id))' => ok, as: :posts
end
end
@@ -3215,6 +3216,11 @@ def app; Routes end
test 'named route called on included module' do
assert_equal '/foo', foo_path
end
+
+ test 'nested optional segments are removed' do
+ assert_equal '/post', Routes.url_helpers.posts_path
+ assert_equal '/post', posts_path
+ end
end
class TestNamedRouteUrlHelpers < ActionDispatch::IntegrationTest

0 comments on commit 86cf7a2

Please sign in to comment.
Something went wrong with that request. Please try again.