Restore the lost blank line before Markdown tables

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()

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: "<h1>Report</h1>\n<table>\n<thead>\n<tr>\n<th>Foo</th>\n<th>Bar</th>\n</tr>\n</thead>\n<tbody>\n<tr>\n<td>Baz</td>\n<td>Qux</td>\n</tr>\n</tbody>\n</table>",
+			expected: "<h1>Report</h1>\n<p><br></p>\n<table>\n<thead>\n<tr>\n<th>Foo</th>\n<th>Bar</th>\n</tr>\n</thead>\n<tbody>\n<tr>\n<td>Baz</td>\n<td>Qux</td>\n</tr>\n</tbody>\n</table>",
 		},
 	}
 
@@ -2467,11 +2467,11 @@ func TestMarkdownToHTMLParagraphSeparatorsMatchMarkdownPath(t *testing.T) {
 // Basecamp's editor discards a bare top-level <br> the first time the content is
 // edited, collapsing the spacing. No blank line between blocks may rely on one.
 func TestMarkdownToHTMLEmitsNoBareTopLevelBreaks(t *testing.T) {
-	markdown := "Para one.\n\nPara two.\n\n## Heading\n\n- a\n- b\n\n> A quote\n\n```\ncode\n```\n\n---\n\nClosing."
+	markdown := "Para one.\n\nPara two.\n\n## Heading\n\n- a\n- b\n\n> A quote\n\n```\ncode\n```\n\n---\n\n| a | b |\n|---|---|\n\nClosing."
 
 	html := MarkdownToHTML(markdown)
 
-	for _, block := range []string{"<p>", "<h2>", "<ul>", "<blockquote>", "<pre>", "<hr>"} {
+	for _, block := range []string{"<p>", "<h2>", "<ul>", "<blockquote>", "<pre>", "<hr>", "<table>"} {
 		if strings.Contains(html, "<br>\n"+block) {
 			t.Errorf("bare <br> 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: "<p>Intro.</p>\n<p><br></p>\n<table>\n<thead>\n<tr>\n<th>a</th>\n</tr>\n</thead>\n</table>\n<p><br></p>\n<p>After.</p>",
+		},
+		{
+			name:     "table interrupting a paragraph stays attached",
+			input:    "Intro.\n| a |\n|---|",
+			expected: "<p>Intro.</p>\n<table>\n<thead>\n<tr>\n<th>a</th>\n</tr>\n</thead>\n</table>",
+		},
+		{
+			name:     "blank line before an interrupted paragraph separates the paragraph, not the table",
+			input:    "X.\n\nIntro.\n| a |\n|---|",
+			expected: "<p>X.</p>\n<p><br></p>\n<p>Intro.</p>\n<table>\n<thead>\n<tr>\n<th>a</th>\n</tr>\n</thead>\n</table>",
+		},
+		{
+			name:     "consecutive blank-line-separated tables are separated",
+			input:    "| a |\n|---|\n\n| b |\n|---|",
+			expected: "<table>\n<thead>\n<tr>\n<th>a</th>\n</tr>\n</thead>\n</table>\n<p><br></p>\n<table>\n<thead>\n<tr>\n<th>b</th>\n</tr>\n</thead>\n</table>",
+		},
+		{
+			name:     "table after a heading gets a separator",
+			input:    "## H\n\n| a |\n|---|",
+			expected: "<h2>H</h2>\n<p><br></p>\n<table>\n<thead>\n<tr>\n<th>a</th>\n</tr>\n</thead>\n</table>",
+		},
+		{
+			name:     "table as first block gets no separator",
+			input:    "| a |\n|---|\n\nAfter.",
+			expected: "<table>\n<thead>\n<tr>\n<th>a</th>\n</tr>\n</thead>\n</table>\n<p><br></p>\n<p>After.</p>",
+		},
+		{
+			name:     "CRLF blank line before a table is recognized",
+			input:    "Intro.\r\n\r\n| a |\r\n|---|",
+			expected: "<p>Intro.</p>\n<p><br></p>\n<table>\n<thead>\n<tr>\n<th>a</th>\n</tr>\n</thead>\n</table>",
+		},
+	}
+
+	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