Rails core - Nov 2007 - Sensible option hashes in ActionView::Helpers::FormOptionsHelper

First post in here, and first time I''m "contributing" to
rails, so
please don''t slaughter me.

The dual options hash in ActionView::Helpers::FormOptionsHelper is
bugging me. It has one option hash for options on the method itself,
and another option hash for html options, passed directly to the HTML
tag.

f.select :category_id, the_options, {:include_blank => true}, {:class
=> "hello"}

This is bad for two reasons. First of all, it''s very un-common (and
not very pretty, either). If you don''t have any method-options,
you''ll
have a pretty silly empty hash as the 3rd argument. Second of all, it
nerfs the usage of with_options on that method. I''d suggest something
like this:

f.select :category_id, the_options, :include_blank => true, :html =>
{:class => "hai"}

Just like e.g. form_for is doing things.

To ensure backwards compability, one could do something like this:

def select(object, method, choices, *args)
  if args.size == 2
    options = args.first
    html_options = args.last
  else
    options = args.first
    html_options = options.delete(:html)
  end
  InstanceTag.new(object, method, self, nil,
options.delete(:object)).to_select_tag(choices, options, html_options)
end

Sounds sensible? Should I make a patch and submit a ticket?
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On 11/20/07, August Lilleaas <augustlilleaas@gmail.com>
wrote:
> First post in here, and first time I''m "contributing" to
rails, so
> please don''t slaughter me.
>
> The dual options hash in ActionView::Helpers::FormOptionsHelper is
> bugging me. It has one option hash for options on the method itself,
> and another option hash for html options, passed directly to the HTML
> tag.
>
> f.select :category_id, the_options, {:include_blank => true}, {:class
> => "hello"}
>
> This is bad for two reasons. First of all, it''s very un-common
(and
> not very pretty, either). If you don''t have any method-options,
you''ll
> have a pretty silly empty hash as the 3rd argument. Second of all, it
> nerfs the usage of with_options on that method. I''d suggest
something
> like this:
>
> f.select :category_id, the_options, :include_blank => true, :html =>
> {:class => "hai"}
>
> Just like e.g. form_for is doing things.
>
> To ensure backwards compability, one could do something like this:
>
> def select(object, method, choices, *args)
>   if args.size == 2
>     options = args.first
>     html_options = args.last
>   else
>     options = args.first
>     html_options = options.delete(:html)
>   end
>   InstanceTag.new(object, method, self, nil,
> options.delete(:object)).to_select_tag(choices, options, html_options)
> end
>
> Sounds sensible? Should I make a patch and submit a ticket?
Yes it does. I agree with the two concerns you raise and have been
similarly annoyed by this clunky aspect of the API. Please pursue a
patch.

Best,
jeremy

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

Thanks, patch created.

*insert rant about this being my first patch to rails, and the code
might not be completely ideomatic to Rails coding style*

Here it is: http://dev.rubyonrails.org/ticket/10233

On Nov 21, 12:25 am, "Jeremy Kemper" <jer...@bitsweat.net>
wrote:
> On 11/20/07, August Lilleaas <augustlille...@gmail.com> wrote:
>
>
>
> > First post in here, and first time I''m
"contributing" to rails, so
> > please don''t slaughter me.
>
> > The dual options hash in ActionView::Helpers::FormOptionsHelper is
> > bugging me. It has one option hash for options on the method itself,
> > and another option hash for html options, passed directly to the HTML
> > tag.
>
> > f.select :category_id, the_options, {:include_blank => true},
{:class
> > => "hello"}
>
> > This is bad for two reasons. First of all, it''s very
un-common (and
> > not very pretty, either). If you don''t have any
method-options, you''ll
> > have a pretty silly empty hash as the 3rd argument. Second of all, it
> > nerfs the usage of with_options on that method. I''d suggest
something
> > like this:
>
> > f.select :category_id, the_options, :include_blank => true, :html
=>
> > {:class => "hai"}
>
> > Just like e.g. form_for is doing things.
>
> > To ensure backwards compability, one could do something like this:
>
> > def select(object, method, choices, *args)
> >   if args.size == 2
> >     options = args.first
> >     html_options = args.last
> >   else
> >     options = args.first
> >     html_options = options.delete(:html)
> >   end
> >   InstanceTag.new(object, method, self, nil,
> > options.delete(:object)).to_select_tag(choices, options, html_options)
> > end
>
> > Sounds sensible? Should I make a patch and submit a ticket?
>
> Yes it does. I agree with the two concerns you raise and have been
> similarly annoyed by this clunky aspect of the API. Please pursue a
> patch.
>
> Best,
> jeremy
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

