diff --git a/internal/richtext/richtext.go b/internal/richtext/richtext.go index 675a6743..1c649c44 100644 --- a/internal/richtext/richtext.go +++ b/internal/richtext/richtext.go @@ -222,13 +222,57 @@ func (t *trixTransformer) Transform(node *ast.Document, reader text.Reader, pc p // Phase 2: Insert TrixBreak nodes before blank-line-separated top-level blocks for child := node.FirstChild(); child != nil; child = child.NextSibling() { - if child.HasBlankPreviousLines() && child.PreviousSibling() != nil { + if child.PreviousSibling() != nil && hasBlankPreviousLines(child, reader.Source()) { br := &TrixBreak{} node.InsertBefore(node, child, br) } } } +// hasBlankPreviousLines reports whether a top-level block was separated from +// the previous block by a blank line. For most blocks this is the parser's own +// flag. Tables are the exception: 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 a blank-line-separated table would lose its separator. Recover the +// answer from the source instead: 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. +// +// A table that interrupted a paragraph ("Intro.\n| a | b |\n|---|---|") is the +// case the flag being false is right about: the transformer leaves the leading +// lines behind as a paragraph sharing the table's Pos, so the line above Pos +// belongs to whatever preceded that paragraph, not to the table. Detect it by +// that shared Pos and keep the table attached. +func hasBlankPreviousLines(child ast.Node, source []byte) bool { + table, ok := child.(*east.Table) + if !ok { + return child.HasBlankPreviousLines() + } + if p, isPara := child.PreviousSibling().(*ast.Paragraph); isPara && p.Pos() == table.Pos() { + return false + } + return precedingLineIsBlank(source, table.Pos()) +} + +// precedingLineIsBlank reports whether the line immediately above pos in +// source is blank — empty or whitespace-only. +func precedingLineIsBlank(source []byte, pos int) bool { + 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 +} + func replaceParagraphsWithTextBlocks(parent ast.Node) { for child := parent.FirstChild(); child != nil; { next := child.NextSibling() diff --git a/internal/richtext/richtext_test.go b/internal/richtext/richtext_test.go index 007f453b..e1d8aa00 100644 --- a/internal/richtext/richtext_test.go +++ b/internal/richtext/richtext_test.go @@ -222,7 +222,7 @@ func TestMarkdownToHTML(t *testing.T) { // heading+table regression. name: "gfm table with heading (issue #405)", input: "# Report\n\n| Foo | Bar |\n| --- | --- |\n| Baz | Qux |", - expected: "
| Foo | \nBar | \n
|---|---|
| Baz | \nQux | \n
| Foo | \nBar | \n
|---|---|
| Baz | \nQux | \n
", "
", "", "
"} { + for _, block := range []string{"", "
", "
", "
", "", "
", ""} { if strings.Contains(html, "
\n"+block) { t.Errorf("bare
separator before %s in %q", block, html) } @@ -2481,6 +2481,64 @@ func TestMarkdownToHTMLEmitsNoBareTopLevelBreaks(t *testing.T) { } } +// Goldmark's table extension builds the Table node in a paragraph transformer +// that replaces the source paragraph without carrying its blank-previous-lines +// flag over, so without recovery a blank-line-separated table is the one block +// kind that loses its leading separator. The blank line is recovered from the +// source; a table that interrupts a paragraph mid-flight stays attached. +func TestMarkdownToHTMLTableSeparators(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "blank-line-separated table gets a separator before and after", + input: "Intro.\n\n| a |\n|---|\n\nAfter.", + expected: "Intro.
\n\n
\n\n
\n\n \n\na \n\n
After.
", + }, + { + name: "table interrupting a paragraph stays attached", + input: "Intro.\n| a |\n|---|", + expected: "Intro.
\n\n\n
", + }, + { + name: "blank line before an interrupted paragraph separates the paragraph, not the table", + input: "X.\n\nIntro.\n| a |\n|---|", + expected: "\n \n\na \nX.
\n\n
Intro.
\n\n\n
", + }, + { + name: "consecutive blank-line-separated tables are separated", + input: "| a |\n|---|\n\n| b |\n|---|", + expected: "\n \n\na \n\n\n
\n\n \n\na \n\n
\n\n
", + }, + { + name: "table after a heading gets a separator", + input: "## H\n\n| a |\n|---|", + expected: "\n \n\nb \nH
\n\n
\n\n
", + }, + { + name: "table as first block gets no separator", + input: "| a |\n|---|\n\nAfter.", + expected: "\n \n\na \n\n\n
\n\n \n\na \n\n
After.
", + }, + { + name: "CRLF blank line before a table is recognized", + input: "Intro.\r\n\r\n| a |\r\n|---|", + expected: "Intro.
\n\n
\n\n
", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := MarkdownToHTML(tt.input) + if result != tt.expected { + t.Errorf("MarkdownToHTML(%q)\ngot: %q\nwant: %q", tt.input, result, tt.expected) + } + }) + } +} + // TestResolveMentions_MarkdownLinkInHTMLBlock covers the regression from the // "broken @mention rendering" report: a [@Name](mention:SGID) link authored // inside an HTML block is passed through verbatim by MarkdownToHTML (never\n \n\na \n