Dropped the use of ; as a separator of non-crud actions on resources and went back to the vanilla slash. It was a neat idea, but lots of the non-crud actions turned out not to be RPC (as the ; was primarily intended to discourage), but legitimate sub-resources, like /parties/recent, which didn't deserve the uglification of /parties;recent. Further more, the semicolon caused issues with caching and HTTP authentication in Safari. Just Not Worth It [DHH]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@6485 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 29 insertions, 27 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index e680b0292f..e90e390397 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Dropped the use of ; as a separator of non-crud actions on resources and went back to the vanilla slash. It was a neat idea, but lots of the non-crud actions turned out not to be RPC (as the ; was primarily intended to discourage), but legitimate sub-resources, like /parties/recent, which didn't deserve the uglification of /parties;recent. Further more, the semicolon caused issues with caching and HTTP authentication in Safari. Just Not Worth It [DHH]
+
 * Added that FormTagHelper#submit_tag will return to its original state if the submit fails and you're using :disable_with [DHH]
 
 * Cleaned up, corrected, and mildly expanded ActionPack documentation.  Closes #7190 [jeremymcanally]
diff --git a/actionpack/lib/action_controller/resources.rb b/actionpack/lib/action_controller/resources.rb
index d5aeaf1d45..506440c5c4 100644
--- a/actionpack/lib/action_controller/resources.rb
+++ b/actionpack/lib/action_controller/resources.rb
@@ -199,7 +199,7 @@ module ActionController
     #
     # * <tt>:collection</tt> -- add named routes for other actions that operate on the collection.
     #   Takes a hash of <tt>#{action} => #{method}</tt>, where method is <tt>:get</tt>/<tt>:post</tt>/<tt>:put</tt>/<tt>:delete</tt>
-    #   or <tt>:any</tt> if the method does not matter.  These routes map to a URL like /messages;rss, with a route of rss_messages_url.
+    #   or <tt>:any</tt> if the method does not matter.  These routes map to a URL like /messages/rss, with a route of rss_messages_url.
     # * <tt>:member</tt> -- same as :collection, but for actions that operate on a specific member.
     # * <tt>:new</tt> -- same as :collection, but for actions that operate on the new resource action.
     #
@@ -211,19 +211,19 @@ module ActionController
     #   # --> GET /thread/7/messages/1
     #  
     #   map.resources :messages, :collection => { :rss => :get }
-    #   # --> GET /messages;rss (maps to the #rss action)
+    #   # --> GET /messages/rss (maps to the #rss action)
     #   #     also adds a named route called "rss_messages"
     # 
     #   map.resources :messages, :member => { :mark => :post }
-    #   # --> POST /messages/1;mark (maps to the #mark action)
+    #   # --> POST /messages/1/mark (maps to the #mark action)
     #   #     also adds a named route called "mark_message"
     # 
     #   map.resources :messages, :new => { :preview => :post }
-    #   # --> POST /messages/new;preview (maps to the #preview action)
+    #   # --> POST /messages/new/preview (maps to the #preview action)
     #   #     also adds a named route called "preview_new_message"
     # 
     #   map.resources :messages, :new => { :new => :any, :preview => :post }
-    #   # --> POST /messages/new;preview (maps to the #preview action)
+    #   # --> POST /messages/new/preview (maps to the #preview action)
     #   #     also adds a named route called "preview_new_message"
     #   # --> /messages/new can be invoked via any request method
     # 
@@ -331,8 +331,8 @@ module ActionController
         resource.collection_methods.each do |method, actions|
           actions.each do |action|
             action_options = action_options_for(action, resource, method)
-            map.named_route("#{resource.name_prefix}#{action}_#{resource.plural}", "#{resource.path};#{action}", action_options)
-            map.named_route("formatted_#{resource.name_prefix}#{action}_#{resource.plural}", "#{resource.path}.:format;#{action}", action_options)
+            map.named_route("#{resource.name_prefix}#{action}_#{resource.plural}", "#{resource.path}/#{action}", action_options)
+            map.named_route("formatted_#{resource.name_prefix}#{action}_#{resource.plural}", "#{resource.path}/#{action}.:format", action_options)
           end
         end
       end
@@ -361,8 +361,8 @@ module ActionController
               map.named_route("#{resource.name_prefix}new_#{resource.singular}", resource.new_path, action_options)
               map.named_route("formatted_#{resource.name_prefix}new_#{resource.singular}", "#{resource.new_path}.:format", action_options)
             else
