Extractor: Fix multi-line ERB comment extraction in extract_ruby (#1297)

This pull request updates the ERB comment `<%# ... %>` extraction in the
`extract_ruby` method.

When `comments: true` is set in `herb_extract_ruby`, single-line ERB
comments (`<%# comment %>`) correctly convert to ` # comment `.

However, multi-line ERB comments produced invalid Ruby because only the
first line got the `#` prefix. Subsequent lines were output as raw text.

Additionally, when `comments: false` (default) was set, multi-line ERB
comment newlines were collapsed into spaces.

Resolves #102

Author: marcoroth

rust/tests/extract_ruby_options_test.rs

@@ -29,6 +29,24 @@ fn test_extract_ruby_with_comments() {
   assert_eq!(result, "  # comment   \n   code  ;");
 }
 
+#[test]
+fn test_extract_ruby_multiline_erb_comment_with_comments() {
+  let source = "<%#\n  end\n  end\n%>\n";
+  let options = ExtractRubyOptions {
+    comments: true,
+    ..Default::default()
+  };
+  let result = extract_ruby_with_options(source, &options).unwrap();
+  assert_eq!(result, "#  \n# end\n# end\n#  \n");
+}
+
+#[test]
+fn test_extract_ruby_multiline_erb_comment_default() {
+  let source = "<%#\n  end\n  end\n%>\n";
+  let result = extract_ruby(source).unwrap();
+  assert_eq!(result, "   \n     \n     \n  \n");
+}
+
 #[test]
 fn test_extract_ruby_without_preserve_positions() {
   let source = "<% x = 1 %> <% y = 2 %>";

src/extract.c

@@ -45,8 +45,23 @@ void herb_extract_ruby_to_buffer_with_options(
             is_comment_tag = false;
 
             if (extract_options.preserve_positions) {
-              hb_buffer_append_whitespace(output, 2);
-              hb_buffer_append_char(output, '#');
+              bool is_multiline = false;
+
+              if (i + 1 < hb_array_size(tokens)) {
+                const token_T* next = hb_array_get(tokens, i + 1);
+
+                if (next->type == TOKEN_ERB_CONTENT && next->value != NULL && strchr(next->value, '\n') != NULL) {
+                  is_multiline = true;
+                }
+              }
+
+              if (is_multiline) {
+                hb_buffer_append_char(output, '#');
+                hb_buffer_append_whitespace(output, 2);
+              } else {
+                hb_buffer_append_whitespace(output, 2);
+                hb_buffer_append_char(output, '#');
+              }
             } else {
               if (need_newline) { hb_buffer_append_char(output, '\n'); }
               hb_buffer_append_char(output, '#');
@@ -96,12 +111,45 @@ void herb_extract_ruby_to_buffer_with_options(
 
           if (is_inline_comment) {
             if (extract_options.preserve_positions) { hb_buffer_append_whitespace(output, range_length(token->range)); }
+          } else if (is_erb_comment_tag && token->value != NULL) {
+            const char* content = token->value;
+
+            while (*content != '\0') {
+              if (*content == '\n') {
+                hb_buffer_append_char(output, '\n');
+                content++;
+
+                if (extract_options.preserve_positions && *content == ' ') { content++; }
+
+                hb_buffer_append_char(output, '#');
+              } else {
+                hb_buffer_append_char(output, *content);
+                content++;
+              }
+            }
+
+            if (!extract_options.preserve_positions) { need_newline = true; }
           } else {
             hb_buffer_append(output, token->value);
+
             if (!extract_options.preserve_positions) { need_newline = true; }
           }
         } else {
-          if (extract_options.preserve_positions) { hb_buffer_append_whitespace(output, range_length(token->range)); }
+          if (is_erb_comment_tag && extract_options.preserve_positions && token->value != NULL) {
+            const char* content = token->value;
+
+            while (*content != '\0') {
+              if (*content == '\n') {
+                hb_buffer_append_char(output, '\n');
+              } else {
+                hb_buffer_append_char(output, ' ');
+              }
+
+              content++;
+            }
+          } else if (extract_options.preserve_positions) {
+            hb_buffer_append_whitespace(output, range_length(token->range));
+          }
         }
 
         break;

test/extractor/extract_ruby_test.rb

@@ -69,10 +69,11 @@ class ExtractRubyTest < Minitest::Spec
         %>
       HTML
 
-      expected = "            \n"
-
-      # TODO: it should also preserve the newlines in the ERB content
-      # expected = "\n  #\n  end\n  "
+      expected = <<~EXPECTED
+        \s\s\s
+        \s\s\s\s\s
+        \s\s
+      EXPECTED
 
       assert_equal expected, actual
     end
@@ -87,10 +88,52 @@ class ExtractRubyTest < Minitest::Spec
         %>
       HTML
 
-      expected = "                              \n"
+      expected = <<~EXPECTED
+        \s\s\s
+        \s\s\s\s\s
+        \s\s\s\s\s
+        \s\s\s\s\s
+        \s\s\s\s\s
+        \s\s
+      EXPECTED
+
+      assert_equal expected, actual
+    end
+
+    test "multi-line erb comment with comments: true" do
+      actual = Herb.extract_ruby(<<~HTML, comments: true)
+        <%#
+          end
+          end
+          end
+          end
+        %>
+      HTML
+
+      expected = <<~EXPECTED
+        #\s\s
+        # end
+        # end
+        # end
+        # end
+        #\s\s
+      EXPECTED
+
+      assert_equal expected, actual
+    end
+
+    test "erb comment broken up over multiple lines with comments: true" do
+      actual = Herb.extract_ruby(<<~HTML, comments: true)
+        <%#
+          end
+        %>
+      HTML
 
-      # TODO: it should also preserve the newlines in the ERB content
-      # expected = "   \n     \n      \n     \n     \n  \n"
+      expected = <<~EXPECTED
+        #\s\s
+        # end
+        #\s\s
+      EXPECTED
 
       assert_equal expected, actual
     end