Fix partial_override helper method

[estum]

This commit fixes an issue with duplicating yielded content if the method was invoked from a template handled by slim. Also it prevents repeating of invoking the lookup_context method for unfound partials in performance reasons.

[jensljungblad]

Looks good to me. Thanks for the contribution!

[dgmstuart]

@estum this change breaks displaying values in the table for me (Rails 6, Ruby 2.7), using a fresh install: the return value of capture(&block) always returns nil. It took a long time to work out that this was the cause, and I'm not sure why it's happening.

I'm in theory maintaining this project now, and I'm tempted to roll back the yield/capture part of the change. Are you still using Godmin? Could you explain how the issue you fixed here arose?

[estum]

@estum this change breaks displaying values in the table for me (Rails 6, Ruby 2.7), using a fresh install: the return value of capture(&block) always returns nil. It took a long time to work out that this was the cause, and I'm not sure why it's happening.

I'm in theory maintaining this project now, and I'm tempted to roll back the yield/capture part of the change. Are you still using Godmin? Could you explain how the issue you fixed here arose?

@dgmstuart Yes, I’m still using godmin, but I am afraid it is the old version and on rails 5.2/ruby 2.6. I have modified too many parts of that by domain reasons for, I guess, almost three years. So I thought it was not reasonable to update for me or to continue contrinute here (maybe wrongly). I will look for the issue in the near future, starting from discovering changes of the capture method in Rails.

[dgmstuart]

@estum no worries! It got merged by varvet, so the responsibility lies with us.

Please don't feel like you need to fix something, unless you plan on upgrading to Rails 6 soon: I was just messaging to try and understand the situation.

There seem to be two parts to this change:

• changing how we're checking @_partial_override
• switching from implicit block + yield to explicity block +capture

Basically my question is, are both parts required in order to fix the issue you were experiencing? Maybe that would take a bit of time to work out, so no worries if you don't have time.

Maintenance on Godmin is pretty dormant at the moment but it's something we at Varvet plan to invest some more time in in the future.

[estum]

@dgmstuart no worries too! It is interesting for myself ;). So I inspected some changes: it looks like the ActionView::LookupContext is the only part which has been touched since that time. Also, it is strange but all the tests linked with partial_override are running fine, need to see an issue in real app.

Do you have an issue with capture at all, or only with partial_override? Did you tried to open an issue in the rails repo?

[dgmstuart]

@estum thanks - the issue is only with capture - the @partial override part is fine

[dgmstuart]

Perhaps you're using partials rather than plain values and that's why you're not seeing the issue?