Skip to content

Commit 3f9c3e7

Browse files
authored
Refactor render system (#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
1 parent 985e2a8 commit 3f9c3e7

32 files changed

+237
-257
lines changed

models/repo/repo.go

-2
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,6 @@ func (repo *Repository) ComposeMetas(ctx context.Context) map[string]string {
479479
metas := map[string]string{
480480
"user": repo.OwnerName,
481481
"repo": repo.Name,
482-
"mode": "comment",
483482
}
484483

485484
unit, err := repo.GetUnit(ctx, unit.TypeExternalTracker)
@@ -521,7 +520,6 @@ func (repo *Repository) ComposeDocumentMetas(ctx context.Context) map[string]str
521520
for k, v := range repo.ComposeMetas(ctx) {
522521
metas[k] = v
523522
}
524-
metas["mode"] = "document"
525523
repo.DocumentRenderingMetas = metas
526524
}
527525
return repo.DocumentRenderingMetas

modules/markup/console/console.go

+1-22
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"io"
99
"path/filepath"
1010
"regexp"
11-
"strings"
1211

1312
"code.gitea.io/gitea/modules/markup"
1413
"code.gitea.io/gitea/modules/setting"
@@ -17,9 +16,6 @@ import (
1716
"github.com/go-enry/go-enry/v2"
1817
)
1918

20-
// MarkupName describes markup's name
21-
var MarkupName = "console"
22-
2319
func init() {
2420
markup.RegisterRenderer(Renderer{})
2521
}
@@ -29,7 +25,7 @@ type Renderer struct{}
2925

3026
// Name implements markup.Renderer
3127
func (Renderer) Name() string {
32-
return MarkupName
28+
return "console"
3329
}
3430

3531
// Extensions implements markup.Renderer
@@ -67,20 +63,3 @@ func (Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Wri
6763
_, err = output.Write(buf)
6864
return err
6965
}
70-
71-
// Render renders terminal colors to HTML with all specific handling stuff.
72-
func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error {
73-
if ctx.Type == "" {
74-
ctx.Type = MarkupName
75-
}
76-
return markup.Render(ctx, input, output)
77-
}
78-
79-
// RenderString renders terminal colors in string to HTML with all specific handling stuff and return string
80-
func RenderString(ctx *markup.RenderContext, content string) (string, error) {
81-
var buf strings.Builder
82-
if err := Render(ctx, strings.NewReader(content), &buf); err != nil {
83-
return "", err
84-
}
85-
return buf.String(), nil
86-
}

modules/markup/html.go

+4-5
Original file line numberDiff line numberDiff line change
@@ -442,12 +442,11 @@ func createLink(href, content, class string) *html.Node {
442442
a := &html.Node{
443443
Type: html.ElementNode,
444444
Data: atom.A.String(),
445-
Attr: []html.Attribute{
446-
{Key: "href", Val: href},
447-
{Key: "data-markdown-generated-content"},
448-
},
445+
Attr: []html.Attribute{{Key: "href", Val: href}},
446+
}
447+
if !RenderBehaviorForTesting.DisableInternalAttributes {
448+
a.Attr = append(a.Attr, html.Attribute{Key: "data-markdown-generated-content"})
449449
}
450-
451450
if class != "" {
452451
a.Attr = append(a.Attr, html.Attribute{Key: "class", Val: class})
453452
}

modules/markup/html_codepreview_test.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"code.gitea.io/gitea/modules/git"
1313
"code.gitea.io/gitea/modules/markup"
14+
"code.gitea.io/gitea/modules/markup/markdown"
1415

1516
"github.com/stretchr/testify/assert"
1617
)
@@ -23,8 +24,8 @@ func TestRenderCodePreview(t *testing.T) {
2324
})
2425
test := func(input, expected string) {
2526
buffer, err := markup.RenderString(&markup.RenderContext{
26-
Ctx: git.DefaultContext,
27-
Type: "markdown",
27+
Ctx: git.DefaultContext,
28+
MarkupType: markdown.MarkupName,
2829
}, input)
2930
assert.NoError(t, err)
3031
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer))

modules/markup/html_internal_test.go

+11-11
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"code.gitea.io/gitea/modules/git"
1313
"code.gitea.io/gitea/modules/setting"
14+
testModule "code.gitea.io/gitea/modules/test"
1415
"code.gitea.io/gitea/modules/util"
1516

