Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor some Str2html code #29397

Merged
merged 5 commits into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions models/issues/comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package issues
import (
"context"
"fmt"
"html/template"
"strconv"
"unicode/utf8"

Expand Down Expand Up @@ -259,8 +260,8 @@ type Comment struct {
CommitID int64
Line int64 // - previous line / + proposed line
TreePath string
Content string `xorm:"LONGTEXT"`
RenderedContent string `xorm:"-"`
Content string `xorm:"LONGTEXT"`
RenderedContent template.HTML `xorm:"-"`

// Path represents the 4 lines of code cemented by this comment
Patch string `xorm:"-"`
Expand Down
3 changes: 2 additions & 1 deletion models/issues/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package issues
import (
"context"
"fmt"
"html/template"
"regexp"
"slices"

Expand Down Expand Up @@ -105,7 +106,7 @@ type Issue struct {
OriginalAuthorID int64 `xorm:"index"`
Title string `xorm:"name"`
Content string `xorm:"LONGTEXT"`
RenderedContent string `xorm:"-"`
RenderedContent template.HTML `xorm:"-"`
Labels []*Label `xorm:"-"`
MilestoneID int64 `xorm:"INDEX"`
Milestone *Milestone `xorm:"-"`
Expand Down
5 changes: 3 additions & 2 deletions models/issues/milestone.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package issues
import (
"context"
"fmt"
"html/template"
"strings"

"code.gitea.io/gitea/models/db"
Expand Down Expand Up @@ -47,8 +48,8 @@ type Milestone struct {
RepoID int64 `xorm:"INDEX"`
Repo *repo_model.Repository `xorm:"-"`
Name string
Content string `xorm:"TEXT"`
RenderedContent string `xorm:"-"`
Content string `xorm:"TEXT"`
RenderedContent template.HTML `xorm:"-"`
IsClosed bool
NumIssues int
NumClosedIssues int
Expand Down
3 changes: 2 additions & 1 deletion models/project/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package project
import (
"context"
"fmt"
"html/template"

"code.gitea.io/gitea/models/db"
repo_model "code.gitea.io/gitea/models/repo"
Expand Down Expand Up @@ -100,7 +101,7 @@ type Project struct {
CardType CardType
Type Type

RenderedContent string `xorm:"-"`
RenderedContent template.HTML `xorm:"-"`

CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
Expand Down
3 changes: 2 additions & 1 deletion models/repo/release.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package repo
import (
"context"
"fmt"
"html/template"
"net/url"
"sort"
"strconv"
Expand Down Expand Up @@ -80,7 +81,7 @@ type Release struct {
NumCommits int64
NumCommitsBehind int64 `xorm:"-"`
Note string `xorm:"TEXT"`
RenderedNote string `xorm:"-"`
RenderedNote template.HTML `xorm:"-"`
IsDraft bool `xorm:"NOT NULL DEFAULT false"`
IsPrerelease bool `xorm:"NOT NULL DEFAULT false"`
IsTag bool `xorm:"NOT NULL DEFAULT false"` // will be true only if the record is a tag and has no related releases
Expand Down
8 changes: 4 additions & 4 deletions modules/markup/html_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ func TestRender_ShortLinks(t *testing.T) {
},
}, input)
assert.NoError(t, err)
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer))
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(buffer)))
buffer, err = markdown.RenderString(&markup.RenderContext{
Ctx: git.DefaultContext,
Links: markup.Links{
Expand All @@ -407,7 +407,7 @@ func TestRender_ShortLinks(t *testing.T) {
IsWiki: true,
}, input)
assert.NoError(t, err)
assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(buffer))
assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(string(buffer)))
}

mediatree := util.URLJoin(markup.TestRepoURL, "media", "master")
Expand Down Expand Up @@ -510,7 +510,7 @@ func TestRender_RelativeImages(t *testing.T) {
Metas: localMetas,
}, input)
assert.NoError(t, err)
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer))
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(buffer)))
buffer, err = markdown.RenderString(&markup.RenderContext{
Ctx: git.DefaultContext,
Links: markup.Links{
Expand All @@ -520,7 +520,7 @@ func TestRender_RelativeImages(t *testing.T) {
IsWiki: true,
}, input)
assert.NoError(t, err)
assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(buffer))
assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(string(buffer)))
}

rawwiki := util.URLJoin(markup.TestRepoURL, "wiki", "raw")
Expand Down
5 changes: 3 additions & 2 deletions modules/markup/markdown/markdown.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package markdown

import (
"fmt"
"html/template"
"io"
"strings"
"sync"
Expand Down Expand Up @@ -262,12 +263,12 @@ func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error
}