-              map.named_route("#{resource.name_prefix}#{action}_new_#{resource.singular}", "#{resource.new_path};#{action}", action_options)
-              map.named_route("formatted_#{resource.name_prefix}#{action}_new_#{resource.singular}", "#{resource.new_path}.:format;#{action}", action_options)
+              map.named_route("#{resource.name_prefix}#{action}_new_#{resource.singular}", "#{resource.new_path}/#{action}", action_options)
+              map.named_route("formatted_#{resource.name_prefix}#{action}_new_#{resource.singular}", "#{resource.new_path}/#{action}.:format", action_options)
             end
           end
         end
@@ -372,8 +372,8 @@ module ActionController
         resource.member_methods.each do |method, actions|
           actions.each do |action|
             action_options = action_options_for(action, resource, method)
-            map.named_route("#{resource.name_prefix}#{action}_#{resource.singular}", "#{resource.member_path};#{action}", action_options)
-            map.named_route("formatted_#{resource.name_prefix}#{action}_#{resource.singular}", "#{resource.member_path}.:format;#{action}",action_options)
+            map.named_route("#{resource.name_prefix}#{action}_#{resource.singular}", "#{resource.member_path}/#{action}", action_options)
+            map.named_route("formatted_#{resource.name_prefix}#{action}_#{resource.singular}", "#{resource.member_path}/#{action}.:format",action_options)
           end
         end
 
diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb
index 2b65ae21c2..3d85f49c4c 100644
--- a/actionpack/test/controller/resources_test.rb
+++ b/actionpack/test/controller/resources_test.rb
@@ -103,7 +103,7 @@ class ResourcesTest < Test::Unit::TestCase
 
   def test_with_collection_action
     rss_options = {:action => 'rss'}
-    rss_path    = "/messages;rss"
+    rss_path    = "/messages/rss"
     actions = { 'a' => :put, 'b' => :post, 'c' => :delete }
 
     with_restful_routing :messages, :collection => { :rss => :get }.merge(actions) do
@@ -111,14 +111,14 @@ class ResourcesTest < Test::Unit::TestCase
         assert_routing rss_path, options.merge(rss_options)
 
         actions.each do |action, method|
-          assert_recognizes(options.merge(:action => action), :path => "/messages;#{action}", :method => method)
+          assert_recognizes(options.merge(:action => action), :path => "/messages/#{action}", :method => method)
         end
       end
 
       assert_restful_named_routes_for :messages do |options|
         assert_named_route rss_path, :rss_messages_path, rss_options
         actions.keys.each do |action|
-          assert_named_route "/messages;#{action}", "#{action}_messages_path", :action => action
+          assert_named_route "/messages/#{action}", "#{action}_messages_path", :action => action
         end
       end
     end
@@ -128,7 +128,7 @@ class ResourcesTest < Test::Unit::TestCase
     [:put, :post].each do |method|
       with_restful_routing :messages, :member => { :mark => method } do
         mark_options = {:action => 'mark', :id => '1'}
-        mark_path    = "/messages/1;mark"
+        mark_path    = "/messages/1/mark"
         assert_restful_routes_for :messages do |options|
           assert_recognizes(options.merge(mark_options), :path => mark_path, :method => method)
         end
@@ -145,7 +145,7 @@ class ResourcesTest < Test::Unit::TestCase
       with_restful_routing :messages, :member => { :mark => method, :unmark => method } do
         %w(mark unmark).each do |action|
           action_options = {:action => action, :id => '1'}
-          action_path    = "/messages/1;#{action}"
+          action_path    = "/messages/1/#{action}"
           assert_restful_routes_for :messages do |options|
             assert_recognizes(options.merge(action_options), :path => action_path, :method => method)
           end
@@ -161,7 +161,7 @@ class ResourcesTest < Test::Unit::TestCase
   def test_with_new_action
     with_restful_routing :messages, :new => { :preview => :post } do
       preview_options = {:action => 'preview'}
-      preview_path    = "/messages/new;preview"
+      preview_path    = "/messages/new/preview"
       assert_restful_routes_for :messages do |options|
         assert_recognizes(options.merge(preview_options), :path => preview_path, :method => :post)
       end
@@ -252,7 +252,7 @@ class ResourcesTest < Test::Unit::TestCase
     [:put, :post].each do |method|
       with_singleton_resources :account, :member => { :reset => method } do
         reset_options = {:action => 'reset'}
-        reset_path    = "/account;reset"
+        reset_path    = "/account/reset"
         assert_singleton_routes_for :account do |options|
           assert_recognizes(options.merge(reset_options), :path => reset_path, :method => method)
         end
