Rails core - Mar 2008 - Patch review request: [BUG] hash_for_*_url does not accept ordered parameters as does the regular *_url named route helper

Hi,

I was developing an app that uses hash_for_NAMED_ROUTE_url helper
instead of the more common NAMED_ROUTE_url helper and found out the
former doesn''t work exactly as the latter.

I doesn''t work with ordered parameters such as:

hash_for_user_post_path(@user, @post)

(I needed to use hash_for_user_post_path(:user_id=>@user, :id=>@post)

So I created a ticket:

http://dev.rubyonrails.org/ticket/11460

With a little hacking a managed to fix it and posted a patch there as
well. It includes passing tests.

Could someone review it and give their "+1"s so it gets accepted?

Thanks,

Bernardo de Pádua
--~--~---------~--~----~------------~-------~--~----~
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 my request is getting lost in the middle of a zillion of
"build failed, build fixed" messages, so I am "pinging" it.

Come on, guys, its is a really simple patch claiming for just a little
love!

Cheers,

Bernardo

On Mar 28, 8:55 pm, Bernardo de Pádua <berpa...@gmail.com>
wrote:
> Hi,
>
> I was developing an app that uses hash_for_NAMED_ROUTE_url helper
> instead of the more common NAMED_ROUTE_url helper and found out the
> former doesn''t work exactly as the latter.
>
> I doesn''t work with ordered parameters such as:
>
> hash_for_user_post_path(@user, @post)
>
> (I needed to use hash_for_user_post_path(:user_id=>@user, :id=>@post)
>
> So I created a ticket:
>
> http://dev.rubyonrails.org/ticket/11460
>
> With a little hacking a managed to fix it and posted a patch there as
> well. It includes passing tests.
>
> Could someone review it and give their "+1"s so it gets accepted?
>
> Thanks,
>
> Bernardo de Pádua
--~--~---------~--~----~------------~-------~--~----~
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 was developing an app that uses hash_for_NAMED_ROUTE_url helper
>  instead of the more common NAMED_ROUTE_url helper and found out the
>  former doesn''t work exactly as the latter.
>
>  I doesn''t work with ordered parameters such as:
>
>  hash_for_user_post_path(@user, @post)
>
>  (I needed to use hash_for_user_post_path(:user_id=>@user,
:id=>@post)
>
>  So I created a ticket:
>
>  http://dev.rubyonrails.org/ticket/11460
>
>  With a little hacking a managed to fix it and posted a patch there as
>  well. It includes passing tests.
>
>  Could someone review it and give their "+1"s so it gets
accepted?
I''m a little confused, why are you accessing those hash helpers?  From
my perspective they''re just an implementation detail...


-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

Well, it is documented, so...

But there is an actual reason. I was designing a helper that creates
links and wanted the following interface:

  link_helper(optional_link_text, url, optional_html_options)

I will explain why I need the optional text in the end of the message
(you can just trust I am not that lame or read the more detailed
explanation below). I also don''t want to change the order of the
parameters not to confuse everyone that is already used to the link_to
helper interface.

The problem is if I accept url to be a string (that *_url will
generate), I will have a big problem trying to differentiate the
parameters for the following hypothetical calls:

  link_helper(named_route_url, {:title=>''link!!''})
  link_helper("My link optional text",
{:controller=>''foo''})

See, the types are the same, so I would have to investigate the hash
content to see if it is a url_options hash or a html_options hash. The
code to only parse the parameters gets 10 times bigger than the actual
helper (this is the price you pay for dynamic typing and no function
overloading)... Too much for a simple helper.

So, I just took an easier path and decided to force url to be an
url_options hash, so the parameters get really easy to parse. Using
''hash_for_*_url'' instead of ''*_url'' seemed
like a small price to pay
(if it worked).

  link_helper(optional_string, url_hash_options,
optional_html_options)

I am sure there must be other use cases for the hash_for_*_url. If the
Rails Core team felt otherwise it shouldn''t be exposed.

****** More details
I actually created a menu/tab generator helper. In it, every link must
have a unique identifier (actually a symbol), so it can be set as
selected (and the corresponding tab maybe be highlighted using CSS)
somewhere else in the view or controller. To reduce verbosity, if the
link text is not provided, the identifier is ''titleized'' and
used
instead. Some code excerpt:

<% tabs_for :menu_name do |tab| %>
  <%= tab.home :controller =>''/user'', :action =>
''index'' %>
  <%= tab.profile "My profile",
hash_for_user_profile_path(current_user) %>
  <%= tab.contacts hash_for_contacts_path %>
  <%= tab.jobs hash_for_jobs_path, {:title=>''Just showing
off''} %>
<% end %>


Cheers,

Bernardo

On Apr 1, 12:25 pm, "Michael Koziarski" <mich...@koziarski.com>
wrote:
> >  I was developing an app that uses hash_for_NAMED_ROUTE_url helper
> >  instead of the more common NAMED_ROUTE_url helper and found out the
> >  former doesn''t work exactly as the latter.
>
> >  I doesn''t work with ordered parameters such as:
>
> >  hash_for_user_post_path(@user, @post)
>
> >  (I needed to use hash_for_user_post_path(:user_id=>@user,
:id=>@post)
>
> >  So I created a ticket:
>
> >  http://dev.rubyonrails.org/ticket/11460
>
> >  With a little hacking a managed to fix it and posted a patch there as
> >  well. It includes passing tests.
>
> >  Could someone review it and give their "+1"s so it gets
accepted?
>
> I''m a little confused, why are you accessing those hash helpers? 
From
> my perspective they''re just an implementation detail...
>
> --
> Cheers
>
> Koz
--~--~---------~--~----~------------~-------~--~----~
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 am sure there must be other use cases for the hash_for_*_url. If the
>  Rails Core team felt otherwise it shouldn''t be exposed.
In your particular case you could also make it easier by using an
options hash instead of postional arguments.


So  link_helper("My link optional text",
{:controller=>''foo''}) becomes

link_helper(:text=>"My link optional text", :url=>foo_url)

I''m still not seeing anything which justifies having these helpers be
considered part of the public api, perhaps they should be
''undocumented'' for 2.1?

-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
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 Fri, Apr 4, 2008 at 7:13 AM, Michael Koziarski <michael@koziarski.com>
wrote:
>
> I''m still not seeing anything which justifies having these helpers
be
> considered part of the public api, perhaps they should be
> ''undocumented'' for 2.1?
>
I once built the hash_for methods into a project of mine because they showed
up via ''rake routes'' and I figured they''d be a
shortcut.  It took me a while
to figure out where I''d gone wrong.  So I''m for either
undocumenting them or
at the very least putting some documentation in that explains the specific
cases where one might want to use them (if there are any).

::Jack Danger

--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---

Koz,

Yes, you are right, in my use case your solution is much better.

I think hash_for_*_url has been documented for quite a while and I bet
it''s been used in a lot of projects (a plugin that requires its usage:
http://groups.google.com/group/rails-widgets/browse_thread/thread/d1ff75e9f5ff428e/63c148ddf3e89d36),
I don''t see a reason to deprecate it, it would just break stuff. And,
if it stays, it should behave like *_url parameter-wise.

But I think there are other use cases that actually need
hash_for_*_url (mine was just my motivation, I didn''t quite needed it
for what it was, just to make my life a little easier).

One that crosses my mind now is if you wanted to "inject" parameters
in a given path or url depending on some context (or just manipulate
it):

#route
map.resources :users do |user|
  user.resource :profile
  user.resource :blog
end

#some helper
my_link_helper(text, url_hash) do
  url_hash[:user_id] = "me" if url_hash[:user_id] == current_user.id
end

So I could enforce nice urls to show the current user stuff:
/users/me/profile
instead of
/users/132123123/profile

Or

#other helper
user_stuff_link_helper(text, url_hash) do
  url_hash[:user_id] = current_user unless url_hash[:user_id]
end

To always send a given parameter depending on the context.

I actually think it would make a lot more sense for Rails to always
have urls in the hash form (easy to manipulate) and only convert them
to the string form in the last minute, but that is another story...

Cheers,

Bernardo


On Apr 4, 11:13 am, "Michael Koziarski" <mich...@koziarski.com>
wrote:
> >  I am sure there must be other use cases for the hash_for_*_url. If
the
> >  Rails Core team felt otherwise it shouldn''t be exposed.
>
> In your particular case you could also make it easier by using an
> options hash instead of postional arguments.
>
> So  link_helper("My link optional text",
{:controller=>''foo''}) becomes
>
> link_helper(:text=>"My link optional text", :url=>foo_url)
>
> I''m still not seeing anything which justifies having these helpers
be
> considered part of the public api, perhaps they should be
> ''undocumented'' for 2.1?
>
> --
> Cheers
>
> Koz
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---