diff --git a/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx b/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx index 9e0cb96c..312c1b5b 100644 --- a/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx +++ b/packages/cli/src/ui/utils/MarkdownDisplay.test.tsx @@ -5,323 +5,172 @@ */ import { render } from 'ink-testing-library'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; 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 | -`; +describe('', () => { + const baseProps = { + isPending: false, + terminalWidth: 80, + availableTerminalHeight: 40, + }; - 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(''); - }); - - 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'); - }); + beforeEach(() => { + vi.clearAllMocks(); }); - describe('Existing Functionality', () => { - it('should render headers correctly', () => { - const headerMarkdown = ` -# H1 Header -## H2 Header -### H3 Header -#### H4 Header + it('renders nothing for empty text', () => { + const { lastFrame } = render(); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('renders a simple paragraph', () => { + const text = 'Hello, world.'; + const { lastFrame } = render( + , + ); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('renders headers with correct levels', () => { + const text = ` +# Header 1 +## Header 2 +### Header 3 +#### Header 4 `; + const { lastFrame } = render( + , + ); + expect(lastFrame()).toMatchSnapshot(); + }); - const { lastFrame } = render( - , - ); + it('renders a fenced code block with a language', () => { + const text = '```javascript\nconst x = 1;\nconsole.log(x);\n```'; + const { lastFrame } = render( + , + ); + expect(lastFrame()).toMatchSnapshot(); + }); - expect(lastFrame()).toContain('H1 Header'); - expect(lastFrame()).toContain('H2 Header'); - expect(lastFrame()).toContain('H3 Header'); - expect(lastFrame()).toContain('H4 Header'); - }); + it('renders a fenced code block without a language', () => { + const text = '```\nplain text\n```'; + const { lastFrame } = render( + , + ); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('handles unclosed (pending) code blocks', () => { + const text = '```typescript\nlet y = 2;'; + const { lastFrame } = render( + , + ); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('renders unordered lists with different markers', () => { + const text = ` +- item A +* item B ++ item C +`; + const { lastFrame } = render( + , + ); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('renders nested unordered lists', () => { + const text = ` +* Level 1 + * Level 2 + * Level 3 +`; + const { lastFrame } = render( + , + ); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('renders ordered lists', () => { + const text = ` +1. First item +2. Second item +`; + const { lastFrame } = render( + , + ); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('renders horizontal rules', () => { + const text = ` +Hello +--- +World +*** +Test +`; + const { lastFrame } = render( + , + ); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('renders tables correctly', () => { + const text = ` +| Header 1 | Header 2 | +|----------|:--------:| +| Cell 1 | Cell 2 | +| Cell 3 | Cell 4 | +`; + const { lastFrame } = render( + , + ); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('handles a table at the end of the input', () => { + const text = ` +Some text before. +| A | B | +|---| +| 1 | 2 |`; + const { lastFrame } = render( + , + ); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('inserts a single space between paragraphs', () => { + const text = `Paragraph 1. + +Paragraph 2.`; + const { lastFrame } = render( + , + ); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('correctly parses a mix of markdown elements', () => { + const text = ` +# Main Title + +Here is a paragraph. + +- List item 1 +- List item 2 - it('should render code blocks correctly', () => { - const codeMarkdown = ` -\`\`\`javascript -const x = 42; -console.log(x); \`\`\` +some code +\`\`\` + +Another paragraph. `; - - const { lastFrame } = render( - , - ); - - expect(lastFrame()).toContain('const x = 42;'); - expect(lastFrame()).toContain('console.log(x);'); - }); + const { lastFrame } = render( + , + ); + expect(lastFrame()).toMatchSnapshot(); }); }); diff --git a/packages/cli/src/ui/utils/MarkdownDisplay.tsx b/packages/cli/src/ui/utils/MarkdownDisplay.tsx index 572ae12e..eb5dc453 100644 --- a/packages/cli/src/ui/utils/MarkdownDisplay.tsx +++ b/packages/cli/src/ui/utils/MarkdownDisplay.tsx @@ -21,7 +21,7 @@ interface MarkdownDisplayProps { // Constants for Markdown parsing and rendering const EMPTY_LINE_HEIGHT = 1; -const CODE_BLOCK_PADDING = 1; +const CODE_BLOCK_PREFIX_PADDING = 1; const LIST_ITEM_PREFIX_PADDING = 1; const LIST_ITEM_TEXT_FLEX_GROW = 1; @@ -44,6 +44,7 @@ const MarkdownDisplayInternal: React.FC = ({ const contentBlocks: React.ReactNode[] = []; let inCodeBlock = false; + let lastLineEmpty = true; let codeBlockContent: string[] = []; let codeBlockLang: string | null = null; let codeBlockFence = ''; @@ -51,6 +52,13 @@ const MarkdownDisplayInternal: React.FC = ({ let tableRows: string[][] = []; let tableHeaders: string[] = []; + function addContentBlock(block: React.ReactNode) { + if (block) { + contentBlocks.push(block); + lastLineEmpty = false; + } + } + lines.forEach((line, index) => { const key = `line-${index}`; @@ -61,7 +69,7 @@ const MarkdownDisplayInternal: React.FC = ({ fenceMatch[1].startsWith(codeBlockFence[0]) && fenceMatch[1].length >= codeBlockFence.length ) { - contentBlocks.push( + addContentBlock( = ({ tableRows = []; } else { // Not a table, treat as regular text - contentBlocks.push( + addContentBlock( @@ -128,7 +136,7 @@ const MarkdownDisplayInternal: React.FC = ({ } else if (inTable && !tableRowMatch) { // End of table if (tableHeaders.length > 0 && tableRows.length > 0) { - contentBlocks.push( + addContentBlock( = ({ // Process current line as normal if (line.trim().length > 0) { - contentBlocks.push( + addContentBlock( @@ -152,7 +160,7 @@ const MarkdownDisplayInternal: React.FC = ({ ); } } else if (hrMatch) { - contentBlocks.push( + addContentBlock( --- , @@ -198,12 +206,12 @@ const MarkdownDisplayInternal: React.FC = ({ ); break; } - if (headerNode) contentBlocks.push({headerNode}); + if (headerNode) addContentBlock({headerNode}); } else if (ulMatch) { const leadingWhitespace = ulMatch[1]; const marker = ulMatch[2]; const itemText = ulMatch[3]; - contentBlocks.push( + addContentBlock( = ({ const leadingWhitespace = olMatch[1]; const marker = olMatch[2]; const itemText = olMatch[3]; - contentBlocks.push( + addContentBlock( = ({ />, ); } else { - if (line.trim().length === 0) { - if (contentBlocks.length > 0 && !inCodeBlock) { - contentBlocks.push(); + if (line.trim().length === 0 && !inCodeBlock) { + if (!lastLineEmpty) { + contentBlocks.push( + , + ); + lastLineEmpty = true; } } else { - contentBlocks.push( + addContentBlock( @@ -243,7 +254,7 @@ const MarkdownDisplayInternal: React.FC = ({ }); if (inCodeBlock) { - contentBlocks.push( + addContentBlock( = ({ // Handle table at end of content if (inTable && tableHeaders.length > 0 && tableRows.length > 0) { - contentBlocks.push( + addContentBlock( = ({ if (isPending && availableTerminalHeight !== undefined) { const MAX_CODE_LINES_WHEN_PENDING = Math.max( 0, - availableTerminalHeight - CODE_BLOCK_PADDING * 2 - RESERVED_LINES, + availableTerminalHeight - RESERVED_LINES, ); if (content.length > MAX_CODE_LINES_WHEN_PENDING) { if (MAX_CODE_LINES_WHEN_PENDING < MIN_LINES_FOR_MESSAGE) { // Not enough space to even show the message meaningfully return ( - + ... code is being written ... ); @@ -310,10 +321,10 @@ const RenderCodeBlockInternal: React.FC = ({ truncatedContent.join('\n'), lang, availableTerminalHeight, - terminalWidth - CODE_BLOCK_PADDING * 2, + terminalWidth - CODE_BLOCK_PREFIX_PADDING, ); return ( - + {colorizedTruncatedCode} ... generating more ... @@ -326,13 +337,13 @@ const RenderCodeBlockInternal: React.FC = ({ fullContent, lang, availableTerminalHeight, - terminalWidth - CODE_BLOCK_PADDING * 2, + terminalWidth - CODE_BLOCK_PREFIX_PADDING, ); return ( diff --git a/packages/cli/src/ui/utils/__snapshots__/MarkdownDisplay.test.tsx.snap b/packages/cli/src/ui/utils/__snapshots__/MarkdownDisplay.test.tsx.snap new file mode 100644 index 00000000..66436bbd --- /dev/null +++ b/packages/cli/src/ui/utils/__snapshots__/MarkdownDisplay.test.tsx.snap @@ -0,0 +1,89 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[` > correctly parses a mix of markdown elements 1`] = ` +"Main Title + +Here is a paragraph. + + - List item 1 + - List item 2 + + 1 some code + +Another paragraph. +" +`; + +exports[` > handles a table at the end of the input 1`] = ` +"Some text before. +| A | B | +|---| +| 1 | 2 |" +`; + +exports[` > handles unclosed (pending) code blocks 1`] = `" 1 let y = 2;"`; + +exports[` > inserts a single space between paragraphs 1`] = ` +"Paragraph 1. + +Paragraph 2." +`; + +exports[` > renders a fenced code block with a language 1`] = ` +" 1 const x = 1; + 2 console.log(x);" +`; + +exports[` > renders a fenced code block without a language 1`] = `" 1 plain text"`; + +exports[` > renders a simple paragraph 1`] = `"Hello, world."`; + +exports[` > renders headers with correct levels 1`] = ` +"Header 1 +Header 2 +Header 3 +Header 4 +" +`; + +exports[` > renders horizontal rules 1`] = ` +"Hello +--- +World +--- +Test +" +`; + +exports[` > renders nested unordered lists 1`] = ` +" * Level 1 + * Level 2 + * Level 3 +" +`; + +exports[` > renders nothing for empty text 1`] = `""`; + +exports[` > renders ordered lists 1`] = ` +" 1. First item + 2. Second item +" +`; + +exports[` > renders tables correctly 1`] = ` +" +┌──────────┬──────────┐ +│ Header 1 │ Header 2 │ +├──────────┼──────────┤ +│ Cell 1 │ Cell 2 │ +│ Cell 3 │ Cell 4 │ +└──────────┴──────────┘ +" +`; + +exports[` > renders unordered lists with different markers 1`] = ` +" - item A + * item B + + item C +" +`;