Review, please: Capture block so content won't leak.

[josemotanet]

Beware: tests are red.

The following pull request fixed
the block being passed to the appropriate helper method. However, the content
being passed into the block is generating repeated markup on the page due to
some weird ERb evaluation.

This commit tries to capture the block's generated output so the page isn't
flooded with markup, but I'm not able to make the tests pass due to a missing
output buffer. Please help me review this behavior.

Example 1 - it leaks markup to the page.

<%= f.collection_radio_buttons :category, Category.all, :id, :name do |b|%>
  <%= b.label { b.radio_button + b.text } %>
<%- end%>

Example 2 - doesn't leak

<%= f.collection_radio_buttons :category, Category.all, :id, :name do |b| b.label { b.radio_button + b.text }; end %>

[josemotanet]

@rafaelfranca can you help me with this?

[josevalim]

This fix seems correct.

[josemotanet]

@josevalim Indeed, but it breaks half a dozen tests and I'm not sure how to fix them.

[vimutter]

Seems like <%= b.label { b.radio_button + b.text } %> is cause of a problem. I think that there problem inside example. If you use builder, you shouldn't use <%=%>, as builder method will not return markup, usually it expected to return self.

Correct usage is

  <%= f.collection_radio_buttons :category, Category.all, :id, :name do |b|%>
    <% b.label { b.radio_button + b.text } %>
  <%- end%>

[josemotanet]

That's the short term correct answer, I thought of that too :) Thanks @vimutter

However, form_for captures the passed block and it should be the same in here, imo. Developers shouldn't be confused about whether to use one or another.

[rafaelfranca]

@josemota yes, agree. Could you add some tests cases (for the raw template method and for the form helper)?

[josemotanet]

@rafaelfranca OK. I'll give it a stab and see if I can pull it off.

[josemotanet]

So I'm picking on the topic and, as I said, I'm getting 6 failing tests that look exactly the same:

NoMethodError: undefined method `output_buffer=' for #<ActionView::Helpers::Tags::CollectionRadioButtons:0x000000057b5b60>

I'm not understanding the meaning of this, should the helper contain that writer method and it does not? 😧

I feel obligated to make these tests green again before moving on, can anyone point me somewhere so I can take a better look? Thank you.

[rafaelfranca]

@josemota I updated your commit with tests and the proper fix. Thank you so much

[josemotanet]

Thanks @rafaelfranca!

I apologize for not being able to solve the problem that was at hand, I really wanted to grasp the whole situation. Thanks for giving me a hint, I look forward to contributing more.

[rafaelfranca]

No problem. Thank you for the bug report and the fixes. I applied because I want this fixes ASAP since I'll need to release simple_form 😄