Refactor markdown render (#32728)

Follow up recent render system refactoring PRs (split test code), and
fine tune the math render (added some new cases)

modules/markup/markdown/markdown_attention_test.go

@@ -0,0 +1,56 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package markdown_test
+
+import (
+	"strings"
+	"testing"
+
+	"code.gitea.io/gitea/modules/markup"
+	"code.gitea.io/gitea/modules/markup/markdown"
+	"code.gitea.io/gitea/modules/svg"
+
+	"github.com/stretchr/testify/assert"
+	"golang.org/x/text/cases"
+	"golang.org/x/text/language"
+)
+
+func TestAttention(t *testing.T) {
+	defer svg.MockIcon("octicon-info")()
+	defer svg.MockIcon("octicon-light-bulb")()
+	defer svg.MockIcon("octicon-report")()
+	defer svg.MockIcon("octicon-alert")()
+	defer svg.MockIcon("octicon-stop")()
+
+	renderAttention := func(attention, icon string) string {
+		tmpl := `<blockquote class="attention-header attention-{attention}"><p><svg class="attention-icon attention-{attention} svg {icon}" width="16" height="16"></svg><strong class="attention-{attention}">{Attention}</strong></p>`
+		tmpl = strings.ReplaceAll(tmpl, "{attention}", attention)
+		tmpl = strings.ReplaceAll(tmpl, "{icon}", icon)
+		tmpl = strings.ReplaceAll(tmpl, "{Attention}", cases.Title(language.English).String(attention))
+		return tmpl
+	}
+
+	test := func(input, expected string) {
+		result, err := markdown.RenderString(markup.NewTestRenderContext(), input)
+		assert.NoError(t, err)
+		assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(result)))
+	}
+
+	test(`
+> [!NOTE]
+> text
+`, renderAttention("note", "octicon-info")+"\n<p>text</p>\n</blockquote>")
+
+	test(`> [!note]`, renderAttention("note", "octicon-info")+"\n</blockquote>")
+	test(`> [!tip]`, renderAttention("tip", "octicon-light-bulb")+"\n</blockquote>")
+	test(`> [!important]`, renderAttention("important", "octicon-report")+"\n</blockquote>")
+	test(`> [!warning]`, renderAttention("warning", "octicon-alert")+"\n</blockquote>")
+	test(`> [!caution]`, renderAttention("caution", "octicon-stop")+"\n</blockquote>")
+
+	// escaped by mdformat
+	test(`> \[!NOTE\]`, renderAttention("note", "octicon-info")+"\n</blockquote>")
+
+	// legacy GitHub style
+	test(`> **warning**`, renderAttention("warning", "octicon-alert")+"\n</blockquote>")
+}

modules/markup/markdown/markdown_benchmark_test.go

@@ -0,0 +1,25 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package markdown_test
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/modules/markup"
+	"code.gitea.io/gitea/modules/markup/markdown"
+)
+
+func BenchmarkSpecializedMarkdown(b *testing.B) {
+	// 240856	      4719 ns/op
+	for i := 0; i < b.N; i++ {
+		markdown.SpecializedMarkdown(&markup.RenderContext{})
+	}
+}
+
+func BenchmarkMarkdownRender(b *testing.B) {
+	// 23202	     50840 ns/op
+	for i := 0; i < b.N; i++ {
+		_, _ = markdown.RenderString(markup.NewTestRenderContext(), "https://example.com\n- a\n- b\n")
+	}
+}

modules/markup/markdown/markdown_math_test.go

