k0kubun / hamlit

High Performance Haml Implementation
https://rubygems.org/gems/hamlit
Other
981 stars 59 forks source link

Block-support and Capture #53

Closed apotonick closed 8 years ago

apotonick commented 8 years ago

Hey Takashi, first of all, thanks for that great gem. We can't wait to integrate it with Cells to have lightning-fast HAML rendering! :beers: :boom: :beers:

I have a bit of trouble with blocks and capturing, though. Here's a sample Hamlit template. Note that this is unrelated to Rails!

%New

= form model do |f|
  ID
  = f.input :id

Really simple, just a form invocation with a block. Internally, I want to capture that block. However, here's the compiled template.

_buf = []; _buf << ("<New></New>\n".freeze); 
; 
; _hamlit_compiler0 = form model do |f|; 
; _buf << ("ID\n".freeze); 
; _buf << (f.input :id); _buf << ("\n".freeze); end; _buf << (_hamlit_compiler0); 
; _buf = _buf.join

As you can see, the result of the form method is assigned to _hamlit_compiler0 (good!). The content of the block, whatsoever, is assigned to the global _buf (bad!!!). How am I supposed to change _buf to another variable to capture that? Is there any trick I'm missing?

In Slim, this works by assigning each block to a separate variable:

@output_buffer = [];
 @output_buffer << (\"<New></New>\".freeze);
 \n;
 \n;
 _slim_controls1 = form model do |f|;
 _slim_controls2 = '';
 \n;
 _slim_controls2 << (\"ID\".freeze);
 \n;
 _slim_controls2 << ((f.input :id).to_s);
 \n;
 _slim_controls2;
 end;

Here, I don't have to do anything to capture the block content. Am I doing anything wrong with Hamlit? Can you help me?

k0kubun commented 8 years ago

This is an interesting topic. As written in limitations, Hamlit does not have Haml's capture helper. That's because we don't compile the code to be capable of capturing as you discovered in Slim. So there is no trick you're missing.

Since it has a tradeoff in performance to support it, I'll consider to support an option to compile the code like it. If I find some way to avoid performance regression with supporting it, I'll activate it by default.

apotonick commented 8 years ago

Ok, that makes sense! If you could add that, that would be fantastic, because then I can add Hamlit support for Cells.

We pass blocks between different cell instances, so please don't use any instance variables! :wink: Thanks!!

k0kubun commented 8 years ago

@apotonick I have several questions to add support for cells.

Really simple, just a form invocation with a block. Internally, I want to capture that block

What is the definition of form for original Haml? I did git grep "def form" in cells and cells-haml but I couldn't find it. And if you have desired rendering code for Hamlit, how do you define form helper?

We pass blocks between different cell instances, so please don't use any instance variables!

Do you use block's return value for capturing?

In https://github.com/k0kubun/hamlit/issues/53#issuecomment-169511714, I guessed you want the way to define a helper like capture_haml in original Haml. But capture_haml uses instance variables https://github.com/haml/haml/blob/4.0.7/lib/haml/helpers.rb#L604-L613. How do you achieve capturing for original Haml?

k0kubun commented 8 years ago

While I don't try, possibly this can be a trick you're missing to handle Hamlit's _buf in your helper :wink: https://github.com/haml/haml/blob/4.0.7/lib/haml/helpers.rb#L363.

apotonick commented 8 years ago

We don't use any of Haml's Rails helpers! All we need is the return value from the block, the way we can do that in Slim. The great thing about that is, we don't need capture anymore, we just do as follows.

def form(options, &block)
  content = yield self

And that's soooo elegant, and if we had that in Hamlit, it'll be dreamily perfect.

apotonick commented 8 years ago

BTW, the form example I posted is not in Cells. Cells is the generic view model implementation, the new form helper will be here: https://github.com/apotonick/formular

k0kubun commented 8 years ago

One more question. Do you want to support for both = .. do and - .. do?

k0kubun commented 8 years ago

The content of the block, whatsoever, is assigned to the global _buf (bad!!!)

Do you mean that whether block's return value is rendered or not should be handled by the method using block?

I mean, currently a following template

= 3.times do |i|
  = i

is compiled to:

_buf = []
_hamlit_compiler1 =  3.times do |i|
  _hamlit_compiler2 = i
  _buf << (::Hamlit::Utils.escape_html(((_hamlit_compiler2).to_s)))
  _buf << ("\n".freeze)
end; 
_buf << (::Hamlit::Utils.escape_html(((_hamlit_compiler1).to_s)))
_buf = _buf.join

and rendered as:

0
1
2
3

But if you don't want to touch _buf inside block, a compiled result will be:

_buf = []
_hamlit_compiler1 =  3.times do |i|
  _hamlit_compiler2 = ''
  _hamlit_compiler3 = i
  _hamlit_compiler2 << (::Hamlit::Utils.escape_html(((_hamlit_compiler3).to_s)))
  _hamlit_compiler2 << ("\n".freeze)
  _hamlit_compiler2
end; 
_buf << (::Hamlit::Utils.escape_html(((_hamlit_compiler1).to_s)))
_buf = _buf.join

and a rendering result will be:

3

How do you keep backward-compatibility without touching _buf? Will you set _buf to self.output_buffer and replace it in with_output_buffer like cells-haml?

apotonick commented 8 years ago

Very good point, and thanks for clarifying! :beer: How does Slim handle that? Because, they do use a separate buffer per block and then return the content per block, which works perfectly fine.

k0kubun commented 8 years ago

How does Slim handle that?

In Slim, a following template

= 3.times do |i|
  = i

is compiled to:

_buf = []
_slim_controls1 = 3.times do |i|
  _slim_controls2 = ''
  _slim_controls2 << ((::Temple::Utils.escape_html((i))).to_s)
  _slim_controls2
end
_buf << (::Temple::Utils.escape_html((_slim_controls1)))
_buf = _buf.join

and rendered as:

3

This means that script rendering is different between Haml and Slim. Slim does not need to render contents inside block, but Haml does.

apotonick commented 8 years ago

Ah ok, that makes sense! But, in Slim, how would you do something like that? I mean, how would you do an each and render every iteration?

k0kubun commented 8 years ago

how would you do something like that? I mean, how would you do an each and render every iteration?

Slim has disable_capture option. You can render every iteration with disable_capture: true.

Thus Slim and Haml have different approaches to support capturing. Since original Haml does both rendering every iteration and enabling capturing (whose method isn't yield) at the same time, maybe it's difficult to achieve capturing with just calling yield (I mean, original Haml does not have a behavior like Slim's disable_capture: false.)

apotonick commented 8 years ago

As far as I can say: Slim has been the most pleasant lib to use as I did not had to change anything because of the very clean capture. I now see how this doesn't work with iterations, but I think we could live with that (capture: false). What do you think?

k0kubun commented 8 years ago

I think we could live with that (capture: false). What do you think?

That's true. It's not so hard to achieve it.

But I want to provide capturing with the same way as haml.gem in hamlit.gem. I want to provide an extra gem which activates capturing with yield because the feature may be used only in cells. (haml.gem users may expect hamlit.gem to be able to capture via capture_haml helper and no disable_capture option) It's like https://github.com/apotonick/erbse you created for erubis.

The gem's name will be hamlit-block or hamlit-capture? If you are ok for the plan, I'll make it.

apotonick commented 8 years ago

The way it works in Erbse is wrong, I have to fix it myself there! :grimacing: In Erbse, it still collects input with the instance variable @output_buffer, which is wrong. It should return per block, the way we discussed it.

An extra gem sounds amazing and makes totally sense to me. Actually, this might finally open the discussion for a more standardised template world, where capturing finally is handled via the template engine and not the user. I'd say hamlit-block, as the capturing is only one nice side-effect?

Thanks so much, you rock!!!

k0kubun commented 8 years ago

Done. https://github.com/hamlit/hamlit-block

By just doing require 'hamlit/block', you'll get Hamlit you desired. Try code like this: https://github.com/hamlit/hamlit-block/blob/v0.1.0/spec/hamlit/block_spec.rb#L45-L48

apotonick commented 8 years ago

You are amazing. I will try it out later today in my new form builder gem. Thanks so much! :heart: Will report later!

apotonick commented 8 years ago

This works great, thank you so much! I just tested it with cells-hamlit and everything but the escaping is fine. It seems as if Hamlit escapes strings that we don't want to be escaped! Is there any setting to turn auto-escaping off? Remember, we are not always in Rails-context, so there's no html_safe marker.

Thanks again, I'll keep investigating and let you know! :heart_eyes:

k0kubun commented 8 years ago

Is there any setting to turn auto-escaping off?

escape_html and escape_attrs do the trick. https://github.com/k0kubun/hamlit/blob/v2.2.0/REFERENCE.md#engine-options

FYI: I turned off only escape_html in https://github.com/hamlit/hamlit-block/blob/v0.2.0/lib/hamlit/block.rb#L11.

apotonick commented 8 years ago

Oh, so this is not enough? https://github.com/trailblazer/cells-hamlit/blob/master/lib/cell/hamlit.rb#L8

apotonick commented 8 years ago

Aah, I think it might be the Rails helpers doing it wrong! Hang tight..

apotonick commented 8 years ago

Some things don't work as expected.

Hallo
- bla = 1
- capture do
  Hidden!

The rendered template still shows Hidden!. My implementation for capture is a simple yield.

def capture
  yield
end

Here's the compiled Ruby.

_buf = []; _buf << (\"Hallo\\n\".freeze); \n; \n;  bla = 1; \n; \n;  capture do; \n; _buf << (\"Hidden!\\n\".freeze); end; _buf = _buf.join

It still writes to _buf. Any idea?

apotonick commented 8 years ago

Here's what Slim compiles:

@output_buffer = []; @output_buffer << (\"Hallo\".freeze); \n; \n; bla = 1; \n; \n; capture do; \n; @output_buffer << (\"<Hidden>!</Hidden>\".freeze); \n; end; @output_buffer = @output_buffer.join
k0kubun commented 8 years ago

The rendered template still shows Hidden!.

Ah, so you may want capture support for - ... do as I asked you in https://github.com/k0kubun/hamlit/issues/53#issuecomment-170317415. It's not hidden because I didn't implement it. I'll fix it.

apotonick commented 8 years ago

Oh sorry, I didn't read it properly! Thanks so much! I will test as soon as you tell me where to point my Gemfile. Thanks!!!

k0kubun commented 8 years ago

I will test as soon as you tell me where to point my Gemfile. Thanks!!!

@apotonick I implemented and released it in hamlit-block v0.3.0.

apotonick commented 8 years ago

Just letting you know: it all looks pretty great. You are the best! :heart: :rock:

apotonick commented 8 years ago

Amazing, everything works! You're incredible!!!

One thing though that I had to change in my tests, and this is different to Haml.

In Hamlit, it has to be = in capture to be captured.

- content = capture do
  = "takashi is king!"

In Haml, this works (note the -).

- content = capture do
  - "takashi is king!"
k0kubun commented 8 years ago

Amazing, everything works!

I'm glad to hear that. :)

One thing though that I had to change in my tests, and this is different to Haml.

So you want - to be the same as =? I can't understand what is the difference between the sources you listed.

Do you have a way to reproduce your cells-haml result easily? I mean, I want to see compiled code and rendering result like this.

require 'haml'

haml = <<~HAML
  - content = capture_haml do
    hello
    - "hidden"
    world
  = content
HAML

Haml::Engine.new(haml).precompiled #=> " content = capture_haml do\n_hamlout.push_text(\"hello\\n\", 0, false);\n \"hidden\"\n_hamlout.push_text(\"world\\n\", 0, false);end;_hamlout.push_text(\"\#{\n_hamlout.format_script_false_false_false_false_false_true_false(( content\n));}\\n\", 0, false);"
Haml::Engine.new(haml).render #=> "hello\nworld\n"
apotonick commented 8 years ago

I actually think that the second example is wrong, it should not work that way:

- content = capture do
  - "takashi is king!"

This shouldn't capture the "taskashi is king!" string (even though the content is true ;) - I think we should leave it as it is.

Do you still want the compiled output?

k0kubun commented 8 years ago

I think we should leave it as it is.

OK. I guess everything works fine now. Thank you for your helps to integrate hamlit with cells!

apotonick commented 8 years ago

No no, thank YOU! :heart: I will give you more feedback as this moves along!

apotonick commented 8 years ago

It works and is insanely fast! You are amazing, thanks again. As soon as @timoschilling gives me push rights for cells-hamlit I will tweet/write about it. Hamlit will be big! :beers: :trophy:

Example project, Cells with Hamlit and Formular (form renderer): https://github.com/apotonick/gemgem-sinatra/blob/formular-hamlit/concepts/post/view/new.haml

screenshot_2016-01-22_14-15-19

k0kubun commented 8 years ago

It works and is insanely fast! Example project, Cells with Hamlit and Formular (form renderer): https://github.com/apotonick/gemgem-sinatra/blob/formular-hamlit/concepts/post/view/new.haml

Cool! :beers:

dux commented 7 years ago

I really like plain old yiled instead of capture_haml in haml, but just to say two things

  1. I think that 'hamlit-block' gem should be included in 'hamlit' gem

  2. my measured performance improvment was 12%. that is of course better, but far away from 8X wet dreams :)

all together, great work, thank you very much @k0kubun . mutch better and cleaner implementation of the standard.

k0kubun commented 7 years ago

I think that 'hamlit-block' gem should be included in 'hamlit' gem

It's not just performance optimization or internal changes. It is a feature-level difference. So it should be introduced to Haml first. Hamlit is designed to be the same as Haml as possible.

my measured performance improvment was 12%. that is of course better, but far away from 8X wet dreams :)

Thank you for reporting that! Actually rendering has many layers and 12% improvement is impressive.

apotonick commented 7 years ago

@dux Now, switch to Cells with Hamlit and you will have at least 25%.