From 2cef8eb135c9db034c6e737151d7fa51008f405b Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Mon, 2 Mar 2026 17:09:47 +0100 Subject: [PATCH] Formatter: Extract Attribute Renderer --- .../formatter/src/attribute-renderer.ts | 309 +++++++++++++ .../packages/formatter/src/format-helpers.ts | 6 + .../packages/formatter/src/format-printer.ts | 326 ++------------ .../formatter/src/text-flow-engine.ts | 7 +- .../formatter/test/attribute-renderer.test.ts | 421 ++++++++++++++++++ 5 files changed, 767 insertions(+), 302 deletions(-) create mode 100644 javascript/packages/formatter/src/attribute-renderer.ts create mode 100644 javascript/packages/formatter/test/attribute-renderer.test.ts diff --git a/javascript/packages/formatter/src/attribute-renderer.ts b/javascript/packages/formatter/src/attribute-renderer.ts new file mode 100644 index 000000000..4f8dc3b74 --- /dev/null +++ b/javascript/packages/formatter/src/attribute-renderer.ts @@ -0,0 +1,309 @@ +import { IdentityPrinter } from "@herb-tools/printer" +import { HTMLAttributeNode, HTMLAttributeValueNode, HTMLTextNode, LiteralNode, ERBContentNode } from "@herb-tools/core" + +import { getCombinedAttributeName, getCombinedStringFromNodes, isNode } from "@herb-tools/core" + +import { ASCII_WHITESPACE, FORMATTABLE_ATTRIBUTES, TOKEN_LIST_ATTRIBUTES } from "./format-helpers.js" + +import type { Node, ERBNode } from "@herb-tools/core" + +/** + * Interface that the delegate must implement to provide + * ERB reconstruction capabilities to the AttributeRenderer. + */ +export interface AttributeRendererDelegate { + reconstructERBNode(node: ERBNode, withFormatting: boolean): string +} + +/** + * AttributeRenderer converts HTMLAttributeNode AST nodes into formatted strings. + * It handles class attribute wrapping, multiline attribute formatting, + * quote normalization, and token list attribute spacing. + */ +export class AttributeRenderer { + private delegate: AttributeRendererDelegate + private maxLineLength: number + private indentWidth: number + + public currentAttributeName: string | null = null + public indentLevel: number = 0 + + constructor( + delegate: AttributeRendererDelegate, + maxLineLength: number, + indentWidth: number, + ) { + this.delegate = delegate + this.maxLineLength = maxLineLength + this.indentWidth = indentWidth + } + + /** + * Check if we're currently processing a token list attribute that needs spacing + */ + get isInTokenListAttribute(): boolean { + return this.currentAttributeName !== null && TOKEN_LIST_ATTRIBUTES.has(this.currentAttributeName) + } + + /** + * Render attributes as a space-separated string + */ + renderAttributesString(attributes: HTMLAttributeNode[], tagName: string): string { + if (attributes.length === 0) return "" + + return ` ${attributes.map(attribute => this.renderAttribute(attribute, tagName)).join(" ")}` + } + + /** + * Determine if a tag should be rendered inline based on attribute count and other factors + */ + shouldRenderInline( + totalAttributeCount: number, + inlineLength: number, + indentLength: number, + maxLineLength: number = this.maxLineLength, + hasComplexERB: boolean = false, + hasMultilineAttributes: boolean = false, + attributes: HTMLAttributeNode[] = [] + ): boolean { + if (hasComplexERB || hasMultilineAttributes) return false + + if (totalAttributeCount === 0) { + return inlineLength + indentLength <= maxLineLength + } + + if (totalAttributeCount === 1 && attributes.length === 1) { + const attribute = attributes[0] + const attributeName = this.getAttributeName(attribute) + + if (attributeName === 'class') { + const attributeValue = this.getAttributeValue(attribute) + const wouldBeMultiline = this.wouldClassAttributeBeMultiline(attributeValue, indentLength) + + if (!wouldBeMultiline) { + return true + } else { + return false + } + } + } + + if (totalAttributeCount > 3 || inlineLength + indentLength > maxLineLength) { + return false + } + + return true + } + + wouldClassAttributeBeMultiline(content: string, indentLength: number): boolean { + const normalizedContent = content.replace(ASCII_WHITESPACE, ' ').trim() + const hasActualNewlines = /\r?\n/.test(content) + + if (hasActualNewlines && normalizedContent.length > 80) { + const lines = content.split(/\r?\n/).map(line => line.trim()).filter(line => line) + + if (lines.length > 1) { + return true + } + } + + const attributeLine = `class="${normalizedContent}"` + const currentIndent = indentLength + + if (currentIndent + attributeLine.length > this.maxLineLength && normalizedContent.length > 60) { + if (/<%[^%]*%>/.test(normalizedContent)) { + return false + } + + const classes = normalizedContent.split(' ') + const lines = this.breakTokensIntoLines(classes, currentIndent) + return lines.length > 1 + } + + return false + } + + // TOOD: extract to core or reuse function from core + getAttributeName(attribute: HTMLAttributeNode): string { + return attribute.name ? getCombinedAttributeName(attribute.name) : "" + } + + // TOOD: extract to core or reuse function from core + getAttributeValue(attribute: HTMLAttributeNode): string { + if (isNode(attribute.value, HTMLAttributeValueNode)) { + return attribute.value.children.map(child => isNode(child, HTMLTextNode) ? child.content : IdentityPrinter.print(child)).join('') + } + + return '' + } + + hasMultilineAttributes(attributes: HTMLAttributeNode[]): boolean { + return attributes.some(attribute => { + if (isNode(attribute.value, HTMLAttributeValueNode)) { + const content = getCombinedStringFromNodes(attribute.value.children) + + if (/\r?\n/.test(content)) { + const name = attribute.name ? getCombinedAttributeName(attribute.name) : "" + + if (name === "class") { + const normalizedContent = content.replace(ASCII_WHITESPACE, ' ').trim() + + return normalizedContent.length > 80 + } + + const lines = content.split(/\r?\n/) + + if (lines.length > 1) { + return lines.slice(1).some(line => /^[ \t\n\r]+/.test(line)) + } + } + } + + return false + }) + } + + formatClassAttribute(content: string, name: string, equals: string, open_quote: string, close_quote: string): string { + const normalizedContent = content.replace(ASCII_WHITESPACE, ' ').trim() + const hasActualNewlines = /\r?\n/.test(content) + + if (hasActualNewlines && normalizedContent.length > 80) { + const lines = content.split(/\r?\n/).map(line => line.trim()).filter(line => line) + + if (lines.length > 1) { + return open_quote + this.formatMultilineAttributeValue(lines) + close_quote + } + } + + const currentIndent = this.indentLevel * this.indentWidth + const attributeLine = `${name}${equals}${open_quote}${normalizedContent}${close_quote}` + + if (currentIndent + attributeLine.length > this.maxLineLength && normalizedContent.length > 60) { + if (/<%[^%]*%>/.test(normalizedContent)) { + return open_quote + normalizedContent + close_quote + } + + const classes = normalizedContent.split(' ') + const lines = this.breakTokensIntoLines(classes, currentIndent) + + if (lines.length > 1) { + return open_quote + this.formatMultilineAttributeValue(lines) + close_quote + } + } + + return open_quote + normalizedContent + close_quote + } + + isFormattableAttribute(attributeName: string, tagName: string): boolean { + const globalFormattable = FORMATTABLE_ATTRIBUTES['*'] || [] + const tagSpecificFormattable = FORMATTABLE_ATTRIBUTES[tagName.toLowerCase()] || [] + + return globalFormattable.includes(attributeName) || tagSpecificFormattable.includes(attributeName) + } + + formatMultilineAttribute(content: string, name: string, open_quote: string, close_quote: string): string { + if (name === 'srcset' || name === 'sizes') { + const normalizedContent = content.replace(ASCII_WHITESPACE, ' ').trim() + + return open_quote + normalizedContent + close_quote + } + + const lines = content.split('\n') + + if (lines.length <= 1) { + return open_quote + content + close_quote + } + + const formattedContent = this.formatMultilineAttributeValue(lines) + + return open_quote + formattedContent + close_quote + } + + formatMultilineAttributeValue(lines: string[]): string { + const indent = " ".repeat((this.indentLevel + 1) * this.indentWidth) + const closeIndent = " ".repeat(this.indentLevel * this.indentWidth) + + return "\n" + lines.map(line => indent + line).join("\n") + "\n" + closeIndent + } + + breakTokensIntoLines(tokens: string[], currentIndent: number, separator: string = ' '): string[] { + const lines: string[] = [] + let currentLine = '' + + for (const token of tokens) { + const testLine = currentLine ? currentLine + separator + token : token + + if (testLine.length > (this.maxLineLength - currentIndent - 6)) { + if (currentLine) { + lines.push(currentLine) + currentLine = token + } else { + lines.push(token) + } + } else { + currentLine = testLine + } + } + + if (currentLine) lines.push(currentLine) + + return lines + } + + renderAttribute(attribute: HTMLAttributeNode, tagName: string): string { + const name = attribute.name ? getCombinedAttributeName(attribute.name) : "" + const equals = attribute.equals?.value ?? "" + + this.currentAttributeName = name + + let value = "" + + if (isNode(attribute.value, HTMLAttributeValueNode)) { + const attributeValue = attribute.value + + let open_quote = attributeValue.open_quote?.value ?? "" + let close_quote = attributeValue.close_quote?.value ?? "" + let htmlTextContent = "" + + const content = attributeValue.children.map((child: Node) => { + if (isNode(child, HTMLTextNode) || isNode(child, LiteralNode)) { + htmlTextContent += child.content + + return child.content + } else if (isNode(child, ERBContentNode)) { + return this.delegate.reconstructERBNode(child, true) + } else { + const printed = IdentityPrinter.print(child) + + if (this.isInTokenListAttribute) { + return printed.replace(/%>([^<\s])/g, '%> $1').replace(/([^>\s])<%/g, '$1 <%') + } + + return printed + } + }).join("") + + if (open_quote === "" && close_quote === "") { + open_quote = '"' + close_quote = '"' + } else if (open_quote === "'" && close_quote === "'" && !htmlTextContent.includes('"')) { + open_quote = '"' + close_quote = '"' + } + + if (this.isFormattableAttribute(name, tagName)) { + if (name === 'class') { + value = this.formatClassAttribute(content, name, equals, open_quote, close_quote) + } else { + value = this.formatMultilineAttribute(content, name, open_quote, close_quote) + } + } else { + value = open_quote + content + close_quote + } + } + + this.currentAttributeName = null + + return name + equals + value + } +} diff --git a/javascript/packages/formatter/src/format-helpers.ts b/javascript/packages/formatter/src/format-helpers.ts index fe80b5d09..75c9f27ef 100644 --- a/javascript/packages/formatter/src/format-helpers.ts +++ b/javascript/packages/formatter/src/format-helpers.ts @@ -34,6 +34,12 @@ export interface ContentUnitWithNode { // --- Constants --- +/** + * ASCII whitespace pattern - use instead of \s to preserve Unicode whitespace + * characters like NBSP (U+00A0) and full-width space (U+3000) + */ +export const ASCII_WHITESPACE = /[ \t\n\r]+/g + // TODO: we can probably expand this list with more tags/attributes export const FORMATTABLE_ATTRIBUTES: Record = { '*': ['class'], diff --git a/javascript/packages/formatter/src/format-printer.ts b/javascript/packages/formatter/src/format-printer.ts index 15a9d5067..6a8e3b746 100644 --- a/javascript/packages/formatter/src/format-printer.ts +++ b/javascript/packages/formatter/src/format-printer.ts @@ -2,17 +2,18 @@ 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 { isTextFlowNode } from "./text-flow-helpers.js" import type { ERBNode } from "@herb-tools/core" import type { FormatOptions } from "./options.js" import type { TextFlowDelegate } from "./text-flow-engine.js" +import type { AttributeRendererDelegate } from "./attribute-renderer.js" import type { ElementFormattingAnalysis } from "./format-helpers.js" import { getTagName, getCombinedAttributeName, - getCombinedStringFromNodes, isNode, isToken, isParseResult, @@ -46,10 +47,9 @@ import { } from "./format-helpers.js" import { - FORMATTABLE_ATTRIBUTES, + ASCII_WHITESPACE, INLINE_ELEMENTS, SPACEABLE_CONTAINERS, - TOKEN_LIST_ATTRIBUTES, } from "./format-helpers.js" import { @@ -91,12 +91,6 @@ import { Token } from "@herb-tools/core" -/** - * ASCII whitespace pattern - use instead of \s to preserve Unicode whitespace - * characters like NBSP (U+00A0) and full-width space (U+3000) - */ -const ASCII_WHITESPACE = /[ \t\n\r]+/g - /** * Gets the children of an open tag, narrowing from the union type. * Returns empty array for conditional open tags. @@ -117,7 +111,7 @@ function getOpenTagClosing(element: HTMLElementNode): Token | null { * Printer traverses the Herb AST using the Visitor pattern * and emits a formatted string with proper indentation, line breaks, and attribute wrapping. */ -export class FormatPrinter extends Printer implements TextFlowDelegate { +export class FormatPrinter extends Printer implements TextFlowDelegate, AttributeRendererDelegate { /** * @deprecated integrate indentWidth into this.options and update FormatOptions to extend from @herb-tools/printer options */ @@ -135,7 +129,6 @@ export class FormatPrinter extends Printer implements TextFlowDelegate { private indentLevel: number = 0 private inlineMode: boolean = false private inConditionalOpenTagContext: boolean = false - private currentAttributeName: string | null = null private elementStack: HTMLElementNode[] = [] private elementFormattingAnalysis = new Map() private nodeIsMultiline = new Map() @@ -143,7 +136,7 @@ export class FormatPrinter extends Printer implements TextFlowDelegate { private tagGroupsCache = new Map>() private allSingleLineCache = new Map() private textFlow: TextFlowEngine - + private attributeRenderer: AttributeRenderer public source: string @@ -154,6 +147,7 @@ export class FormatPrinter extends Printer implements TextFlowDelegate { this.indentWidth = options.indentWidth this.maxLineLength = options.maxLineLength this.textFlow = new TextFlowEngine(this) + this.attributeRenderer = new AttributeRenderer(this, this.maxLineLength, this.indentWidth) } print(input: Node | ParseResult | Token): string { @@ -519,218 +513,6 @@ export class FormatPrinter extends Printer implements TextFlowDelegate { return isBlockElement || isERBBlock || isComment } - /** - * Check if we're currently processing a token list attribute that needs spacing - */ - private get isInTokenListAttribute(): boolean { - return this.currentAttributeName !== null && TOKEN_LIST_ATTRIBUTES.has(this.currentAttributeName) - } - - /** - * Render attributes as a space-separated string - */ - private renderAttributesString(attributes: HTMLAttributeNode[]): string { - if (attributes.length === 0) return "" - - return ` ${attributes.map(attribute => this.renderAttribute(attribute)).join(" ")}` - } - - /** - * Determine if a tag should be rendered inline based on attribute count and other factors - */ - private shouldRenderInline( - totalAttributeCount: number, - inlineLength: number, - indentLength: number, - maxLineLength: number = this.maxLineLength, - hasComplexERB: boolean = false, - hasMultilineAttributes: boolean = false, - attributes: HTMLAttributeNode[] = [] - ): boolean { - if (hasComplexERB || hasMultilineAttributes) return false - - if (totalAttributeCount === 0) { - return inlineLength + indentLength <= maxLineLength - } - - if (totalAttributeCount === 1 && attributes.length === 1) { - const attribute = attributes[0] - const attributeName = this.getAttributeName(attribute) - - if (attributeName === 'class') { - const attributeValue = this.getAttributeValue(attribute) - const wouldBeMultiline = this.wouldClassAttributeBeMultiline(attributeValue, indentLength) - - if (!wouldBeMultiline) { - return true - } else { - return false - } - } - } - - if (totalAttributeCount > 3 || inlineLength + indentLength > maxLineLength) { - return false - } - - return true - } - - private wouldClassAttributeBeMultiline(content: string, indentLength: number): boolean { - const normalizedContent = content.replace(ASCII_WHITESPACE, ' ').trim() - const hasActualNewlines = /\r?\n/.test(content) - - if (hasActualNewlines && normalizedContent.length > 80) { - const lines = content.split(/\r?\n/).map(line => line.trim()).filter(line => line) - - if (lines.length > 1) { - return true - } - } - - const attributeLine = `class="${normalizedContent}"` - const currentIndent = indentLength - - if (currentIndent + attributeLine.length > this.maxLineLength && normalizedContent.length > 60) { - if (/<%[^%]*%>/.test(normalizedContent)) { - return false - } - - const classes = normalizedContent.split(' ') - const lines = this.breakTokensIntoLines(classes, currentIndent) - return lines.length > 1 - } - - return false - } - - // TOOD: extract to core or reuse function from core - private getAttributeName(attribute: HTMLAttributeNode): string { - return attribute.name ? getCombinedAttributeName(attribute.name) : "" - } - - // TOOD: extract to core or reuse function from core - private getAttributeValue(attribute: HTMLAttributeNode): string { - if (isNode(attribute.value, HTMLAttributeValueNode)) { - return attribute.value.children.map(child => isNode(child, HTMLTextNode) ? child.content : IdentityPrinter.print(child)).join('') - } - - return '' - } - - private hasMultilineAttributes(attributes: HTMLAttributeNode[]): boolean { - return attributes.some(attribute => { - if (isNode(attribute.value, HTMLAttributeValueNode)) { - const content = getCombinedStringFromNodes(attribute.value.children) - - if (/\r?\n/.test(content)) { - const name = attribute.name ? getCombinedAttributeName(attribute.name) : "" - - if (name === "class") { - const normalizedContent = content.replace(ASCII_WHITESPACE, ' ').trim() - - return normalizedContent.length > 80 - } - - const lines = content.split(/\r?\n/) - - if (lines.length > 1) { - return lines.slice(1).some(line => /^[ \t\n\r]+/.test(line)) - } - } - } - - return false - }) - } - - private formatClassAttribute(content: string, name: string, equals: string, open_quote: string, close_quote: string): string { - const normalizedContent = content.replace(ASCII_WHITESPACE, ' ').trim() - const hasActualNewlines = /\r?\n/.test(content) - - if (hasActualNewlines && normalizedContent.length > 80) { - const lines = content.split(/\r?\n/).map(line => line.trim()).filter(line => line) - - if (lines.length > 1) { - return open_quote + this.formatMultilineAttributeValue(lines) + close_quote - } - } - - const currentIndent = this.indentLevel * this.indentWidth - const attributeLine = `${name}${equals}${open_quote}${normalizedContent}${close_quote}` - - if (currentIndent + attributeLine.length > this.maxLineLength && normalizedContent.length > 60) { - if (/<%[^%]*%>/.test(normalizedContent)) { - return open_quote + normalizedContent + close_quote - } - - const classes = normalizedContent.split(' ') - const lines = this.breakTokensIntoLines(classes, currentIndent) - - if (lines.length > 1) { - return open_quote + this.formatMultilineAttributeValue(lines) + close_quote - } - } - - return open_quote + normalizedContent + close_quote - } - - private isFormattableAttribute(attributeName: string, tagName: string): boolean { - const globalFormattable = FORMATTABLE_ATTRIBUTES['*'] || [] - const tagSpecificFormattable = FORMATTABLE_ATTRIBUTES[tagName.toLowerCase()] || [] - - return globalFormattable.includes(attributeName) || tagSpecificFormattable.includes(attributeName) - } - - private formatMultilineAttribute(content: string, name: string, open_quote: string, close_quote: string): string { - if (name === 'srcset' || name === 'sizes') { - const normalizedContent = content.replace(ASCII_WHITESPACE, ' ').trim() - - return open_quote + normalizedContent + close_quote - } - - const lines = content.split('\n') - - if (lines.length <= 1) { - return open_quote + content + close_quote - } - - const formattedContent = this.formatMultilineAttributeValue(lines) - - return open_quote + formattedContent + close_quote - } - - private formatMultilineAttributeValue(lines: string[]): string { - const indent = " ".repeat((this.indentLevel + 1) * this.indentWidth) - const closeIndent = " ".repeat(this.indentLevel * this.indentWidth) - - return "\n" + lines.map(line => indent + line).join("\n") + "\n" + closeIndent - } - - private breakTokensIntoLines(tokens: string[], currentIndent: number, separator: string = ' '): string[] { - const lines: string[] = [] - let currentLine = '' - - for (const token of tokens) { - const testLine = currentLine ? currentLine + separator + token : token - - if (testLine.length > (this.maxLineLength - currentIndent - 6)) { - if (currentLine) { - lines.push(currentLine) - currentLine = token - } else { - lines.push(token) - } - } else { - currentLine = testLine - } - } - - if (currentLine) lines.push(currentLine) - - return lines - } - /** * Render multiline attributes for a tag */ @@ -758,9 +540,10 @@ export class FormatPrinter extends Printer implements TextFlowDelegate { this.pushWithIndent(openingLine) this.withIndent(() => { + this.attributeRenderer.indentLevel = this.indentLevel allChildren.forEach(child => { if (isNode(child, HTMLAttributeNode)) { - this.pushWithIndent(this.renderAttribute(child)) + this.pushWithIndent(this.attributeRenderer.renderAttribute(child, tagName)) } else if (!isNode(child, WhitespaceNode)) { if (isNode(child, ERBContentNode) && isHerbDisableComment(child)) { return @@ -782,7 +565,7 @@ export class FormatPrinter extends Printer implements TextFlowDelegate { * Reconstruct the text representation of an ERB node * @param withFormatting - if true, format the content; if false, preserve original */ - private reconstructERBNode(node: ERBNode, withFormatting: boolean = true): string { + reconstructERBNode(node: ERBNode, withFormatting: boolean = true): string { const open = node.tag_opening?.value ?? "" const close = node.tag_closing?.value ?? "" const content = node.content?.value ?? "" @@ -1287,13 +1070,14 @@ export class FormatPrinter extends Printer implements TextFlowDelegate { const inline = this.renderInlineOpen(getTagName(node), attributes, isSelfClosing, inlineNodes, node.children) const totalAttributeCount = this.getTotalAttributeCount(attributes, inlineNodes) - const shouldKeepInline = this.shouldRenderInline( + this.attributeRenderer.indentLevel = this.indentLevel + const shouldKeepInline = this.attributeRenderer.shouldRenderInline( totalAttributeCount, inline.length, this.indent.length, this.maxLineLength, false, - this.hasMultilineAttributes(attributes), + this.attributeRenderer.hasMultilineAttributes(attributes), attributes ) @@ -1352,7 +1136,8 @@ export class FormatPrinter extends Printer implements TextFlowDelegate { } visitHTMLAttributeNode(node: HTMLAttributeNode) { - this.pushWithIndent(this.renderAttribute(node)) + this.attributeRenderer.indentLevel = this.indentLevel + this.pushWithIndent(this.attributeRenderer.renderAttribute(node, this.currentTagName)) } visitHTMLAttributeNameNode(node: HTMLAttributeNameNode) { @@ -1553,9 +1338,10 @@ export class FormatPrinter extends Printer implements TextFlowDelegate { node.statements.forEach(child => { if (isNode(child, HTMLAttributeNode)) { this.lines.push(" ") - this.lines.push(this.renderAttribute(child)) + this.attributeRenderer.indentLevel = this.indentLevel + this.lines.push(this.attributeRenderer.renderAttribute(child, this.currentTagName)) } else { - const shouldAddSpaces = this.isInTokenListAttribute + const shouldAddSpaces = this.attributeRenderer.isInTokenListAttribute if (shouldAddSpaces) { this.lines.push(" ") @@ -1570,7 +1356,7 @@ export class FormatPrinter extends Printer implements TextFlowDelegate { }) const hasHTMLAttributes = node.statements.some(child => isNode(child, HTMLAttributeNode)) - const isTokenList = this.isInTokenListAttribute + const isTokenList = this.attributeRenderer.isInTokenListAttribute if ((hasHTMLAttributes || isTokenList) && node.end_node) { this.lines.push(" ") @@ -1712,7 +1498,8 @@ export class FormatPrinter extends Printer implements TextFlowDelegate { if (hasComplexERB) return false const totalAttributeCount = this.getTotalAttributeCount(attributes, inlineNodes) - const hasMultilineAttrs = this.hasMultilineAttributes(attributes) + this.attributeRenderer.indentLevel = this.indentLevel + const hasMultilineAttrs = this.attributeRenderer.hasMultilineAttributes(attributes) if (hasMultilineAttrs) return false @@ -1724,7 +1511,7 @@ export class FormatPrinter extends Printer implements TextFlowDelegate { children ) - return this.shouldRenderInline( + return this.attributeRenderer.shouldRenderInline( totalAttributeCount, inline.length, this.indent.length, @@ -1898,7 +1685,8 @@ export class FormatPrinter extends Printer implements TextFlowDelegate { if (element.is_void || tagClosing?.value === "/>") { const attributes = filterNodes(getOpenTagChildren(element), HTMLAttributeNode) - const attributesString = this.renderAttributesString(attributes) + this.attributeRenderer.indentLevel = this.indentLevel + const attributesString = this.attributeRenderer.renderAttributesString(attributes, tagName) const isSelfClosing = tagClosing?.value === "/>" return `<${tagName}${attributesString}${isSelfClosing ? " />" : ">"}` @@ -1936,7 +1724,8 @@ export class FormatPrinter extends Printer implements TextFlowDelegate { private renderInlineOpen(name: string, attributes: HTMLAttributeNode[], selfClose: boolean, inlineNodes: Node[] = [], allChildren: Node[] = []): string { - const parts = attributes.map(attribute => this.renderAttribute(attribute)) + this.attributeRenderer.indentLevel = this.indentLevel + const parts = attributes.map(attribute => this.attributeRenderer.renderAttribute(attribute, name)) if (inlineNodes.length > 0) { let result = `<${name}` @@ -1945,7 +1734,7 @@ export class FormatPrinter extends Printer implements TextFlowDelegate { const lines = this.capture(() => { allChildren.forEach(child => { if (isNode(child, HTMLAttributeNode)) { - this.lines.push(" " + this.renderAttribute(child)) + this.lines.push(" " + this.attributeRenderer.renderAttribute(child, name)) } else if (!(isNode(child, WhitespaceNode))) { const wasInlineMode = this.inlineMode @@ -1989,70 +1778,14 @@ export class FormatPrinter extends Printer implements TextFlowDelegate { return `<${name}${parts.length ? " " + parts.join(" ") : ""}${selfClose ? " />" : ">"}` } - renderAttribute(attribute: HTMLAttributeNode): string { - const name = attribute.name ? getCombinedAttributeName(attribute.name) : "" - const equals = attribute.equals?.value ?? "" - - this.currentAttributeName = name - - let value = "" - - if (isNode(attribute.value, HTMLAttributeValueNode)) { - const attributeValue = attribute.value - - let open_quote = attributeValue.open_quote?.value ?? "" - let close_quote = attributeValue.close_quote?.value ?? "" - let htmlTextContent = "" - - const content = attributeValue.children.map((child: Node) => { - if (isNode(child, HTMLTextNode) || isNode(child, LiteralNode)) { - htmlTextContent += child.content - - return child.content - } else if (isNode(child, ERBContentNode)) { - return this.reconstructERBNode(child, true) - } else { - const printed = IdentityPrinter.print(child) - - if (this.isInTokenListAttribute) { - return printed.replace(/%>([^<\s])/g, '%> $1').replace(/([^>\s])<%/g, '$1 <%') - } - - return printed - } - }).join("") - - if (open_quote === "" && close_quote === "") { - open_quote = '"' - close_quote = '"' - } else if (open_quote === "'" && close_quote === "'" && !htmlTextContent.includes('"')) { - open_quote = '"' - close_quote = '"' - } - - if (this.isFormattableAttribute(name, this.currentTagName)) { - if (name === 'class') { - value = this.formatClassAttribute(content, name, equals, open_quote, close_quote) - } else { - value = this.formatMultilineAttribute(content, name, open_quote, close_quote) - } - } else { - value = open_quote + content + close_quote - } - } - - this.currentAttributeName = null - - return name + equals + value - } - /** * Try to render a complete element inline including opening tag, children, and closing tag */ private tryRenderInlineFull(_node: HTMLElementNode, tagName: string, attributes: HTMLAttributeNode[], children: Node[]): string | null { let result = `<${tagName}` - result += this.renderAttributesString(attributes) + this.attributeRenderer.indentLevel = this.indentLevel + result += this.attributeRenderer.renderAttributesString(attributes, tagName) result += ">" const childrenContent = this.tryRenderChildrenInline(children, tagName) @@ -2218,7 +1951,8 @@ export class FormatPrinter extends Printer implements TextFlowDelegate { } else if (isNode(child, HTMLElementNode)) { const tagName = getTagName(child) const attributes = filterNodes(getOpenTagChildren(child), HTMLAttributeNode) - const attributesString = this.renderAttributesString(attributes) + this.attributeRenderer.indentLevel = this.indentLevel + const attributesString = this.attributeRenderer.renderAttributesString(attributes, tagName) const childContent = this.renderElementInline(child) content += `<${tagName}${attributesString}>${childContent}` diff --git a/javascript/packages/formatter/src/text-flow-engine.ts b/javascript/packages/formatter/src/text-flow-engine.ts index 0ea76e98f..88dc38252 100644 --- a/javascript/packages/formatter/src/text-flow-engine.ts +++ b/javascript/packages/formatter/src/text-flow-engine.ts @@ -4,6 +4,7 @@ import { Node, HTMLTextNode, HTMLElementNode, ERBContentNode, WhitespaceNode } f import type { ContentUnitWithNode } from "./format-helpers.js" import { + ASCII_WHITESPACE, buildLineWithWord, countAdjacentInlineElements, isClosingPunctuation, @@ -23,12 +24,6 @@ import { import { TextFlowAnalyzer } from "./text-flow-analyzer.js" import type { TextFlowAnalyzerDelegate } from "./text-flow-analyzer.js" -/** - * ASCII whitespace pattern - use instead of \s to preserve Unicode whitespace - * characters like NBSP (U+00A0) and full-width space (U+3000) - */ -const ASCII_WHITESPACE = /[ \t\n\r]+/g - /** * Interface that the FormatPrinter implements to provide * rendering capabilities to the TextFlowEngine. diff --git a/javascript/packages/formatter/test/attribute-renderer.test.ts b/javascript/packages/formatter/test/attribute-renderer.test.ts new file mode 100644 index 000000000..198dfa5b2 --- /dev/null +++ b/javascript/packages/formatter/test/attribute-renderer.test.ts @@ -0,0 +1,421 @@ +import { describe, test, expect, beforeAll } from "vitest" +import { Herb } from "@herb-tools/node-wasm" +import { isNode, getTagName, filterNodes, isHTMLOpenTagNode } from "@herb-tools/core" +import { HTMLElementNode, HTMLAttributeNode, HTMLOpenTagNode } from "@herb-tools/core" + +import { AttributeRenderer } from "../src/attribute-renderer.js" +import type { AttributeRendererDelegate } from "../src/attribute-renderer.js" + +import type { ERBNode } from "@herb-tools/core" + +function createMockDelegate(): AttributeRendererDelegate { + return { + reconstructERBNode(node: ERBNode, _withFormatting: boolean): string { + const open = node.tag_opening?.value ?? "" + const close = node.tag_closing?.value ?? "" + const content = node.content?.value ?? "" + + return `${open} ${content.trim()} ${close}` + }, + } +} + +function createRenderer(maxLineLength: number = 120, indentWidth: number = 2): AttributeRenderer { + return new AttributeRenderer(createMockDelegate(), maxLineLength, indentWidth) +} + +function parseAttributes(source: string): { attributes: HTMLAttributeNode[]; tagName: string } { + const result = Herb.parse(source) + const element = result.value.children[0] + + if (isNode(element, HTMLElementNode) && isHTMLOpenTagNode(element.open_tag)) { + const openTag = element.open_tag as HTMLOpenTagNode + const attributes = filterNodes(openTag.children, HTMLAttributeNode) + const tagName = getTagName(element) + + return { attributes, tagName } + } + + return { attributes: [], tagName: "" } +} + +function parseFirstAttribute(source: string): { attribute: HTMLAttributeNode; tagName: string } { + const { attributes, tagName } = parseAttributes(source) + + return { attribute: attributes[0], tagName } +} + +describe("AttributeRenderer", () => { + beforeAll(async () => { + await Herb.load() + }) + + describe("renderAttribute", () => { + test("renders a boolean attribute", () => { + const renderer = createRenderer() + const { attribute, tagName } = parseFirstAttribute('') + + expect(renderer.renderAttribute(attribute, tagName)).toBe("disabled") + }) + + test("renders an attribute with value", () => { + const renderer = createRenderer() + const { attribute, tagName } = parseFirstAttribute('') + + expect(renderer.renderAttribute(attribute, tagName)).toBe('type="text"') + }) + + test("normalizes single quotes to double quotes", () => { + const renderer = createRenderer() + const { attribute, tagName } = parseFirstAttribute("") + + expect(renderer.renderAttribute(attribute, tagName)).toBe('type="text"') + }) + + test("preserves single quotes when value contains double quotes", () => { + const renderer = createRenderer() + const { attribute, tagName } = parseFirstAttribute(`
`) + + expect(renderer.renderAttribute(attribute, tagName)).toBe(`data-value='"hello"'`) + }) + + test("adds quotes to unquoted attributes", () => { + const renderer = createRenderer() + const { attribute, tagName } = parseFirstAttribute('') + + expect(renderer.renderAttribute(attribute, tagName)).toBe('type="text"') + }) + + test("renders class attribute with formatting", () => { + const renderer = createRenderer() + const { attribute, tagName } = parseFirstAttribute('
') + + expect(renderer.renderAttribute(attribute, tagName)).toBe('class="foo bar baz"') + }) + + test("renders ERB in attribute values", () => { + const renderer = createRenderer() + const { attribute, tagName } = parseFirstAttribute('
') + + expect(renderer.renderAttribute(attribute, tagName)).toBe('id="<%= @id %>"') + }) + + test("sets and clears currentAttributeName during render", () => { + const renderer = createRenderer() + const { attribute, tagName } = parseFirstAttribute('
') + + expect(renderer.currentAttributeName).toBeNull() + renderer.renderAttribute(attribute, tagName) + expect(renderer.currentAttributeName).toBeNull() + }) + }) + + describe("renderAttributesString", () => { + test("returns empty string for no attributes", () => { + const renderer = createRenderer() + + expect(renderer.renderAttributesString([], "div")).toBe("") + }) + + test("renders single attribute with leading space", () => { + const renderer = createRenderer() + const { attributes, tagName } = parseAttributes('
') + + expect(renderer.renderAttributesString(attributes, tagName)).toBe(' class="foo"') + }) + + test("renders multiple attributes space-separated", () => { + const renderer = createRenderer() + const { attributes, tagName } = parseAttributes('') + const result = renderer.renderAttributesString(attributes, tagName) + + expect(result).toBe(' type="text" name="field"') + }) + }) + + describe("formatClassAttribute", () => { + test("returns single-line for short classes", () => { + const renderer = createRenderer() + const result = renderer.formatClassAttribute("foo bar baz", "class", "=", '"', '"') + + expect(result).toBe('"foo bar baz"') + }) + + test("wraps long class lists into multiple lines", () => { + const renderer = createRenderer(80) + renderer.indentLevel = 1 + + const longClasses = "container mx-auto px-4 py-8 flex items-center justify-between bg-gray-100 text-gray-800 font-medium" + const result = renderer.formatClassAttribute(longClasses, "class", "=", '"', '"') + + expect(result).toBe('"\n container mx-auto px-4 py-8 flex items-center justify-between\n bg-gray-100 text-gray-800 font-medium\n "') + }) + + test("does not wrap class with ERB content", () => { + const renderer = createRenderer(80) + renderer.indentLevel = 1 + + const classWithERB = "container mx-auto px-4 py-8 flex items-center justify-between <%= @dynamic_class %> text-gray-800" + const result = renderer.formatClassAttribute(classWithERB, "class", "=", '"', '"') + + expect(result).toBe('"container mx-auto px-4 py-8 flex items-center justify-between <%= @dynamic_class %> text-gray-800"') + }) + }) + + describe("formatMultilineAttribute", () => { + test("normalizes srcset content", () => { + const renderer = createRenderer() + const content = "image-1x.png 1x,\n image-2x.png 2x" + const result = renderer.formatMultilineAttribute(content, "srcset", '"', '"') + + expect(result).toBe('"image-1x.png 1x, image-2x.png 2x"') + }) + + test("normalizes sizes content", () => { + const renderer = createRenderer() + const content = "(max-width: 600px) 480px,\n 800px" + const result = renderer.formatMultilineAttribute(content, "sizes", '"', '"') + + expect(result).toBe('"(max-width: 600px) 480px, 800px"') + }) + + test("preserves single-line values", () => { + const renderer = createRenderer() + const result = renderer.formatMultilineAttribute("some value", "data-value", '"', '"') + + expect(result).toBe('"some value"') + }) + + test("formats multiline values", () => { + const renderer = createRenderer() + renderer.indentLevel = 1 + + const result = renderer.formatMultilineAttribute("line1\nline2", "data-value", '"', '"') + + expect(result).toBe('"\n line1\n line2\n "') + }) + }) + + describe("breakTokensIntoLines", () => { + test("keeps tokens on one line if they fit", () => { + const renderer = createRenderer(120) + const tokens = ["foo", "bar", "baz"] + const result = renderer.breakTokensIntoLines(tokens, 4) + + expect(result).toEqual(["foo bar baz"]) + }) + + test("wraps tokens that exceed the line length", () => { + const renderer = createRenderer(30) + const tokens = ["container", "mx-auto", "px-4", "py-8", "flex", "items-center"] + const result = renderer.breakTokensIntoLines(tokens, 4) + + expect(result).toEqual(["container mx-auto", "px-4 py-8 flex", "items-center"]) + }) + + test("uses custom separator", () => { + const renderer = createRenderer(120) + const tokens = ["a", "b", "c"] + const result = renderer.breakTokensIntoLines(tokens, 0, ", ") + + expect(result).toEqual(["a, b, c"]) + }) + }) + + describe("hasMultilineAttributes", () => { + test("detects multiline attribute values with indentation", () => { + const renderer = createRenderer() + const { attributes } = parseAttributes('') + + expect(renderer.hasMultilineAttributes(attributes)).toBe(true) + }) + + test("returns false for single-line attributes", () => { + const renderer = createRenderer() + const { attributes } = parseAttributes('
') + + expect(renderer.hasMultilineAttributes(attributes)).toBe(false) + }) + }) + + describe("shouldRenderInline", () => { + test("returns true for 0 attributes within line length", () => { + const renderer = createRenderer(120) + + expect(renderer.shouldRenderInline(0, 20, 4)).toBe(true) + }) + + test("returns false for 0 attributes exceeding line length", () => { + const renderer = createRenderer(40) + + expect(renderer.shouldRenderInline(0, 40, 4)).toBe(false) + }) + + test("returns true for 1-3 attributes within line length", () => { + const renderer = createRenderer(120) + + expect(renderer.shouldRenderInline(2, 40, 4)).toBe(true) + }) + + test("returns false for more than 3 attributes", () => { + const renderer = createRenderer(120) + + expect(renderer.shouldRenderInline(4, 40, 4)).toBe(false) + }) + + test("returns false when exceeding line length", () => { + const renderer = createRenderer(40) + + expect(renderer.shouldRenderInline(2, 40, 4)).toBe(false) + }) + + test("returns false with complex ERB", () => { + const renderer = createRenderer(120) + + expect(renderer.shouldRenderInline(1, 20, 4, 120, true)).toBe(false) + }) + + test("returns false with multiline attributes", () => { + const renderer = createRenderer(120) + + expect(renderer.shouldRenderInline(1, 20, 4, 120, false, true)).toBe(false) + }) + }) + + describe("getAttributeName", () => { + test("extracts simple attribute name", () => { + const renderer = createRenderer() + const { attribute } = parseFirstAttribute('
') + + expect(renderer.getAttributeName(attribute)).toBe("class") + }) + + test("extracts data attribute name", () => { + const renderer = createRenderer() + const { attribute } = parseFirstAttribute('
') + + expect(renderer.getAttributeName(attribute)).toBe("data-controller") + }) + }) + + describe("getAttributeValue", () => { + test("extracts attribute value", () => { + const renderer = createRenderer() + const { attribute } = parseFirstAttribute('
') + + expect(renderer.getAttributeValue(attribute)).toBe("foo bar") + }) + + test("returns empty string for boolean attributes", () => { + const renderer = createRenderer() + const { attribute } = parseFirstAttribute('') + + expect(renderer.getAttributeValue(attribute)).toBe("") + }) + }) + + describe("isFormattableAttribute", () => { + test("class is formattable for any tag", () => { + const renderer = createRenderer() + + expect(renderer.isFormattableAttribute("class", "div")).toBe(true) + expect(renderer.isFormattableAttribute("class", "span")).toBe(true) + }) + + test("srcset is formattable for img", () => { + const renderer = createRenderer() + + expect(renderer.isFormattableAttribute("srcset", "img")).toBe(true) + }) + + test("sizes is formattable for img", () => { + const renderer = createRenderer() + + expect(renderer.isFormattableAttribute("sizes", "img")).toBe(true) + }) + + test("srcset is not formattable for non-img tags", () => { + const renderer = createRenderer() + + expect(renderer.isFormattableAttribute("srcset", "div")).toBe(false) + }) + + test("non-formattable attribute returns false", () => { + const renderer = createRenderer() + + expect(renderer.isFormattableAttribute("id", "div")).toBe(false) + expect(renderer.isFormattableAttribute("style", "div")).toBe(false) + }) + }) + + describe("isInTokenListAttribute", () => { + test("returns false when no attribute is being rendered", () => { + const renderer = createRenderer() + + expect(renderer.isInTokenListAttribute).toBe(false) + }) + + test("returns true during class attribute rendering", () => { + const renderer = createRenderer() + renderer.currentAttributeName = "class" + + expect(renderer.isInTokenListAttribute).toBe(true) + }) + + test("returns true for data-controller", () => { + const renderer = createRenderer() + renderer.currentAttributeName = "data-controller" + + expect(renderer.isInTokenListAttribute).toBe(true) + }) + + test("returns true for data-action", () => { + const renderer = createRenderer() + renderer.currentAttributeName = "data-action" + + expect(renderer.isInTokenListAttribute).toBe(true) + }) + + test("returns false for non-token-list attributes", () => { + const renderer = createRenderer() + renderer.currentAttributeName = "id" + + expect(renderer.isInTokenListAttribute).toBe(false) + }) + }) + + describe("formatMultilineAttributeValue", () => { + test("formats lines with proper indentation", () => { + const renderer = createRenderer(120, 2) + renderer.indentLevel = 1 + + const result = renderer.formatMultilineAttributeValue(["line1", "line2"]) + + expect(result).toBe("\n line1\n line2\n ") + }) + + test("respects indent level", () => { + const renderer = createRenderer(120, 2) + renderer.indentLevel = 2 + + const result = renderer.formatMultilineAttributeValue(["line1"]) + + expect(result).toBe("\n line1\n ") + }) + }) + + describe("wouldClassAttributeBeMultiline", () => { + test("returns false for short content", () => { + const renderer = createRenderer(120) + + expect(renderer.wouldClassAttributeBeMultiline("foo bar", 4)).toBe(false) + }) + + test("returns true for long content that would wrap", () => { + const renderer = createRenderer(50) + const longContent = "container mx-auto px-4 py-8 flex items-center justify-between bg-gray-100 text-gray-800" + + expect(renderer.wouldClassAttributeBeMultiline(longContent, 4)).toBe(true) + }) + }) +})