fix: preserve markdown tables in RichInput Source/Edit roundtrip#6441
fix: preserve markdown tables in RichInput Source/Edit roundtrip#6441mvanhorn wants to merge 1 commit into
Conversation
…ource/Edit roundtrip
There was a problem hiding this comment.
Code Review
This pull request introduces support for rendering tables in markdown by adding the @tiptap/extension-table dependency and implementing a custom MarkdownTable extension in both ExpandRichInputDialog.jsx and RichInput.jsx. The review feedback correctly identifies two issues with the custom markdown table serializer: it fails to escape pipe characters (|) inside table cells, which breaks the markdown structure, and it incorrectly handles tables without explicit header cells by rendering an empty header row. The reviewer has provided an actionable, simplified code suggestion for both files to resolve these issues by escaping pipes and falling back to treating the first row as the header.
| const MarkdownTable = Table.extend({ | ||
| renderMarkdown: (node, h) => { | ||
| const rows = | ||
| node.content?.map((rowNode) => | ||
| (rowNode.content || []).map((cellNode) => { | ||
| const text = (cellNode.content || []) | ||
| .map((childNode) => h.renderChildren(childNode)) | ||
| .join(' ') | ||
| .replace(/\s+/g, ' ') | ||
| .trim() | ||
| return { text, isHeader: cellNode.type === 'tableHeader' } | ||
| }) | ||
| ) || [] | ||
| const columnCount = rows.reduce((max, row) => Math.max(max, row.length), 0) | ||
|
|
||
| if (!columnCount) return '' | ||
|
|
||
| const renderRow = (row = []) => | ||
| `| ${new Array(columnCount) | ||
| .fill(0) | ||
| .map((_, index) => row[index]?.text || '') | ||
| .join(' | ')} |` | ||
|
|
||
| const headerRow = rows[0] || [] | ||
| const hasHeader = headerRow.some((cell) => cell.isHeader) | ||
| const bodyRows = hasHeader ? rows.slice(1) : rows | ||
|
|
||
| return [ | ||
| renderRow(hasHeader ? headerRow : []), | ||
| `| ${new Array(columnCount).fill('---').join(' | ')} |`, | ||
| ...bodyRows.map(renderRow) | ||
| ].join('\n') | ||
| } | ||
| }) |
There was a problem hiding this comment.
The current markdown table serializer has two issues:
- If a table cell contains a pipe character (
|), it will break the markdown table structure. We should escape|as\|inside the cell text. - If a table has no explicit header cells (
hasHeaderis false), the current logic renders an empty header row and shifts all actual data to the body. Since Markdown tables require a header row, we should fallback to treating the first row as the header row.
We can simplify the code and address both issues by removing the isHeader tracking and always treating the first row as the header row, which is the standard fallback for Markdown tables.
const MarkdownTable = Table.extend({
renderMarkdown: (node, h) => {
const rows =
node.content?.map((rowNode) =>
(rowNode.content || []).map((cellNode) =>
(cellNode.content || [])
.map((childNode) => h.renderChildren(childNode))
.join(' ')
.replace(/\s+/g, ' ')
.trim()
.replace(/\|/g, '\\|')
)
) || []
const columnCount = rows.reduce((max, row) => Math.max(max, row.length), 0)
if (!columnCount) return ''
const renderRow = (row = []) =>
'| ' + new Array(columnCount)
.fill(0)
.map((_, index) => row[index] || '')
.join(' | ') + ' |'
const headerRow = rows[0] || []
const bodyRows = rows.slice(1)
return [
renderRow(headerRow),
'| ' + new Array(columnCount).fill('---').join(' | ') + ' |',
...bodyRows.map(renderRow)
].join('\n')
}
})References
- When using a heuristic for detection, ensure a safe fallback mechanism is in place to correctly handle cases where the heuristic fails.
| const MarkdownTable = Table.extend({ | ||
| renderMarkdown: (node, h) => { | ||
| const rows = | ||
| node.content?.map((rowNode) => | ||
| (rowNode.content || []).map((cellNode) => { | ||
| const text = (cellNode.content || []) | ||
| .map((childNode) => h.renderChildren(childNode)) | ||
| .join(' ') | ||
| .replace(/\s+/g, ' ') | ||
| .trim() | ||
| return { text, isHeader: cellNode.type === 'tableHeader' } | ||
| }) | ||
| ) || [] | ||
| const columnCount = rows.reduce((max, row) => Math.max(max, row.length), 0) | ||
|
|
||
| if (!columnCount) return '' | ||
|
|
||
| const renderRow = (row = []) => | ||
| `| ${new Array(columnCount) | ||
| .fill(0) | ||
| .map((_, index) => row[index]?.text || '') | ||
| .join(' | ')} |` | ||
|
|
||
| const headerRow = rows[0] || [] | ||
| const hasHeader = headerRow.some((cell) => cell.isHeader) | ||
| const bodyRows = hasHeader ? rows.slice(1) : rows | ||
|
|
||
| return [ | ||
| renderRow(hasHeader ? headerRow : []), | ||
| `| ${new Array(columnCount).fill('---').join(' | ')} |`, | ||
| ...bodyRows.map(renderRow) | ||
| ].join('\n') | ||
| } | ||
| }) |
There was a problem hiding this comment.
The current markdown table serializer has two issues:
- If a table cell contains a pipe character (
|), it will break the markdown table structure. We should escape|as\|inside the cell text. - If a table has no explicit header cells (
hasHeaderis false), the current logic renders an empty header row and shifts all actual data to the body. Since Markdown tables require a header row, we should fallback to treating the first row as the header row.
We can simplify the code and address both issues by removing the isHeader tracking and always treating the first row as the header row, which is the standard fallback for Markdown tables.
const MarkdownTable = Table.extend({
renderMarkdown: (node, h) => {
const rows =
node.content?.map((rowNode) =>
(rowNode.content || []).map((cellNode) =>
(cellNode.content || [])
.map((childNode) => h.renderChildren(childNode))
.join(' ')
.replace(/\s+/g, ' ')
.trim()
.replace(/\|/g, '\\|')
)
) || []
const columnCount = rows.reduce((max, row) => Math.max(max, row.length), 0)
if (!columnCount) return ''
const renderRow = (row = []) =>
'| ' + new Array(columnCount)
.fill(0)
.map((_, index) => row[index] || '')
.join(' | ') + ' |'
const headerRow = rows[0] || []
const bodyRows = rows.slice(1)
return [
renderRow(headerRow),
'| ' + new Array(columnCount).fill('---').join(' | ') + ' |',
...bodyRows.map(renderRow)
].join('\n')
}
})References
- When using a heuristic for detection, ensure a safe fallback mechanism is in place to correctly handle cases where the heuristic fails.
Summary
fix: preserve markdown tables and inline-code escaping in RichInput Source/Edit roundtrip
Closes #6387
AI was used for assistance.