From a4062cb44aab771822fcc4d0fb772bbcde256c1d Mon Sep 17 00:00:00 2001 From: Tian Jian Wang Date: Mon, 30 Jun 2025 20:25:19 -0700 Subject: [PATCH] feat: Add markdown table rendering support (#1955) Co-authored-by: heartyguy Co-authored-by: Allen Hutchison --- .../cli/src/ui/utils/MarkdownDisplay.test.tsx | 280 ++++++++++++++++++ packages/cli/src/ui/utils/MarkdownDisplay.tsx | 94 ++++++ packages/cli/src/ui/utils/TableRenderer.tsx | 114 +++++++ 3 files changed, 488 insertions(+) create mode 100644 packages/cli/src/ui/utils/MarkdownDisplay.test.tsx create mode 100644 packages/cli/src/ui/utils/TableRenderer.tsx diff --git a/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx b/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx new file mode 100644 index 00000000..316c2d5d --- /dev/null +++ b/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx @@ -0,0 +1,280 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { render } from 'ink-testing-library'; +import { MarkdownDisplay } from './MarkdownDisplay.js'; + +describe('MarkdownDisplay', () => { + describe('Table Rendering', () => { + it('should render a simple table', () => { + const tableMarkdown = ` +| Name | Age | City | +|------|-----|------| +| John | 25 | NYC | +| Jane | 30 | LA | +`; + + const { lastFrame } = render( + , + ); + + expect(lastFrame()).toContain('Name'); + expect(lastFrame()).toContain('Age'); + expect(lastFrame()).toContain('City'); + expect(lastFrame()).toContain('John'); + expect(lastFrame()).toContain('25'); + expect(lastFrame()).toContain('NYC'); + expect(lastFrame()).toContain('Jane'); + expect(lastFrame()).toContain('30'); + expect(lastFrame()).toContain('LA'); + }); + + it('should handle tables with varying column widths', () => { + const tableMarkdown = ` +| Short | Medium Column | Very Long Column Name | +|-------|---------------|----------------------| +| A | Some text | This is a longer text content | +| B | More content | Another piece of content here | +`; + + const { lastFrame } = render( + , + ); + + expect(lastFrame()).toContain('Short'); + expect(lastFrame()).toContain('Medium Column'); + expect(lastFrame()).toContain('Very Long Column Name'); + }); + + it('should handle empty cells in tables', () => { + const tableMarkdown = ` +| Col1 | Col2 | Col3 | +|------|------|------| +| A | | C | +| | B | | +`; + + const { lastFrame } = render( + , + ); + + expect(lastFrame()).toContain('Col1'); + expect(lastFrame()).toContain('Col2'); + expect(lastFrame()).toContain('Col3'); + expect(lastFrame()).toContain('A'); + expect(lastFrame()).toContain('B'); + expect(lastFrame()).toContain('C'); + }); + + it('should handle mixed content with tables', () => { + const mixedMarkdown = ` +# Header + +Some paragraph text before the table. + +| Feature | Status | Notes | +|---------|--------|-------| +| Auth | Done | OAuth | +| API | WIP | REST | + +Some text after the table. +`; + + const { lastFrame } = render( + , + ); + + expect(lastFrame()).toContain('Header'); + expect(lastFrame()).toContain('Some paragraph text before the table.'); + expect(lastFrame()).toContain('Feature'); + expect(lastFrame()).toContain('Status'); + expect(lastFrame()).toContain('Auth'); + expect(lastFrame()).toContain('Done'); + expect(lastFrame()).toContain('Some text after the table.'); + }); + + it('should handle tables with empty cells at edges', () => { + const tableMarkdown = ` +| | Middle | | +|-|--------|-| +| | Value | | +`; + + const { lastFrame } = render( + , + ); + + expect(lastFrame()).toContain('Middle'); + expect(lastFrame()).toContain('Value'); + // Should maintain column structure even with empty edge cells + }); + + it('should handle PR reviewer test case 1', () => { + const tableMarkdown = ` +| Package | Lines of Code | +|---------|---------------| +| CLI | 18407 | +| Core | 14445 | +`; + + const { lastFrame } = render( + , + ); + + const output = lastFrame(); + expect(output).toContain('Package'); + expect(output).toContain('Lines of Code'); + expect(output).toContain('CLI'); + expect(output).toContain('18407'); + expect(output).toContain('Core'); + expect(output).toContain('14445'); + }); + + it('should handle PR reviewer test case 2 - long table', () => { + const tableMarkdown = ` +| Letter | Count | +|--------|-------| +| a | 15 | +| b | 2 | +| c | 26 | +| Total | 283 | +`; + + const { lastFrame } = render( + , + ); + + const output = lastFrame(); + expect(output).toContain('Letter'); + expect(output).toContain('Count'); + expect(output).toContain('a'); + expect(output).toContain('15'); + expect(output).toContain('Total'); + expect(output).toContain('283'); + }); + + it('should not render malformed tables', () => { + const malformedMarkdown = ` +| This looks like a table | +But there's no separator line +| So it shouldn't render as table | +`; + + const { lastFrame } = render( + , + ); + + // Should render as regular text, not a table + expect(lastFrame()).toContain('| This looks like a table |'); + expect(lastFrame()).toContain("But there's no separator line"); + expect(lastFrame()).toContain("| So it shouldn't render as table |"); + }); + + it('should not crash when rendering a very wide table in a narrow terminal', () => { + const wideTable = ` +| Col 1 | Col 2 | Col 3 | Col 4 | Col 5 | Col 6 | Col 7 | Col 8 | Col 9 | Col 10 | +|-------|-------|-------|-------|-------|-------|-------|-------|-------|--------| +| ${'a'.repeat(20)} | ${'b'.repeat(20)} | ${'c'.repeat(20)} | ${'d'.repeat( + 20, + )} | ${'e'.repeat(20)} | ${'f'.repeat(20)} | ${'g'.repeat(20)} | ${'h'.repeat( + 20, + )} | ${'i'.repeat(20)} | ${'j'.repeat(20)} | + `; + + const renderNarrow = () => + render( + , + ); + + // The important part is that this does not throw an error. + expect(renderNarrow).not.toThrow(); + + // We can also check that it rendered *something*. + const { lastFrame } = renderNarrow(); + expect(lastFrame()).not.toBe(''); + }); + }); + + describe('Existing Functionality', () => { + it('should render headers correctly', () => { + const headerMarkdown = ` +# H1 Header +## H2 Header +### H3 Header +#### H4 Header +`; + + const { lastFrame } = render( + , + ); + + expect(lastFrame()).toContain('H1 Header'); + expect(lastFrame()).toContain('H2 Header'); + expect(lastFrame()).toContain('H3 Header'); + expect(lastFrame()).toContain('H4 Header'); + }); + + it('should render code blocks correctly', () => { + const codeMarkdown = ` +\`\`\`javascript +const x = 42; +console.log(x); +\`\`\` +`; + + const { lastFrame } = render( + , + ); + + expect(lastFrame()).toContain('const x = 42;'); + expect(lastFrame()).toContain('console.log(x);'); + }); + }); +}); diff --git a/packages/cli/src/ui/utils/MarkdownDisplay.tsx b/packages/cli/src/ui/utils/MarkdownDisplay.tsx index d78360b5..55f1ce57 100644 --- a/packages/cli/src/ui/utils/MarkdownDisplay.tsx +++ b/packages/cli/src/ui/utils/MarkdownDisplay.tsx @@ -8,6 +8,7 @@ import React from 'react'; import { Text, Box } from 'ink'; import { Colors } from '../colors.js'; import { colorizeCode } from './CodeColorizer.js'; +import { TableRenderer } from './TableRenderer.js'; interface MarkdownDisplayProps { text: string; @@ -43,12 +44,17 @@ const MarkdownDisplayInternal: React.FC = ({ const ulItemRegex = /^([ \t]*)([-*+]) +(.*)/; const olItemRegex = /^([ \t]*)(\d+)\. +(.*)/; const hrRegex = /^ *([-*_] *){3,} *$/; + const tableRowRegex = /^\s*\|(.+)\|\s*$/; + const tableSeparatorRegex = /^\s*\|?\s*(:?-+:?)\s*(\|\s*(:?-+:?)\s*)+\|?\s*$/; const contentBlocks: React.ReactNode[] = []; let inCodeBlock = false; let codeBlockContent: string[] = []; let codeBlockLang: string | null = null; let codeBlockFence = ''; + let inTable = false; + let tableRows: string[][] = []; + let tableHeaders: string[] = []; lines.forEach((line, index) => { const key = `line-${index}`; @@ -85,11 +91,71 @@ const MarkdownDisplayInternal: React.FC = ({ const ulMatch = line.match(ulItemRegex); const olMatch = line.match(olItemRegex); const hrMatch = line.match(hrRegex); + const tableRowMatch = line.match(tableRowRegex); + const tableSeparatorMatch = line.match(tableSeparatorRegex); if (codeFenceMatch) { inCodeBlock = true; codeBlockFence = codeFenceMatch[1]; codeBlockLang = codeFenceMatch[2] || null; + } else if (tableRowMatch && !inTable) { + // Potential table start - check if next line is separator + if ( + index + 1 < lines.length && + lines[index + 1].match(tableSeparatorRegex) + ) { + inTable = true; + tableHeaders = tableRowMatch[1].split('|').map((cell) => cell.trim()); + tableRows = []; + } else { + // Not a table, treat as regular text + contentBlocks.push( + + + + + , + ); + } + } else if (inTable && tableSeparatorMatch) { + // Skip separator line - already handled + } else if (inTable && tableRowMatch) { + // Add table row + const cells = tableRowMatch[1].split('|').map((cell) => cell.trim()); + // Ensure row has same column count as headers + while (cells.length < tableHeaders.length) { + cells.push(''); + } + if (cells.length > tableHeaders.length) { + cells.length = tableHeaders.length; + } + tableRows.push(cells); + } else if (inTable && !tableRowMatch) { + // End of table + if (tableHeaders.length > 0 && tableRows.length > 0) { + contentBlocks.push( + , + ); + } + inTable = false; + tableRows = []; + tableHeaders = []; + + // Process current line as normal + if (line.trim().length > 0) { + contentBlocks.push( + + + + + , + ); + } } else if (hrMatch) { contentBlocks.push( @@ -194,6 +260,18 @@ const MarkdownDisplayInternal: React.FC = ({ ); } + // Handle table at end of content + if (inTable && tableHeaders.length > 0 && tableRows.length > 0) { + contentBlocks.push( + , + ); + } + return <>{contentBlocks}; }; @@ -443,4 +521,20 @@ const RenderListItemInternal: React.FC = ({ const RenderListItem = React.memo(RenderListItemInternal); +interface RenderTableProps { + headers: string[]; + rows: string[][]; + terminalWidth: number; +} + +const RenderTableInternal: React.FC = ({ + headers, + rows, + terminalWidth, +}) => ( + +); + +const RenderTable = React.memo(RenderTableInternal); + export const MarkdownDisplay = React.memo(MarkdownDisplayInternal); diff --git a/packages/cli/src/ui/utils/TableRenderer.tsx b/packages/cli/src/ui/utils/TableRenderer.tsx new file mode 100644 index 00000000..745e5135 --- /dev/null +++ b/packages/cli/src/ui/utils/TableRenderer.tsx @@ -0,0 +1,114 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import React from 'react'; +import { Text, Box } from 'ink'; +import { Colors } from '../colors.js'; + +interface TableRendererProps { + headers: string[]; + rows: string[][]; + terminalWidth: number; +} + +/** + * Custom table renderer for markdown tables + * We implement our own instead of using ink-table due to module compatibility issues + */ +export const TableRenderer: React.FC = ({ + headers, + rows, + terminalWidth, +}) => { + // Calculate column widths + const columnWidths = headers.map((header, index) => { + const headerWidth = header.length; + const maxRowWidth = Math.max( + ...rows.map((row) => (row[index] || '').length), + ); + return Math.max(headerWidth, maxRowWidth) + 2; // Add padding + }); + + // Ensure table fits within terminal width + const totalWidth = columnWidths.reduce((sum, width) => sum + width + 1, 1); + const scaleFactor = + totalWidth > terminalWidth ? terminalWidth / totalWidth : 1; + const adjustedWidths = columnWidths.map((width) => + Math.floor(width * scaleFactor), + ); + + const renderCell = (content: string, width: number, isHeader = false) => { + // The actual space for content inside the padding + const contentWidth = Math.max(0, width - 2); + + let cellContent = content; + if (content.length > contentWidth) { + if (contentWidth <= 3) { + // Not enough space for '...' + cellContent = content.substring(0, contentWidth); + } else { + cellContent = content.substring(0, contentWidth - 3) + '...'; + } + } + + // Pad the content to fill the cell + const padded = cellContent.padEnd(contentWidth, ' '); + + if (isHeader) { + return ( + + {padded} + + ); + } + return {padded}; + }; + + const renderRow = (cells: string[], isHeader = false) => ( + + │ + {cells.map((cell, index) => ( + + {renderCell(cell, adjustedWidths[index] || 0, isHeader)} + │ + + ))} + + ); + + const renderSeparator = () => { + const separator = adjustedWidths + .map((width) => '─'.repeat(Math.max(0, (width || 0) - 2))) + .join('─┼─'); + return ├─{separator}─┤; + }; + + const renderTopBorder = () => { + const border = adjustedWidths + .map((width) => '─'.repeat(Math.max(0, (width || 0) - 2))) + .join('─┬─'); + return ┌─{border}─┐; + }; + + const renderBottomBorder = () => { + const border = adjustedWidths + .map((width) => '─'.repeat(Math.max(0, (width || 0) - 2))) + .join('─┴─'); + return └─{border}─┘; + }; + + return ( + + {renderTopBorder()} + {renderRow(headers, true)} + {renderSeparator()} + {rows.map((row, index) => ( + {renderRow(row)} + ))} + {renderBottomBorder()} + + ); +};