Permalink
Browse files

Cleanup instantiate builder method definition

  • Loading branch information...
1 parent 7e6145b commit 6871cda693a9a3e5d26ab0884f19132064acbf0a @carlosantoniodasilva carlosantoniodasilva committed May 14, 2011
Showing with 2 additions and 5 deletions.
  1. +2 −5 actionpack/lib/action_view/helpers/form_helper.rb
@@ -243,7 +243,7 @@ def convert_to_model(object)
#
# === Setting the method
#
- # You can force the form to use the full array of HTTP verbs by setting
+ # You can force the form to use the full array of HTTP verbs by setting
#
# :method => (:get|:post|:put|:delete)
#
@@ -898,10 +898,7 @@ def range_field(object_name, method, options = {})
private
- def instantiate_builder(record, *args, &block)
- options = args.extract_options!
- record_object = args.shift
-
+ def instantiate_builder(record, record_object, options, &block)
case record
when String, Symbol
object = record_object

6 comments on commit 6871cda

Contributor

wildchild replied Jun 1, 2011

This commit breaks previously applied 0509bf3

Contributor

josevalim replied Jun 1, 2011

I can happily revert if someone provides a test case.

Contributor

wildchild replied Jun 1, 2011

New patch with test https://gist.github.com/a01c429a80d1bb65e2ff

I think it's too tiny for fork and pull request.

Hey folks, I confirm it breakes the previous patch, unfortunately I didn't get that. But I'm not sure we should not change instantiate_builder to do that, I believe this change should go on fields_for itself, which is where the issue lies. The form_for helper and the FormBuilder#fields_for already take care of giving the right params in that order to instantiate_builder. What do you think?

Contributor

josevalim replied Jun 1, 2011

@wildchild please fork and pull request as it is much easier for us to apply it within a pull request.

Contributor

josevalim replied Jun 1, 2011

Please sign in to comment.