Fix ERB text flow formatting to keep adjacent inline elements together

Fixes: #903

Author: ydah

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

@@ -1742,8 +1742,71 @@ export class FormatPrinter extends Printer {
     const adjacentInlineCount = countAdjacentInlineElements(children)
 
     if (adjacentInlineCount >= 2) {
-      const { processedIndices } = this.renderAdjacentInlineElements(children, adjacentInlineCount)
-      this.visitRemainingChildren(children, processedIndices)
+      let processedCount = 0
+      const adjacentInlineContent = this.capture(() => {
+        const oldInlineMode = this.inlineMode
+        this.inlineMode = true
+
+        for (
+          let i = 0;
+          i < children.length && processedCount < adjacentInlineCount;
+          i++
+        ) {
+          const child = children[i]
+
+          if (isPureWhitespaceNode(child) || isNode(child, WhitespaceNode)) {
+            continue
+          }
+
+          const isInlineOrERB =
+            (isNode(child, HTMLElementNode) &&
+              isInlineElement(getTagName(child))) ||
+            isNode(child, ERBContentNode)
+
+          if (isInlineOrERB) {
+            if (isNode(child, HTMLElementNode)) {
+              this.push(this.renderInlineElementAsString(child))
+            } else if (isNode(child, ERBContentNode)) {
+              this.push(this.renderERBAsString(child))
+            }
+            processedCount++
+          } else {
+            break
+          }
+        }
+
+        this.inlineMode = oldInlineMode
+      }).join("")
+
+      processedCount = 0
+      const remainingChildren = children.filter((child) => {
+        if (processedCount >= adjacentInlineCount) return true
+
+        if (isPureWhitespaceNode(child) || isNode(child, WhitespaceNode)) {
+          return true
+        }
+
+        const isInlineOrERB =
+          (isNode(child, HTMLElementNode) &&
+            isInlineElement(getTagName(child))) ||
+          isNode(child, ERBContentNode)
+
+        if (isInlineOrERB) {
+          processedCount++
+          return false
+        }
+
+        return true
+      })
+
+      if (adjacentInlineContent) {
+        this.buildAndWrapTextFlowWithPrefix(
+          adjacentInlineContent,
+          remainingChildren,
+        )
+      } else if (remainingChildren.length > 0) {
+        this.buildAndWrapTextFlow(remainingChildren)
+      }
 
       return
     }
@@ -1983,12 +2046,16 @@ export class FormatPrinter extends Printer {
   }
 
   /**
-   * Build words array from text/inline/ERB and wrap them
+   * Build words array from text/inline/ERB with an optional prefix and wrap them
    */
-  private buildAndWrapTextFlow(children: Node[]): void {
+  private buildAndWrapTextFlowWithPrefix(prefix: string, children: Node[]): void {
     const unitsWithNodes: ContentUnitWithNode[] = this.buildContentUnitsWithNodes(children)
     const words: Array<{ word: string, isHerbDisable: boolean }> = []
 
+    if (prefix) {
+      words.push({ word: prefix, isHerbDisable: false })
+    }
+
     for (const { unit, node } of unitsWithNodes) {
       if (unit.breaksFlow) {
         this.flushWords(words)
@@ -2036,6 +2103,13 @@ export class FormatPrinter extends Printer {
     this.flushWords(words)
   }
 
+  /**
+   * Build words array from text/inline/ERB and wrap them
+   */
+  private buildAndWrapTextFlow(children: Node[]): void {
+    this.buildAndWrapTextFlowWithPrefix('', children)
+  }
+
   /**
    * Try to merge text that follows an atomic unit (ERB/inline) with no whitespace
    * Returns true if merge was performed

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

@@ -1379,7 +1379,22 @@ describe("@herb-tools/formatter", () => {
      `
 
     const result = formatter.format(input)
-    expect(result).toBe(input)
+    expect(result).toBe(dedent`
+      <% if cover.present? %>
+        <figure class="figure">
+          <%= image_tag attachment_url(cover), size: "460x249", class: "img-fluid figure-img" %>
+
+          <figcaption class="figure-caption text-center">
+            <span>
+              <strong>Cover Image</strong><br> Dimensions
+              <strong><%= "#{cover.metadata['width']}x#{cover.metadata['height']}" %></strong>
+              &mdash;
+              <%= link_to "View original", rails_blob_path(cover), target: "_blank", rel: "noopener" %>
+            </span>
+          </figcaption>
+        </figure>
+      <% end %>
+    `)
   })
 
   test("adjecent ERB text within elements", () => {

javascript/packages/formatter/test/erb/whitespace-formatting.test.ts

@@ -504,6 +504,63 @@ describe("ERB whitespace formatting", () => {
     })
   })
 
+  describe("line breaking elements and text flow", () => {
+    test("does not add line breaks after ERB tags following <br>", () => {
+      const source = dedent`
+        <p><strong>Ut enim ad minima veniam</strong><br>
+         <%= foo %> sed quia consequuntur magni <%= bar %>. Lorem ipsum dolor sit amet...</p>
+      `
+      const result = formatter.format(source)
+
+      expect(result).not.toContain(`<%= foo %>\n`)
+      expect(result).not.toContain(`<%= bar %>\n .`)
+    })
+
+    test("keeps period attached to ERB tag in text flow", () => {
+      const source = `<p><strong>Summary:</strong><br> Lorem ipsum <%= foo %> dolor <%= bar %>. Sit amet...</p>`
+      const result = formatter.format(source)
+
+      expect(result).not.toContain(`<%= bar %>\n.`)
+    })
+
+    test("does not separate ERB tags from surrounding text", () => {
+      const source = `Failed to <%= model.ignored? ? "unignore" : "ignore" %> model`
+      const result = formatter.format(source)
+
+      expect(result).toBe(
+        `Failed to <%= model.ignored? ? "unignore" : "ignore" %> model`,
+      )
+    })
+
+    test("preserves apostrophe with ERB tag inline", () => {
+      const source = `<p><%= user.name %>'s profile</p>`
+      const result = formatter.format(source)
+
+      expect(result).toBe(`<p><%= user.name %>'s profile</p>`)
+    })
+
+    test("preserves possessive apostrophe after ERB tag", () => {
+      const source = `Waiting for <%= contractor.name.first_or_business %>'s signature.`
+      const result = formatter.format(source)
+
+      expect(result).toBe(
+        `Waiting for <%= contractor.name.first_or_business %>'s signature.`,
+      )
+    })
+
+    test("handles long text with ERB tags after <br>", () => {
+      const source = dedent`
+        <p><strong>Ut enim ad minima veniam</strong><br>
+          <%= foo %> sed quia consequuntur magni <%= bar %>. Lorem ipsum dolor sit amet consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.</p>
+      `
+      const result = formatter.format(source)
+
+      expect(result).not.toContain(`<%= foo %>\n  sed`)
+      expect(result).not.toContain(`<%= bar %>\n  .`)
+      expect(result).toContain(`<%= bar %>. Lorem`)
+    })
+  })
+
   describe("shared utility validation", () => {
     test("demonstrates consistent ERB content formatting where it applies", () => {
       const erbContentCases = [

javascript/packages/formatter/test/html/text-content.test.ts

@@ -269,9 +269,9 @@ describe("@herb-tools/formatter", () => {
         <body>
           <div class="main">
             <p>
-              <strong>Bold Heading:</strong><br>
-              <%= Date.current %>: Lorem ipsum dolor sit amet, consectetur adipiscing
-              elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
+              <strong>Bold Heading:</strong><br> <%= Date.current %>: Lorem ipsum
+              dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor
+              incididunt ut labore et dolore magna aliqua.
             </p>
           </div>
         </body>