@@ -269,7 +269,7 @@ class ResourcesTest < Test::Unit::TestCase
       with_singleton_resources :account, :member => { :reset => method, :disable => method } do
         %w(reset disable).each do |action|
           action_options = {:action => action}
-          action_path    = "/account;#{action}"
+          action_path    = "/account/#{action}"
           assert_singleton_routes_for :account do |options|
             assert_recognizes(options.merge(action_options), :path => action_path, :method => method)
           end
@@ -369,8 +369,8 @@ class ResourcesTest < Test::Unit::TestCase
       collection_path            = "/#{options[:path_prefix]}#{controller_name}"
       member_path                = "#{collection_path}/1"
       new_path                   = "#{collection_path}/new"
-      edit_member_path           = "#{member_path};edit"
-      formatted_edit_member_path = "#{member_path}.xml;edit"
+      edit_member_path           = "#{member_path}/edit"
+      formatted_edit_member_path = "#{member_path}/edit.xml"
 
       with_options(options[:options]) do |controller|
         controller.assert_routing collection_path,            :action => 'index'
@@ -422,11 +422,11 @@ class ResourcesTest < Test::Unit::TestCase
       assert_named_route "#{full_prefix}",            "#{name_prefix}#{controller_name}_path",              options[:options]
       assert_named_route "#{full_prefix}/new",        "#{name_prefix}new_#{singular_name}_path",            options[:options]
       assert_named_route "#{full_prefix}/1",          "#{name_prefix}#{singular_name}_path",                options[:options].merge(:id => '1')
-      assert_named_route "#{full_prefix}/1;edit",     "#{name_prefix}edit_#{singular_name}_path",           options[:options].merge(:id => '1')
+      assert_named_route "#{full_prefix}/1/edit",     "#{name_prefix}edit_#{singular_name}_path",           options[:options].merge(:id => '1')
       assert_named_route "#{full_prefix}.xml",        "formatted_#{name_prefix}#{controller_name}_path",    options[:options].merge(            :format => 'xml')
       assert_named_route "#{full_prefix}/new.xml",    "formatted_#{name_prefix}new_#{singular_name}_path",  options[:options].merge(            :format => 'xml')
       assert_named_route "#{full_prefix}/1.xml",      "formatted_#{name_prefix}#{singular_name}_path",      options[:options].merge(:id => '1', :format => 'xml')
-      assert_named_route "#{full_prefix}/1.xml;edit", "formatted_#{name_prefix}edit_#{singular_name}_path", options[:options].merge(:id => '1', :format => 'xml')
+      assert_named_route "#{full_prefix}/1/edit.xml", "formatted_#{name_prefix}edit_#{singular_name}_path", options[:options].merge(:id => '1', :format => 'xml')
       yield options[:options] if block_given?
     end
 
@@ -435,8 +435,8 @@ class ResourcesTest < Test::Unit::TestCase
 
       full_path           = "/#{options[:path_prefix]}#{singleton_name}"
       new_path            = "#{full_path}/new"
-      edit_path           = "#{full_path};edit"
-      formatted_edit_path = "#{full_path}.xml;edit"
+      edit_path           = "#{full_path}/edit"
+      formatted_edit_path = "#{full_path}/edit.xml"
 
       with_options options[:options] do |controller|
         controller.assert_routing full_path,           :action => 'show'
@@ -476,10 +476,10 @@ class ResourcesTest < Test::Unit::TestCase
 
       assert_named_route "#{full_path}",          "#{singleton_name}_path",                options[:options]
       assert_named_route "#{full_path}/new",      "new_#{singleton_name}_path",            options[:options]
-      assert_named_route "#{full_path};edit",     "edit_#{singleton_name}_path",           options[:options]
+      assert_named_route "#{full_path}/edit",     "edit_#{singleton_name}_path",           options[:options]
       assert_named_route "#{full_path}.xml",      "formatted_#{singleton_name}_path",      options[:options].merge(:format => 'xml')
       assert_named_route "#{full_path}/new.xml",  "formatted_new_#{singleton_name}_path",  options[:options].merge(:format => 'xml')
-      assert_named_route "#{full_path}.xml;edit", "formatted_edit_#{singleton_name}_path", options[:options].merge(:format => 'xml')
+      assert_named_route "#{full_path}/edit.xml", "formatted_edit_#{singleton_name}_path", options[:options].merge(:format => 'xml')
     end
 
     def assert_named_route(expected, route, options)