From bb8f6b376d83a9b70406279c87ab8b163fb32a38 Mon Sep 17 00:00:00 2001 From: zfflxx <106017702+zfflxx@users.noreply.github.com> Date: Mon, 7 Jul 2025 13:33:46 +0800 Subject: [PATCH] Fix nested markdown Rendering for table headers and rows #3331 (#3362) Co-authored-by: Ryan Fang --- .../src/ui/utils/InlineMarkdownRenderer.tsx | 162 ++++++++++++++++++ .../cli/src/ui/utils/MarkdownDisplay.test.tsx | 47 +++++ packages/cli/src/ui/utils/MarkdownDisplay.tsx | 144 +--------------- packages/cli/src/ui/utils/TableRenderer.tsx | 143 ++++++++++------ 4 files changed, 304 insertions(+), 192 deletions(-) create mode 100644 packages/cli/src/ui/utils/InlineMarkdownRenderer.tsx diff --git a/packages/cli/src/ui/utils/InlineMarkdownRenderer.tsx b/packages/cli/src/ui/utils/InlineMarkdownRenderer.tsx new file mode 100644 index 00000000..ff8d6257 --- /dev/null +++ b/packages/cli/src/ui/utils/InlineMarkdownRenderer.tsx @@ -0,0 +1,162 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import React from 'react'; +import { Text } from 'ink'; +import { Colors } from '../colors.js'; +import stringWidth from 'string-width'; + +// Constants for Markdown parsing +const BOLD_MARKER_LENGTH = 2; // For "**" +const ITALIC_MARKER_LENGTH = 1; // For "*" or "_" +const STRIKETHROUGH_MARKER_LENGTH = 2; // For "~~" +const INLINE_CODE_MARKER_LENGTH = 1; // For "`" +const UNDERLINE_TAG_START_LENGTH = 3; // For "" +const UNDERLINE_TAG_END_LENGTH = 4; // For "" + +interface RenderInlineProps { + text: string; +} + +const RenderInlineInternal: React.FC = ({ text }) => { + const nodes: React.ReactNode[] = []; + let lastIndex = 0; + const inlineRegex = + /(\*\*.*?\*\*|\*.*?\*|_.*?_|~~.*?~~|\[.*?\]\(.*?\)|`+.+?`+|.*?<\/u>)/g; + let match; + + while ((match = inlineRegex.exec(text)) !== null) { + if (match.index > lastIndex) { + nodes.push( + + {text.slice(lastIndex, match.index)} + , + ); + } + + const fullMatch = match[0]; + let renderedNode: React.ReactNode = null; + const key = `m-${match.index}`; + + try { + if ( + fullMatch.startsWith('**') && + fullMatch.endsWith('**') && + fullMatch.length > BOLD_MARKER_LENGTH * 2 + ) { + renderedNode = ( + + {fullMatch.slice(BOLD_MARKER_LENGTH, -BOLD_MARKER_LENGTH)} + + ); + } else if ( + fullMatch.length > ITALIC_MARKER_LENGTH * 2 && + ((fullMatch.startsWith('*') && fullMatch.endsWith('*')) || + (fullMatch.startsWith('_') && fullMatch.endsWith('_'))) && + !/\w/.test(text.substring(match.index - 1, match.index)) && + !/\w/.test( + text.substring(inlineRegex.lastIndex, inlineRegex.lastIndex + 1), + ) && + !/\S[./\\]/.test(text.substring(match.index - 2, match.index)) && + !/[./\\]\S/.test( + text.substring(inlineRegex.lastIndex, inlineRegex.lastIndex + 2), + ) + ) { + renderedNode = ( + + {fullMatch.slice(ITALIC_MARKER_LENGTH, -ITALIC_MARKER_LENGTH)} + + ); + } else if ( + fullMatch.startsWith('~~') && + fullMatch.endsWith('~~') && + fullMatch.length > STRIKETHROUGH_MARKER_LENGTH * 2 + ) { + renderedNode = ( + + {fullMatch.slice( + STRIKETHROUGH_MARKER_LENGTH, + -STRIKETHROUGH_MARKER_LENGTH, + )} + + ); + } else if ( + fullMatch.startsWith('`') && + fullMatch.endsWith('`') && + fullMatch.length > INLINE_CODE_MARKER_LENGTH + ) { + const codeMatch = fullMatch.match(/^(`+)(.+?)\1$/s); + if (codeMatch && codeMatch[2]) { + renderedNode = ( + + {codeMatch[2]} + + ); + } + } else if ( + fullMatch.startsWith('[') && + fullMatch.includes('](') && + fullMatch.endsWith(')') + ) { + const linkMatch = fullMatch.match(/\[(.*?)\]\((.*?)\)/); + if (linkMatch) { + const linkText = linkMatch[1]; + const url = linkMatch[2]; + renderedNode = ( + + {linkText} + ({url}) + + ); + } + } else if ( + fullMatch.startsWith('') && + fullMatch.endsWith('') && + fullMatch.length > + UNDERLINE_TAG_START_LENGTH + UNDERLINE_TAG_END_LENGTH - 1 // -1 because length is compared to combined length of start and end tags + ) { + renderedNode = ( + + {fullMatch.slice( + UNDERLINE_TAG_START_LENGTH, + -UNDERLINE_TAG_END_LENGTH, + )} + + ); + } + } catch (e) { + console.error('Error parsing inline markdown part:', fullMatch, e); + renderedNode = null; + } + + nodes.push(renderedNode ?? {fullMatch}); + lastIndex = inlineRegex.lastIndex; + } + + if (lastIndex < text.length) { + nodes.push({text.slice(lastIndex)}); + } + + return <>{nodes.filter((node) => node !== null)}; +}; + +export const RenderInline = React.memo(RenderInlineInternal); + +/** + * Utility function to get the plain text length of a string with markdown formatting + * This is useful for calculating column widths in tables + */ +export const getPlainTextLength = (text: string): number => { + const cleanText = text + .replace(/\*\*(.*?)\*\*/g, '$1') + .replace(/\*(.*?)\*/g, '$1') + .replace(/_(.*?)_/g, '$1') + .replace(/~~(.*?)~~/g, '$1') + .replace(/`(.*?)`/g, '$1') + .replace(/(.*?)<\/u>/g, '$1') + .replace(/\[(.*?)\]\(.*?\)/g, '$1'); + return stringWidth(cleanText); +}; diff --git a/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx b/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx index 316c2d5d..9e0cb96c 100644 --- a/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx +++ b/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx @@ -232,6 +232,53 @@ But there's no separator line const { lastFrame } = renderNarrow(); expect(lastFrame()).not.toBe(''); }); + + it('should handle inline markdown in tables', () => { + // Test content from MarkdownDisplay.demo.tsx + const testContent = ` +# execSync vs spawn + +| Characteristic | \`execSync\` (Old Way) | \`spawn\` (New Way in PR) | +|----------------|------------------------|---------------------------| +| **Execution** | Synchronous (blocks everything) | Asynchronous (non-blocking) | +| **I/O Handling** | Buffers entire output in memory | Streams data in chunks (memory efficient) | +| **Security** | **Vulnerable to shell injection** | **Safe from shell injection** | +| **Use Case** | Simple, quick commands with small, trusted... | Long-running processes, large I/O, and especially for running user-configur... | + +`; + + const { lastFrame } = render( + , + ); + + const output = lastFrame(); + + // Check header + expect(output).toContain('execSync vs spawn'); + + // Check table headers - handle possible truncation + expect(output).toMatch(/Cha(racteristic)?/); // Match "Cha" or "Characteristic" + expect(output).toContain('execSync'); + expect(output).toContain('spawn'); + + // Check table content - test keywords rather than full sentences + expect(output).toMatch(/Exe(cution)?/); // Match "Exe" or "Execution" + expect(output).toContain('Synchronous'); + expect(output).toContain('Asynchronous'); + expect(output).toMatch(/I\/O|Handling/); // Match "I/O" or "Handling" + expect(output).toContain('Buffers'); + expect(output).toContain('Streams'); + expect(output).toMatch(/Sec(urity)?/); // Match "Sec" or "Security" + expect(output).toContain('Vulnerable'); + expect(output).toContain('Safe'); + expect(output).toMatch(/Use|Case/); // Match "Use" or "Case" + expect(output).toContain('Simple'); + expect(output).toContain('Long-running'); + }); }); describe('Existing Functionality', () => { diff --git a/packages/cli/src/ui/utils/MarkdownDisplay.tsx b/packages/cli/src/ui/utils/MarkdownDisplay.tsx index 55f1ce57..572ae12e 100644 --- a/packages/cli/src/ui/utils/MarkdownDisplay.tsx +++ b/packages/cli/src/ui/utils/MarkdownDisplay.tsx @@ -9,6 +9,7 @@ import { Text, Box } from 'ink'; import { Colors } from '../colors.js'; import { colorizeCode } from './CodeColorizer.js'; import { TableRenderer } from './TableRenderer.js'; +import { RenderInline } from './InlineMarkdownRenderer.js'; interface MarkdownDisplayProps { text: string; @@ -18,12 +19,6 @@ interface MarkdownDisplayProps { } // Constants for Markdown parsing and rendering -const BOLD_MARKER_LENGTH = 2; // For "**" -const ITALIC_MARKER_LENGTH = 1; // For "*" or "_" -const STRIKETHROUGH_MARKER_LENGTH = 2; // For "~~" -const INLINE_CODE_MARKER_LENGTH = 1; // For "`" -const UNDERLINE_TAG_START_LENGTH = 3; // For "" -const UNDERLINE_TAG_END_LENGTH = 4; // For "" const EMPTY_LINE_HEIGHT = 1; const CODE_BLOCK_PADDING = 1; @@ -277,143 +272,6 @@ const MarkdownDisplayInternal: React.FC = ({ // Helper functions (adapted from static methods of MarkdownRenderer) -interface RenderInlineProps { - text: string; -} - -const RenderInlineInternal: React.FC = ({ text }) => { - const nodes: React.ReactNode[] = []; - let lastIndex = 0; - const inlineRegex = - /(\*\*.*?\*\*|\*.*?\*|_.*?_|~~.*?~~|\[.*?\]\(.*?\)|`+.+?`+|.*?<\/u>)/g; - let match; - - while ((match = inlineRegex.exec(text)) !== null) { - if (match.index > lastIndex) { - nodes.push( - - {text.slice(lastIndex, match.index)} - , - ); - } - - const fullMatch = match[0]; - let renderedNode: React.ReactNode = null; - const key = `m-${match.index}`; - - try { - if ( - fullMatch.startsWith('**') && - fullMatch.endsWith('**') && - fullMatch.length > BOLD_MARKER_LENGTH * 2 - ) { - renderedNode = ( - - {fullMatch.slice(BOLD_MARKER_LENGTH, -BOLD_MARKER_LENGTH)} - - ); - } else if ( - fullMatch.length > ITALIC_MARKER_LENGTH * 2 && - ((fullMatch.startsWith('*') && fullMatch.endsWith('*')) || - (fullMatch.startsWith('_') && fullMatch.endsWith('_'))) && - !/\w/.test(text.substring(match.index - 1, match.index)) && - !/\w/.test( - text.substring(inlineRegex.lastIndex, inlineRegex.lastIndex + 1), - ) && - !/\S[./\\]/.test(text.substring(match.index - 2, match.index)) && - !/[./\\]\S/.test( - text.substring(inlineRegex.lastIndex, inlineRegex.lastIndex + 2), - ) - ) { - renderedNode = ( - - {fullMatch.slice(ITALIC_MARKER_LENGTH, -ITALIC_MARKER_LENGTH)} - - ); - } else if ( - fullMatch.startsWith('~~') && - fullMatch.endsWith('~~') && - fullMatch.length > STRIKETHROUGH_MARKER_LENGTH * 2 - ) { - renderedNode = ( - - {fullMatch.slice( - STRIKETHROUGH_MARKER_LENGTH, - -STRIKETHROUGH_MARKER_LENGTH, - )} - - ); - } else if ( - fullMatch.startsWith('`') && - fullMatch.endsWith('`') && - fullMatch.length > INLINE_CODE_MARKER_LENGTH - ) { - const codeMatch = fullMatch.match(/^(`+)(.+?)\1$/s); - if (codeMatch && codeMatch[2]) { - renderedNode = ( - - {codeMatch[2]} - - ); - } else { - renderedNode = ( - - {fullMatch.slice( - INLINE_CODE_MARKER_LENGTH, - -INLINE_CODE_MARKER_LENGTH, - )} - - ); - } - } else if ( - fullMatch.startsWith('[') && - fullMatch.includes('](') && - fullMatch.endsWith(')') - ) { - const linkMatch = fullMatch.match(/\[(.*?)\]\((.*?)\)/); - if (linkMatch) { - const linkText = linkMatch[1]; - const url = linkMatch[2]; - renderedNode = ( - - {linkText} - ({url}) - - ); - } - } else if ( - fullMatch.startsWith('') && - fullMatch.endsWith('') && - fullMatch.length > - UNDERLINE_TAG_START_LENGTH + UNDERLINE_TAG_END_LENGTH - 1 // -1 because length is compared to combined length of start and end tags - ) { - renderedNode = ( - - {fullMatch.slice( - UNDERLINE_TAG_START_LENGTH, - -UNDERLINE_TAG_END_LENGTH, - )} - - ); - } - } catch (e) { - console.error('Error parsing inline markdown part:', fullMatch, e); - renderedNode = null; - } - - nodes.push(renderedNode ?? {fullMatch}); - lastIndex = inlineRegex.lastIndex; - } - - if (lastIndex < text.length) { - nodes.push({text.slice(lastIndex)}); - } - - return <>{nodes.filter((node) => node !== null)}; -}; - -const RenderInline = React.memo(RenderInlineInternal); - interface RenderCodeBlockProps { content: string[]; lang: string | null; diff --git a/packages/cli/src/ui/utils/TableRenderer.tsx b/packages/cli/src/ui/utils/TableRenderer.tsx index 745e5135..2ec19549 100644 --- a/packages/cli/src/ui/utils/TableRenderer.tsx +++ b/packages/cli/src/ui/utils/TableRenderer.tsx @@ -7,6 +7,7 @@ import React from 'react'; import { Text, Box } from 'ink'; import { Colors } from '../colors.js'; +import { RenderInline, getPlainTextLength } from './InlineMarkdownRenderer.js'; interface TableRendererProps { headers: string[]; @@ -23,11 +24,11 @@ export const TableRenderer: React.FC = ({ rows, terminalWidth, }) => { - // Calculate column widths + // Calculate column widths using actual display width after markdown processing const columnWidths = headers.map((header, index) => { - const headerWidth = header.length; + const headerWidth = getPlainTextLength(header); const maxRowWidth = Math.max( - ...rows.map((row) => (row[index] || '').length), + ...rows.map((row) => getPlainTextLength(row[index] || '')), ); return Math.max(headerWidth, maxRowWidth) + 2; // Add padding }); @@ -40,75 +41,119 @@ export const TableRenderer: React.FC = ({ Math.floor(width * scaleFactor), ); - const renderCell = (content: string, width: number, isHeader = false) => { - // The actual space for content inside the padding + // Helper function to render a cell with proper width + const renderCell = ( + content: string, + width: number, + isHeader = false, + ): React.ReactNode => { const contentWidth = Math.max(0, width - 2); + const displayWidth = getPlainTextLength(content); let cellContent = content; - if (content.length > contentWidth) { + if (displayWidth > contentWidth) { if (contentWidth <= 3) { - // Not enough space for '...' - cellContent = content.substring(0, contentWidth); + // Just truncate by character count + cellContent = content.substring( + 0, + Math.min(content.length, contentWidth), + ); } else { - cellContent = content.substring(0, contentWidth - 3) + '...'; + // Truncate preserving markdown formatting using binary search + let left = 0; + let right = content.length; + let bestTruncated = content; + + // Binary search to find the optimal truncation point + while (left <= right) { + const mid = Math.floor((left + right) / 2); + const candidate = content.substring(0, mid); + const candidateWidth = getPlainTextLength(candidate); + + if (candidateWidth <= contentWidth - 3) { + bestTruncated = candidate; + left = mid + 1; + } else { + right = mid - 1; + } + } + + cellContent = bestTruncated + '...'; } } - // Pad the content to fill the cell - const padded = cellContent.padEnd(contentWidth, ' '); + // Calculate exact padding needed + const actualDisplayWidth = getPlainTextLength(cellContent); + const paddingNeeded = Math.max(0, contentWidth - actualDisplayWidth); - if (isHeader) { - return ( - - {padded} - - ); - } - return {padded}; + return ( + + {isHeader ? ( + + + + ) : ( + + )} + {' '.repeat(paddingNeeded)} + + ); }; - const renderRow = (cells: string[], isHeader = false) => ( - - │ - {cells.map((cell, index) => ( - - {renderCell(cell, adjustedWidths[index] || 0, isHeader)} - │ - - ))} - - ); + // Helper function to render border + const renderBorder = (type: 'top' | 'middle' | 'bottom'): React.ReactNode => { + const chars = { + top: { left: '┌', middle: '┬', right: '┐', horizontal: '─' }, + middle: { left: '├', middle: '┼', right: '┤', horizontal: '─' }, + bottom: { left: '└', middle: '┴', right: '┘', horizontal: '─' }, + }; - const renderSeparator = () => { - const separator = adjustedWidths - .map((width) => '─'.repeat(Math.max(0, (width || 0) - 2))) - .join('─┼─'); - return ├─{separator}─┤; + const char = chars[type]; + const borderParts = adjustedWidths.map((w) => char.horizontal.repeat(w)); + const border = char.left + borderParts.join(char.middle) + char.right; + + return {border}; }; - const renderTopBorder = () => { - const border = adjustedWidths - .map((width) => '─'.repeat(Math.max(0, (width || 0) - 2))) - .join('─┬─'); - return ┌─{border}─┐; - }; + // Helper function to render a table row + const renderRow = (cells: string[], isHeader = false): React.ReactNode => { + const renderedCells = cells.map((cell, index) => { + const width = adjustedWidths[index] || 0; + return renderCell(cell || '', width, isHeader); + }); - const renderBottomBorder = () => { - const border = adjustedWidths - .map((width) => '─'.repeat(Math.max(0, (width || 0) - 2))) - .join('─┴─'); - return └─{border}─┘; + return ( + + │{' '} + {renderedCells.map((cell, index) => ( + + {cell} + {index < renderedCells.length - 1 ? ' │ ' : ''} + + ))}{' '} + │ + + ); }; return ( - {renderTopBorder()} + {/* Top border */} + {renderBorder('top')} + + {/* Header row */} {renderRow(headers, true)} - {renderSeparator()} + + {/* Middle border */} + {renderBorder('middle')} + + {/* Data rows */} {rows.map((row, index) => ( {renderRow(row)} ))} - {renderBottomBorder()} + + {/* Bottom border */} + {renderBorder('bottom')} ); };