// RenderString renders Markdown string to HTML with all specific handling stuff and return string
func RenderString(ctx *markup.RenderContext, content string) (string, error) {
func RenderString(ctx *markup.RenderContext, content string) (template.HTML, error) {
var buf strings.Builder
if err := Render(ctx, strings.NewReader(content), &buf); err != nil {
return "", err
}
return buf.String(), nil
return template.HTML(buf.String()), nil
}

// RenderRaw renders Markdown to HTML without handling special links.
Expand Down
30 changes: 16 additions & 14 deletions modules/markup/markdown/markdown_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package markdown_test

import (
"context"
"html/template"
"os"
"strings"
"testing"
Expand Down Expand Up @@ -56,7 +57,7 @@ func TestRender_StandardLinks(t *testing.T) {
},
}, input)
assert.NoError(t, err)
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer))
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(buffer)))

buffer, err = markdown.RenderString(&markup.RenderContext{
Ctx: git.DefaultContext,
Expand All @@ -66,7 +67,7 @@ func TestRender_StandardLinks(t *testing.T) {
IsWiki: true,
}, input)
assert.NoError(t, err)
assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(buffer))
assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(string(buffer)))
}

googleRendered := `<p><a href="https://google.com/" rel="nofollow">https://google.com/</a></p>`
Expand All @@ -90,7 +91,7 @@ func TestRender_Images(t *testing.T) {
},
}, input)
assert.NoError(t, err)
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer))
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(buffer)))
}

url := "../../.images/src/02/train.jpg"
Expand Down Expand Up @@ -299,7 +300,7 @@ func TestTotal_RenderWiki(t *testing.T) {
IsWiki: true,
}, sameCases[i])
assert.NoError(t, err)
assert.Equal(t, answers[i], line)
assert.Equal(t, template.HTML(answers[i]), line)
}

testCases := []string{
Expand All @@ -324,7 +325,7 @@ func TestTotal_RenderWiki(t *testing.T) {
IsWiki: true,
}, testCases[i])
assert.NoError(t, err)
assert.Equal(t, testCases[i+1], line)
assert.Equal(t, template.HTML(testCases[i+1]), line)
}
}

Expand All @@ -343,7 +344,7 @@ func TestTotal_RenderString(t *testing.T) {
Metas: localMetas,
}, sameCases[i])
assert.NoError(t, err)
assert.Equal(t, answers[i], line)
assert.Equal(t, template.HTML(answers[i]), line)
}

testCases := []string{}
Expand All @@ -356,7 +357,7 @@ func TestTotal_RenderString(t *testing.T) {
},
}, testCases[i])
assert.NoError(t, err)
assert.Equal(t, testCases[i+1], line)
assert.Equal(t, template.HTML(testCases[i+1]), line)
}
}

