From 0f18046df4a9560351fcd8a7d9c9a80adf10efc2 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 6 Dec 2024 12:29:09 +0800 Subject: [PATCH] Refactor markdown render (#32728) Follow up recent render system refactoring PRs (split test code), and fine tune the math render (added some new cases) --- .../markdown/markdown_attention_test.go | 56 ++++++ .../markdown/markdown_benchmark_test.go | 25 +++ modules/markup/markdown/markdown_math_test.go | 163 ++++++++++++++++++ modules/markup/markdown/markdown_test.go | 131 -------------- modules/markup/markdown/math/block_parser.go | 11 +- modules/markup/markdown/math/inline_parser.go | 48 +++--- 6 files changed, 268 insertions(+), 166 deletions(-) create mode 100644 modules/markup/markdown/markdown_attention_test.go create mode 100644 modules/markup/markdown/markdown_benchmark_test.go create mode 100644 modules/markup/markdown/markdown_math_test.go diff --git a/modules/markup/markdown/markdown_attention_test.go b/modules/markup/markdown/markdown_attention_test.go new file mode 100644 index 0000000000..f6ec775b2c --- /dev/null +++ b/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 := `

{Attention}

` + 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

text

\n
") + + test(`> [!note]`, renderAttention("note", "octicon-info")+"\n") + test(`> [!tip]`, renderAttention("tip", "octicon-light-bulb")+"\n") + test(`> [!important]`, renderAttention("important", "octicon-report")+"\n") + test(`> [!warning]`, renderAttention("warning", "octicon-alert")+"\n") + test(`> [!caution]`, renderAttention("caution", "octicon-stop")+"\n") + + // escaped by mdformat + test(`> \[!NOTE\]`, renderAttention("note", "octicon-info")+"\n") + + // legacy GitHub style + test(`> **warning**`, renderAttention("warning", "octicon-alert")+"\n") +} diff --git a/modules/markup/markdown/markdown_benchmark_test.go b/modules/markup/markdown/markdown_benchmark_test.go new file mode 100644 index 0000000000..0f7e3eea6f --- /dev/null +++ b/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") + } +} diff --git a/modules/markup/markdown/markdown_math_test.go b/modules/markup/markdown/markdown_math_test.go new file mode 100644 index 0000000000..0e5adeeac8 --- /dev/null +++ b/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$", + `

a

` + nl, + }, + { + "$ a $", + `

a

` + nl, + }, + { + "$a$ $b$", + `

a b

` + nl, + }, + { + `\(a\) \(b\)`, + `

a b

` + nl, + }, + { + `$a$.`, + `

a.

` + nl, + }, + { + `.$a$`, + `

.$a$

` + nl, + }, + { + `$a a$b b$`, + `

$a a$b b$

` + nl, + }, + { + `a a$b b`, + `

a a$b b

` + nl, + }, + { + `a$b $a a$b b$`, + `

a$b $a a$b b$

` + nl, + }, + { + "a$x$", + `

a$x$

` + nl, + }, + { + "$x$a", + `

$x$a

` + nl, + }, + { + "$a$ ($b$) [$c$] {$d$}", + `

a (b) [$c$] {$d$}

` + nl, + }, + { + "$$a$$", + `
a
` + nl, + }, + { + "$$a$$ test", + `

a test

` + nl, + }, + { + "test $$a$$", + `

test a

` + nl, + }, + { + "foo $x=\\$$ bar", + `

foo x=\$ bar

` + 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 +\] +`, + `

+\alpha
+
+`, + }, + { + "indent-1", + ` + \[ + \alpha + \] +`, + `

+\alpha
+
+`, + }, + { + "indent-2", + ` + \[ + \alpha + \] +`, + `

+\alpha
+
+`, + }, + { + "indent-0-oneline", + `$$ x $$ +foo`, + `
 x
+

foo

+`, + }, + { + "indent-3-oneline", + ` $$ x $$ +foo`, + `
 x
+

foo

+`, + }, + } + + for _, test := range testcases { + t.Run(test.name, func(t *testing.T) { + res, err := RenderString(markup.NewTestRenderContext(), strings.ReplaceAll(test.testcase, "", " ")) + assert.NoError(t, err) + assert.Equal(t, test.expected, string(res), "unexpected result for test case:\n%s", test.testcase) + }) + } +} diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index 22ab39ebfa..7a09be8665 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/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$", - `

a

` + nl, - }, - { - "$ a $", - `

a

` + nl, - }, - { - "$a$ $b$", - `

a b

` + nl, - }, - { - `\(a\) \(b\)`, - `

a b

` + nl, - }, - { - `$a$.`, - `

a.

` + nl, - }, - { - `.$a$`, - `

.$a$

` + nl, - }, - { - `$a a$b b$`, - `

$a a$b b$

` + nl, - }, - { - `a a$b b`, - `

a a$b b

` + nl, - }, - { - `a$b $a a$b b$`, - `

a$b $a a$b b$

` + nl, - }, - { - "a$x$", - `

a$x$

` + nl, - }, - { - "$x$a", - `

$x$a

` + nl, - }, - { - "$$a$$", - `
a
` + nl, - }, - { - "$a$ ($b$) [$c$] {$d$}", - `

a (b) [$c$] {$d$}

` + nl, - }, - { - "$$a$$ test", - `

a test

` + nl, - }, - { - "test $$a$$", - `

test a

` + 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

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 := `

{Attention}

` - 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

text

\n
") - - test(`> [!note]`, renderAttention("note", "octicon-info")+"\n") - test(`> [!tip]`, renderAttention("tip", "octicon-light-bulb")+"\n") - test(`> [!important]`, renderAttention("important", "octicon-report")+"\n") - test(`> [!warning]`, renderAttention("warning", "octicon-alert")+"\n") - test(`> [!caution]`, renderAttention("caution", "octicon-stop")+"\n") - - // escaped by mdformat - test(`> \[!NOTE\]`, renderAttention("note", "octicon-info")+"\n") - - // legacy GitHub style - test(`> **warning**`, renderAttention("warning", "octicon-alert")+"\n") -} - -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") - } -} diff --git a/modules/markup/markdown/math/block_parser.go b/modules/markup/markdown/math/block_parser.go index a23f48b637..f31cfb09ad 100644 --- a/modules/markup/markdown/math/block_parser.go +++ b/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 } diff --git a/modules/markup/markdown/math/inline_parser.go b/modules/markup/markdown/math/inline_parser.go index b11195d551..56ae3d57eb 100644 --- a/modules/markup/markdown/math/inline_parser.go +++ b/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 += 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 { + 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)] + } + // check valid ending character + isValidEndingChar := isPunctuation(succeedingCharacter) || isBracket(succeedingCharacter) || + succeedingCharacter == ' ' || succeedingCharacter == '\n' || succeedingCharacter == 0 + if !isValidEndingChar { + break + } + ender = i break } - suceedingCharacter := line[pos] - // check valid ending character - if !isPunctuation(suceedingCharacter) && - !(suceedingCharacter == ' ') && - !(suceedingCharacter == '\n') && - !isBracket(suceedingCharacter) { - return nil + if line[i] == '\\' { + i++ + continue } - if line[ender-1] != '\\' { - break - } - - // move the pointer onwards - ender += len(parser.end) + } + if ender == -1 { + return nil } block.Advance(opener)