Formatter: Fix punctuation separating from ERB tags in mixed content (#1266)

Resolves #931

Author: marcoroth

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

@@ -1099,6 +1099,117 @@ export class FormatPrinter extends Printer {
     })
   }
 
+  /**
+   * Check if there's a blank line (double newline) in the nodes at the given index
+   */
+  private hasBlankLineBetween(body: Node[], index: number): boolean {
+    for (let lookbackIndex = index - 1; lookbackIndex >= 0 && lookbackIndex >= index - 2; lookbackIndex--) {
+      const node = body[lookbackIndex]
+
+      if (isNode(node, HTMLTextNode) && node.content.includes('\n\n')) {
+        return true
+      }
+
+      if (isNode(node, WhitespaceNode)) {
+        continue
+      }
+
+      break
+    }
+
+    for (let lookaheadIndex = index; lookaheadIndex < body.length && lookaheadIndex <= index + 1; lookaheadIndex++) {
+      const node = body[lookaheadIndex]
+
+      if (isNode(node, HTMLTextNode) && node.content.includes('\n\n')) {
+        return true
+      }
+
+      if (isNode(node, WhitespaceNode)) {
+        continue
+      }
+
+      break
+    }
+
+    return false
+  }
+
+  /**
+   * Check if a node is part of a text flow run (text, ERB, or inline element)
+   */
+  private isTextFlowNode(node: Node): boolean {
+    if (isNode(node, ERBContentNode)) return true
+    if (isNode(node, HTMLTextNode) && node.content.trim() !== "") return true
+    if (isNode(node, HTMLElementNode) && isInlineElement(getTagName(node))) return true
+
+    return false
+  }
+
+  /**
+   * Check if a node is whitespace that can appear within a text flow run
+   */
+  private isTextFlowWhitespace(node: Node): boolean {
+    if (isNode(node, WhitespaceNode)) return true
+    if (isNode(node, HTMLTextNode) && node.content.trim() === "" && !node.content.includes('\n\n')) return true
+
+    return false
+  }
+
+  /**
+   * Collect a run of text flow nodes starting at the given index.
+   * Returns the nodes in the run and the index after the last node.
+   */
+  private collectTextFlowRun(body: Node[], startIndex: number): { nodes: Node[], endIndex: number } | null {
+    const nodes: Node[] = []
+    let index = startIndex
+    let textFlowCount = 0
+
+    while (index < body.length) {
+      const child = body[index]
+
+      if (this.isTextFlowNode(child)) {
+        nodes.push(child)
+        textFlowCount++
+        index++
+      } else if (this.isTextFlowWhitespace(child)) {
+        let hasMoreTextFlow = false
+
+        for (let lookaheadIndex = index + 1; lookaheadIndex < body.length; lookaheadIndex++) {
+          if (this.isTextFlowNode(body[lookaheadIndex])) {
+            hasMoreTextFlow = true
+            break
+          }
+
+          if (this.isTextFlowWhitespace(body[lookaheadIndex])) {
+            continue
+          }
+
+          break
+        }
+
+        if (hasMoreTextFlow) {
+          nodes.push(child)
+          index++
+        } else {
+          break
+        }
+      } else {
+        break
+      }
+    }
+
+    if (textFlowCount >= 2) {
+      const hasText = nodes.some(node => isNode(node, HTMLTextNode) && node.content.trim() !== "")
+      const hasAtomicContent = nodes.some(node => isNode(node, ERBContentNode) || (isNode(node, HTMLElementNode) && isInlineElement(getTagName(node))))
+
+      if (hasText && hasAtomicContent) {
+        return { nodes, endIndex: index }
+      }
+    }
+
+    return null
+  }
+
   /**
    * Visit element children with intelligent spacing logic
    *
@@ -1130,6 +1241,41 @@ export class FormatPrinter extends Printer {
 
       if (!isNonWhitespaceNode(child)) continue
 
+      if (this.isTextFlowNode(child)) {
+        const run = this.collectTextFlowRun(body, index)
+
+        if (run) {
+          if (lastMeaningfulNode && !hasHandledSpacing) {
+            const hasBlankLineBefore = this.hasBlankLineBetween(body, index)
+
+            if (hasBlankLineBefore) {
+              this.push("")
+            }
+          }
+
+          this.visitTextFlowChildren(run.nodes)
+
+          const lastRunNode = run.nodes[run.nodes.length - 1]
+          const hasBlankLineInTrailing = isNode(lastRunNode, HTMLTextNode) && lastRunNode.content.includes('\n\n')
+          const hasBlankLineAfter = hasBlankLineInTrailing || this.hasBlankLineBetween(body, run.endIndex)
+
+          if (hasBlankLineAfter) {
+            this.push("")
+            hasHandledSpacing = true
+          }
+
+          lastMeaningfulNode = run.nodes[run.nodes.length - 1]
+
+          if (!hasBlankLineAfter) {
+            hasHandledSpacing = false
+          }
+
+          index = run.endIndex - 1
+
+          continue
+        }
+      }
+
       let hasTrailingHerbDisable = false
 
       if (isNode(child, HTMLElementNode) && child.close_tag) {

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

@@ -495,6 +495,43 @@ describe("ERB whitespace formatting", () => {
     })
   })
 
+  describe("issue 931: punctuation after ERB link_to tags", () => {
+    test("issue 931: keeps commas and periods attached to ERB link_to tags (@adrianthedev)", () => {
+      const formatter120 = new Formatter(Herb, { indentWidth: 2, maxLineLength: 120 })
+
+      const source = dedent`
+        <div class="py-6 min-h-24">
+          <div class="px-6 space-y-4">
+            <h3>What a nice new tool</h3>
+
+            Go to <%= link_to "new comment", app.new_resources_comment_path %>, <%= link_to 'the first user', app.resources_user_path(1) %>, or <%= link_to 'hey on main app', main_app.hey_path %>.
+
+            <p>
+              You can edit this file here <code class="p-1 rounded-sm bg-gray-500 text-white text-sm">app/views/app/tools/custom_tool.html.erb</code>.
+            </p>
+          </div>
+        </div>
+      `
+      const result = formatter120.format(source)
+
+      expect(result).toBe(dedent`
+        <div class="py-6 min-h-24">
+          <div class="px-6 space-y-4">
+            <h3>What a nice new tool</h3>
+
+            Go to <%= link_to "new comment", app.new_resources_comment_path %>,
+            <%= link_to 'the first user', app.resources_user_path(1) %>, or <%= link_to 'hey on main app', main_app.hey_path %>.
+
+            <p>
+              You can edit this file here
+              <code class="p-1 rounded-sm bg-gray-500 text-white text-sm">app/views/app/tools/custom_tool.html.erb</code>.
+            </p>
+          </div>
+        </div>
+      `)
+    })
+  })
+
   describe("shared utility validation", () => {
     test("demonstrates consistent ERB content formatting where it applies", () => {
       const erbContentCases = [