Expand Down Expand Up @@ -423,7 +424,7 @@ func TestRenderEmojiInLinks_Issue12331(t *testing.T) {
`
res, err := markdown.RenderString(&markup.RenderContext{Ctx: git.DefaultContext}, testcase)
assert.NoError(t, err)
assert.Equal(t, expected, res)
assert.Equal(t, template.HTML(expected), res)
}

func TestColorPreview(t *testing.T) {
Expand Down Expand Up @@ -457,7 +458,7 @@ func TestColorPreview(t *testing.T) {
for _, test := range positiveTests {
res, err := markdown.RenderString(&markup.RenderContext{Ctx: git.DefaultContext}, test.testcase)
assert.NoError(t, err, "Unexpected error in testcase: %q", test.testcase)
assert.Equal(t, test.expected, res, "Unexpected result in testcase %q", test.testcase)
assert.Equal(t, template.HTML(test.expected), res, "Unexpected result in testcase %q", test.testcase)

}

Expand Down Expand Up @@ -524,7 +525,7 @@ func TestMathBlock(t *testing.T) {
for _, test := range testcases {
res, err := markdown.RenderString(&markup.RenderContext{Ctx: git.DefaultContext}, test.testcase)
assert.NoError(t, err, "Unexpected error in testcase: %q", test.testcase)
assert.Equal(t, test.expected, res, "Unexpected result in testcase %q", test.testcase)
assert.Equal(t, template.HTML(test.expected), res, "Unexpected result in testcase %q", test.testcase)

}
}
Expand Down Expand Up @@ -562,12 +563,12 @@ foo: bar
for _, test := range testcases {
res, err := markdown.RenderString(&markup.RenderContext{Ctx: git.DefaultContext}, test.testcase)
assert.NoError(t, err, "Unexpected error in testcase: %q", test.testcase)
assert.Equal(t, test.expected, res, "Unexpected result in testcase %q", test.testcase)
assert.Equal(t, template.HTML(test.expected), res, "Unexpected result in testcase %q", test.testcase)
}
}

func TestRenderLinks(t *testing.T) {
input := ` space @mention-user
input := ` space @mention-user${SPACE}${SPACE}
/just/a/path.bin
https://example.com/file.bin
[local link](file.bin)
Expand All @@ -588,8 +589,9 @@ com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit
mail@domain.com
@mention-user test
#123
space
space${SPACE}${SPACE}
`
input = strings.ReplaceAll(input, "${SPACE}", " ") // replace ${SPACE} with " ", to avoid some editor's auto-trimming
cases := []struct {
Links markup.Links
IsWiki bool
Expand Down Expand Up @@ -952,6 +954,6 @@ space</p>
for i, c := range cases {
result, err := markdown.RenderString(&markup.RenderContext{Ctx: context.Background(), Links: c.Links, IsWiki: c.IsWiki}, input)
assert.NoError(t, err, "Unexpected error in testcase: %v", i)
assert.Equal(t, c.Expected, result, "Unexpected result in testcase %v", i)
assert.Equal(t, template.HTML(c.Expected), result, "Unexpected result in testcase %v", i)
}
}
2 changes: 1 addition & 1 deletion modules/templates/util_render.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func RenderMarkdownToHtml(ctx context.Context, input string) template.HTML { //n
if err != nil {
log.Error("RenderString: %v", err)
}
return template.HTML(output)
return output
}

func RenderLabels(ctx context.Context, labels []*issues_model.Label, repoLink string) template.HTML {
Expand Down
15 changes: 15 additions & 0 deletions modules/templates/util_string.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package templates

import (
"fmt"
"html/template"
"strings"

"code.gitea.io/gitea/modules/base"
Expand All @@ -17,6 +19,19 @@ func NewStringUtils() *StringUtils {
return &stringUtils
}

func (su *StringUtils) ToString(v any) string {
switch v := v.(type) {
case string:
return v
case template.HTML:
return string(v)
case fmt.Stringer:
return v.String()
default:
return fmt.Sprint(v)
}
}

func (su *StringUtils) HasPrefix(s, prefix string) bool {
return strings.HasPrefix(s, prefix)
}
Expand Down
21 changes: 13 additions & 8 deletions routers/web/feed/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func toReleaseLink(ctx *context.Context, act *activities_model.Action) string {

// renderMarkdown creates a minimal markdown render context from an action.
// If rendering fails, the original markdown text is returned
func renderMarkdown(ctx *context.Context, act *activities_model.Action, content string) string {
func renderMarkdown(ctx *context.Context, act *activities_model.Action, content string) template.HTML {
markdownCtx := &markup.RenderContext{
Ctx: ctx,
Links: markup.Links{
Expand All @@ -64,7 +64,7 @@ func renderMarkdown(ctx *context.Context, act *activities_model.Action, content
}
markdown, err := markdown.RenderString(markdownCtx, content)
if err != nil {
return content
return templates.Str2html(content) // old code did so: use Str2html to render in tmpl
}
return markdown
}
Expand All @@ -74,7 +74,11 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio
for _, act := range actions {
act.LoadActUser(ctx)

var content, desc, title string
// TODO: the code seems quite strange (maybe not right)
// sometimes it uses text content but sometimes it uses HTML content
// it should clearly defines which kind of content it should use for the feed items: plan text or rich HTML
var title, desc string
var content template.HTML

link := &feeds.Link{Href: act.GetCommentHTMLURL(ctx)}

Expand Down Expand Up @@ -228,7 +232,7 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio
desc = act.GetIssueTitle(ctx)
comment := act.GetIssueInfos()[1]
if len(comment) != 0 {
desc += "\n\n" + renderMarkdown(ctx, act, comment)
desc += "\n\n" + string(renderMarkdown(ctx, act, comment))
}
case activities_model.ActionMergePullRequest, activities_model.ActionAutoMergePullRequest:
desc = act.GetIssueInfos()[1]
Expand All @@ -239,7 +243,7 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio
}
}
if len(content) == 0 {
content = desc
content = templates.Str2html(desc)
}

items = append(items, &feeds.Item{
Expand All @@ -253,7 +257,7 @@ func feedActionsToFeedItems(ctx *context.Context, actions activities_model.Actio
},
Id: fmt.Sprintf("%v: %v", strconv.FormatInt(act.ID, 10), link.Href),
Created: act.CreatedUnix.AsTime(),
Content: content,
Content: string(content),
})
}
return items, err
Expand Down Expand Up @@ -282,7 +286,8 @@ func releasesToFeedItems(ctx *context.Context, releases []*repo_model.Release, i
return nil, err
}

var title, content string
var title string
var content template.HTML

if rel.IsTag {
title = rel.TagName
Expand Down Expand Up @@ -311,7 +316,7 @@ func releasesToFeedItems(ctx *context.Context, releases []*repo_model.Release, i
Email: rel.Publisher.GetEmail(),
},
Id: fmt.Sprintf("%v: %v", strconv.FormatInt(rel.ID, 10), link.Href),
Content: content,
Content: string(content),
})
}

Expand Down
Loading
Loading