From 864d74d0b1591ddd20522db4830cf1f40111d7a9 Mon Sep 17 00:00:00 2001 From: AliabbasMerchant Date: Sat, 23 May 2020 00:03:38 +0530 Subject: [PATCH] Categorize Templates as Legacy and NonLegacy --- command/issue.go | 2 +- command/pr_create.go | 2 +- command/title_body_survey.go | 6 +- pkg/githubtemplate/github_template.go | 33 ++-- pkg/githubtemplate/github_template_test.go | 182 ++++++++++++++++----- 5 files changed, 173 insertions(+), 52 deletions(-) diff --git a/command/issue.go b/command/issue.go index af3a03fd3..39a8528d0 100644 --- a/command/issue.go +++ b/command/issue.go @@ -356,7 +356,7 @@ func issueCreate(cmd *cobra.Command, args []string) error { if baseOverride == "" { if rootDir, err := git.ToplevelDir(); err == nil { // TODO: figure out how to stub this in tests - templateFiles = githubtemplate.Find(rootDir, "ISSUE_TEMPLATE") + templateFiles = githubtemplate.FindNonLegacy(rootDir, "ISSUE_TEMPLATE") } } diff --git a/command/pr_create.go b/command/pr_create.go index 1100bf9a0..820e516c2 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -217,7 +217,7 @@ func prCreate(cmd *cobra.Command, _ []string) error { var templateFiles []string if rootDir, err := git.ToplevelDir(); err == nil { // TODO: figure out how to stub this in tests - templateFiles = githubtemplate.Find(rootDir, "PULL_REQUEST_TEMPLATE") + templateFiles = githubtemplate.FindNonLegacy(rootDir, "PULL_REQUEST_TEMPLATE") } err := titleBodySurvey(cmd, &tb, client, baseRepo, title, body, defs, templateFiles, true, baseRepo.ViewerCanTriage()) diff --git a/command/title_body_survey.go b/command/title_body_survey.go index 311c816ea..f0ef0e764 100644 --- a/command/title_body_survey.go +++ b/command/title_body_survey.go @@ -16,8 +16,8 @@ type Action int type metadataStateType int const ( - issueMetadata metadataStateType = 0 - prMetadata metadataStateType = 1 + issueMetadata metadataStateType = iota + prMetadata ) type issueMetadataState struct { @@ -118,7 +118,7 @@ func selectTemplate(templatePaths []string, metadataType metadataStateType) (str if metadataType == issueMetadata { templateNames = append(templateNames, "Open a blank issue") } else if metadataType == prMetadata { - templateNames = append(templateNames, "Open a blank PR") + templateNames = append(templateNames, "Open a blank pull request") } selectQs := []*survey.Question{ diff --git a/pkg/githubtemplate/github_template.go b/pkg/githubtemplate/github_template.go index 89cceda4c..e14ad04c8 100644 --- a/pkg/githubtemplate/github_template.go +++ b/pkg/githubtemplate/github_template.go @@ -10,8 +10,8 @@ import ( "gopkg.in/yaml.v3" ) -// Find returns the list of template file paths -func Find(rootDir string, name string) []string { +// FindNonLegacy returns the list of template file paths from the template folder (according to the "upgraded multiple template builder") +func FindNonLegacy(rootDir string, name string) []string { results := []string{} // https://help.github.com/en/github/building-a-strong-community/creating-a-pull-request-template-for-your-repository @@ -46,21 +46,34 @@ mainLoop: break } } + } + sort.Strings(results) + return results +} + +// FindLegacy returns the file path of the default(legacy) template +func FindLegacy(rootDir string, name string) *string { + // https://help.github.com/en/github/building-a-strong-community/creating-a-pull-request-template-for-your-repository + candidateDirs := []string{ + path.Join(rootDir, ".github"), + rootDir, + path.Join(rootDir, "docs"), + } + for _, dir := range candidateDirs { + files, err := ioutil.ReadDir(dir) + if err != nil { + continue + } // detect a single template file for _, file := range files { if strings.EqualFold(file.Name(), name+".md") { - results = append(results, path.Join(dir, file.Name())) - break + result := path.Join(dir, file.Name()) + return &result } } - if len(results) > 0 { - break - } } - - sort.Strings(results) - return results + return nil } // ExtractName returns the name of the template from YAML front-matter diff --git a/pkg/githubtemplate/github_template_test.go b/pkg/githubtemplate/github_template_test.go index 839e215b3..ee5dfb625 100644 --- a/pkg/githubtemplate/github_template_test.go +++ b/pkg/githubtemplate/github_template_test.go @@ -8,7 +8,7 @@ import ( "testing" ) -func TestFind(t *testing.T) { +func TestFindNonLegacy(t *testing.T) { tmpdir, err := ioutil.TempDir("", "gh-cli") if err != nil { t.Fatal(err) @@ -25,52 +25,69 @@ func TestFind(t *testing.T) { want []string }{ { - name: "Template in root", + name: "Legacy templates ignored", prepare: []string{ "README.md", "ISSUE_TEMPLATE", "issue_template.md", "issue_template.txt", "pull_request_template.md", - }, - args: args{ - rootDir: tmpdir, - name: "ISSUE_TEMPLATE", - }, - want: []string{ - path.Join(tmpdir, "issue_template.md"), - }, - }, - { - name: "Template in .github takes precedence", - prepare: []string{ - "ISSUE_TEMPLATE.md", ".github/issue_template.md", - }, - args: args{ - rootDir: tmpdir, - name: "ISSUE_TEMPLATE", - }, - want: []string{ - path.Join(tmpdir, ".github/issue_template.md"), - }, - }, - { - name: "Template in docs", - prepare: []string{ - "README.md", "docs/issue_template.md", }, args: args{ rootDir: tmpdir, name: "ISSUE_TEMPLATE", }, + want: []string{}, + }, + { + name: "Template folder in .github takes precedence", + prepare: []string{ + "ISSUE_TEMPLATE.md", + "docs/ISSUE_TEMPLATE/abc.md", + "ISSUE_TEMPLATE/abc.md", + ".github/ISSUE_TEMPLATE/abc.md", + }, + args: args{ + rootDir: tmpdir, + name: "ISSUE_TEMPLATE", + }, want: []string{ - path.Join(tmpdir, "docs/issue_template.md"), + path.Join(tmpdir, ".github/ISSUE_TEMPLATE/abc.md"), }, }, { - name: "Multiple templates", + name: "Template folder in root", + prepare: []string{ + "ISSUE_TEMPLATE.md", + "docs/ISSUE_TEMPLATE/abc.md", + "ISSUE_TEMPLATE/abc.md", + }, + args: args{ + rootDir: tmpdir, + name: "ISSUE_TEMPLATE", + }, + want: []string{ + path.Join(tmpdir, "ISSUE_TEMPLATE/abc.md"), + }, + }, + { + name: "Template folder in docs", + prepare: []string{ + "ISSUE_TEMPLATE.md", + "docs/ISSUE_TEMPLATE/abc.md", + }, + args: args{ + rootDir: tmpdir, + name: "ISSUE_TEMPLATE", + }, + want: []string{ + path.Join(tmpdir, "docs/ISSUE_TEMPLATE/abc.md"), + }, + }, + { + name: "Multiple templates in template folder", prepare: []string{ ".github/ISSUE_TEMPLATE/nope.md", ".github/PULL_REQUEST_TEMPLATE.md", @@ -90,18 +107,17 @@ func TestFind(t *testing.T) { }, }, { - name: "Empty multiple templates directory", + name: "Empty template directories", prepare: []string{ - ".github/issue_template.md", - ".github/issue_template/.keep", + ".github/ISSUE_TEMPLATE/.keep", + ".docs/ISSUE_TEMPLATE/.keep", + "ISSUE_TEMPLATE/.keep", }, args: args{ rootDir: tmpdir, name: "ISSUE_TEMPLATE", }, - want: []string{ - path.Join(tmpdir, ".github/issue_template.md"), - }, + want: []string{}, }, } for _, tt := range tests { @@ -116,7 +132,99 @@ func TestFind(t *testing.T) { file.Close() } - if got := Find(tt.args.rootDir, tt.args.name); !reflect.DeepEqual(got, tt.want) { + if got := FindNonLegacy(tt.args.rootDir, tt.args.name); !reflect.DeepEqual(got, tt.want) { + t.Errorf("Find() = %v, want %v", got, tt.want) + } + }) + os.RemoveAll(tmpdir) + } +} + +func TestFindLegacy(t *testing.T) { + tmpdir, err := ioutil.TempDir("", "gh-cli") + if err != nil { + t.Fatal(err) + } + + type args struct { + rootDir string + name string + } + tests := []struct { + name string + prepare []string + args args + want string + }{ + { + name: "Template in root", + prepare: []string{ + "README.md", + "ISSUE_TEMPLATE", + "issue_template.md", + "issue_template.txt", + "pull_request_template.md", + "docs/issue_template.md", + }, + args: args{ + rootDir: tmpdir, + name: "ISSUE_TEMPLATE", + }, + want: path.Join(tmpdir, "issue_template.md"), + }, + { + name: "Template in .github takes precedence", + prepare: []string{ + "ISSUE_TEMPLATE.md", + ".github/issue_template.md", + "docs/issue_template.md", + }, + args: args{ + rootDir: tmpdir, + name: "ISSUE_TEMPLATE", + }, + want: path.Join(tmpdir, ".github/issue_template.md"), + }, + { + name: "Template in docs", + prepare: []string{ + "README.md", + "docs/issue_template.md", + }, + args: args{ + rootDir: tmpdir, + name: "ISSUE_TEMPLATE", + }, + want: path.Join(tmpdir, "docs/issue_template.md"), + }, + { + name: "Non legacy templates ignored", + prepare: []string{ + ".github/PULL_REQUEST_TEMPLATE/abc.md", + "PULL_REQUEST_TEMPLATE/abc.md", + "docs/PULL_REQUEST_TEMPLATE/abc.md", + ".github/PULL_REQUEST_TEMPLATE.md", + }, + args: args{ + rootDir: tmpdir, + name: "PuLl_ReQuEsT_TeMpLaTe", + }, + want: path.Join(tmpdir, ".github/PULL_REQUEST_TEMPLATE.md"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for _, p := range tt.prepare { + fp := path.Join(tmpdir, p) + _ = os.MkdirAll(path.Dir(fp), 0700) + file, err := os.Create(fp) + if err != nil { + t.Fatal(err) + } + file.Close() + } + + if got := FindLegacy(tt.args.rootDir, tt.args.name); *got != tt.want { t.Errorf("Find() = %v, want %v", got, tt.want) } })