Skip to content

Commit f3eb835

Browse files
wxiaoguangwxiaoguang
authored
Refactor locale&string&template related code (#29165)
Clarify when "string" should be used (and be escaped), and when "template.HTML" should be used (no need to escape) And help PRs like #29059 , to render the error messages correctly.
1 parent 94d06be commit f3eb835

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

77 files changed

+356
-284
lines changed

‎models/actions/runner.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func (r *ActionRunner) StatusName() string {
9797
}
9898

9999
func (r *ActionRunner) StatusLocaleName(lang translation.Locale) string {
100-
return lang.Tr("actions.runners.status." + r.StatusName())
100+
return lang.TrString("actions.runners.status." + r.StatusName())
101101
}
102102

103103
func (r *ActionRunner) IsOnline() bool {

‎models/actions/status.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func (s Status) String() string {
4141

4242
// LocaleString returns the locale string name of the Status
4343
func (s Status) LocaleString(lang translation.Locale) string {
44-
return lang.Tr("actions.status." + s.String())
44+
return lang.TrString("actions.status." + s.String())
4545
}
4646

4747
// IsDone returns whether the Status is final

‎models/git/commit_status.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ func (status *CommitStatus) APIURL(ctx context.Context) string {
194194

195195
// LocaleString returns the locale string name of the Status
196196
func (status *CommitStatus) LocaleString(lang translation.Locale) string {
197-
return lang.Tr("repo.commitstatus." + status.State.String())
197+
return lang.TrString("repo.commitstatus." + status.State.String())
198198
}
199199

200200
// CalcCommitStatus returns commit status state via some status, the commit statues should order by id desc

‎models/issues/comment.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -210,12 +210,12 @@ const (
210210

211211
// LocaleString returns the locale string name of the role
212212
func (r RoleInRepo) LocaleString(lang translation.Locale) string {
213-
return lang.Tr("repo.issues.role." + string(r))
213+
return lang.TrString("repo.issues.role." + string(r))
214214
}
215215

216216
// LocaleHelper returns the locale tooltip of the role
217217
func (r RoleInRepo) LocaleHelper(lang translation.Locale) string {
218-
return lang.Tr("repo.issues.role." + string(r) + "_helper")
218+
return lang.TrString("repo.issues.role." + string(r) + "_helper")
219219
}
220220

221221
// Comment represents a comment in commit and issue page.

‎models/shared/types/ownertype.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@ const (
1717
func (o OwnerType) LocaleString(locale translation.Locale) string {
1818
switch o {
1919
case OwnerTypeSystemGlobal:
20-
return locale.Tr("concept_system_global")
20+
return locale.TrString("concept_system_global")
2121
case OwnerTypeIndividual:
22-
return locale.Tr("concept_user_individual")
22+
return locale.TrString("concept_user_individual")
2323
case OwnerTypeRepository:
24-
return locale.Tr("concept_code_repository")
24+
return locale.TrString("concept_code_repository")
2525
case OwnerTypeOrganization:
26-
return locale.Tr("concept_user_organization")
26+
return locale.TrString("concept_user_organization")
2727
}
28-
return locale.Tr("unknown")
28+
return locale.TrString("unknown")
2929
}

‎modules/auth/password/password.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"context"
99
"crypto/rand"
1010
"errors"
11+
"html/template"
1112
"math/big"
1213
"strings"
1314
"sync"
@@ -121,15 +122,15 @@ func Generate(n int) (string, error) {
121122
}
122123

123124
// BuildComplexityError builds the error message when password complexity checks fail
124-
func BuildComplexityError(locale translation.Locale) string {
125+
func BuildComplexityError(locale translation.Locale) template.HTML {
125126
var buffer bytes.Buffer
126-
buffer.WriteString(locale.Tr("form.password_complexity"))
127+
buffer.WriteString(locale.TrString("form.password_complexity"))
127128
buffer.WriteString("<ul>")
128129
for _, c := range requiredList {
129130
buffer.WriteString("<li>")
130-
buffer.WriteString(locale.Tr(c.TrNameOne))
131+
buffer.WriteString(locale.TrString(c.TrNameOne))
131132
buffer.WriteString("</li>")
132133
}
133134
buffer.WriteString("</ul>")
134-
return buffer.String()
135+
return template.HTML(buffer.String())
135136
}

‎modules/charset/escape_stream.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ func (e *escapeStreamer) ambiguousRune(r, c rune) error {
173173
Val: "ambiguous-code-point",
174174
}, html.Attribute{
175175
Key: "data-tooltip-content",
176-
Val: e.locale.Tr("repo.ambiguous_character", r, c),
176+
Val: e.locale.TrString("repo.ambiguous_character", r, c),
177177
}); err != nil {
178178
return err
179179
}

‎modules/context/api.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ func APIContexter() func(http.Handler) http.Handler {
245245
// NotFound handles 404s for APIContext
246246
// String will replace message, errors will be added to a slice
247247
func (ctx *APIContext) NotFound(objs ...any) {
248-
message := ctx.Tr("error.not_found")
248+
message := ctx.Locale.TrString("error.not_found")
249249
var errors []string
250250
for _, obj := range objs {
251251
// Ignore nil

‎modules/context/base.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package context
66
import (
77
"context"
88
"fmt"
9+
"html/template"
910
"io"
1011
"net/http"
1112
"net/url"
@@ -286,11 +287,11 @@ func (b *Base) cleanUp() {
286287
}
287288
}
288289

289-
func (b *Base) Tr(msg string, args ...any) string {
290+
func (b *Base) Tr(msg string, args ...any) template.HTML {
290291
return b.Locale.Tr(msg, args...)
291292
}
292293

293-
func (b *Base) TrN(cnt any, key1, keyN string, args ...any) string {
294+
func (b *Base) TrN(cnt any, key1, keyN string, args ...any) template.HTML {
294295
return b.Locale.TrN(cnt, key1, keyN, args...)
295296
}
296297

‎modules/context/context.go

+10-13
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ package context
66

77
import (
88
"context"
9-
"html"
9+
"fmt"
1010
"html/template"
1111
"io"
1212
"net/http"
@@ -71,16 +71,6 @@ func init() {
7171
})
7272
}
7373

74-
// TrHTMLEscapeArgs runs ".Locale.Tr()" but pre-escapes all arguments with html.EscapeString.
75-
// This is useful if the locale message is intended to only produce HTML content.
76-
func (ctx *Context) TrHTMLEscapeArgs(msg string, args ...string) string {
77-
trArgs := make([]any, len(args))
78-
for i, arg := range args {
79-
trArgs[i] = html.EscapeString(arg)
80-
}
81-
return ctx.Locale.Tr(msg, trArgs...)
82-
}
83-
8474
type webContextKeyType struct{}
8575

8676
var WebContextKey = webContextKeyType{}
@@ -253,6 +243,13 @@ func (ctx *Context) JSONOK() {
253243
ctx.JSON(http.StatusOK, map[string]any{"ok": true}) // this is only a dummy response, frontend seldom uses it
254244
}
255245

256-
func (ctx *Context) JSONError(msg string) {
257-
ctx.JSON(http.StatusBadRequest, map[string]any{"errorMessage": msg})
246+
func (ctx *Context) JSONError(msg any) {
247+
switch v := msg.(type) {
248+
case string:
249+
ctx.JSON(http.StatusBadRequest, map[string]any{"errorMessage": v, "renderFormat": "text"})
250+
case template.HTML:
251+
ctx.JSON(http.StatusBadRequest, map[string]any{"errorMessage": v, "renderFormat": "html"})
252+
default:
253+
panic(fmt.Sprintf("unsupported type: %T", msg))
254+
}
258255
}

‎modules/context/context_response.go

+2-3
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,11 @@ func (ctx *Context) RenderToString(name base.TplName, data map[string]any) (stri
9898
}
9999

100100
// RenderWithErr used for page has form validation but need to prompt error to users.
101-
func (ctx *Context) RenderWithErr(msg string, tpl base.TplName, form any) {
101+
func (ctx *Context) RenderWithErr(msg any, tpl base.TplName, form any) {
102102
if form != nil {
103103
middleware.AssignForm(form, ctx.Data)
104104
}
105-
ctx.Flash.ErrorMsg = msg
106-
ctx.Data["Flash"] = ctx.Flash
105+
ctx.Flash.Error(msg, true)
107106
ctx.HTML(http.StatusOK, tpl)
108107
}
109108

‎modules/context/repo.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package context
66

77
import (
88
"context"
9+
"errors"
910
"fmt"
1011
"html"
1112
"net/http"
@@ -85,7 +86,7 @@ func (r *Repository) CanCreateBranch() bool {
8586
func RepoMustNotBeArchived() func(ctx *Context) {
8687
return func(ctx *Context) {
8788
if ctx.Repo.Repository.IsArchived {
88-
ctx.NotFound("IsArchived", fmt.Errorf(ctx.Tr("repo.archive.title")))
89+
ctx.NotFound("IsArchived", errors.New(ctx.Locale.TrString("repo.archive.title")))
8990
}
9091
}
9192
}

‎modules/csv/csv.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,9 @@ func guessDelimiter(data []byte) rune {
123123
func FormatError(err error, locale translation.Locale) (string, error) {
124124
if perr, ok := err.(*stdcsv.ParseError); ok {
125125
if perr.Err == stdcsv.ErrFieldCount {
126-
return locale.Tr("repo.error.csv.invalid_field_count", perr.Line), nil
126+
return locale.TrString("repo.error.csv.invalid_field_count", perr.Line), nil
127127
}
128-
return locale.Tr("repo.error.csv.unexpected", perr.Line, perr.Column), nil
128+
return locale.TrString("repo.error.csv.unexpected", perr.Line, perr.Column), nil
129129
}
130130

131131
return "", err

‎modules/markup/html.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -804,7 +804,7 @@ func fullIssuePatternProcessor(ctx *RenderContext, node *html.Node) {
804804
// indicate that in the text by appending (comment)
805805
if m[4] != -1 && m[5] != -1 {
806806
if locale, ok := ctx.Ctx.Value(translation.ContextKey).(translation.Locale); ok {
807-
text += " " + locale.Tr("repo.from_comment")
807+
text += " " + locale.TrString("repo.from_comment")
808808
} else {
809809
text += " (comment)"
810810
}

‎modules/markup/markdown/toc.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ func createTOCNode(toc []markup.Header, lang string, detailsAttrs map[string]str
2121
details.SetAttributeString(k, []byte(v))
2222
}
2323

24-
summary.AppendChild(summary, ast.NewString([]byte(translation.NewLocale(lang).Tr("toc"))))
24+
summary.AppendChild(summary, ast.NewString([]byte(translation.NewLocale(lang).TrString("toc"))))
2525
details.AppendChild(details, summary)
2626
ul := ast.NewList('-')
2727
details.AppendChild(details, ul)

‎modules/migration/messenger.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
package migration
55

6-
// Messenger is a formatting function similar to i18n.Tr
6+
// Messenger is a formatting function similar to i18n.TrString
77
type Messenger func(key string, args ...any)
88

99
// NilMessenger represents an empty formatting function

‎modules/templates/helper.go

+39-7
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ func NewFuncMap() template.FuncMap {
3636
"dict": dict, // it's lowercase because this name has been widely used. Our other functions should have uppercase names.
3737
"Eval": Eval,
3838
"Safe": Safe,
39-
"Escape": html.EscapeString,
39+
"Escape": Escape,
4040
"QueryEscape": url.QueryEscape,
4141
"JSEscape": template.JSEscapeString,
4242
"Str2html": Str2html, // TODO: rename it to SanitizeHTML
@@ -159,7 +159,7 @@ func NewFuncMap() template.FuncMap {
159159
"RenderCodeBlock": RenderCodeBlock,
160160
"RenderIssueTitle": RenderIssueTitle,
161161
"RenderEmoji": RenderEmoji,
162-
"RenderEmojiPlain": emoji.ReplaceAliases,
162+
"RenderEmojiPlain": RenderEmojiPlain,
163163
"ReactionToEmoji": ReactionToEmoji,
164164

165165
"RenderMarkdownToHtml": RenderMarkdownToHtml,
@@ -180,13 +180,45 @@ func NewFuncMap() template.FuncMap {
180180
}
181181

182182
// Safe render raw as HTML
183-
func Safe(raw string) template.HTML {
184-
return template.HTML(raw)
183+
func Safe(s any) template.HTML {
184+
switch v := s.(type) {
185+
case string:
186+
return template.HTML(v)
187+
case template.HTML:
188+
return v
189+
}
190+
panic(fmt.Sprintf("unexpected type %T", s))
191+
}
192+
193+
// Str2html sanitizes the input by pre-defined markdown rules
194+
func Str2html(s any) template.HTML {
195+
switch v := s.(type) {
196+
case string:
197+
return template.HTML(markup.Sanitize(v))
198+
case template.HTML:
199+
return template.HTML(markup.Sanitize(string(v)))
200+
}
201+
panic(fmt.Sprintf("unexpected type %T", s))
185202
}
186203

187-
// Str2html render Markdown text to HTML
188-
func Str2html(raw string) template.HTML {
189-
return template.HTML(markup.Sanitize(raw))
204+
func Escape(s any) template.HTML {
205+
switch v := s.(type) {
206+
case string:
207+
return template.HTML(html.EscapeString(v))
208+
case template.HTML:
209+
return v
210+
}
211+
panic(fmt.Sprintf("unexpected type %T", s))
212+
}
213+
214+
func RenderEmojiPlain(s any) any {
215+
switch v := s.(type) {
216+
case string:
217+
return emoji.ReplaceAliases(v)
218+
case template.HTML:
219+
return template.HTML(emoji.ReplaceAliases(string(v)))
220+
}
221+
panic(fmt.Sprintf("unexpected type %T", s))
190222
}
191223

192224
// DotEscape wraps a dots in names with ZWJ [U+200D] in order to prevent autolinkers from detecting these as urls

0 commit comments

Comments
 (0)