From 788fd77359c42a7da7f1936b25384da9f94a7ccf Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Mon, 2 Mar 2026 17:46:52 +0100 Subject: [PATCH 1/2] Formatter: Extract Spacing Analyzer and Comment Helpers --- .../packages/formatter/src/comment-helpers.ts | 129 ++++++ .../packages/formatter/src/format-helpers.ts | 15 + .../packages/formatter/src/format-printer.ts | 378 ++---------------- .../formatter/src/spacing-analyzer.ts | 244 +++++++++++ .../formatter/test/comment-helpers.test.ts | 154 +++++++ .../formatter/test/spacing-analyzer.test.ts | 285 +++++++++++++ 6 files changed, 857 insertions(+), 348 deletions(-) create mode 100644 javascript/packages/formatter/src/comment-helpers.ts create mode 100644 javascript/packages/formatter/src/spacing-analyzer.ts create mode 100644 javascript/packages/formatter/test/comment-helpers.test.ts create mode 100644 javascript/packages/formatter/test/spacing-analyzer.test.ts diff --git a/javascript/packages/formatter/src/comment-helpers.ts b/javascript/packages/formatter/src/comment-helpers.ts new file mode 100644 index 000000000..8f6372550 --- /dev/null +++ b/javascript/packages/formatter/src/comment-helpers.ts @@ -0,0 +1,129 @@ +import dedent from "dedent" + +import { isNode, isERBNode } from "@herb-tools/core" +import { IdentityPrinter } from "@herb-tools/printer" +import { Node, HTMLTextNode, LiteralNode } from "@herb-tools/core" + +/** + * Result of formatting an ERB comment. + * - `single-line`: the caller emits the text on a single line (using push or pushWithIndent) + * - `multi-line`: the caller emits header, indented content lines, and footer separately + */ +export type ERBCommentResult = + | { type: 'single-line'; text: string } + | { type: 'multi-line'; header: string; contentLines: string[]; footer: string } + +/** + * Extract the raw inner text from HTML comment children. + * Joins text/literal nodes by content and ERB nodes via IdentityPrinter. + */ +export function extractHTMLCommentContent(children: Node[]): string { + return children.map(child => { + if (isNode(child, HTMLTextNode) || isNode(child, LiteralNode)) { + return child.content + } else if (isERBNode(child)) { + return IdentityPrinter.print(child) + } else { + return "" + } + }).join("") +} + +/** + * Format the inner content of an HTML comment. + * + * Handles three cases: + * 1. IE conditional comments (`[if ...` / ` 1) { + const contentLines = lines.map(line => line.trim()).filter(line => line !== '') + return '\n' + contentLines.map(line => childIndent + line).join('\n') + '\n' + } else { + const contentLines = lines.filter((line, index) => { + return line.trim() !== '' && !(index === 0 || index === lines.length - 1) + }) + + const minIndent = contentLines.length > 0 ? Math.min(...contentLines.map(line => line.length - line.trimStart().length)) : 0 + + const processedLines = lines.map((line, index) => { + const trimmedLine = line.trim() + + if ((index === 0 || index === lines.length - 1) && trimmedLine === '') { + return line + } + + if (trimmedLine !== '') { + const currentIndent = line.length - line.trimStart().length + const relativeIndent = Math.max(0, currentIndent - minIndent) + + return childIndent + " ".repeat(relativeIndent) + trimmedLine + } + + return line + }) + + return processedLines.join('\n') + } + } else { + return ` ${rawInner.trim()} ` + } +} + +/** + * Format an ERB comment into either a single-line or multi-line result. + * + * @param open - The opening tag (e.g. "<%#") + * @param content - The raw content string between open/close tags + * @param close - The closing tag (e.g. "%>") + * @returns A discriminated union describing how to render the comment + */ +export function formatERBCommentLines(open: string, content: string, close: string): ERBCommentResult { + const contentLines = content.split("\n") + const contentTrimmedLines = content.trim().split("\n") + + if (contentLines.length === 1 && contentTrimmedLines.length === 1) { + const startsWithSpace = content[0] === " " + const before = startsWithSpace ? "" : " " + + return { type: 'single-line', text: open + before + content.trimEnd() + ' ' + close } + } + + if (contentTrimmedLines.length === 1) { + return { type: 'single-line', text: open + ' ' + content.trim() + ' ' + close } + } + + const firstLineEmpty = contentLines[0].trim() === "" + const dedentedContent = dedent(firstLineEmpty ? content : content.trimStart()) + + return { + type: 'multi-line', + header: open, + contentLines: dedentedContent.split("\n"), + footer: close + } +} diff --git a/javascript/packages/formatter/src/format-helpers.ts b/javascript/packages/formatter/src/format-helpers.ts index 75c9f27ef..b03f8e09b 100644 --- a/javascript/packages/formatter/src/format-helpers.ts +++ b/javascript/packages/formatter/src/format-helpers.ts @@ -498,6 +498,21 @@ export function isHerbDisableComment(node: Node): boolean { return trimmed.startsWith("herb:disable") } +/** + * Check if children contain a leading herb:disable comment (after optional whitespace) + */ +export function hasLeadingHerbDisable(children: Node[]): boolean { + for (const child of children) { + if (isNode(child, WhitespaceNode) || (isNode(child, HTMLTextNode) && child.content.trim() === "")) { + continue + } + + return isNode(child, ERBContentNode) && isHerbDisableComment(child) + } + + return false +} + /** * Check if a text node is YAML frontmatter (starts and ends with ---) */ diff --git a/javascript/packages/formatter/src/format-printer.ts b/javascript/packages/formatter/src/format-printer.ts index 6a8e3b746..6c8c19d5b 100644 --- a/javascript/packages/formatter/src/format-printer.ts +++ b/javascript/packages/formatter/src/format-printer.ts @@ -1,9 +1,9 @@ -import dedent from "dedent" - import { Printer, IdentityPrinter } from "@herb-tools/printer" import { TextFlowEngine } from "./text-flow-engine.js" import { AttributeRenderer } from "./attribute-renderer.js" +import { SpacingAnalyzer } from "./spacing-analyzer.js" import { isTextFlowNode } from "./text-flow-helpers.js" +import { extractHTMLCommentContent, formatHTMLCommentInner, formatERBCommentLines } from "./comment-helpers.js" import type { ERBNode } from "@herb-tools/core" import type { FormatOptions } from "./options.js" @@ -22,7 +22,6 @@ import { isCommentNode, isERBControlFlowNode, isERBCommentNode, - isERBOutputNode, isHTMLOpenTagNode, filterNodes, } from "@herb-tools/core" @@ -31,13 +30,12 @@ import { areAllNestedElementsInline, filterEmptyNodesForHerbDisable, filterSignificantChildren, - findPreviousMeaningfulSibling, hasComplexERBControlFlow, hasMixedTextAndInlineContent, hasMultilineTextContent, - isBlockLevelNode, isContentPreserving, isFrontmatter, + hasLeadingHerbDisable, isHerbDisableComment, isInlineElement, isNonWhitespaceNode, @@ -48,7 +46,6 @@ import { import { ASCII_WHITESPACE, - INLINE_ELEMENTS, SPACEABLE_CONTAINERS, } from "./format-helpers.js" @@ -67,7 +64,6 @@ import { HTMLTextNode, HTMLCommentNode, HTMLDoctypeNode, - LiteralNode, WhitespaceNode, ERBContentNode, ERBBlockNode, @@ -133,10 +129,9 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut private elementFormattingAnalysis = new Map() private nodeIsMultiline = new Map() private stringLineCount: number = 0 - private tagGroupsCache = new Map>() - private allSingleLineCache = new Map() private textFlow: TextFlowEngine private attributeRenderer: AttributeRenderer + private spacingAnalyzer: SpacingAnalyzer public source: string @@ -148,6 +143,7 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut this.maxLineLength = options.maxLineLength this.textFlow = new TextFlowEngine(this) this.attributeRenderer = new AttributeRenderer(this, this.maxLineLength, this.indentWidth) + this.spacingAnalyzer = new SpacingAnalyzer(this.nodeIsMultiline) } print(input: Node | ParseResult | Token): string { @@ -160,8 +156,7 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut this.indentLevel = 0 this.stringLineCount = 0 this.nodeIsMultiline.clear() - this.tagGroupsCache.clear() - this.allSingleLineCache.clear() + this.spacingAnalyzer.clear() this.visit(node) @@ -330,189 +325,6 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut return nodes.filter(child => isNoneOf(child, HTMLAttributeNode, WhitespaceNode)) } - /** - * Check if a node will render as multiple lines when formatted. - */ - private isMultilineElement(node: Node): boolean { - if (isNode(node, ERBContentNode)) { - return (node.content?.value || "").includes("\n") - } - - if (isNode(node, HTMLElementNode) && isContentPreserving(node)) { - return true - } - - const tracked = this.nodeIsMultiline.get(node) - - if (tracked !== undefined) { - return tracked - } - - return false - } - - /** - * Get a grouping key for a node (tag name for HTML, ERB type for ERB) - */ - private getGroupingKey(node: Node): string | null { - if (isNode(node, HTMLElementNode)) { - return getTagName(node) - } - - if (isERBOutputNode(node)) return "erb-output" - if (isERBCommentNode(node)) return "erb-comment" - if (isERBNode(node)) return "erb-code" - - return null - } - - /** - * Detect groups of consecutive same-tag/same-type single-line elements - * Returns a map of index -> group info for efficient lookup - */ - private detectTagGroups(siblings: Node[]): Map { - const cached = this.tagGroupsCache.get(siblings) - if (cached) return cached - - const groupMap = new Map() - const meaningfulNodes: Array<{ index: number; groupKey: string }> = [] - - for (let i = 0; i < siblings.length; i++) { - const node = siblings[i] - - if (!this.isMultilineElement(node)) { - const groupKey = this.getGroupingKey(node) - - if (groupKey) { - meaningfulNodes.push({ index: i, groupKey }) - } - } - } - - let groupStart = 0 - - while (groupStart < meaningfulNodes.length) { - const startGroupKey = meaningfulNodes[groupStart].groupKey - let groupEnd = groupStart - - while (groupEnd + 1 < meaningfulNodes.length && meaningfulNodes[groupEnd + 1].groupKey === startGroupKey) { - groupEnd++ - } - - if (groupEnd > groupStart) { - const groupStartIndex = meaningfulNodes[groupStart].index - const groupEndIndex = meaningfulNodes[groupEnd].index - - for (let i = groupStart; i <= groupEnd; i++) { - groupMap.set(meaningfulNodes[i].index, { - tagName: startGroupKey, - groupStart: groupStartIndex, - groupEnd: groupEndIndex - }) - } - } - - groupStart = groupEnd + 1 - } - - this.tagGroupsCache.set(siblings, groupMap) - - return groupMap - } - - /** - * Determine if spacing should be added between sibling elements - * - * This implements the "rule of three" intelligent spacing system: - * - Adds spacing between 3 or more meaningful siblings - * - Respects semantic groupings (e.g., ul/li, nav/a stay tight) - * - Groups comments with following elements - * - Preserves user-added spacing - * - * @param parentElement - The parent element containing the siblings - * @param siblings - Array of all sibling nodes - * @param currentIndex - Index of the current node being evaluated - * @param hasExistingSpacing - Whether user-added spacing already exists - * @returns true if spacing should be added before the current element - */ - private shouldAddSpacingBetweenSiblings(parentElement: HTMLElementNode | null, siblings: Node[], currentIndex: number): boolean { - const currentNode = siblings[currentIndex] - const previousMeaningfulIndex = findPreviousMeaningfulSibling(siblings, currentIndex) - const previousNode = previousMeaningfulIndex !== -1 ? siblings[previousMeaningfulIndex] : null - - if (previousNode && (isNode(previousNode, XMLDeclarationNode) || isNode(previousNode, HTMLDoctypeNode))) { - return true - } - - const hasMixedContent = siblings.some(child => isNode(child, HTMLTextNode) && child.content.trim() !== "") - - if (hasMixedContent) return false - - const isCurrentComment = isCommentNode(currentNode) - const isPreviousComment = previousNode ? isCommentNode(previousNode) : false - const isCurrentMultiline = this.isMultilineElement(currentNode) - const isPreviousMultiline = previousNode ? this.isMultilineElement(previousNode) : false - - if (isPreviousComment && !isCurrentComment && (isNode(currentNode, HTMLElementNode) || isERBNode(currentNode))) { - return isPreviousMultiline && isCurrentMultiline - } - - if (isPreviousComment && isCurrentComment) { - return false - } - - if (isCurrentMultiline || isPreviousMultiline) { - return true - } - - const meaningfulSiblings = siblings.filter(child => isNonWhitespaceNode(child)) - const parentTagName = parentElement ? getTagName(parentElement) : null - const isSpaceableContainer = !parentTagName || SPACEABLE_CONTAINERS.has(parentTagName) - const tagGroups = this.detectTagGroups(siblings) - - const cached = this.allSingleLineCache.get(siblings) - let allSingleLineHTMLElements: boolean - if (cached !== undefined) { - allSingleLineHTMLElements = cached - } else { - allSingleLineHTMLElements = meaningfulSiblings.every(node => isNode(node, HTMLElementNode) && !this.isMultilineElement(node)) - this.allSingleLineCache.set(siblings, allSingleLineHTMLElements) - } - - if (!isSpaceableContainer && meaningfulSiblings.length < 5) { - return false - } - - const currentGroup = tagGroups.get(currentIndex) - const previousGroup = previousNode ? tagGroups.get(previousMeaningfulIndex) : undefined - - if (currentGroup && previousGroup && currentGroup.groupStart === previousGroup.groupStart && currentGroup.groupEnd === previousGroup.groupEnd) { - return false - } - - if (previousGroup && previousGroup.groupEnd === previousMeaningfulIndex) { - return true - } - - if (allSingleLineHTMLElements && tagGroups.size === 0) { - return false - } - - if (isNode(currentNode, HTMLElementNode)) { - const currentTagName = getTagName(currentNode) - - if (currentTagName && INLINE_ELEMENTS.has(currentTagName)) { - return false - } - } - - const isBlockElement = isBlockLevelNode(currentNode) - const isERBBlock = isERBNode(currentNode) && isERBControlFlowNode(currentNode) - const isComment = isCommentNode(currentNode) - - return isBlockElement || isERBBlock || isComment - } - /** * Render multiline attributes for a tag */ @@ -630,7 +442,7 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut this.visit(child) if (lastMeaningfulNode && !hasHandledSpacing) { - const shouldAddSpacing = this.shouldAddSpacingBetweenSiblings( null, children, i) + const shouldAddSpacing = this.spacingAnalyzer.shouldAddSpacingBetweenSiblings( null, children, i) if (shouldAddSpacing) { this.lines.splice(childStartLine, 0, "") @@ -875,41 +687,6 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut }) } - /** - * 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 - } - /** * Visit element children with intelligent spacing logic * @@ -946,7 +723,7 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut if (run) { if (lastMeaningfulNode && !hasHandledSpacing) { - const hasBlankLineBefore = this.hasBlankLineBetween(body, index) + const hasBlankLineBefore = this.spacingAnalyzer.hasBlankLineBetween(body, index) if (hasBlankLineBefore) { this.push("") @@ -957,7 +734,7 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut 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) + const hasBlankLineAfter = hasBlankLineInTrailing || this.spacingAnalyzer.hasBlankLineBetween(body, run.endIndex) if (hasBlankLineAfter) { this.push("") @@ -993,7 +770,7 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut this.visit(child) if (lastMeaningfulNode && !hasHandledSpacing) { - const shouldAddSpacing = this.shouldAddSpacingBetweenSiblings(parentElement, body, index) + const shouldAddSpacing = this.spacingAnalyzer.shouldAddSpacingBetweenSiblings(parentElement, body, index) if (shouldAddSpacing) { this.lines.splice(childStartLine, 0, "") @@ -1028,7 +805,7 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut this.visit(child) if (lastMeaningfulNode && !hasHandledSpacing) { - const shouldAddSpacing = this.shouldAddSpacingBetweenSiblings(parentElement, body, index) + const shouldAddSpacing = this.spacingAnalyzer.shouldAddSpacingBetweenSiblings(parentElement, body, index) if (shouldAddSpacing) { this.lines.splice(childStartLine, 0, "") @@ -1148,119 +925,39 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut this.pushWithIndent(IdentityPrinter.print(node)) } - // TODO: rework visitHTMLCommentNode(node: HTMLCommentNode) { const open = node.comment_start?.value ?? "" const close = node.comment_end?.value ?? "" - - let inner: string - - if (node.children && node.children.length > 0) { - inner = node.children.map(child => { - if (isNode(child, HTMLTextNode) || isNode(child, LiteralNode)) { - return child.content - } else if (isERBNode(child)) { - return IdentityPrinter.print(child) - } else { - return "" - } - }).join("") - - const trimmedInner = inner.trim() - - if (trimmedInner.startsWith('[if ') && trimmedInner.endsWith(' 1) { - const contentLines = lines.map(line => line.trim()).filter(line => line !== '') - inner = '\n' + contentLines.map(line => childIndent + line).join('\n') + '\n' - } else { - const contentLines = lines.filter((line, index) => { - return line.trim() !== '' && !(index === 0 || index === lines.length - 1) - }) - - const minIndent = contentLines.length > 0 ? Math.min(...contentLines.map(line => line.length - line.trimStart().length)) : 0 - - const processedLines = lines.map((line, index) => { - const trimmedLine = line.trim() - - if ((index === 0 || index === lines.length - 1) && trimmedLine === '') { - return line - } - - if (trimmedLine !== '') { - const currentIndent = line.length - line.trimStart().length - const relativeIndent = Math.max(0, currentIndent - minIndent) - - return childIndent + " ".repeat(relativeIndent) + trimmedLine - } - - return line - }) - - inner = processedLines.join('\n') - } - } else { - inner = ` ${inner.trim()} ` - } - } else { - inner = "" - } + const rawInner = node.children && node.children.length > 0 + ? extractHTMLCommentContent(node.children) + : "" + const inner = rawInner ? formatHTMLCommentInner(rawInner, this.indentWidth) : "" this.pushWithIndent(open + inner + close) } visitERBCommentNode(node: ERBContentNode) { - const open = node.tag_opening?.value || "<%#" - const content = node?.content?.value || "" - const close = node.tag_closing?.value || "%>" - - const contentLines = content.split("\n") - const contentTrimmedLines = content.trim().split("\n") - - if (contentLines.length === 1 && contentTrimmedLines.length === 1) { - const startsWithSpace = content[0] === " " - const before = startsWithSpace ? "" : " " + const result = formatERBCommentLines( + node.tag_opening?.value || "<%#", + node?.content?.value || "", + node.tag_closing?.value || "%>" + ) + if (result.type === 'single-line') { if (this.inlineMode) { - this.push(open + before + content.trimEnd() + ' ' + close) + this.push(result.text) } else { - this.pushWithIndent(open + before + content.trimEnd() + ' ' + close) + this.pushWithIndent(result.text) } + } else { + this.pushWithIndent(result.header) - return - } - - if (contentTrimmedLines.length === 1) { - if (this.inlineMode) { - this.push(open + ' ' + content.trim() + ' ' + close) - } else { - this.pushWithIndent(open + ' ' + content.trim() + ' ' + close) - } + this.withIndent(() => { + result.contentLines.forEach(line => this.pushWithIndent(line)) + }) - return + this.pushWithIndent(result.footer) } - - const firstLineEmpty = contentLines[0].trim() === "" - const dedentedContent = dedent(firstLineEmpty ? content : content.trimStart()) - - this.pushWithIndent(open) - - this.withIndent(() => { - dedentedContent.split("\n").forEach(line => this.pushWithIndent(line)) - }) - - this.pushWithIndent(close) } visitHTMLDoctypeNode(node: HTMLDoctypeNode) { @@ -1798,21 +1495,6 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut return result } - /** - * Check if children contain a leading herb:disable comment (after optional whitespace) - */ - private hasLeadingHerbDisable(children: Node[]): boolean { - for (const child of children) { - if (isNode(child, WhitespaceNode) || (isNode(child, HTMLTextNode) && child.content.trim() === "")) { - continue - } - - return isNode(child, ERBContentNode) && isHerbDisableComment(child) - } - - return false - } - /** * Try to render just the children inline (without tags) */ @@ -1821,7 +1503,7 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut let hasInternalWhitespace = false let addedLeadingSpace = false - const hasHerbDisable = this.hasLeadingHerbDisable(children) + const hasHerbDisable = hasLeadingHerbDisable(children) const hasOnlyTextContent = children.every(child => isNode(child, HTMLTextNode) || isNode(child, WhitespaceNode)) const shouldPreserveSpaces = hasOnlyTextContent && tagName && isInlineElement(tagName) diff --git a/javascript/packages/formatter/src/spacing-analyzer.ts b/javascript/packages/formatter/src/spacing-analyzer.ts new file mode 100644 index 000000000..de1a9d5fc --- /dev/null +++ b/javascript/packages/formatter/src/spacing-analyzer.ts @@ -0,0 +1,244 @@ +import { Node, HTMLTextNode, HTMLElementNode, HTMLDoctypeNode, ERBContentNode, WhitespaceNode, XMLDeclarationNode } from "@herb-tools/core" +import { isNode, getTagName, isERBNode, isERBOutputNode, isERBCommentNode, isCommentNode, isERBControlFlowNode } from "@herb-tools/core" +import { findPreviousMeaningfulSibling, isBlockLevelNode, isContentPreserving, isNonWhitespaceNode } from "./format-helpers.js" + +import { INLINE_ELEMENTS, SPACEABLE_CONTAINERS } from "./format-helpers.js" + +/** + * SpacingAnalyzer determines when blank lines should be inserted between + * sibling elements. It implements the "rule of three" intelligent spacing + * system: adds spacing between 3+ meaningful siblings, respects semantic + * groupings, groups comments with following elements, and preserves + * user-added spacing. + */ +export class SpacingAnalyzer { + private nodeIsMultiline: Map + private tagGroupsCache = new Map>() + private allSingleLineCache = new Map() + + constructor(nodeIsMultiline: Map) { + this.nodeIsMultiline = nodeIsMultiline + } + + clear(): void { + this.tagGroupsCache.clear() + this.allSingleLineCache.clear() + } + + /** + * Determine if spacing should be added between sibling elements + * + * This implements the "rule of three" intelligent spacing system: + * - Adds spacing between 3 or more meaningful siblings + * - Respects semantic groupings (e.g., ul/li, nav/a stay tight) + * - Groups comments with following elements + * - Preserves user-added spacing + * + * @param parentElement - The parent element containing the siblings + * @param siblings - Array of all sibling nodes + * @param currentIndex - Index of the current node being evaluated + * @returns true if spacing should be added before the current element + */ + shouldAddSpacingBetweenSiblings(parentElement: HTMLElementNode | null, siblings: Node[], currentIndex: number): boolean { + const currentNode = siblings[currentIndex] + const previousMeaningfulIndex = findPreviousMeaningfulSibling(siblings, currentIndex) + const previousNode = previousMeaningfulIndex !== -1 ? siblings[previousMeaningfulIndex] : null + + if (previousNode && (isNode(previousNode, XMLDeclarationNode) || isNode(previousNode, HTMLDoctypeNode))) { + return true + } + + const hasMixedContent = siblings.some(child => isNode(child, HTMLTextNode) && child.content.trim() !== "") + + if (hasMixedContent) return false + + const isCurrentComment = isCommentNode(currentNode) + const isPreviousComment = previousNode ? isCommentNode(previousNode) : false + const isCurrentMultiline = this.isMultilineElement(currentNode) + const isPreviousMultiline = previousNode ? this.isMultilineElement(previousNode) : false + + if (isPreviousComment && !isCurrentComment && (isNode(currentNode, HTMLElementNode) || isERBNode(currentNode))) { + return isPreviousMultiline && isCurrentMultiline + } + + if (isPreviousComment && isCurrentComment) { + return false + } + + if (isCurrentMultiline || isPreviousMultiline) { + return true + } + + const meaningfulSiblings = siblings.filter(child => isNonWhitespaceNode(child)) + const parentTagName = parentElement ? getTagName(parentElement) : null + const isSpaceableContainer = !parentTagName || SPACEABLE_CONTAINERS.has(parentTagName) + const tagGroups = this.detectTagGroups(siblings) + + const cached = this.allSingleLineCache.get(siblings) + let allSingleLineHTMLElements: boolean + if (cached !== undefined) { + allSingleLineHTMLElements = cached + } else { + allSingleLineHTMLElements = meaningfulSiblings.every(node => isNode(node, HTMLElementNode) && !this.isMultilineElement(node)) + this.allSingleLineCache.set(siblings, allSingleLineHTMLElements) + } + + if (!isSpaceableContainer && meaningfulSiblings.length < 5) { + return false + } + + const currentGroup = tagGroups.get(currentIndex) + const previousGroup = previousNode ? tagGroups.get(previousMeaningfulIndex) : undefined + + if (currentGroup && previousGroup && currentGroup.groupStart === previousGroup.groupStart && currentGroup.groupEnd === previousGroup.groupEnd) { + return false + } + + if (previousGroup && previousGroup.groupEnd === previousMeaningfulIndex) { + return true + } + + if (allSingleLineHTMLElements && tagGroups.size === 0) { + return false + } + + if (isNode(currentNode, HTMLElementNode)) { + const currentTagName = getTagName(currentNode) + + if (currentTagName && INLINE_ELEMENTS.has(currentTagName)) { + return false + } + } + + const isBlockElement = isBlockLevelNode(currentNode) + const isERBBlock = isERBNode(currentNode) && isERBControlFlowNode(currentNode) + const isComment = isCommentNode(currentNode) + + return isBlockElement || isERBBlock || isComment + } + + /** + * Check if there's a blank line (double newline) in the nodes at the given index + */ + 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 will render as multiple lines when formatted. + */ + private isMultilineElement(node: Node): boolean { + if (isNode(node, ERBContentNode)) { + return (node.content?.value || "").includes("\n") + } + + if (isNode(node, HTMLElementNode) && isContentPreserving(node)) { + return true + } + + const tracked = this.nodeIsMultiline.get(node) + + if (tracked !== undefined) { + return tracked + } + + return false + } + + /** + * Get a grouping key for a node (tag name for HTML, ERB type for ERB) + */ + private getGroupingKey(node: Node): string | null { + if (isNode(node, HTMLElementNode)) { + return getTagName(node) + } + + if (isERBOutputNode(node)) return "erb-output" + if (isERBCommentNode(node)) return "erb-comment" + if (isERBNode(node)) return "erb-code" + + return null + } + + /** + * Detect groups of consecutive same-tag/same-type single-line elements + * Returns a map of index -> group info for efficient lookup + */ + private detectTagGroups(siblings: Node[]): Map { + const cached = this.tagGroupsCache.get(siblings) + if (cached) return cached + + const groupMap = new Map() + const meaningfulNodes: Array<{ index: number; groupKey: string }> = [] + + for (let i = 0; i < siblings.length; i++) { + const node = siblings[i] + + if (!this.isMultilineElement(node)) { + const groupKey = this.getGroupingKey(node) + + if (groupKey) { + meaningfulNodes.push({ index: i, groupKey }) + } + } + } + + let groupStart = 0 + + while (groupStart < meaningfulNodes.length) { + const startGroupKey = meaningfulNodes[groupStart].groupKey + let groupEnd = groupStart + + while (groupEnd + 1 < meaningfulNodes.length && meaningfulNodes[groupEnd + 1].groupKey === startGroupKey) { + groupEnd++ + } + + if (groupEnd > groupStart) { + const groupStartIndex = meaningfulNodes[groupStart].index + const groupEndIndex = meaningfulNodes[groupEnd].index + + for (let i = groupStart; i <= groupEnd; i++) { + groupMap.set(meaningfulNodes[i].index, { + tagName: startGroupKey, + groupStart: groupStartIndex, + groupEnd: groupEndIndex + }) + } + } + + groupStart = groupEnd + 1 + } + + this.tagGroupsCache.set(siblings, groupMap) + + return groupMap + } +} diff --git a/javascript/packages/formatter/test/comment-helpers.test.ts b/javascript/packages/formatter/test/comment-helpers.test.ts new file mode 100644 index 000000000..6f93ec5e8 --- /dev/null +++ b/javascript/packages/formatter/test/comment-helpers.test.ts @@ -0,0 +1,154 @@ +import { describe, test, expect, beforeAll } from "vitest" + +import { Herb } from "@herb-tools/node-wasm" +import { HTMLCommentNode, HTMLElementNode } from "@herb-tools/core" + +import { isNode } from "@herb-tools/core" +import { extractHTMLCommentContent, formatHTMLCommentInner, formatERBCommentLines } from "../src/comment-helpers.js" + +function parseComment(source: string) { + const result = Herb.parse(source) + const children = result.value.children + + for (const child of children) { + if (isNode(child, HTMLCommentNode)) { + return child + } + + if (isNode(child, HTMLElementNode)) { + for (const bodyChild of child.body) { + if (isNode(bodyChild, HTMLCommentNode)) { + return bodyChild + } + } + } + } + + return null +} + +describe("comment-helpers", () => { + beforeAll(async () => { + await Herb.load() + }) + + describe("extractHTMLCommentContent", () => { + test("extracts text content from comment children", () => { + const comment = parseComment("") + + expect(comment).not.toBeNull() + const content = extractHTMLCommentContent(comment!.children) + expect(content).toBe(" hello world ") + }) + + test("extracts content with ERB nodes", () => { + const comment = parseComment("") + + expect(comment).not.toBeNull() + const content = extractHTMLCommentContent(comment!.children) + expect(content).toBe(" hello <%= name %> ") + }) + + test("returns empty string for empty children", () => { + const content = extractHTMLCommentContent([]) + expect(content).toBe("") + }) + }) + + describe("formatHTMLCommentInner", () => { + test("wraps single-line content with spaces", () => { + const result = formatHTMLCommentInner(" hello world ", 2) + expect(result).toBe(" hello world ") + }) + + test("trims and wraps single-line content", () => { + const result = formatHTMLCommentInner("hello", 2) + expect(result).toBe(" hello ") + }) + + test("passes through IE conditional comments", () => { + const raw = "[if lte IE 9]>some content { + const raw = "line1\nline2\nline3" + const result = formatHTMLCommentInner(raw, 2) + + expect(result).toBe("\n line1\n line2\n line3\n") + }) + + test("reformats multiline content with relative indent preservation", () => { + const raw = "\n line1\n indented\n line3\n" + const result = formatHTMLCommentInner(raw, 2) + + expect(result).toBe("\n line1\n indented\n line3\n") + }) + + test("handles empty inner string", () => { + const result = formatHTMLCommentInner("", 2) + expect(result).toBe(" ") + }) + }) + + describe("formatERBCommentLines", () => { + test("formats single-line comment without leading space", () => { + const result = formatERBCommentLines("<%#", "comment text", "%>") + + expect(result.type).toBe("single-line") + if (result.type === "single-line") { + expect(result.text).toBe("<%# comment text %>") + } + }) + + test("formats single-line comment with leading space", () => { + const result = formatERBCommentLines("<%#", " comment text", "%>") + + expect(result.type).toBe("single-line") + if (result.type === "single-line") { + expect(result.text).toBe("<%# comment text %>") + } + }) + + test("trims multiline content that reduces to single line", () => { + const result = formatERBCommentLines("<%#", "\n comment text \n", "%>") + + expect(result.type).toBe("single-line") + if (result.type === "single-line") { + expect(result.text).toBe("<%# comment text %>") + } + }) + + test("returns multi-line result for true multiline content", () => { + const result = formatERBCommentLines("<%#", "\n line1\n line2\n", "%>") + + expect(result.type).toBe("multi-line") + if (result.type === "multi-line") { + expect(result.header).toBe("<%#") + expect(result.footer).toBe("%>") + expect(result.contentLines).toEqual(["line1", "line2"]) + } + }) + + test("handles empty content", () => { + const result = formatERBCommentLines("<%#", "", "%>") + + expect(result.type).toBe("single-line") + if (result.type === "single-line") { + expect(result.text).toBe("<%# %>") + } + }) + + test("handles content with leading whitespace on multiple lines", () => { + const result = formatERBCommentLines("<%#", "\n first line\n second line\n", "%>") + + expect(result.type).toBe("multi-line") + if (result.type === "multi-line") { + expect(result.header).toBe("<%#") + expect(result.footer).toBe("%>") + expect(result.contentLines).toEqual(["first line", "second line"]) + } + }) + }) +}) diff --git a/javascript/packages/formatter/test/spacing-analyzer.test.ts b/javascript/packages/formatter/test/spacing-analyzer.test.ts new file mode 100644 index 000000000..bb5757dd5 --- /dev/null +++ b/javascript/packages/formatter/test/spacing-analyzer.test.ts @@ -0,0 +1,285 @@ +import { describe, test, expect, beforeAll } from "vitest" +import { Herb } from "@herb-tools/node-wasm" +import { isNode, getTagName } from "@herb-tools/core" +import { Node, HTMLElementNode, HTMLTextNode } from "@herb-tools/core" + +import { SpacingAnalyzer } from "../src/spacing-analyzer.js" + +function parse(source: string) { + return Herb.parse(source) +} + +function parseChildren(source: string): Node[] { + return parse(source).value.children +} + +function parseBody(source: string): { body: Node[]; element: HTMLElementNode | null } { + const children = parseChildren(source) + const element = children[0] + + if (isNode(element, HTMLElementNode)) { + return { body: element.body, element } + } + + return { body: children, element: null } +} + +describe("SpacingAnalyzer", () => { + beforeAll(async () => { + await Herb.load() + }) + + describe("shouldAddSpacingBetweenSiblings", () => { + test("adds spacing after doctype", () => { + const nodeIsMultiline = new Map() + const analyzer = new SpacingAnalyzer(nodeIsMultiline) + const children = parseChildren("\n") + + let htmlIndex = -1 + + for (let i = 0; i < children.length; i++) { + if (isNode(children[i], HTMLElementNode) && getTagName(children[i] as HTMLElementNode) === "html") { + htmlIndex = i + break + } + } + + expect(htmlIndex).toBeGreaterThan(0) + expect(analyzer.shouldAddSpacingBetweenSiblings(null, children, htmlIndex)).toBe(true) + }) + + test("returns false when siblings have mixed text content", () => { + const nodeIsMultiline = new Map() + const analyzer = new SpacingAnalyzer(nodeIsMultiline) + const { body, element } = parseBody("
text a b
") + + let targetIndex = -1 + + for (let i = 1; i < body.length; i++) { + if (isNode(body[i], HTMLElementNode)) { + targetIndex = i + break + } + } + + if (targetIndex > 0) { + expect(analyzer.shouldAddSpacingBetweenSiblings(element, body, targetIndex)).toBe(false) + } + }) + + test("adds spacing between multiline elements", () => { + const nodeIsMultiline = new Map() + const analyzer = new SpacingAnalyzer(nodeIsMultiline) + const { body, element } = parseBody("
\n

a

\n

b

\n
") + + for (const child of body) { + if (isNode(child, HTMLElementNode)) { + nodeIsMultiline.set(child, true) + } + } + + let count = 0 + let secondPIndex = -1 + + for (let i = 0; i < body.length; i++) { + if (isNode(body[i], HTMLElementNode) && getTagName(body[i] as HTMLElementNode) === "p") { + count++ + if (count === 2) { + secondPIndex = i + break + } + } + } + + expect(secondPIndex).toBeGreaterThan(0) + expect(analyzer.shouldAddSpacingBetweenSiblings(element, body, secondPIndex)).toBe(true) + }) + + test("returns false for inline elements", () => { + const nodeIsMultiline = new Map() + const analyzer = new SpacingAnalyzer(nodeIsMultiline) + const { body, element } = parseBody("
\na\nb\nc\n
") + + let lastSpanIndex = -1 + + for (let i = body.length - 1; i >= 0; i--) { + if (isNode(body[i], HTMLElementNode) && getTagName(body[i] as HTMLElementNode) === "span") { + lastSpanIndex = i + break + } + } + + if (lastSpanIndex > 0) { + expect(analyzer.shouldAddSpacingBetweenSiblings(element, body, lastSpanIndex)).toBe(false) + } + }) + + test("returns false for elements in same tag group", () => { + const nodeIsMultiline = new Map() + const analyzer = new SpacingAnalyzer(nodeIsMultiline) + const { body, element } = parseBody("
\n

a

\n

b

\n

c

\n
x
\n
") + + let count = 0 + let secondPIndex = -1 + + for (let i = 0; i < body.length; i++) { + if (isNode(body[i], HTMLElementNode) && getTagName(body[i] as HTMLElementNode) === "p") { + count++ + if (count === 2) { + secondPIndex = i + break + } + } + } + + expect(secondPIndex).toBeGreaterThan(0) + expect(analyzer.shouldAddSpacingBetweenSiblings(element, body, secondPIndex)).toBe(false) + }) + + test("adds spacing at group boundary in spaceable container", () => { + const nodeIsMultiline = new Map() + const analyzer = new SpacingAnalyzer(nodeIsMultiline) + const { body, element } = parseBody("
\n

a

\n

b

\n
x
\n
") + + let divIndex = -1 + + for (let i = 0; i < body.length; i++) { + if (isNode(body[i], HTMLElementNode) && getTagName(body[i] as HTMLElementNode) === "div") { + divIndex = i + break + } + } + + expect(divIndex).toBeGreaterThan(0) + expect(analyzer.shouldAddSpacingBetweenSiblings(element, body, divIndex)).toBe(true) + }) + + test("returns false for non-spaceable containers with fewer than 5 meaningful children", () => { + const nodeIsMultiline = new Map() + const analyzer = new SpacingAnalyzer(nodeIsMultiline) + const { body, element } = parseBody("
    \n
  • a
    • \n
  • b
    • \n
  • c
    • \n
") + + let count = 0 + let secondLiIndex = -1 + + for (let i = 0; i < body.length; i++) { + if (isNode(body[i], HTMLElementNode) && getTagName(body[i] as HTMLElementNode) === "li") { + count++ + if (count === 2) { + secondLiIndex = i + break + } + } + } + + if (secondLiIndex > 0) { + expect(analyzer.shouldAddSpacingBetweenSiblings(element, body, secondLiIndex)).toBe(false) + } + }) + + test("does not add spacing between consecutive comments", () => { + const nodeIsMultiline = new Map() + const analyzer = new SpacingAnalyzer(nodeIsMultiline) + const { body, element } = parseBody("
\n\n\n
") + + let commentCount = 0 + let secondCommentIndex = -1 + + for (let i = 0; i < body.length; i++) { + const child = body[i] + + if (!isNode(child, HTMLTextNode) || child.content.trim() !== "") { + if (!isNode(child, HTMLTextNode)) { + commentCount++ + + if (commentCount === 2) { + secondCommentIndex = i + break + } + } + } + } + + if (secondCommentIndex > 0) { + expect(analyzer.shouldAddSpacingBetweenSiblings(element, body, secondCommentIndex)).toBe(false) + } + }) + }) + + describe("hasBlankLineBetween", () => { + test("detects double newline in preceding text node", () => { + const nodeIsMultiline = new Map() + const analyzer = new SpacingAnalyzer(nodeIsMultiline) + const { body } = parseBody("
\n

a

\n\n

b

\n
") + + let count = 0 + let secondPIndex = -1 + + for (let i = 0; i < body.length; i++) { + if (isNode(body[i], HTMLElementNode) && getTagName(body[i] as HTMLElementNode) === "p") { + count++ + + if (count === 2) { + secondPIndex = i + break + } + } + } + + expect(secondPIndex).toBeGreaterThan(0) + expect(analyzer.hasBlankLineBetween(body, secondPIndex)).toBe(true) + }) + + test("returns false for single newlines", () => { + const nodeIsMultiline = new Map() + const analyzer = new SpacingAnalyzer(nodeIsMultiline) + const { body } = parseBody("
\n

a

\n

b

\n
") + + let count = 0 + let secondPIndex = -1 + + for (let i = 0; i < body.length; i++) { + if (isNode(body[i], HTMLElementNode) && getTagName(body[i] as HTMLElementNode) === "p") { + count++ + + if (count === 2) { + secondPIndex = i + break + } + } + } + + expect(secondPIndex).toBeGreaterThan(0) + expect(analyzer.hasBlankLineBetween(body, secondPIndex)).toBe(false) + }) + }) + + describe("clear", () => { + test("resets internal caches", () => { + const nodeIsMultiline = new Map() + const analyzer = new SpacingAnalyzer(nodeIsMultiline) + const { body, element } = parseBody("
\n

a

\n

b

\n
x
\n
") + + for (let i = 0; i < body.length; i++) { + if (isNode(body[i], HTMLElementNode)) { + analyzer.shouldAddSpacingBetweenSiblings(element, body, i) + } + } + + expect(() => analyzer.clear()).not.toThrow() + + let divIndex = -1 + + for (let i = 0; i < body.length; i++) { + if (isNode(body[i], HTMLElementNode) && getTagName(body[i] as HTMLElementNode) === "div") { + divIndex = i + break + } + } + + if (divIndex > 0) { + expect(analyzer.shouldAddSpacingBetweenSiblings(element, body, divIndex)).toBe(true) + } + }) + }) +}) From 11af7921f0da01666e8c4e075fde0f7adf743cc0 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Mon, 2 Mar 2026 21:05:22 +0100 Subject: [PATCH 2/2] More simplification --- .../packages/formatter/src/format-printer.ts | 400 +++++++++--------- 1 file changed, 204 insertions(+), 196 deletions(-) diff --git a/javascript/packages/formatter/src/format-printer.ts b/javascript/packages/formatter/src/format-printer.ts index 6c8c19d5b..b6a1ec7b6 100644 --- a/javascript/packages/formatter/src/format-printer.ts +++ b/javascript/packages/formatter/src/format-printer.ts @@ -11,6 +11,12 @@ import type { TextFlowDelegate } from "./text-flow-engine.js" import type { AttributeRendererDelegate } from "./attribute-renderer.js" import type { ElementFormattingAnalysis } from "./format-helpers.js" +interface ChildVisitResult { + newIndex: number + lastMeaningfulNode: Node | null + hasHandledSpacing: boolean +} + import { getTagName, getCombinedAttributeName, @@ -19,7 +25,6 @@ import { isParseResult, isNoneOf, isERBNode, - isCommentNode, isERBControlFlowNode, isERBCommentNode, isHTMLOpenTagNode, @@ -520,97 +525,124 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut } visitHTMLElementBody(body: Node[], element: HTMLElementNode) { - const tagName = getTagName(element) - if (isContentPreserving(element)) { - element.body.map(child => { - if (isNode(child, HTMLElementNode)) { - const wasInlineMode = this.inlineMode - this.inlineMode = true - - const formattedElement = this.capture(() => this.visit(child)).join("") - this.pushToLastLine(formattedElement) - - this.inlineMode = wasInlineMode - } else { - this.pushToLastLine(IdentityPrinter.print(child)) - } - }) - + this.visitContentPreservingBody(element) return } + const tagName = getTagName(element) const analysis = this.elementFormattingAnalysis.get(element) const hasTextFlow = this.textFlow.isInTextFlowContext(body) const children = filterSignificantChildren(body) if (analysis?.elementContentInline) { - if (children.length === 0) return + this.visitInlineElementBody(body, tagName, hasTextFlow, children) + return + } - const oldInlineMode = this.inlineMode - const nodesToRender = hasTextFlow ? body : children + if (children.length === 0) return - const hasOnlyTextContent = nodesToRender.every(child => isNode(child, HTMLTextNode) || isNode(child, WhitespaceNode)) - const shouldPreserveSpaces = hasOnlyTextContent && isInlineElement(tagName) + const { comment, hasLeadingWhitespace, remainingChildren, remainingBody } = this.stripLeadingHerbDisable(children, body) - this.inlineMode = true + if (comment) { + const herbDisableString = this.captureHerbDisableInline(comment) + this.pushToLastLine((hasLeadingWhitespace ? ' ' : '') + herbDisableString) + } + + if (remainingChildren.length === 0) return + + this.withIndent(() => { + if (hasTextFlow) { + this.textFlow.visitTextFlowChildren(remainingBody) + } else { + this.visitElementChildren(comment ? remainingChildren : body, element) + } + }) + } + + private visitContentPreservingBody(element: HTMLElementNode) { + element.body.map(child => { + if (isNode(child, HTMLElementNode)) { + const wasInlineMode = this.inlineMode + this.inlineMode = true + + const formattedElement = this.capture(() => this.visit(child)).join("") + this.pushToLastLine(formattedElement) + + this.inlineMode = wasInlineMode + } else { + this.pushToLastLine(IdentityPrinter.print(child)) + } + }) + } - const lines = this.capture(() => { - nodesToRender.forEach(child => { - if (isNode(child, HTMLTextNode)) { - if (hasTextFlow) { - const normalizedContent = child.content.replace(ASCII_WHITESPACE, ' ') + private visitInlineElementBody(body: Node[], tagName: string, hasTextFlow: boolean, children: Node[]) { + if (children.length === 0) return + + const oldInlineMode = this.inlineMode + const nodesToRender = hasTextFlow ? body : children + + const hasOnlyTextContent = nodesToRender.every(child => isNode(child, HTMLTextNode) || isNode(child, WhitespaceNode)) + const shouldPreserveSpaces = hasOnlyTextContent && isInlineElement(tagName) - if (normalizedContent && normalizedContent !== ' ') { - this.push(normalizedContent) + this.inlineMode = true + + const lines = this.capture(() => { + nodesToRender.forEach(child => { + if (isNode(child, HTMLTextNode)) { + if (hasTextFlow) { + const normalizedContent = child.content.replace(ASCII_WHITESPACE, ' ') + + if (normalizedContent && normalizedContent !== ' ') { + this.push(normalizedContent) + } else if (normalizedContent === ' ') { + this.push(' ') + } + } else { + const normalizedContent = child.content.replace(ASCII_WHITESPACE, ' ') + + if (shouldPreserveSpaces && normalizedContent) { + this.push(normalizedContent) + } else { + const trimmedContent = normalizedContent.trim() + + if (trimmedContent) { + this.push(trimmedContent) } else if (normalizedContent === ' ') { this.push(' ') } - } else { - const normalizedContent = child.content.replace(ASCII_WHITESPACE, ' ') - - if (shouldPreserveSpaces && normalizedContent) { - this.push(normalizedContent) - } else { - const trimmedContent = normalizedContent.trim() - - if (trimmedContent) { - this.push(trimmedContent) - } else if (normalizedContent === ' ') { - this.push(' ') - } - } } - } else if (isNode(child, WhitespaceNode)) { - return - } else { - this.visit(child) } - }) + } else if (isNode(child, WhitespaceNode)) { + return + } else { + this.visit(child) + } }) + }) - const content = lines.join('') - - const inlineContent = shouldPreserveSpaces - ? (hasTextFlow ? content.replace(ASCII_WHITESPACE, ' ') : content) - : (hasTextFlow ? content.replace(ASCII_WHITESPACE, ' ').trim() : content.trim()) - - if (inlineContent) { - this.pushToLastLine(inlineContent) - } + const content = lines.join('') - this.inlineMode = oldInlineMode + const inlineContent = shouldPreserveSpaces + ? (hasTextFlow ? content.replace(ASCII_WHITESPACE, ' ') : content) + : (hasTextFlow ? content.replace(ASCII_WHITESPACE, ' ').trim() : content.trim()) - return + if (inlineContent) { + this.pushToLastLine(inlineContent) } - if (children.length === 0) return + this.inlineMode = oldInlineMode + } + private stripLeadingHerbDisable(children: Node[], body: Node[]): { + comment: Node | null + hasLeadingWhitespace: boolean + remainingChildren: Node[] + remainingBody: Node[] + } { let leadingHerbDisableComment: Node | null = null let leadingHerbDisableIndex = -1 let firstWhitespaceIndex = -1 - let remainingChildren = children - let remainingBodyUnfiltered = body for (let i = 0; i < children.length; i++) { const child = children[i] @@ -631,60 +663,30 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut break } - if (leadingHerbDisableComment && leadingHerbDisableIndex >= 0) { - remainingChildren = children.filter((_, index) => { - if (index === leadingHerbDisableIndex) return false - - if (firstWhitespaceIndex >= 0 && index === leadingHerbDisableIndex - 1) { - const child = children[index] - - if (isNode(child, WhitespaceNode) || isPureWhitespaceNode(child)) { - return false - } - } - - return true - }) + if (!leadingHerbDisableComment || leadingHerbDisableIndex < 0) { + return { comment: null, hasLeadingWhitespace: false, remainingChildren: children, remainingBody: body } + } - remainingBodyUnfiltered = body.filter((_, index) => { - if (index === leadingHerbDisableIndex) return false + const filterOut = (nodes: Node[]) => nodes.filter((_, index) => { + if (index === leadingHerbDisableIndex) return false - if (firstWhitespaceIndex >= 0 && index === leadingHerbDisableIndex - 1) { - const child = body[index] + if (firstWhitespaceIndex >= 0 && index === leadingHerbDisableIndex - 1) { + const child = nodes[index] - if (isNode(child, WhitespaceNode) || isPureWhitespaceNode(child)) { - return false - } + if (isNode(child, WhitespaceNode) || isPureWhitespaceNode(child)) { + return false } + } - return true - }) - } - - if (leadingHerbDisableComment) { - const herbDisableString = this.capture(() => { - const savedIndentLevel = this.indentLevel - this.indentLevel = 0 - this.inlineMode = true - this.visit(leadingHerbDisableComment) - this.inlineMode = false - this.indentLevel = savedIndentLevel - }).join("") - - const hasLeadingWhitespace = firstWhitespaceIndex >= 0 && firstWhitespaceIndex < leadingHerbDisableIndex + return true + }) - this.pushToLastLine((hasLeadingWhitespace ? ' ' : '') + herbDisableString) + return { + comment: leadingHerbDisableComment, + hasLeadingWhitespace: firstWhitespaceIndex >= 0 && firstWhitespaceIndex < leadingHerbDisableIndex, + remainingChildren: filterOut(children), + remainingBody: filterOut(body), } - - if (remainingChildren.length === 0) return - - this.withIndent(() => { - if (hasTextFlow) { - this.textFlow.visitTextFlowChildren(remainingBodyUnfiltered) - } else { - this.visitElementChildren(leadingHerbDisableComment ? remainingChildren : body, element) - } - }) } /** @@ -718,89 +720,87 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut if (!isNonWhitespaceNode(child)) continue - if (isTextFlowNode(child)) { - const run = this.textFlow.collectTextFlowRun(body, index) - - if (run) { - if (lastMeaningfulNode && !hasHandledSpacing) { - const hasBlankLineBefore = this.spacingAnalyzer.hasBlankLineBetween(body, index) - - if (hasBlankLineBefore) { - this.push("") - } - } + const textFlowResult = this.visitTextFlowRunInChildren(body, index, lastMeaningfulNode, hasHandledSpacing) - this.textFlow.visitTextFlowChildren(run.nodes) + if (textFlowResult) { + index = textFlowResult.newIndex + lastMeaningfulNode = textFlowResult.lastMeaningfulNode + hasHandledSpacing = textFlowResult.hasHandledSpacing + continue + } - const lastRunNode = run.nodes[run.nodes.length - 1] - const hasBlankLineInTrailing = isNode(lastRunNode, HTMLTextNode) && lastRunNode.content.includes('\n\n') - const hasBlankLineAfter = hasBlankLineInTrailing || this.spacingAnalyzer.hasBlankLineBetween(body, run.endIndex) + const herbDisableResult: ChildVisitResult | null = + isNode(child, HTMLElementNode) && child.close_tag + ? this.visitChildWithTrailingHerbDisable(child, body, index, parentElement, lastMeaningfulNode, hasHandledSpacing) + : null - if (hasBlankLineAfter) { - this.push("") - hasHandledSpacing = true - } - - lastMeaningfulNode = run.nodes[run.nodes.length - 1] + if (herbDisableResult) { + index = herbDisableResult.newIndex + lastMeaningfulNode = herbDisableResult.lastMeaningfulNode + hasHandledSpacing = herbDisableResult.hasHandledSpacing + continue + } - if (!hasBlankLineAfter) { - hasHandledSpacing = false - } + const childStartLine = this.stringLineCount + this.visit(child) - index = run.endIndex - 1 + if (lastMeaningfulNode && !hasHandledSpacing) { + const shouldAddSpacing = this.spacingAnalyzer.shouldAddSpacingBetweenSiblings(parentElement, body, index) - continue + if (shouldAddSpacing) { + this.lines.splice(childStartLine, 0, "") + this.stringLineCount++ } } - let hasTrailingHerbDisable = false + lastMeaningfulNode = child + hasHandledSpacing = false + } + } - if (isNode(child, HTMLElementNode) && child.close_tag) { - for (let j = index + 1; j < body.length; j++) { - const nextChild = body[j] + private visitTextFlowRunInChildren(body: Node[], index: number, lastMeaningfulNode: Node | null, hasHandledSpacing: boolean): ChildVisitResult | null { + const child = body[index] - if (isNode(nextChild, WhitespaceNode) || isPureWhitespaceNode(nextChild)) { - continue - } + if (!isTextFlowNode(child)) return null - if (isNode(nextChild, ERBContentNode) && isHerbDisableComment(nextChild)) { - hasTrailingHerbDisable = true + const run = this.textFlow.collectTextFlowRun(body, index) - const childStartLine = this.stringLineCount - this.visit(child) + if (!run) return null - if (lastMeaningfulNode && !hasHandledSpacing) { - const shouldAddSpacing = this.spacingAnalyzer.shouldAddSpacingBetweenSiblings(parentElement, body, index) + if (lastMeaningfulNode && !hasHandledSpacing) { + const hasBlankLineBefore = this.spacingAnalyzer.hasBlankLineBetween(body, index) - if (shouldAddSpacing) { - this.lines.splice(childStartLine, 0, "") - this.stringLineCount++ - } - } + if (hasBlankLineBefore) { + this.push("") + } + } - const herbDisableString = this.capture(() => { - const savedIndentLevel = this.indentLevel - this.indentLevel = 0 - this.inlineMode = true - this.visit(nextChild) - this.inlineMode = false - this.indentLevel = savedIndentLevel - }).join("") + this.textFlow.visitTextFlowChildren(run.nodes) - this.pushToLastLine(' ' + herbDisableString) + const lastRunNode = run.nodes[run.nodes.length - 1] + const hasBlankLineInTrailing = isNode(lastRunNode, HTMLTextNode) && lastRunNode.content.includes('\n\n') + const hasBlankLineAfter = hasBlankLineInTrailing || this.spacingAnalyzer.hasBlankLineBetween(body, run.endIndex) - index = j - lastMeaningfulNode = child - hasHandledSpacing = false + if (hasBlankLineAfter) { + this.push("") + } - break - } + return { + newIndex: run.endIndex - 1, + lastMeaningfulNode: run.nodes[run.nodes.length - 1], + hasHandledSpacing: hasBlankLineAfter, + } + } - break - } + private visitChildWithTrailingHerbDisable(child: HTMLElementNode, body: Node[], index: number, parentElement: HTMLElementNode | null, lastMeaningfulNode: Node | null, hasHandledSpacing: boolean): ChildVisitResult | null { + for (let j = index + 1; j < body.length; j++) { + const nextChild = body[j] + + if (isNode(nextChild, WhitespaceNode) || isPureWhitespaceNode(nextChild)) { + continue } - if (!hasTrailingHerbDisable) { + if (isNode(nextChild, ERBContentNode) && isHerbDisableComment(nextChild)) { const childStartLine = this.stringLineCount this.visit(child) @@ -813,10 +813,21 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut } } - lastMeaningfulNode = child - hasHandledSpacing = false + const herbDisableString = this.captureHerbDisableInline(nextChild) + + this.pushToLastLine(' ' + herbDisableString) + + return { + newIndex: j, + lastMeaningfulNode: child, + hasHandledSpacing: false, + } } + + break } + + return null } visitHTMLOpenTagNode(node: HTMLOpenTagNode) { @@ -1240,19 +1251,7 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut if (hasNonInlineChildElements) return false - let hasLeadingHerbDisable = false - - for (const child of node.body) { - if (isNode(child, WhitespaceNode) || isPureWhitespaceNode(child)) { - continue - } - if (isNode(child, ERBContentNode) && isHerbDisableComment(child)) { - hasLeadingHerbDisable = true - } - break - } - - if (hasLeadingHerbDisable && !isInlineElement(tagName)) { + if (hasLeadingHerbDisable(node.body) && !isInlineElement(tagName)) { return false } @@ -1260,8 +1259,7 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut const fullInlineResult = this.tryRenderInlineFull(node, tagName, filterNodes(getOpenTagChildren(node), HTMLAttributeNode), node.body) if (fullInlineResult) { - const totalLength = this.indent.length + fullInlineResult.length - return totalLength <= this.maxLineLength + return this.fitsOnCurrentLine(fullInlineResult) } return false @@ -1280,12 +1278,8 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut if (allNestedAreInline && (!hasMultilineText || hasMixedContent)) { const fullInlineResult = this.tryRenderInlineFull(node, tagName, filterNodes(getOpenTagChildren(node), HTMLAttributeNode), node.body) - if (fullInlineResult) { - const totalLength = this.indent.length + fullInlineResult.length - - if (totalLength <= this.maxLineLength) { - return true - } + if (fullInlineResult && this.fitsOnCurrentLine(fullInlineResult)) { + return true } } @@ -1302,9 +1296,8 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut const childrenContent = this.renderChildrenInline(children) const fullLine = openTagResult + childrenContent + `` - const totalLength = this.indent.length + fullLine.length - if (totalLength <= this.maxLineLength) { + if (this.fitsOnCurrentLine(fullLine)) { return true } } @@ -1330,6 +1323,21 @@ export class FormatPrinter extends Printer implements TextFlowDelegate, Attribut // --- Utility methods --- + private captureHerbDisableInline(node: Node): string { + return this.capture(() => { + const savedIndentLevel = this.indentLevel + this.indentLevel = 0 + this.inlineMode = true + this.visit(node) + this.inlineMode = false + this.indentLevel = savedIndentLevel + }).join("") + } + + private fitsOnCurrentLine(content: string): boolean { + return this.indent.length + content.length <= this.maxLineLength + } + private formatFrontmatter(node: DocumentNode): Node[] { const firstChild = node.children[0] const hasFrontmatter = firstChild && isFrontmatter(firstChild)