Restore the lost blank line before Markdown tables - #648
Merged
Conversation
Every top-level block gets a <p><br></p> separator when the Markdown put a blank line before it — except tables. Goldmark's table extension builds the Table node inside a paragraph transformer that replaces the source paragraph without carrying its blank-previous-lines flag over, so the flag is always false and "Intro.\n\n| a | b |..." rendered the table jammed against the paragraph while every other block kind kept its spacing (the separator after the table was unaffected: the next block keeps its own flag). Recover the answer from the source. The table's Pos is the start of the replaced paragraph's first line, so the table was blank-line-separated exactly when the line above that position is blank. The one case the false flag is right about — a table interrupting a paragraph mid-flight, where the transformer leaves the leading lines behind as a paragraph — is detected by that leftover paragraph sharing the table's Pos, and stays attached. The issue-#405 fixture encoded the old flag-loss behavior (heading, blank line, table, no separator); it now expects the same separator every other block gets.
There was a problem hiding this comment.
Pull request overview
Restores Markdown table spacing while preserving paragraph-interrupting tables.
Changes:
- Recovers lost table separator metadata from source positions.
- Adds table separator and CRLF regression coverage.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
internal/richtext/richtext.go |
Recovers blank-line detection for tables. |
internal/richtext/richtext_test.go |
Adds table separator regression tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+261
to
+273
| i := pos - 1 | ||
| if i >= 0 && source[i] == '\n' { | ||
| i-- | ||
| } | ||
| if i >= 0 && source[i] == '\r' { | ||
| i-- | ||
| } | ||
| for ; i >= 0 && source[i] != '\n'; i-- { | ||
| if c := source[i]; c != ' ' && c != '\t' && c != '\r' { | ||
| return false | ||
| } | ||
| } | ||
| return true |
This was referenced Aug 22, 2026
jeremy
added a commit
that referenced
this pull request
Aug 22, 2026
HTMLToMarkdown had no table handling: <table> markup survived every pass untouched until the final tag-stripping regex deleted the tags and ran the cell text together. Display of any table-bearing message, comment, or to-do description came out as one smeared line, and the TUI in-place editors had to refuse table-bearing content wholesale to avoid destroying it on resubmit. Convert tables to GFM pipe tables instead. A new pass runs while row and cell tags are still intact: each <table> block is extracted (BC3 rich text is sanitized editor output — always flat editor-authored grids, so a non-greedy block match is safe), rows and cells are pulled out by regex, and the block is emitted as a pipe table. The first row is the header whether its cells are <th> or <td> (GFM has no headerless tables); align attributes — exactly what MarkdownToHTML emits for GFM column alignment — map back to :--- / :---: / ---: markers; the widest row sizes the table, with narrower rows padded, so no row is ever truncated. Cell content runs through the same inline conversions as body text (bold, italic, code, links, strikethrough, mentions, attachments) and block boundaries collapse to spaces. Escaping is GFM-exact: pipes escape as \| and literal backslashes double — GFM processes escapes left to right, so a lone \ before an escaped pipe would swallow its backslash and turn the pipe back into a delimiter. Code spans pass through as placeholders since backslashes are literal inside code; only their pipes are escaped. A goldmark-backed test proves the parsed cell content survives the full HTML → Markdown → HTML cycle (backslash-pipe text, code spans with pipes and backslashes, pipes in link destinations, adjacent tables), and pipe tables round-trip byte-identical through MarkdownToHTML → HTMLToMarkdown, alignment and the #648 blank-line separator included. Shapes GFM can't represent still display, best-effort — colspan/rowspan cells emit as ordinary cells, a merged grid displaying better flattened than smeared — but editing them stays blocked: the blanket HasTableHTML gate on the three TUI in-place editors is replaced by HasComplexTableHTML, which fails closed on merged cells, captions, nested tables, attachments/images or block elements inside the table, cells spanning multiple paragraphs or lines, a table nested in a blockquote or list, and unclosed (unparseable) tables — checking every table in the content, not just the first. Mentions are the one rich element cells may keep: they convert to **@name** exactly as they do in body text. Simple grids open for editing like any other content. The mention-conversion closure moves to a named mentionMarkdown function so cell conversion can reuse it; behavior is unchanged.
jeremy
added a commit
that referenced
this pull request
Aug 22, 2026
HTMLToMarkdown had no table handling: <table> markup survived every pass untouched until the final tag-stripping regex deleted the tags and ran the cell text together. Display of any table-bearing message, comment, or to-do description came out as one smeared line, and the TUI in-place editors had to refuse table-bearing content wholesale to avoid destroying it on resubmit. Convert tables to GFM pipe tables instead. A new pass runs while row and cell tags are still intact: each <table> block is extracted (BC3 rich text is sanitized editor output — always flat editor-authored grids, so a non-greedy block match is safe), rows and cells are pulled out by regex, and the block is emitted as a pipe table. The first row is the header whether its cells are <th> or <td> (GFM has no headerless tables); align attributes — exactly what MarkdownToHTML emits for GFM column alignment — map back to :--- / :---: / ---: markers; the widest row sizes the table, with narrower rows padded, so no row is ever truncated. Cell content runs through the same inline conversions as body text (bold, italic, code, links, strikethrough, mentions, attachments) and block boundaries collapse to spaces. Escaping is GFM-exact: pipes escape as \| and literal backslashes double — GFM processes escapes left to right, so a lone \ before an escaped pipe would swallow its backslash and turn the pipe back into a delimiter. Code spans pass through as placeholders since backslashes are literal inside code; only their pipes are escaped. A goldmark-backed test proves the parsed cell content survives the full HTML → Markdown → HTML cycle (backslash-pipe text, code spans with pipes and backslashes, pipes in link destinations, adjacent tables), and pipe tables round-trip byte-identical through MarkdownToHTML → HTMLToMarkdown, alignment and the #648 blank-line separator included. Shapes GFM can't represent still display, best-effort — colspan/rowspan cells emit as ordinary cells, a merged grid displaying better flattened than smeared — but editing them stays blocked: the blanket HasTableHTML gate on the three TUI in-place editors is replaced by HasComplexTableHTML, which fails closed on merged cells, captions, nested tables, attachments/images or block elements inside the table, cells spanning multiple paragraphs or lines, a table nested in a blockquote or list, and unclosed (unparseable) tables — checking every table in the content, not just the first. Mentions are the one rich element cells may keep: they convert to **@name** exactly as they do in body text. Simple grids open for editing like any other content. The mention-conversion closure moves to a named mentionMarkdown function so cell conversion can reuse it; behavior is unchanged.
jeremy
added a commit
that referenced
this pull request
Aug 22, 2026
HTMLToMarkdown had no table handling: <table> markup survived every pass untouched until the final tag-stripping regex deleted the tags and ran the cell text together. Display of any table-bearing message, comment, or to-do description came out as one smeared line, and the TUI in-place editors had to refuse table-bearing content wholesale to avoid destroying it on resubmit. Convert tables to GFM pipe tables instead. A new pass runs while row and cell tags are still intact: each <table> block is extracted (BC3 rich text is sanitized editor output — always flat editor-authored grids, so a non-greedy block match is safe), rows and cells are pulled out by regex, and the block is emitted as a pipe table. The first row is the header whether its cells are <th> or <td> (GFM has no headerless tables); align attributes — exactly what MarkdownToHTML emits for GFM column alignment — map back to :--- / :---: / ---: markers; the widest row sizes the table, with narrower rows padded, so no row is ever truncated. Cell content runs through the same inline conversions as body text (bold, italic, code, links, strikethrough, mentions, attachments) and block boundaries collapse to spaces. Escaping is GFM-exact: pipes escape as \| and literal backslashes double — GFM processes escapes left to right, so a lone \ before an escaped pipe would swallow its backslash and turn the pipe back into a delimiter. Code spans pass through as placeholders since backslashes are literal inside code; only their pipes are escaped. Cell entities are fully decoded before escaping — an encoded pipe is still a pipe, and goldmark would decode | on the next render — and the emitted tables are parked behind placeholders until HTMLToMarkdown's document-level unescape pass has run, so nothing double-decodes. A goldmark-backed test proves the parsed cell content survives the full HTML → Markdown → HTML cycle (backslash-pipe text, code spans with pipes and backslashes, entity-encoded pipes and backslashes in text and code, pipes in link destinations, adjacent tables), and pipe tables round-trip byte-identical through MarkdownToHTML → HTMLToMarkdown, alignment and the #648 blank-line separator included. Shapes GFM can't represent still display, best-effort — colspan/rowspan cells emit as ordinary cells, a merged grid displaying better flattened than smeared — but editing them stays blocked: the blanket HasTableHTML gate on the three TUI in-place editors is replaced by HasComplexTableHTML, which fails closed on merged cells, captions, nested tables, attachments/images or block elements inside the table, cells spanning multiple paragraphs or lines, a table nested in a blockquote or list, unclosed tables, and tables the converter can't extract a grid from (they'd otherwise vanish with their content) — checking every table in the content, not just the first. Mentions are the one rich element cells may keep: they convert to **@name** exactly as they do in body text. Simple grids open for editing like any other content. The mention-conversion closure moves to a named mentionMarkdown function so cell conversion can reuse it; behavior is unchanged. Editing simple tables in the todos view requires the composer Reset fix (previous commit): without it, Reset dropped the description composer to single-line mode and SetValue flattened the freshly-converted pipe table. The todos test asserts the exact multiline table to prove the pairing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
Every top-level block gets the
<p><br></p>separator when the Markdown put a blank line before it — except tables:rendered as
<p>Intro.</p>\n<table>…— no separator, table jammed against the paragraph — while the separator after a table worked fine.Why: goldmark's table extension builds the
Tablenode inside a paragraph transformer that replaces the source paragraph without carryingHasBlankPreviousLinesover, so the flag is always false on tables and the phase-2TrixBreakinsertion skips them. Pre-existing since #498; surfaced while verifying #637 (its "no bare top-level breaks" invariant test couldn't include a table without tripping over this).Fix
Recover the answer from the source.
Table.Pos()is the start of the replaced paragraph's first line, so the table was blank-line-separated exactly when the line above that position is blank (precedingLineIsBlank, CRLF-aware). The one case the false flag is right about — a table interrupting a paragraph mid-flight (Intro.\n| a | b |…), where the transformer leaves the leading lines behind as a paragraph — is detected by that leftover paragraph sharing the table'sPos, and stays attached.X.\n\nIntro.\n| a |…still separates before the paragraph only.Tests
TestMarkdownToHTMLTableSeparators: blank-separated (before+after), interrupting (attached), blank-before-interrupted-paragraph, consecutive tables, after-heading, first-block, CRLF.TestMarkdownToHTMLEmitsNoBareTopLevelBreaksnow includes a table block and<table>in its block list — the separator count invariant holds for all 8 block kinds.bin/cigreen locally.Summary by cubic
Restores the blank-line
<p><br></p>separator before Markdown tables. Previously, tables lost the leading separator and rendered flush with the prior block; now tables get the same spacing as other blocks, while tables that interrupt a paragraph remain attached.hasBlankPreviousLinescheck that recovers table separation from the source text, including CRLF handling; preserves the interrupting-table case by detecting a preceding paragraph with the samePos.<table>in the “no bare top-level breaks” invariant.internal/richtext/richtext.goand its tests; behavior for non-table blocks is unchanged.Written for commit 99b6016. Summary will update on new commits.