nebulab / erb-formatter

Format ERB files with speed and precision
MIT License
151 stars 25 forks source link

Parsing issues with `yield`? #51

Closed AliOsm closed 5 months ago

AliOsm commented 6 months ago

I have the following ERB template:

<section>
  <div class="flex flex-col pb-4 mx-auto">
    <div class="w-full bg-white rounded-lg shadow sm:max-w-lg md:mt-0 xl:p-0 dark:bg-gray-800 dark:border dark:border-gray-700">
      <div class="p-6 space-y-4 sm:p-8 md:space-y-6">
        <%= yield %>
      </div>
    </div>
  </div>
</section>

When I save it, erb-formatter VSCode extension shows the following:

Failed in line 8 ==> ERROR: Unmatched close tag, tried with ["div", "</div>"], but ["%erb%", "yield"] was on the stack

But in other templates like the following one:

<%# locals: (text:, url:, args: {}) %>

<%= link_to url, **{ class: "flex items-center text-sm font-medium text-primary-600 dark:text-primary-500 hover:underline", **args } do %>
  <%= yield %>

    <%= text %>
  <% end %>

No issues happens and the formatter works, but the formatting is not correct, I'm pasting the formatted template.

I think erb-formatter has some issues with yield with used with HTML tags.

AliOsm commented 6 months ago

The same issue happens with this template also:

<div id="flash">
  <% flash.each do |type, message| %>
    <% next unless message.is_a?(String) %>

    <% case type %>
    <% when "alert" %>
      <%= render("shared/alert_flash", message: message) %>
    <% when "notice" %>
      <%= render("shared/notice_flash", message: message) %>
    <% end %>
  <% end %>
</div>

The error is:

Failed in line 12 ==> ERROR: Unmatched close tag, tried with ["div", "</div>"], but ["%erb%", "flash.each do |type, message|"] was on the stack
AliOsm commented 6 months ago

For this template:

<section>
  <div class="flex flex-col pb-4 mx-auto">
    <div class="w-full bg-white rounded-lg shadow sm:max-w-lg md:mt-0 xl:p-0 dark:bg-gray-800 dark:border dark:border-gray-700">
      <div class="p-6 space-y-4 sm:p-8 md:space-y-6">
        <%= yield %>
      </div>
    </div>
  </div>
</section>

Adding <% end %> after the yield line fixes the issue, but the page is not rendering any more :3

IbraheemTuffaha commented 6 months ago

I'm facing the same issue I tried closing the yield tag with an end tag and the formatter is working But as you said, the code no longer renders, instead of gives an error :)

IbraheemTuffaha commented 6 months ago

I dug a bit into the code and I found this case statement: https://github.com/nebulab/erb-formatter/blob/8c1c7b2720761106defff210e3b44eaeb8e06f1b/lib/erb/formatter.rb#L308-L326 So the issue here is that the yield tag should be handled in else I believe, but it's being handled in when RUBY_OPEN_BLOCK I tried replicating the code in else into a when right at the start, where this one checks if the tag starts with yield:

 when /\Ayield\b/ 
   ruby_code = format_ruby(ruby_code, autoclose: false) 
   full_erb_tag = "#{erb_open}#{ruby_code} #{erb_close}" 
   html << (erb_pre_match.match?(/\s+\z/) ? indented(full_erb_tag) : full_erb_tag) 

So final code looked like this:

 case ruby_code 
 when /\Ayield\b/ 
   ruby_code = format_ruby(ruby_code, autoclose: false) 
   full_erb_tag = "#{erb_open}#{ruby_code} #{erb_close}" 
   html << (erb_pre_match.match?(/\s+\z/) ? indented(full_erb_tag) : full_erb_tag) 
 when RUBY_CLOSE_BLOCK 
   full_erb_tag = "#{erb_open}#{ruby_code} #{erb_close}" 
   ...

The formatter seemed to start working correctly I'm not sure if this solution is generic enough to cover all cases though Also, I'm sure it's not the cleanest way to handle this case for sure :)

IbraheemTuffaha commented 6 months ago

I'm facing a similar issue when there's a ruby code with next unless:

<% flash.each do |type, message| %>
  <% next unless message.is_a?(String) %>
  <div ... >
    ...
  </div>
<% end %>

For some reason, erb-formatter thinks that the next unless statement is an opening tag and pushed the whole div inside one tab

edusurf10 commented 6 months ago

what solution for this problem?

lakim commented 5 months ago

I have the same issue with yield. It's very weird, because I thought it was working before. But when I install previous versions up to 0.5.0, I have the same error.

elia commented 5 months ago

@IbraheemTuffaha could you please send a PR with your solution? I can't fix it right now, and I'm happy to accept it, even if it's not "optimal" and only improves the situation. 🙏

lakim commented 5 months ago

I've tried your branch @IbraheemTuffaha and I confirm that it fixes my issue. 🙏

anthmatic commented 5 months ago

Just wanted to add that I'm also having the same issue with when tags:

<% when "something" %>

causes this error:

Failed in line 37 ==> ERROR: Unmatched close tag, tried with ["div", "</div>"], but ["%erb%", "when \\"something\\""] was on the stack
IbraheemTuffaha commented 5 months ago

Just wanted to add that I'm also having the same issue with when tags:

<% when "something" %>

causes this error:

Failed in line 37 ==> ERROR: Unmatched close tag, tried with ["div", "</div>"], but ["%erb%", "when \\"something\\""] was on the stack

@anthmatic It should be correctly handled here: https://github.com/nebulab/erb-formatter/blob/a7ce8b46fb2ade3ee2e935973558f25a7e9a4658/lib/erb/formatter.rb#L313 Which is defined here: https://github.com/nebulab/erb-formatter/blob/a7ce8b46fb2ade3ee2e935973558f25a7e9a4658/lib/erb/formatter.rb#L65 Are you sure you closed the whole case statement properly?

<% case var %>
<% when "bar" %>
  ...
<% when "foo" %>
  ...
<% end %>
lakim commented 5 months ago

Just wanted to add that I'm also having the same issue with when tags:

<% when "something" %>

causes this error:

Failed in line 37 ==> ERROR: Unmatched close tag, tried with ["div", "</div>"], but ["%erb%", "when \\"something\\""] was on the stack

I also had issues with case/when, and this was the only syntax that worked:

<% case var
   when "bar" %>
  ...
<% when "foo" %>
  ...
<% end %>

But it now seems to work with @IbraheemTuffaha 's PR.

@elia do you think you can merge it?

IbraheemTuffaha commented 5 months ago

Should be closed now after #53 is merged?

elia commented 5 months ago

Fixed by #53