1617
"github.com/stretchr/testify/assert"
@@ -123,8 +124,9 @@ func TestRender_IssueIndexPattern2(t *testing.T) {
123124
}
124125
expectedNil := fmt.Sprintf(expectedFmt, links...)
125126
testRenderIssueIndexPattern(t, s, expectedNil, &RenderContext{
126-
Ctx: git.DefaultContext,
127-
Metas: localMetas,
127+
Ctx: git.DefaultContext,
128+
Metas: localMetas,
129+
ContentMode: RenderContentAsComment,
128130
})
129131

130132
class := "ref-issue"
@@ -137,8 +139,9 @@ func TestRender_IssueIndexPattern2(t *testing.T) {
137139
}
138140
expectedNum := fmt.Sprintf(expectedFmt, links...)
139141
testRenderIssueIndexPattern(t, s, expectedNum, &RenderContext{
140-
Ctx: git.DefaultContext,
141-
Metas: numericMetas,
142+
Ctx: git.DefaultContext,
143+
Metas: numericMetas,
144+
ContentMode: RenderContentAsComment,
142145
})
143146
}
144147

@@ -266,7 +269,6 @@ func TestRender_IssueIndexPattern_Document(t *testing.T) {
266269
"user": "someUser",
267270
"repo": "someRepo",
268271
"style": IssueNameStyleNumeric,
269-
"mode": "document",
270272
}
271273

272274
testRenderIssueIndexPattern(t, "#1", "#1", &RenderContext{
@@ -316,8 +318,8 @@ func TestRender_AutoLink(t *testing.T) {
316318
Links: Links{
317319
Base: TestRepoURL,
318320
},
319-
Metas: localMetas,
320-
IsWiki: true,
321+
Metas: localMetas,
322+
ContentMode: RenderContentAsWiki,
321323
}, strings.NewReader(input), &buffer)
322324
assert.Equal(t, err, nil)
323325
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer.String()))
@@ -340,7 +342,7 @@ func TestRender_AutoLink(t *testing.T) {
340342

341343
func TestRender_FullIssueURLs(t *testing.T) {
342344
setting.AppURL = TestAppURL
343-
345+
defer testModule.MockVariableValue(&RenderBehaviorForTesting.DisableInternalAttributes, true)()
344346
test := func(input, expected string) {
345347
var result strings.Builder
346348
err := postProcess(&RenderContext{
@@ -351,9 +353,7 @@ func TestRender_FullIssueURLs(t *testing.T) {
351353
Metas: localMetas,
352354
}, []processor{fullIssuePatternProcessor}, strings.NewReader(input), &result)
353355
assert.NoError(t, err)
354-
actual := result.String()
355-
actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "")
356-
assert.Equal(t, expected, actual)
356+
assert.Equal(t, expected, result.String())
357357
}
358358
test("Here is a link https://git.osgeo.org/gogs/postgis/postgis/pulls/6",
359359
"Here is a link https://git.osgeo.org/gogs/postgis/postgis/pulls/6")

modules/markup/html_issue.go

+2-3
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,8 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) {
6767
return
6868
}
6969

70-
// FIXME: the use of "mode" is quite dirty and hacky, for example: what is a "document"? how should it be rendered?
71-
// The "mode" approach should be refactored to some other more clear&reliable way.
72-
crossLinkOnly := ctx.Metas["mode"] == "document" && !ctx.IsWiki
70+
// crossLinkOnly if not comment and not wiki
71+
crossLinkOnly := ctx.ContentMode != RenderContentAsTitle && ctx.ContentMode != RenderContentAsComment && ctx.ContentMode != RenderContentAsWiki
7372

7473
var (
7574
found bool

modules/markup/html_link.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func ResolveLink(ctx *RenderContext, link, userContentAnchorPrefix string) (resu
2020
isAnchorFragment := link != "" && link[0] == '#'
2121
if !isAnchorFragment && !IsFullURLString(link) {
2222
linkBase := ctx.Links.Base
23-
if ctx.IsWiki {
23+
if ctx.ContentMode == RenderContentAsWiki {
2424
// no need to check if the link should be resolved as a wiki link or a wiki raw link
2525
// just use wiki link here and it will be redirected to a wiki raw link if necessary
2626
linkBase = ctx.Links.WikiLink()
@@ -147,7 +147,7 @@ func shortLinkProcessor(ctx *RenderContext, node *html.Node) {
147147
}
148148
if image {
149149
if !absoluteLink {
150-
link = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), link)
150+
link = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.ContentMode == RenderContentAsWiki), link)
151151
}
152152
title := props["title"]
153153
if title == "" {

modules/markup/html_node.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ func visitNodeImg(ctx *RenderContext, img *html.Node) (next *html.Node) {
1717
}
1818

1919
if IsNonEmptyRelativePath(attr.Val) {
20-
attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), attr.Val)
20+
attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.ContentMode == RenderContentAsWiki), attr.Val)
2121

2222
// By default, the "<img>" tag should also be clickable,
2323
// 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) {
5353
continue
5454
}
5555
if IsNonEmptyRelativePath(attr.Val) {
56-
attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), attr.Val)
56+
attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.ContentMode == RenderContentAsWiki), attr.Val)
5757
}
5858
attr.Val = camoHandleLink(attr.Val)
5959
node.Attr[i] = attr

