Improve white-space and general rendering of CommonMark in Matrix

pkg/channels/matrix/matrix.go

@@ -15,6 +15,7 @@ import (
 	"time"
 
 	"github.com/gomarkdown/markdown"
+	mdast "github.com/gomarkdown/markdown/ast"
 	mdhtml "github.com/gomarkdown/markdown/html"
 	"github.com/gomarkdown/markdown/parser"
 	"maunium.net/go/mautrix"
@@ -41,6 +42,36 @@ const (
 
 var matrixMentionHrefRegexp = regexp.MustCompile(`(?i)<a[^>]+href=["']([^"']+)["']`)
 
+// matrixHTMLRenderer wraps the standard gomarkdown HTML renderer and suppresses <p>
+// wrappers throughout the document. Matrix clients apply default browser paragraph
+// margins to <p> elements, which stack with the margins of surrounding block elements
+// (lists, code blocks, headings) and produce excessive vertical spacing. Instead we
+// emit a single <br> between consecutive paragraphs for visual separation; adjacent
+// block-level elements already carry their own CSS margin.
+type matrixHTMLRenderer struct {
+	*mdhtml.Renderer
+}
+
+func (r *matrixHTMLRenderer) RenderNode(w io.Writer, node mdast.Node, entering bool) mdast.WalkStatus {
+	if _, ok := node.(*mdast.Paragraph); ok {
+		if !entering {
+			// Between consecutive paragraphs emit a line break for separation.
+			// Before block siblings (lists, headings, code blocks) their own margin suffices.
+			siblings := node.GetParent().GetChildren()
+			for i, child := range siblings {
+				if child == node && i+1 < len(siblings) {
+					if _, ok := siblings[i+1].(*mdast.Paragraph); ok {
+						io.WriteString(w, "<br>") //nolint:errcheck
+					}
+					break
+				}
+			}
+		}
+		return mdast.GoToNext
+	}
+	return r.Renderer.RenderNode(w, node, entering)
+}
+
 type roomKindCacheEntry struct {
 	isGroup   bool
 	expiresAt time.Time
@@ -273,8 +304,9 @@ func (c *MatrixChannel) Stop(ctx context.Context) error {
 }
 
 func markdownToHTML(md string) string {
-	p := parser.NewWithExtensions(parser.CommonExtensions | parser.AutoHeadingIDs)
-	renderer := mdhtml.NewRenderer(mdhtml.RendererOptions{Flags: mdhtml.CommonFlags})
+	extensions := (parser.CommonExtensions | parser.AutoHeadingIDs) &^ parser.DefinitionLists
+	p := parser.NewWithExtensions(extensions)
+	renderer := &matrixHTMLRenderer{mdhtml.NewRenderer(mdhtml.RendererOptions{Flags: mdhtml.CommonFlags})}
 	return strings.TrimSpace(string(markdown.ToHTML([]byte(md), p, renderer)))
 }
 

pkg/channels/matrix/matrix_test.go

@@ -340,23 +340,82 @@ func TestMatrixOutboundContent(t *testing.T) {
 }
 
 func TestMarkdownToHTML(t *testing.T) {
-	tests := []struct {
+	cases := []struct {
 		name     string
-		input    string
-		contains string
+		md       string
+		rendered string
 	}{
-		{"bold", "**hello**", "<strong>hello</strong>"},
-		{"italic", "_world_", "<em>world</em>"},
-		{"header", "### Title", "<h3"},
-		{"code block", "```\nfoo()\n```", "<code>"},
-		{"inline code", "`x`", "<code>x</code>"},
-		{"plain text", "just text", "just text"},
+		{
+			name:     "bold",
+			md:       "**hello**",
+			rendered: `<strong>hello</strong>`,
+		},
+		{
+			name:     "italic",
+			md:       "_world_",
+			rendered: `<em>world</em>`,
+		},
+		{
+			name:     "heading",
+			md:       "### Title",
+			rendered: `<h3 id="title">Title</h3>`,
+		},
+		{
+			name:     "fenced code block",
+			md:       "```\nfoo()\n```",
+			rendered: "<pre><code>foo()\n</code></pre>",
+		},
+		{
+			name:     "inline code",
+			md:       "`x`",
+			rendered: `<code>x</code>`,
+		},
+		{
+			name:     "plain text has no block wrapper",
+			md:       "just text",
+			rendered: `just text`,
+		},
+		{
+			name: "loose list has no <p> wrapper around items",
+			md:   "- Item one\n\n- Item two\n",
+			rendered: `<ul>
+<li>Item one</li>
+
+<li>Item two</li>
+</ul>`,
+		},
+		{
+			name: "list item with nested sublist has no <p> wrapper",
+			md:   "1. Steps overview:\n\n   - Step A\n   - Step B\n",
+			rendered: `<ol>
+<li>Steps overview:
+<ul>
+<li>Step A</li>
+<li>Step B</li>
+</ul></li>
+</ol>`,
+		},
+		{
+			name: "tight list has no <p> wrapper",
+			md:   "- Alpha\n- Beta\n",
+			rendered: `<ul>
+<li>Alpha</li>
+<li>Beta</li>
+</ul>`,
+		},
+		{
+			name: "paragraph before list has no <p> wrapper",
+			md:   "Introduction text.\n\n- Point one\n",
+			rendered: `Introduction text.<ul>
+<li>Point one</li>
+</ul>`,
+		},
 	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got := markdownToHTML(tt.input)
-			if !strings.Contains(got, tt.contains) {
-				t.Fatalf("markdownToHTML(%q) = %q, want it to contain %q", tt.input, got, tt.contains)
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := markdownToHTML(tc.md); got != tc.rendered {
+				t.Fatalf("markdownToHTML(%q)\n got: %q\nwant: %q", tc.md, got, tc.rendered)
 			}
 		})
 	}