Avoid grouping single ERB-tags and handle ERB-comments with newlines (#62)

* Avoid grouping single elements

* Handles ERB-comments with newlines

Author: davidwessman

CHANGELOG.md

@@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) a
 
 ## [Unreleased]
 
+- Avoid grouping single tags
+- Handle multiline ERB-comments
+
 ## [0.10.3] - 2023-08-27
 
 ## Fixes

lib/syntax_tree/erb/format.rb

@@ -114,6 +114,7 @@ def visit_erb(node)
           q.text(" ")
           visit(node.keyword)
         end
+
         node.content.blank? ? q.text(" ") : visit(node.content)
 
         visit(node.closing_tag)
@@ -137,27 +138,25 @@ def visit_erb_end(node)
       end
 
       def visit_erb_content(node)
-        if node.value.is_a?(String)
-          output_rows(node.value.split("\n"))
-        else
-          nodes = node.value&.statements&.child_nodes || []
-          nodes = nodes.reject { |node| node.is_a?(SyntaxTree::VoidStmt) }
+        # Reject all VoidStmt to avoid empty lines
+        nodes =
+          (node.value&.statements&.child_nodes || []).reject do |node|
+            node.is_a?(SyntaxTree::VoidStmt)
+          end
 
-          if nodes.size == 1
-            q.text(" ")
+        if nodes.size == 1
+          q.text(" ")
+          format_statement(nodes.first)
+          q.text(" ")
+        elsif nodes.size > 1
+          q.indent do
+            q.breakable("")
             q.seplist(nodes, -> { q.breakable("") }) do |child_node|
               format_statement(child_node)
             end
-            q.text(" ")
-          elsif nodes.size > 1
-            q.indent do
-              q.breakable("")
-              q.seplist(nodes, -> { q.breakable("") }) do |child_node|
-                format_statement(child_node)
-              end
-            end
-            q.breakable
           end
+
+          q.breakable
         end
       end
 
@@ -341,20 +340,26 @@ def handle_child_nodes(child_nodes)
       end
 
       def handle_group(nodes, break_after:)
-        return unless nodes.any?
-
-        q.group do
-          nodes.each_with_index do |node, group_index|
-            visit(node)
-            next_node = nodes[group_index + 1]
-            next if next_node.nil?
-            breakable_between_group(node, next_node)
-          end
+        if nodes.size == 1
+          handle_group_nodes(nodes)
+        elsif nodes.size > 1
+          q.group { handle_group_nodes(nodes) }
+        else
+          return
         end
 
         breakable_between_group(nodes.last, nil) if break_after
       end
 
+      def handle_group_nodes(nodes)
+        nodes.each_with_index do |node, group_index|
+          visit(node)
+          next_node = nodes[group_index + 1]
+          next if next_node.nil?
+          breakable_between_group(node, next_node)
+        end
+      end
+
       def node_should_group(node)
         node.is_a?(SyntaxTree::ERB::CharData) ||
           node.is_a?(SyntaxTree::ERB::ErbNode)

lib/syntax_tree/erb/parser.rb

@@ -94,7 +94,7 @@ def make_tokens
                 # <!DOCTYPE
                 enum.yield :doctype, $&, index, line
                 state << :inside
-              when /\A<%#.*%>/
+              when /\A<%#[\s\S]*?%>/
                 # An ERB-comment
                 # <%# this is an ERB comment %>
                 enum.yield :erb_comment, $&, index, line

test/erb_test.rb

@@ -43,9 +43,10 @@ def test_if_and_end_in_same_output_tag_short
     end
 
     def test_if_and_end_in_same_tag
-      source = "<% if true then this elsif false then that else maybe end %>"
+      source =
+        "Hello\n<% if true then this elsif false then that else maybe end %>\n<h1>Hey</h1>"
       expected =
-        "<% if true\n  this\nelsif false\n  that\nelse\n  maybe\nend %>\n"
+        "Hello\n<% if true\n  this\nelsif false\n  that\nelse\n  maybe\nend %>\n<h1>Hey</h1>\n"
 
       assert_formatting(source, expected)
     end
@@ -98,12 +99,21 @@ def test_erb_only_ruby_comment
       assert_formatting(source, source)
     end
 
-    def test_erb_only_erb_comment
+    def test_erb_comment
       source = "<%# This should be written on one line %>\n"
 
       assert_formatting(source, source)
     end
 
+    def test_erb_multiline_comment
+      source =
+        "<%#\n  This is the first\nThis is the second\nThis is the third %>"
+      expected =
+        "<%#\n  This is the first\nThis is the second\nThis is the third %>\n"
+
+      assert_formatting(source, expected)
+    end
+
     def test_erb_ternary_as_argument_without_parentheses
       source =
         "<%=     f.submit( f.object.id.present?     ? t('buttons.titles.save'):t('buttons.titles.create'))   %>"

test/fixture/erb_syntax_formatted.html.erb

@@ -3,6 +3,11 @@
 <%== rails_raw_output %>
 <%- "this only works in ERB not erubis" %>
 <% # This should be written on one line %>
+<%#
+  This is a comment
+  It can be mutiline
+  Treat it as a comment
+%>
 
 <% if this -%>
   <%= form.submit -%>

test/fixture/erb_syntax_unformatted.html.erb

@@ -3,6 +3,11 @@
 <%== rails_raw_output%>
 <%-"this only works in ERB not erubis"%>
 <% # This should be written on one line %>
+<%#
+  This is a comment
+  It can be mutiline
+  Treat it as a comment
+%>
 
 <% if this -%>
   <%= form.submit -%>