From c1113deea94213ea922a9592f510e5a36df43d25 Mon Sep 17 00:00:00 2001 From: Ahmed Kamal Date: Wed, 17 Dec 2025 18:13:27 +0200 Subject: [PATCH] Fix markdown tables overflowing/wrapping in TUI (width-aware rendering) (#206) Fix markdown tables overflowing/wrapping in TUI --- packages/tui/src/components/markdown.ts | 124 ++++++++++++---- packages/tui/test/markdown.test.ts | 186 ++++++++++++++++++++++++ 2 files changed, 281 insertions(+), 29 deletions(-) diff --git a/packages/tui/src/components/markdown.ts b/packages/tui/src/components/markdown.ts index 51c8d696..c601c7be 100644 --- a/packages/tui/src/components/markdown.ts +++ b/packages/tui/src/components/markdown.ts @@ -284,7 +284,7 @@ export class Markdown implements Component { } case "table": { - const tableLines = this.renderTable(token as any); + const tableLines = this.renderTable(token as any, width); lines.push(...tableLines); break; } @@ -489,56 +489,122 @@ export class Markdown implements Component { } /** - * Render a table + * Wrap a table cell to fit into a column. + * + * Delegates to wrapTextWithAnsi() so ANSI codes + long tokens are handled + * consistently with the rest of the renderer. */ - private renderTable(token: Token & { header: any[]; rows: any[][] }): string[] { + private wrapCellText(text: string, maxWidth: number): string[] { + return wrapTextWithAnsi(text, Math.max(1, maxWidth)); + } + + /** + * Render a table with width-aware cell wrapping. + * Cells that don't fit are wrapped to multiple lines. + */ + private renderTable( + token: Token & { header: any[]; rows: any[][]; raw?: string }, + availableWidth: number, + ): string[] { const lines: string[] = []; + const numCols = token.header.length; - // Calculate column widths - const columnWidths: number[] = []; - - // Check header - for (let i = 0; i < token.header.length; i++) { - const headerText = this.renderInlineTokens(token.header[i].tokens || []); - const width = visibleWidth(headerText); - columnWidths[i] = Math.max(columnWidths[i] || 0, width); + if (numCols === 0) { + return lines; } - // Check rows + // Calculate border overhead: "│ " + (n-1) * " │ " + " │" + // = 2 + (n-1) * 3 + 2 = 3n + 1 + const borderOverhead = 3 * numCols + 1; + + // Minimum width for a bordered table with at least 1 char per column. + const minTableWidth = borderOverhead + numCols; + if (availableWidth < minTableWidth) { + // Too narrow to render a stable table. Fall back to raw markdown. + const fallbackLines = token.raw ? wrapTextWithAnsi(token.raw, availableWidth) : []; + fallbackLines.push(""); + return fallbackLines; + } + + // Calculate natural column widths (what each column needs without constraints) + const naturalWidths: number[] = []; + for (let i = 0; i < numCols; i++) { + const headerText = this.renderInlineTokens(token.header[i].tokens || []); + naturalWidths[i] = visibleWidth(headerText); + } for (const row of token.rows) { for (let i = 0; i < row.length; i++) { const cellText = this.renderInlineTokens(row[i].tokens || []); - const width = visibleWidth(cellText); - columnWidths[i] = Math.max(columnWidths[i] || 0, width); + naturalWidths[i] = Math.max(naturalWidths[i] || 0, visibleWidth(cellText)); } } - // Limit column widths to reasonable max - const maxColWidth = 40; - for (let i = 0; i < columnWidths.length; i++) { - columnWidths[i] = Math.min(columnWidths[i], maxColWidth); + // Calculate column widths that fit within available width + const totalNaturalWidth = naturalWidths.reduce((a, b) => a + b, 0) + borderOverhead; + let columnWidths: number[]; + + if (totalNaturalWidth <= availableWidth) { + // Everything fits naturally + columnWidths = naturalWidths; + } else { + // Need to shrink columns to fit + const availableForCells = availableWidth - borderOverhead; + if (availableForCells <= numCols) { + // Extremely narrow - give each column at least 1 char + columnWidths = naturalWidths.map(() => Math.max(1, Math.floor(availableForCells / numCols))); + } else { + // Distribute space proportionally based on natural widths + const totalNatural = naturalWidths.reduce((a, b) => a + b, 0); + columnWidths = naturalWidths.map((w) => { + const proportion = w / totalNatural; + return Math.max(1, Math.floor(proportion * availableForCells)); + }); + + // Adjust for rounding errors - distribute remaining space + const allocated = columnWidths.reduce((a, b) => a + b, 0); + let remaining = availableForCells - allocated; + for (let i = 0; remaining > 0 && i < numCols; i++) { + columnWidths[i]++; + remaining--; + } + } } - // Render header - const headerCells = token.header.map((cell, i) => { + // Render header with wrapping + const headerCellLines: string[][] = token.header.map((cell, i) => { const text = this.renderInlineTokens(cell.tokens || []); - return this.theme.bold(text.padEnd(columnWidths[i])); + return this.wrapCellText(text, columnWidths[i]); }); - lines.push("│ " + headerCells.join(" │ ") + " │"); + const headerLineCount = Math.max(...headerCellLines.map((c) => c.length)); + + for (let lineIdx = 0; lineIdx < headerLineCount; lineIdx++) { + const rowParts = headerCellLines.map((cellLines, colIdx) => { + const text = cellLines[lineIdx] || ""; + const padded = text + " ".repeat(Math.max(0, columnWidths[colIdx] - visibleWidth(text))); + return this.theme.bold(padded); + }); + lines.push("│ " + rowParts.join(" │ ") + " │"); + } // Render separator - const separatorCells = columnWidths.map((width) => "─".repeat(width)); + const separatorCells = columnWidths.map((w) => "─".repeat(w)); lines.push("├─" + separatorCells.join("─┼─") + "─┤"); - // Render rows + // Render rows with wrapping for (const row of token.rows) { - const rowCells = row.map((cell, i) => { + const rowCellLines: string[][] = row.map((cell, i) => { const text = this.renderInlineTokens(cell.tokens || []); - const visWidth = visibleWidth(text); - const padding = " ".repeat(Math.max(0, columnWidths[i] - visWidth)); - return text + padding; + return this.wrapCellText(text, columnWidths[i]); }); - lines.push("│ " + rowCells.join(" │ ") + " │"); + const rowLineCount = Math.max(...rowCellLines.map((c) => c.length)); + + for (let lineIdx = 0; lineIdx < rowLineCount; lineIdx++) { + const rowParts = rowCellLines.map((cellLines, colIdx) => { + const text = cellLines[lineIdx] || ""; + return text + " ".repeat(Math.max(0, columnWidths[colIdx] - visibleWidth(text))); + }); + lines.push("│ " + rowParts.join(" │ ") + " │"); + } } lines.push(""); // Add spacing after table diff --git a/packages/tui/test/markdown.test.ts b/packages/tui/test/markdown.test.ts index aaa1fa37..d313902c 100644 --- a/packages/tui/test/markdown.test.ts +++ b/packages/tui/test/markdown.test.ts @@ -164,6 +164,192 @@ describe("Markdown component", () => { assert.ok(plainLines.some((line) => line.includes("Very long column header"))); assert.ok(plainLines.some((line) => line.includes("This is a much longer cell content"))); }); + + it("should wrap table cells when table exceeds available width", () => { + const markdown = new Markdown( + `| Command | Description | Example | +| --- | --- | --- | +| npm install | Install all dependencies | npm install | +| npm run build | Build the project | npm run build |`, + 0, + 0, + defaultMarkdownTheme, + ); + + // Render at narrow width that forces wrapping + const lines = markdown.render(50); + const plainLines = lines.map((line) => line.replace(/\x1b\[[0-9;]*m/g, "").trimEnd()); + + // All lines should fit within width + for (const line of plainLines) { + assert.ok(line.length <= 50, `Line exceeds width 50: "${line}" (length: ${line.length})`); + } + + // Content should still be present (possibly wrapped across lines) + const allText = plainLines.join(" "); + assert.ok(allText.includes("Command"), "Should contain 'Command'"); + assert.ok(allText.includes("Description"), "Should contain 'Description'"); + assert.ok(allText.includes("npm install"), "Should contain 'npm install'"); + assert.ok(allText.includes("Install"), "Should contain 'Install'"); + }); + + it("should wrap long cell content to multiple lines", () => { + const markdown = new Markdown( + `| Header | +| --- | +| This is a very long cell content that should wrap |`, + 0, + 0, + defaultMarkdownTheme, + ); + + // Render at width that forces the cell to wrap + const lines = markdown.render(25); + const plainLines = lines.map((line) => line.replace(/\x1b\[[0-9;]*m/g, "").trimEnd()); + + // Should have multiple data rows due to wrapping + const dataRows = plainLines.filter((line) => line.startsWith("│") && !line.includes("─")); + assert.ok(dataRows.length > 2, `Expected wrapped rows, got ${dataRows.length} rows`); + + // All content should be preserved (may be split across lines) + const allText = plainLines.join(" "); + assert.ok(allText.includes("very long"), "Should preserve 'very long'"); + assert.ok(allText.includes("cell content"), "Should preserve 'cell content'"); + assert.ok(allText.includes("should wrap"), "Should preserve 'should wrap'"); + }); + + it("should wrap long unbroken tokens inside table cells (not only at line start)", () => { + const url = "https://example.com/this/is/a/very/long/url/that/should/wrap"; + const markdown = new Markdown( + `| Value | +| --- | +| prefix ${url} |`, + 0, + 0, + defaultMarkdownTheme, + ); + + const width = 30; + const lines = markdown.render(width); + const plainLines = lines.map((line) => line.replace(/\x1b\[[0-9;]*m/g, "").trimEnd()); + + for (const line of plainLines) { + assert.ok(line.length <= width, `Line exceeds width ${width}: "${line}" (length: ${line.length})`); + } + + // Borders should stay intact (exactly 2 vertical borders for a 1-col table) + const tableLines = plainLines.filter((line) => line.startsWith("│")); + for (const line of tableLines) { + const borderCount = line.split("│").length - 1; + assert.strictEqual(borderCount, 2, `Expected 2 borders, got ${borderCount}: "${line}"`); + } + + // Strip box drawing characters + whitespace so we can assert the URL is preserved + // even if it was split across multiple wrapped lines. + const extracted = plainLines.join("").replace(/[│├┤─\s]/g, ""); + assert.ok(extracted.includes("prefix"), "Should preserve 'prefix'"); + assert.ok(extracted.includes(url), "Should preserve URL"); + }); + + it("should wrap styled inline code inside table cells without breaking borders", () => { + const markdown = new Markdown( + `| Code | +| --- | +| \`averyveryveryverylongidentifier\` |`, + 0, + 0, + defaultMarkdownTheme, + ); + + const width = 20; + const lines = markdown.render(width); + const joinedOutput = lines.join("\n"); + assert.ok(joinedOutput.includes("\x1b[33m"), "Inline code should be styled (yellow)"); + + const plainLines = lines.map((line) => line.replace(/\x1b\[[0-9;]*m/g, "").trimEnd()); + for (const line of plainLines) { + assert.ok(line.length <= width, `Line exceeds width ${width}: "${line}" (length: ${line.length})`); + } + + const tableLines = plainLines.filter((line) => line.startsWith("│")); + for (const line of tableLines) { + const borderCount = line.split("│").length - 1; + assert.strictEqual(borderCount, 2, `Expected 2 borders, got ${borderCount}: "${line}"`); + } + }); + + it("should handle extremely narrow width gracefully", () => { + const markdown = new Markdown( + `| A | B | C | +| --- | --- | --- | +| 1 | 2 | 3 |`, + 0, + 0, + defaultMarkdownTheme, + ); + + // Very narrow width + const lines = markdown.render(15); + const plainLines = lines.map((line) => line.replace(/\x1b\[[0-9;]*m/g, "").trimEnd()); + + // Should not crash and should produce output + assert.ok(lines.length > 0, "Should produce output"); + + // Lines should not exceed width + for (const line of plainLines) { + assert.ok(line.length <= 15, `Line exceeds width 15: "${line}" (length: ${line.length})`); + } + }); + + it("should render table correctly when it fits naturally", () => { + const markdown = new Markdown( + `| A | B | +| --- | --- | +| 1 | 2 |`, + 0, + 0, + defaultMarkdownTheme, + ); + + // Wide width where table fits naturally + const lines = markdown.render(80); + const plainLines = lines.map((line) => line.replace(/\x1b\[[0-9;]*m/g, "").trimEnd()); + + // Should have proper table structure + const headerLine = plainLines.find((line) => line.includes("A") && line.includes("B")); + assert.ok(headerLine, "Should have header row"); + assert.ok(headerLine?.includes("│"), "Header should have borders"); + + const separatorLine = plainLines.find((line) => line.includes("├") && line.includes("┼")); + assert.ok(separatorLine, "Should have separator row"); + + const dataLine = plainLines.find((line) => line.includes("1") && line.includes("2")); + assert.ok(dataLine, "Should have data row"); + }); + + it("should respect paddingX when calculating table width", () => { + const markdown = new Markdown( + `| Column One | Column Two | +| --- | --- | +| Data 1 | Data 2 |`, + 2, // paddingX = 2 + 0, + defaultMarkdownTheme, + ); + + // Width 40 with paddingX=2 means contentWidth=36 + const lines = markdown.render(40); + const plainLines = lines.map((line) => line.replace(/\x1b\[[0-9;]*m/g, "").trimEnd()); + + // All lines should respect width + for (const line of plainLines) { + assert.ok(line.length <= 40, `Line exceeds width 40: "${line}" (length: ${line.length})`); + } + + // Table rows should have left padding + const tableRow = plainLines.find((line) => line.includes("│")); + assert.ok(tableRow?.startsWith(" "), "Table should have left padding"); + }); }); describe("Combined features", () => {