From 22f012a859367cd72ab3258853e9ea68e8d2a740 Mon Sep 17 00:00:00 2001 From: Solomon Victorino Date: Thu, 22 Aug 2024 14:35:04 -0600 Subject: [PATCH] fix: improve PR/issue short link display text - include subpaths - don't append "(comment)" for unrelated hash/query strings --- modules/markup/html.go | 50 ++++++++-------------------- modules/markup/html_internal_test.go | 10 +++--- 2 files changed, 18 insertions(+), 42 deletions(-) diff --git a/modules/markup/html.go b/modules/markup/html.go index b4c4c15368..9280a67d62 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -93,30 +93,15 @@ var issueFullPattern *regexp.Regexp // Once for to prevent races var issueFullPatternOnce sync.Once -// regexp for full links to hash comment in pull request files changed tab -var filesChangedFullPattern *regexp.Regexp - -// Once for to prevent races -var filesChangedFullPatternOnce sync.Once - func getIssueFullPattern() *regexp.Regexp { issueFullPatternOnce.Do(func() { // example: https://domain/org/repo/pulls/27#hash issueFullPattern = regexp.MustCompile(regexp.QuoteMeta(setting.AppURL) + - `[\w_.-]+/[\w_.-]+/(?:issues|pulls)/((?:\w{1,10}-)?[1-9][0-9]*)([\?|#](\S+)?)?\b`) + `(?P[\w_.-]+)\/(?P[\w_.-]+)\/(?:issues|pulls)\/(?P(?:\w{1,10}-)?[1-9][0-9]*)(?P\/[\w_.-]+)?(?:(?P#(?:issue|issuecomment)-\d+)|(?:[\?#](?:\S+)?))?\b`) }) return issueFullPattern } -func getFilesChangedFullPattern() *regexp.Regexp { - filesChangedFullPatternOnce.Do(func() { - // example: https://domain/org/repo/pulls/27/files#hash - filesChangedFullPattern = regexp.MustCompile(regexp.QuoteMeta(setting.AppURL) + - `[\w_.-]+/[\w_.-]+/pulls/((?:\w{1,10}-)?[1-9][0-9]*)/files([\?|#](\S+)?)?\b`) - }) - return filesChangedFullPattern -} - // CustomLinkURLSchemes allows for additional schemes to be detected when parsing links within text func CustomLinkURLSchemes(schemes []string) { schemes = append(schemes, "http", "https") @@ -775,22 +760,16 @@ func fullIssuePatternProcessor(ctx *RenderContext, node *html.Node) { } next := node.NextSibling for node != nil && node != next { - m := getIssueFullPattern().FindStringSubmatchIndex(node.Data) - if m == nil { + re := getIssueFullPattern() + linkIndex, m := re.FindStringIndex(node.Data), re.FindStringSubmatch(node.Data) + if linkIndex == nil || m == nil { return } - mDiffView := getFilesChangedFullPattern().FindStringSubmatchIndex(node.Data) - // leave it as it is if the link is from "Files Changed" tab in PR Diff View https://domain/org/repo/pulls/27/files - if mDiffView != nil { - return - } + link := node.Data[linkIndex[0]:linkIndex[1]] + text := "#" + m[re.SubexpIndex("num")] + m[re.SubexpIndex("subpath")] - link := node.Data[m[0]:m[1]] - text := "#" + node.Data[m[2]:m[3]] - // if m[4] and m[5] is not -1, then link is to a comment - // indicate that in the text by appending (comment) - if m[4] != -1 && m[5] != -1 { + if len(m[re.SubexpIndex("comment")]) > 0 { if locale, ok := ctx.Ctx.Value(translation.ContextKey).(translation.Locale); ok { text += " " + locale.TrString("repo.from_comment") } else { @@ -798,17 +777,14 @@ func fullIssuePatternProcessor(ctx *RenderContext, node *html.Node) { } } - // extract repo and org name from matched link like - // http://localhost:3000/gituser/myrepo/issues/1 - linkParts := strings.Split(link, "/") - matchOrg := linkParts[len(linkParts)-4] - matchRepo := linkParts[len(linkParts)-3] + matchUser := m[re.SubexpIndex("user")] + matchRepo := m[re.SubexpIndex("repo")] - if matchOrg == ctx.Metas["user"] && matchRepo == ctx.Metas["repo"] { - replaceContent(node, m[0], m[1], createLink(link, text, "ref-issue")) + if matchUser == ctx.Metas["user"] && matchRepo == ctx.Metas["repo"] { + replaceContent(node, linkIndex[0], linkIndex[1], createLink(link, text, "ref-issue")) } else { - text = matchOrg + "/" + matchRepo + text - replaceContent(node, m[0], m[1], createLink(link, text, "ref-issue")) + text = matchUser + "/" + matchRepo + text + replaceContent(node, linkIndex[0], linkIndex[1], createLink(link, text, "ref-issue")) } node = node.NextSibling.NextSibling } diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go index adc93adb2f..a72be9f8cf 100644 --- a/modules/markup/html_internal_test.go +++ b/modules/markup/html_internal_test.go @@ -383,12 +383,12 @@ func TestRender_FullIssueURLs(t *testing.T) { `#4`) test("http://localhost:3000/gogits/gogs/issues/4 test", `#4 test`) - test("http://localhost:3000/gogits/gogs/issues/4?a=1&b=2#comment-123 test", - `#4 (comment) test`) + test("http://localhost:3000/gogits/gogs/issues/4?a=1&b=2#comment-form test", + `#4 test`) test("http://localhost:3000/testOrg/testOrgRepo/pulls/2/files#issuecomment-24", - "http://localhost:3000/testOrg/testOrgRepo/pulls/2/files#issuecomment-24") - test("http://localhost:3000/testOrg/testOrgRepo/pulls/2/files", - "http://localhost:3000/testOrg/testOrgRepo/pulls/2/files") + `testOrg/testOrgRepo#2/files (comment)`) + test("http://localhost:3000/testOrg/testOrgRepo/pulls/2/commits", + `testOrg/testOrgRepo#2/commits`) } func TestRegExp_sha1CurrentPattern(t *testing.T) {