Refactor render system (go-gitea#32492)

There were too many patches to the Render system, it's really difficult to make further improvements. This PR clears the legacy problems and fix TODOs. 1. Rename `RenderContext.Type` to `RenderContext.MarkupType` to clarify its usage. 2. Use `ContentMode` to replace `meta["mode"]` and `IsWiki`, to clarify the rendering behaviors. 3. Use "wiki" mode instead of "mode=gfm + wiki=true" 4. Merge `renderByType` and `renderByFile` 5. Add more comments ---- The problem of "mode=document": in many cases it is not set, so many non-comment places use comment's hard line break incorrectly
zjjhot · Nov 14, 2024 · 3f9c3e7 · 3f9c3e7
1 parent 985e2a8
commit 3f9c3e7
Show file tree

Hide file tree

Showing 32 changed files with 237 additions and 257 deletions.
diff --git a/models/repo/repo.go b/models/repo/repo.go
@@ -479,7 +479,6 @@ func (repo *Repository) ComposeMetas(ctx context.Context) map[string]string {
 		metas := map[string]string{
 			"user": repo.OwnerName,
 			"repo": repo.Name,
-			"mode": "comment",
 		}
 
 		unit, err := repo.GetUnit(ctx, unit.TypeExternalTracker)
@@ -521,7 +520,6 @@ func (repo *Repository) ComposeDocumentMetas(ctx context.Context) map[string]str
 		for k, v := range repo.ComposeMetas(ctx) {
 			metas[k] = v
 		}
-		metas["mode"] = "document"
 		repo.DocumentRenderingMetas = metas
 	}
 	return repo.DocumentRenderingMetas

diff --git a/modules/markup/console/console.go b/modules/markup/console/console.go
@@ -8,7 +8,6 @@ import (
 	"io"
 	"path/filepath"
 	"regexp"
-	"strings"
 
 	"code.gitea.io/gitea/modules/markup"
 	"code.gitea.io/gitea/modules/setting"
@@ -17,9 +16,6 @@ import (
 	"github.com/go-enry/go-enry/v2"
 )
 
-// MarkupName describes markup's name
-var MarkupName = "console"
-
 func init() {
 	markup.RegisterRenderer(Renderer{})
 }
@@ -29,7 +25,7 @@ type Renderer struct{}
 
 // Name implements markup.Renderer
 func (Renderer) Name() string {
-	return MarkupName
+	return "console"
 }
 
 // Extensions implements markup.Renderer
@@ -67,20 +63,3 @@ func (Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Wri
 	_, err = output.Write(buf)
 	return err
 }
-
-// Render renders terminal colors to HTML with all specific handling stuff.
-func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error {
-	if ctx.Type == "" {
-		ctx.Type = MarkupName
-	}
-	return markup.Render(ctx, input, output)
-}
-
-// RenderString renders terminal colors in string to HTML with all specific handling stuff and return string
-func RenderString(ctx *markup.RenderContext, content string) (string, error) {
-	var buf strings.Builder
-	if err := Render(ctx, strings.NewReader(content), &buf); err != nil {
-		return "", err
-	}
-	return buf.String(), nil
-}
diff --git a/modules/markup/html.go b/modules/markup/html.go
@@ -442,12 +442,11 @@ func createLink(href, content, class string) *html.Node {
 	a := &html.Node{
 		Type: html.ElementNode,
 		Data: atom.A.String(),
-		Attr: []html.Attribute{
-			{Key: "href", Val: href},
-			{Key: "data-markdown-generated-content"},
-		},
+		Attr: []html.Attribute{{Key: "href", Val: href}},
+	}
+	if !RenderBehaviorForTesting.DisableInternalAttributes {
+		a.Attr = append(a.Attr, html.Attribute{Key: "data-markdown-generated-content"})
 	}
-
 	if class != "" {
 		a.Attr = append(a.Attr, html.Attribute{Key: "class", Val: class})
 	}

diff --git a/modules/markup/html_codepreview_test.go b/modules/markup/html_codepreview_test.go
@@ -11,6 +11,7 @@ import (
 
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/markup"
+	"code.gitea.io/gitea/modules/markup/markdown"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -23,8 +24,8 @@ func TestRenderCodePreview(t *testing.T) {
 	})
 	test := func(input, expected string) {
 		buffer, err := markup.RenderString(&markup.RenderContext{
-			Ctx:  git.DefaultContext,
-			Type: "markdown",
+			Ctx:        git.DefaultContext,
+			MarkupType: markdown.MarkupName,
 		}, input)
 		assert.NoError(t, err)
 		assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer))

diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go
@@ -11,6 +11,7 @@ import (
 
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/setting"
+	testModule "code.gitea.io/gitea/modules/test"
 	"code.gitea.io/gitea/modules/util"
 
 	"github.com/stretchr/testify/assert"
@@ -123,8 +124,9 @@ func TestRender_IssueIndexPattern2(t *testing.T) {
 		}
 		expectedNil := fmt.Sprintf(expectedFmt, links...)
 		testRenderIssueIndexPattern(t, s, expectedNil, &RenderContext{
-			Ctx:   git.DefaultContext,
-			Metas: localMetas,
+			Ctx:         git.DefaultContext,
+			Metas:       localMetas,
+			ContentMode: RenderContentAsComment,
 		})
 
 		class := "ref-issue"
@@ -137,8 +139,9 @@ func TestRender_IssueIndexPattern2(t *testing.T) {
 		}
 		expectedNum := fmt.Sprintf(expectedFmt, links...)
 		testRenderIssueIndexPattern(t, s, expectedNum, &RenderContext{
-			Ctx:   git.DefaultContext,
-			Metas: numericMetas,
+			Ctx:         git.DefaultContext,
+			Metas:       numericMetas,
+			ContentMode: RenderContentAsComment,
 		})
 	}
 
@@ -266,7 +269,6 @@ func TestRender_IssueIndexPattern_Document(t *testing.T) {
 		"user":   "someUser",
 		"repo":   "someRepo",
 		"style":  IssueNameStyleNumeric,
-		"mode":   "document",
 	}
 
 	testRenderIssueIndexPattern(t, "#1", "#1", &RenderContext{
@@ -316,8 +318,8 @@ func TestRender_AutoLink(t *testing.T) {
 			Links: Links{
 				Base: TestRepoURL,
 			},
-			Metas:  localMetas,
-			IsWiki: true,
+			Metas:       localMetas,
+			ContentMode: RenderContentAsWiki,
 		}, strings.NewReader(input), &buffer)
 		assert.Equal(t, err, nil)
 		assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer.String()))
@@ -340,7 +342,7 @@ func TestRender_AutoLink(t *testing.T) {
 
 func TestRender_FullIssueURLs(t *testing.T) {
 	setting.AppURL = TestAppURL
-
+	defer testModule.MockVariableValue(&RenderBehaviorForTesting.DisableInternalAttributes, true)()
 	test := func(input, expected string) {
 		var result strings.Builder
 		err := postProcess(&RenderContext{
@@ -351,9 +353,7 @@ func TestRender_FullIssueURLs(t *testing.T) {
 			Metas: localMetas,
 		}, []processor{fullIssuePatternProcessor}, strings.NewReader(input), &result)
 		assert.NoError(t, err)
-		actual := result.String()
-		actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "")
-		assert.Equal(t, expected, actual)
+		assert.Equal(t, expected, result.String())
 	}
 	test("Here is a link https://git.osgeo.org/gogs/postgis/postgis/pulls/6",
 		"Here is a link https://git.osgeo.org/gogs/postgis/postgis/pulls/6")

diff --git a/modules/markup/html_issue.go b/modules/markup/html_issue.go
@@ -67,9 +67,8 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) {
 		return
 	}
 
-	// FIXME: the use of "mode" is quite dirty and hacky, for example: what is a "document"? how should it be rendered?
-	// The "mode" approach should be refactored to some other more clear&reliable way.
-	crossLinkOnly := ctx.Metas["mode"] == "document" && !ctx.IsWiki
+	// crossLinkOnly if not comment and not wiki
+	crossLinkOnly := ctx.ContentMode != RenderContentAsTitle && ctx.ContentMode != RenderContentAsComment && ctx.ContentMode != RenderContentAsWiki
 
 	var (
 		found bool

diff --git a/modules/markup/html_link.go b/modules/markup/html_link.go
@@ -20,7 +20,7 @@ func ResolveLink(ctx *RenderContext, link, userContentAnchorPrefix string) (resu
 	isAnchorFragment := link != "" && link[0] == '#'
 	if !isAnchorFragment && !IsFullURLString(link) {
 		linkBase := ctx.Links.Base
-		if ctx.IsWiki {
+		if ctx.ContentMode == RenderContentAsWiki {
 			// no need to check if the link should be resolved as a wiki link or a wiki raw link
 			// just use wiki link here and it will be redirected to a wiki raw link if necessary
 			linkBase = ctx.Links.WikiLink()
@@ -147,7 +147,7 @@ func shortLinkProcessor(ctx *RenderContext, node *html.Node) {
 		}
 		if image {
 			if !absoluteLink {
-				link = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), link)
+				link = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.ContentMode == RenderContentAsWiki), link)
 			}
 			title := props["title"]
 			if title == "" {

diff --git a/modules/markup/html_node.go b/modules/markup/html_node.go
@@ -17,7 +17,7 @@ func visitNodeImg(ctx *RenderContext, img *html.Node) (next *html.Node) {
 		}
 
 		if IsNonEmptyRelativePath(attr.Val) {
-			attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), attr.Val)
+			attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.ContentMode == RenderContentAsWiki), attr.Val)
 
 			// By default, the "<img>" tag should also be clickable,
 			// because frontend use `<img>` to paste the re-scaled image into the markdown,
@@ -53,7 +53,7 @@ func visitNodeVideo(ctx *RenderContext, node *html.Node) (next *html.Node) {
 			continue
 		}
 		if IsNonEmptyRelativePath(attr.Val) {
-			attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), attr.Val)
+			attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.ContentMode == RenderContentAsWiki), attr.Val)
 		}
 		attr.Val = camoHandleLink(attr.Val)
 		node.Attr[i] = attr

diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go
@@ -14,6 +14,7 @@ import (
 	"code.gitea.io/gitea/modules/markup"
 	"code.gitea.io/gitea/modules/markup/markdown"
 	"code.gitea.io/gitea/modules/setting"
+	testModule "code.gitea.io/gitea/modules/test"
 	"code.gitea.io/gitea/modules/util"
 
 	"github.com/stretchr/testify/assert"
@@ -104,7 +105,7 @@ func TestRender_Commits(t *testing.T) {
 
 func TestRender_CrossReferences(t *testing.T) {
 	setting.AppURL = markup.TestAppURL
-
+	defer testModule.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)()
 	test := func(input, expected string) {
 		buffer, err := markup.RenderString(&markup.RenderContext{
 			Ctx:          git.DefaultContext,
@@ -116,9 +117,7 @@ func TestRender_CrossReferences(t *testing.T) {
 			Metas: localMetas,
 		}, input)
 		assert.NoError(t, err)
-		actual := strings.TrimSpace(buffer)
-		actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "")
-		assert.Equal(t, strings.TrimSpace(expected), actual)
+		assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer))
 	}
 
 	test(
@@ -148,7 +147,7 @@ func TestRender_CrossReferences(t *testing.T) {
 
 func TestRender_links(t *testing.T) {
 	setting.AppURL = markup.TestAppURL
-
+	defer testModule.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)()
 	test := func(input, expected string) {
 		buffer, err := markup.RenderString(&markup.RenderContext{
 			Ctx:          git.DefaultContext,
@@ -158,9 +157,7 @@ func TestRender_links(t *testing.T) {
 			},
 		}, input)
 		assert.NoError(t, err)
-		actual := strings.TrimSpace(buffer)
-		actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "")
-		assert.Equal(t, strings.TrimSpace(expected), actual)
+		assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer))
 	}
 
 	oldCustomURLSchemes := setting.Markdown.CustomURLSchemes
