k0kubun / hamlit

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

Minor issue with rendering blocks inside templates #191

Closed dux closed 2 years ago

dux commented 2 years ago

I have this peace of code

image

Before, I called blocks like

= @gen.call('Active modules', :selected) {|code| @org.use(code) }

and now in v3 this produces some junk output, so right way to call it is as in image

- @gen.call('Active modules', :selected) {|code| @org.use(code) }

this does not produce junk. How does junk look like? Like this

image

To me, former behaviour is the right one = @block.call and not - @block.call, so I am posting this issue. I know it is fringe border one, I really use this rarely.

k0kubun commented 2 years ago

Can you create a minimum repository that reproduces this issue? I cannot run your code in your screenshot, e.g. you didn't provide the definition of Apps.

dux commented 2 years ago

I can, but the issue is simple.

  1. proc can be created as expected
  2. when you want to call it and add proc response to output, in v3 you have to use - foo.call and in v2, I used = foo.call

if I use = foo.call in v3, I get expected result + junk.

To me it looks like you somewhere return stack array where you before returned nil.

Still need failing test?

k0kubun commented 2 years ago

Yes, that'd be really useful. At least I cannot run the code in your screenshot. If you don't want to create a repository, you may write a single Haml template in a comment that could be rendered with hamlit render template.haml.

k0kubun commented 2 years ago

Thank you for adding the test! That was really helpful for me. I hope you'll find it helpful for you as well for the discussion below.

when you want to call it and add proc response to output, in v3 you have to use - foo.call and in v2, I used = foo.call

Which version was your "v2"? I checked out v2.16.2, added your test, and ran it, but it failed the same way. I think you can reproduce the same thing, now that we have the reproduction repository we wanted. I'd like to know which version started to cause the issue.

dux commented 2 years ago

I'd like to know which version started to cause the issue.

Sorry I cant help you with that. I used hamlit + hamlit-block and it worked fine. This was only issue I had when migrating to V3 on a pretty large project.

When I upgraded to V3, it did not work, but when I removed hamlit-block gem, then it worked, that is all I remember. After that, this was the only Issue I encountered.

k0kubun commented 2 years ago

This was only issue I had when migrating to V3 on a pretty large project.

I'm so sorry. hamlit-block was a weird-behaving gem, which is why I implemented it as a separate gem. I finally figured out a sustainable way to achieve the goal, which is to use the same strategy as Slim as they share the same framework, so it's now part of the gem.

So, it's the intended behavior going forward. Sorry about the migration effort, but this behavior is intended in v3.

Do you have any other questions?

k0kubun commented 2 years ago

To clarify why the "junk output" is intended, by the very nature of template engine implementation, a template engine needs to decide whether it returns a value or writes to a shared buffer for every code fragment. Because most template engines use a variable visible to a block for a shared buffer, it needs to give up supporting either returning a value or writing to a shared buffer when compiling a block. So, if you use a block in a template, it may or may not work. It's impossible to always prevent a "junk output" in a block. For that reason, you'd like a way to switch the behavior, and it's in fact switched by whether it's - or = here.

dux commented 2 years ago

ok, thx.

I just thought that - suppresses all output and just calculates data as <% ... %>, so this looked logical to report.

If that makes a lot of problems for you because of gem internals, all ok, not a problem at all.

k0kubun commented 2 years ago

It's just that only either

- users.each do |user|
  %p= user.name

or

- @gen = proc do |user|
  %p= user.name
# to be used by `= @gen.call(user)`

could be supported because the second line of each template would need to do completely different things while they look like the same thing to Hamlit. And I believe the former is a more common use case, which is why it's not supported.

dux commented 2 years ago

I get it now, and ofc agree. Thank you.