Allow `erb` and `haml` helpers to take a block #641

Merged
merged 6 commits into from Feb 26, 2013

Conversation

Projects
None yet
3 participants
@alexeymuranov
Contributor

alexeymuranov commented Feb 17, 2013

What do you think about this? I have changed only erb and haml helpers because for other template engines i do not know which ones have "layouts".

This is to be able to use nested layouts using erb and haml helpers. Real nested layouts, not in the sense of Rails.

When i want to nest two layouts in my application, i currently can do like this:

render :haml, :outer_layout, :layout => false do
  render :haml, :inner_layout1 do
    haml :page1
  end
end

I want to be able to do simply:

haml :outer_layout, :layout => false do
  haml :inner_layout1 do
    haml :page1
  end
end
@zzak

This comment has been minimized.

Show comment Hide comment
@zzak

zzak Feb 17, 2013

Owner

@alexeymuranov Could you provide a test?

Owner

zzak commented Feb 17, 2013

@alexeymuranov Could you provide a test?

@alexeymuranov

This comment has been minimized.

Show comment Hide comment
@alexeymuranov

alexeymuranov Feb 17, 2013

Contributor

Here are the tests. I only made changes for Haml and ERB, but i imagine there are other template engines that can accept blocks. I was tempted to use meta-programming to define these two helpers in one loop, but i think it would make the documentation harder to generate. Maybe there was a reason meta-programming was not used here before.

Contributor

alexeymuranov commented Feb 17, 2013

Here are the tests. I only made changes for Haml and ERB, but i imagine there are other template engines that can accept blocks. I was tempted to use meta-programming to define these two helpers in one loop, but i think it would make the documentation harder to generate. Maybe there was a reason meta-programming was not used here before.

@zzak

This comment has been minimized.

Show comment Hide comment
@zzak

zzak Feb 17, 2013

Owner

@alexeymuranov Some of them already support blocks, maybe you can go through and add support for the rest of them that will allow it?

I think it's best to leave it without any meta-programming. This way it's obvious which template helpers are supported, and can easily be removed or deprecated. That is probably the intention. Maybe @rkh can comment further.

Owner

zzak commented Feb 17, 2013

@alexeymuranov Some of them already support blocks, maybe you can go through and add support for the rest of them that will allow it?

I think it's best to leave it without any meta-programming. This way it's obvious which template helpers are supported, and can easily be removed or deprecated. That is probably the intention. Maybe @rkh can comment further.

@alexeymuranov

This comment has been minimized.

Show comment Hide comment
@alexeymuranov

alexeymuranov Feb 17, 2013

Contributor

@zzak, do you know which ones exactly support blocks? If you do, can you tell me please?

Contributor

alexeymuranov commented Feb 17, 2013

@zzak, do you know which ones exactly support blocks? If you do, can you tell me please?

@zzak

This comment has been minimized.

Show comment Hide comment
@zzak

zzak Feb 17, 2013

Owner

