Group template helper functions, remove Printf, improve template error messages (#23982)

Follow #23328 


Major changes:

* Group the function in `templates/help.go` by their purposes. It could
make future work easier.
* Remove the `Printf` helper function, there is already a builtin
`printf`.
* Remove `DiffStatsWidth`, replace with `Eval` in template
* Rename the `NewTextFuncMap` to `mailSubjectTextFuncMap`, it's for
subject text template only, no need to make it support HTML functions.


----

And fine tune template error messages, to make it more friendly to
developers and users.


![image](https://user-images.githubusercontent.com/2114189/230714245-4fd202d1-2b25-41b2-8be5-03c5fee45091.png)


![image](https://user-images.githubusercontent.com/2114189/230714277-66783577-2a03-49d5-8e8c-ceba5e07a2d4.png)

---------

Co-authored-by: silverwind <me@silverwind.io>
This commit is contained in:
2023-04-08 21:15:22 +08:00
committed by GitHub
parent cf5a281fdc
commit fdbd646113
15 changed files with 307 additions and 295 deletions

View File

@ -47,7 +47,7 @@ import (
// Render represents a template render
type Render interface {
TemplateLookup(tmpl string) *template.Template
TemplateLookup(tmpl string) (*template.Template, error)
HTML(w io.Writer, status int, name string, data interface{}) error
}
@ -228,7 +228,7 @@ func (ctx *Context) HTML(status int, name base.TplName) {
}
if err := ctx.Render.HTML(ctx.Resp, status, string(name), templates.BaseVars().Merge(ctx.Data)); err != nil {
if status == http.StatusInternalServerError && name == base.TplName("status/500") {
ctx.PlainText(http.StatusInternalServerError, "Unable to find status/500 template")
ctx.PlainText(http.StatusInternalServerError, "Unable to find HTML templates, the template system is not initialized, or Gitea can't find your template files.")
return
}
if execErr, ok := err.(texttemplate.ExecError); ok {
@ -247,7 +247,7 @@ func (ctx *Context) HTML(status int, name base.TplName) {
if errorTemplateName != string(name) {
filename += " (subtemplate of " + string(name) + ")"
}
err = fmt.Errorf("%w\nin template file %s:\n%s", err, filename, templates.GetLineFromTemplate(errorTemplateName, line, target, pos))
err = fmt.Errorf("failed to render %s, error: %w:\n%s", filename, err, templates.GetLineFromTemplate(errorTemplateName, line, target, pos))
} else {
filename, filenameErr := templates.GetAssetFilename("templates/" + execErr.Name + ".tmpl")
if filenameErr != nil {
@ -256,7 +256,7 @@ func (ctx *Context) HTML(status int, name base.TplName) {
if execErr.Name != string(name) {
filename += " (subtemplate of " + string(name) + ")"
}
err = fmt.Errorf("%w\nin template file %s", err, filename)
err = fmt.Errorf("failed to render %s, error: %w", filename, err)
}
}
ctx.ServerError("Render failed", err)

View File

@ -6,20 +6,13 @@
package templates
import (
"html/template"
"io/fs"
"os"
"path/filepath"
texttmpl "text/template"
"code.gitea.io/gitea/modules/setting"
)
var (
subjectTemplates = texttmpl.New("")
bodyTemplates = template.New("")
)
// GetAsset returns asset content via name
func GetAsset(name string) ([]byte, error) {
bs, err := os.ReadFile(filepath.Join(setting.CustomPath, name))

File diff suppressed because it is too large Load Diff

View File

@ -6,6 +6,7 @@ package templates
import (
"bytes"
"context"
"errors"
"fmt"
"html/template"
"io"
@ -15,9 +16,11 @@ import (
"strconv"
"strings"
"sync/atomic"
texttemplate "text/template"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/modules/watcher"
)
@ -34,6 +37,8 @@ type HTMLRender struct {
templates atomic.Pointer[template.Template]
}
var ErrTemplateNotInitialized = errors.New("template system is not initialized, check your log for errors")
func (h *HTMLRender) HTML(w io.Writer, status int, name string, data interface{}) error {
if respWriter, ok := w.(http.ResponseWriter); ok {
if respWriter.Header().Get("Content-Type") == "" {
@ -41,11 +46,23 @@ func (h *HTMLRender) HTML(w io.Writer, status int, name string, data interface{}
}
respWriter.WriteHeader(status)
}
return h.templates.Load().ExecuteTemplate(w, name, data)
t, err := h.TemplateLookup(name)
if err != nil {
return texttemplate.ExecError{Name: name, Err: err}
}
return t.Execute(w, data)
}
func (h *HTMLRender) TemplateLookup(t string) *template.Template {
return h.templates.Load().Lookup(t)
func (h *HTMLRender) TemplateLookup(name string) (*template.Template, error) {
tmpls := h.templates.Load()
if tmpls == nil {
return nil, ErrTemplateNotInitialized
}
tmpl := tmpls.Lookup(name)
if tmpl == nil {
return nil, util.ErrNotExist
}
return tmpl, nil
}
func (h *HTMLRender) CompileTemplates() error {
@ -237,6 +254,12 @@ func GetLineFromTemplate(templateName string, targetLineNum int, target string,
}
}
// FIXME: this algorithm could provide incorrect results and mislead the developers.
// For example: Undefined function "file" in template .....
// {{Func .file.Addition file.Deletion .file.Addition}}
// ^^^^ ^(the real error is here)
// The pointer is added to the first one, but the second one is the real incorrect one.
//
// If there is a provided target to look for in the line add a pointer to it
// e.g. ^^^^^^^
if target != "" {

View File

@ -11,16 +11,53 @@ import (
"strings"
texttmpl "text/template"
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/watcher"
)
// mailSubjectTextFuncMap returns functions for injecting to text templates, it's only used for mail subject
func mailSubjectTextFuncMap() texttmpl.FuncMap {
return texttmpl.FuncMap{
"dict": dict,
"Eval": Eval,
"EllipsisString": base.EllipsisString,
"AppName": func() string {
return setting.AppName
},
"AppDomain": func() string { // documented in mail-templates.md
return setting.Domain
},
}
}
func buildSubjectBodyTemplate(stpl *texttmpl.Template, btpl *template.Template, name string, content []byte) {
// Split template into subject and body
var subjectContent []byte
bodyContent := content
loc := mailSubjectSplit.FindIndex(content)
if loc != nil {
subjectContent = content[0:loc[0]]
bodyContent = content[loc[1]:]
}
if _, err := stpl.New(name).
Parse(string(subjectContent)); err != nil {
log.Warn("Failed to parse template [%s/subject]: %v", name, err)
}
if _, err := btpl.New(name).
Parse(string(bodyContent)); err != nil {
log.Warn("Failed to parse template [%s/body]: %v", name, err)
}
}
// Mailer provides the templates required for sending notification mails.
func Mailer(ctx context.Context) (*texttmpl.Template, *template.Template) {
for _, funcs := range NewTextFuncMap() {
subjectTemplates.Funcs(funcs)
}
subjectTemplates := texttmpl.New("")
bodyTemplates := template.New("")
subjectTemplates.Funcs(mailSubjectTextFuncMap())
for _, funcs := range NewFuncMap() {
bodyTemplates.Funcs(funcs)
}

View File

@ -133,8 +133,8 @@ func (rw *mockResponseWriter) Push(target string, opts *http.PushOptions) error
type mockRender struct{}
func (tr *mockRender) TemplateLookup(tmpl string) *template.Template {
return nil
func (tr *mockRender) TemplateLookup(tmpl string) (*template.Template, error) {
return nil, nil
}
func (tr *mockRender) HTML(w io.Writer, status int, _ string, _ interface{}) error {

View File

@ -578,12 +578,15 @@ func GrantApplicationOAuth(ctx *context.Context) {
// OIDCWellKnown generates JSON so OIDC clients know Gitea's capabilities
func OIDCWellKnown(ctx *context.Context) {
t := ctx.Render.TemplateLookup("user/auth/oidc_wellknown")
t, err := ctx.Render.TemplateLookup("user/auth/oidc_wellknown")
if err != nil {
ctx.ServerError("unable to find template", err)
return
}
ctx.Resp.Header().Set("Content-Type", "application/json")
ctx.Data["SigningKey"] = oauth2.DefaultSigningKey
if err := t.Execute(ctx.Resp, ctx.Data); err != nil {
log.Error("%v", err)
ctx.Error(http.StatusInternalServerError)
if err = t.Execute(ctx.Resp, ctx.Data); err != nil {
ctx.ServerError("unable to execute template", err)
}
}

View File

@ -4,11 +4,8 @@
package web
import (
"net/http"
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/log"
)
// tplSwaggerV1Json swagger v1 json template
@ -16,10 +13,13 @@ const tplSwaggerV1Json base.TplName = "swagger/v1_json"
// SwaggerV1Json render swagger v1 json
func SwaggerV1Json(ctx *context.Context) {
t := ctx.Render.TemplateLookup(string(tplSwaggerV1Json))
t, err := ctx.Render.TemplateLookup(string(tplSwaggerV1Json))
if err != nil {
ctx.ServerError("unable to find template", err)
return
}
ctx.Resp.Header().Set("Content-Type", "application/json")
if err := t.Execute(ctx.Resp, ctx.Data); err != nil {
log.Error("%v", err)
ctx.Error(http.StatusInternalServerError)
if err = t.Execute(ctx.Resp, ctx.Data); err != nil {
ctx.ServerError("unable to execute template", err)
}
}

View File

@ -42,7 +42,7 @@
</div>
{{end}}
{{end}}
{{template "repo/issue/view_content/add_reaction" dict "ctxData" $.root "ActionURL" (Printf "%s/comments/%d/reactions" $.root.RepoLink .ID)}}
{{template "repo/issue/view_content/add_reaction" dict "ctxData" $.root "ActionURL" (printf "%s/comments/%d/reactions" $.root.RepoLink .ID)}}
{{template "repo/issue/view_content/context_menu" dict "ctxData" $.root "item" . "delete" true "issue" false "diff" true "IsCommentPoster" (and $.root.IsSigned (eq $.root.SignedUserID .PosterID))}}
</div>
</div>
@ -60,7 +60,7 @@
{{$reactions := .Reactions.GroupByType}}
{{if $reactions}}
<div class="ui attached segment reactions">
{{template "repo/issue/view_content/reactions" dict "ctxData" $.root "ActionURL" (Printf "%s/comments/%d/reactions" $.root.RepoLink .ID) "Reactions" $reactions}}
{{template "repo/issue/view_content/reactions" dict "ctxData" $.root "ActionURL" (printf "%s/comments/%d/reactions" $.root.RepoLink .ID) "Reactions" $reactions}}
</div>
{{end}}
</div>

View File

@ -1,4 +1,5 @@
{{Eval .file.Addition "+" .file.Deletion}}
<span class="diff-stats-bar gt-mx-3" data-tooltip-content="{{.root.locale.Tr "repo.diff.stats_desc_file" (Eval .file.Addition "+" .file.Deletion) .file.Addition .file.Deletion | Str2html}}">
<div class="diff-stats-add-bar" style="width: {{DiffStatsWidth .file.Addition .file.Deletion}}%"></div>
{{/* if the denominator is zero, then the float result is "width: NaNpx", as before, it just works */}}
<div class="diff-stats-add-bar" style="width: {{Eval 100 "*" .file.Addition "/" "(" .file.Addition "+" .file.Deletion "+" 0.0 ")"}}%"></div>
</span>

View File

@ -64,7 +64,7 @@
{{end}}
{{end}}
{{if not $.Repository.IsArchived}}
{{template "repo/issue/view_content/add_reaction" dict "ctxData" $ "ActionURL" (Printf "%s/issues/%d/reactions" $.RepoLink .Issue.Index)}}
{{template "repo/issue/view_content/add_reaction" dict "ctxData" $ "ActionURL" (printf "%s/issues/%d/reactions" $.RepoLink .Issue.Index)}}
{{template "repo/issue/view_content/context_menu" dict "ctxData" $ "item" .Issue "delete" false "issue" true "diff" false "IsCommentPoster" $.IsIssuePoster}}
{{end}}
</div>
@ -86,7 +86,7 @@
{{$reactions := .Issue.Reactions.GroupByType}}
{{if $reactions}}
<div class="ui attached segment reactions" role="note">
{{template "repo/issue/view_content/reactions" dict "ctxData" $ "ActionURL" (Printf "%s/issues/%d/reactions" $.RepoLink .Issue.Index) "Reactions" $reactions}}
{{template "repo/issue/view_content/reactions" dict "ctxData" $ "ActionURL" (printf "%s/issues/%d/reactions" $.RepoLink .Issue.Index) "Reactions" $reactions}}
</div>
{{end}}
</div>

View File

@ -64,7 +64,7 @@
</div>
{{end}}
{{if not $.Repository.IsArchived}}
{{template "repo/issue/view_content/add_reaction" dict "ctxData" $ "ActionURL" (Printf "%s/comments/%d/reactions" $.RepoLink .ID)}}
{{template "repo/issue/view_content/add_reaction" dict "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID)}}
{{template "repo/issue/view_content/context_menu" dict "ctxData" $ "item" . "delete" true "issue" true "diff" false "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}}
{{end}}
</div>
@ -86,7 +86,7 @@
{{$reactions := .Reactions.GroupByType}}
{{if $reactions}}
<div class="ui attached segment reactions" role="note">
{{template "repo/issue/view_content/reactions" dict "ctxData" $ "ActionURL" (Printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}}
{{template "repo/issue/view_content/reactions" dict "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}}
</div>
{{end}}
</div>
@ -436,7 +436,7 @@
</div>
{{end}}
{{if not $.Repository.IsArchived}}
{{template "repo/issue/view_content/add_reaction" dict "ctxData" $ "ActionURL" (Printf "%s/comments/%d/reactions" $.RepoLink .ID)}}
{{template "repo/issue/view_content/add_reaction" dict "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID)}}
{{template "repo/issue/view_content/context_menu" dict "ctxData" $ "item" . "delete" false "issue" true "diff" false "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}}
{{end}}
</div>
@ -458,7 +458,7 @@
{{$reactions := .Reactions.GroupByType}}
{{if $reactions}}
<div class="ui attached segment reactions">
{{template "repo/issue/view_content/reactions" dict "ctxData" $ "ActionURL" (Printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}}
{{template "repo/issue/view_content/reactions" dict "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}}
</div>
{{end}}
</div>
@ -563,7 +563,7 @@
</div>
{{end}}
{{if not $.Repository.IsArchived}}
{{template "repo/issue/view_content/add_reaction" dict "ctxData" $ "ActionURL" (Printf "%s/comments/%d/reactions" $.RepoLink .ID)}}
{{template "repo/issue/view_content/add_reaction" dict "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID)}}
{{template "repo/issue/view_content/context_menu" dict "ctxData" $ "item" . "delete" true "issue" true "diff" true "IsCommentPoster" (and $.IsSigned (eq $.SignedUserID .PosterID))}}
{{end}}
</div>
@ -582,7 +582,7 @@
{{$reactions := .Reactions.GroupByType}}
{{if $reactions}}
<div class="ui attached segment reactions">
{{template "repo/issue/view_content/reactions" dict "ctxData" $ "ActionURL" (Printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}}
{{template "repo/issue/view_content/reactions" dict "ctxData" $ "ActionURL" (printf "%s/comments/%d/reactions" $.RepoLink .ID) "Reactions" $reactions}}
</div>
{{end}}
</div>

View File

@ -6,9 +6,9 @@
<div class="menu">
{{$referenceUrl := ""}}
{{if .issue}}
{{$referenceUrl = Printf "%s#%s" .ctxData.Issue.Link .item.HashTag}}
{{$referenceUrl = printf "%s#%s" .ctxData.Issue.Link .item.HashTag}}
{{else}}
{{$referenceUrl = Printf "%s/files#%s" .ctxData.Issue.Link .item.HashTag}}
{{$referenceUrl = printf "%s/files#%s" .ctxData.Issue.Link .item.HashTag}}
{{end}}
<div class="item context js-aria-clickable" data-clipboard-text-type="url" data-clipboard-text="{{AppSubUrl}}{{$referenceUrl}}">{{.ctxData.locale.Tr "repo.issues.context.copy_link"}}</div>
<div class="item context js-aria-clickable quote-reply {{if .diff}}quote-reply-diff{{end}}" data-target="{{.item.HashTag}}-raw">{{.ctxData.locale.Tr "repo.issues.context.quote_reply"}}</div>

View File

@ -3,7 +3,7 @@
{{.locale.Tr "repo.issues.context.reference_issue"}}
</div>
<div class="content" style="text-align:left">
<form class="ui form" action="{{Printf "%s/issues/new" .Repository.Link}}" method="post">
<form class="ui form" action="{{printf "%s/issues/new" .Repository.Link}}" method="post">
{{.CsrfTokenHtml}}
<div class="ui segment content">
<div class="field">

View File

@ -6,13 +6,13 @@
<div class="ui three stackable cards">
{{range .Services}}
<a class="ui card gt-df gt-ac" href="{{AppSubUrl}}/repo/migrate?service_type={{.}}&org={{$.Org}}&mirror={{$.Mirror}}">
{{svg (Printf "gitea-%s" .Name) 184}}
{{svg (printf "gitea-%s" .Name) 184}}
<div class="content">
<div class="header gt-tc">
{{.Title}}
</div>
<div class="description gt-tc">
{{(Printf "repo.migrate.%s.description" .Name) | $.locale.Tr}}
{{(printf "repo.migrate.%s.description" .Name) | $.locale.Tr}}
</div>
</div>
</a>