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
166 changes: 162 additions & 4 deletions javascript/packages/formatter/src/format-printer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1792,7 +1792,7 @@ export class FormatPrinter extends Printer {
}
}

const match = trimmedText.match(/^[.!?:;]+/)
const match = trimmedText.match(/^[.!?:;%]+/)

if (!match) {
return {
Expand Down Expand Up @@ -1899,7 +1899,7 @@ export class FormatPrinter extends Printer {
if (isNode(child, HTMLTextNode)) {
const trimmed = child.content.trim()

if (trimmed && /^[.!?:;]/.test(trimmed)) {
if (trimmed && /^[.!?:;%]/.test(trimmed)) {
const wrapWidth = this.maxLineLength - this.indent.length
const result = this.tryMergePunctuationText(inlineContent, trimmed, wrapWidth)

Expand Down Expand Up @@ -1963,22 +1963,180 @@ export class FormatPrinter extends Printer {
}).join("")
}

/**
* Count adjacent inline elements starting from a specific index
* Skips indices that are already processed
*/
private countAdjacentInlineElementsFromIndex(
children: Node[],
startIndex: number,
processedIndices: Set<number>
): number {
let count = 0
let lastSignificantIndex = -1

for (let i = startIndex; i < children.length; i++) {
const child = children[i]

if (isPureWhitespaceNode(child) || isNode(child, WhitespaceNode)) {
continue
}

if (processedIndices.has(i)) {
break
}

const isInlineOrERB =
(isNode(child, HTMLElementNode) && isInlineElement(getTagName(child))) ||
isNode(child, ERBContentNode)

if (!isInlineOrERB) {
break
}

if (lastSignificantIndex >= 0 && hasWhitespaceBetween(children, lastSignificantIndex, i)) {
break
}

count++
lastSignificantIndex = i
}

return count
}

/**
* Render adjacent inline elements starting from a specific index
* Similar to renderAdjacentInlineElements but works from an arbitrary start point
*/
private renderAdjacentInlineElementsFromIndex(
children: Node[],
startIndex: number,
count: number,
alreadyProcessed: Set<number>
): { processedIndices: Set<number>; lastIndex: number } {
let inlineContent = ""
let processedCount = 0
let lastProcessedIndex = -1
const processedIndices = new Set<number>()

for (let index = startIndex; index < children.length && processedCount < count; index++) {
const child = children[index]

if (isPureWhitespaceNode(child) || isNode(child, WhitespaceNode)) {
continue
}

if (alreadyProcessed.has(index)) {
continue
}

if (isNode(child, HTMLElementNode) && isInlineElement(getTagName(child))) {
inlineContent += this.renderInlineElementAsString(child)
processedCount++
lastProcessedIndex = index
processedIndices.add(index)

if (inlineContent && isLineBreakingElement(child)) {
this.pushWithIndent(inlineContent)
inlineContent = ""
}
} else if (isNode(child, ERBContentNode)) {
inlineContent += this.renderERBAsString(child)
processedCount++
lastProcessedIndex = index
processedIndices.add(index)
}
}

if (lastProcessedIndex >= 0) {
for (let index = lastProcessedIndex + 1; index < children.length; index++) {
const child = children[index]

if (isPureWhitespaceNode(child) || isNode(child, WhitespaceNode)) {
continue
}

if (alreadyProcessed.has(index)) {
break
}

if (isNode(child, ERBContentNode)) {
inlineContent += this.renderERBAsString(child)
processedIndices.add(index)
lastProcessedIndex = index
continue
}

if (isNode(child, HTMLTextNode)) {
const trimmed = child.content.trim()

if (trimmed && /^[.!?:;%]/.test(trimmed)) {
const wrapWidth = this.maxLineLength - this.indent.length
const result = this.tryMergePunctuationText(inlineContent, trimmed, wrapWidth)

inlineContent = result.mergedContent
processedIndices.add(index)
lastProcessedIndex = index

if (result.shouldStop) {
if (inlineContent) {
this.pushWithIndent(inlineContent)
}

result.wrappedLines.forEach(line => this.push(line))

return { processedIndices, lastIndex: lastProcessedIndex }
}
}
}

break
}
}

if (inlineContent) {
this.pushWithIndent(inlineContent)
}

return {
processedIndices,
lastIndex: lastProcessedIndex >= 0 ? lastProcessedIndex : startIndex + count - 1
}
}

/**
* Visit remaining children after processing adjacent inline elements
* Recursively handles groups of adjacent inline elements
*/
private visitRemainingChildren(children: Node[], processedIndices: Set<number>): void {
for (let index = 0; index < children.length; index++) {
let index = 0

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

if (isPureWhitespaceNode(child) || isNode(child, WhitespaceNode)) {
index++
continue
}

if (processedIndices.has(index)) {
index++
continue
}

this.visit(child)
const adjacentCount = this.countAdjacentInlineElementsFromIndex(children, index, processedIndices)

if (adjacentCount >= 2) {
const { processedIndices: newProcessedIndices, lastIndex } =
this.renderAdjacentInlineElementsFromIndex(children, index, adjacentCount, processedIndices)

newProcessedIndices.forEach(i => processedIndices.add(i))
index = lastIndex + 1
} else {
this.visit(child)
index++
}
}
}

Expand Down
24 changes: 24 additions & 0 deletions javascript/packages/formatter/test/erb/erb.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,30 @@ describe("@herb-tools/formatter", () => {
expect(secondFormat).toBe(result)
})

test("issue 978: does not duplicate content with multiple adjacent inline/ERB groups separated by br", () => {
const input = dedent`
<p class="text-reversed">
<strong><%= t("admin.tickets.form.savings") %></strong><%= ticket.ticketable.savings_percentage %>%
<br>
<strong><%= t("admin.tickets.form.price_per_ticket") %></strong><%= format_price(ticket.ticketable.price_per_ticket) %>
</p>
`

const result = formatter.format(input)

expect(result).toBe(dedent`
<p class="text-reversed">
<strong><%= t("admin.tickets.form.savings") %></strong><%= ticket.ticketable.savings_percentage %>%
<br>
<strong><%= t("admin.tickets.form.price_per_ticket") %></strong><%= format_price(ticket.ticketable.price_per_ticket) %>
</p>
`)

// Test idempotency
const secondFormat = formatter.format(result)
expect(secondFormat).toBe(result)
})

test("https://github.com/hanakai-rb/site/blob/8adc128d9d464f3e37615be2aa29d57979904533/app/templates/pages/home.html.erb", () => {
const input = dedent`
<h1>Hanakai</h1>
Expand Down
Loading