modules/markup/html_test.go

+20-32
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"code.gitea.io/gitea/modules/markup"
1515
"code.gitea.io/gitea/modules/markup/markdown"
1616
"code.gitea.io/gitea/modules/setting"
17+
testModule "code.gitea.io/gitea/modules/test"
1718
"code.gitea.io/gitea/modules/util"
1819

1920
"github.com/stretchr/testify/assert"
@@ -104,7 +105,7 @@ func TestRender_Commits(t *testing.T) {
104105

105106
func TestRender_CrossReferences(t *testing.T) {
106107
setting.AppURL = markup.TestAppURL
107-
108+
defer testModule.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)()
108109
test := func(input, expected string) {
109110
buffer, err := markup.RenderString(&markup.RenderContext{
110111
Ctx: git.DefaultContext,
@@ -116,9 +117,7 @@ func TestRender_CrossReferences(t *testing.T) {
116117
Metas: localMetas,
117118
}, input)
118119
assert.NoError(t, err)
119-
actual := strings.TrimSpace(buffer)
120-
actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "")
121-
assert.Equal(t, strings.TrimSpace(expected), actual)
120+
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer))
122121
}
123122

124123
test(
@@ -148,7 +147,7 @@ func TestRender_CrossReferences(t *testing.T) {
148147

149148
func TestRender_links(t *testing.T) {
150149
setting.AppURL = markup.TestAppURL
151-
150+
defer testModule.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)()
152151
test := func(input, expected string) {
153152
buffer, err := markup.RenderString(&markup.RenderContext{
154153
Ctx: git.DefaultContext,
@@ -158,9 +157,7 @@ func TestRender_links(t *testing.T) {
158157
},
159158
}, input)
160159
assert.NoError(t, err)
161-
actual := strings.TrimSpace(buffer)
162-
actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "")
163-
assert.Equal(t, strings.TrimSpace(expected), actual)
160+
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer))
164161
}
165162

166163
oldCustomURLSchemes := setting.Markdown.CustomURLSchemes
@@ -261,7 +258,7 @@ func TestRender_links(t *testing.T) {
261258

262259
func TestRender_email(t *testing.T) {
263260
setting.AppURL = markup.TestAppURL
264-
261+
defer testModule.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)()
265262
test := func(input, expected string) {
266263
res, err := markup.RenderString(&markup.RenderContext{
267264
Ctx: git.DefaultContext,
@@ -271,9 +268,7 @@ func TestRender_email(t *testing.T) {
271268
},
272269
}, input)
273270
assert.NoError(t, err)
274-
actual := strings.TrimSpace(res)
275-
actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "")
276-
assert.Equal(t, strings.TrimSpace(expected), actual)
271+
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(res))
277272
}
278273
// Text that should be turned into email link
279274

@@ -302,10 +297,10 @@ func TestRender_email(t *testing.T) {
302297
j.doe@example.com;
303298
j.doe@example.com?
304299
j.doe@example.com!`,
305-
`<p><a href="mailto:j.doe@example.com" rel="nofollow">j.doe@example.com</a>,<br/>
306-
<a href="mailto:j.doe@example.com" rel="nofollow">j.doe@example.com</a>.<br/>
307-
<a href="mailto:j.doe@example.com" rel="nofollow">j.doe@example.com</a>;<br/>
308-
<a href="mailto:j.doe@example.com" rel="nofollow">j.doe@example.com</a>?<br/>
300+
`<p><a href="mailto:j.doe@example.com" rel="nofollow">j.doe@example.com</a>,
301+
<a href="mailto:j.doe@example.com" rel="nofollow">j.doe@example.com</a>.
302+
<a href="mailto:j.doe@example.com" rel="nofollow">j.doe@example.com</a>;
303+
<a href="mailto:j.doe@example.com" rel="nofollow">j.doe@example.com</a>?
309304
<a href="mailto:j.doe@example.com" rel="nofollow">j.doe@example.com</a>!</p>`)
310305

