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

Fix issue title rendering and refactor legacy function names #32703

Merged
merged 1 commit into from
Dec 4, 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
2 changes: 1 addition & 1 deletion models/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ func (repo *Repository) CanEnableEditor() bool {

// DescriptionHTML does special handles to description and return HTML string.
func (repo *Repository) DescriptionHTML(ctx context.Context) template.HTML {
desc, err := markup.RenderDescriptionHTML(markup.NewRenderContext(ctx), repo.Description)
desc, err := markup.PostProcessDescriptionHTML(markup.NewRenderContext(ctx), repo.Description)
if err != nil {
log.Error("Failed to render description for %s (ID: %d): %v", repo.Name, repo.ID, err)
return template.HTML(markup.SanitizeDescription(repo.Description))
Expand Down
45 changes: 25 additions & 20 deletions modules/markup/html.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,9 @@ func PostProcessDefault(ctx *RenderContext, input io.Reader, output io.Writer) e
return postProcess(ctx, procs, input, output)
}

// RenderCommitMessage will use the same logic as PostProcess, but will disable
// PostProcessCommitMessage will use the same logic as PostProcess, but will disable
// the shortLinkProcessor.
func RenderCommitMessage(ctx *RenderContext, content string) (string, error) {
func PostProcessCommitMessage(ctx *RenderContext, content string) (string, error) {
procs := []processor{
fullIssuePatternProcessor,
comparePatternProcessor,
Expand All @@ -183,11 +183,11 @@ var emojiProcessors = []processor{
emojiProcessor,
}

// RenderCommitMessageSubject will use the same logic as PostProcess and
// RenderCommitMessage, but will disable the shortLinkProcessor and
// PostProcessCommitMessageSubject will use the same logic as PostProcess and
// PostProcessCommitMessage, but will disable the shortLinkProcessor and
// 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) {
func PostProcessCommitMessageSubject(ctx *RenderContext, defaultLink, content string) (string, error) {
procs := []processor{
fullIssuePatternProcessor,
comparePatternProcessor,
Expand All @@ -211,40 +211,45 @@ func RenderCommitMessageSubject(ctx *RenderContext, defaultLink, content string)
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.
// PostProcessIssueTitle to process title on individual issue/pull page
func PostProcessIssueTitle(ctx *RenderContext, title string) (string, error) {
return postProcessString(ctx, []processor{
issueIndexPatternProcessor,
commitCrossReferencePatternProcessor,
hashCurrentPatternProcessor,
emojiShortCodeProcessor,
emojiProcessor,
}, title)
}

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
}
return buf.String(), nil
}

// RenderDescriptionHTML will use similar logic as PostProcess, but will
// PostProcessDescriptionHTML will use similar logic as PostProcess, but will
// use a single special linkProcessor.
func RenderDescriptionHTML(ctx *RenderContext, content string) (string, error) {
func PostProcessDescriptionHTML(ctx *RenderContext, content string) (string, error) {
return postProcessString(ctx, []processor{
descriptionLinkProcessor,
emojiShortCodeProcessor,
emojiProcessor,
}, content)
}

// RenderEmoji for when we want to just process emoji and shortcodes
// PostProcessEmoji 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) {
func PostProcessEmoji(ctx *RenderContext, content string) (string, error) {
return postProcessString(ctx, emojiProcessors, content)
}

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
}
return buf.String(), nil
}

func postProcess(ctx *RenderContext, procs []processor, input io.Reader, output io.Writer) error {
if !ctx.usedByRender && ctx.RenderHelper != nil {
defer ctx.RenderHelper.CleanUp()
}
// FIXME: don't read all content to memory
rawHTML, err := io.ReadAll(input)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions modules/markup/html_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,15 +252,15 @@ func TestRender_IssueIndexPattern_NoShortPattern(t *testing.T) {
testRenderIssueIndexPattern(t, "!1", "!1", NewTestRenderContext(metas))
}

func TestRender_RenderIssueTitle(t *testing.T) {
func TestRender_PostProcessIssueTitle(t *testing.T) {
setting.AppURL = TestAppURL
metas := map[string]string{
"format": "https://someurl.com/{user}/{repo}/{index}",
"user": "someUser",
"repo": "someRepo",
"style": IssueNameStyleNumeric,
}
actual, err := RenderIssueTitle(NewTestRenderContext(metas), "#1")
actual, err := PostProcessIssueTitle(NewTestRenderContext(metas), "#1")
assert.NoError(t, err)
assert.Equal(t, "#1", actual)
}
Expand Down
4 changes: 4 additions & 0 deletions modules/markup/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ type RenderOptions struct {
type RenderContext struct {
ctx context.Context

// the context might be used by the "render" function, but it might also be used by "postProcess" function
usedByRender bool

SidebarTocNode ast.Node

RenderHelper RenderHelper
Expand Down Expand Up @@ -182,6 +185,7 @@ func pipes() (io.ReadCloser, io.WriteCloser, func()) {
}

func render(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Writer) error {
ctx.usedByRender = true
if ctx.RenderHelper != nil {
defer ctx.RenderHelper.CleanUp()
}
Expand Down
27 changes: 17 additions & 10 deletions modules/templates/util_render.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ func (ut *RenderUtils) RenderCommitMessage(msg string, metas map[string]string)
cleanMsg := template.HTMLEscapeString(msg)
// we can safely assume that it will not return any error, since there
// shouldn't be any special HTML.
fullMessage, err := markup.RenderCommitMessage(markup.NewRenderContext(ut.ctx).WithMetas(metas), cleanMsg)
fullMessage, err := markup.PostProcessCommitMessage(markup.NewRenderContext(ut.ctx).WithMetas(metas), cleanMsg)
if err != nil {
log.Error("RenderCommitMessage: %v", err)
log.Error("PostProcessCommitMessage: %v", err)
return ""
}
msgLines := strings.Split(strings.TrimSpace(fullMessage), "\n")
Expand All @@ -65,9 +65,9 @@ func (ut *RenderUtils) RenderCommitMessageLinkSubject(msg, urlDefault string, me

// we can safely assume that it will not return any error, since there
// shouldn't be any special HTML.
renderedMessage, err := markup.RenderCommitMessageSubject(markup.NewRenderContext(ut.ctx).WithMetas(metas), urlDefault, template.HTMLEscapeString(msgLine))
renderedMessage, err := markup.PostProcessCommitMessageSubject(markup.NewRenderContext(ut.ctx).WithMetas(metas), urlDefault, template.HTMLEscapeString(msgLine))
if err != nil {
log.Error("RenderCommitMessageSubject: %v", err)
log.Error("PostProcessCommitMessageSubject: %v", err)
return ""
}
return renderCodeBlock(template.HTML(renderedMessage))
Expand All @@ -87,9 +87,9 @@ func (ut *RenderUtils) RenderCommitBody(msg string, metas map[string]string) tem
return ""
}

renderedMessage, err := markup.RenderCommitMessage(markup.NewRenderContext(ut.ctx).WithMetas(metas), template.HTMLEscapeString(msgLine))
renderedMessage, err := markup.PostProcessCommitMessage(markup.NewRenderContext(ut.ctx).WithMetas(metas), template.HTMLEscapeString(msgLine))
if err != nil {
log.Error("RenderCommitMessage: %v", err)
log.Error("PostProcessCommitMessage: %v", err)
return ""
}
return template.HTML(renderedMessage)
Expand All @@ -106,12 +106,19 @@ func renderCodeBlock(htmlEscapedTextToRender template.HTML) template.HTML {

// RenderIssueTitle renders issue/pull title with defined post processors
func (ut *RenderUtils) RenderIssueTitle(text string, metas map[string]string) template.HTML {
renderedText, err := markup.RenderIssueTitle(markup.NewRenderContext(ut.ctx).WithMetas(metas), template.HTMLEscapeString(text))
renderedText, err := markup.PostProcessIssueTitle(markup.NewRenderContext(ut.ctx).WithMetas(metas), template.HTMLEscapeString(text))
if err != nil {
log.Error("RenderIssueTitle: %v", err)
log.Error("PostProcessIssueTitle: %v", err)
return ""
}
return template.HTML(renderedText)
return renderCodeBlock(template.HTML(renderedText))
}

// RenderIssueSimpleTitle only renders with emoji and inline code block
func (ut *RenderUtils) RenderIssueSimpleTitle(text string) template.HTML {
ret := ut.RenderEmoji(text)
ret = renderCodeBlock(ret)
return ret
}
silverwind marked this conversation as resolved.
Show resolved Hide resolved

// RenderLabel renders a label
Expand Down Expand Up @@ -174,7 +181,7 @@ func (ut *RenderUtils) RenderLabel(label *issues_model.Label) template.HTML {

// RenderEmoji renders html text with emoji post processors
func (ut *RenderUtils) RenderEmoji(text string) template.HTML {
renderedText, err := markup.RenderEmoji(markup.NewRenderContext(ut.ctx), template.HTMLEscapeString(text))
renderedText, err := markup.PostProcessEmoji(markup.NewRenderContext(ut.ctx), template.HTMLEscapeString(text))
if err != nil {
log.Error("RenderEmoji: %v", err)
return ""
Expand Down
4 changes: 2 additions & 2 deletions modules/templates/util_render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,11 @@ com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit
<span class="emoji" aria-label="thumbs up">👍</span>
[email protected]
@mention-user test
#123
<a href="/user13/repo11/issues/123" class="ref-issue">#123</a>
space<SPACE><SPACE>
`
expected = strings.ReplaceAll(expected, "<SPACE>", " ")
assert.EqualValues(t, expected, string(newTestRenderUtils().RenderIssueTitle(testInput(), nil)))
assert.EqualValues(t, expected, string(newTestRenderUtils().RenderIssueTitle(testInput(), testMetas)))
}

func TestRenderMarkdownToHtml(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions routers/web/repo/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,9 +394,9 @@ func Diff(ctx *context.Context) {
ctx.Data["NoteCommit"] = note.Commit
ctx.Data["NoteAuthor"] = user_model.ValidateCommitWithEmail(ctx, note.Commit)
rctx := renderhelper.NewRenderContextRepoComment(ctx, ctx.Repo.Repository, renderhelper.RepoCommentOptions{CurrentRefPath: path.Join("commit", util.PathEscapeSegments(commitID))})
ctx.Data["NoteRendered"], err = markup.RenderCommitMessage(rctx, template.HTMLEscapeString(string(charset.ToUTF8WithFallback(note.Message, charset.ConvertOpts{}))))
ctx.Data["NoteRendered"], err = markup.PostProcessCommitMessage(rctx, template.HTMLEscapeString(string(charset.ToUTF8WithFallback(note.Message, charset.ConvertOpts{}))))
if err != nil {
ctx.ServerError("RenderCommitMessage", err)
ctx.ServerError("PostProcessCommitMessage", err)
return
}
}
Expand Down
2 changes: 1 addition & 1 deletion templates/repo/diff/compare.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@
<div class="ui segment flex-text-block tw-gap-4">
{{template "shared/issueicon" .}}
<div class="issue-title tw-break-anywhere">
{{ctx.RenderUtils.RenderIssueTitle .PullRequest.Issue.Title ($.Repository.ComposeMetas ctx) | RenderCodeBlock}}
{{ctx.RenderUtils.RenderIssueTitle .PullRequest.Issue.Title ($.Repository.ComposeMetas ctx)}}
<span class="index">#{{.PullRequest.Issue.Index}}</span>
</div>
<a href="{{$.RepoLink}}/pulls/{{.PullRequest.Issue.Index}}" class="ui compact button primary">
Expand Down
2 changes: 1 addition & 1 deletion templates/repo/issue/card.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<div class="issue-card-icon">
{{template "shared/issueicon" .}}
</div>
<a class="issue-card-title muted issue-title tw-break-anywhere" href="{{.Link}}">{{.Title | ctx.RenderUtils.RenderEmoji | RenderCodeBlock}}</a>
<a class="issue-card-title muted issue-title tw-break-anywhere" href="{{.Link}}">{{.Title | ctx.RenderUtils.RenderIssueSimpleTitle}}</a>
{{if and $.isPinnedIssueCard $.Page.IsRepoAdmin}}
<a role="button" class="issue-card-unpin muted tw-flex tw-items-center" data-tooltip-content={{ctx.Locale.Tr "repo.issues.unpin_issue"}} data-issue-id="{{.ID}}" data-unpin-url="{{$.Page.Link}}/unpin/{{.Index}}">
{{svg "octicon-x" 16}}
Expand Down
2 changes: 1 addition & 1 deletion templates/repo/issue/view_title.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
{{$canEditIssueTitle := and (or .HasIssuesOrPullsWritePermission .IsIssuePoster) (not .Repository.IsArchived)}}
<div class="issue-title" id="issue-title-display">
<h1 class="tw-break-anywhere">
{{ctx.RenderUtils.RenderIssueTitle .Issue.Title ($.Repository.ComposeMetas ctx) | RenderCodeBlock}}
{{ctx.RenderUtils.RenderIssueTitle .Issue.Title ($.Repository.ComposeMetas ctx)}}
<span class="index">#{{.Issue.Index}}</span>
</h1>
<div class="issue-title-buttons">
Expand Down
14 changes: 7 additions & 7 deletions templates/repo/pulse.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@
<span class="ui green label">{{ctx.Locale.Tr "repo.activity.published_release_label"}}</span>
{{.TagName}}
{{if not .IsTag}}
<a class="title" href="{{$.RepoLink}}/src/{{.TagName | PathEscapeSegments}}">{{.Title | ctx.RenderUtils.RenderEmoji | RenderCodeBlock}}</a>
<a class="title" href="{{$.RepoLink}}/src/{{.TagName | PathEscapeSegments}}">{{.Title | ctx.RenderUtils.RenderIssueSimpleTitle}}</a>
{{end}}
{{DateUtils.TimeSince .CreatedUnix}}
</p>
Expand All @@ -145,7 +145,7 @@
{{range .Activity.MergedPRs}}
<p class="desc">
<span class="ui purple label">{{ctx.Locale.Tr "repo.activity.merged_prs_label"}}</span>
#{{.Index}} <a class="title" href="{{$.RepoLink}}/pulls/{{.Index}}">{{.Issue.Title | ctx.RenderUtils.RenderEmoji | RenderCodeBlock}}</a>
#{{.Index}} <a class="title" href="{{$.RepoLink}}/pulls/{{.Index}}">{{.Issue.Title | ctx.RenderUtils.RenderIssueSimpleTitle}}</a>
{{DateUtils.TimeSince .MergedUnix}}
</p>
{{end}}
Expand All @@ -164,7 +164,7 @@
{{range .Activity.OpenedPRs}}
<p class="desc">
<span class="ui green label">{{ctx.Locale.Tr "repo.activity.opened_prs_label"}}</span>
#{{.Index}} <a class="title" href="{{$.RepoLink}}/pulls/{{.Index}}">{{.Issue.Title | ctx.RenderUtils.RenderEmoji | RenderCodeBlock}}</a>
#{{.Index}} <a class="title" href="{{$.RepoLink}}/pulls/{{.Index}}">{{.Issue.Title | ctx.RenderUtils.RenderIssueSimpleTitle}}</a>
{{DateUtils.TimeSince .Issue.CreatedUnix}}
</p>
{{end}}
Expand All @@ -183,7 +183,7 @@
{{range .Activity.ClosedIssues}}
<p class="desc">
<span class="ui red label">{{ctx.Locale.Tr "repo.activity.closed_issue_label"}}</span>
#{{.Index}} <a class="title" href="{{$.RepoLink}}/issues/{{.Index}}">{{.Title | ctx.RenderUtils.RenderEmoji | RenderCodeBlock}}</a>
#{{.Index}} <a class="title" href="{{$.RepoLink}}/issues/{{.Index}}">{{.Title | ctx.RenderUtils.RenderIssueSimpleTitle}}</a>
{{DateUtils.TimeSince .ClosedUnix}}
</p>
{{end}}
Expand All @@ -202,7 +202,7 @@
{{range .Activity.OpenedIssues}}
<p class="desc">
<span class="ui green label">{{ctx.Locale.Tr "repo.activity.new_issue_label"}}</span>
#{{.Index}} <a class="title" href="{{$.RepoLink}}/issues/{{.Index}}">{{.Title | ctx.RenderUtils.RenderEmoji | RenderCodeBlock}}</a>
#{{.Index}} <a class="title" href="{{$.RepoLink}}/issues/{{.Index}}">{{.Title | ctx.RenderUtils.RenderIssueSimpleTitle}}</a>
{{DateUtils.TimeSince .CreatedUnix}}
</p>
{{end}}
Expand All @@ -220,9 +220,9 @@
<span class="ui green label">{{ctx.Locale.Tr "repo.activity.unresolved_conv_label"}}</span>
#{{.Index}}
{{if .IsPull}}
<a class="title" href="{{$.RepoLink}}/pulls/{{.Index}}">{{.Title | ctx.RenderUtils.RenderEmoji | RenderCodeBlock}}</a>
<a class="title" href="{{$.RepoLink}}/pulls/{{.Index}}">{{.Title | ctx.RenderUtils.RenderIssueSimpleTitle}}</a>
{{else}}
<a class="title" href="{{$.RepoLink}}/issues/{{.Index}}">{{.Title | ctx.RenderUtils.RenderEmoji | RenderCodeBlock}}</a>
<a class="title" href="{{$.RepoLink}}/issues/{{.Index}}">{{.Title | ctx.RenderUtils.RenderIssueSimpleTitle}}</a>
{{end}}
{{DateUtils.TimeSince .UpdatedUnix}}
</p>
Expand Down
2 changes: 1 addition & 1 deletion templates/shared/issuelist.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<div class="flex-item-main">
<div class="flex-item-header">
<div class="flex-item-title">
<a class="tw-no-underline issue-title" href="{{if .Link}}{{.Link}}{{else}}{{$.Link}}/{{.Index}}{{end}}">{{ctx.RenderUtils.RenderEmoji .Title | RenderCodeBlock}}</a>
<a class="tw-no-underline issue-title" href="{{if .Link}}{{.Link}}{{else}}{{$.Link}}/{{.Index}}{{end}}">{{.Title | ctx.RenderUtils.RenderIssueSimpleTitle}}</a>
{{if .IsPull}}
{{if (index $.CommitStatuses .PullRequest.ID)}}
{{template "repo/commit_statuses" dict "Status" (index $.CommitLastStatus .PullRequest.ID) "Statuses" (index $.CommitStatuses .PullRequest.ID)}}
Expand Down
8 changes: 4 additions & 4 deletions templates/user/dashboard/feeds.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -100,19 +100,19 @@
<a href="{{AppSubUrl}}/{{$push.CompareURL}}">{{ctx.Locale.Tr "action.compare_commits" $push.Len}} »</a>
{{end}}
{{else if .GetOpType.InActions "create_issue"}}
<span class="text truncate issue title">{{index .GetIssueInfos 1 | ctx.RenderUtils.RenderEmoji | RenderCodeBlock}}</span>
<span class="text truncate issue title">{{index .GetIssueInfos 1 | ctx.RenderUtils.RenderIssueSimpleTitle}}</span>
{{else if .GetOpType.InActions "create_pull_request"}}
<span class="text truncate issue title">{{index .GetIssueInfos 1 | ctx.RenderUtils.RenderEmoji | RenderCodeBlock}}</span>
<span class="text truncate issue title">{{index .GetIssueInfos 1 | ctx.RenderUtils.RenderIssueSimpleTitle}}</span>
{{else if .GetOpType.InActions "comment_issue" "approve_pull_request" "reject_pull_request" "comment_pull"}}
<a href="{{.GetCommentLink ctx}}" class="text truncate issue title">{{(.GetIssueTitle ctx) | ctx.RenderUtils.RenderEmoji | RenderCodeBlock}}</a>
<a href="{{.GetCommentLink ctx}}" class="text truncate issue title">{{(.GetIssueTitle ctx) | ctx.RenderUtils.RenderIssueSimpleTitle}}</a>
{{$comment := index .GetIssueInfos 1}}
{{if $comment}}
<div class="markup tw-text-14">{{ctx.RenderUtils.MarkdownToHtml $comment}}</div>
{{end}}
{{else if .GetOpType.InActions "merge_pull_request"}}
<div class="flex-item-body text black">{{index .GetIssueInfos 1}}</div>
{{else if .GetOpType.InActions "close_issue" "reopen_issue" "close_pull_request" "reopen_pull_request"}}
<span class="text truncate issue title">{{(.GetIssueTitle ctx) | ctx.RenderUtils.RenderEmoji | RenderCodeBlock}}</span>
<span class="text truncate issue title">{{(.GetIssueTitle ctx) | ctx.RenderUtils.RenderIssueSimpleTitle}}</span>
{{else if .GetOpType.InActions "pull_review_dismissed"}}
<div class="flex-item-body text black">{{ctx.Locale.Tr "action.review_dismissed_reason"}}</div>
<div class="flex-item-body text black">{{index .GetIssueInfos 2 | ctx.RenderUtils.RenderEmoji}}</div>
Expand Down
2 changes: 1 addition & 1 deletion templates/user/notification/notification_div.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
<div class="notifications-bottom-row tw-text-16 tw-py-0.5">
<span class="issue-title tw-break-anywhere">
{{if .Issue}}
{{.Issue.Title | ctx.RenderUtils.RenderEmoji | RenderCodeBlock}}
{{.Issue.Title | ctx.RenderUtils.RenderIssueSimpleTitle}}
{{else}}
{{.Repository.FullName}}
{{end}}
Expand Down
Loading