Refactor markup render system (#32645)

This PR mainly removes some global variables, moves some code and
renames some functions to make code clearer.

This PR also removes a testing-only option ForceHardLineBreak during
refactoring since the behavior is clear now.
This commit is contained in:
wxiaoguang 2024-11-27 00:46:02 +08:00 committed by GitHub
parent 87bb5ed0bc
commit b6ce2d6dc9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 188 additions and 225 deletions

View File

@ -5,9 +5,9 @@ package markup
import (
"bytes"
"fmt"
"io"
"regexp"
"slices"
"strings"
"sync"
@ -133,75 +133,49 @@ func CustomLinkURLSchemes(schemes []string) {
common.GlobalVars().LinkRegex, _ = xurls.StrictMatchingScheme(strings.Join(withAuth, "|"))
}
type postProcessError struct {
context string
err error
}
func (p *postProcessError) Error() string {
return "PostProcess: " + p.context + ", " + p.err.Error()
}
type processor func(ctx *RenderContext, node *html.Node)
var defaultProcessors = []processor{
fullIssuePatternProcessor,
comparePatternProcessor,
codePreviewPatternProcessor,
fullHashPatternProcessor,
shortLinkProcessor,
linkProcessor,
mentionProcessor,
issueIndexPatternProcessor,
commitCrossReferencePatternProcessor,
hashCurrentPatternProcessor,
emailAddressProcessor,
emojiProcessor,
emojiShortCodeProcessor,
}
// PostProcess does the final required transformations to the passed raw HTML
// PostProcessDefault does the final required transformations to the passed raw HTML
// data, and ensures its validity. Transformations include: replacing links and
// emails with HTML links, parsing shortlinks in the format of [[Link]], like
// MediaWiki, linking issues in the format #ID, and mentions in the format
// @user, and others.
func PostProcess(ctx *RenderContext, input io.Reader, output io.Writer) error {
return postProcess(ctx, defaultProcessors, input, output)
}
var commitMessageProcessors = []processor{
fullIssuePatternProcessor,
comparePatternProcessor,
fullHashPatternProcessor,
linkProcessor,
mentionProcessor,
issueIndexPatternProcessor,
commitCrossReferencePatternProcessor,
hashCurrentPatternProcessor,
emailAddressProcessor,
emojiProcessor,
emojiShortCodeProcessor,
func PostProcessDefault(ctx *RenderContext, input io.Reader, output io.Writer) error {
procs := []processor{
fullIssuePatternProcessor,
comparePatternProcessor,
codePreviewPatternProcessor,
fullHashPatternProcessor,
shortLinkProcessor,
linkProcessor,
mentionProcessor,
issueIndexPatternProcessor,
commitCrossReferencePatternProcessor,
hashCurrentPatternProcessor,
emailAddressProcessor,
emojiProcessor,
emojiShortCodeProcessor,
}
return postProcess(ctx, procs, input, output)
}
// RenderCommitMessage will use the same logic as PostProcess, but will disable
// the shortLinkProcessor and will add a defaultLinkProcessor if defaultLink is
// set, which changes every text node into a link to the passed default link.
// the shortLinkProcessor.
func RenderCommitMessage(ctx *RenderContext, content string) (string, error) {
procs := commitMessageProcessors
return renderProcessString(ctx, procs, content)
}
var commitMessageSubjectProcessors = []processor{
fullIssuePatternProcessor,
comparePatternProcessor,
fullHashPatternProcessor,
linkProcessor,
mentionProcessor,
issueIndexPatternProcessor,
commitCrossReferencePatternProcessor,
hashCurrentPatternProcessor,
emojiShortCodeProcessor,
emojiProcessor,
procs := []processor{
fullIssuePatternProcessor,
comparePatternProcessor,
fullHashPatternProcessor,
linkProcessor,
mentionProcessor,
issueIndexPatternProcessor,
commitCrossReferencePatternProcessor,
hashCurrentPatternProcessor,
emailAddressProcessor,
emojiProcessor,
emojiShortCodeProcessor,
}
return postProcessString(ctx, procs, content)
}
var emojiProcessors = []processor{
@ -214,7 +188,18 @@ var emojiProcessors = []processor{
// emailAddressProcessor, will add a defaultLinkProcessor if defaultLink is set,
// which changes every text node into a link to the passed default link.
func RenderCommitMessageSubject(ctx *RenderContext, defaultLink, content string) (string, error) {
procs := slices.Clone(commitMessageSubjectProcessors)
procs := []processor{
fullIssuePatternProcessor,
comparePatternProcessor,
fullHashPatternProcessor,
linkProcessor,
mentionProcessor,
issueIndexPatternProcessor,
commitCrossReferencePatternProcessor,
hashCurrentPatternProcessor,
emojiShortCodeProcessor,
emojiProcessor,
}
procs = append(procs, func(ctx *RenderContext, node *html.Node) {
ch := &html.Node{Parent: node, Type: html.TextNode, Data: node.Data}
node.Type = html.ElementNode
@ -223,19 +208,19 @@ func RenderCommitMessageSubject(ctx *RenderContext, defaultLink, content string)
node.Attr = []html.Attribute{{Key: "href", Val: defaultLink}, {Key: "class", Val: "muted"}}
node.FirstChild, node.LastChild = ch, ch
})
return renderProcessString(ctx, procs, content)
return postProcessString(ctx, procs, content)
}
// RenderIssueTitle to process title on individual issue/pull page
func RenderIssueTitle(ctx *RenderContext, title string) (string, error) {
// do not render other issue/commit links in an issue's title - which in most cases is already a link.
return renderProcessString(ctx, []processor{
return postProcessString(ctx, []processor{
emojiShortCodeProcessor,
emojiProcessor,
}, title)
}
func renderProcessString(ctx *RenderContext, procs []processor, content string) (string, error) {
func postProcessString(ctx *RenderContext, procs []processor, content string) (string, error) {
var buf strings.Builder
if err := postProcess(ctx, procs, strings.NewReader(content), &buf); err != nil {
return "", err
@ -246,7 +231,7 @@ func renderProcessString(ctx *RenderContext, procs []processor, content string)
// RenderDescriptionHTML will use similar logic as PostProcess, but will
// use a single special linkProcessor.
func RenderDescriptionHTML(ctx *RenderContext, content string) (string, error) {
return renderProcessString(ctx, []processor{
return postProcessString(ctx, []processor{
descriptionLinkProcessor,
emojiShortCodeProcessor,
emojiProcessor,
@ -256,7 +241,7 @@ func RenderDescriptionHTML(ctx *RenderContext, content string) (string, error) {
// RenderEmoji for when we want to just process emoji and shortcodes
// in various places it isn't already run through the normal markdown processor
func RenderEmoji(ctx *RenderContext, content string) (string, error) {
return renderProcessString(ctx, emojiProcessors, content)
return postProcessString(ctx, emojiProcessors, content)
}
func postProcess(ctx *RenderContext, procs []processor, input io.Reader, output io.Writer) error {
@ -276,7 +261,7 @@ func postProcess(ctx *RenderContext, procs []processor, input io.Reader, output
strings.NewReader("</body></html>"),
))
if err != nil {
return &postProcessError{"invalid HTML", err}
return fmt.Errorf("markup.postProcess: invalid HTML: %w", err)
}
if node.Type == html.DocumentNode {
@ -308,7 +293,7 @@ func postProcess(ctx *RenderContext, procs []processor, input io.Reader, output
// Render everything to buf.
for _, node := range newNodes {
if err := html.Render(output, node); err != nil {
return &postProcessError{"error rendering processed HTML", err}
return fmt.Errorf("markup.postProcess: html.Render: %w", err)
}
}
return nil

View File

@ -277,12 +277,12 @@ func TestRender_AutoLink(t *testing.T) {
test := func(input, expected string) {
var buffer strings.Builder
err := PostProcess(NewTestRenderContext(localMetas), strings.NewReader(input), &buffer)
err := PostProcessDefault(NewTestRenderContext(localMetas), strings.NewReader(input), &buffer)
assert.Equal(t, err, nil)
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer.String()))
buffer.Reset()
err = PostProcess(NewTestRenderContext(localMetas), strings.NewReader(input), &buffer)
err = PostProcessDefault(NewTestRenderContext(localMetas), strings.NewReader(input), &buffer)
assert.Equal(t, err, nil)
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer.String()))
}

View File

@ -445,14 +445,14 @@ func Test_ParseClusterFuzz(t *testing.T) {
data := "<A><maTH><tr><MN><bodY ÿ><temPlate></template><tH><tr></A><tH><d<bodY "
var res strings.Builder
err := markup.PostProcess(markup.NewTestRenderContext(localMetas), strings.NewReader(data), &res)
err := markup.PostProcessDefault(markup.NewTestRenderContext(localMetas), strings.NewReader(data), &res)
assert.NoError(t, err)
assert.NotContains(t, res.String(), "<html")
data = "<!DOCTYPE html>\n<A><maTH><tr><MN><bodY ÿ><temPlate></template><tH><tr></A><tH><d<bodY "
res.Reset()
err = markup.PostProcess(markup.NewTestRenderContext(localMetas), strings.NewReader(data), &res)
err = markup.PostProcessDefault(markup.NewTestRenderContext(localMetas), strings.NewReader(data), &res)
assert.NoError(t, err)
assert.NotContains(t, res.String(), "<html")
@ -464,7 +464,7 @@ func TestPostProcess_RenderDocument(t *testing.T) {
test := func(input, expected string) {
var res strings.Builder
err := markup.PostProcess(markup.NewTestRenderContext(markup.TestAppURL, map[string]string{"user": "go-gitea", "repo": "gitea"}), strings.NewReader(input), &res)
err := markup.PostProcessDefault(markup.NewTestRenderContext(markup.TestAppURL, map[string]string{"user": "go-gitea", "repo": "gitea"}), strings.NewReader(input), &res)
assert.NoError(t, err)
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(res.String()))
}
@ -501,7 +501,7 @@ func TestIssue16020(t *testing.T) {
data := `<img src="data:image/png;base64,i//V"/>`
var res strings.Builder
err := markup.PostProcess(markup.NewTestRenderContext(localMetas), strings.NewReader(data), &res)
err := markup.PostProcessDefault(markup.NewTestRenderContext(localMetas), strings.NewReader(data), &res)
assert.NoError(t, err)
assert.Equal(t, data, res.String())
}
@ -514,7 +514,7 @@ func BenchmarkEmojiPostprocess(b *testing.B) {
b.ResetTimer()
for i := 0; i < b.N; i++ {
var res strings.Builder
err := markup.PostProcess(markup.NewTestRenderContext(localMetas), strings.NewReader(data), &res)
err := markup.PostProcessDefault(markup.NewTestRenderContext(localMetas), strings.NewReader(data), &res)
assert.NoError(b, err)
}
}
@ -522,7 +522,7 @@ func BenchmarkEmojiPostprocess(b *testing.B) {
func TestFuzz(t *testing.T) {
s := "t/l/issues/8#/../../a"
renderContext := markup.NewTestRenderContext()
err := markup.PostProcess(renderContext, strings.NewReader(s), io.Discard)
err := markup.PostProcessDefault(renderContext, strings.NewReader(s), io.Discard)
assert.NoError(t, err)
}
@ -530,7 +530,7 @@ func TestIssue18471(t *testing.T) {
data := `http://domain/org/repo/compare/783b039...da951ce`
var res strings.Builder
err := markup.PostProcess(markup.NewTestRenderContext(localMetas), strings.NewReader(data), &res)
err := markup.PostProcessDefault(markup.NewTestRenderContext(localMetas), strings.NewReader(data), &res)
assert.NoError(t, err)
assert.Equal(t, `<a href="http://domain/org/repo/compare/783b039...da951ce" class="compare"><code class="nohighlight">783b039...da951ce</code></a>`, res.String())

View File

@ -80,9 +80,7 @@ func (g *ASTTransformer) Transform(node *ast.Document, reader text.Reader, pc pa
// many places render non-comment contents with no mode=document, then these contents also use comment's hard line break setting
// especially in many tests.
markdownLineBreakStyle := ctx.RenderOptions.Metas["markdownLineBreakStyle"]
if markup.RenderBehaviorForTesting.ForceHardLineBreak {
v.SetHardLineBreak(true)
} else if markdownLineBreakStyle == "comment" {
if markdownLineBreakStyle == "comment" {
v.SetHardLineBreak(setting.Markdown.EnableHardLineBreakInComments)
} else if markdownLineBreakStyle == "document" {
v.SetHardLineBreak(setting.Markdown.EnableHardLineBreakInDocuments)

File diff suppressed because it is too large Load Diff

View File

@ -28,14 +28,6 @@ const (
)
var RenderBehaviorForTesting struct {
// Markdown line break rendering has 2 default behaviors:
// * Use hard: replace "\n" with "<br>" for comments, setting.Markdown.EnableHardLineBreakInComments=true
// * Keep soft: "\n" for non-comments (a.k.a. documents), setting.Markdown.EnableHardLineBreakInDocuments=false
// In history, there was a mess:
// * The behavior was controlled by `Metas["mode"] != "document",
// * However, many places render the content without setting "mode" in Metas, all these places used comment line break setting incorrectly
ForceHardLineBreak bool
// Gitea will emit some additional attributes for various purposes, these attributes don't affect rendering.
// But there are too many hard-coded test cases, to avoid changing all of them again and again, we can disable emitting these internal attributes.
DisableAdditionalAttributes bool
@ -218,7 +210,7 @@ func render(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Wr
eg.Go(func() (err error) {
if r, ok := renderer.(PostProcessRenderer); ok && r.NeedPostProcess() {
err = PostProcess(ctx, pr1, pw2)
err = PostProcessDefault(ctx, pr1, pw2)
} else {
_, err = io.Copy(pw2, pr1)
}

View File

@ -27,6 +27,6 @@ func FuzzMarkdownRenderRaw(f *testing.F) {
func FuzzMarkupPostProcess(f *testing.F) {
f.Fuzz(func(t *testing.T, data []byte) {
setting.AppURL = "http://localhost:3000/"
markup.PostProcess(newFuzzRenderContext(), bytes.NewReader(data), io.Discard)
markup.PostProcessDefault(newFuzzRenderContext(), bytes.NewReader(data), io.Discard)
})
}