From b127d79626b1ce18d73e44ce517d9d73277bc336 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Thu, 21 Nov 2019 12:41:23 -0300 Subject: [PATCH 1/9] Add support for local vs. remote xrefs --- modules/markup/html.go | 21 +++++++--- modules/markup/html_internal_test.go | 57 ++++++++++++++++----------- modules/references/references.go | 54 +++++++++++++++++-------- modules/references/references_test.go | 52 +++++++++++++++--------- 4 files changed, 121 insertions(+), 63 deletions(-) diff --git a/modules/markup/html.go b/modules/markup/html.go index 924d0089a554..773f16a28971 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -634,10 +634,19 @@ func issueIndexPatternProcessor(ctx *postProcessCtx, node *html.Node) { ref *references.RenderizableReference ) - if ctx.metas["style"] == IssueNameStyleAlphanumeric { - found, ref = references.FindRenderizableReferenceAlphanumeric(node.Data) - } else { - found, ref = references.FindRenderizableReferenceNumeric(node.Data) + _, exttrack := ctx.metas["format"] + alphanum := ctx.metas["style"] == IssueNameStyleAlphanumeric + + // 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 + found, ref = references.FindRenderizableReferenceNumeric(node.Data, exttrack && alphanum) + if exttrack && alphanum { + if found2, ref2 := references.FindRenderizableReferenceAlphanumeric(node.Data); found2 { + if !found || ref2.RefLocation.Start < ref.RefLocation.Start { + found = true + ref = ref2 + } + } } if !found { return @@ -645,7 +654,7 @@ func issueIndexPatternProcessor(ctx *postProcessCtx, node *html.Node) { var link *html.Node reftext := node.Data[ref.RefLocation.Start:ref.RefLocation.End] - if _, ok := ctx.metas["format"]; ok { + if exttrack && !ref.Local { ctx.metas["index"] = ref.Issue link = createLink(com.Expand(ctx.metas["format"], ctx.metas), reftext, "issue") } else if ref.Owner == "" { @@ -661,7 +670,7 @@ func issueIndexPatternProcessor(ctx *postProcessCtx, node *html.Node) { // Decorate action keywords if actionable var keyword *html.Node - if references.IsXrefActionable(ref.Action) { + if references.IsXrefActionable(ref, exttrack, alphanum) { keyword = createKeyword(node.Data[ref.ActionLocation.Start:ref.ActionLocation.End]) } else { keyword = &html.Node{ diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go index 9722063e177a..79a845d0e24a 100644 --- a/modules/markup/html_internal_test.go +++ b/modules/markup/html_internal_test.go @@ -25,8 +25,8 @@ func alphanumIssueLink(baseURL, class, name string) string { } // numericLink an HTML to a numeric-style issue -func numericIssueLink(baseURL, class string, index int) string { - return link(util.URLJoin(baseURL, strconv.Itoa(index)), class, fmt.Sprintf("#%d", index)) +func numericIssueLink(baseURL, class string, index int, marker string) string { + return link(util.URLJoin(baseURL, strconv.Itoa(index)), class, fmt.Sprintf("%s%d", marker, index)) } // link an HTML link @@ -75,8 +75,12 @@ func TestRender_IssueIndexPattern(t *testing.T) { test("#abcd") test("test#1234") test("#1234test") - test(" test #1234test") + test("#abcd") + test("test!1234") + test("!1234test") + test(" test !1234test") test("/home/gitea/#1234") + test("/home/gitea/!1234") // should not render issue mention without leading space test("test#54321 issue") @@ -90,42 +94,51 @@ func TestRender_IssueIndexPattern2(t *testing.T) { setting.AppSubURL = AppSubURL // numeric: render inputs with valid mentions - test := func(s, expectedFmt string, indices ...int) { + test := func(s, expectedFmt, marker string, indices ...int) { links := make([]interface{}, len(indices)) for i, index := range indices { - links[i] = numericIssueLink(util.URLJoin(setting.AppSubURL, "issues"), "issue", index) + links[i] = numericIssueLink(util.URLJoin(setting.AppSubURL, "issues"), "issue", index, marker) } expectedNil := fmt.Sprintf(expectedFmt, links...) testRenderIssueIndexPattern(t, s, expectedNil, &postProcessCtx{metas: localMetas}) - for i, index := range indices { - links[i] = numericIssueLink("https://someurl.com/someUser/someRepo/", "issue", index) + if marker == "#" { + for i, index := range indices { + links[i] = numericIssueLink("https://someurl.com/someUser/someRepo/", "issue", index, marker) + } + } else { + for i, index := range indices { + links[i] = numericIssueLink("http://localhost:3000/someUser/someRepo/issues/", "issue", index, marker) + } } expectedNum := fmt.Sprintf(expectedFmt, links...) testRenderIssueIndexPattern(t, s, expectedNum, &postProcessCtx{metas: numericMetas}) } // should render freestanding mentions - test("#1234 test", "%s test", 1234) - test("test #8 issue", "test %s issue", 8) - test("test issue #1234", "test issue %s", 1234) - test("fixes issue #1234.", "fixes issue %s.", 1234) + test("#1234 test", "%s test", "#", 1234) + test("test #8 issue", "test %s issue", "#", 8) + test("!1234 test", "%s test", "!", 1234) + test("test !8 issue", "test %s issue", "!", 8) + test("test issue #1234", "test issue %s", "#", 1234) + test("fixes issue #1234.", "fixes issue %s.", "#", 1234) // should render mentions in parentheses / brackets - test("(#54321 issue)", "(%s issue)", 54321) - test("[#54321 issue]", "[%s issue]", 54321) - test("test (#9801 extra) issue", "test (%s extra) issue", 9801) - test("test (#1)", "test (%s)", 1) + test("(#54321 issue)", "(%s issue)", "#", 54321) + test("[#54321 issue]", "[%s issue]", "#", 54321) + test("test (#9801 extra) issue", "test (%s extra) issue", "#", 9801) + test("test (!9801 extra) issue", "test (%s extra) issue", "!", 9801) + test("test (#1)", "test (%s)", "#", 1) // should render multiple issue mentions in the same line - test("#54321 #1243", "%s %s", 54321, 1243) - test("wow (#54321 #1243)", "wow (%s %s)", 54321, 1243) - test("(#4)(#5)", "(%s)(%s)", 4, 5) - test("#1 (#4321) test", "%s (%s) test", 1, 4321) + test("#54321 #1243", "%s %s", "#", 54321, 1243) + test("wow (#54321 #1243)", "wow (%s %s)", "#", 54321, 1243) + test("(#4)(#5)", "(%s)(%s)", "#", 4, 5) + test("#1 (#4321) test", "%s (%s) test", "#", 1, 4321) // should render with : - test("#1234: test", "%s: test", 1234) - test("wow (#54321: test)", "wow (%s: test)", 54321) + test("#1234: test", "%s: test", "#", 1234) + test("wow (#54321: test)", "wow (%s: test)", "#", 54321) } func TestRender_IssueIndexPattern3(t *testing.T) { @@ -201,7 +214,7 @@ func TestRender_AutoLink(t *testing.T) { // render valid issue URLs test(util.URLJoin(setting.AppSubURL, "issues", "3333"), - numericIssueLink(util.URLJoin(setting.AppSubURL, "issues"), "issue", 3333)) + numericIssueLink(util.URLJoin(setting.AppSubURL, "issues"), "issue", 3333, "#")) // render valid commit URLs tmp := util.URLJoin(AppSubURL, "commit", "d8a994ef243349f321568f9e36d5c3f444b99cae") diff --git a/modules/references/references.go b/modules/references/references.go index 17e9ec2c910e..65e0cc21f7d2 100644 --- a/modules/references/references.go +++ b/modules/references/references.go @@ -29,12 +29,12 @@ 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|$))`) issueCloseKeywordsPat, issueReopenKeywordsPat *regexp.Regexp issueKeywordsOnce sync.Once @@ -70,6 +70,7 @@ type RenderizableReference struct { Issue string Owner string Name string + Local bool RefLocation *RefSpan Action XRefAction ActionLocation *RefSpan @@ -79,6 +80,7 @@ type rawReference struct { index int64 owner string name string + local bool action XRefAction issue string refLocation *RefSpan @@ -202,14 +204,14 @@ func FindAllIssueReferences(content string) []IssueReference { } // FindRenderizableReferenceNumeric returns the first unvalidated reference found in a string. -func FindRenderizableReferenceNumeric(content string) (bool, *RenderizableReference) { +func FindRenderizableReferenceNumeric(content string, localOnly bool) (bool, *RenderizableReference) { match := issueNumericPattern.FindStringSubmatchIndex(content) if match == nil { if match = crossReferenceIssueNumericPattern.FindStringSubmatchIndex(content); match == nil { return false, nil } } - r := getCrossReference([]byte(content), match[2], match[3], false) + r := getCrossReference([]byte(content), match[2], match[3], false, localOnly) if r == nil { return false, nil } @@ -218,6 +220,7 @@ func FindRenderizableReferenceNumeric(content string) (bool, *RenderizableRefere Issue: r.issue, Owner: r.owner, Name: r.name, + Local: r.local, RefLocation: r.refLocation, Action: r.action, ActionLocation: r.actionLocation, @@ -238,6 +241,7 @@ func FindRenderizableReferenceAlphanumeric(content string) (bool, *RenderizableR RefLocation: &RefSpan{Start: match[2], End: match[3]}, Action: action, ActionLocation: location, + Local: false, } } @@ -248,14 +252,14 @@ func findAllIssueReferencesBytes(content []byte, links []string) []*rawReference matches := issueNumericPattern.FindAllSubmatchIndex(content, -1) for _, match := range matches { - if ref := getCrossReference(content, match[2], match[3], false); ref != nil { + if ref := getCrossReference(content, match[2], match[3], false, false); ref != nil { ret = append(ret, ref) } } matches = crossReferenceIssueNumericPattern.FindAllSubmatchIndex(content, -1) for _, match := range matches { - if ref := getCrossReference(content, match[2], match[3], false); ref != nil { + if ref := getCrossReference(content, match[2], match[3], false, false); ref != nil { ret = append(ret, ref) } } @@ -273,12 +277,19 @@ func findAllIssueReferencesBytes(content []byte, links []string) []*rawReference if len(parts) != 5 || parts[0] != "" { continue } - if parts[3] != "issues" && parts[3] != "pulls" { + var sep string + if parts[3] == "issues" { + // Issues can be external + sep = "#" + } else if parts[3] == "pulls" { + // PRs are always local + sep = "!" + } else { continue } // Note: closing/reopening keywords not supported with URLs - bytes := []byte(parts[1] + "/" + parts[2] + "#" + parts[4]) - if ref := getCrossReference(bytes, 0, len(bytes), true); ref != nil { + bytes := []byte(parts[1] + "/" + parts[2] + sep + parts[4]) + if ref := getCrossReference(bytes, 0, len(bytes), true, false); ref != nil { ref.refLocation = nil ret = append(ret, ref) } @@ -288,13 +299,18 @@ func findAllIssueReferencesBytes(content []byte, links []string) []*rawReference return ret } -func getCrossReference(content []byte, start, end int, fromLink bool) *rawReference { +func getCrossReference(content []byte, start, end int, fromLink bool, localOnly bool) *rawReference { refid := string(content[start:end]) - parts := strings.Split(refid, "#") - if len(parts) != 2 { + sep := strings.IndexAny(refid, "#!") + if sep < 0 { + return nil + } + local := refid[sep] == '!' + if localOnly && !local { return nil } - repo, issue := parts[0], parts[1] + repo := refid[:sep] + issue := refid[sep+1:] index, err := strconv.ParseInt(issue, 10, 64) if err != nil { return nil @@ -309,11 +325,12 @@ func getCrossReference(content []byte, start, end int, fromLink bool) *rawRefere index: index, action: action, issue: issue, + local: local, refLocation: &RefSpan{Start: start, End: end}, actionLocation: location, } } - parts = strings.Split(strings.ToLower(repo), "/") + parts := strings.Split(strings.ToLower(repo), "/") if len(parts) != 2 { return nil } @@ -328,6 +345,7 @@ func getCrossReference(content []byte, start, end int, fromLink bool) *rawRefere name: name, action: action, issue: issue, + local: local, refLocation: &RefSpan{Start: start, End: end}, actionLocation: location, } @@ -352,6 +370,10 @@ func findActionKeywords(content []byte, start int) (XRefAction, *RefSpan) { } // IsXrefActionable returns true if the xref action is actionable (i.e. produces a result when resolved) -func IsXrefActionable(a XRefAction) bool { - return a == XRefActionCloses || a == XRefActionReopens +func IsXrefActionable(ref *RenderizableReference, extTracker bool, alphaNum bool) bool { + // External references cannot be automatically closed + if extTracker && !ref.Local { + return false + } + return ref.Action == XRefActionCloses || ref.Action == XRefActionReopens } diff --git a/modules/references/references_test.go b/modules/references/references_test.go index d46c5e85d72a..a373c7e26a7c 100644 --- a/modules/references/references_test.go +++ b/modules/references/references_test.go @@ -22,6 +22,7 @@ type testResult struct { Owner string Name string Issue string + Local bool Action XRefAction RefLocation *RefSpan ActionLocation *RefSpan @@ -33,7 +34,13 @@ func TestFindAllIssueReferences(t *testing.T) { { "Simply closes: #29 yes", []testResult{ - {29, "", "", "29", XRefActionCloses, &RefSpan{Start: 15, End: 18}, &RefSpan{Start: 7, End: 13}}, + {29, "", "", "29", false, XRefActionCloses, &RefSpan{Start: 15, End: 18}, &RefSpan{Start: 7, End: 13}}, + }, + }, + { + "Simply closes: !29 yes", + []testResult{ + {29, "", "", "29", true, XRefActionCloses, &RefSpan{Start: 15, End: 18}, &RefSpan{Start: 7, End: 13}}, }, }, { @@ -43,7 +50,7 @@ func TestFindAllIssueReferences(t *testing.T) { { " #124 yes, this is a reference.", []testResult{ - {124, "", "", "124", XRefActionNone, &RefSpan{Start: 0, End: 4}, nil}, + {124, "", "", "124", false, XRefActionNone, &RefSpan{Start: 0, End: 4}, nil}, }, }, { @@ -57,7 +64,13 @@ func TestFindAllIssueReferences(t *testing.T) { { "This user3/repo4#200 yes.", []testResult{ - {200, "user3", "repo4", "200", XRefActionNone, &RefSpan{Start: 5, End: 20}, nil}, + {200, "user3", "repo4", "200", false, XRefActionNone, &RefSpan{Start: 5, End: 20}, nil}, + }, + }, + { + "This user3/repo4!200 yes.", + []testResult{ + {200, "user3", "repo4", "200", true, XRefActionNone, &RefSpan{Start: 5, End: 20}, nil}, }, }, { @@ -67,19 +80,19 @@ func TestFindAllIssueReferences(t *testing.T) { { "This [two](/user2/repo1/issues/921) yes.", []testResult{ - {921, "user2", "repo1", "921", XRefActionNone, nil, nil}, + {921, "user2", "repo1", "921", false, XRefActionNone, nil, nil}, }, }, { "This [three](/user2/repo1/pulls/922) yes.", []testResult{ - {922, "user2", "repo1", "922", XRefActionNone, nil, nil}, + {922, "user2", "repo1", "922", true, XRefActionNone, nil, nil}, }, }, { "This [four](http://gitea.com:3000/user3/repo4/issues/203) yes.", []testResult{ - {203, "user3", "repo4", "203", XRefActionNone, nil, nil}, + {203, "user3", "repo4", "203", false, XRefActionNone, nil, nil}, }, }, { @@ -93,50 +106,50 @@ func TestFindAllIssueReferences(t *testing.T) { { "This http://gitea.com:3000/user4/repo5/pulls/202 yes.", []testResult{ - {202, "user4", "repo5", "202", XRefActionNone, nil, nil}, + {202, "user4", "repo5", "202", true, XRefActionNone, nil, nil}, }, }, { "This http://GiTeA.COM:3000/user4/repo6/pulls/205 yes.", []testResult{ - {205, "user4", "repo6", "205", XRefActionNone, nil, nil}, + {205, "user4", "repo6", "205", true, XRefActionNone, nil, nil}, }, }, { "Reopens #15 yes", []testResult{ - {15, "", "", "15", XRefActionReopens, &RefSpan{Start: 8, End: 11}, &RefSpan{Start: 0, End: 7}}, + {15, "", "", "15", false, XRefActionReopens, &RefSpan{Start: 8, End: 11}, &RefSpan{Start: 0, End: 7}}, }, }, { "This closes #20 for you yes", []testResult{ - {20, "", "", "20", XRefActionCloses, &RefSpan{Start: 12, End: 15}, &RefSpan{Start: 5, End: 11}}, + {20, "", "", "20", false, XRefActionCloses, &RefSpan{Start: 12, End: 15}, &RefSpan{Start: 5, End: 11}}, }, }, { "Do you fix user6/repo6#300 ? yes", []testResult{ - {300, "user6", "repo6", "300", XRefActionCloses, &RefSpan{Start: 11, End: 26}, &RefSpan{Start: 7, End: 10}}, + {300, "user6", "repo6", "300", false, XRefActionCloses, &RefSpan{Start: 11, End: 26}, &RefSpan{Start: 7, End: 10}}, }, }, { "For 999 #1235 no keyword, but yes", []testResult{ - {1235, "", "", "1235", XRefActionNone, &RefSpan{Start: 8, End: 13}, nil}, + {1235, "", "", "1235", false, XRefActionNone, &RefSpan{Start: 8, End: 13}, nil}, }, }, { "Which abc. #9434 same as above", []testResult{ - {9434, "", "", "9434", XRefActionNone, &RefSpan{Start: 11, End: 16}, nil}, + {9434, "", "", "9434", false, XRefActionNone, &RefSpan{Start: 11, End: 16}, nil}, }, }, { "This closes #600 and reopens #599", []testResult{ - {600, "", "", "600", XRefActionCloses, &RefSpan{Start: 12, End: 16}, &RefSpan{Start: 5, End: 11}}, - {599, "", "", "599", XRefActionReopens, &RefSpan{Start: 29, End: 33}, &RefSpan{Start: 21, End: 28}}, + {600, "", "", "600", false, XRefActionCloses, &RefSpan{Start: 12, End: 16}, &RefSpan{Start: 5, End: 11}}, + {599, "", "", "599", false, XRefActionReopens, &RefSpan{Start: 29, End: 33}, &RefSpan{Start: 21, End: 28}}, }, }, } @@ -190,6 +203,7 @@ func testFixtures(t *testing.T, fixtures []testFixture, context string) { index: e.Index, owner: e.Owner, name: e.Name, + local: e.Local, action: e.Action, issue: e.Issue, refLocation: e.RefLocation, @@ -329,25 +343,25 @@ func TestCustomizeCloseKeywords(t *testing.T) { { "Simplemente cierra: #29 yes", []testResult{ - {29, "", "", "29", XRefActionCloses, &RefSpan{Start: 20, End: 23}, &RefSpan{Start: 12, End: 18}}, + {29, "", "", "29", false, XRefActionCloses, &RefSpan{Start: 20, End: 23}, &RefSpan{Start: 12, End: 18}}, }, }, { "Closes: #123 no, this English.", []testResult{ - {123, "", "", "123", XRefActionNone, &RefSpan{Start: 8, End: 12}, nil}, + {123, "", "", "123", false, XRefActionNone, &RefSpan{Start: 8, End: 12}, nil}, }, }, { "Cerró user6/repo6#300 yes", []testResult{ - {300, "user6", "repo6", "300", XRefActionCloses, &RefSpan{Start: 7, End: 22}, &RefSpan{Start: 0, End: 6}}, + {300, "user6", "repo6", "300", false, XRefActionCloses, &RefSpan{Start: 7, End: 22}, &RefSpan{Start: 0, End: 6}}, }, }, { "Reabre user3/repo4#200 yes", []testResult{ - {200, "user3", "repo4", "200", XRefActionReopens, &RefSpan{Start: 7, End: 22}, &RefSpan{Start: 0, End: 6}}, + {200, "user3", "repo4", "200", false, XRefActionReopens, &RefSpan{Start: 7, End: 22}, &RefSpan{Start: 0, End: 6}}, }, }, } From fca2907b03e6144bbaa45ac973b1b97d611014ef Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Thu, 21 Nov 2019 18:09:05 -0300 Subject: [PATCH 2/9] Add doc for references --- .../doc/usage/issue-references.en-us.md | 149 ++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 docs/content/doc/usage/issue-references.en-us.md diff --git a/docs/content/doc/usage/issue-references.en-us.md b/docs/content/doc/usage/issue-references.en-us.md new file mode 100644 index 000000000000..adebdbc6fe16 --- /dev/null +++ b/docs/content/doc/usage/issue-references.en-us.md @@ -0,0 +1,149 @@ +--- +date: "2019-11-21T17:00:00-03:00" +title: "Usage: Automatically Linked References" +slug: "automatically-linked-references" +weight: 15 +toc: true +draft: false +menu: + sidebar: + parent: "usage" + name: "Automatically Linked References" + weight: 15 + identifier: "automatically-linked-references" +--- + +# Automatically Linked References in Issues, Pull Requests and Commit Messages + +When an issue, pull request or comment is posted, the text description is parsed +in search for references. These references will be shown as links in the Issue View +and, in some cases, produce certain _actions_. + +Likewise, commit messages are parsed when they are listed, and _actions_ +are can be triggered when they are pushed to the main branch. + +To prevent the creation of unintended references, there are certain rules +for them to be recognized. For example, they should not be included inside code +text. They should also be reasonably cleared from their surrounding text +(for example, using spaces). + +## User Mentions + +When a text in the form `@username` is found and `username` matches the name +of an existing user, a _mention_ reference is created. This will be shown +by changing the text into a link to said user's profile, and possibly create +a notification for the mentioned user depending on whether they have +the necessary permission to access the contents. + +Example: + +> [@John](#), can you give this a look? + +Commit messages do not produce user notifications. + +## Commits + +Commits can be referenced using their SHA1 hash, or a portion of it of +at least seven characters. They will be shown as a link to the corresponding +commit. + +Example: + +> This bug was introduced in [e59ff077](#) + +## Issues and Pull Requests + +A reference to another issue or pull request can be created using the simple +notation `#1234`, where _1234_ is the number of an issue or pull request +in the same repository. These references will be shown as links to the +referenced content. + +The effect of creating this type of reference is that a _notice_ will be +created in the referenced document, provided the creator of the reference +has reading permissions on it. + +Example: + +> This seems related to [#1234](#) + +Issues and pull requests in other repositories can be referred to as well +using the form `owner/repository#1234`: + +> This seems related to [mike/compiler#1234](#) + +## Actionable References in Pull Requests and Commit Messages + +Sometimes a commit or pull request may fix or bring back a problem documented +in a particular issue. Gitea supports closing and reopening the referenced +issues by preceding the reference with a particular _keyword_. Common keywords +include "closes", "fixes", "reopens", etc. This list can be +[customized]({{< ref "/doc/advanced/config-cheat-sheet.en-us.md" >}}) by the +site administrator. + +Example: + +> This PR _closes_ [#1234](#) + +If the actionable reference is accepted, this will create a notice on the +referenced issue announcing that it will be closed when the referencing PR +is merged. + +For an actionable reference to be accepted, _at least one_ of the following +conditions must be met: + +* The commenter has permissions to close or reopen the issue at the moment +of creating the reference. +* The reference is inside a commit message. +* The reference is posted as part of the pull request description. + +In the last case, the issue will be closed or reopened only if the merger +of the pull request has permissions to do so. + +Additionally, only pull requests and commit messages can create an action, +and only issues can be closed or reopened this way. + +The default _keywords_ are: + +* **Closing**: close, closes, closed, fix, fixes, fixed, resolve, resolves, resolved +* **Reopening**: reopen, reopens, reopened + +## External Trackers + +Gitea supports the use of external issue trackers, and references to issues +hosted externally can be created in pull requests. However, if the external +tracker uses numbers to identify issues, they will be indistinguishable from +the pull requests hosted in Gitea. To address this, Gitea allows the use of +the `!` marker to identify pull requests. For example: + +> This is issue [#1234](#), and links to the external tracker. + +> This is pull request [!1234](#), and links to a pull request in Gitea. + +The `!` and `#` can be used interchangeably for issues and pull request _except_ +for this case, where a distinction is required. + +## Issues and Pull Requests References Summary + +| Reference in User1/Repo1 | Repo1 issues are external | RepoZ issues are external | Should render | +|---------------------------|:-------------------------:|:-------------------------:|----------------------------------| +| `#1234` | no | N/A | A link to issue/pull 1234 in `User1/Repo1` | +| `!1234` | no | N/A | A link to issue/pull 1234 in `User1/Repo1` | +| `#1234` | yes | N/A | A link to _external issue_ 1234 for `User1/Repo1` | +| `!1234` | yes | N/A | A link to _PR_ 1234 for `User1/Repo1` | +| `User1/Repo1#1234` | no | N/A | A link to issue/pull 1234 in `User1/Repo1` | +| `User1/Repo1!1234` | no | N/A | A link to issue/pull 1234 in `User1/Repo1` | +| `User1/Repo1#1234` | yes | N/A | A link to _external issue_ 1234 for `User1/Repo1` | +| `User1/Repo1!1234` | yes | N/A | A link to _PR_ 1234 for `User1/Repo1` | +| `UserZ/RepoZ#1234` | N/A | no | A link to issue/pull 1234 in `UserZ/RepoZ` | +| `UserZ/RepoZ!1234` | N/A | no | A link to issue/pull 1234 in `UserZ/RepoZ` | +| `UserZ/RepoZ#1234` | N/A | yes | A link to _external issue_ 1234 for `UserZ/RepoZ` | +| `UserZ/RepoZ!1234` | N/A | yes | A link to _PR_ 1234 for `UserZ/RepoZ` | +| **Alphanumeric issue IDs:** | - | - | - | +| `AAA-1234` | yes | N/A | A link to _external issue_ `AAA-1234` for `User1/Repo1` | +| `!1234` | yes | N/A | A link to _PR_ 1234 for `User1/Repo1` | +| `User1/Repo1!1234` | yes | N/A | A link to _PR_ 1234 for `User1/Repo1` | +| N/A | N/A | yes | A link to _external issue_ `AAA-1234` for `UserZ/RepoZ` | +| `UserZ/RepoZ!1234` | N/A | yes | A link to _PR_ 1234 in `UserZ/RepoZ` | + +_The last section is for repositories with external issue trackers that use alphanumeric format._ + From fc57c671c96bc3dd91a169d16113338c35a9638c Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Thu, 21 Nov 2019 21:07:18 -0300 Subject: [PATCH 3/9] Docs: fix cases not currently supported --- docs/content/doc/usage/issue-references.en-us.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/content/doc/usage/issue-references.en-us.md b/docs/content/doc/usage/issue-references.en-us.md index adebdbc6fe16..c6976f9a3f8a 100644 --- a/docs/content/doc/usage/issue-references.en-us.md +++ b/docs/content/doc/usage/issue-references.en-us.md @@ -136,8 +136,8 @@ for this case, where a distinction is required. | `User1/Repo1!1234` | yes | N/A | A link to _PR_ 1234 for `User1/Repo1` | | `UserZ/RepoZ#1234` | N/A | no | A link to issue/pull 1234 in `UserZ/RepoZ` | | `UserZ/RepoZ!1234` | N/A | no | A link to issue/pull 1234 in `UserZ/RepoZ` | -| `UserZ/RepoZ#1234` | N/A | yes | A link to _external issue_ 1234 for `UserZ/RepoZ` | -| `UserZ/RepoZ!1234` | N/A | yes | A link to _PR_ 1234 for `UserZ/RepoZ` | +| N/A | N/A | yes | A link to _external issue_ 1234 for `UserZ/RepoZ` | +| N/A | N/A | yes | A link to _PR_ 1234 for `UserZ/RepoZ` | | **Alphanumeric issue IDs:** | - | - | - | | `AAA-1234` | yes | N/A | A link to _external issue_ `AAA-1234` for `User1/Repo1` | | `!1234` | yes | N/A | A link to _PR_ 1234 for `User1/Repo1` | From 8d33c80282ec9c8eadadd5e1c5d0676e92a077b8 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Thu, 21 Nov 2019 21:08:21 -0300 Subject: [PATCH 4/9] One more doc fix --- docs/content/doc/usage/issue-references.en-us.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/doc/usage/issue-references.en-us.md b/docs/content/doc/usage/issue-references.en-us.md index c6976f9a3f8a..2094ab481141 100644 --- a/docs/content/doc/usage/issue-references.en-us.md +++ b/docs/content/doc/usage/issue-references.en-us.md @@ -143,7 +143,7 @@ for this case, where a distinction is required. | `!1234` | yes | N/A | A link to _PR_ 1234 for `User1/Repo1` | | `User1/Repo1!1234` | yes | N/A | A link to _PR_ 1234 for `User1/Repo1` | | N/A | N/A | yes | A link to _external issue_ `AAA-1234` for `UserZ/RepoZ` | -| `UserZ/RepoZ!1234` | N/A | yes | A link to _PR_ 1234 in `UserZ/RepoZ` | +| N/A | N/A | yes | A link to _PR_ 1234 in `UserZ/RepoZ` | _The last section is for repositories with external issue trackers that use alphanumeric format._ From 87629f1a113b1c61f4abf1c38c3982fd6a3b6b14 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Fri, 22 Nov 2019 10:59:58 -0300 Subject: [PATCH 5/9] Doc: mentions for teams and orgs --- docs/content/doc/usage/issue-references.en-us.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/content/doc/usage/issue-references.en-us.md b/docs/content/doc/usage/issue-references.en-us.md index 2094ab481141..b2f57b9a0388 100644 --- a/docs/content/doc/usage/issue-references.en-us.md +++ b/docs/content/doc/usage/issue-references.en-us.md @@ -27,7 +27,7 @@ for them to be recognized. For example, they should not be included inside code text. They should also be reasonably cleared from their surrounding text (for example, using spaces). -## User Mentions +## User, Team and Organization Mentions When a text in the form `@username` is found and `username` matches the name of an existing user, a _mention_ reference is created. This will be shown @@ -39,6 +39,14 @@ Example: > [@John](#), can you give this a look? +This is also valid for teams and organizations: + +> [@Documenters](#), we need to plan for this. + +> [@CoolCompanyInc](#), this issue concerns us all! + +Teams will receive mail notifications when appropriate, but whole organizations won't. + Commit messages do not produce user notifications. ## Commits From 8a2402f820532d63bb7b417907e4c11a3091555a Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Fri, 22 Nov 2019 16:38:37 -0300 Subject: [PATCH 6/9] Change !num ref concept, no change in functionality --- .../doc/usage/issue-references.en-us.md | 13 ++++++-- modules/markup/html.go | 17 +++++++--- modules/references/references.go | 31 ++++++++++--------- modules/references/references_test.go | 4 +-- 4 files changed, 42 insertions(+), 23 deletions(-) diff --git a/docs/content/doc/usage/issue-references.en-us.md b/docs/content/doc/usage/issue-references.en-us.md index b2f57b9a0388..eaa1565a3409 100644 --- a/docs/content/doc/usage/issue-references.en-us.md +++ b/docs/content/doc/usage/issue-references.en-us.md @@ -79,6 +79,14 @@ using the form `owner/repository#1234`: > This seems related to [mike/compiler#1234](#) +Alternatively, the `!1234` notation can be used as well. Even when in Gitea +a pull request is a form of issue, the `#1234` form will always link to +an issue; if the linked entry happens to be a pull request instead, Gitea +will redirect as appropriate. With the `!1234` notation, a pull request +link will be created, which will be redirected to an issue if required. +However, this distinction could be important if an external tracker is +used, where links to issues and pull requests are not interchangeable. + ## Actionable References in Pull Requests and Commit Messages Sometimes a commit or pull request may fix or bring back a problem documented @@ -145,13 +153,14 @@ for this case, where a distinction is required. | `UserZ/RepoZ#1234` | N/A | no | A link to issue/pull 1234 in `UserZ/RepoZ` | | `UserZ/RepoZ!1234` | N/A | no | A link to issue/pull 1234 in `UserZ/RepoZ` | | N/A | N/A | yes | A link to _external issue_ 1234 for `UserZ/RepoZ` | -| N/A | N/A | yes | A link to _PR_ 1234 for `UserZ/RepoZ` | +| `UserZ/RepoZ!1234` | N/A | yes | A link to _PR_ 1234 for `UserZ/RepoZ` | | **Alphanumeric issue IDs:** | - | - | - | | `AAA-1234` | yes | N/A | A link to _external issue_ `AAA-1234` for `User1/Repo1` | | `!1234` | yes | N/A | A link to _PR_ 1234 for `User1/Repo1` | | `User1/Repo1!1234` | yes | N/A | A link to _PR_ 1234 for `User1/Repo1` | | N/A | N/A | yes | A link to _external issue_ `AAA-1234` for `UserZ/RepoZ` | -| N/A | N/A | yes | A link to _PR_ 1234 in `UserZ/RepoZ` | +| `UserZ/RepoZ!1234` | N/A | yes | A link to _PR_ 1234 in `UserZ/RepoZ` | _The last section is for repositories with external issue trackers that use alphanumeric format._ +Note: automatic references between repositories with different types of issues (external vs. internal) are not fully supported and may render invalid links. diff --git a/modules/markup/html.go b/modules/markup/html.go index 773f16a28971..53d31b33d30a 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -654,13 +654,22 @@ func issueIndexPatternProcessor(ctx *postProcessCtx, node *html.Node) { var link *html.Node reftext := node.Data[ref.RefLocation.Start:ref.RefLocation.End] - if exttrack && !ref.Local { + if exttrack && !ref.IsPull { ctx.metas["index"] = ref.Issue link = createLink(com.Expand(ctx.metas["format"], ctx.metas), reftext, "issue") - } else if ref.Owner == "" { - link = createLink(util.URLJoin(setting.AppURL, ctx.metas["user"], ctx.metas["repo"], "issues", ref.Issue), reftext, "issue") } else { - link = createLink(util.URLJoin(setting.AppURL, ref.Owner, ref.Name, "issues", ref.Issue), reftext, "issue") + // Path determines the type of link that will be rendered. It's unknown at this point whether + // the linked item is actually a PR or an issue. Luckily it's of no real consequence because + // Gitea will redirect on click as appropriate. + path := "issues" + if ref.IsPull { + path = "pulls" + } + if ref.Owner == "" { + link = createLink(util.URLJoin(setting.AppURL, ctx.metas["user"], ctx.metas["repo"], path, ref.Issue), reftext, "issue") + } else { + link = createLink(util.URLJoin(setting.AppURL, ref.Owner, ref.Name, path, ref.Issue), reftext, "issue") + } } if ref.Action == references.XRefActionNone { diff --git a/modules/references/references.go b/modules/references/references.go index 65e0cc21f7d2..dec77b57b268 100644 --- a/modules/references/references.go +++ b/modules/references/references.go @@ -66,11 +66,14 @@ type IssueReference struct { } // RenderizableReference contains an unverified cross-reference to with rendering information +// The IsPull member means that a `!num` reference was used instead of `#num`. +// This kind of reference is used to make pulls available when an external issue tracker +// is used. Otherwise, `#` and `!` are completely interchangeable. type RenderizableReference struct { Issue string Owner string Name string - Local bool + IsPull bool RefLocation *RefSpan Action XRefAction ActionLocation *RefSpan @@ -80,7 +83,7 @@ type rawReference struct { index int64 owner string name string - local bool + isPull bool action XRefAction issue string refLocation *RefSpan @@ -204,14 +207,14 @@ func FindAllIssueReferences(content string) []IssueReference { } // FindRenderizableReferenceNumeric returns the first unvalidated reference found in a string. -func FindRenderizableReferenceNumeric(content string, localOnly bool) (bool, *RenderizableReference) { +func FindRenderizableReferenceNumeric(content string, prOnly bool) (bool, *RenderizableReference) { match := issueNumericPattern.FindStringSubmatchIndex(content) if match == nil { if match = crossReferenceIssueNumericPattern.FindStringSubmatchIndex(content); match == nil { return false, nil } } - r := getCrossReference([]byte(content), match[2], match[3], false, localOnly) + r := getCrossReference([]byte(content), match[2], match[3], false, prOnly) if r == nil { return false, nil } @@ -220,7 +223,7 @@ func FindRenderizableReferenceNumeric(content string, localOnly bool) (bool, *Re Issue: r.issue, Owner: r.owner, Name: r.name, - Local: r.local, + IsPull: r.isPull, RefLocation: r.refLocation, Action: r.action, ActionLocation: r.actionLocation, @@ -241,7 +244,7 @@ func FindRenderizableReferenceAlphanumeric(content string) (bool, *RenderizableR RefLocation: &RefSpan{Start: match[2], End: match[3]}, Action: action, ActionLocation: location, - Local: false, + IsPull: false, } } @@ -279,10 +282,8 @@ func findAllIssueReferencesBytes(content []byte, links []string) []*rawReference } var sep string if parts[3] == "issues" { - // Issues can be external sep = "#" } else if parts[3] == "pulls" { - // PRs are always local sep = "!" } else { continue @@ -299,14 +300,14 @@ func findAllIssueReferencesBytes(content []byte, links []string) []*rawReference return ret } -func getCrossReference(content []byte, start, end int, fromLink bool, localOnly bool) *rawReference { +func getCrossReference(content []byte, start, end int, fromLink bool, prOnly bool) *rawReference { refid := string(content[start:end]) sep := strings.IndexAny(refid, "#!") if sep < 0 { return nil } - local := refid[sep] == '!' - if localOnly && !local { + isPull := refid[sep] == '!' + if prOnly && !isPull { return nil } repo := refid[:sep] @@ -325,7 +326,7 @@ func getCrossReference(content []byte, start, end int, fromLink bool, localOnly index: index, action: action, issue: issue, - local: local, + isPull: isPull, refLocation: &RefSpan{Start: start, End: end}, actionLocation: location, } @@ -345,7 +346,7 @@ func getCrossReference(content []byte, start, end int, fromLink bool, localOnly name: name, action: action, issue: issue, - local: local, + isPull: isPull, refLocation: &RefSpan{Start: start, End: end}, actionLocation: location, } @@ -371,8 +372,8 @@ func findActionKeywords(content []byte, start int) (XRefAction, *RefSpan) { // IsXrefActionable returns true if the xref action is actionable (i.e. produces a result when resolved) func IsXrefActionable(ref *RenderizableReference, extTracker bool, alphaNum bool) bool { - // External references cannot be automatically closed - if extTracker && !ref.Local { + if extTracker { + // External issues cannot be automatically closed return false } return ref.Action == XRefActionCloses || ref.Action == XRefActionReopens diff --git a/modules/references/references_test.go b/modules/references/references_test.go index a373c7e26a7c..b38ee102ba15 100644 --- a/modules/references/references_test.go +++ b/modules/references/references_test.go @@ -22,7 +22,7 @@ type testResult struct { Owner string Name string Issue string - Local bool + IsPull bool Action XRefAction RefLocation *RefSpan ActionLocation *RefSpan @@ -203,7 +203,7 @@ func testFixtures(t *testing.T, fixtures []testFixture, context string) { index: e.Index, owner: e.Owner, name: e.Name, - local: e.Local, + isPull: e.IsPull, action: e.Action, issue: e.Issue, refLocation: e.RefLocation, From e16b9c3c2c2e0305b762b17c3466af87fb11411c Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Fri, 22 Nov 2019 19:22:03 -0300 Subject: [PATCH 7/9] Fix test --- modules/markup/html_internal_test.go | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go index 79a845d0e24a..2746dec2cf06 100644 --- a/modules/markup/html_internal_test.go +++ b/modules/markup/html_internal_test.go @@ -95,21 +95,24 @@ func TestRender_IssueIndexPattern2(t *testing.T) { // numeric: render inputs with valid mentions test := func(s, expectedFmt, marker string, indices ...int) { + var path, prefix string + if marker == "!" { + path = "pulls" + prefix = "http://localhost:3000/someUser/someRepo/pulls/" + } else { + path = "issues" + prefix = "https://someurl.com/someUser/someRepo/" + } + links := make([]interface{}, len(indices)) for i, index := range indices { - links[i] = numericIssueLink(util.URLJoin(setting.AppSubURL, "issues"), "issue", index, marker) + links[i] = numericIssueLink(util.URLJoin(setting.AppSubURL, path), "issue", index, marker) } expectedNil := fmt.Sprintf(expectedFmt, links...) testRenderIssueIndexPattern(t, s, expectedNil, &postProcessCtx{metas: localMetas}) - if marker == "#" { - for i, index := range indices { - links[i] = numericIssueLink("https://someurl.com/someUser/someRepo/", "issue", index, marker) - } - } else { - for i, index := range indices { - links[i] = numericIssueLink("http://localhost:3000/someUser/someRepo/issues/", "issue", index, marker) - } + for i, index := range indices { + links[i] = numericIssueLink(prefix, "issue", index, marker) } expectedNum := fmt.Sprintf(expectedFmt, links...) testRenderIssueIndexPattern(t, s, expectedNum, &postProcessCtx{metas: numericMetas}) From 1db800ae1ea439de66e93aa2ab15054f1f0bbda5 Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Mon, 25 Nov 2019 15:06:44 -0300 Subject: [PATCH 8/9] Improve table of issue reference types --- ...eferences.en-us.md => linked-references.en-us.md} | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) rename docs/content/doc/usage/{issue-references.en-us.md => linked-references.en-us.md} (94%) diff --git a/docs/content/doc/usage/issue-references.en-us.md b/docs/content/doc/usage/linked-references.en-us.md similarity index 94% rename from docs/content/doc/usage/issue-references.en-us.md rename to docs/content/doc/usage/linked-references.en-us.md index eaa1565a3409..46f8f18a1870 100644 --- a/docs/content/doc/usage/issue-references.en-us.md +++ b/docs/content/doc/usage/linked-references.en-us.md @@ -140,6 +140,10 @@ for this case, where a distinction is required. ## Issues and Pull Requests References Summary +This table illustrates the different kinds of cross-reference for issues and pull requests. +In the examples, `User1/Repo1` refers to the repository where the reference is used, while +`UserZ/RepoZ` indicates a different repository. + | Reference in User1/Repo1 | Repo1 issues are external | RepoZ issues are external | Should render | |---------------------------|:-------------------------:|:-------------------------:|----------------------------------| | `#1234` | no | N/A | A link to issue/pull 1234 in `User1/Repo1` | @@ -152,15 +156,17 @@ for this case, where a distinction is required. | `User1/Repo1!1234` | yes | N/A | A link to _PR_ 1234 for `User1/Repo1` | | `UserZ/RepoZ#1234` | N/A | no | A link to issue/pull 1234 in `UserZ/RepoZ` | | `UserZ/RepoZ!1234` | N/A | no | A link to issue/pull 1234 in `UserZ/RepoZ` | -| N/A | N/A | yes | A link to _external issue_ 1234 for `UserZ/RepoZ` | +| _Not supported_ | N/A | yes | A link to _external issue_ 1234 for `UserZ/RepoZ` | | `UserZ/RepoZ!1234` | N/A | yes | A link to _PR_ 1234 for `UserZ/RepoZ` | | **Alphanumeric issue IDs:** | - | - | - | | `AAA-1234` | yes | N/A | A link to _external issue_ `AAA-1234` for `User1/Repo1` | | `!1234` | yes | N/A | A link to _PR_ 1234 for `User1/Repo1` | | `User1/Repo1!1234` | yes | N/A | A link to _PR_ 1234 for `User1/Repo1` | -| N/A | N/A | yes | A link to _external issue_ `AAA-1234` for `UserZ/RepoZ` | +| _Not supported_ | N/A | yes | A link to _external issue_ `AAA-1234` for `UserZ/RepoZ` | | `UserZ/RepoZ!1234` | N/A | yes | A link to _PR_ 1234 in `UserZ/RepoZ` | _The last section is for repositories with external issue trackers that use alphanumeric format._ +_**N/A**: not applicable._ -Note: automatic references between repositories with different types of issues (external vs. internal) are not fully supported and may render invalid links. +Note: automatic references between repositories with different types of issues (external vs. internal) are not fully supported +and may render invalid links. From 491c94b64f71008ab361b55ba30ce8ea5153803e Mon Sep 17 00:00:00 2001 From: Guillermo Prandi Date: Mon, 25 Nov 2019 15:10:16 -0300 Subject: [PATCH 9/9] Fix paragraph mark --- docs/content/doc/usage/linked-references.en-us.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/content/doc/usage/linked-references.en-us.md b/docs/content/doc/usage/linked-references.en-us.md index 46f8f18a1870..4dd84426880a 100644 --- a/docs/content/doc/usage/linked-references.en-us.md +++ b/docs/content/doc/usage/linked-references.en-us.md @@ -166,6 +166,7 @@ In the examples, `User1/Repo1` refers to the repository where the reference is u | `UserZ/RepoZ!1234` | N/A | yes | A link to _PR_ 1234 in `UserZ/RepoZ` | _The last section is for repositories with external issue trackers that use alphanumeric format._ + _**N/A**: not applicable._ Note: automatic references between repositories with different types of issues (external vs. internal) are not fully supported