Fix nested markdown Rendering for table headers and rows #3331 (#3362)

Co-authored-by: Ryan Fang <ryan.fang@gllue.com>
2025-07-07 13:33:46 +08:00 · 2025-07-07 13:33:46 +08:00 · bb8f6b376d
parent b70fba5b09
commit bb8f6b376d
4 changed files with 304 additions and 192 deletions
--- a/packages/cli/src/ui/utils/InlineMarkdownRenderer.tsx
+++ 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 "<u>"
+const UNDERLINE_TAG_END_LENGTH = 4; // For "</u>"
+
+interface RenderInlineProps {
+  text: string;
+}
+
+const RenderInlineInternal: React.FC<RenderInlineProps> = ({ text }) => {
+  const nodes: React.ReactNode[] = [];
+  let lastIndex = 0;
+  const inlineRegex =
+    /(\*\*.*?\*\*|\*.*?\*|_.*?_|~~.*?~~|\[.*?\]\(.*?\)|`+.+?`+|<u>.*?<\/u>)/g;
+  let match;
+
+  while ((match = inlineRegex.exec(text)) !== null) {
+    if (match.index > lastIndex) {
+      nodes.push(
+        <Text key={`t-${lastIndex}`}>
+          {text.slice(lastIndex, match.index)}
+        </Text>,
+      );
+    }
+
+    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 = (
+          <Text key={key} bold>
+            {fullMatch.slice(BOLD_MARKER_LENGTH, -BOLD_MARKER_LENGTH)}
+          </Text>
+        );
+      } 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 = (
+          <Text key={key} italic>
+            {fullMatch.slice(ITALIC_MARKER_LENGTH, -ITALIC_MARKER_LENGTH)}
+          </Text>
+        );
+      } else if (
+        fullMatch.startsWith('~~') &&
+        fullMatch.endsWith('~~') &&
+        fullMatch.length > STRIKETHROUGH_MARKER_LENGTH * 2
+      ) {
+        renderedNode = (
+          <Text key={key} strikethrough>
+            {fullMatch.slice(
+              STRIKETHROUGH_MARKER_LENGTH,
+              -STRIKETHROUGH_MARKER_LENGTH,
+            )}
+          </Text>
+        );
+      } else if (
+        fullMatch.startsWith('`') &&
+        fullMatch.endsWith('`') &&
+        fullMatch.length > INLINE_CODE_MARKER_LENGTH
+      ) {
+        const codeMatch = fullMatch.match(/^(`+)(.+?)\1$/s);
+        if (codeMatch && codeMatch[2]) {
+          renderedNode = (
+            <Text key={key} color={Colors.AccentPurple}>
+              {codeMatch[2]}
+            </Text>
+          );
+        }
+      } else if (
+        fullMatch.startsWith('[') &&
+        fullMatch.includes('](') &&
+        fullMatch.endsWith(')')
+      ) {
+        const linkMatch = fullMatch.match(/\[(.*?)\]\((.*?)\)/);
+        if (linkMatch) {
+          const linkText = linkMatch[1];
+          const url = linkMatch[2];
+          renderedNode = (
+            <Text key={key}>
+              {linkText}
+              <Text color={Colors.AccentBlue}> ({url})</Text>
+            </Text>
+          );
+        }
+      } else if (
+        fullMatch.startsWith('<u>') &&
+        fullMatch.endsWith('</u>') &&
+        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 = (
+          <Text key={key} underline>
+            {fullMatch.slice(
+              UNDERLINE_TAG_START_LENGTH,
+              -UNDERLINE_TAG_END_LENGTH,
+            )}
+          </Text>
+        );
+      }
+    } catch (e) {
+      console.error('Error parsing inline markdown part:', fullMatch, e);
+      renderedNode = null;
+    }
+
+    nodes.push(renderedNode ?? <Text key={key}>{fullMatch}</Text>);
+    lastIndex = inlineRegex.lastIndex;
+  }
+
+  if (lastIndex < text.length) {
+    nodes.push(<Text key={`t-${lastIndex}`}>{text.slice(lastIndex)}</Text>);
+  }
+
+  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>(.*?)<\/u>/g, '$1')
+    .replace(/\[(.*?)\]\(.*?\)/g, '$1');
+  return stringWidth(cleanText);
+};
--- 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(
+        <MarkdownDisplay
+          text={testContent}
+          isPending={false}
+          terminalWidth={120}
+        />,
+      );
+
+      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', () => {
--- 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 "<u>"
-const UNDERLINE_TAG_END_LENGTH = 4; // For "</u>"

 const EMPTY_LINE_HEIGHT = 1;
 const CODE_BLOCK_PADDING = 1;
@ -277,143 +272,6 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({

 // Helper functions (adapted from static methods of MarkdownRenderer)

-interface RenderInlineProps {
-  text: string;
-}
-
-const RenderInlineInternal: React.FC<RenderInlineProps> = ({ text }) => {
-  const nodes: React.ReactNode[] = [];
-  let lastIndex = 0;
-  const inlineRegex =
-    /(\*\*.*?\*\*|\*.*?\*|_.*?_|~~.*?~~|\[.*?\]\(.*?\)|`+.+?`+|<u>.*?<\/u>)/g;
-  let match;
-
-  while ((match = inlineRegex.exec(text)) !== null) {
-    if (match.index > lastIndex) {
-      nodes.push(
-        <Text key={`t-${lastIndex}`}>
-          {text.slice(lastIndex, match.index)}
-        </Text>,
-      );
-    }
-
-    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 = (
-          <Text key={key} bold>
-            {fullMatch.slice(BOLD_MARKER_LENGTH, -BOLD_MARKER_LENGTH)}
-          </Text>
-        );
-      } 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 = (
-          <Text key={key} italic>
-            {fullMatch.slice(ITALIC_MARKER_LENGTH, -ITALIC_MARKER_LENGTH)}
-          </Text>
-        );
-      } else if (
-        fullMatch.startsWith('~~') &&
-        fullMatch.endsWith('~~') &&
-        fullMatch.length > STRIKETHROUGH_MARKER_LENGTH * 2
-      ) {
-        renderedNode = (
-          <Text key={key} strikethrough>
-            {fullMatch.slice(
-              STRIKETHROUGH_MARKER_LENGTH,
-              -STRIKETHROUGH_MARKER_LENGTH,
-            )}
-          </Text>
-        );
-      } else if (
-        fullMatch.startsWith('`') &&
-        fullMatch.endsWith('`') &&
-        fullMatch.length > INLINE_CODE_MARKER_LENGTH
-      ) {
-        const codeMatch = fullMatch.match(/^(`+)(.+?)\1$/s);
-        if (codeMatch && codeMatch[2]) {
-          renderedNode = (
-            <Text key={key} color={Colors.AccentPurple}>
-              {codeMatch[2]}
-            </Text>
-          );
-        } else {
-          renderedNode = (
-            <Text key={key} color={Colors.AccentPurple}>
-              {fullMatch.slice(
-                INLINE_CODE_MARKER_LENGTH,
-                -INLINE_CODE_MARKER_LENGTH,
-              )}
-            </Text>
-          );
-        }
-      } else if (
-        fullMatch.startsWith('[') &&
-        fullMatch.includes('](') &&
-        fullMatch.endsWith(')')
-      ) {
-        const linkMatch = fullMatch.match(/\[(.*?)\]\((.*?)\)/);
-        if (linkMatch) {
-          const linkText = linkMatch[1];
-          const url = linkMatch[2];
-          renderedNode = (
-            <Text key={key}>
-              {linkText}
-              <Text color={Colors.AccentBlue}> ({url})</Text>
-            </Text>
-          );
-        }
-      } else if (
-        fullMatch.startsWith('<u>') &&
-        fullMatch.endsWith('</u>') &&
-        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 = (
-          <Text key={key} underline>
-            {fullMatch.slice(
-              UNDERLINE_TAG_START_LENGTH,
-              -UNDERLINE_TAG_END_LENGTH,
-            )}
-          </Text>
-        );
-      }
-    } catch (e) {
-      console.error('Error parsing inline markdown part:', fullMatch, e);
-      renderedNode = null;
-    }
-
-    nodes.push(renderedNode ?? <Text key={key}>{fullMatch}</Text>);
-    lastIndex = inlineRegex.lastIndex;
-  }
-
-  if (lastIndex < text.length) {
-    nodes.push(<Text key={`t-${lastIndex}`}>{text.slice(lastIndex)}</Text>);
-  }
-
-  return <>{nodes.filter((node) => node !== null)}</>;
-};
-
-const RenderInline = React.memo(RenderInlineInternal);
-
 interface RenderCodeBlockProps {
  content: string[];
  lang: string | null;
--- 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<TableRendererProps> = ({
  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<TableRendererProps> = ({
    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 (
-        <Text bold color={Colors.AccentCyan}>
-          {padded}
-        </Text>
-      );
-    }
-    return <Text>{padded}</Text>;
+    return (
+      <Text>
+        {isHeader ? (
+          <Text bold color={Colors.AccentCyan}>
+            <RenderInline text={cellContent} />
+          </Text>
+        ) : (
+          <RenderInline text={cellContent} />
+        )}
+        {' '.repeat(paddingNeeded)}
+      </Text>
+    );
  };

-  const renderRow = (cells: string[], isHeader = false) => (
-    <Box flexDirection="row">
-      <Text>│ </Text>
-      {cells.map((cell, index) => (
-        <React.Fragment key={index}>
-          {renderCell(cell, adjustedWidths[index] || 0, isHeader)}
-          <Text> │ </Text>
-        </React.Fragment>
-      ))}
-    </Box>
-  );
+  // 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 <Text>├─{separator}─┤</Text>;
+    const char = chars[type];
+    const borderParts = adjustedWidths.map((w) => char.horizontal.repeat(w));
+    const border = char.left + borderParts.join(char.middle) + char.right;
+
+    return <Text>{border}</Text>;
  };

-  const renderTopBorder = () => {
-    const border = adjustedWidths
-      .map((width) => '─'.repeat(Math.max(0, (width || 0) - 2)))
-      .join('─┬─');
-    return <Text>┌─{border}─┐</Text>;
-  };
+  // 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 <Text>└─{border}─┘</Text>;
+    return (
+      <Text>
+        │{' '}
+        {renderedCells.map((cell, index) => (
+          <React.Fragment key={index}>
+            {cell}
+            {index < renderedCells.length - 1 ? ' │ ' : ''}
+          </React.Fragment>
+        ))}{' '}
+        │
+      </Text>
+    );
  };

  return (
    <Box flexDirection="column" marginY={1}>
-      {renderTopBorder()}
+      {/* Top border */}
+      {renderBorder('top')}
+
+      {/* Header row */}
      {renderRow(headers, true)}
-      {renderSeparator()}
+
+      {/* Middle border */}
+      {renderBorder('middle')}
+
+      {/* Data rows */}
      {rows.map((row, index) => (
        <React.Fragment key={index}>{renderRow(row)}</React.Fragment>
      ))}
-      {renderBottomBorder()}
+
+      {/* Bottom border */}
+      {renderBorder('bottom')}
    </Box>
  );
 };