+1

I've tested and this works well.

I just recommend you to put some parenthesis in your patch, to agree with Rails code:

def nested_child_index(association_name)

new patch, with parens

Will test this out later today (evening for me).

thanks a bunch. i'm running patched here and it's a-ok.

There was no test for the case where the association would be passed in as a string and a symbol. However, the code seemed to indicate that you were, rightfully so, thinking of this case.

well, it isn't the case that the association should be a symbol or a string - it could be anything that responds to #to_s, a custom class, a stringio, etc. that's why there is no test for symbol explicitly but simply ducktype to string no matter what's passed in. for instance ['posts'] would work and so would [:posts]. this is useful in a *args case. however, testing all possible stringlike intputs seems unreasonable.

Why did you choose to use "ensure"? There was at least no test which indicated why afaik.

it's there because ruby lacks a prefix increment function (++i). the previous code uses the obfusication pattern of initializing with -1 and using +=, but that really doesn't read 'starts at zero and increments by 1'. ensure is a good way to implement prefix increment in ruby since it increments on the way out of the method, after the return value has been determined.

The test name did not really describe what it was gonna test

guilty as charged. ;-) not for lack of trying though!

thanks for the work on this - it really makes adding elements via js vastly simpler.

oh crap. should have read 'Formatting help'. sorry bout that.

well, it isn't the case that the association should be a symbol or a string - it could be anything that responds to #to_s, a custom class, a stringio, etc. that's why there is no test for symbol explicitly but simply ducktype to string no matter what's passed in. for instance ['posts'] would work and so would [:posts]. this is useful in a *args case. however, testing all possible stringlike intputs seems unreasonable.

True, I changed it back to #to_s again. But adding at least one different object in the test is at place.

it's there because ruby lacks a prefix increment function (++i). the previous code uses the obfusication pattern of initializing with -1 and using +=, but that really doesn't read 'starts at zero and increments by 1'. ensure is a good way to implement prefix increment in ruby since it increments on the way out of the method, after the return value has been determined.

Fair enough, I changed this back. Thanks for the trick ;-)

oh crap. should have read 'Formatting help'. sorry bout that.

Yeah the lack of a preview on LH doesn't help either…

guilty as charged. ;-) not for lack of trying though! thanks for the work on this - it really makes adding elements via js vastly simpler.

Yes definitely, thanks again.

+1

May I suggest a slightly different patch for this problem. Take a look at the patch.

It uses the local string 'name' as key. It is more informative, but it may be slower, since it is not a symbol.

checkout my initial patch, it did exactly this. the concept simply got dropped when others hacked on it.

@ara.t.howard

Your initial patch uses "association_name" as argument to nested_child_index, whereas I suggest (in my patch) to use the local variable called "name" as argument to nested_child _index.

I fear that if not using a fully qualified parameter name (such as "name"), then using a unqualified value like association_name will eventually turn up again further down the parameter structure. If you are interested I will try to make a test that fails that illustrates the problem.

@Jarl You're right, it's a good improvement. Please do add the failing test you are talking about.

As per request by Eloy Duran.

I have changed the test (initially made by ara.t.howard) to a stricter test testing for the case where sub-associations have same names.

Now the test fails with the initial patch, but passes with my patch.

(from [e61afed6f8d2ef6a580ba00a5beaaaf0bf2ddb9a]) My suggestion to fix ticket 2401 [#2401 fields_for nested fubars index state:resolved]

Signed-off-by: Yehuda Katz + Carl Lerche ykatz+clerche@engineyard.com
http://github.com/rails/rails/commit/e61afed6f8d2ef6a580ba00a5beaaa...