mirror of
https://github.com/getcompanion-ai/co-mono.git
synced 2026-04-20 11:03:07 +00:00
Fix markdown tables overflowing/wrapping in TUI (width-aware rendering) (#206)
Fix markdown tables overflowing/wrapping in TUI
This commit is contained in:
parent
d70edf571e
commit
c1113deea9
2 changed files with 281 additions and 29 deletions
|
|
@ -284,7 +284,7 @@ export class Markdown implements Component {
|
||||||
}
|
}
|
||||||
|
|
||||||
case "table": {
|
case "table": {
|
||||||
const tableLines = this.renderTable(token as any);
|
const tableLines = this.renderTable(token as any, width);
|
||||||
lines.push(...tableLines);
|
lines.push(...tableLines);
|
||||||
break;
|
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 lines: string[] = [];
|
||||||
|
const numCols = token.header.length;
|
||||||
|
|
||||||
// Calculate column widths
|
if (numCols === 0) {
|
||||||
const columnWidths: number[] = [];
|
return lines;
|
||||||
|
|
||||||
// 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);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// 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 (const row of token.rows) {
|
||||||
for (let i = 0; i < row.length; i++) {
|
for (let i = 0; i < row.length; i++) {
|
||||||
const cellText = this.renderInlineTokens(row[i].tokens || []);
|
const cellText = this.renderInlineTokens(row[i].tokens || []);
|
||||||
const width = visibleWidth(cellText);
|
naturalWidths[i] = Math.max(naturalWidths[i] || 0, visibleWidth(cellText));
|
||||||
columnWidths[i] = Math.max(columnWidths[i] || 0, width);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Limit column widths to reasonable max
|
// Calculate column widths that fit within available width
|
||||||
const maxColWidth = 40;
|
const totalNaturalWidth = naturalWidths.reduce((a, b) => a + b, 0) + borderOverhead;
|
||||||
for (let i = 0; i < columnWidths.length; i++) {
|
let columnWidths: number[];
|
||||||
columnWidths[i] = Math.min(columnWidths[i], maxColWidth);
|
|
||||||
|
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
|
// Render header with wrapping
|
||||||
const headerCells = token.header.map((cell, i) => {
|
const headerCellLines: string[][] = token.header.map((cell, i) => {
|
||||||
const text = this.renderInlineTokens(cell.tokens || []);
|
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
|
// Render separator
|
||||||
const separatorCells = columnWidths.map((width) => "─".repeat(width));
|
const separatorCells = columnWidths.map((w) => "─".repeat(w));
|
||||||
lines.push("├─" + separatorCells.join("─┼─") + "─┤");
|
lines.push("├─" + separatorCells.join("─┼─") + "─┤");
|
||||||
|
|
||||||
// Render rows
|
// Render rows with wrapping
|
||||||
for (const row of token.rows) {
|
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 text = this.renderInlineTokens(cell.tokens || []);
|
||||||
const visWidth = visibleWidth(text);
|
return this.wrapCellText(text, columnWidths[i]);
|
||||||
const padding = " ".repeat(Math.max(0, columnWidths[i] - visWidth));
|
|
||||||
return text + padding;
|
|
||||||
});
|
});
|
||||||
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
|
lines.push(""); // Add spacing after table
|
||||||
|
|
|
||||||
|
|
@ -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("Very long column header")));
|
||||||
assert.ok(plainLines.some((line) => line.includes("This is a much longer cell content")));
|
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", () => {
|
describe("Combined features", () => {
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue