Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,18 @@ describe("@herb-tools/formatter - content preserving tags", () => {
expect(formatter.format(result)).toEqual(result)
})

test("preserves attribute spacing on an element nested inside an ERB block (#2142)", () => {
const source = `<pre><% if condition %><span class="x">x</span><% end %></pre>`
const result = formatter.format(source)
expect(result).toEqual(`<pre><% if condition %><span class="x">x</span><% end %></pre>`)
})

test("preserves spacing between multiple attributes on an element nested inside an ERB block", () => {
const source = `<pre><% if condition %><span class="x" id="y">x</span><% end %></pre>`
const result = formatter.format(source)
expect(result).toEqual(`<pre><% if condition %><span class="x" id="y">x</span><% end %></pre>`)
})

test("preserves textarea with ERB control flow", () => {
const source = dedent`
<textarea>
Expand Down
39 changes: 37 additions & 2 deletions javascript/packages/printer/src/identity-printer.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Printer } from "./printer.js"
import { getNodesBeforePosition, getNodesAfterPosition } from "@herb-tools/core"
import { getNodesBeforePosition, getNodesAfterPosition, isWhitespaceNode } from "@herb-tools/core"

import type * as Nodes from "@herb-tools/core"

Expand Down Expand Up @@ -44,13 +44,48 @@ export class IdentityPrinter extends Printer {
this.write(node.tag_name.value)
}

this.visitChildNodes(node)
// Without `track_whitespace: true` the parser doesn't emit a node for the
// whitespace that separates the tag name from the first attribute, or the
// whitespace between attributes, so reconstructing children back-to-back
// would merge them together (e.g. `<span class="x">` becoming
// `<spanclass="x">`). Restore a single separating space wherever the
// previous node's end position doesn't line up with the next node's start.
//
// WhitespaceNode children already print their own whitespace (including
// synthetic ones inserted by autofixers/rewriters, whose location doesn't
// line up with the surrounding nodes), so the gap-fill check is skipped
// both for the WhitespaceNode itself and for whatever child follows it -
// otherwise the gap-fill logic would see the following child's position
// doesn't line up with `previousEnd` (which the WhitespaceNode leaves
// untouched) and write a second, duplicate separating space.
let previousEnd = node.tag_name?.location.end ?? node.tag_opening?.location.end
let previousWasWhitespace = false

node.children.forEach(child => {
const childIsWhitespace = isWhitespaceNode(child)

if (previousEnd && !childIsWhitespace && !previousWasWhitespace && !this.samePosition(previousEnd, child.location.start)) {
this.write(" ")
}

this.visit(child)

previousWasWhitespace = childIsWhitespace

if (!childIsWhitespace) {
previousEnd = child.location.end
}
})

if (node.tag_closing) {
this.write(node.tag_closing.value)
}
}

private samePosition(a: Nodes.Position, b: Nodes.Position): boolean {
return a.line === b.line && a.column === b.column
}

visitHTMLCloseTagNode(node: Nodes.HTMLCloseTagNode): void {
if (node.tag_opening) {
this.write(node.tag_opening.value)
Expand Down
51 changes: 49 additions & 2 deletions javascript/packages/printer/test/nodes/html-open-tag-node.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { describe, test, beforeAll } from "vitest"

import { Herb } from "@herb-tools/node-wasm"
import { HTMLOpenTagNode } from "@herb-tools/core"
import { HTMLOpenTagNode, HTMLAttributeNode, HTMLAttributeNameNode, HTMLAttributeValueNode, WhitespaceNode } from "@herb-tools/core"

import { expectNodeToPrint, expectPrintRoundTrip, location, createToken } from "../helpers/printer-test-helpers.js"
import { expectNodeToPrint, expectPrintRoundTrip, location, createLocation, createToken, createLiteralNode } from "../helpers/printer-test-helpers.js"

describe("HTMLOpenTagNode Printing", () => {
beforeAll(async () => {
Expand Down Expand Up @@ -51,4 +51,51 @@ describe("HTMLOpenTagNode Printing", () => {
expectPrintRoundTrip(`<a id="id" >Content</a>`)
expectPrintRoundTrip(`<a id="id" <%= content %> class="class">Content</a>`)
})

test("does not double a space when a synthesized WhitespaceNode child precedes an attribute whose own location doesn't line up with the tag name (e.g. rewriter-inserted attributes)", () => {
// Rewriters (e.g. ActionViewTagHelperToHTMLRewriter) insert a synthetic
// WhitespaceNode ahead of an attribute pulled in from elsewhere in the
// source, so the attribute's own location never lines up with the tag
// name's end position. The gap-fill logic must not add its own space on
// top of the one the WhitespaceNode already prints.
const attributeName = HTMLAttributeNameNode.build({
location: createLocation(1, 20),
children: [
createLiteralNode("class")
]
})

const attributeValue = HTMLAttributeValueNode.build({
location: createLocation(1, 26),
open_quote: createToken("TOKEN_QUOTE", `"`),
close_quote: createToken("TOKEN_QUOTE", `"`),
children: [
createLiteralNode("content")
],
quoted: true
})

const attribute = HTMLAttributeNode.build({
location: createLocation(1, 20),
name: attributeName,
equals: createToken("TOKEN_EQUALS", "="),
value: attributeValue
})

const whitespace = WhitespaceNode.build({
location,
value: createToken("TOKEN_WHITESPACE", " ")
})

const node = HTMLOpenTagNode.build({
location,
tag_opening: createToken("TOKEN_HTML_TAG_START", "<"),
tag_name: createToken("TOKEN_IDENTIFIER", "div"),
tag_closing: createToken("TOKEN_HTML_TAG_END", ">"),
children: [whitespace, attribute],
is_void: false
})

expectNodeToPrint(node, `<div class="content">`)
})
})
Loading