Accept punctuation after simple+cross repository issue references (#10091)

* Support references ending in , . and ;

* Accept :;, in simple refs; fix 2+ consecutive refs

* Include cross-repository references

* Add ?!, fix spacing problem

2 files changed, 97 insertions, 13 deletions

diff --git a/modules/references/references.go b/modules/references/references.go
index dec77b57b2..c9dabb06ba 100644
--- a/modules/references/references.go
+++ b/modules/references/references.go
@@ -29,12 +29,14 @@ var (
 	// mentionPattern matches all mentions in the form of "@user"
 	mentionPattern = regexp.MustCompile(`(?:\s|^|\(|\[)(@[0-9a-zA-Z-_]+|@[0-9a-zA-Z-_][0-9a-zA-Z-_.]+[0-9a-zA-Z-_])(?:\s|[:,;.?!]\s|[:,;.?!]?$|\)|\])`)
 	// issueNumericPattern matches string that references to a numeric issue, e.g. #1287
-	issueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([#!][0-9]+)(?:\s|$|\)|\]|:|\.(\s|$))`)
+	issueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([#!][0-9]+)(?:\s|$|\)|\]|[:;,.?!]\s|[:;,.?!]$)`)
 	// issueAlphanumericPattern matches string that references to an alphanumeric issue, e.g. ABC-1234
 	issueAlphanumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([A-Z]{1,10}-[1-9][0-9]*)(?:\s|$|\)|\]|:|\.(\s|$))`)
 	// crossReferenceIssueNumericPattern matches string that references a numeric issue in a different repository
 	// e.g. gogits/gogs#12345
-	crossReferenceIssueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([0-9a-zA-Z-_\.]+/[0-9a-zA-Z-_\.]+[#!][0-9]+)(?:\s|$|\)|\]|\.(\s|$))`)
+	crossReferenceIssueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)([0-9a-zA-Z-_\.]+/[0-9a-zA-Z-_\.]+[#!][0-9]+)(?:\s|$|\)|\]|[:;,.?!]\s|[:;,.?!]$)`)
+	// spaceTrimmedPattern let's us find the trailing space
+	spaceTrimmedPattern = regexp.MustCompile(`(?:.*[0-9a-zA-Z-_])\s`)
 
 	issueCloseKeywordsPat, issueReopenKeywordsPat *regexp.Regexp
 	issueKeywordsOnce                             sync.Once
@@ -172,10 +174,24 @@ func FindAllMentionsMarkdown(content string) []string {
 // FindAllMentionsBytes matches mention patterns in given content
 // and returns a list of locations for the unvalidated user names, including the @ prefix.
 func FindAllMentionsBytes(content []byte) []RefSpan {
-	mentions := mentionPattern.FindAllSubmatchIndex(content, -1)
-	ret := make([]RefSpan, len(mentions))
-	for i, val := range mentions {
-		ret[i] = RefSpan{Start: val[2], End: val[3]}
+	// Sadly we can't use FindAllSubmatchIndex because our pattern checks for starting and
+	// trailing spaces (\s@mention,\s), so if we get two consecutive references, the space
+	// from the second reference will be "eaten" by the first one:
+	// ...\s@mention1\s@mention2\s...	--> ...`\s@mention1\s`, (not) `@mention2,\s...`
+	ret := make([]RefSpan, 0, 5)
+	pos := 0
+	for {
+		match := mentionPattern.FindSubmatchIndex(content[pos:])
+		if match == nil {
+			break
+		}
+		ret = append(ret, RefSpan{Start: match[2] + pos, End: match[3] + pos})
+		notrail := spaceTrimmedPattern.FindSubmatchIndex(content[match[2]+pos : match[3]+pos])
+		if notrail == nil {
+			pos = match[3] + pos
+		} else {
+			pos = match[3] + pos + notrail[1] - notrail[3]
+		}
 	}
 	return ret
 }
@@ -252,19 +268,44 @@ func FindRenderizableReferenceAlphanumeric(content string) (bool, *RenderizableR
 func findAllIssueReferencesBytes(content []byte, links []string) []*rawReference {
 
 	ret := make([]*rawReference, 0, 10)
-
-	matches := issueNumericPattern.FindAllSubmatchIndex(content, -1)
-	for _, match := range matches {
-		if ref := getCrossReference(content, match[2], match[3], false, false); ref != nil {
+	pos := 0
+
+	// Sadly we can't use FindAllSubmatchIndex because our pattern checks for starting and
+	// trailing spaces (\s#ref,\s), so if we get two consecutive references, the space
+	// from the second reference will be "eaten" by the first one:
+	// ...\s#ref1\s#ref2\s...	--> ...`\s#ref1\s`, (not) `#ref2,\s...`
+	for {
+		match := issueNumericPattern.FindSubmatchIndex(content[pos:])
+		if match == nil {
+			break
+		}
+		if ref := getCrossReference(content, match[2]+pos, match[3]+pos, false, false); ref != nil {
 			ret = append(ret, ref)
 		}
+		notrail := spaceTrimmedPattern.FindSubmatchIndex(content[match[2]+pos : match[3]+pos])
+		if notrail == nil {
+			pos = match[3] + pos
+		} else {
+			pos = match[3] + pos + notrail[1] - notrail[3]
+		}
 	}
 
-	matches = crossReferenceIssueNumericPattern.FindAllSubmatchIndex(content, -1)
-	for _, match := range matches {
-		if ref := getCrossReference(content, match[2], match[3], false, false); ref != nil {
+	pos = 0
+
+	for {
+		match := crossReferenceIssueNumericPattern.FindSubmatchIndex(content[pos:])
+		if match == nil {
+			break
+		}
+		if ref := getCrossReference(content, match[2]+pos, match[3]+pos, false, false); ref != nil {
 			ret = append(ret, ref)
 		}
+		notrail := spaceTrimmedPattern.FindSubmatchIndex(content[match[2]+pos : match[3]+pos])
+		if notrail == nil {
+			pos = match[3] + pos
+		} else {
+			pos = match[3] + pos + notrail[1] - notrail[3]
+		}
 	}
 
 	localhost := getGiteaHostName()
diff --git a/modules/references/references_test.go b/modules/references/references_test.go
index bcb2c4384f..48589c1637 100644
--- a/modules/references/references_test.go
+++ b/modules/references/references_test.go
@@ -136,6 +136,39 @@ func TestFindAllIssueReferences(t *testing.T) {
 			},
 		},
 		{
+			"For [!123] yes",
+			[]testResult{
+				{123, "", "", "123", true, XRefActionNone, &RefSpan{Start: 5, End: 9}, nil},
+			},
+		},
+		{
+			"For (#345) yes",
+			[]testResult{
+				{345, "", "", "345", false, XRefActionNone, &RefSpan{Start: 5, End: 9}, nil},
+			},
+		},
+		{
+			"For #22,#23 no, neither #28:#29 or !30!31#32;33 should",
+			[]testResult{},
+		},
+		{
+			"For #24, and #25. yes; also #26; #27? #28! and #29: should",
+			[]testResult{
+				{24, "", "", "24", false, XRefActionNone, &RefSpan{Start: 4, End: 7}, nil},
+				{25, "", "", "25", false, XRefActionNone, &RefSpan{Start: 13, End: 16}, nil},
+				{26, "", "", "26", false, XRefActionNone, &RefSpan{Start: 28, End: 31}, nil},
+				{27, "", "", "27", false, XRefActionNone, &RefSpan{Start: 33, End: 36}, nil},
+				{28, "", "", "28", false, XRefActionNone, &RefSpan{Start: 38, End: 41}, nil},
+				{29, "", "", "29", false, XRefActionNone, &RefSpan{Start: 47, End: 50}, nil},
+			},
+		},
+		{
+			"This user3/repo4#200, yes.",
+			[]testResult{
+				{200, "user3", "repo4", "200", false, XRefActionNone, &RefSpan{Start: 5, End: 20}, nil},
+			},
+		},
+		{
 			"Which abc. #9434 same as above",
 			[]testResult{
 				{9434, "", "", "9434", false, XRefActionNone, &RefSpan{Start: 11, End: 16}, nil},
@@ -217,6 +250,16 @@ func testFixtures(t *testing.T, fixtures []testFixture, context string) {
 	setting.AppURL = prevURL
 }
 
+func TestFindAllMentions(t *testing.T) {
+	res := FindAllMentionsBytes([]byte("@tasha, @mike; @lucy: @john"))
+	assert.EqualValues(t, []RefSpan{
+		{Start: 0, End: 6},
+		{Start: 8, End: 13},
+		{Start: 15, End: 20},
+		{Start: 22, End: 27},
+	}, res)
+}
+
 func TestRegExp_mentionPattern(t *testing.T) {
 	trueTestCases := []struct {
 		pat string