From 601a61ed31efb281e6bb396c5f15c491bfbca27d Mon Sep 17 00:00:00 2001 From: Taylor Mullen Date: Thu, 15 May 2025 21:49:26 -0700 Subject: [PATCH] Addressed code review comments --- packages/cli/src/ui/utils/MarkdownDisplay.tsx | 179 ++++++++++++------ 1 file changed, 126 insertions(+), 53 deletions(-) diff --git a/packages/cli/src/ui/utils/MarkdownDisplay.tsx b/packages/cli/src/ui/utils/MarkdownDisplay.tsx index 4e49a013..63888a8b 100644 --- a/packages/cli/src/ui/utils/MarkdownDisplay.tsx +++ b/packages/cli/src/ui/utils/MarkdownDisplay.tsx @@ -13,14 +13,25 @@ interface MarkdownDisplayProps { text: string; } -function MarkdownDisplayComponent({ - text, -}: MarkdownDisplayProps): React.ReactElement { +// 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; +const LIST_ITEM_PREFIX_PADDING = 1; +const LIST_ITEM_TEXT_FLEX_GROW = 1; + +const MarkdownDisplayInternal: React.FC = ({ text }) => { if (!text) return <>; const lines = text.split('\n'); const headerRegex = /^ *(#{1,4}) +(.*)/; - const codeFenceRegex = /^ *(`{3,}|~{3,}) *(\S*?) *$/; + const codeFenceRegex = /^ *(`{3,}|~{3,}) *(\w*?) *$/; const ulItemRegex = /^([ \t]*)([-*+]) +(.*)/; const olItemRegex = /^([ \t]*)(\d+)\. +(.*)/; const hrRegex = /^ *([-*_] *){3,} *$/; @@ -42,7 +53,11 @@ function MarkdownDisplayComponent({ fenceMatch[1].length >= codeBlockFence.length ) { contentBlocks.push( - _renderCodeBlock(key, codeBlockContent, codeBlockLang), + , ); inCodeBlock = false; codeBlockContent = []; @@ -73,35 +88,42 @@ function MarkdownDisplayComponent({ } else if (headerMatch) { const level = headerMatch[1].length; const headerText = headerMatch[2]; - const renderedHeaderText = _renderInline(headerText); let headerNode: React.ReactNode = null; switch (level) { case 1: headerNode = ( - {renderedHeaderText} + ); break; case 2: headerNode = ( - {renderedHeaderText} + ); break; case 3: - headerNode = {renderedHeaderText}; + headerNode = ( + + + + ); break; case 4: headerNode = ( - {renderedHeaderText} + ); break; default: - headerNode = {renderedHeaderText}; + headerNode = ( + + + + ); break; } if (headerNode) contentBlocks.push({headerNode}); @@ -110,43 +132,64 @@ function MarkdownDisplayComponent({ const marker = ulMatch[2]; const itemText = ulMatch[3]; contentBlocks.push( - _renderListItem(key, itemText, 'ul', marker, leadingWhitespace), + , ); } else if (olMatch) { const leadingWhitespace = olMatch[1]; const marker = olMatch[2]; const itemText = olMatch[3]; contentBlocks.push( - _renderListItem(key, itemText, 'ol', marker, leadingWhitespace), + , ); } else { - const renderedLine = _renderInline(line); - if (renderedLine.length > 0 || line.length > 0) { + if (line.trim().length === 0) { + if (contentBlocks.length > 0 && !inCodeBlock) { + contentBlocks.push(); + } + } else { contentBlocks.push( - {renderedLine} + + + , ); - } else if (line.trim().length === 0) { - if (contentBlocks.length > 0 && !inCodeBlock) { - contentBlocks.push(); - } } } }); if (inCodeBlock) { contentBlocks.push( - _renderCodeBlock(`line-eof`, codeBlockContent, codeBlockLang), + , ); } return <>{contentBlocks}; -} +}; // Helper functions (adapted from static methods of MarkdownRenderer) -function _renderInline(text: string): React.ReactNode[] { +interface RenderInlineProps { + text: string; +} + +const RenderInlineInternal: React.FC = ({ text }) => { const nodes: React.ReactNode[] = []; let lastIndex = 0; const inlineRegex = @@ -170,37 +213,40 @@ function _renderInline(text: string): React.ReactNode[] { if ( fullMatch.startsWith('**') && fullMatch.endsWith('**') && - fullMatch.length > 4 + fullMatch.length > BOLD_MARKER_LENGTH * 2 ) { renderedNode = ( - {fullMatch.slice(2, -2)} + {fullMatch.slice(BOLD_MARKER_LENGTH, -BOLD_MARKER_LENGTH)} ); } else if ( ((fullMatch.startsWith('*') && fullMatch.endsWith('*')) || (fullMatch.startsWith('_') && fullMatch.endsWith('_'))) && - fullMatch.length > 2 + fullMatch.length > ITALIC_MARKER_LENGTH * 2 ) { renderedNode = ( - {fullMatch.slice(1, -1)} + {fullMatch.slice(ITALIC_MARKER_LENGTH, -ITALIC_MARKER_LENGTH)} ); } else if ( fullMatch.startsWith('~~') && fullMatch.endsWith('~~') && - fullMatch.length > 4 + fullMatch.length > STRIKETHROUGH_MARKER_LENGTH * 2 ) { renderedNode = ( - {fullMatch.slice(2, -2)} + {fullMatch.slice( + STRIKETHROUGH_MARKER_LENGTH, + -STRIKETHROUGH_MARKER_LENGTH, + )} ); } else if ( fullMatch.startsWith('`') && fullMatch.endsWith('`') && - fullMatch.length > 1 + fullMatch.length > INLINE_CODE_MARKER_LENGTH ) { const codeMatch = fullMatch.match(/^(`+)(.+?)\1$/s); if (codeMatch && codeMatch[2]) { @@ -212,7 +258,10 @@ function _renderInline(text: string): React.ReactNode[] { } else { renderedNode = ( - {fullMatch.slice(1, -1)} + {fullMatch.slice( + INLINE_CODE_MARKER_LENGTH, + -INLINE_CODE_MARKER_LENGTH, + )} ); } @@ -235,11 +284,15 @@ function _renderInline(text: string): React.ReactNode[] { } else if ( fullMatch.startsWith('') && fullMatch.endsWith('') && - fullMatch.length > 6 + 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(3, -4)} + {fullMatch.slice( + UNDERLINE_TAG_START_LENGTH, + -UNDERLINE_TAG_END_LENGTH, + )} ); } @@ -256,46 +309,66 @@ function _renderInline(text: string): React.ReactNode[] { nodes.push({text.slice(lastIndex)}); } - return nodes.filter((node) => node !== null); + return <>{nodes.filter((node) => node !== null)}; +}; + +const RenderInline = React.memo(RenderInlineInternal); + +interface RenderCodeBlockProps { + content: string[]; + lang: string | null; } -function _renderCodeBlock( - key: string, - content: string[], - lang: string | null, -): React.ReactNode { +const RenderCodeBlockInternal: React.FC = ({ + content, + lang, +}) => { const fullContent = content.join('\n'); const colorizedCode = colorizeCode(fullContent, lang); return ( - + {colorizedCode} ); +}; + +const RenderCodeBlock = React.memo(RenderCodeBlockInternal); + +interface RenderListItemProps { + itemText: string; + type: 'ul' | 'ol'; + marker: string; + leadingWhitespace?: string; } -function _renderListItem( - key: string, - text: string, - type: 'ul' | 'ol', - marker: string, - leadingWhitespace: string = '', -): React.ReactNode { - const renderedText = _renderInline(text); +const RenderListItemInternal: React.FC = ({ + itemText, + type, + marker, + leadingWhitespace = '', +}) => { const prefix = type === 'ol' ? `${marker}. ` : `${marker} `; const prefixWidth = prefix.length; const indentation = leadingWhitespace.length; return ( - + {prefix} - - {renderedText} + + + + ); -} +}; -export const MarkdownDisplay = React.memo(MarkdownDisplayComponent); +const RenderListItem = React.memo(RenderListItemInternal); + +export const MarkdownDisplay = React.memo(MarkdownDisplayInternal);