From 6ec1391ebba100adfadde2246648e6e175058523 Mon Sep 17 00:00:00 2001 From: Mario Zechner Date: Fri, 5 Dec 2025 21:49:44 +0100 Subject: [PATCH] fix: escape codes not properly wrapped during line wrapping - ANSI codes now attach to next visible content, not trailing whitespace - Rewrote AnsiCodeTracker to track individual attributes - Line-end resets only turn off underline, preserving background colors - Added vitest config to exclude node:test files fixes #109 --- packages/coding-agent/CHANGELOG.md | 1 + packages/tui/src/utils.ts | 226 ++++++++++++++++++++++++++-- packages/tui/test/wrap-ansi.test.ts | 206 +++++++++++++------------ packages/tui/vitest.config.ts | 7 + 4 files changed, 332 insertions(+), 108 deletions(-) create mode 100644 packages/tui/vitest.config.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index c7af84bd..29c1e21f 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -9,6 +9,7 @@ ### Fixed - **Multi-key sequences in inputs**: Inputs like model search now handle multi-key sequences identically to the main prompt editor. ([#122](https://github.com/badlogic/pi-mono/pull/122) by [@markusylisiurunen](https://github.com/markusylisiurunen)) +- **Line wrapping escape codes**: Fixed underline style bleeding into padding when wrapping long URLs. ANSI codes now attach to the correct content, and line-end resets only turn off underline (preserving background colors). ([#109](https://github.com/badlogic/pi-mono/issues/109)) ### Added diff --git a/packages/tui/src/utils.ts b/packages/tui/src/utils.ts index 718ac99a..a724cf7f 100644 --- a/packages/tui/src/utils.ts +++ b/packages/tui/src/utils.ts @@ -35,27 +35,199 @@ function extractAnsiCode(str: string, pos: number): { code: string; length: numb * Track active ANSI SGR codes to preserve styling across line breaks. */ class AnsiCodeTracker { - private activeAnsiCodes: string[] = []; + // Track individual attributes separately so we can reset them specifically + private bold = false; + private dim = false; + private italic = false; + private underline = false; + private blink = false; + private inverse = false; + private hidden = false; + private strikethrough = false; + private fgColor: string | null = null; // Stores the full code like "31" or "38;5;240" + private bgColor: string | null = null; // Stores the full code like "41" or "48;5;240" process(ansiCode: string): void { if (!ansiCode.endsWith("m")) { return; } - // Full reset clears everything - if (ansiCode === "\x1b[0m" || ansiCode === "\x1b[m") { - this.activeAnsiCodes.length = 0; - } else { - this.activeAnsiCodes.push(ansiCode); + // Extract the parameters between \x1b[ and m + const match = ansiCode.match(/\x1b\[([\d;]*)m/); + if (!match) return; + + const params = match[1]; + if (params === "" || params === "0") { + // Full reset + this.reset(); + return; + } + + // Parse parameters (can be semicolon-separated) + const parts = params.split(";"); + let i = 0; + while (i < parts.length) { + const code = Number.parseInt(parts[i], 10); + + // Handle 256-color and RGB codes which consume multiple parameters + if (code === 38 || code === 48) { + // 38;5;N (256 color fg) or 38;2;R;G;B (RGB fg) + // 48;5;N (256 color bg) or 48;2;R;G;B (RGB bg) + if (parts[i + 1] === "5" && parts[i + 2] !== undefined) { + // 256 color: 38;5;N or 48;5;N + const colorCode = `${parts[i]};${parts[i + 1]};${parts[i + 2]}`; + if (code === 38) { + this.fgColor = colorCode; + } else { + this.bgColor = colorCode; + } + i += 3; + continue; + } else if (parts[i + 1] === "2" && parts[i + 4] !== undefined) { + // RGB color: 38;2;R;G;B or 48;2;R;G;B + const colorCode = `${parts[i]};${parts[i + 1]};${parts[i + 2]};${parts[i + 3]};${parts[i + 4]}`; + if (code === 38) { + this.fgColor = colorCode; + } else { + this.bgColor = colorCode; + } + i += 5; + continue; + } + } + + // Standard SGR codes + switch (code) { + case 0: + this.reset(); + break; + case 1: + this.bold = true; + break; + case 2: + this.dim = true; + break; + case 3: + this.italic = true; + break; + case 4: + this.underline = true; + break; + case 5: + this.blink = true; + break; + case 7: + this.inverse = true; + break; + case 8: + this.hidden = true; + break; + case 9: + this.strikethrough = true; + break; + case 21: + this.bold = false; + break; // Some terminals + case 22: + this.bold = false; + this.dim = false; + break; + case 23: + this.italic = false; + break; + case 24: + this.underline = false; + break; + case 25: + this.blink = false; + break; + case 27: + this.inverse = false; + break; + case 28: + this.hidden = false; + break; + case 29: + this.strikethrough = false; + break; + case 39: + this.fgColor = null; + break; // Default fg + case 49: + this.bgColor = null; + break; // Default bg + default: + // Standard foreground colors 30-37, 90-97 + if ((code >= 30 && code <= 37) || (code >= 90 && code <= 97)) { + this.fgColor = String(code); + } + // Standard background colors 40-47, 100-107 + else if ((code >= 40 && code <= 47) || (code >= 100 && code <= 107)) { + this.bgColor = String(code); + } + break; + } + i++; } } + private reset(): void { + this.bold = false; + this.dim = false; + this.italic = false; + this.underline = false; + this.blink = false; + this.inverse = false; + this.hidden = false; + this.strikethrough = false; + this.fgColor = null; + this.bgColor = null; + } + getActiveCodes(): string { - return this.activeAnsiCodes.join(""); + const codes: string[] = []; + if (this.bold) codes.push("1"); + if (this.dim) codes.push("2"); + if (this.italic) codes.push("3"); + if (this.underline) codes.push("4"); + if (this.blink) codes.push("5"); + if (this.inverse) codes.push("7"); + if (this.hidden) codes.push("8"); + if (this.strikethrough) codes.push("9"); + if (this.fgColor) codes.push(this.fgColor); + if (this.bgColor) codes.push(this.bgColor); + + if (codes.length === 0) return ""; + return `\x1b[${codes.join(";")}m`; } hasActiveCodes(): boolean { - return this.activeAnsiCodes.length > 0; + return ( + this.bold || + this.dim || + this.italic || + this.underline || + this.blink || + this.inverse || + this.hidden || + this.strikethrough || + this.fgColor !== null || + this.bgColor !== null + ); + } + + /** + * Get reset codes for attributes that need to be turned off at line end, + * specifically underline which bleeds into padding. + * Returns empty string if no problematic attributes are active. + */ + getLineEndReset(): string { + // Only underline causes visual bleeding into padding + // Other attributes like colors don't visually bleed to padding + if (this.underline) { + return "\x1b[24m"; // Underline off only + } + return ""; } } @@ -78,13 +250,15 @@ function updateTrackerFromText(text: string, tracker: AnsiCodeTracker): void { function splitIntoTokensWithAnsi(text: string): string[] { const tokens: string[] = []; let current = ""; + let pendingAnsi = ""; // ANSI codes waiting to be attached to next visible content let inWhitespace = false; let i = 0; while (i < text.length) { const ansiResult = extractAnsiCode(text, i); if (ansiResult) { - current += ansiResult.code; + // Hold ANSI codes separately - they'll be attached to the next visible char + pendingAnsi += ansiResult.code; i += ansiResult.length; continue; } @@ -98,11 +272,22 @@ function splitIntoTokensWithAnsi(text: string): string[] { current = ""; } + // Attach any pending ANSI codes to this visible character + if (pendingAnsi) { + current += pendingAnsi; + pendingAnsi = ""; + } + inWhitespace = charIsSpace; current += char; i++; } + // Handle any remaining pending ANSI codes (attach to last token) + if (pendingAnsi) { + current += pendingAnsi; + } + if (current) { tokens.push(current); } @@ -161,12 +346,17 @@ function wrapSingleLine(line: string, width: number): string[] { // Token itself is too long - break it character by character if (tokenVisibleLength > width && !isWhitespace) { if (currentLine) { + // Add specific reset for underline only (preserves background) + const lineEndReset = tracker.getLineEndReset(); + if (lineEndReset) { + currentLine += lineEndReset; + } wrapped.push(currentLine); currentLine = ""; currentVisibleLength = 0; } - // Break long token + // Break long token - breakLongWord handles its own resets const broken = breakLongWord(token, width, tracker); wrapped.push(...broken.slice(0, -1)); currentLine = broken[broken.length - 1]; @@ -178,8 +368,13 @@ function wrapSingleLine(line: string, width: number): string[] { const totalNeeded = currentVisibleLength + tokenVisibleLength; if (totalNeeded > width && currentVisibleLength > 0) { - // Wrap to next line - don't carry trailing whitespace - wrapped.push(currentLine.trimEnd()); + // Add specific reset for underline only (preserves background) + let lineToWrap = currentLine.trimEnd(); + const lineEndReset = tracker.getLineEndReset(); + if (lineEndReset) { + lineToWrap += lineEndReset; + } + wrapped.push(lineToWrap); if (isWhitespace) { // Don't start new line with whitespace currentLine = tracker.getActiveCodes(); @@ -198,6 +393,7 @@ function wrapSingleLine(line: string, width: number): string[] { } if (currentLine) { + // No reset at end of final line - let caller handle it wrapped.push(currentLine); } @@ -251,6 +447,11 @@ function breakLongWord(word: string, width: number, tracker: AnsiCodeTracker): s const graphemeWidth = visibleWidth(grapheme); if (currentWidth + graphemeWidth > width) { + // Add specific reset for underline only (preserves background) + const lineEndReset = tracker.getLineEndReset(); + if (lineEndReset) { + currentLine += lineEndReset; + } lines.push(currentLine); currentLine = tracker.getActiveCodes(); currentWidth = 0; @@ -261,6 +462,7 @@ function breakLongWord(word: string, width: number, tracker: AnsiCodeTracker): s } if (currentLine) { + // No reset at end of final segment - caller handles continuation lines.push(currentLine); } diff --git a/packages/tui/test/wrap-ansi.test.ts b/packages/tui/test/wrap-ansi.test.ts index 7e5af229..03b933ae 100644 --- a/packages/tui/test/wrap-ansi.test.ts +++ b/packages/tui/test/wrap-ansi.test.ts @@ -1,112 +1,126 @@ -import assert from "node:assert"; -import { describe, it } from "node:test"; -import { Chalk } from "chalk"; - -// We'll implement these -import { applyBackgroundToLine, visibleWidth, wrapTextWithAnsi } from "../src/utils.js"; - -const chalk = new Chalk({ level: 3 }); +import { describe, expect, it } from "vitest"; +import { visibleWidth, wrapTextWithAnsi } from "../src/utils.js"; describe("wrapTextWithAnsi", () => { - it("wraps plain text at word boundaries", () => { - const text = "hello world this is a test"; - const lines = wrapTextWithAnsi(text, 15); + describe("underline styling", () => { + it("should not apply underline style before the styled text", () => { + const underlineOn = "\x1b[4m"; + const underlineOff = "\x1b[24m"; + const url = "https://example.com/very/long/path/that/will/wrap"; + const text = `read this thread ${underlineOn}${url}${underlineOff}`; - assert.strictEqual(lines.length, 2); - assert.strictEqual(lines[0], "hello world"); - assert.strictEqual(lines[1], "this is a test"); + const wrapped = wrapTextWithAnsi(text, 40); + + // First line should NOT contain underline code - it's just "read this thread " + expect(wrapped[0]).toBe("read this thread "); + + // Second line should start with underline, have URL content + expect(wrapped[1].startsWith(underlineOn)).toBe(true); + expect(wrapped[1]).toContain("https://"); + }); + + it("should not bleed underline to padding - each line should end with reset for underline only", () => { + const underlineOn = "\x1b[4m"; + const underlineOff = "\x1b[24m"; + const url = "https://example.com/very/long/path/that/will/definitely/wrap"; + const text = `prefix ${underlineOn}${url}${underlineOff} suffix`; + + const wrapped = wrapTextWithAnsi(text, 30); + + // Middle lines (with underlined content) should end with underline-off, not full reset + // Line 1 and 2 contain underlined URL parts + for (let i = 1; i < wrapped.length - 1; i++) { + const line = wrapped[i]; + if (line.includes(underlineOn)) { + // Should end with underline off, NOT full reset + expect(line.endsWith(underlineOff)).toBe(true); + expect(line.endsWith("\x1b[0m")).toBe(false); + } + } + }); }); - it("preserves ANSI codes across wrapped lines", () => { - const text = chalk.bold("hello world this is bold text"); - const lines = wrapTextWithAnsi(text, 20); + describe("background color preservation", () => { + it("should preserve background color across wrapped lines without full reset", () => { + const bgBlue = "\x1b[44m"; + const reset = "\x1b[0m"; + const text = `${bgBlue}hello world this is blue background text${reset}`; - // Should have bold code at start of each line - assert.ok(lines[0].includes("\x1b[1m")); - assert.ok(lines[1].includes("\x1b[1m")); + const wrapped = wrapTextWithAnsi(text, 15); - // Each line should be <= 20 visible chars - assert.ok(visibleWidth(lines[0]) <= 20); - assert.ok(visibleWidth(lines[1]) <= 20); + // Each line should have background color + for (const line of wrapped) { + expect(line.includes(bgBlue)).toBe(true); + } + + // Middle lines should NOT end with full reset (kills background for padding) + for (let i = 0; i < wrapped.length - 1; i++) { + expect(wrapped[i].endsWith("\x1b[0m")).toBe(false); + } + }); + + it("should reset underline but preserve background when wrapping underlined text inside background", () => { + const underlineOn = "\x1b[4m"; + const underlineOff = "\x1b[24m"; + const reset = "\x1b[0m"; + + const text = `\x1b[41mprefix ${underlineOn}UNDERLINED_CONTENT_THAT_WRAPS${underlineOff} suffix${reset}`; + + const wrapped = wrapTextWithAnsi(text, 20); + + console.log("Wrapped lines:"); + for (let i = 0; i < wrapped.length; i++) { + console.log(` [${i}]: ${JSON.stringify(wrapped[i])}`); + } + + // All lines should have background color 41 (either as \x1b[41m or combined like \x1b[4;41m) + for (const line of wrapped) { + const hasBgColor = line.includes("[41m") || line.includes(";41m") || line.includes("[41;"); + expect(hasBgColor).toBe(true); + } + + // Lines with underlined content should use underline-off at end, not full reset + for (let i = 0; i < wrapped.length - 1; i++) { + const line = wrapped[i]; + // If this line has underline on, it should end with underline off (not full reset) + if ( + (line.includes("[4m") || line.includes("[4;") || line.includes(";4m")) && + !line.includes(underlineOff) + ) { + expect(line.endsWith(underlineOff)).toBe(true); + expect(line.endsWith("\x1b[0m")).toBe(false); + } + } + }); }); - it("handles text with resets", () => { - const text = chalk.bold("bold ") + "normal " + chalk.cyan("cyan"); - const lines = wrapTextWithAnsi(text, 30); + describe("basic wrapping", () => { + it("should wrap plain text correctly", () => { + const text = "hello world this is a test"; + const wrapped = wrapTextWithAnsi(text, 10); - assert.strictEqual(lines.length, 1); - // Should contain the reset code from chalk - assert.ok(lines[0].includes("\x1b[")); - }); + expect(wrapped.length).toBeGreaterThan(1); + wrapped.forEach((line) => { + expect(visibleWidth(line)).toBeLessThanOrEqual(10); + }); + }); - it("does NOT pad lines", () => { - const text = "hello"; - const lines = wrapTextWithAnsi(text, 20); + it("should preserve color codes across wraps", () => { + const red = "\x1b[31m"; + const reset = "\x1b[0m"; + const text = `${red}hello world this is red${reset}`; - assert.strictEqual(lines.length, 1); - assert.strictEqual(visibleWidth(lines[0]), 5); // NOT 20 - }); + const wrapped = wrapTextWithAnsi(text, 10); - it("handles empty text", () => { - const lines = wrapTextWithAnsi("", 20); - assert.strictEqual(lines.length, 1); - assert.strictEqual(lines[0], ""); - }); + // Each continuation line should start with red code + for (let i = 1; i < wrapped.length; i++) { + expect(wrapped[i].startsWith(red)).toBe(true); + } - it("handles newlines", () => { - const text = "line1\nline2\nline3"; - const lines = wrapTextWithAnsi(text, 20); - - assert.strictEqual(lines.length, 3); - assert.strictEqual(lines[0], "line1"); - assert.strictEqual(lines[1], "line2"); - assert.strictEqual(lines[2], "line3"); - }); -}); - -describe("applyBackgroundToLine", () => { - const greenBg = (text: string) => chalk.bgGreen(text); - - it("applies background to plain text and pads to width", () => { - const line = "hello"; - const result = applyBackgroundToLine(line, 20, greenBg); - - // Should be exactly 20 visible chars - const stripped = result.replace(/\x1b\[[0-9;]*m/g, ""); - assert.strictEqual(stripped.length, 20); - - // Should have background codes - assert.ok(result.includes("\x1b[48") || result.includes("\x1b[42m")); - assert.ok(result.includes("\x1b[49m")); - }); - - it("handles text with ANSI codes and resets", () => { - const line = chalk.bold("hello") + " world"; - const result = applyBackgroundToLine(line, 20, greenBg); - - // Should be exactly 20 visible chars - const stripped = result.replace(/\x1b\[[0-9;]*m/g, ""); - assert.strictEqual(stripped.length, 20); - - // Should still have bold - assert.ok(result.includes("\x1b[1m")); - - // Should have background throughout (even after resets) - assert.ok(result.includes("\x1b[48") || result.includes("\x1b[42m")); - }); - - it("handles text with 0m resets by reapplying background", () => { - // Simulate: bold text + reset + normal text - const line = "\x1b[1mhello\x1b[0m world"; - const result = applyBackgroundToLine(line, 20, greenBg); - - // Should NOT have black cells (spaces without background) - // Pattern we DON'T want: 49m or 0m followed by spaces before bg reapplied - const blackCellPattern = /(\x1b\[49m|\x1b\[0m)\s+\x1b\[48;2/; - assert.ok(!blackCellPattern.test(result), `Found black cells in: ${JSON.stringify(result)}`); - - // Should be exactly 20 chars - const stripped = result.replace(/\x1b\[[0-9;]*m/g, ""); - assert.strictEqual(stripped.length, 20); + // Middle lines should not end with full reset + for (let i = 0; i < wrapped.length - 1; i++) { + expect(wrapped[i].endsWith("\x1b[0m")).toBe(false); + } + }); }); }); diff --git a/packages/tui/vitest.config.ts b/packages/tui/vitest.config.ts new file mode 100644 index 00000000..a90c176d --- /dev/null +++ b/packages/tui/vitest.config.ts @@ -0,0 +1,7 @@ +import { defineConfig } from "vitest/config"; + +export default defineConfig({ + test: { + include: ["test/wrap-ansi.test.ts"], + }, +});