@@ -261,7 +258,7 @@ func TestRender_links(t *testing.T) {
 
 func TestRender_email(t *testing.T) {
 	setting.AppURL = markup.TestAppURL
-
+	defer testModule.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)()
 	test := func(input, expected string) {
 		res, err := markup.RenderString(&markup.RenderContext{
 			Ctx:          git.DefaultContext,
@@ -271,9 +268,7 @@ func TestRender_email(t *testing.T) {
 			},
 		}, input)
 		assert.NoError(t, err)
-		actual := strings.TrimSpace(res)
-		actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "")
-		assert.Equal(t, strings.TrimSpace(expected), actual)
+		assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(res))
 	}
 	// Text that should be turned into email link
 
@@ -302,10 +297,10 @@ func TestRender_email(t *testing.T) {
 	j.doe@example.com;
 	j.doe@example.com?
 	j.doe@example.com!`,
-		`<p><a href="mailto:j.doe@example.com" rel="nofollow">j.doe@example.com</a>,<br/>
-<a href="mailto:j.doe@example.com" rel="nofollow">j.doe@example.com</a>.<br/>
-<a href="mailto:j.doe@example.com" rel="nofollow">j.doe@example.com</a>;<br/>
-<a href="mailto:j.doe@example.com" rel="nofollow">j.doe@example.com</a>?<br/>
+		`<p><a href="mailto:j.doe@example.com" rel="nofollow">j.doe@example.com</a>,
+<a href="mailto:j.doe@example.com" rel="nofollow">j.doe@example.com</a>.
+<a href="mailto:j.doe@example.com" rel="nofollow">j.doe@example.com</a>;
+<a href="mailto:j.doe@example.com" rel="nofollow">j.doe@example.com</a>?
 <a href="mailto:j.doe@example.com" rel="nofollow">j.doe@example.com</a>!</p>`)
 
 	// Test that should *not* be turned into email links