Is anyone pursuing this? In addition to the helps in the match, it  
would be great if this was uniform across helpers (eg link_to,  
link_to_remote, button_to etc...)

Fred

On 21 Nov 2007, at 11:26, August Lilleaas wrote:
>
> Thanks, patch created.
>
> *insert rant about this being my first patch to rails, and the code
> might not be completely ideomatic to Rails coding style*
>
> Here it is: http://dev.rubyonrails.org/ticket/10233
>
> On Nov 21, 12:25 am, "Jeremy Kemper" <jer...@bitsweat.net>
wrote:
>> On 11/20/07, August Lilleaas <augustlille...@gmail.com> wrote:
>>
>>
>>
>>> First post in here, and first time I''m
"contributing" to rails, so
>>> please don''t slaughter me.
>>
>>> The dual options hash in ActionView::Helpers::FormOptionsHelper is
>>> bugging me. It has one option hash for options on the method
itself,
>>> and another option hash for html options, passed directly to the  
>>> HTML
>>> tag.
>>
>>> f.select :category_id, the_options, {:include_blank => true},  
>>> {:class
>>> => "hello"}
>>
>>> This is bad for two reasons. First of all, it''s very
un-common (and
>>> not very pretty, either). If you don''t have any
method-options,
>>> you''ll
>>> have a pretty silly empty hash as the 3rd argument. Second of all,
>>> it
>>> nerfs the usage of with_options on that method. I''d
suggest
>>> something
>>> like this:
>>
>>> f.select :category_id, the_options, :include_blank => true,
:html =>
>>> {:class => "hai"}
>>
>>> Just like e.g. form_for is doing things.
>>
>>> To ensure backwards compability, one could do something like this:
>>
>>> def select(object, method, choices, *args)
>>>  if args.size == 2
>>>    options = args.first
>>>    html_options = args.last
>>>  else
>>>    options = args.first
>>>    html_options = options.delete(:html)
>>>  end
>>>  InstanceTag.new(object, method, self, nil,
>>> options.delete(:object)).to_select_tag(choices, options,  
>>> html_options)
>>> end
>>
>>> Sounds sensible? Should I make a patch and submit a ticket?
>>
>> Yes it does. I agree with the two concerns you raise and have been
>> similarly annoyed by this clunky aspect of the API. Please pursue a
>> patch.
>>
>> Best,
>> jeremy
> >

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

I think that''s a good idea as well, and I''ve actually had that
very
idea myself. Not sure how link_to could be done with one option hash
and still make sense. Perhaps something like what form_for does.

link_to "Edit", :class => "important", :url =>
edit_post_url(@post)

What do you think?

On Dec 8, 6:36 pm, Frederick Cheung <frederick.che...@gmail.com>
wrote:
> Is anyone pursuing this? In addition to the helps in the match, it  
> would be great if this was uniform across helpers (eg link_to,  
> link_to_remote, button_to etc...)
>
> Fred
>
> On 21 Nov 2007, at 11:26, August Lilleaas wrote:
>
>
>
> > Thanks, patch created.
>
> > *insert rant about this being my first patch to rails, and the code
> > might not be completely ideomatic to Rails coding style*
>
> > Here it is:http://dev.rubyonrails.org/ticket/10233
>
> > On Nov 21, 12:25 am, "Jeremy Kemper"
<jer...@bitsweat.net> wrote:
> >> On 11/20/07, August Lilleaas <augustlille...@gmail.com>
wrote:
>
> >>> First post in here, and first time I''m
"contributing" to rails, so
> >>> please don''t slaughter me.
>
> >>> The dual options hash in
ActionView::Helpers::FormOptionsHelper is
> >>> bugging me. It has one option hash for options on the method
itself,
> >>> and another option hash for html options, passed directly to
the
> >>> HTML
> >>> tag.
>
> >>> f.select :category_id, the_options, {:include_blank =>
true},
> >>> {:class
> >>> => "hello"}
>
> >>> This is bad for two reasons. First of all, it''s very
un-common (and
> >>> not very pretty, either). If you don''t have any
method-options,
> >>> you''ll
> >>> have a pretty silly empty hash as the 3rd argument. Second of
all,
> >>> it
> >>> nerfs the usage of with_options on that method. I''d
suggest
> >>> something
> >>> like this:
>
> >>> f.select :category_id, the_options, :include_blank => true,
:html =>
> >>> {:class => "hai"}
>
> >>> Just like e.g. form_for is doing things.
>
> >>> To ensure backwards compability, one could do something like
this:
>
> >>> def select(object, method, choices, *args)
> >>>  if args.size == 2
> >>>    options = args.first
> >>>    html_options = args.last
> >>>  else
> >>>    options = args.first
> >>>    html_options = options.delete(:html)
> >>>  end
> >>>  InstanceTag.new(object, method, self, nil,
> >>> options.delete(:object)).to_select_tag(choices, options,  
> >>> html_options)
> >>> end
>
> >>> Sounds sensible? Should I make a patch and submit a ticket?
>
> >> Yes it does. I agree with the two concerns you raise and have been
> >> similarly annoyed by this clunky aspect of the API. Please pursue
a
> >> patch.
>
> >> Best,
> >> jeremy
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On 8 Dec 2007, at 21:29, August Lilleaas wrote:
>
> I think that''s a good idea as well, and I''ve actually had
that very
> idea myself. Not sure how link_to could be done with one option hash
> and still make sense. Perhaps something like what form_for does.
>
> link_to "Edit", :class => "important", :url =>
edit_post_url(@post)
>
> What do you think?
>
Well link_to_remote is currently

