Skip to content

Commit

Permalink
Its now possible to use match 'stuff' => 'what#stuff' instead of usin…
Browse files Browse the repository at this point in the history
…g the :to for simple routes
  • Loading branch information
dhh committed Dec 21, 2009
1 parent 8763b78 commit 3ff9e9e
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 6 deletions.
10 changes: 7 additions & 3 deletions actionpack/lib/action_dispatch/routing/mapper.rb
Expand Up @@ -39,9 +39,13 @@ def root(options = {})
end

def match(*args)
options = args.extract_options!

path = args.first
if args.one? && args.first.is_a?(Hash)
path = args.first.keys.first

This comment has been minimized.

Copy link
@splattael

splattael Dec 21, 2009

Contributor

Nice one!
The args.first thingy is ugly (line marked). You could just use:
path, to = args.shift
options = { :to => to }

options = { :to => args.first.values.first }
else
path = args.first
options = args.extract_options!
end

conditions, defaults = {}, {}

Expand Down
16 changes: 13 additions & 3 deletions actionpack/test/dispatch/routing_test.rb
Expand Up @@ -22,6 +22,7 @@ def self.matches?(request)
delete 'logout', :to => :destroy, :as => :logout
end

match 'account/logout' => redirect("/logout")
match 'account/login', :to => redirect("/login")

match 'account/modulo/:name', :to => redirect("/%{name}s")
Expand All @@ -37,11 +38,11 @@ def self.matches?(request)
end

constraints(:ip => /192\.168\.1\.\d\d\d/) do
get 'admin', :to => "queenbee#index"
get 'admin' => "queenbee#index"
end

constraints ::TestRoutingMapper::IpRestrictor do
get 'admin/accounts', :to => "queenbee#accounts"
get 'admin/accounts' => "queenbee#accounts"
end

resources :projects, :controller => :project do
Expand Down Expand Up @@ -85,7 +86,7 @@ def self.matches?(request)
end
end

match 'sprockets.js', :to => ::TestRoutingMapper::SprocketsApp
match 'sprockets.js' => ::TestRoutingMapper::SprocketsApp

match 'people/:id/update', :to => 'people#update', :as => :update_person
match '/projects/:project_id/people/:id/update', :to => 'people#update', :as => :update_project_person
Expand Down Expand Up @@ -148,6 +149,15 @@ def test_login_redirect
end
end

def test_logout_redirect_without_to
with_test_routes do
get '/account/logout'
assert_equal 301, @response.status
assert_equal 'http://www.example.com/logout', @response.headers['Location']
assert_equal 'Moved Permanently', @response.body
end
end

def test_redirect_modulo
with_test_routes do
get '/account/modulo/name'
Expand Down

3 comments on commit 3ff9e9e

@splattael
Copy link
Contributor

Choose a reason for hiding this comment

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

Correction: args.first.shift or args.first.first :D

@rubys
Copy link
Contributor

@rubys rubys commented on 3ff9e9e Dec 23, 2009

Choose a reason for hiding this comment

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

My feeling is that the following should not behave differently, but they currently do:

root 'store#index', :as => 'store'
match '/' => 'store#index', :as => 'store'
match '/', :to => 'store#index', :as => 'store'

@jeremy
Copy link
Member

@jeremy jeremy commented on 3ff9e9e Dec 24, 2009

Choose a reason for hiding this comment

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

rubys - now #1 raises an exception and #2 names the route 'store' as expected

Please sign in to comment.