aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorwxiaoguang <wxiaoguang@gmail.com>2023-04-10 16:44:02 +0800
committerGitHub <noreply@github.com>2023-04-10 16:44:02 +0800
commit4e3348135723dfc03dcc91b196b5da6f20b8a4ea (patch)
tree8734e1d537d93243dd6ba478b9f2890cc8ac0d89
parentbb6c670cff1a081d9f5f8bdb3dc91abe5d9e35b9 (diff)
downloadgitea-4e3348135723dfc03dcc91b196b5da6f20b8a4ea.tar.gz
gitea-4e3348135723dfc03dcc91b196b5da6f20b8a4ea.zip
Make label templates have consistent behavior and priority (#23749)
Fix https://github.com/go-gitea/gitea/issues/23715 Other related PRs: * #23717 * #23716 * #23719 This PR is different from others, it tries to resolve the problem fundamentally (and brings more benefits) Although it looks like some more lines are added, actually many new lines are for tests. ---- Before, the code was just "guessing" the file type and try to parse them. <details> ![image](https://user-images.githubusercontent.com/2114189/228002245-57d58e27-1078-4da9-bf42-5bc0b264c6ce.png) </details> This PR: * Always remember the original option file names, and always use correct parser for them. * Another benefit is that we can sort the Label Templates now (before there was a map, its key order is undefined) ![image](https://user-images.githubusercontent.com/2114189/228002432-931b9f18-3908-484b-a36b-04760c9ad132.png)
-rw-r--r--modules/label/parser.go48
-rw-r--r--modules/repository/create.go3
-rw-r--r--modules/repository/init.go117
-rw-r--r--modules/repository/init_test.go30
-rw-r--r--routers/web/org/setting.go2
-rw-r--r--routers/web/repo/issue_label.go2
-rw-r--r--routers/web/repo/issue_label_test.go2
-rw-r--r--routers/web/repo/repo.go4
-rw-r--r--services/repository/repository.go4
-rw-r--r--templates/repo/create.tmpl4
-rw-r--r--templates/repo/issue/labels/label_load_template.tmpl4
11 files changed, 133 insertions, 87 deletions
diff --git a/modules/label/parser.go b/modules/label/parser.go
index 55bf570de6..511bac823f 100644
--- a/modules/label/parser.go
+++ b/modules/label/parser.go
@@ -30,31 +30,23 @@ func IsErrTemplateLoad(err error) bool {
}
func (err ErrTemplateLoad) Error() string {
- return fmt.Sprintf("Failed to load label template file '%s': %v", err.TemplateFile, err.OriginalError)
+ return fmt.Sprintf("failed to load label template file %q: %v", err.TemplateFile, err.OriginalError)
}
-// GetTemplateFile loads the label template file by given name,
-// then parses and returns a list of name-color pairs and optionally description.
-func GetTemplateFile(name string) ([]*Label, error) {
- data, err := options.Labels(name + ".yaml")
- if err == nil && len(data) > 0 {
- return parseYamlFormat(name+".yaml", data)
- }
-
- data, err = options.Labels(name + ".yml")
- if err == nil && len(data) > 0 {
- return parseYamlFormat(name+".yml", data)
- }
-
- data, err = options.Labels(name)
+// LoadTemplateFile loads the label template file by given file name, returns a slice of Label structs.
+func LoadTemplateFile(fileName string) ([]*Label, error) {
+ data, err := options.Labels(fileName)
if err != nil {
- return nil, ErrTemplateLoad{name, fmt.Errorf("GetRepoInitFile: %w", err)}
+ return nil, ErrTemplateLoad{fileName, fmt.Errorf("LoadTemplateFile: %w", err)}
}
- return parseLegacyFormat(name, data)
+ if strings.HasSuffix(fileName, ".yaml") || strings.HasSuffix(fileName, ".yml") {
+ return parseYamlFormat(fileName, data)
+ }
+ return parseLegacyFormat(fileName, data)
}
-func parseYamlFormat(name string, data []byte) ([]*Label, error) {
+func parseYamlFormat(fileName string, data []byte) ([]*Label, error) {
lf := &labelFile{}
if err := yaml.Unmarshal(data, lf); err != nil {
@@ -65,11 +57,11 @@ func parseYamlFormat(name string, data []byte) ([]*Label, error) {
for _, l := range lf.Labels {
l.Color = strings.TrimSpace(l.Color)
if len(l.Name) == 0 || len(l.Color) == 0 {
- return nil, ErrTemplateLoad{name, errors.New("label name and color are required fields")}
+ return nil, ErrTemplateLoad{fileName, errors.New("label name and color are required fields")}
}
color, err := NormalizeColor(l.Color)
if err != nil {
- return nil, ErrTemplateLoad{name, fmt.Errorf("bad HTML color code '%s' in label: %s", l.Color, l.Name)}
+ return nil, ErrTemplateLoad{fileName, fmt.Errorf("bad HTML color code '%s' in label: %s", l.Color, l.Name)}
}
l.Color = color
}
@@ -77,7 +69,7 @@ func parseYamlFormat(name string, data []byte) ([]*Label, error) {
return lf.Labels, nil
}
-func parseLegacyFormat(name string, data []byte) ([]*Label, error) {
+func parseLegacyFormat(fileName string, data []byte) ([]*Label, error) {
lines := strings.Split(string(data), "\n")
list := make([]*Label, 0, len(lines))
for i := 0; i < len(lines); i++ {
@@ -88,18 +80,18 @@ func parseLegacyFormat(name string, data []byte) ([]*Label, error) {
parts, description, _ := strings.Cut(line, ";")
- color, name, ok := strings.Cut(parts, " ")
+ color, labelName, ok := strings.Cut(parts, " ")
if !ok {
- return nil, ErrTemplateLoad{name, fmt.Errorf("line is malformed: %s", line)}
+ return nil, ErrTemplateLoad{fileName, fmt.Errorf("line is malformed: %s", line)}
}
color, err := NormalizeColor(color)
if err != nil {
- return nil, ErrTemplateLoad{name, fmt.Errorf("bad HTML color code '%s' in line: %s", color, line)}
+ return nil, ErrTemplateLoad{fileName, fmt.Errorf("bad HTML color code '%s' in line: %s", color, line)}
}
list = append(list, &Label{
- Name: strings.TrimSpace(name),
+ Name: strings.TrimSpace(labelName),
Color: color,
Description: strings.TrimSpace(description),
})
@@ -108,10 +100,10 @@ func parseLegacyFormat(name string, data []byte) ([]*Label, error) {
return list, nil
}
-// LoadFormatted loads the labels' list of a template file as a string separated by comma
-func LoadFormatted(name string) (string, error) {
+// LoadTemplateDescription loads the labels from a template file, returns a description string by joining each Label.Name with comma
+func LoadTemplateDescription(fileName string) (string, error) {
var buf strings.Builder
- list, err := GetTemplateFile(name)
+ list, err := LoadTemplateFile(fileName)
if err != nil {
return "", err
}
diff --git a/modules/repository/create.go b/modules/repository/create.go
index 6a1fa41b6b..c1395242c5 100644
--- a/modules/repository/create.go
+++ b/modules/repository/create.go
@@ -23,7 +23,6 @@ import (
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/models/webhook"
"code.gitea.io/gitea/modules/git"
- "code.gitea.io/gitea/modules/label"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs"
@@ -190,7 +189,7 @@ func CreateRepository(doer, u *user_model.User, opts CreateRepoOptions) (*repo_m
// Check if label template exist
if len(opts.IssueLabels) > 0 {
- if _, err := label.GetTemplateFile(opts.IssueLabels); err != nil {
+ if _, err := LoadTemplateLabelsByDisplayName(opts.IssueLabels); err != nil {
return nil, err
}
}
diff --git a/modules/repository/init.go b/modules/repository/init.go
index f9a33cd4f6..38dd8a0c4f 100644
--- a/modules/repository/init.go
+++ b/modules/repository/init.go
@@ -8,7 +8,6 @@ import (
"context"
"fmt"
"os"
- "path"
"path/filepath"
"sort"
"strings"
@@ -27,6 +26,11 @@ import (
asymkey_service "code.gitea.io/gitea/services/asymkey"
)
+type OptionFile struct {
+ DisplayName string
+ Description string
+}
+
var (
// Gitignores contains the gitiginore files
Gitignores []string
@@ -37,65 +41,73 @@ var (
// Readmes contains the readme files
Readmes []string
- // LabelTemplates contains the label template files and the list of labels for each file
- LabelTemplates map[string]string
+ // LabelTemplateFiles contains the label template files, each item has its DisplayName and Description
+ LabelTemplateFiles []OptionFile
+ labelTemplateFileMap = map[string]string{} // DisplayName => FileName mapping
)
+type optionFileList struct {
+ all []string // all files provided by bindata & custom-path. Sorted.
+ custom []string // custom files provided by custom-path. Non-sorted, internal use only.
+}
+
+// mergeCustomLabelFiles merges the custom label files. Always use the file's main name (DisplayName) as the key to de-duplicate.
+func mergeCustomLabelFiles(fl optionFileList) []string {
+ exts := map[string]int{"": 0, ".yml": 1, ".yaml": 2} // "yaml" file has the highest priority to be used.
+
+ m := map[string]string{}
+ merge := func(list []string) {
+ sort.Slice(list, func(i, j int) bool { return exts[filepath.Ext(list[i])] < exts[filepath.Ext(list[j])] })
+ for _, f := range list {
+ m[strings.TrimSuffix(f, filepath.Ext(f))] = f
+ }
+ }
+ merge(fl.all)
+ merge(fl.custom)
+
+ files := make([]string, 0, len(m))
+ for _, f := range m {
+ files = append(files, f)
+ }
+ sort.Strings(files)
+ return files
+}
+
// LoadRepoConfig loads the repository config
-func LoadRepoConfig() {
- // Load .gitignore and license files and readme templates.
- types := []string{"gitignore", "license", "readme", "label"}
- typeFiles := make([][]string, 4)
+func LoadRepoConfig() error {
+ types := []string{"gitignore", "license", "readme", "label"} // option file directories
+ typeFiles := make([]optionFileList, len(types))
for i, t := range types {
- files, err := options.Dir(t)
- if err != nil {
- log.Fatal("Failed to get %s files: %v", t, err)
+ var err error
+ if typeFiles[i].all, err = options.Dir(t); err != nil {
+ return fmt.Errorf("failed to list %s files: %w", t, err)
}
- if t == "label" {
- for i, f := range files {
- ext := strings.ToLower(filepath.Ext(f))
- if ext == ".yaml" || ext == ".yml" {
- files[i] = f[:len(f)-len(ext)]
- }
+ sort.Strings(typeFiles[i].all)
+ customPath := filepath.Join(setting.CustomPath, "options", t)
+ if isDir, err := util.IsDir(customPath); err != nil {
+ return fmt.Errorf("failed to check custom %s dir: %w", t, err)
+ } else if isDir {
+ if typeFiles[i].custom, err = util.StatDir(customPath); err != nil {
+ return fmt.Errorf("failed to list custom %s files: %w", t, err)
}
}
- customPath := path.Join(setting.CustomPath, "options", t)
- isDir, err := util.IsDir(customPath)
- if err != nil {
- log.Fatal("Failed to get custom %s files: %v", t, err)
- }
- if isDir {
- customFiles, err := util.StatDir(customPath)
- if err != nil {
- log.Fatal("Failed to get custom %s files: %v", t, err)
- }
-
- for _, f := range customFiles {
- if !util.SliceContainsString(files, f, true) {
- files = append(files, f)
- }
- }
- }
- typeFiles[i] = files
}
- Gitignores = typeFiles[0]
- Licenses = typeFiles[1]
- Readmes = typeFiles[2]
- LabelTemplatesFiles := typeFiles[3]
- sort.Strings(Gitignores)
- sort.Strings(Licenses)
- sort.Strings(Readmes)
- sort.Strings(LabelTemplatesFiles)
+ Gitignores = typeFiles[0].all
+ Licenses = typeFiles[1].all
+ Readmes = typeFiles[2].all
// Load label templates
- LabelTemplates = make(map[string]string)
- for _, templateFile := range LabelTemplatesFiles {
- labels, err := label.LoadFormatted(templateFile)
+ LabelTemplateFiles = nil
+ labelTemplateFileMap = map[string]string{}
+ for _, file := range mergeCustomLabelFiles(typeFiles[3]) {
+ description, err := label.LoadTemplateDescription(file)
if err != nil {
- log.Error("Failed to load labels: %v", err)
+ return fmt.Errorf("failed to load labels: %w", err)
}
- LabelTemplates[templateFile] = labels
+ displayName := strings.TrimSuffix(file, filepath.Ext(file))
+ labelTemplateFileMap[displayName] = file
+ LabelTemplateFiles = append(LabelTemplateFiles, OptionFile{DisplayName: displayName, Description: description})
}
// Filter out invalid names and promote preferred licenses.
@@ -111,6 +123,7 @@ func LoadRepoConfig() {
}
}
Licenses = sortedLicenses
+ return nil
}
func prepareRepoCommit(ctx context.Context, repo *repo_model.Repository, tmpDir, repoPath string, opts CreateRepoOptions) error {
@@ -344,7 +357,7 @@ func initRepository(ctx context.Context, repoPath string, u *user_model.User, re
// InitializeLabels adds a label set to a repository using a template
func InitializeLabels(ctx context.Context, id int64, labelTemplate string, isOrg bool) error {
- list, err := label.GetTemplateFile(labelTemplate)
+ list, err := LoadTemplateLabelsByDisplayName(labelTemplate)
if err != nil {
return err
}
@@ -370,3 +383,11 @@ func InitializeLabels(ctx context.Context, id int64, labelTemplate string, isOrg
}
return nil
}
+
+// LoadTemplateLabelsByDisplayName loads a label template by its display name
+func LoadTemplateLabelsByDisplayName(displayName string) ([]*label.Label, error) {
+ if fileName, ok := labelTemplateFileMap[displayName]; ok {
+ return label.LoadTemplateFile(fileName)
+ }
+ return nil, label.ErrTemplateLoad{TemplateFile: displayName, OriginalError: fmt.Errorf("label template %q not found", displayName)}
+}
diff --git a/modules/repository/init_test.go b/modules/repository/init_test.go
new file mode 100644
index 0000000000..227efdc1db
--- /dev/null
+++ b/modules/repository/init_test.go
@@ -0,0 +1,30 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package repository
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+)
+
+func TestMergeCustomLabels(t *testing.T) {
+ files := mergeCustomLabelFiles(optionFileList{
+ all: []string{"a", "a.yaml", "a.yml"},
+ custom: nil,
+ })
+ assert.EqualValues(t, []string{"a.yaml"}, files, "yaml file should win")
+
+ files = mergeCustomLabelFiles(optionFileList{
+ all: []string{"a", "a.yaml"},
+ custom: []string{"a"},
+ })
+ assert.EqualValues(t, []string{"a"}, files, "custom file should win")
+
+ files = mergeCustomLabelFiles(optionFileList{
+ all: []string{"a", "a.yml", "a.yaml"},
+ custom: []string{"a", "a.yml"},
+ })
+ assert.EqualValues(t, []string{"a.yml"}, files, "custom yml file should win if no yaml")
+}
diff --git a/routers/web/org/setting.go b/routers/web/org/setting.go
index 7d84c101d8..db8fc728df 100644
--- a/routers/web/org/setting.go
+++ b/routers/web/org/setting.go
@@ -246,6 +246,6 @@ func Labels(ctx *context.Context) {
ctx.Data["Title"] = ctx.Tr("repo.labels")
ctx.Data["PageIsOrgSettings"] = true
ctx.Data["PageIsOrgSettingsLabels"] = true
- ctx.Data["LabelTemplates"] = repo_module.LabelTemplates
+ ctx.Data["LabelTemplateFiles"] = repo_module.LabelTemplateFiles
ctx.HTML(http.StatusOK, tplSettingsLabels)
}
diff --git a/routers/web/repo/issue_label.go b/routers/web/repo/issue_label.go
index 3123359a65..002acbf1d3 100644
--- a/routers/web/repo/issue_label.go
+++ b/routers/web/repo/issue_label.go
@@ -28,7 +28,7 @@ func Labels(ctx *context.Context) {
ctx.Data["Title"] = ctx.Tr("repo.labels")
ctx.Data["PageIsIssueList"] = true
ctx.Data["PageIsLabels"] = true
- ctx.Data["LabelTemplates"] = repo_module.LabelTemplates
+ ctx.Data["LabelTemplateFiles"] = repo_module.LabelTemplateFiles
ctx.HTML(http.StatusOK, tplLabels)
}
diff --git a/routers/web/repo/issue_label_test.go b/routers/web/repo/issue_label_test.go
index a62d2afaa8..c24fe898b6 100644
--- a/routers/web/repo/issue_label_test.go
+++ b/routers/web/repo/issue_label_test.go
@@ -10,6 +10,7 @@ import (
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/unittest"
+ "code.gitea.io/gitea/modules/repository"
"code.gitea.io/gitea/modules/test"
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/services/forms"
@@ -30,6 +31,7 @@ func int64SliceToCommaSeparated(a []int64) string {
func TestInitializeLabels(t *testing.T) {
unittest.PrepareTestEnv(t)
+ assert.NoError(t, repository.LoadRepoConfig())
ctx := test.MockContext(t, "user2/repo1/labels/initialize")
test.LoadUser(t, ctx, 2)
test.LoadRepo(t, ctx, 2)
diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go
index b4e7b5a46e..9b80e85324 100644
--- a/routers/web/repo/repo.go
+++ b/routers/web/repo/repo.go
@@ -132,7 +132,7 @@ func Create(ctx *context.Context) {
// Give default value for template to render.
ctx.Data["Gitignores"] = repo_module.Gitignores
- ctx.Data["LabelTemplates"] = repo_module.LabelTemplates
+ ctx.Data["LabelTemplateFiles"] = repo_module.LabelTemplateFiles
ctx.Data["Licenses"] = repo_module.Licenses
ctx.Data["Readmes"] = repo_module.Readmes
ctx.Data["readme"] = "Default"
@@ -200,7 +200,7 @@ func CreatePost(ctx *context.Context) {
ctx.Data["Title"] = ctx.Tr("new_repo")
ctx.Data["Gitignores"] = repo_module.Gitignores
- ctx.Data["LabelTemplates"] = repo_module.LabelTemplates
+ ctx.Data["LabelTemplateFiles"] = repo_module.LabelTemplateFiles
ctx.Data["Licenses"] = repo_module.Licenses
ctx.Data["Readmes"] = repo_module.Readmes
diff --git a/services/repository/repository.go b/services/repository/repository.go
index 000b1a3da6..0d6529383c 100644
--- a/services/repository/repository.go
+++ b/services/repository/repository.go
@@ -81,7 +81,9 @@ func PushCreateRepo(ctx context.Context, authUser, owner *user_model.User, repoN
// Init start repository service
func Init() error {
- repo_module.LoadRepoConfig()
+ if err := repo_module.LoadRepoConfig(); err != nil {
+ return err
+ }
system_model.RemoveAllWithNotice(db.DefaultContext, "Clean up temporary repository uploads", setting.Repository.Upload.TempPath)
system_model.RemoveAllWithNotice(db.DefaultContext, "Clean up temporary repositories", repo_module.LocalCopyPath())
return initPushQueue()
diff --git a/templates/repo/create.tmpl b/templates/repo/create.tmpl
index 721caec23f..85b02f394d 100644
--- a/templates/repo/create.tmpl
+++ b/templates/repo/create.tmpl
@@ -117,8 +117,8 @@
<div class="default text">{{.locale.Tr "repo.issue_labels_helper"}}</div>
<div class="menu">
<div class="item" data-value="">{{.locale.Tr "repo.issue_labels_helper"}}</div>
- {{range $template, $labels := .LabelTemplates}}
- <div class="item" data-value="{{$template}}">{{$template}}<br><i>({{$labels}})</i></div>
+ {{range .LabelTemplateFiles}}
+ <div class="item" data-value="{{.DisplayName}}">{{.DisplayName}}<br><i>({{.Description}})</i></div>
{{end}}
</div>
</div>
diff --git a/templates/repo/issue/labels/label_load_template.tmpl b/templates/repo/issue/labels/label_load_template.tmpl
index 21caf2b472..d883953bf6 100644
--- a/templates/repo/issue/labels/label_load_template.tmpl
+++ b/templates/repo/issue/labels/label_load_template.tmpl
@@ -10,8 +10,8 @@
<input type="hidden" name="template_name" value="Default">
<div class="default text">{{.locale.Tr "repo.issues.label_templates.helper"}}</div>
<div class="menu">
- {{range $template, $labels := .LabelTemplates}}
- <div class="item" data-value="{{$template}}">{{$template}}<br><i>({{$labels}})</i></div>
+ {{range .LabelTemplateFiles}}
+ <div class="item" data-value="{{.DisplayName}}">{{.DisplayName}}<br><i>({{.Description}})</i></div>
{{end}}
</div>
{{svg "octicon-triangle-down" 18 "dropdown icon"}}