From 4ec84d72fdf35630ee8fa282799bb022024fcf21 Mon Sep 17 00:00:00 2001 From: Andy Waite Date: Sun, 28 Jun 2026 17:12:00 -0400 Subject: [PATCH 1/4] Formatter: Wrap attributes based on `maxLineLength` only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, the formatter wrapped tags one-attribute-per-line under two conditions: when the opening tag would exceed `maxLineLength`, OR when the tag had more than 3 attributes. The count rule was surprising in practice — `
` wrapped at 36 chars, well under the 80-char default. This removes the count rule. Wrapping is now driven purely by `maxLineLength`, which matches user expectation and the docs. Refs #928, #1823 Co-Authored-By: Claude Opus 4.7 (1M context) --- javascript/packages/formatter/README.md | 2 +- .../formatter/src/attribute-renderer.ts | 6 +- .../formatter/test/attribute-renderer.test.ts | 6 +- .../formatter/test/html/attributes.test.ts | 58 ++++++++++--------- 4 files changed, 35 insertions(+), 37 deletions(-) diff --git a/javascript/packages/formatter/README.md b/javascript/packages/formatter/README.md index 7a1c46991..b694130f6 100644 --- a/javascript/packages/formatter/README.md +++ b/javascript/packages/formatter/README.md @@ -226,7 +226,7 @@ The `include` patterns are **additive** - they add to the defaults. - **`enabled`**: `true` or `false` - Must be `true` to enable formatting - **`indentWidth`**: Number (default: `2`) - Spaces per indentation level -- **`maxLineLength`**: Number (default: `80`) - Maximum line length before wrapping. Note: tags with **more than 3 attributes** are always wrapped one-per-line, independent of `maxLineLength`. +- **`maxLineLength`**: Number (default: `80`) - Maximum line length before wrapping. Tags whose attributes would exceed this length are wrapped one-per-line. - **`include`**: Array of glob patterns - Additional patterns to format (additive to defaults) - **`exclude`**: Array of glob patterns - Patterns to exclude from formatting diff --git a/javascript/packages/formatter/src/attribute-renderer.ts b/javascript/packages/formatter/src/attribute-renderer.ts index 358b8074b..3a1d089b1 100644 --- a/javascript/packages/formatter/src/attribute-renderer.ts +++ b/javascript/packages/formatter/src/attribute-renderer.ts @@ -88,11 +88,7 @@ export class AttributeRenderer { } } - if (totalAttributeCount > 3 || inlineLength + indentLength > maxLineLength) { - return false - } - - return true + return inlineLength + indentLength <= maxLineLength } wouldClassAttributeBeMultiline(content: string, indentLength: number): boolean { diff --git a/javascript/packages/formatter/test/attribute-renderer.test.ts b/javascript/packages/formatter/test/attribute-renderer.test.ts index 198dfa5b2..6931283d6 100644 --- a/javascript/packages/formatter/test/attribute-renderer.test.ts +++ b/javascript/packages/formatter/test/attribute-renderer.test.ts @@ -251,16 +251,16 @@ describe("AttributeRenderer", () => { expect(renderer.shouldRenderInline(0, 40, 4)).toBe(false) }) - test("returns true for 1-3 attributes within line length", () => { + test("returns true for multiple attributes within line length", () => { const renderer = createRenderer(120) expect(renderer.shouldRenderInline(2, 40, 4)).toBe(true) }) - test("returns false for more than 3 attributes", () => { + test("returns true for many attributes that still fit within line length", () => { const renderer = createRenderer(120) - expect(renderer.shouldRenderInline(4, 40, 4)).toBe(false) + expect(renderer.shouldRenderInline(4, 40, 4)).toBe(true) }) test("returns false when exceeding line length", () => { diff --git a/javascript/packages/formatter/test/html/attributes.test.ts b/javascript/packages/formatter/test/html/attributes.test.ts index d455bb3ad..16ddec270 100644 --- a/javascript/packages/formatter/test/html/attributes.test.ts +++ b/javascript/packages/formatter/test/html/attributes.test.ts @@ -46,22 +46,33 @@ describe("@herb-tools/formatter", () => { `) }) - test("wraps 4+ attributes correctly", () => { + test("keeps 4+ attributes inline when they fit within maxLineLength", () => { const source = dedent`
` const result = formatter.format(source) + expect(result).toEqual(dedent` +
+ `) + }) + + test("wraps attributes when they would exceed maxLineLength", () => { + const source = dedent` +
+ ` + const result = formatter.format(source) expect(result).toEqual(dedent`
`) }) - test("keeps empty attribute values inline when ≤3 attributes", () => { + test("keeps empty attribute values inline", () => { const source = dedent`
` @@ -71,28 +82,13 @@ describe("@herb-tools/formatter", () => { `) }) - test("keeps multiple empty attributes inline when <= 3 attributes", () => { - const source = dedent` -
- ` - const result = formatter.format(source) - expect(result).toEqual(dedent` -
- `) - }) - - test("splits multiple empty attributes inline when > 3 attributes", () => { + test("keeps multiple empty attributes inline when within maxLineLength", () => { const source = dedent`
` const result = formatter.format(source) expect(result).toEqual(dedent` -
+
`) }) @@ -136,33 +132,39 @@ describe("@herb-tools/formatter", () => { `) }) - test("formats self-closing input with 4+ attributes", () => { + test("keeps self-closing input with 4+ attributes inline when within maxLineLength", () => { const source = dedent` ` const result = formatter.format(source) expect(result).toEqual(dedent` - + `) }) - test("formats input with 4+ attributes without closing slash", () => { + test("keeps input with 4+ attributes without closing slash inline when within maxLineLength", () => { const source = dedent` ` const result = formatter.format(source) + expect(result).toEqual(dedent` + + `) + }) + + test("wraps self-closing input when attributes exceed maxLineLength", () => { + const source = dedent` + + ` + const result = formatter.format(source) expect(result).toEqual(dedent` + /> `) }) From 8abb7374243ccafb4ad6d8ea03a7e0938b7a49cb Mon Sep 17 00:00:00 2001 From: Andy Waite Date: Sun, 28 Jun 2026 17:24:20 -0400 Subject: [PATCH 2/4] Formatter: Update remaining tests that relied on >3 attribute wrap rule Five tests in CI still expected one-per-line wrapping based on the old count heuristic. Under the new length-only behavior these inputs fit within the 80-char default and stay inline; one test's input was widened to genuinely exceed maxLineLength so it still demonstrates wrap behavior. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../formatter/test/document-formatting.test.ts | 11 ++--------- .../erb-formatter/erb-formatter-additional.test.ts | 9 +-------- .../packages/formatter/test/html/attributes.test.ts | 12 ++++++------ .../formatter/test/html/quote-normalization.test.ts | 9 +-------- .../formatter/test/html/script-style-tags.test.ts | 7 +------ 5 files changed, 11 insertions(+), 37 deletions(-) diff --git a/javascript/packages/formatter/test/document-formatting.test.ts b/javascript/packages/formatter/test/document-formatting.test.ts index 173da48a6..63d518227 100644 --- a/javascript/packages/formatter/test/document-formatting.test.ts +++ b/javascript/packages/formatter/test/document-formatting.test.ts @@ -341,21 +341,14 @@ describe("Document-level formatting", () => { `) }) - test("formats regular HTML elements in multiline when attributes > 3", () => { + test("keeps regular HTML elements inline when attributes fit within maxLineLength", () => { const source = dedent`
Content
` const result = formatter.format(source) expect(result).toEqual(dedent` -
- Content -
+
Content
`) }) diff --git a/javascript/packages/formatter/test/erb-formatter/erb-formatter-additional.test.ts b/javascript/packages/formatter/test/erb-formatter/erb-formatter-additional.test.ts index ba5d7232e..4adfaa663 100644 --- a/javascript/packages/formatter/test/erb-formatter/erb-formatter-additional.test.ts +++ b/javascript/packages/formatter/test/erb-formatter/erb-formatter-additional.test.ts @@ -223,14 +223,7 @@ describe("ERB Formatter Additional Tests", () => { const result = formatter.format(source) - expect(result).toBe(dedent` - - `) + expect(result).toBe(``) }) }) }) diff --git a/javascript/packages/formatter/test/html/attributes.test.ts b/javascript/packages/formatter/test/html/attributes.test.ts index 16ddec270..f37a20aef 100644 --- a/javascript/packages/formatter/test/html/attributes.test.ts +++ b/javascript/packages/formatter/test/html/attributes.test.ts @@ -58,16 +58,16 @@ describe("@herb-tools/formatter", () => { test("wraps attributes when they would exceed maxLineLength", () => { const source = dedent` -
+ ` const result = formatter.format(source) expect(result).toEqual(dedent`
`) }) diff --git a/javascript/packages/formatter/test/html/quote-normalization.test.ts b/javascript/packages/formatter/test/html/quote-normalization.test.ts index 7ef1323b4..8cd5924ac 100644 --- a/javascript/packages/formatter/test/html/quote-normalization.test.ts +++ b/javascript/packages/formatter/test/html/quote-normalization.test.ts @@ -171,14 +171,7 @@ describe("Quote normalization", () => { const result = formatter.format(source) - expect(result).toBe(dedent` - - `) + expect(result).toBe(``) }) test("handles data attributes with complex JSON", () => { diff --git a/javascript/packages/formatter/test/html/script-style-tags.test.ts b/javascript/packages/formatter/test/html/script-style-tags.test.ts index 2aeff732a..7083843d9 100644 --- a/javascript/packages/formatter/test/html/script-style-tags.test.ts +++ b/javascript/packages/formatter/test/html/script-style-tags.test.ts @@ -259,12 +259,7 @@ describe("@herb-tools/formatter - script and style tags", () => { ` const result = formatter.format(source) expect(result).toEqual(dedent` - `) From 9bb5961963304d942facfae8f84a24ae78cac75a Mon Sep 17 00:00:00 2001 From: Andy Waite Date: Sun, 28 Jun 2026 18:26:32 -0400 Subject: [PATCH 3/4] Formatter: Fix document-formatting test to reflect inline-tag behavior The 71-char opening tag fits within maxLineLength so it stays inline, but the whole `
...
Content` line is 85 chars, so the formatter still moves the content onto separate lines. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../packages/formatter/test/document-formatting.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/javascript/packages/formatter/test/document-formatting.test.ts b/javascript/packages/formatter/test/document-formatting.test.ts index 63d518227..76086e45d 100644 --- a/javascript/packages/formatter/test/document-formatting.test.ts +++ b/javascript/packages/formatter/test/document-formatting.test.ts @@ -341,14 +341,16 @@ describe("Document-level formatting", () => { `) }) - test("keeps regular HTML elements inline when attributes fit within maxLineLength", () => { + test("keeps opening tag inline when attributes fit within maxLineLength", () => { const source = dedent`
Content
` const result = formatter.format(source) expect(result).toEqual(dedent` -
Content
+
+ Content +
`) }) From bb73d9973805335ac19cee31f9dcc76220227daa Mon Sep 17 00:00:00 2001 From: Andy Waite Date: Sun, 28 Jun 2026 18:37:27 -0400 Subject: [PATCH 4/4] ci: trigger re-run