Formatter: Respect maxLineLength for inline elements with ERB (#889)

Previously, inline elements containing ERB control flow could exceed the
configured maxLineLength due to:

1. A hardcoded 120-character limit that bypassed `maxLineLength` setting
2. No detection of whether child elements could be rendered inline

This pull request fixes these issues and adds recursive inline
detection.

**Input**
```html+erb
<div>
  <span>
    <% [10, 20, 30, 50, 100].each do |per| %>
      <%= link_to_unless per_page == per, per.to_s, params.merge({ action:, page: 1, per:, remote: }) %>
    <% end %>
  </span>
</div>
```


**Before**
```html+erb
<div><span><% [10, 20, 30, 50, 100].each do |per| %><%= link_to_unless per_page == per, per.to_s, params.merge({ action:, page: 1, per:, remote: }) %><% end %></span></div>
```


**After**
```html+erb
<div>
  <span>
    <% [10, 20, 30, 50, 100].each do |per| %>
      <%= link_to_unless per_page == per, per.to_s, params.merge({ action:, page: 1, per:, remote: }) %>
    <% end %>
  </span>
</div>
```

Resolves #882

Author: marcoroth

javascript/packages/formatter/src/format-printer.ts

@@ -1517,6 +1517,16 @@ export class FormatPrinter extends Printer {
     if (!openTagInline) return false
     if (children.length === 0) return true
 
+    const hasNonInlineChildElements = children.some(child => {
+      if (isNode(child, HTMLElementNode)) {
+        return !this.shouldRenderElementContentInline(child)
+      }
+
+      return false
+    })
+
+    if (hasNonInlineChildElements) return false
+
     let hasLeadingHerbDisable = false
 
     for (const child of node.body) {
@@ -1538,7 +1548,7 @@ export class FormatPrinter extends Printer {
 
       if (fullInlineResult) {
         const totalLength = this.indent.length + fullInlineResult.length
-        return totalLength <= this.maxLineLength || totalLength <= 120
+        return totalLength <= this.maxLineLength
       }
 
       return false
@@ -1573,8 +1583,9 @@ export class FormatPrinter extends Printer {
 
       const childrenContent = this.renderChildrenInline(children)
       const fullLine = openTagResult + childrenContent + `</${tagName}>`
+      const totalLength = this.indent.length + fullLine.length
 
-      if ((this.indent.length + fullLine.length) <= this.maxLineLength) {
+      if (totalLength <= this.maxLineLength) {
         return true
       }
     }

javascript/packages/formatter/test/erb/case.test.ts

@@ -174,4 +174,99 @@ describe("@herb-tools/formatter", () => {
     const output = formatter.format(input)
     expect(output).toEqual(input)
   })
+
+  test("case/when with long outputs inside inline element - breaks at ERB boundaries", () => {
+    const input = dedent`
+      <div>
+        <span>
+          <% case status %>
+          <% when 'pending' %>
+            <%= render partial: 'status_badge', locals: { text: 'Pending Review', color: 'yellow' } %>
+          <% when 'approved' %>
+            <%= render partial: 'status_badge', locals: { text: 'Approved', color: 'green' } %>
+          <% when 'rejected' %>
+            <%= render partial: 'status_badge', locals: { text: 'Rejected', color: 'red' } %>
+          <% end %>
+        </span>
+      </div>
+    `
+    const output = formatter.format(input)
+    expect(output).toEqual(input)
+  })
+
+  test("case/when with method calls and long paths - breaks at ERB boundaries", () => {
+    const input = dedent`
+      <div>
+        <span>
+          <% case user.subscription_tier %>
+          <% when :premium %>
+            <%= link_to "Premium Dashboard", premium_dashboard_path(user_id: user.id, features: :all) %>
+          <% when :basic %>
+            <%= link_to "Basic Dashboard", basic_dashboard_path(user_id: user.id, features: :limited) %>
+          <% else %>
+            <%= link_to "Free Dashboard", free_dashboard_path(user_id: user.id, features: :minimal) %>
+          <% end %>
+        </span>
+      </div>
+    `
+    const output = formatter.format(input)
+    expect(output).toEqual(input)
+  })
+
+  test("case/in with complex patterns and long outputs - breaks at ERB boundaries", () => {
+    const input = dedent`
+      <div>
+        <span>
+          <% case user_data %>
+          <% in { role: 'admin', permissions: { level: level } } if level > 5 %>
+            <%= link_to "Super Admin Panel", super_admin_path(user: user_data, access: :full) %>
+          <% in { role: 'moderator', permissions: } %>
+            <%= link_to "Moderator Tools", moderator_path(user: user_data, access: :limited) %>
+          <% else %>
+            <%= link_to "User Profile", profile_path(user: user_data, access: :basic) %>
+          <% end %>
+        </span>
+      </div>
+    `
+    const output = formatter.format(input)
+    expect(output).toEqual(input)
+  })
+
+  describe("no whitespace introduction on single-line case statements", () => {
+    test("short case/when stays on one line without whitespace", () => {
+      const input = `<span><% case x %><% when 1 %>One<% when 2 %>Two<% else %>Other<% end %></span>`
+      const output = formatter.format(input)
+      expect(output).toEqual(input)
+    })
+
+    test("short case/when with ERB output stays on one line - no whitespace", () => {
+      const input = `<span><% case type %><% when :ok %><%= msg %><% else %>Error<% end %></span>`
+      const output = formatter.format(input)
+      expect(output).toEqual(input)
+    })
+
+    test("short case/when with only text stays on one line - no whitespace", () => {
+      const input = `<span><% case status %><% when 'ok' %>OK<% else %>Fail<% end %></span>`
+      const output = formatter.format(input)
+      expect(output).toEqual(input)
+    })
+
+    test("short case/in stays on one line - no whitespace", () => {
+      const input = `<span><% case val %><% in 1 %>A<% in 2 %>B<% else %>C<% end %></span>`
+      const output = formatter.format(input)
+      expect(output).toEqual(input)
+    })
+
+    test("short case with symbols stays on one line - no whitespace", () => {
+      const input = `<span><% case role %><% when :admin %>Admin<% when :user %>User<% end %></span>`
+      const output = formatter.format(input)
+      expect(output).toEqual(input)
+    })
+
+    test("short case with ERB output stays on one line - no whitespace", () => {
+      const input = `<span><% case a %><% when 1 %><%= x %><% when 2 %><%= y %><% end %></span>`
+      const output = formatter.format(input)
+      expect(output).toEqual(input)
+    })
+  })
 })

javascript/packages/formatter/test/erb/erb.test.ts

@@ -795,100 +795,102 @@ describe("@herb-tools/formatter", () => {
       </script>
      `
 
-     const expected = dedent`
-       <div class="relative">
-         <button id="column-toggle-btn" class="d-btn d-btn-sm d-btn-secondary">
-           <span>Show/Hide Columns</span>
-         </button>
-
-         <div
-           id="column-dropdown"
-           class="
-             absolute right-0 mt-1 bg-white border rounded-md shadow-lg z-10 p-2
-             hidden
-           "
-         >
-           <div class="flex flex-col space-y-1 min-w-[200px]">
-             <% columns.each do |column| %>
-               <div class="form-control">
-                 <label class="cursor-pointer label justify-start gap-2">
-                   <input
-                     type="checkbox"
-                     class="column-toggle checkbox checkbox-sm"
-                     data-column="<%= column[:name] %>"
-                     <%= 'checked' if column[:default_visible] %>
-                   >
-
-                   <span class="label-text text-muted-foreground"><%= column[:label] %></span>
-                 </label>
-               </div>
-             <% end %>
-           </div>
-         </div>
-       </div>
+    const expected = dedent`
+      <div class="relative">
+        <button id="column-toggle-btn" class="d-btn d-btn-sm d-btn-secondary">
+          <span>Show/Hide Columns</span>
+        </button>
+
+        <div
+          id="column-dropdown"
+          class="
+            absolute right-0 mt-1 bg-white border rounded-md shadow-lg z-10 p-2
+            hidden
+          "
+        >
+          <div class="flex flex-col space-y-1 min-w-[200px]">
+            <% columns.each do |column| %>
+              <div class="form-control">
+                <label class="cursor-pointer label justify-start gap-2">
+                  <input
+                    type="checkbox"
+                    class="column-toggle checkbox checkbox-sm"
+                    data-column="<%= column[:name] %>"
+                    <%= 'checked' if column[:default_visible] %>
+                  >
 
-       <script>
-       document.addEventListener('DOMContentLoaded', function() {
-         // Column toggle functionality
-         const columnToggleBtn = document.getElementById('column-toggle-btn');
-         const columnDropdown = document.getElementById('column-dropdown');
-
-         // Toggle dropdown visibility
-         columnToggleBtn.addEventListener('click', function() {
-           columnDropdown.classList.toggle('hidden');
-         });
+                  <span class="label-text text-muted-foreground">
+                    <%= column[:label] %>
+                  </span>
+                </label>
+              </div>
+            <% end %>
+          </div>
+        </div>
+      </div>
 
-         // Close dropdown when clicking outside
-         document.addEventListener('click', function(event) {
-           if (!columnToggleBtn.contains(event.target) && !columnDropdown.contains(event.target)) {
-             columnDropdown.classList.add('hidden');
-           }
-         });
+      <script>
+      document.addEventListener('DOMContentLoaded', function() {
+        // Column toggle functionality
+        const columnToggleBtn = document.getElementById('column-toggle-btn');
+        const columnDropdown = document.getElementById('column-dropdown');
 
-         // Handle column visibility toggles
-         const columnToggles = document.querySelectorAll('.column-toggle');
-         columnToggles.forEach(toggle => {
-           toggle.addEventListener('change', function() {
-             const columnName = this.dataset.column;
-             const columnCells = document.querySelectorAll(\`.column-\${columnName}\`);
-
-             columnCells.forEach(cell => {
-               if (this.checked) {
-                 cell.style.display = '';
-               } else {
-                 cell.style.display = 'none';
-               }
-             });
+        // Toggle dropdown visibility
+        columnToggleBtn.addEventListener('click', function() {
+          columnDropdown.classList.toggle('hidden');
+        });
 
-             // Save column visibility preferences to localStorage
-             localStorage.setItem(\`column_\${columnName}\`, this.checked ? 'visible' : 'hidden');
-           });
+        // Close dropdown when clicking outside
+        document.addEventListener('click', function(event) {
+          if (!columnToggleBtn.contains(event.target) && !columnDropdown.contains(event.target)) {
+            columnDropdown.classList.add('hidden');
+          }
+        });
 
-           // Apply saved preferences on load
-           const columnName = toggle.dataset.column;
-           const savedPreference = localStorage.getItem(\`column_\${columnName}\`);
-
-           if (savedPreference === 'hidden') {
-             toggle.checked = false;
-             const columnCells = document.querySelectorAll(\`.column-\${columnName}\`);
-             columnCells.forEach(cell => {
-               cell.style.display = 'none';
-             });
-           }
-         });
+        // Handle column visibility toggles
+        const columnToggles = document.querySelectorAll('.column-toggle');
+        columnToggles.forEach(toggle => {
+          toggle.addEventListener('change', function() {
+            const columnName = this.dataset.column;
+            const columnCells = document.querySelectorAll(\`.column-\${columnName}\`);
 
-          columnToggles.forEach(toggle => {
-           if (!toggle.checked) {
-             const columnName = toggle.dataset.column;
-             const columnCells = document.querySelectorAll(\`.column-\${columnName}\`);
-             columnCells.forEach(cell => {
-               cell.style.display = 'none';
-             });
-           }
-         });
-       });
-       </script>
-     `
+            columnCells.forEach(cell => {
+              if (this.checked) {
+                cell.style.display = '';
+              } else {
+                cell.style.display = 'none';
+              }
+            });
+
+            // Save column visibility preferences to localStorage
+            localStorage.setItem(\`column_\${columnName}\`, this.checked ? 'visible' : 'hidden');
+          });
+
+          // Apply saved preferences on load
+          const columnName = toggle.dataset.column;
+          const savedPreference = localStorage.getItem(\`column_\${columnName}\`);
+
+          if (savedPreference === 'hidden') {
+            toggle.checked = false;
+            const columnCells = document.querySelectorAll(\`.column-\${columnName}\`);
+            columnCells.forEach(cell => {
+              cell.style.display = 'none';
+            });
+          }
+        });
+
+         columnToggles.forEach(toggle => {
+          if (!toggle.checked) {
+            const columnName = toggle.dataset.column;
+            const columnCells = document.querySelectorAll(\`.column-\${columnName}\`);
+            columnCells.forEach(cell => {
+              cell.style.display = 'none';
+            });
+          }
+        });
+      });
+      </script>
+    `
 
     const result = formatter.format(input)
     expect(result).toBe(expected)
@@ -1365,7 +1367,9 @@ describe("@herb-tools/formatter", () => {
             <span>
               <strong>Cover Image</strong><br>
               Dimensions
-              <strong><%= "#{cover.metadata['width']}x#{cover.metadata['height']}" %></strong>
+              <strong>
+                <%= "#{cover.metadata['width']}x#{cover.metadata['height']}" %>
+              </strong>
               &mdash;
               <%= link_to "View original", rails_blob_path(cover), target: "_blank", rel: "noopener" %>
             </span>