Merge pull request 'fix: improve the display of PR & issue short links' (#5075) from solomonv/pr-short-link-text-fixes into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/5075
Reviewed-by: Gusted <gusted@noreply.codeberg.org>

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<user>[\w_.-]+)\/(?P<repo>[\w_.-]+)\/(?:issues|pulls)\/(?P<num>(?:\w{1,10}-)?[1-9][0-9]*)(?P<subpath>\/[\w_.-]+)?(?:(?P<comment>#(?: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)
+		re := getIssueFullPattern()
-		if m == nil {
+		linkIndex, m := re.FindStringIndex(node.Data), re.FindStringSubmatch(node.Data)
+		if linkIndex == nil || m == nil {
 			return
 		}
 
-		mDiffView := getFilesChangedFullPattern().FindStringSubmatchIndex(node.Data)
+		link := node.Data[linkIndex[0]:linkIndex[1]]
-		// leave it as it is if the link is from "Files Changed" tab in PR Diff View https://domain/org/repo/pulls/27/files
+		text := "#" + m[re.SubexpIndex("num")] + m[re.SubexpIndex("subpath")]
-		if mDiffView != nil {
-			return
-		}
 
-		link := node.Data[m[0]:m[1]]
+		if len(m[re.SubexpIndex("comment")]) > 0 {
-		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 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
+		matchUser := m[re.SubexpIndex("user")]
-		// http://localhost:3000/gituser/myrepo/issues/1
+		matchRepo := m[re.SubexpIndex("repo")]
-		linkParts := strings.Split(link, "/")
-		matchOrg := linkParts[len(linkParts)-4]
-		matchRepo := linkParts[len(linkParts)-3]
 
-		if matchOrg == ctx.Metas["user"] && matchRepo == ctx.Metas["repo"] {
+		if matchUser == ctx.Metas["user"] && matchRepo == ctx.Metas["repo"] {
-			replaceContent(node, m[0], m[1], createLink(link, text, "ref-issue"))
+			replaceContent(node, linkIndex[0], linkIndex[1], createLink(link, text, "ref-issue"))
 		} else {
-			text = matchOrg + "/" + matchRepo + text
+			text = matchUser + "/" + matchRepo + text
-			replaceContent(node, m[0], m[1], createLink(link, text, "ref-issue"))
+			replaceContent(node, linkIndex[0], linkIndex[1], createLink(link, text, "ref-issue"))
 		}
 		node = node.NextSibling.NextSibling
 	}

modules/markup/html_internal_test.go

@@ -383,12 +383,12 @@ func TestRender_FullIssueURLs(t *testing.T) {
 		`<a href="http://localhost:3000/gogits/gogs/issues/4" class="ref-issue">#4</a>`)
 	test("http://localhost:3000/gogits/gogs/issues/4 test",
 		`<a href="http://localhost:3000/gogits/gogs/issues/4" class="ref-issue">#4</a> test`)
-	test("http://localhost:3000/gogits/gogs/issues/4?a=1&b=2#comment-123 test",
+	test("http://localhost:3000/gogits/gogs/issues/4?a=1&b=2#comment-form test",
-		`<a href="http://localhost:3000/gogits/gogs/issues/4?a=1&amp;b=2#comment-123" class="ref-issue">#4 (comment)</a> test`)
+		`<a href="http://localhost:3000/gogits/gogs/issues/4?a=1&amp;b=2#comment-form" class="ref-issue">#4</a> test`)
 	test("http://localhost:3000/testOrg/testOrgRepo/pulls/2/files#issuecomment-24",
-		"http://localhost:3000/testOrg/testOrgRepo/pulls/2/files#issuecomment-24")
+		`<a href="http://localhost:3000/testOrg/testOrgRepo/pulls/2/files#issuecomment-24" class="ref-issue">testOrg/testOrgRepo#2/files (comment)</a>`)
-	test("http://localhost:3000/testOrg/testOrgRepo/pulls/2/files",
+	test("http://localhost:3000/testOrg/testOrgRepo/pulls/2/commits",
-		"http://localhost:3000/testOrg/testOrgRepo/pulls/2/files")
+		`<a href="http://localhost:3000/testOrg/testOrgRepo/pulls/2/commits" class="ref-issue">testOrg/testOrgRepo#2/commits</a>`)
 }
 
 func TestRegExp_sha1CurrentPattern(t *testing.T) {