@@ -418,8 +413,8 @@ func TestRender_ShortLinks(t *testing.T) {
 			Links: markup.Links{
 				Base: markup.TestRepoURL,
 			},
-			Metas:  localMetas,
-			IsWiki: true,
+			Metas:       localMetas,
+			ContentMode: markup.RenderContentAsWiki,
 		}, input)
 		assert.NoError(t, err)
 		assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(string(buffer)))
@@ -531,10 +526,10 @@ func TestRender_ShortLinks(t *testing.T) {
 func TestRender_RelativeMedias(t *testing.T) {
 	render := func(input string, isWiki bool, links markup.Links) string {
 		buffer, err := markdown.RenderString(&markup.RenderContext{
-			Ctx:    git.DefaultContext,
-			Links:  links,
-			Metas:  localMetas,
-			IsWiki: isWiki,
+			Ctx:         git.DefaultContext,
+			Links:       links,
+			Metas:       localMetas,
+			ContentMode: util.Iif(isWiki, markup.RenderContentAsWiki, markup.RenderContentAsComment),
 		}, input)
 		assert.NoError(t, err)
 		return strings.TrimSpace(string(buffer))
@@ -604,12 +599,7 @@ func Test_ParseClusterFuzz(t *testing.T) {
 func TestPostProcess_RenderDocument(t *testing.T) {
 	setting.AppURL = markup.TestAppURL
 	setting.StaticURLPrefix = markup.TestAppURL // can't run standalone
-
-	localMetas := map[string]string{
-		"user": "go-gitea",
-		"repo": "gitea",
-		"mode": "document",
-	}
+	defer testModule.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)()
 
 	test := func(input, expected string) {
 		var res strings.Builder
@@ -619,12 +609,10 @@ func TestPostProcess_RenderDocument(t *testing.T) {
 				AbsolutePrefix: true,
 				Base:           "https://example.com",
 			},
-			Metas: localMetas,
+			Metas: map[string]string{"user": "go-gitea", "repo": "gitea"},
 		}, strings.NewReader(input), &res)
 		assert.NoError(t, err)
-		actual := strings.TrimSpace(res.String())
-		actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "")
-		assert.Equal(t, strings.TrimSpace(expected), actual)
+		assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(res.String()))
 	}
 
 	// Issue index shouldn't be post processing in a document.

diff --git a/modules/markup/markdown/goldmark.go b/modules/markup/markdown/goldmark.go
@@ -72,7 +72,12 @@ func (g *ASTTransformer) Transform(node *ast.Document, reader text.Reader, pc pa
 			g.transformList(ctx, v, rc)
 		case *ast.Text:
 			if v.SoftLineBreak() && !v.HardLineBreak() {
-				if ctx.Metas["mode"] != "document" {
+				// TODO: this was a quite unclear part, old code: `if metas["mode"] != "document" { use comment link break setting }`
+				// 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.
+				if markup.RenderBehaviorForTesting.ForceHardLineBreak {
+					v.SetHardLineBreak(true)
+				} else if ctx.ContentMode == markup.RenderContentAsComment {
 					v.SetHardLineBreak(setting.Markdown.EnableHardLineBreakInComments)
 				} else {
 					v.SetHardLineBreak(setting.Markdown.EnableHardLineBreakInDocuments)

diff --git a/modules/markup/markdown/markdown.go b/modules/markup/markdown/markdown.go
@@ -257,9 +257,7 @@ func (Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Wri
 
 // Render renders Markdown to HTML with all specific handling stuff.
 func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error {
-	if ctx.Type == "" {
-		ctx.Type = MarkupName
-	}
+	ctx.MarkupType = MarkupName
 	return markup.Render(ctx, input, output)
 }