311306
// Test that should *not* be turned into email links
@@ -418,8 +413,8 @@ func TestRender_ShortLinks(t *testing.T) {
418413
Links: markup.Links{
419414
Base: markup.TestRepoURL,
420415
},
421-
Metas: localMetas,
422-
IsWiki: true,
416+
Metas: localMetas,
417+
ContentMode: markup.RenderContentAsWiki,
423418
}, input)
424419
assert.NoError(t, err)
425420
assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(string(buffer)))
@@ -531,10 +526,10 @@ func TestRender_ShortLinks(t *testing.T) {
531526
func TestRender_RelativeMedias(t *testing.T) {
532527
render := func(input string, isWiki bool, links markup.Links) string {
533528
buffer, err := markdown.RenderString(&markup.RenderContext{
534-
Ctx: git.DefaultContext,
535-
Links: links,
536-
Metas: localMetas,
537-
IsWiki: isWiki,
529+
Ctx: git.DefaultContext,
530+
Links: links,
531+
Metas: localMetas,
532+
ContentMode: util.Iif(isWiki, markup.RenderContentAsWiki, markup.RenderContentAsComment),
538533
}, input)
539534
assert.NoError(t, err)
540535
return strings.TrimSpace(string(buffer))
@@ -604,12 +599,7 @@ func Test_ParseClusterFuzz(t *testing.T) {
604599
func TestPostProcess_RenderDocument(t *testing.T) {
605600
setting.AppURL = markup.TestAppURL
606601
setting.StaticURLPrefix = markup.TestAppURL // can't run standalone
607-
608-
localMetas := map[string]string{
609-
"user": "go-gitea",
610-
"repo": "gitea",
611-
"mode": "document",
612-
}
602+
defer testModule.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)()
613603

614604
test := func(input, expected string) {
615605
var res strings.Builder
@@ -619,12 +609,10 @@ func TestPostProcess_RenderDocument(t *testing.T) {
619609
AbsolutePrefix: true,
620610
Base: "https://example.com",
621611
},
622-
Metas: localMetas,
612+
Metas: map[string]string{"user": "go-gitea", "repo": "gitea"},
623613
}, strings.NewReader(input), &res)
624614
assert.NoError(t, err)
625-
actual := strings.TrimSpace(res.String())
626-
actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "")
627-
assert.Equal(t, strings.TrimSpace(expected), actual)
615+
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(res.String()))
628616
}
629617

630618
// Issue index shouldn't be post processing in a document.

modules/markup/markdown/goldmark.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,12 @@ func (g *ASTTransformer) Transform(node *ast.Document, reader text.Reader, pc pa
7272
g.transformList(ctx, v, rc)
7373
case *ast.Text:
7474
if v.SoftLineBreak() && !v.HardLineBreak() {
75-
if ctx.Metas["mode"] != "document" {
75+
// TODO: this was a quite unclear part, old code: `if metas["mode"] != "document" { use comment link break setting }`
76+
// many places render non-comment contents with no mode=document, then these contents also use comment's hard line break setting
77+
// especially in many tests.
78+
if markup.RenderBehaviorForTesting.ForceHardLineBreak {
79+
v.SetHardLineBreak(true)
80+
} else if ctx.ContentMode == markup.RenderContentAsComment {
7681
v.SetHardLineBreak(setting.Markdown.EnableHardLineBreakInComments)
7782
} else {
7883
v.SetHardLineBreak(setting.Markdown.EnableHardLineBreakInDocuments)

modules/markup/markdown/markdown.go

+1-3
Original file line numberDiff line numberDiff line change
@@ -257,9 +257,7 @@ func (Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Wri
257257

258258
// Render renders Markdown to HTML with all specific handling stuff.
259259
func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error {
260-
if ctx.Type == "" {
261-
ctx.Type = MarkupName
262-
}
260+
ctx.MarkupType = MarkupName
263261
return markup.Render(ctx, input, output)
264262
}
265263

0 commit comments

Comments
 (0)