No love for those using named routes and form_for in trunk?
form_for(:comment, comment, new_comment_url
(article.hash_for_permalink)) do |f|
=> can''t convert Symbol into String
also the docs are no longer valid as it doesn''t use :url => {}
def form_for(object_name, object, options = {}, &proc)
raise ArgumentError, "Missing block" unless block_given?
concat(form_tag(options, options.delete(:html) || {}),
proc.binding)
fields_for(object_name, object, options, &proc)
concat(''</form>'', proc.binding)
end
why not use the same convention as start_form_tag instead of html?
def form_for(object_name, object, url_options = {},
html_options = {}, &proc)
raise ArgumentError, "Missing block" unless block_given?
concat(form_tag(url_options, html_options), proc.binding)
fields_for(object_name, object, url_options, &proc)
concat(''</form>'', proc.binding)
end
ah... actually no... because you need to support the :builder option.
So should we be returning back to the old syntax that uses :url?
-- tim lucas
> No love for those using named routes and form_for in trunk? > > form_for(:comment, comment, new_comment_url > (article.hash_for_permalink)) do |f| > => can''t convert Symbol into String > > also the docs are no longer valid as it doesn''t use :url => {}form_for( :comment, comment, :url => new_comment_url(article.hash_for_permalink)) do |f| -- David Heinemeier Hansson http://www.loudthinking.com -- Broadcasting Brain http://www.basecamphq.com -- Online project management http://www.backpackit.com -- Personal information manager http://www.rubyonrails.com -- Web-application framework
On 26/02/2006, at 3:18 AM, David Heinemeier Hansson wrote:>> No love for those using named routes and form_for in trunk? >> >> form_for(:comment, comment, new_comment_url >> (article.hash_for_permalink)) do |f| >> => can''t convert Symbol into String >> >> also the docs are no longer valid as it doesn''t use :url => {} > > form_for( > :comment, comment, > :url => new_comment_url(article.hash_for_permalink)) do |f|The :url option is broken in trunk... form_for : comment, comment, :url => { :action => ''update'' } do; end => <form action="/?url=actionupdate" method="post"> form_for : comment, comment, :action => ''update'' do; end => <form action="/comments/update" method="post"> ...hence why I was asking what you wanted to do with it. I think it needs to be reverted to the old API. http://dev.rubyonrails.org/browser/trunk/actionpack/lib/action_view/ helpers/form_helper.rb#L123 http://dev.rubyonrails.org/browser/trunk/actionpack/lib/action_view/ helpers/form_tag_helper.rb#L18 -- tim
> form_for : comment, comment, :action => ''update'' do; end > => <form action="/comments/update" method="post"> > > ...hence why I was asking what you wanted to do with it. I think it > needs to be reverted to the old API.I can see that it''s broken, but why do you think it needs to be reverted?> http://dev.rubyonrails.org/browser/trunk/actionpack/lib/action_view/ > helpers/form_helper.rb#L123 > http://dev.rubyonrails.org/browser/trunk/actionpack/lib/action_view/ > helpers/form_tag_helper.rb#L18-- Cheers Koz
On 26/02/2006, at 3:14 PM, Michael Koziarski wrote:>> form_for : comment, comment, :action => ''update'' do; end >> => <form action="/comments/update" method="post"> > > I can see that it''s broken, but why do you think it needs to be > reverted?Well for one, the current implementation can''t handle named routes. form_for : comment, comment, named_route_url do; end throws an error because it expects a hash instead of a string as the 3rd param. ...but even after handling that case its ugly having all the options mashed together in the one hash. What if, for example, you wanted to use a cgi param named ''html'' or ''builder''? Other helpers use the :url and :html option convention (remote_form_tag comes to mind)... and considering you have to handle builder options, url options and html options I think the old API is the cleanest syntax: form_for :comment, comment, :url => named_route_url, :html => { ''id'' => ''some-form'' }, :builder => LabellingFormBuilder -- tim
> Other helpers use the :url and :html option convention > (remote_form_tag comes to mind)... and considering you have to > handle builder options, url options and html options I think the > old API is the cleanest syntax: > > form_for :comment, comment, :url => named_route_url, :html => > { ''id'' => ''some-form'' }, :builder => LabellingFormBuilderbtw the fix for this is trivial, replace: concat(form_tag(options, options.delete(:html) || {}), proc.binding) with: concat(form_tag(options.delete(:url) || {}, options.delete(:html) || {}), proc.binding) I''ll file a bug with some tests but for now the patch is below. -- tim Index: actionpack/lib/action_view/helpers/form_helper.rb ==================================================================--- actionpack/lib/action_view/helpers/form_helper.rb (revision 3664) +++ actionpack/lib/action_view/helpers/form_helper.rb (working copy) @@ -122,7 +122,7 @@ # def form_for(object_name, object, options = {}, &proc) raise ArgumentError, "Missing block" unless block_given? - concat(form_tag(options, options.delete(:html) || {}), proc.binding) + concat(form_tag(options.delete(:url) || {}, options.delete (:html) || {}), proc.binding) fields_for(object_name, object, options, &proc) concat(''</form>'', proc.binding) end