[PATCH] when link_to options are blank, use object specified as first argument

I'm really down for this convention, however I don't think its been tested enough for a Core patch. I've been playing with to_s on models for a while and started to extract the conventions into a plugin.

http://svn.joshpeek.com/projects/plugins/stringgable_shortcuts/

I've just added a minor modification to existing patch uploaded by gbuesing :

1. It kinda makes more sense to join .to_s values if multiple objects passed. 2. Used "options = name if options.blank?" instead of "options = options.blank? ? name : options"

Thanks.

lifofifo --

1. joining #to_s values -- actually, I think it makes more sense to use just the #to_s of the last object in the array, since that's the object being linked to. The first object just reflects the nesting used for the url.

2. "options = name if options.blank?" -- thanks, that's an improvement

updated v1 with lifofifo's suggestion #2

gbuesing - Agree. You can probably update your patch with #2 and I can somehow remove my patch ( if there's a way ? ).

Now stop reading my mind!

Are you making these decisions based a real world app you've tested it in, or are you just making the API up as you go? It doesn't seem like this is actually coming from an extraction from a real app.

I still stand that it needs more research and would best a plugin before its commited. (Hey it already is)

added a test, & test method has more descriptive name

html_escapes link text when first argument is not a String

link text is automatically html_escaped only if first argument is an AR object

This is exactly what I wanted and was wondering why it wasn't already included. Nice work!

As an extension, I wonder if the following can be done, with dynamic method names according to resources in config/routes.rb:

link_to_edit @person
# same as:
# link_to @person, edit_person_path(@person)

link_to_destroy @person
# same as:
# link_to @person, person_path(@person), :method => :delete

link_to_block @person
# same as
# link_to @person, block_person_path(@person), :method => :put
# if routes.rb contains:
# map.resources :people, :member => {:block => :put}

We might need a :absolute or :relative options for urls generated by simply_helpful. Currently, it seems as if simply_helpful uses _path instead of _url for all its url generation. Sometimes, I need an absolute path instead of the relative. For instance, if you send an email with a link_to(@person), the email recipient will not be able to click and go to the correct url. Maybe there's already a way around it, and I'm just oblivious?

link_to(@person, :absolute => true)

would be better than

link_to(@person, person_url(@person))

Maybe we could do this?:

abs_link_to(@person)

I think there's a fairly fine line here between being DRY and then being overly clever and confusing. For example, the API used to allow for link_to(:name, @person), but that was indeed deemed to clever and frankly unnecessary. It really didn't save that much. Sometimes it's OK to repeat a little for clarity.

Second, I'm not sure that it's generally applicable that most records will have a single textual representation that's always a good fit in the view. I just grep'ed through my code for link_to in a number of apps and didn't see that pattern to hold particularly true. Unlike, say, looking up the resource path for a record which does generally hold true.

But more so, it creates a weird penalty for specifying HTML options. For example, if link_to(@person) works, how do you specify :class => "person"? It would seem that you then have to expand it out to link_to(@person, @person, :class => "person") and that's frankly not very clear since the two first arguments do not reveal their intentions by anything but position. Thus, you'd be encouraged to expand it even further to link_to(@person.name, @person, :class => "person") which now duplicates the to_s behavior and makes it very far removed from the link_to(@person) counterparts.

By the same token, I think the whole :absolute linking is unnecessary. We already have the split between _path/_url to give us that. No need to introduce another API for specifying that.

Similarly, while I sympathize with the savings of link_to_destroy(@person), I think it has the same problem of not being able to guess a reasonable link text in enough cases (when would you ever want to have a destroy link just be, say, the name of a person)?

So in all, I appreciate the zeal to find ways to make things shorter, but I think this is one of those cases where shorter is not better. There are two many sacrifices and too many exceptions to counterbalance the somewhat modest gains for core inclusion. The ball is of course always open for plugins.

As to your comment about there being a "weird penalty for specifying HTML options," wouldn't the following work?

link_to(@person, :class => "person")
#=> link_to(h(@person.to_s), person_url(@person), :class => 'person')

# Specify link text
link_to(content_tag(:span, @person.name), @person, :class => "person")
#=> link_to(content_tag(:b, @person.name), person_url(@person), :class => 'person'))

I don't see any weird penalty if HTML options are implemented as such.

Also, concerning the _path/_url thing, I think it's a pretty important shortcut. In a view for an email, for example, there could be many links, and I'd wish I could type

abs_link_to(person)

rather than

link_to(h(person.to_s), person_url(person)

Or perhaps another option would be to set a class variable to UrlHelper:

UrlHelper.default_to_absolute #=> false (default)
link_to("a", person) #=> link_to("a", person_path(person))

UrlHelper.default_to_absolute = true
link_to("b", person) #=> link_to("b", person_url(person))

The last method would make any changes invisible by default so that people who don't know about it won't be affected at all.

I don't like the double positioning of parameters like that. Some times parameter two will mean html options, some times it'll mean the URL specification. That'll be a mess to untangle. And I don't like how it feels either.

And I really don't like the abs_ approach to link_to either. I guess that's just different aesthetics.