@alexeymuranov you will have to check tilt, see tilt/lib/tilt/* and look for #evaluate in each template class.

Owner

zzak commented Feb 17, 2013

@alexeymuranov you will have to check tilt, see tilt/lib/tilt/* and look for #evaluate in each template class.

@rkh

This comment has been minimized.

Show comment Hide comment
@rkh

rkh Feb 18, 2013

Owner

Could you also add this to liquid, slim, creole, wlang, yajl and rabl and corresponding tests?

Owner

rkh commented Feb 18, 2013

Could you also add this to liquid, slim, creole, wlang, yajl and rabl and corresponding tests?

@rkh

View changes

test/erb_test.rb
+ template(:a_page) { "<p>Contents.</p>\n" }
+ get('/') do
+ erb :main_outer_layout, :layout => false do
+ erb :an_inner_layout, :layout => false do

This comment has been minimized.

Show comment Hide comment
@rkh

rkh Feb 18, 2013

Owner

The inner erb call should not need the :layout => false.

@rkh

rkh Feb 18, 2013

Owner

The inner erb call should not need the :layout => false.

@alexeymuranov

This comment has been minimized.

Show comment Hide comment
@alexeymuranov

alexeymuranov Feb 18, 2013

Contributor

Do not know how to use layouts in creole, yajl, and rabl, will need to look for examples...

Contributor

alexeymuranov commented Feb 18, 2013

Do not know how to use layouts in creole, yajl, and rabl, will need to look for examples...

@zzak

This comment has been minimized.

Show comment Hide comment
@zzak

zzak Feb 18, 2013

Owner

@alexeymuranov creole, yajl and rabl all have tests, can you check there? these should all have tests, including the new block support.

Owner

zzak commented Feb 18, 2013

@alexeymuranov creole, yajl and rabl all have tests, can you check there? these should all have tests, including the new block support.

@alexeymuranov

This comment has been minimized.

Show comment Hide comment
@alexeymuranov

alexeymuranov Feb 18, 2013

Contributor

I do not see how to make a layout in creole for example. There are no such examples among tests, different layout engines are always used with creole.

Contributor

alexeymuranov commented Feb 18, 2013

I do not see how to make a layout in creole for example. There are no such examples among tests, different layout engines are always used with creole.

@rkh

This comment has been minimized.

Show comment Hide comment
@rkh

rkh Feb 18, 2013

Owner

Maybe it doesn't work, those were just the ones were the code permits layouts.

Owner

rkh commented Feb 18, 2013

Maybe it doesn't work, those were just the ones were the code permits layouts.

@alexeymuranov

This comment has been minimized.

Show comment Hide comment
@alexeymuranov

alexeymuranov Feb 19, 2013

Contributor

It is possible to call yield in rabl: https://github.com/nesquena/rabl/wiki/Using-Layouts, but i have not figured out yet how exactly to use it.

Contributor

alexeymuranov commented Feb 19, 2013

It is possible to call yield in rabl: https://github.com/nesquena/rabl/wiki/Using-Layouts, but i have not figured out yet how exactly to use it.

@rkh

This comment has been minimized.

Show comment Hide comment
@rkh

rkh Feb 25, 2013

Owner

Also, still needs docs in README.md.

Owner

rkh commented Feb 25, 2013

Also, still needs docs in README.md.

@alexeymuranov

This comment has been minimized.

Show comment Hide comment
@alexeymuranov

alexeymuranov Feb 25, 2013

Contributor

I can add docs to README. However, i would need much more time to figure out how to use yield in creole, yajl, and rabl. It seems that the semantics is slightly different from the five HTML templates that i have modified: in those five, yield can be used to insert ready-to-use blocks of HTML.

Contributor

alexeymuranov commented Feb 25, 2013

I can add docs to README. However, i would need much more time to figure out how to use yield in creole, yajl, and rabl. It seems that the semantics is slightly different from the five HTML templates that i have modified: in those five, yield can be used to insert ready-to-use blocks of HTML.

@rkh

This comment has been minimized.

Show comment Hide comment
@rkh

rkh Feb 25, 2013

Owner

I guess the current code is fine.

Owner

rkh commented Feb 25, 2013

I guess the current code is fine.

alexeymuranov added some commits Feb 25, 2013

Move "Embedded Templates" subsection in README
Move "Embedded Templates" subsection in English and Russian README to a more appropriate place.
Fix Russian README
In Russian README, "Вложенные шаблоны" -> "Включённые шаблоны"
"Embedded Templates" -> "Literal Templates"
In English and Russian README, rename "Embedded Templates" to "Literal Templates".
@alexeymuranov

This comment has been minimized.

Show comment Hide comment
@alexeymuranov

alexeymuranov Feb 25, 2013

Contributor

I have made several changes in English and Russian README, please tell me if you want them separated to another PR.

In Russian README, i had to change the term "вложенные шаблоны" because in my opinion it was not an appropriate translation of "Inline Templates", and conflicted with "вложенные раскладки" for "nested layouts" that i was adding.

I have also moved "Embedded Templates" subsection and renamed them to "Literal Templates". What do you think?

Those are all in separate commits, but if you do not want to merge them now, i can make a separate pull request.

Contributor

alexeymuranov commented Feb 25, 2013

I have made several changes in English and Russian README, please tell me if you want them separated to another PR.

In Russian README, i had to change the term "вложенные шаблоны" because in my opinion it was not an appropriate translation of "Inline Templates", and conflicted with "вложенные раскладки" for "nested layouts" that i was adding.

I have also moved "Embedded Templates" subsection and renamed them to "Literal Templates". What do you think?

Those are all in separate commits, but if you do not want to merge them now, i can make a separate pull request.

Add README section on new way of nesting layouts
Add README section "Templates with `yield` and nested layouts" in English and Russian.

rkh added a commit that referenced this pull request Feb 26, 2013

Merge pull request #641 from alexeymuranov/template-helpers-with-blocks
Allow `erb` and `haml` helpers to take a block

@rkh rkh merged commit bee7dd4 into sinatra:master Feb 26, 2013

1 check passed

default The Travis build passed
Details

@alexeymuranov alexeymuranov deleted the alexeymuranov:template-helpers-with-blocks branch Feb 26, 2013

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