diff --git a/javascript/packages/formatter/src/format-printer.ts b/javascript/packages/formatter/src/format-printer.ts index 9acadc9e8..6bc0fbedf 100644 --- a/javascript/packages/formatter/src/format-printer.ts +++ b/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) { diff --git a/javascript/packages/formatter/test/erb/whitespace-formatting.test.ts b/javascript/packages/formatter/test/erb/whitespace-formatting.test.ts index 70ab853de..6ef7fe1b2 100644 --- a/javascript/packages/formatter/test/erb/whitespace-formatting.test.ts +++ b/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` +
+
+

What a nice new tool

+ + 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 %>. + +

+ You can edit this file here app/views/app/tools/custom_tool.html.erb. +

+
+
+ ` + const result = formatter120.format(source) + + expect(result).toBe(dedent` +
+
+

What a nice new tool

+ + 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 %>. + +

+ You can edit this file here + app/views/app/tools/custom_tool.html.erb. +

+
+
+ `) + }) + }) + describe("shared utility validation", () => { test("demonstrates consistent ERB content formatting where it applies", () => { const erbContentCases = [