From 88201914767e15a51c910d295647d97973fa09bf Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 23 Apr 2023 02:16:22 +0800 Subject: [PATCH] Improve template helper functions: string/slice (#24266) Follow #23328 The improvements: 1. The `contains` functions are covered by tests 2. The inconsistent behavior of `containGeneric` is replaced by `StringUtils.Contains` and `SliceUtils.Contains` 3. In the future we can move more help functions into XxxUtils to simplify the `helper.go` and reduce unnecessary global functions. FAQ: 1. Why it's called `StringUtils.Contains` but not `strings.Contains` like Golang? Because our `StringUtils` is not Golang's `strings` package. There will be our own string functions. --------- Co-authored-by: silverwind --- modules/templates/helper.go | 36 +++---------------- modules/templates/{util.go => util_dict.go} | 0 modules/templates/util_slice.go | 35 ++++++++++++++++++ modules/templates/util_string.go | 20 +++++++++++ modules/templates/util_test.go | 36 +++++++++++++++++++ templates/repo/graph/commits.tmpl | 2 +- templates/repo/header.tmpl | 2 +- templates/repo/issue/list.tmpl | 2 +- templates/repo/issue/milestone_issues.tmpl | 4 +-- .../repo/issue/view_content/attachments.tmpl | 4 +-- templates/repo/settings/tags.tmpl | 4 +-- 11 files changed, 105 insertions(+), 40 deletions(-) rename modules/templates/{util.go => util_dict.go} (100%) create mode 100644 modules/templates/util_slice.go create mode 100644 modules/templates/util_string.go diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 42680827eb..c5b77989be 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -15,7 +15,6 @@ import ( "mime" "net/url" "path/filepath" - "reflect" "regexp" "strings" "time" @@ -68,11 +67,15 @@ func NewFuncMap() []template.FuncMap { "PathEscape": url.PathEscape, "PathEscapeSegments": util.PathEscapeSegments, + // utils + "StringUtils": NewStringUtils, + "SliceUtils": NewSliceUtils, + // ----------------------------------------------------------------- // string / json + // TODO: move string helper functions to StringUtils "Join": strings.Join, "DotEscape": DotEscape, - "HasPrefix": strings.HasPrefix, "EllipsisString": base.EllipsisString, "DumpVar": dumpVar, @@ -144,35 +147,6 @@ func NewFuncMap() []template.FuncMap { return fmt.Sprint(time.Since(startTime).Nanoseconds()/1e6) + "ms" }, - // ----------------------------------------------------------------- - // slice - "containGeneric": func(arr, v interface{}) bool { - arrV := reflect.ValueOf(arr) - if arrV.Kind() == reflect.String && reflect.ValueOf(v).Kind() == reflect.String { - return strings.Contains(arr.(string), v.(string)) - } - if arrV.Kind() == reflect.Slice { - for i := 0; i < arrV.Len(); i++ { - iV := arrV.Index(i) - if !iV.CanInterface() { - continue - } - if iV.Interface() == v { - return true - } - } - } - return false - }, - "contain": func(s []int64, id int64) bool { - for i := 0; i < len(s); i++ { - if s[i] == id { - return true - } - } - return false - }, - // ----------------------------------------------------------------- // setting "AppName": func() string { diff --git a/modules/templates/util.go b/modules/templates/util_dict.go similarity index 100% rename from modules/templates/util.go rename to modules/templates/util_dict.go diff --git a/modules/templates/util_slice.go b/modules/templates/util_slice.go new file mode 100644 index 0000000000..a3318cc11a --- /dev/null +++ b/modules/templates/util_slice.go @@ -0,0 +1,35 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package templates + +import ( + "fmt" + "reflect" +) + +type SliceUtils struct{} + +func NewSliceUtils() *SliceUtils { + return &SliceUtils{} +} + +func (su *SliceUtils) Contains(s, v any) bool { + if s == nil { + return false + } + sv := reflect.ValueOf(s) + if sv.Kind() != reflect.Slice && sv.Kind() != reflect.Array { + panic(fmt.Sprintf("invalid type, expected slice or array, but got: %T", s)) + } + for i := 0; i < sv.Len(); i++ { + it := sv.Index(i) + if !it.CanInterface() { + continue + } + if it.Interface() == v { + return true + } + } + return false +} diff --git a/modules/templates/util_string.go b/modules/templates/util_string.go new file mode 100644 index 0000000000..e86bbe9e70 --- /dev/null +++ b/modules/templates/util_string.go @@ -0,0 +1,20 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package templates + +import "strings" + +type StringUtils struct{} + +func NewStringUtils() *StringUtils { + return &StringUtils{} +} + +func (su *StringUtils) HasPrefix(s, prefix string) bool { + return strings.HasPrefix(s, prefix) +} + +func (su *StringUtils) Contains(s, substr string) bool { + return strings.Contains(s, substr) +} diff --git a/modules/templates/util_test.go b/modules/templates/util_test.go index dfa691c5e2..febaf7fa88 100644 --- a/modules/templates/util_test.go +++ b/modules/templates/util_test.go @@ -4,6 +4,9 @@ package templates import ( + "html/template" + "io" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -41,3 +44,36 @@ func TestDict(t *testing.T) { assert.Error(t, err) } } + +func TestUtils(t *testing.T) { + execTmpl := func(code string, data any) string { + tmpl := template.New("test") + tmpl.Funcs(template.FuncMap{"SliceUtils": NewSliceUtils, "StringUtils": NewStringUtils}) + template.Must(tmpl.Parse(code)) + w := &strings.Builder{} + assert.NoError(t, tmpl.Execute(w, data)) + return w.String() + } + + actual := execTmpl("{{SliceUtils.Contains .Slice .Value}}", map[string]any{"Slice": []string{"a", "b"}, "Value": "a"}) + assert.Equal(t, "true", actual) + + actual = execTmpl("{{SliceUtils.Contains .Slice .Value}}", map[string]any{"Slice": []string{"a", "b"}, "Value": "x"}) + assert.Equal(t, "false", actual) + + actual = execTmpl("{{SliceUtils.Contains .Slice .Value}}", map[string]any{"Slice": []int64{1, 2}, "Value": int64(2)}) + assert.Equal(t, "true", actual) + + actual = execTmpl("{{StringUtils.Contains .String .Value}}", map[string]any{"String": "abc", "Value": "b"}) + assert.Equal(t, "true", actual) + + actual = execTmpl("{{StringUtils.Contains .String .Value}}", map[string]any{"String": "abc", "Value": "x"}) + assert.Equal(t, "false", actual) + + tmpl := template.New("test") + tmpl.Funcs(template.FuncMap{"SliceUtils": NewSliceUtils, "StringUtils": NewStringUtils}) + template.Must(tmpl.Parse("{{SliceUtils.Contains .Slice .Value}}")) + // error is like this: `template: test:1:12: executing "test" at : error calling Contains: ...` + err := tmpl.Execute(io.Discard, map[string]any{"Slice": struct{}{}}) + assert.ErrorContains(t, err, "invalid type, expected slice or array") +} diff --git a/templates/repo/graph/commits.tmpl b/templates/repo/graph/commits.tmpl index 4a01fefedd..44f63a7d76 100644 --- a/templates/repo/graph/commits.tmpl +++ b/templates/repo/graph/commits.tmpl @@ -35,7 +35,7 @@ {{range $commit.Refs}} {{$refGroup := .RefGroup}} {{if eq $refGroup "pull"}} - {{if or (not $.HidePRRefs) (containGeneric $.SelectedBranches .Name)}} + {{if or (not $.HidePRRefs) (SliceUtils.Contains $.SelectedBranches .Name)}} {{svg "octicon-git-pull-request" 16 "gt-mr-2"}}#{{.ShortName}} diff --git a/templates/repo/header.tmpl b/templates/repo/header.tmpl index ef2549eb7c..a999c73804 100644 --- a/templates/repo/header.tmpl +++ b/templates/repo/header.tmpl @@ -217,7 +217,7 @@ {{end}} {{if or (.Permission.CanRead $.UnitTypeWiki) (.Permission.CanRead $.UnitTypeExternalWiki)}} - + {{svg "octicon-book"}} {{.locale.Tr "repo.wiki"}} {{end}} diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index 6cc390cd5e..9c31262355 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -227,7 +227,7 @@ {{end}} {{$previousExclusiveScope = $exclusiveScope}}
- {{if contain $.SelLabelIDs .ID}}{{if $exclusiveScope}}{{svg "octicon-dot-fill"}}{{else}}{{svg "octicon-check"}}{{end}}{{end}} {{RenderLabel $.Context .}} + {{if SliceUtils.Contains $.SelLabelIDs .ID}}{{if $exclusiveScope}}{{svg "octicon-dot-fill"}}{{else}}{{svg "octicon-check"}}{{end}}{{end}} {{RenderLabel $.Context .}}
{{end}} diff --git a/templates/repo/issue/milestone_issues.tmpl b/templates/repo/issue/milestone_issues.tmpl index 1d55eb39cd..137f2c2d9d 100644 --- a/templates/repo/issue/milestone_issues.tmpl +++ b/templates/repo/issue/milestone_issues.tmpl @@ -65,7 +65,7 @@ {{.locale.Tr "repo.issues.filter_label_exclude" | Safe}} {{.locale.Tr "repo.issues.filter_label_no_select"}} {{range .Labels}} - {{if .IsExcluded}}{{svg "octicon-circle-slash"}}{{else if contain $.SelLabelIDs .ID}}{{svg "octicon-check"}}{{end}} {{RenderLabel $.Context .}} + {{if .IsExcluded}}{{svg "octicon-circle-slash"}}{{else if SliceUtils.Contains $.SelLabelIDs .ID}}{{svg "octicon-check"}}{{end}} {{RenderLabel $.Context .}} {{end}} @@ -171,7 +171,7 @@
{{range .Labels}}
- {{if contain $.SelLabelIDs .ID}}{{svg "octicon-check"}}{{end}} {{RenderLabel $.Context .}} + {{if SliceUtils.Contains $.SelLabelIDs .ID}}{{svg "octicon-check"}}{{end}} {{RenderLabel $.Context .}}
{{end}}
diff --git a/templates/repo/issue/view_content/attachments.tmpl b/templates/repo/issue/view_content/attachments.tmpl index 7a429c31ec..f342340fe4 100644 --- a/templates/repo/issue/view_content/attachments.tmpl +++ b/templates/repo/issue/view_content/attachments.tmpl @@ -8,7 +8,7 @@
{{if FilenameIsImage .Name}} - {{if not (containGeneric $.Content .UUID)}} + {{if not (StringUtils.Contains $.Content .UUID)}} {{$hasThumbnails = true}} {{end}} {{svg "octicon-file"}} @@ -29,7 +29,7 @@
{{- range .Attachments -}} {{if FilenameIsImage .Name}} - {{if not (containGeneric $.Content .UUID)}} + {{if not (StringUtils.Contains $.Content .UUID)}} {{.Name}} diff --git a/templates/repo/settings/tags.tmpl b/templates/repo/settings/tags.tmpl index 0ebaff6fd2..6387a20de6 100644 --- a/templates/repo/settings/tags.tmpl +++ b/templates/repo/settings/tags.tmpl @@ -92,14 +92,14 @@ {{if or .AllowlistUserIDs (and $.Owner.IsOrganization .AllowlistTeamIDs)}} {{$userIDs := .AllowlistUserIDs}} {{range $.Users}} - {{if contain $userIDs .ID}} + {{if SliceUtils.Contains $userIDs .ID}} {{avatar $.Context . 26}} {{.GetDisplayName}} {{end}} {{end}} {{if $.Owner.IsOrganization}} {{$teamIDs := .AllowlistTeamIDs}} {{range $.Teams}} - {{if contain $teamIDs .ID}} + {{if SliceUtils.Contains $teamIDs .ID}} {{.Name}} {{end}} {{end}}