New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

capture ruby block in erb and slim templates, fixes #1582 #1585

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@ujifgc
Member

ujifgc commented Feb 12, 2014

ref #1582

This patch allows this weird stuff:

ERB:

<%= content_tag(:a) { content_tag :b, 'c' } %>

Slim:

= content_tag(:a) { content_tag :b, 'c' }

It was not working before because __in_erb_template and __in_slim_template flag was set in the preamble for generated ruby code and these flags were affecting behavior of #content_tag called with ruby or template block.

@minad, @padrino/core-members better suggestions would be appreciated

@minad

This comment has been minimized.

Show comment
Hide comment
@minad

minad Feb 13, 2014

Contributor

Uh, that looks really bad. I have to think about it.

Contributor

minad commented Feb 13, 2014

Uh, that looks really bad. I have to think about it.

@minad

This comment has been minimized.

Show comment
Hide comment
@minad

minad Feb 13, 2014

Contributor

It also doesn't work in haml for me.

Contributor

minad commented Feb 13, 2014

It also doesn't work in haml for me.

@minad

This comment has been minimized.

Show comment
Hide comment
@minad

minad Feb 13, 2014

Contributor

I found a much nicer solution: Please take a look at #1586. It is fixed for erubis, but it would also require a patched Slim version (2.0.2), which I haven't released yet. But I will do that if you want to accept the patch in this form.

Contributor

minad commented Feb 13, 2014

I found a much nicer solution: Please take a look at #1586. It is fixed for erubis, but it would also require a patched Slim version (2.0.2), which I haven't released yet. But I will do that if you want to accept the patch in this form.

@minad

This comment has been minimized.

Show comment
Hide comment
@minad

minad Feb 13, 2014

Contributor

Slim patch is here: slim-template/slim@c5ed58d

The question is if it would be ok for you to require Slim >= 2.0.2 if I would release that.

Contributor

minad commented Feb 13, 2014

Slim patch is here: slim-template/slim@c5ed58d

The question is if it would be ok for you to require Slim >= 2.0.2 if I would release that.

@ujifgc

This comment has been minimized.

Show comment
Hide comment
@ujifgc

ujifgc Feb 13, 2014

Member

I added failing tests to master and rebased this PR.

@padrino/core-members should we force Slim >= 2.0.2 or maybe skip the test for earlier versions?

Erubis extension can be fixed individually with no dependency impact.

Member

ujifgc commented Feb 13, 2014

I added failing tests to master and rebased this PR.

@padrino/core-members should we force Slim >= 2.0.2 or maybe skip the test for earlier versions?

Erubis extension can be fixed individually with no dependency impact.

@minad

This comment has been minimized.

Show comment
Hide comment
@minad

minad Feb 13, 2014

Contributor

UPDATE: I reopened a new pull request which works with current Slim as is.

#1588

Contributor

minad commented Feb 13, 2014

UPDATE: I reopened a new pull request which works with current Slim as is.

#1588

@ujifgc ujifgc closed this Feb 13, 2014

@ujifgc ujifgc deleted the fix-1582 branch Feb 13, 2014

ujifgc added a commit that referenced this pull request Oct 3, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment