Fix extra whitespace in markdown rendering. (#3943)

Co-authored-by: Scott Densmore <scottdensmore@mac.com>
This commit is contained in:
Jacob Richman 2025-07-11 20:38:07 -07:00 committed by GitHub
parent d89ccf2250
commit c4ea17692f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 277 additions and 328 deletions

View File

@ -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('<MarkdownDisplay />', () => {
const baseProps = {
isPending: false,
terminalWidth: 80,
availableTerminalHeight: 40,
};
beforeEach(() => {
vi.clearAllMocks();
});
it('renders nothing for empty text', () => {
const { lastFrame } = render(<MarkdownDisplay {...baseProps} text="" />);
expect(lastFrame()).toMatchSnapshot();
});
it('renders a simple paragraph', () => {
const text = 'Hello, world.';
const { lastFrame } = render(
<MarkdownDisplay
text={tableMarkdown}
isPending={false}
terminalWidth={80}
/>,
<MarkdownDisplay {...baseProps} text={text} />,
);
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');
expect(lastFrame()).toMatchSnapshot();
});
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 |
it('renders headers with correct levels', () => {
const text = `
# Header 1
## Header 2
### Header 3
#### Header 4
`;
const { lastFrame } = render(
<MarkdownDisplay
text={tableMarkdown}
isPending={false}
terminalWidth={80}
/>,
<MarkdownDisplay {...baseProps} text={text} />,
);
expect(lastFrame()).toContain('Short');
expect(lastFrame()).toContain('Medium Column');
expect(lastFrame()).toContain('Very Long Column Name');
expect(lastFrame()).toMatchSnapshot();
});
it('should handle empty cells in tables', () => {
const tableMarkdown = `
| Col1 | Col2 | Col3 |
|------|------|------|
| A | | C |
| | B | |
`;
it('renders a fenced code block with a language', () => {
const text = '```javascript\nconst x = 1;\nconsole.log(x);\n```';
const { lastFrame } = render(
<MarkdownDisplay
text={tableMarkdown}
isPending={false}
terminalWidth={80}
/>,
<MarkdownDisplay {...baseProps} text={text} />,
);
expect(lastFrame()).toContain('Col1');
expect(lastFrame()).toContain('Col2');
expect(lastFrame()).toContain('Col3');
expect(lastFrame()).toContain('A');
expect(lastFrame()).toContain('B');
expect(lastFrame()).toContain('C');
expect(lastFrame()).toMatchSnapshot();
});
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.
`;
it('renders a fenced code block without a language', () => {
const text = '```\nplain text\n```';
const { lastFrame } = render(
<MarkdownDisplay
text={mixedMarkdown}
isPending={false}
terminalWidth={80}
/>,
<MarkdownDisplay {...baseProps} text={text} />,
);
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.');
expect(lastFrame()).toMatchSnapshot();
});
it('should handle tables with empty cells at edges', () => {
const tableMarkdown = `
| | Middle | |
|-|--------|-|
| | Value | |
`;
it('handles unclosed (pending) code blocks', () => {
const text = '```typescript\nlet y = 2;';
const { lastFrame } = render(
<MarkdownDisplay
text={tableMarkdown}
isPending={false}
terminalWidth={80}
/>,
<MarkdownDisplay {...baseProps} text={text} isPending={true} />,
);
expect(lastFrame()).toContain('Middle');
expect(lastFrame()).toContain('Value');
// Should maintain column structure even with empty edge cells
expect(lastFrame()).toMatchSnapshot();
});
it('should handle PR reviewer test case 1', () => {
const tableMarkdown = `
| Package | Lines of Code |
|---------|---------------|
| CLI | 18407 |
| Core | 14445 |
it('renders unordered lists with different markers', () => {
const text = `
- item A
* item B
+ item C
`;
const { lastFrame } = render(
<MarkdownDisplay
text={tableMarkdown}
isPending={false}
terminalWidth={80}
/>,
<MarkdownDisplay {...baseProps} text={text} />,
);
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');
expect(lastFrame()).toMatchSnapshot();
});
it('should handle PR reviewer test case 2 - long table', () => {
const tableMarkdown = `
| Letter | Count |
|--------|-------|
| a | 15 |
| b | 2 |
| c | 26 |
| Total | 283 |
it('renders nested unordered lists', () => {
const text = `
* Level 1
* Level 2
* Level 3
`;
const { lastFrame } = render(
<MarkdownDisplay
text={tableMarkdown}
isPending={false}
terminalWidth={80}
/>,
<MarkdownDisplay {...baseProps} text={text} />,
);
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');
expect(lastFrame()).toMatchSnapshot();
});
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 |
it('renders ordered lists', () => {
const text = `
1. First item
2. Second item
`;
const { lastFrame } = render(
<MarkdownDisplay
text={malformedMarkdown}
isPending={false}
terminalWidth={80}
/>,
<MarkdownDisplay {...baseProps} text={text} />,
);
// 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 |");
expect(lastFrame()).toMatchSnapshot();
});
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)} |
it('renders horizontal rules', () => {
const text = `
Hello
---
World
***
Test
`;
const renderNarrow = () =>
render(
<MarkdownDisplay
text={wideTable}
isPending={false}
terminalWidth={40}
/>,
);
// 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(
<MarkdownDisplay
text={testContent}
isPending={false}
terminalWidth={120}
/>,
<MarkdownDisplay {...baseProps} text={text} />,
);
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');
});
expect(lastFrame()).toMatchSnapshot();
});
describe('Existing Functionality', () => {
it('should render headers correctly', () => {
const headerMarkdown = `
# H1 Header
## H2 Header
### H3 Header
#### H4 Header
it('renders tables correctly', () => {
const text = `
| Header 1 | Header 2 |
|----------|:--------:|
| Cell 1 | Cell 2 |
| Cell 3 | Cell 4 |
`;
const { lastFrame } = render(
<MarkdownDisplay
text={headerMarkdown}
isPending={false}
terminalWidth={80}
/>,
<MarkdownDisplay {...baseProps} text={text} />,
);
expect(lastFrame()).toContain('H1 Header');
expect(lastFrame()).toContain('H2 Header');
expect(lastFrame()).toContain('H3 Header');
expect(lastFrame()).toContain('H4 Header');
expect(lastFrame()).toMatchSnapshot();
});
it('should render code blocks correctly', () => {
const codeMarkdown = `
\`\`\`javascript
const x = 42;
console.log(x);
it('handles a table at the end of the input', () => {
const text = `
Some text before.
| A | B |
|---|
| 1 | 2 |`;
const { lastFrame } = render(
<MarkdownDisplay {...baseProps} text={text} />,
);
expect(lastFrame()).toMatchSnapshot();
});
it('inserts a single space between paragraphs', () => {
const text = `Paragraph 1.
Paragraph 2.`;
const { lastFrame } = render(
<MarkdownDisplay {...baseProps} text={text} />,
);
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
\`\`\`
some code
\`\`\`
Another paragraph.
`;
const { lastFrame } = render(
<MarkdownDisplay
text={codeMarkdown}
isPending={false}
terminalWidth={80}
/>,
<MarkdownDisplay {...baseProps} text={text} />,
);
expect(lastFrame()).toContain('const x = 42;');
expect(lastFrame()).toContain('console.log(x);');
});
expect(lastFrame()).toMatchSnapshot();
});
});

View File

@ -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<MarkdownDisplayProps> = ({
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<MarkdownDisplayProps> = ({
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<MarkdownDisplayProps> = ({
fenceMatch[1].startsWith(codeBlockFence[0]) &&
fenceMatch[1].length >= codeBlockFence.length
) {
contentBlocks.push(
addContentBlock(
<RenderCodeBlock
key={key}
content={codeBlockContent}
@ -104,7 +112,7 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({
tableRows = [];
} else {
// Not a table, treat as regular text
contentBlocks.push(
addContentBlock(
<Box key={key}>
<Text wrap="wrap">
<RenderInline text={line} />
@ -128,7 +136,7 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({
} else if (inTable && !tableRowMatch) {
// End of table
if (tableHeaders.length > 0 && tableRows.length > 0) {
contentBlocks.push(
addContentBlock(
<RenderTable
key={`table-${contentBlocks.length}`}
headers={tableHeaders}
@ -143,7 +151,7 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({
// Process current line as normal
if (line.trim().length > 0) {
contentBlocks.push(
addContentBlock(
<Box key={key}>
<Text wrap="wrap">
<RenderInline text={line} />
@ -152,7 +160,7 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({
);
}
} else if (hrMatch) {
contentBlocks.push(
addContentBlock(
<Box key={key}>
<Text dimColor>---</Text>
</Box>,
@ -198,12 +206,12 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({
);
break;
}
if (headerNode) contentBlocks.push(<Box key={key}>{headerNode}</Box>);
if (headerNode) addContentBlock(<Box key={key}>{headerNode}</Box>);
} else if (ulMatch) {
const leadingWhitespace = ulMatch[1];
const marker = ulMatch[2];
const itemText = ulMatch[3];
contentBlocks.push(
addContentBlock(
<RenderListItem
key={key}
itemText={itemText}
@ -216,7 +224,7 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({
const leadingWhitespace = olMatch[1];
const marker = olMatch[2];
const itemText = olMatch[3];
contentBlocks.push(
addContentBlock(
<RenderListItem
key={key}
itemText={itemText}
@ -226,12 +234,15 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({
/>,
);
} else {
if (line.trim().length === 0) {
if (contentBlocks.length > 0 && !inCodeBlock) {
contentBlocks.push(<Box key={key} height={EMPTY_LINE_HEIGHT} />);
if (line.trim().length === 0 && !inCodeBlock) {
if (!lastLineEmpty) {
contentBlocks.push(
<Box key={`spacer-${index}`} height={EMPTY_LINE_HEIGHT} />,
);
lastLineEmpty = true;
}
} else {
contentBlocks.push(
addContentBlock(
<Box key={key}>
<Text wrap="wrap">
<RenderInline text={line} />
@ -243,7 +254,7 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({
});
if (inCodeBlock) {
contentBlocks.push(
addContentBlock(
<RenderCodeBlock
key="line-eof"
content={codeBlockContent}
@ -257,7 +268,7 @@ const MarkdownDisplayInternal: React.FC<MarkdownDisplayProps> = ({
// Handle table at end of content
if (inTable && tableHeaders.length > 0 && tableRows.length > 0) {
contentBlocks.push(
addContentBlock(
<RenderTable
key={`table-${contentBlocks.length}`}
headers={tableHeaders}
@ -293,14 +304,14 @@ const RenderCodeBlockInternal: React.FC<RenderCodeBlockProps> = ({
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 (
<Box padding={CODE_BLOCK_PADDING}>
<Box paddingLeft={CODE_BLOCK_PREFIX_PADDING}>
<Text color={Colors.Gray}>... code is being written ...</Text>
</Box>
);
@ -310,10 +321,10 @@ const RenderCodeBlockInternal: React.FC<RenderCodeBlockProps> = ({
truncatedContent.join('\n'),
lang,
availableTerminalHeight,
terminalWidth - CODE_BLOCK_PADDING * 2,
terminalWidth - CODE_BLOCK_PREFIX_PADDING,
);
return (
<Box flexDirection="column" padding={CODE_BLOCK_PADDING}>
<Box paddingLeft={CODE_BLOCK_PREFIX_PADDING} flexDirection="column">
{colorizedTruncatedCode}
<Text color={Colors.Gray}>... generating more ...</Text>
</Box>
@ -326,13 +337,13 @@ const RenderCodeBlockInternal: React.FC<RenderCodeBlockProps> = ({
fullContent,
lang,
availableTerminalHeight,
terminalWidth - CODE_BLOCK_PADDING * 2,
terminalWidth - CODE_BLOCK_PREFIX_PADDING,
);
return (
<Box
paddingLeft={CODE_BLOCK_PREFIX_PADDING}
flexDirection="column"
padding={CODE_BLOCK_PADDING}
width={terminalWidth}
flexShrink={0}
>

View File

@ -0,0 +1,89 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
exports[`<MarkdownDisplay /> > 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[`<MarkdownDisplay /> > handles a table at the end of the input 1`] = `
"Some text before.
| A | B |
|---|
| 1 | 2 |"
`;
exports[`<MarkdownDisplay /> > handles unclosed (pending) code blocks 1`] = `" 1 let y = 2;"`;
exports[`<MarkdownDisplay /> > inserts a single space between paragraphs 1`] = `
"Paragraph 1.
Paragraph 2."
`;
exports[`<MarkdownDisplay /> > renders a fenced code block with a language 1`] = `
" 1 const x = 1;
2 console.log(x);"
`;
exports[`<MarkdownDisplay /> > renders a fenced code block without a language 1`] = `" 1 plain text"`;
exports[`<MarkdownDisplay /> > renders a simple paragraph 1`] = `"Hello, world."`;
exports[`<MarkdownDisplay /> > renders headers with correct levels 1`] = `
"Header 1
Header 2
Header 3
Header 4
"
`;
exports[`<MarkdownDisplay /> > renders horizontal rules 1`] = `
"Hello
---
World
---
Test
"
`;
exports[`<MarkdownDisplay /> > renders nested unordered lists 1`] = `
" * Level 1
* Level 2
* Level 3
"
`;
exports[`<MarkdownDisplay /> > renders nothing for empty text 1`] = `""`;
exports[`<MarkdownDisplay /> > renders ordered lists 1`] = `
" 1. First item
2. Second item
"
`;
exports[`<MarkdownDisplay /> > renders tables correctly 1`] = `
"
┌──────────┬──────────┐
│ Header 1 │ Header 2 │
├──────────┼──────────┤
│ Cell 1 │ Cell 2 │
│ Cell 3 │ Cell 4 │
└──────────┴──────────┘
"
`;
exports[`<MarkdownDisplay /> > renders unordered lists with different markers 1`] = `
" - item A
* item B
+ item C
"
`;