Skip to content
Merged
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
146 changes: 146 additions & 0 deletions javascript/packages/formatter/src/format-printer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1099,6 +1099,117 @@ export class FormatPrinter extends Printer {
})
}

/**
* Check if there's a blank line (double newline) in the nodes at the given index
*/
private hasBlankLineBetween(body: Node[], index: number): boolean {
for (let lookbackIndex = index - 1; lookbackIndex >= 0 && lookbackIndex >= index - 2; lookbackIndex--) {
const node = body[lookbackIndex]

if (isNode(node, HTMLTextNode) && node.content.includes('\n\n')) {
return true
}

if (isNode(node, WhitespaceNode)) {
continue
}

break
}

for (let lookaheadIndex = index; lookaheadIndex < body.length && lookaheadIndex <= index + 1; lookaheadIndex++) {
const node = body[lookaheadIndex]

if (isNode(node, HTMLTextNode) && node.content.includes('\n\n')) {
return true
}

if (isNode(node, WhitespaceNode)) {
continue
}

break
}

return false
}

/**
* Check if a node is part of a text flow run (text, ERB, or inline element)
*/
private isTextFlowNode(node: Node): boolean {
if (isNode(node, ERBContentNode)) return true
if (isNode(node, HTMLTextNode) && node.content.trim() !== "") return true
if (isNode(node, HTMLElementNode) && isInlineElement(getTagName(node))) return true

return false
}

/**
* Check if a node is whitespace that can appear within a text flow run
*/
private isTextFlowWhitespace(node: Node): boolean {
if (isNode(node, WhitespaceNode)) return true
if (isNode(node, HTMLTextNode) && node.content.trim() === "" && !node.content.includes('\n\n')) return true

return false
}

/**
* Collect a run of text flow nodes starting at the given index.
* Returns the nodes in the run and the index after the last node.
*/
private collectTextFlowRun(body: Node[], startIndex: number): { nodes: Node[], endIndex: number } | null {
const nodes: Node[] = []
let index = startIndex
let textFlowCount = 0

while (index < body.length) {
const child = body[index]

if (this.isTextFlowNode(child)) {
nodes.push(child)
textFlowCount++
index++
} else if (this.isTextFlowWhitespace(child)) {
let hasMoreTextFlow = false

for (let lookaheadIndex = index + 1; lookaheadIndex < body.length; lookaheadIndex++) {
if (this.isTextFlowNode(body[lookaheadIndex])) {
hasMoreTextFlow = true
break
}

if (this.isTextFlowWhitespace(body[lookaheadIndex])) {
continue
}

break
}

if (hasMoreTextFlow) {
nodes.push(child)
index++
} else {
break
}
} else {
break
}
}

if (textFlowCount >= 2) {
const hasText = nodes.some(node => isNode(node, HTMLTextNode) && node.content.trim() !== "")
const hasAtomicContent = nodes.some(node => isNode(node, ERBContentNode) || (isNode(node, HTMLElementNode) && isInlineElement(getTagName(node))))

if (hasText && hasAtomicContent) {
return { nodes, endIndex: index }
}
}

return null
}

/**
* Visit element children with intelligent spacing logic
*
Expand Down Expand Up @@ -1130,6 +1241,41 @@ export class FormatPrinter extends Printer {

if (!isNonWhitespaceNode(child)) continue

if (this.isTextFlowNode(child)) {
const run = this.collectTextFlowRun(body, index)

if (run) {
if (lastMeaningfulNode && !hasHandledSpacing) {
const hasBlankLineBefore = this.hasBlankLineBetween(body, index)

if (hasBlankLineBefore) {
this.push("")
}
}

this.visitTextFlowChildren(run.nodes)

const lastRunNode = run.nodes[run.nodes.length - 1]
const hasBlankLineInTrailing = isNode(lastRunNode, HTMLTextNode) && lastRunNode.content.includes('\n\n')
const hasBlankLineAfter = hasBlankLineInTrailing || this.hasBlankLineBetween(body, run.endIndex)

if (hasBlankLineAfter) {
this.push("")
hasHandledSpacing = true
}

lastMeaningfulNode = run.nodes[run.nodes.length - 1]

if (!hasBlankLineAfter) {
hasHandledSpacing = false
}

index = run.endIndex - 1

continue
}
}

let hasTrailingHerbDisable = false

if (isNode(child, HTMLElementNode) && child.close_tag) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,43 @@ describe("ERB whitespace formatting", () => {
})
})

describe("issue 931: punctuation after ERB link_to tags", () => {
test("issue 931: keeps commas and periods attached to ERB link_to tags (@adrianthedev)", () => {
const formatter120 = new Formatter(Herb, { indentWidth: 2, maxLineLength: 120 })

const source = dedent`
<div class="py-6 min-h-24">
<div class="px-6 space-y-4">
<h3>What a nice new tool</h3>

Go to <%= link_to "new comment", app.new_resources_comment_path %>, <%= link_to 'the first user', app.resources_user_path(1) %>, or <%= link_to 'hey on main app', main_app.hey_path %>.

<p>
You can edit this file here <code class="p-1 rounded-sm bg-gray-500 text-white text-sm">app/views/app/tools/custom_tool.html.erb</code>.
</p>
</div>
</div>
`
const result = formatter120.format(source)

expect(result).toBe(dedent`
<div class="py-6 min-h-24">
<div class="px-6 space-y-4">
<h3>What a nice new tool</h3>

Go to <%= link_to "new comment", app.new_resources_comment_path %>,
<%= link_to 'the first user', app.resources_user_path(1) %>, or <%= link_to 'hey on main app', main_app.hey_path %>.

<p>
You can edit this file here
<code class="p-1 rounded-sm bg-gray-500 text-white text-sm">app/views/app/tools/custom_tool.html.erb</code>.
</p>
</div>
</div>
`)
})
})

describe("shared utility validation", () => {
test("demonstrates consistent ERB content formatting where it applies", () => {
const erbContentCases = [
Expand Down
Loading