@@ -0,0 +1,163 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package markdown
+
+import (
+	"strings"
+	"testing"
+
+	"code.gitea.io/gitea/modules/markup"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestMathRender(t *testing.T) {
+	const nl = "\n"
+	testcases := []struct {
+		testcase string
+		expected string
+	}{
+		{
+			"$a$",
+			`<p><code class="language-math is-loading">a</code></p>` + nl,
+		},
+		{
+			"$ a $",
+			`<p><code class="language-math is-loading">a</code></p>` + nl,
+		},
+		{
+			"$a$ $b$",
+			`<p><code class="language-math is-loading">a</code> <code class="language-math is-loading">b</code></p>` + nl,
+		},
+		{
+			`\(a\) \(b\)`,
+			`<p><code class="language-math is-loading">a</code> <code class="language-math is-loading">b</code></p>` + nl,
+		},
+		{
+			`$a$.`,
+			`<p><code class="language-math is-loading">a</code>.</p>` + nl,
+		},
+		{
+			`.$a$`,
+			`<p>.$a$</p>` + nl,
+		},
+		{
+			`$a a$b b$`,
+			`<p>$a a$b b$</p>` + nl,
+		},
+		{
+			`a a$b b`,
+			`<p>a a$b b</p>` + nl,
+		},
+		{
+			`a$b $a a$b b$`,
+			`<p>a$b $a a$b b$</p>` + nl,
+		},
+		{
+			"a$x$",
+			`<p>a$x$</p>` + nl,
+		},
+		{
+			"$x$a",
+			`<p>$x$a</p>` + nl,
+		},
+		{
+			"$a$ ($b$) [$c$] {$d$}",
+			`<p><code class="language-math is-loading">a</code> (<code class="language-math is-loading">b</code>) [$c$] {$d$}</p>` + nl,
+		},
+		{
+			"$$a$$",
+			`<pre class="code-block is-loading"><code class="chroma language-math display">a</code></pre>` + nl,
+		},
+		{
+			"$$a$$ test",
+			`<p><code class="language-math display is-loading">a</code> test</p>` + nl,
+		},
+		{
+			"test $$a$$",
+			`<p>test <code class="language-math display is-loading">a</code></p>` + nl,
+		},
+		{
+			"foo $x=\\$$ bar",
+			`<p>foo <code class="language-math is-loading">x=\$</code> bar</p>` + nl,
+		},
+	}
+
+	for _, test := range testcases {
+		t.Run(test.testcase, func(t *testing.T) {
+			res, err := RenderString(markup.NewTestRenderContext(), test.testcase)
+			assert.NoError(t, err)
+			assert.Equal(t, test.expected, string(res))
+		})
+	}
+}
+
+func TestMathRenderBlockIndent(t *testing.T) {
+	testcases := []struct {
+		name     string
+		testcase string
+		expected string
+	}{
+		{
+			"indent-0",
+			`
+\[
+\alpha
+\]
+`,
+			`<pre class="code-block is-loading"><code class="chroma language-math display">
+\alpha
+</code></pre>
+`,
+		},
+		{
+			"indent-1",
+			`
+ \[
+ \alpha
+ \]
+`,
+			`<pre class="code-block is-loading"><code class="chroma language-math display">
+\alpha
+</code></pre>
+`,
+		},
+		{
+			"indent-2",
+			`
+  \[
+  \alpha
+  \]
+`,
+			`<pre class="code-block is-loading"><code class="chroma language-math display">
+\alpha
+</code></pre>
+`,
+		},
+		{
+			"indent-0-oneline",
+			`$$ x $$
+foo`,
+			`<pre class="code-block is-loading"><code class="chroma language-math display"> x </code></pre>
+<p>foo</p>
+`,
+		},
+		{
+			"indent-3-oneline",
+			`   $$ x $$<SPACE>
+foo`,
+			`<pre class="code-block is-loading"><code class="chroma language-math display"> x </code></pre>
+<p>foo</p>
+`,
+		},
+	}
+
+	for _, test := range testcases {
+		t.Run(test.name, func(t *testing.T) {
+			res, err := RenderString(markup.NewTestRenderContext(), strings.ReplaceAll(test.testcase, "<SPACE>", " "))
+			assert.NoError(t, err)
+			assert.Equal(t, test.expected, string(res), "unexpected result for test case:\n%s", test.testcase)
+		})
+	}
+}

modules/markup/markdown/markdown_test.go

@@ -13,13 +13,10 @@ import (
 	"code.gitea.io/gitea/modules/markup"
 	"code.gitea.io/gitea/modules/markup/markdown"
 	"code.gitea.io/gitea/modules/setting"
-	"code.gitea.io/gitea/modules/svg"
 	"code.gitea.io/gitea/modules/test"
 	"code.gitea.io/gitea/modules/util"
 
 	"github.com/stretchr/testify/assert"
-	"golang.org/x/text/cases"
-	"golang.org/x/text/language"
 )
 
 const (
@@ -386,81 +383,6 @@ func TestColorPreview(t *testing.T) {
 	}
 }
 
-func TestMathBlock(t *testing.T) {
-	const nl = "\n"
-	testcases := []struct {
-		testcase string
-		expected string
-	}{
-		{
-			"$a$",
-			`<p><code class="language-math is-loading">a</code></p>` + nl,
-		},
-		{
-			"$ a $",
-			`<p><code class="language-math is-loading">a</code></p>` + nl,
-		},
-		{
-			"$a$ $b$",
-			`<p><code class="language-math is-loading">a</code> <code class="language-math is-loading">b</code></p>` + nl,
-		},
-		{
-			`\(a\) \(b\)`,
-			`<p><code class="language-math is-loading">a</code> <code class="language-math is-loading">b</code></p>` + nl,
-		},
-		{
-			`$a$.`,
-			`<p><code class="language-math is-loading">a</code>.</p>` + nl,
-		},
-		{
-			`.$a$`,
-			`<p>.$a$</p>` + nl,
-		},
-		{
-			`$a a$b b$`,
-			`<p>$a a$b b$</p>` + nl,
-		},
-		{
-			`a a$b b`,
-			`<p>a a$b b</p>` + nl,
-		},
-		{
-			`a$b $a a$b b$`,
-			`<p>a$b $a a$b b$</p>` + nl,
-		},
-		{
-			"a$x$",
-			`<p>a$x$</p>` + nl,
-		},
-		{
-			"$x$a",
-			`<p>$x$a</p>` + nl,
-		},
-		{
-			"$$a$$",
-			`<pre class="code-block is-loading"><code class="chroma language-math display">a</code></pre>` + nl,
-		},
-		{
-			"$a$ ($b$) [$c$] {$d$}",
-			`<p><code class="language-math is-loading">a</code> (<code class="language-math is-loading">b</code>) [$c$] {$d$}</p>` + nl,
-		},
-		{
-			"$$a$$ test",
-			`<p><code class="language-math display is-loading">a</code> test</p>` + nl,
-		},
-		{
-			"test $$a$$",
-			`<p>test <code class="language-math display is-loading">a</code></p>` + nl,
-		},
-	}
-
-	for _, test := range testcases {
-		res, err := markdown.RenderString(markup.NewTestRenderContext(), test.testcase)
-		assert.NoError(t, err, "Unexpected error in testcase: %q", test.testcase)
-		assert.Equal(t, template.HTML(test.expected), res, "Unexpected result in testcase %q", test.testcase)
-	}
-}
-
 func TestTaskList(t *testing.T) {
 	testcases := []struct {
 		testcase string
@@ -551,56 +473,3 @@ space</p>
 	assert.NoError(t, err)
 	assert.Equal(t, expected, string(result))
 }
-
-func TestAttention(t *testing.T) {
-	defer svg.MockIcon("octicon-info")()
-	defer svg.MockIcon("octicon-light-bulb")()
-	defer svg.MockIcon("octicon-report")()
-	defer svg.MockIcon("octicon-alert")()
-	defer svg.MockIcon("octicon-stop")()
-
-	renderAttention := func(attention, icon string) string {
-		tmpl := `<blockquote class="attention-header attention-{attention}"><p><svg class="attention-icon attention-{attention} svg {icon}" width="16" height="16"></svg><strong class="attention-{attention}">{Attention}</strong></p>`
-		tmpl = strings.ReplaceAll(tmpl, "{attention}", attention)
-		tmpl = strings.ReplaceAll(tmpl, "{icon}", icon)
-		tmpl = strings.ReplaceAll(tmpl, "{Attention}", cases.Title(language.English).String(attention))
-		return tmpl
-	}
-
-	test := func(input, expected string) {
-		result, err := markdown.RenderString(markup.NewTestRenderContext(), input)
-		assert.NoError(t, err)
-		assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(result)))
-	}
-
-	test(`
-> [!NOTE]
-> text
-`, renderAttention("note", "octicon-info")+"\n<p>text</p>\n</blockquote>")
-
-	test(`> [!note]`, renderAttention("note", "octicon-info")+"\n</blockquote>")
-	test(`> [!tip]`, renderAttention("tip", "octicon-light-bulb")+"\n</blockquote>")
-	test(`> [!important]`, renderAttention("important", "octicon-report")+"\n</blockquote>")
-	test(`> [!warning]`, renderAttention("warning", "octicon-alert")+"\n</blockquote>")
-	test(`> [!caution]`, renderAttention("caution", "octicon-stop")+"\n</blockquote>")
-
-	// escaped by mdformat
-	test(`> \[!NOTE\]`, renderAttention("note", "octicon-info")+"\n</blockquote>")
-
-	// legacy GitHub style
-	test(`> **warning**`, renderAttention("warning", "octicon-alert")+"\n</blockquote>")
-}
-
-func BenchmarkSpecializedMarkdown(b *testing.B) {
-	// 240856	      4719 ns/op
-	for i := 0; i < b.N; i++ {
-		markdown.SpecializedMarkdown(&markup.RenderContext{})
-	}
-}
-
-func BenchmarkMarkdownRender(b *testing.B) {
-	// 23202	     50840 ns/op
-	for i := 0; i < b.N; i++ {
-		_, _ = markdown.RenderString(markup.NewTestRenderContext(), "https://example.com\n- a\n- b\n")
-	}
-}

modules/markup/markdown/math/block_parser.go

@@ -54,21 +54,19 @@ func (b *blockParser) Open(parent ast.Node, reader text.Reader, pc parser.Contex
 	idx := bytes.Index(line[pos+2:], endBytes)
 	if idx >= 0 {
 		// for case $$ ... $$ any other text
-		for i := pos + idx + 4; i < len(line); i++ {
+		for i := pos + 2 + idx + 2; i < len(line); i++ {
 			if line[i] != ' ' && line[i] != '\n' {
 				return nil, parser.NoChildren
 			}
 		}
-		segment.Stop = segment.Start + idx + 2
-		reader.Advance(segment.Len() - 1)
-		segment.Start += 2
+		segment.Start += pos + 2
+		segment.Stop = segment.Start + idx
 		node.Lines().Append(segment)
 		node.Closed = true
 		return node, parser.Close | parser.NoChildren
 	}
 
-	reader.Advance(segment.Len() - 1)
-	segment.Start += 2
+	segment.Start += pos + 2
 	node.Lines().Append(segment)
 	return node, parser.NoChildren
 }
@@ -103,7 +101,6 @@ func (b *blockParser) Continue(node ast.Node, reader text.Reader, pc parser.Cont
 	pos, padding := util.IndentPosition(line, 0, block.Indent)
 	seg := text.NewSegmentPadding(segment.Start+pos, segment.Stop, padding)
 	node.Lines().Append(seg)
-	reader.AdvanceAndSetPadding(segment.Stop-segment.Start-pos-1, padding)
 	return parser.Continue | parser.NoChildren
 }
 

modules/markup/markdown/math/inline_parser.go

@@ -26,7 +26,6 @@ var defaultDualDollarParser = &inlineParser{
 	end:   []byte{'$', '$'},
 }
 
-// NewInlineDollarParser returns a new inline parser
 func NewInlineDollarParser() parser.InlineParser {
 	return defaultInlineDollarParser
 }
@@ -40,7 +39,6 @@ var defaultInlineBracketParser = &inlineParser{
 	end:   []byte{'\\', ')'},
 }
 
-// NewInlineDollarParser returns a new inline parser
 func NewInlineBracketParser() parser.InlineParser {
 	return defaultInlineBracketParser
 }
@@ -81,35 +79,29 @@ func (parser *inlineParser) Parse(parent ast.Node, block text.Reader, pc parser.
 	opener := len(parser.start)
 
 	// Now look for an ending line
-	ender := opener
-	for {
-		pos := bytes.Index(line[ender:], parser.end)
-		if pos < 0 {
-			return nil
+	ender := -1
+	for i := opener; i < len(line); i++ {
+		if bytes.HasPrefix(line[i:], parser.end) {
+			succeedingCharacter := byte(0)
+			if i+len(parser.end) < len(line) {
+				succeedingCharacter = line[i+len(parser.end)]
 			}
-
-		ender += pos
-
-		// Now we want to check the character at the end of our parser section
-		// that is ender + len(parser.end) and check if char before ender is '\'
-		pos = ender + len(parser.end)
-		if len(line) <= pos {
-			break
-		}
-		suceedingCharacter := line[pos]
 			// check valid ending character
-		if !isPunctuation(suceedingCharacter) &&
-			!(suceedingCharacter == ' ') &&
-			!(suceedingCharacter == '\n') &&
-			!isBracket(suceedingCharacter) {
-			return nil
-		}
-		if line[ender-1] != '\\' {
+			isValidEndingChar := isPunctuation(succeedingCharacter) || isBracket(succeedingCharacter) ||
+				succeedingCharacter == ' ' || succeedingCharacter == '\n' || succeedingCharacter == 0
+			if !isValidEndingChar {
 				break
 			}
-
-		// move the pointer onwards
-		ender += len(parser.end)
+			ender = i
+			break
+		}
+		if line[i] == '\\' {
+			i++
+			continue
+		}
+	}
+	if ender == -1 {
+		return nil
 	}
 
 	block.Advance(opener)