enhance: improve issue-pattern capture groups and support both internal&external trackers enabled (#39354)

* Fix #39351
* Fix #17621
* Fix #34881

By the way, fix error handling bugs in `updateRepoUnits`

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

modules/markup/html_internal_test.go

@@ -39,7 +39,7 @@ func link(href, class, contents string) string {
 }
 
 var numericMetas = map[string]string{
-	"format":                       "https://someurl.com/{user}/{repo}/{index}",
+	"externalTrackerLinkFormat":    "https://someurl.com/{user}/{repo}/{index}",
 	"user":                         "someUser",
 	"repo":                         "someRepo",
 	"style":                        IssueNameStyleNumeric,
@@ -47,7 +47,7 @@ var numericMetas = map[string]string{
 }
 
 var alphanumericMetas = map[string]string{
-	"format":                       "https://someurl.com/{user}/{repo}/{index}",
+	"externalTrackerLinkFormat":    "https://someurl.com/{user}/{repo}/{index}",
 	"user":                         "someUser",
 	"repo":                         "someRepo",
 	"style":                        IssueNameStyleAlphanumeric,
@@ -55,10 +55,10 @@ var alphanumericMetas = map[string]string{
 }
 
 var regexpMetas = map[string]string{
-	"format": "https://someurl.com/{user}/{repo}/{index}",
-	"user":   "someUser",
-	"repo":   "someRepo",
-	"style":  IssueNameStyleRegexp,
+	"externalTrackerLinkFormat": "https://someurl.com/{user}/{repo}/{index}",
+	"user":                      "someUser",
+	"repo":                      "someRepo",
+	"style":                     IssueNameStyleRegexp,
 }
 
 // these values should match the TestOrgRepo const above
@@ -219,23 +219,29 @@ func TestRender_IssueIndexPattern5(t *testing.T) {
 	}
 
 	test("abc ISSUE-123 def", "abc %s def",
-		"ISSUE-(\\d+)",
+		`ISSUE-(\d+)`,
 		[]string{"123"},
 		[]string{"ISSUE-123"},
 	)
 
 	test("abc (ISSUE 123) def", "abc %s def",
-		"\\(ISSUE (\\d+)\\)",
+		`\(ISSUE (\d+)\)`,
 		[]string{"123"},
 		[]string{"(ISSUE 123)"},
 	)
 
 	test("abc ISSUE-123 def", "abc %s def",
-		"(ISSUE-(\\d+))",
+		`(ISSUE-(\d+))`,
 		[]string{"ISSUE-123"},
 		[]string{"ISSUE-123"},
 	)
 
+	test("123456: TEST-123456", "%s %s",
+		`(\d+):|TEST-(\d+)`,
+		[]string{"123456", "123456"},
+		[]string{"123456:", "TEST-123456"},
+	)
+
 	testRenderIssueIndexPattern(t, "will not match", "will not match", NewTestRenderContext(regexpMetas))
 }
 

modules/markup/html_issue.go

@@ -110,16 +110,23 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) {
 
 	next := node.NextSibling
 	for node != nil && node != next {
-		_, hasExtTrackFormat := ctx.RenderOptions.Metas["format"]
-
+		_, hasExternalTracker := ctx.RenderOptions.Metas["externalTrackerLinkFormat"]
+		hasInternalTracker := ctx.RenderOptions.Metas["internalTrackerEnabled"] == "true"
+		if !hasExternalTracker && !hasInternalTracker {
+			hasInternalTracker = true // legacy logic: if no tracker is enabled, fallback to internal
+		}
 		// Repos with external issue trackers might still need to reference local PRs
 		// We need to concern with the first one that shows up in the text, whichever it is
 		isNumericStyle := ctx.RenderOptions.Metas["style"] == "" || ctx.RenderOptions.Metas["style"] == IssueNameStyleNumeric
-		refNumeric := references.FindRenderizableReferenceNumeric(node.Data, hasExtTrackFormat && !isNumericStyle, crossLinkOnly)
+		prOnly := hasExternalTracker && !isNumericStyle
+		refNumeric := references.FindRenderizableReferenceNumeric(node.Data, prOnly, crossLinkOnly)
 
+		useExtTrackerLink := true
 		switch ctx.RenderOptions.Metas["style"] {
 		case "", IssueNameStyleNumeric:
 			ref = refNumeric
+			// when internal tracker is enabled, Numeric (#123) style should only be use for internal tracker
+			useExtTrackerLink = !hasInternalTracker
 		case IssueNameStyleAlphanumeric:
 			ref = references.FindRenderizableReferenceAlphanumeric(node.Data)
 		case IssueNameStyleRegexp:
@@ -132,7 +139,7 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) {
 
 		// Repos with external issue trackers might still need to reference local PRs
 		// We need to concern with the first one that shows up in the text, whichever it is
-		if hasExtTrackFormat && !isNumericStyle && refNumeric != nil {
+		if useExtTrackerLink && !isNumericStyle && refNumeric != nil {
 			// If numeric (PR) was found, and it was BEFORE the non-numeric pattern, use that
 			// Allow a free-pass when non-numeric pattern wasn't found.
 			if ref == nil || refNumeric.RefLocation.Start < ref.RefLocation.Start {
@@ -146,10 +153,10 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) {
 
 		var link *html.Node
 		refText := node.Data[ref.RefLocation.Start:ref.RefLocation.End]
-		if hasExtTrackFormat && !ref.IsPull {
+		if useExtTrackerLink && !ref.IsPull {
 			ctx.RenderOptions.Metas["index"] = ref.Issue
 
-			res, err := vars.ExpandCurlyBrace(ctx.RenderOptions.Metas["format"], ctx.RenderOptions.Metas)
+			res, err := vars.ExpandCurlyBrace(ctx.RenderOptions.Metas["externalTrackerLinkFormat"], ctx.RenderOptions.Metas)
 			if err != nil {
 				// here we could just log the error and continue the rendering
 				log.Error("unable to expand template vars for ref %s, err: %v", ref.Issue, err)
@@ -183,7 +190,7 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) {
 
 		// Decorate action keywords if actionable
 		var keyword *html.Node
-		if references.IsXrefActionable(ref, hasExtTrackFormat) {
+		if references.IsXrefActionable(ref, useExtTrackerLink) {
 			keyword = createKeyword(ctx, node.Data[ref.ActionLocation.Start:ref.ActionLocation.End])
 		} else {
 			keyword = &html.Node{

modules/references/references.go

@@ -378,9 +378,23 @@ func FindRenderizableReferenceRegexp(content string, pattern *regexp.Regexp) *Re
 		return nil
 	}
 
-	action, location := findActionKeywords([]byte(content), match[2])
+	// The external tracker pattern can use alternatives with separate capture
+	// groups. Pick the first group that participated in this match instead of
+	// assuming the first group always did.
+	issueStart, issueEnd := -1, -1
+	for i := 2; i+1 < len(match); i += 2 {
+		if match[i] >= 0 {
+			issueStart, issueEnd = match[i], match[i+1]
+			break
+		}
+	}
+	if issueStart < 0 {
+		return nil
+	}
+
+	action, location := findActionKeywords([]byte(content), issueStart)
 	return &RenderizableReference{
-		Issue:          content[match[2]:match[3]],
+		Issue:          content[issueStart:issueEnd],
 		RefLocation:    &RefSpan{Start: match[0], End: match[1]},
 		Action:         action,
 		ActionLocation: location,