Rails Ruby on Rails

Comments and changes to this ticket

• You flagged this item as spam.
  

  Jeff Dean June 12th, 2010 @ 01:29 AM

  The attached patch contains:

  • a fix for the nil-id
  • tests
  • a minor refactoring to the fetch syntax for better performance (using the block syntax instead of the arity-2 syntax)

  (If that minor refactoring needs it's own ticket and acceptance cycle I can split them out)

  • nil-id.diff 3.4 KB

• You flagged this item as spam.
  

  Paul Barry June 12th, 2010 @ 02:01 AM

  +1

  Patch applies cleanly, looks good. Make sense to me too, I would expect providing an explicit value would override the value of the object.

  Out of curiosity, why did you choose to use options.fetch(:foo){ :bar } instead of options.fetch(:foo, :bar)? To avoid the unnecessary method call if the key is present?

• You flagged this item as spam.
  

  Jeff Dean June 12th, 2010 @ 02:18 AM

  Thank you for verifying.

  The block syntax of fetch only evaluates the default value if the key is not present in the hash.
  

  In the options.fetch(:foo, bar) case, the bar method is evaluated even if it's not used. In the options.fetch(:foo){bar} form, the block is only called when it's needed. It's a small performance optimization, but if you are rendering a lot of text fields with nil values on a page, it can add up.

• You flagged this item as spam.
  

  Joseph Palermo June 14th, 2010 @ 08:40 AM

  +1 Works for me.

  This has bitten me several times. Especially the id case where I have to have the same form on the page in two different places (such as the same search form on different parts of the page), and I'd like to be able to remove the ids.

• 

  Repository June 23rd, 2010 @ 05:25 AM

  • State changed from “new” to “resolved”

  (from [52c922fad1e7260a4af87409d04055af85df25f8]) make text_field and hidden_field omit the value attribute if the developer explicitly passes in :value => nil [#4839 state:resolved]

  Signed-off-by: Michael Koziarski michael@koziarski.com

  Conflicts:

  actionpack/lib/action_view/helpers/form_helper.rb
  

  http://github.com/rails/rails/commit/52c922fad1e7260a4af87409d04055...

• 

  Repository June 23rd, 2010 @ 05:25 AM

  (from [ac8d3e3acabf3ece495f591a7195d794d6c611e1]) make text_field and hidden_field omit the value attribute if the developer explicitly passes in :value => nil [#4839 state:resolved]

  Signed-off-by: Michael Koziarski michael@koziarski.com
  http://github.com/rails/rails/commit/ac8d3e3acabf3ece495f591a7195d7...

• 

  Repository June 23rd, 2010 @ 06:00 AM

  (from [cbf36cf57c8ba89b5595dfb39b19e1c8ea4956a0]) Revert "make text_field and hidden_field omit the value attribute if the developer explicitly passes in :value => nil [#4839 state:reopened]"

  This reverts commit 52c922fad1e7260a4af87409d04055af85df25f8
  http://github.com/rails/rails/commit/cbf36cf57c8ba89b5595dfb39b19e1...

• You flagged this item as spam.
  

  Jeff Dean June 23rd, 2010 @ 07:19 AM

  Sorry - that patch didn't come through too well on 2-3-stable. Here's an updated version for 2-3-stable. It applies cleanly for me, and tests pass.

  • nil-value-2-3-stable.diff 2.5 KB
• 

  Michael Koziarski July 5th, 2010 @ 12:33 AM

  • Importance changed from “” to “Low”

  As discussed with Jeff over email, there are scenarios where this behaviour change could catch people out, so I'm hesitant to include it in a 2.3 release given how rocky the last one was.