Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

content_tag should accept block using variable

#492

When using content_tag inside of a helper method, there is not currently a nice way of returning strings that contain more than one variable.

Examples:

content_tag(:p) do
string_one = fetch_value
string_two = fetch_value(2)
string_three = another_value
string_one + string_two + string_three
# or even [string_one, string_two, string_three].join
end

Code could be cleaner if you used the following approach, which I believe is also used in form_for.

content_tag(:p) do |content|
content << fetch_value
content << fetch_value(2)
content << another_value
end

Reported by ronin-23439 (at lighthouseapp) · June 26th, 2008 @ 02:40 PM

State: wontfix
Milestone: none
Assigned to: nobody
Importance: none

Activity

  1. wildchild
    wildchild

    Nice proposal! Where is a patch?

    June 26th, 2008 @ 03:08 PM

  2. ronin-23439 (at lighthouseapp)
    ronin-23439 (at lighthouseapp)

    I would have submitted one but I'm not sure how to do it without breaking compatibility with the standard use in the view.

    June 27th, 2008 @ 04:33 AM

  3. Clemens Kofler
    Clemens Kofler

    I don't like this idea at all. If you feel that you have to do something complex like modifying the output multiple times, you should consider refactoring it out into a helper method.

    June 30th, 2008 @ 09:15 PM

  4. Glenn Powell
    Glenn Powell

    Clemens, I don't really understand what the problem is with this proposal. I don't see why this:

    
    content_tag(:p) do |content|
      content << fetch_value
      content << fetch_value(2)
      content << another_value
    end
    

    would be any worse than what you have to do now which is:

    
    content_tag(:p) do
      content = ""
      content << fetch_value
      content << fetch_value(2)
      content << another_value
      content
    end
    

    And I believe the point was that this whole call should in fact be in a helper method, as stated in the original post, but that it would be nicer to have the content variable passed to the content_tag block.

    August 10th, 2008 @ 09:04 AM

  5. Clemens Kofler
    Clemens Kofler

    Glenn, I see your point but I still think the only way to keep it really clean is a helper method - and while I do care about style in the helper, I don't care that much that the two examples you gave would really make a difference for me. I didn't like the whole "view helpers now accept blocks" thing in the first place because IMO it clutters up the view (like when you use the block style with link_to), so I may be biased here ...

    August 10th, 2008 @ 01:24 PM

  6. Tom Lea
    Tom Lea
    • Tag changed from content_tag, enhancement, helper, view to content_tag, enhancement, helper, patch, view

    What about...

    
    content_tag(:p) do
      returning("") do |content|
        content << fetch_value
        content << fetch_value(2)
        content << another_value
      end
    end
    

    ...as a middle ground?

    Either way, I've attached a patch to do what you describe... but I don't think it's quite what's needed yet.

    Issue:

    content += "fish" will simply loose the initial object reference. This would be a bit of a suprise for some users.

    Fix:

    Use a subclass of String that aliases += to << or similar.

    Enhancement:

    Allow something as follows:

    
    content_tag(:div) do |div|
      my_text.each_line do |line|
        div.content_tag(:p, line)
      end
    end
    

    but if we allow this, we would also want to allow:

    
    content_tag(:div) do |div|
      div.form_for(:magic) do |f|
        ...
      end
    end
    

    So we would end up having to proxy calls to all helpers... eugh!

    August 13th, 2008 @ 02:20 PM

  7. DHH
    DHH
    • State changed from new to wontfix

    I think this is probably too clever for its own good when it has to work in the view too.

    November 16th, 2008 @ 09:06 PM