From 3ff0a126e12109b6c3aceaa229dd1bf229b6ad4b Mon Sep 17 00:00:00 2001 From: mrsdizzie Date: Fri, 12 Apr 2019 01:53:34 -0400 Subject: [PATCH] Improve issue autolinks (#6273) * Improve issue autolinks Update autolinks to match what github does here: Issue in same repo: #1 Issue in different repo: org/repo#1 Fixes #6264 * Use setting.AppURL when parsing URL Using setting.AppURL here is a more reliable way of parsing the current URL and what other functions in this file seem to use. * Make ComposeMetas always return a valid context * Add per repository markdown renderers for better context * Update for use of context metas Now that we include the user and repo name inside context metas, update various code and tests for this new logic --- models/repo.go | 18 ++++++------- models/repo_test.go | 5 +++- modules/markup/html.go | 34 ++++++++++++++++++------ modules/markup/html_internal_test.go | 20 ++++++++++---- modules/markup/markdown/markdown_test.go | 14 +++++++--- routers/api/v1/api.go | 2 ++ routers/api/v1/misc/markdown.go | 22 ++++++++++++--- templates/repo/diff/box.tmpl | 2 +- templates/repo/diff/comment_form.tmpl | 2 +- templates/repo/editor/edit.tmpl | 4 +-- templates/repo/issue/comment_tab.tmpl | 4 +-- templates/repo/issue/view_content.tmpl | 2 +- templates/repo/wiki/new.tmpl | 2 +- 13 files changed, 94 insertions(+), 37 deletions(-) diff --git a/models/repo.go b/models/repo.go index 5cdee6c3f9..8b51f14043 100644 --- a/models/repo.go +++ b/models/repo.go @@ -469,19 +469,19 @@ func (repo *Repository) mustOwnerName(e Engine) string { return repo.OwnerName } -// ComposeMetas composes a map of metas for rendering external issue tracker URL. +// ComposeMetas composes a map of metas for properly rendering issue links and external issue trackers. func (repo *Repository) ComposeMetas() map[string]string { - unit, err := repo.GetUnit(UnitTypeExternalTracker) - if err != nil { - return nil - } - if repo.ExternalMetas == nil { repo.ExternalMetas = map[string]string{ - "format": unit.ExternalTrackerConfig().ExternalTrackerFormat, - "user": repo.MustOwner().Name, - "repo": repo.Name, + "user": repo.MustOwner().Name, + "repo": repo.Name, } + unit, err := repo.GetUnit(UnitTypeExternalTracker) + if err != nil { + return repo.ExternalMetas + } + + repo.ExternalMetas["format"] = unit.ExternalTrackerConfig().ExternalTrackerFormat switch unit.ExternalTrackerConfig().ExternalTrackerStyle { case markup.IssueNameStyleAlphanumeric: repo.ExternalMetas["style"] = markup.IssueNameStyleAlphanumeric diff --git a/models/repo_test.go b/models/repo_test.go index 752ffc2dd9..a5b8cce9b9 100644 --- a/models/repo_test.go +++ b/models/repo_test.go @@ -20,7 +20,10 @@ func TestRepo(t *testing.T) { repo.Owner = &User{Name: "testOwner"} repo.Units = nil - assert.Nil(t, repo.ComposeMetas()) + + metas := repo.ComposeMetas() + assert.Equal(t, "testRepo", metas["repo"]) + assert.Equal(t, "testOwner", metas["user"]) externalTracker := RepoUnit{ Type: UnitTypeExternalTracker, diff --git a/modules/markup/html.go b/modules/markup/html.go index 136f76d301..930c6b3a3e 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -551,20 +551,37 @@ func shortLinkProcessorFull(ctx *postProcessCtx, node *html.Node, noLink bool) { } func fullIssuePatternProcessor(ctx *postProcessCtx, node *html.Node) { + if ctx.metas == nil { + return + } m := getIssueFullPattern().FindStringSubmatchIndex(node.Data) if m == nil { return } link := node.Data[m[0]:m[1]] id := "#" + node.Data[m[2]:m[3]] - // TODO if m[4]:m[5] is not nil, then link is to a comment, - // and we should indicate that in the text somehow - replaceContent(node, m[0], m[1], createLink(link, id)) + + // extract repo and org name from matched link like + // http://localhost:3000/gituser/myrepo/issues/1 + linkParts := strings.Split(path.Clean(link), "/") + matchOrg := linkParts[len(linkParts)-4] + matchRepo := linkParts[len(linkParts)-3] + + if matchOrg == ctx.metas["user"] && matchRepo == ctx.metas["repo"] { + // TODO if m[4]:m[5] is not nil, then link is to a comment, + // and we should indicate that in the text somehow + replaceContent(node, m[0], m[1], createLink(link, id)) + + } else { + orgRepoID := matchOrg + "/" + matchRepo + id + replaceContent(node, m[0], m[1], createLink(link, orgRepoID)) + } } func issueIndexPatternProcessor(ctx *postProcessCtx, node *html.Node) { - prefix := cutoutVerbosePrefix(ctx.urlPrefix) - + if ctx.metas == nil { + return + } // default to numeric pattern, unless alphanumeric is requested. pattern := issueNumericPattern if ctx.metas["style"] == IssueNameStyleAlphanumeric { @@ -575,11 +592,10 @@ func issueIndexPatternProcessor(ctx *postProcessCtx, node *html.Node) { if match == nil { return } + id := node.Data[match[2]:match[3]] var link *html.Node - if ctx.metas == nil { - link = createLink(util.URLJoin(prefix, "issues", id[1:]), id) - } else { + if _, ok := ctx.metas["format"]; ok { // Support for external issue tracker if ctx.metas["style"] == IssueNameStyleAlphanumeric { ctx.metas["index"] = id @@ -587,6 +603,8 @@ func issueIndexPatternProcessor(ctx *postProcessCtx, node *html.Node) { ctx.metas["index"] = id[1:] } link = createLink(com.Expand(ctx.metas["format"], ctx.metas), id) + } else { + link = createLink(util.URLJoin(setting.AppURL, ctx.metas["user"], ctx.metas["repo"], "issues", id[1:]), id) } replaceContent(node, match[2], match[3], link) } diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go index c71948593d..135a8e103c 100644 --- a/modules/markup/html_internal_test.go +++ b/modules/markup/html_internal_test.go @@ -53,6 +53,12 @@ var alphanumericMetas = map[string]string{ "style": IssueNameStyleAlphanumeric, } +// these values should match the Repo const above +var localMetas = map[string]string{ + "user": "gogits", + "repo": "gogs", +} + func TestRender_IssueIndexPattern(t *testing.T) { // numeric: render inputs without valid mentions test := func(s string) { @@ -91,7 +97,7 @@ func TestRender_IssueIndexPattern2(t *testing.T) { links[i] = numericIssueLink(util.URLJoin(setting.AppSubURL, "issues"), index) } expectedNil := fmt.Sprintf(expectedFmt, links...) - testRenderIssueIndexPattern(t, s, expectedNil, nil) + testRenderIssueIndexPattern(t, s, expectedNil, &postProcessCtx{metas: localMetas}) for i, index := range indices { links[i] = numericIssueLink("https://someurl.com/someUser/someRepo/", index) @@ -171,6 +177,7 @@ func testRenderIssueIndexPattern(t *testing.T, input, expected string, ctx *post if ctx.urlPrefix == "" { ctx.urlPrefix = AppSubURL } + res, err := ctx.postProcess([]byte(input)) assert.NoError(t, err) assert.Equal(t, expected, string(res)) @@ -181,10 +188,10 @@ func TestRender_AutoLink(t *testing.T) { setting.AppSubURL = AppSubURL test := func(input, expected string) { - buffer, err := PostProcess([]byte(input), setting.AppSubURL, nil, false) + buffer, err := PostProcess([]byte(input), setting.AppSubURL, localMetas, false) assert.Equal(t, err, nil) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(buffer))) - buffer, err = PostProcess([]byte(input), setting.AppSubURL, nil, true) + buffer, err = PostProcess([]byte(input), setting.AppSubURL, localMetas, true) assert.Equal(t, err, nil) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(buffer))) } @@ -214,6 +221,7 @@ func TestRender_FullIssueURLs(t *testing.T) { if ctx.urlPrefix == "" { ctx.urlPrefix = AppSubURL } + ctx.metas = localMetas result, err := ctx.postProcess([]byte(input)) assert.NoError(t, err) assert.Equal(t, expected, string(result)) @@ -221,9 +229,11 @@ func TestRender_FullIssueURLs(t *testing.T) { test("Here is a link https://git.osgeo.org/gogs/postgis/postgis/pulls/6", "Here is a link https://git.osgeo.org/gogs/postgis/postgis/pulls/6") test("Look here http://localhost:3000/person/repo/issues/4", - `Look here #4`) + `Look here person/repo#4`) test("http://localhost:3000/person/repo/issues/4#issuecomment-1234", - `#4`) + `person/repo#4`) + test("http://localhost:3000/gogits/gogs/issues/4", + `#4`) } func TestRegExp_issueNumericPattern(t *testing.T) { diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index 5aa9c3d7d2..8ba51e6a1b 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -19,6 +19,12 @@ const AppURL = "http://localhost:3000/" const Repo = "gogits/gogs" const AppSubURL = AppURL + Repo + "/" +// these values should match the Repo const above +var localMetas = map[string]string{ + "user": "gogits", + "repo": "gogs", +} + func TestRender_StandardLinks(t *testing.T) { setting.AppURL = AppURL setting.AppSubURL = AppSubURL @@ -100,7 +106,8 @@ func testAnswers(baseURLContent, baseURLImages string) []string {

Ideas and codes