link_to_remote( ''Click me'', {:url => {...}, :update =>
{''somediv''}},
{:class => ''foo''})
That would be nicer with a single hash

link_to ''Click me'', :url => {...}, :html => {...}
works, but is a little more verbose for the most common case link_to  
''blah'', edit_post_url(@post)  etc...

You could conceivably rig it so that
link_to ''Edit'', some_string, some_options is interpreted as
link_to
''Edit'', :url => some_string, :html => some_options

but link_to ''Edit'', some_hash
expects some_hash to contain :html and :url keys (and if there is  
no :url key then you can assume it''s old-skool.

Fred

> On Dec 8, 6:36 pm, Frederick Cheung <frederick.che...@gmail.com>
> wrote:
>> Is anyone pursuing this? In addition to the helps in the match, it
>> would be great if this was uniform across helpers (eg link_to,
>> link_to_remote, button_to etc...)
>>
>> Fred
>>
>> On 21 Nov 2007, at 11:26, August Lilleaas wrote:
>>
>>
>>
>>> Thanks, patch created.
>>
>>> *insert rant about this being my first patch to rails, and the code
>>> might not be completely ideomatic to Rails coding style*
>>
>>> Here it is:http://dev.rubyonrails.org/ticket/10233
>>
>>> On Nov 21, 12:25 am, "Jeremy Kemper"
<jer...@bitsweat.net> wrote:
>>>> On 11/20/07, August Lilleaas <augustlille...@gmail.com>
wrote:
>>
>>>>> First post in here, and first time I''m
"contributing" to rails, so
>>>>> please don''t slaughter me.
>>
>>>>> The dual options hash in
ActionView::Helpers::FormOptionsHelper is
>>>>> bugging me. It has one option hash for options on the
method
>>>>> itself,
>>>>> and another option hash for html options, passed directly
to the
>>>>> HTML
>>>>> tag.
>>
>>>>> f.select :category_id, the_options, {:include_blank =>
true},
>>>>> {:class
>>>>> => "hello"}
>>
>>>>> This is bad for two reasons. First of all, it''s
very un-common
>>>>> (and
>>>>> not very pretty, either). If you don''t have any
method-options,
>>>>> you''ll
>>>>> have a pretty silly empty hash as the 3rd argument. Second
of all,
>>>>> it
>>>>> nerfs the usage of with_options on that method.
I''d suggest
>>>>> something
>>>>> like this:
>>
>>>>> f.select :category_id, the_options, :include_blank =>  
>>>>> true, :html =>
>>>>> {:class => "hai"}
>>
>>>>> Just like e.g. form_for is doing things.
>>
>>>>> To ensure backwards compability, one could do something
like this:
>>
>>>>> def select(object, method, choices, *args)
>>>>> if args.size == 2
>>>>>   options = args.first
>>>>>   html_options = args.last
>>>>> else
>>>>>   options = args.first
>>>>>   html_options = options.delete(:html)
>>>>> end
>>>>> InstanceTag.new(object, method, self, nil,
>>>>> options.delete(:object)).to_select_tag(choices, options,
>>>>> html_options)
>>>>> end
>>
>>>>> Sounds sensible? Should I make a patch and submit a ticket?
>>
>>>> Yes it does. I agree with the two concerns you raise and have
been
>>>> similarly annoyed by this clunky aspect of the API. Please
pursue a
>>>> patch.
>>
>>>> Best,
>>>> jeremy
> >

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---