detached 6859f89

feat(web): link #N references in Markdown and show merge request links on issue pages

commit
6859f892d4a1a5b893fd0688608bff47279dc3fc
author
brice · · 2026-10-20 15:15 UTC
parent
1386983
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
10 files · +454 -73
Unified
UnifiedSplit
M internal/adapters/web/fragments.go +3 -1
@@ -63,7 +63,7 @@ func liveEvent(we ports.WatchEvent) bool {
6363 case domain.RepoCreated, domain.RepoUpdated, domain.RepoDeleted, domain.PushEvent,
6464 domain.MergeRequestOpened, domain.MergeRequestMerged, domain.MergeRequestClosed, domain.MergeRequestReady, domain.MergeRequestMergeability, domain.CommentAdded, domain.CommentEdited, domain.CommentDeleted, domain.CommentResolved,
6565 domain.RunUpdated, domain.JobUpdated,
66- domain.IssueOpened, domain.IssueClosed, domain.IssueReopened, domain.IssueCommented, domain.IssueSubmitted, domain.IssuePendingRemoved:
66+ domain.IssueOpened, domain.IssueClosed, domain.IssueReopened, domain.IssueCommented, domain.IssueSubmitted, domain.IssuePendingRemoved, domain.IssueReferenced:
6767 return true
6868 }
6969 return false
@@ -210,6 +210,8 @@ func (l *liveStream) repoPage(ctx context.Context, we ports.WatchEvent) ([]fragm
210210 return l.issueNotice(ctx, ev.Issue.Number, "this issue was reopened — reload")
211211 case domain.IssueCommented:
212212 return l.issueNotice(ctx, ev.Number, "a comment was added — reload")
213+ case domain.IssueReferenced:
214+ return l.issueNotice(ctx, ev.Issue, "a merge request mentions this issue — reload")
213215 case domain.PushEvent:
214216 switch l.view {
215217 case "mr":
@@ -63,7 +63,7 @@ func liveEvent(we ports.WatchEvent) bool {
63 case domain.RepoCreated, domain.RepoUpdated, domain.RepoDeleted, domain.PushEvent,63 case domain.RepoCreated, domain.RepoUpdated, domain.RepoDeleted, domain.PushEvent,
64 domain.MergeRequestOpened, domain.MergeRequestMerged, domain.MergeRequestClosed, domain.MergeRequestReady, domain.MergeRequestMergeability, domain.CommentAdded, domain.CommentEdited, domain.CommentDeleted, domain.CommentResolved,64 domain.MergeRequestOpened, domain.MergeRequestMerged, domain.MergeRequestClosed, domain.MergeRequestReady, domain.MergeRequestMergeability, domain.CommentAdded, domain.CommentEdited, domain.CommentDeleted, domain.CommentResolved,
65 domain.RunUpdated, domain.JobUpdated,65 domain.RunUpdated, domain.JobUpdated,
66- domain.IssueOpened, domain.IssueClosed, domain.IssueReopened, domain.IssueCommented, domain.IssueSubmitted, domain.IssuePendingRemoved:66+ domain.IssueOpened, domain.IssueClosed, domain.IssueReopened, domain.IssueCommented, domain.IssueSubmitted, domain.IssuePendingRemoved, domain.IssueReferenced:
67 return true67 return true
68 }68 }
69 return false69 return false
@@ -210,6 +210,8 @@ func (l *liveStream) repoPage(ctx context.Context, we ports.WatchEvent) ([]fragm
210 return l.issueNotice(ctx, ev.Issue.Number, "this issue was reopened — reload")210 return l.issueNotice(ctx, ev.Issue.Number, "this issue was reopened — reload")
211 case domain.IssueCommented:211 case domain.IssueCommented:
212 return l.issueNotice(ctx, ev.Number, "a comment was added — reload")212 return l.issueNotice(ctx, ev.Number, "a comment was added — reload")
213+ case domain.IssueReferenced:
214+ return l.issueNotice(ctx, ev.Issue, "a merge request mentions this issue — reload")
213 case domain.PushEvent:215 case domain.PushEvent:
214 switch l.view {216 switch l.view {
215 case "mr":217 case "mr":
M internal/adapters/web/issues.go +62 -2
@@ -83,6 +83,57 @@ type IssuePage struct {
8383 Form *CommentForm
8484 Transition *IssueActions
8585 Live bool
86+ // ClosedBy is the merge request whose merge closed the issue.
87+ ClosedBy *Link
88+ // References are the merge requests naming the issue.
89+ References []IssueRefView
90+}
91+
92+// IssueRefView is a merge request naming an issue.
93+type IssueRefView struct {
94+ Number, Title, State, Href string
95+ Closes bool
96+}
97+
98+// maxNumberLookups bounds the lookups of #N references of one render.
99+const maxNumberLookups = 50
100+
101+// numberResolver resolves #N to the page of an issue or a merge request of
102+// the repository. It is meant for one render: answers are memoized and at most
103+// maxNumberLookups numbers are asked, the others do not resolve.
104+func (s *Server) numberResolver(ctx context.Context, repo string) func(int64) (string, bool) {
105+ if s.cfg.IssueNumbers == nil {
106+ return nil
107+ }
108+ name, err := domain.ParseRepoName(repo)
109+ if err != nil {
110+ return nil
111+ }
112+ type answer struct {
113+ href string
114+ ok bool
115+ }
116+ seen := map[int64]answer{}
117+ return func(n int64) (string, bool) {
118+ if a, ok := seen[n]; ok {
119+ return a.href, a.ok
120+ }
121+ if len(seen) >= maxNumberLookups {
122+ return "", false
123+ }
124+ var a answer
125+ kind, err := s.cfg.IssueNumbers.Kind(ctx, principal(ctx), name, n)
126+ if err == nil {
127+ switch kind {
128+ case domain.NumberIssue:
129+ a = answer{issueHref(repo, n), true}
130+ case domain.NumberMergeRequest:
131+ a = answer{mrHref(repo, n), true}
132+ }
133+ }
134+ seen[n] = a
135+ return a.href, a.ok
136+ }
86137 }
87138
88139 // IssueNewPage is the form that opens an issue.
@@ -243,15 +294,24 @@ func (s *Server) issueShow(ctx context.Context, r *http.Request) (templ.Componen
243294 var err error
244295 if i.External {
245296 // Written by an anonymous visitor: the untrusted policy.
246- h, err = s.md.renderUntrusted([]byte(body), "issue-desc-")
297+ h, err = s.md.renderUntrusted([]byte(body), "issue-desc-", s.numberResolver(ctx, d.name()))
247298 } else {
248- h, _, err = s.md.renderWithIDs([]byte(body), d.name(), d.repo.DefaultBranch, "", "issue-desc-")
299+ h, err = s.md.renderIssue([]byte(body), d.name(), d.repo.DefaultBranch, "issue-desc-", s.numberResolver(ctx, d.name()))
249300 }
250301 if err != nil {
251302 return nil, 0, err
252303 }
253304 p.Desc = h
254305 }
306+ if i.ClosedByMergeRequest > 0 {
307+ p.ClosedBy = &Link{Label: "!" + strconv.FormatInt(i.ClosedByMergeRequest, 10), Href: mrHref(d.name(), i.ClosedByMergeRequest)}
308+ }
309+ for _, ref := range res.References {
310+ p.References = append(p.References, IssueRefView{
311+ Number: "!" + strconv.FormatInt(ref.Number, 10), Title: visibleText(ref.Title.String()),
312+ State: string(ref.State), Href: mrHref(d.name(), ref.Number), Closes: ref.Closes,
313+ })
314+ }
255315 p.Opened.Name = s.who(i.Author)
256316 if i.External {
257317 // The label is the visitor's, not the owner's: shown as plain text.
@@ -83,6 +83,57 @@ type IssuePage struct {
83 Form *CommentForm83 Form *CommentForm
84 Transition *IssueActions84 Transition *IssueActions
85 Live bool85 Live bool
86+ // ClosedBy is the merge request whose merge closed the issue.
87+ ClosedBy *Link
88+ // References are the merge requests naming the issue.
89+ References []IssueRefView
90+}
91+
92+// IssueRefView is a merge request naming an issue.
93+type IssueRefView struct {
94+ Number, Title, State, Href string
95+ Closes bool
96+}
97+
98+// maxNumberLookups bounds the lookups of #N references of one render.
99+const maxNumberLookups = 50
100+
101+// numberResolver resolves #N to the page of an issue or a merge request of
102+// the repository. It is meant for one render: answers are memoized and at most
103+// maxNumberLookups numbers are asked, the others do not resolve.
104+func (s *Server) numberResolver(ctx context.Context, repo string) func(int64) (string, bool) {
105+ if s.cfg.IssueNumbers == nil {
106+ return nil
107+ }
108+ name, err := domain.ParseRepoName(repo)
109+ if err != nil {
110+ return nil
111+ }
112+ type answer struct {
113+ href string
114+ ok bool
115+ }
116+ seen := map[int64]answer{}
117+ return func(n int64) (string, bool) {
118+ if a, ok := seen[n]; ok {
119+ return a.href, a.ok
120+ }
121+ if len(seen) >= maxNumberLookups {
122+ return "", false
123+ }
124+ var a answer
125+ kind, err := s.cfg.IssueNumbers.Kind(ctx, principal(ctx), name, n)
126+ if err == nil {
127+ switch kind {
128+ case domain.NumberIssue:
129+ a = answer{issueHref(repo, n), true}
130+ case domain.NumberMergeRequest:
131+ a = answer{mrHref(repo, n), true}
132+ }
133+ }
134+ seen[n] = a
135+ return a.href, a.ok
136+ }
86 }137 }
87 138
88 // IssueNewPage is the form that opens an issue.139 // IssueNewPage is the form that opens an issue.
@@ -243,15 +294,24 @@ func (s *Server) issueShow(ctx context.Context, r *http.Request) (templ.Componen
243 var err error294 var err error
244 if i.External {295 if i.External {
245 // Written by an anonymous visitor: the untrusted policy.296 // Written by an anonymous visitor: the untrusted policy.
246- h, err = s.md.renderUntrusted([]byte(body), "issue-desc-")297+ h, err = s.md.renderUntrusted([]byte(body), "issue-desc-", s.numberResolver(ctx, d.name()))
247 } else {298 } else {
248- h, _, err = s.md.renderWithIDs([]byte(body), d.name(), d.repo.DefaultBranch, "", "issue-desc-")299+ h, err = s.md.renderIssue([]byte(body), d.name(), d.repo.DefaultBranch, "issue-desc-", s.numberResolver(ctx, d.name()))
249 }300 }
250 if err != nil {301 if err != nil {
251 return nil, 0, err302 return nil, 0, err
252 }303 }
253 p.Desc = h304 p.Desc = h
254 }305 }
306+ if i.ClosedByMergeRequest > 0 {
307+ p.ClosedBy = &Link{Label: "!" + strconv.FormatInt(i.ClosedByMergeRequest, 10), Href: mrHref(d.name(), i.ClosedByMergeRequest)}
308+ }
309+ for _, ref := range res.References {
310+ p.References = append(p.References, IssueRefView{
311+ Number: "!" + strconv.FormatInt(ref.Number, 10), Title: visibleText(ref.Title.String()),
312+ State: string(ref.State), Href: mrHref(d.name(), ref.Number), Closes: ref.Closes,
313+ })
314+ }
255 p.Opened.Name = s.who(i.Author)315 p.Opened.Name = s.who(i.Author)
256 if i.External {316 if i.External {
257 // The label is the visitor's, not the owner's: shown as plain text.317 // The label is the visitor's, not the owner's: shown as plain text.
M internal/adapters/web/issues.templ +21 -0
@@ -149,6 +149,10 @@ templ IssueShow(l Layout, p IssuePage) {
149149 { " · " }<time datetime={ p.Opened.WhenAt } title={ p.Opened.Abs }>{ p.Opened.When }</time>
150150 }
151151 </dd>
152+ if p.ClosedBy != nil {
153+ <dt>closed by</dt>
154+ <dd><a class="ref" href={ templ.SafeURL(p.ClosedBy.Href) }>{ p.ClosedBy.Label }</a></dd>
155+ }
152156 if p.UpdatedAt != "" {
153157 <dt>updated</dt>
154158 <dd><time datetime={ p.UpdatedAt }>{ p.Updated }</time></dd>
@@ -178,6 +182,23 @@ templ IssueShow(l Layout, p IssuePage) {
178182 }
179183
180184 templ issueRail(p IssuePage) {
185+ if len(p.References) > 0 {
186+ <section aria-labelledby="r-refs">
187+ <h2 id="r-refs">Merge requests</h2>
188+ <ul>
189+ for _, r := range p.References {
190+ <li>
191+ <a class="ref" href={ templ.SafeURL(r.Href) }>{ r.Number }</a>
192+ { " " + r.Title + " " }
193+ <span class="mr-state" data-state={ r.State }>{ r.State }</span>
194+ if r.Closes {
195+ { " · closes" }
196+ }
197+ </li>
198+ }
199+ </ul>
200+ </section>
201+ }
181202 if p.Transition != nil {
182203 <section aria-labelledby="r-act">
183204 <h2 id="r-act">Issue</h2>
@@ -149,6 +149,10 @@ templ IssueShow(l Layout, p IssuePage) {
149 { " · " }<time datetime={ p.Opened.WhenAt } title={ p.Opened.Abs }>{ p.Opened.When }</time>149 { " · " }<time datetime={ p.Opened.WhenAt } title={ p.Opened.Abs }>{ p.Opened.When }</time>
150 }150 }
151 </dd>151 </dd>
152+ if p.ClosedBy != nil {
153+ <dt>closed by</dt>
154+ <dd><a class="ref" href={ templ.SafeURL(p.ClosedBy.Href) }>{ p.ClosedBy.Label }</a></dd>
155+ }
152 if p.UpdatedAt != "" {156 if p.UpdatedAt != "" {
153 <dt>updated</dt>157 <dt>updated</dt>
154 <dd><time datetime={ p.UpdatedAt }>{ p.Updated }</time></dd>158 <dd><time datetime={ p.UpdatedAt }>{ p.Updated }</time></dd>
@@ -178,6 +182,23 @@ templ IssueShow(l Layout, p IssuePage) {
178 }182 }
179 183
180 templ issueRail(p IssuePage) {184 templ issueRail(p IssuePage) {
185+ if len(p.References) > 0 {
186+ <section aria-labelledby="r-refs">
187+ <h2 id="r-refs">Merge requests</h2>
188+ <ul>
189+ for _, r := range p.References {
190+ <li>
191+ <a class="ref" href={ templ.SafeURL(r.Href) }>{ r.Number }</a>
192+ { " " + r.Title + " " }
193+ <span class="mr-state" data-state={ r.State }>{ r.State }</span>
194+ if r.Closes {
195+ { " · closes" }
196+ }
197+ </li>
198+ }
199+ </ul>
200+ </section>
201+ }
181 if p.Transition != nil {202 if p.Transition != nil {
182 <section aria-labelledby="r-act">203 <section aria-labelledby="r-act">
183 <h2 id="r-act">Issue</h2>204 <h2 id="r-act">Issue</h2>
M internal/adapters/web/issues_templ.go +164 -36 generated file: collapsed
load diff
M internal/adapters/web/issues_test.go +45 -0
@@ -309,6 +309,8 @@ func TestIssueLiveNotices(t *testing.T) {
309309 {domain.IssueReopened{Issue: issue}, "issue", "5", "this issue was reopened"},
310310 {domain.IssueCommented{Repo: issue.Repo, Number: 5}, "issue", "5", "a comment was added"},
311311 {domain.IssueCommented{Repo: issue.Repo, Number: 6}, "issue", "5", ""},
312+ {domain.IssueReferenced{Repo: issue.Repo, Issue: 5, MergeRequest: 2}, "issue", "5", "a merge request mentions this issue"},
313+ {domain.IssueReferenced{Repo: issue.Repo, Issue: 6, MergeRequest: 2}, "issue", "5", ""},
312314 {domain.IssueClosed{Issue: issue}, "mrs", "", ""},
313315 {domain.IssueClosed{Issue: issue}, "summary", "", ""},
314316 }
@@ -330,3 +332,46 @@ func TestIssueLiveNotices(t *testing.T) {
330332 require.NoError(t, err)
331333 assert.Empty(t, got)
332334 }
335+
336+func TestIssueShowReferences(t *testing.T) {
337+ t.Parallel()
338+ r := newIssueRig(t, domain.IssuesPublic, true)
339+ title, err := domain.ParseMRTitle("fix <b>it</b>")
340+ require.NoError(t, err)
341+ issue := testIssue(t, 5, domain.IssueStateClosed, "bug", "see #7 and #8")
342+ issue.ClosedByMergeRequest = 2
343+ r.issues.EXPECT().Get(mock.Anything, mock.Anything, repoName(t, "proj"), int64(5)).Return(ports.IssueDetail{
344+ Issue: issue,
345+ References: []ports.IssueReference{
346+ {Number: 2, Title: title, State: domain.MRStateMerged, Closes: true},
347+ {Number: 3, Title: title, State: domain.MRStateOpen},
348+ },
349+ }, nil)
350+ r.nums.EXPECT().Kind(mock.Anything, mock.Anything, repoName(t, "proj"), int64(7)).Return(domain.NumberMergeRequest, nil)
351+ r.nums.EXPECT().Kind(mock.Anything, mock.Anything, repoName(t, "proj"), int64(8)).Return(domain.NumberNone, nil)
352+
353+ rec := r.do("GET", "/proj/issues/5", false, nil)
354+ require.Equal(t, 200, rec.Code, rec.Body.String())
355+ body := rec.Body.String()
356+ assert.Contains(t, body, "closed by")
357+ assert.Contains(t, body, `href="/proj/merge-requests/2"`)
358+ assert.Contains(t, body, ">!2<")
359+ assert.Contains(t, body, ">!3<")
360+ assert.Contains(t, body, `id="r-refs"`)
361+ assert.Contains(t, body, "closes")
362+ assert.Contains(t, body, "fix &lt;b&gt;it&lt;/b&gt;")
363+ assert.Contains(t, body, `href="/proj/merge-requests/7"`, "#7 in the body")
364+ assert.Contains(t, body, `title="merge request 7"`)
365+ assert.NotContains(t, body, `/issues/8"`)
366+}
367+
368+func TestIssueShowWithoutReferences(t *testing.T) {
369+ t.Parallel()
370+ r := newIssueRig(t, domain.IssuesPublic, false)
371+ r.issues.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, int64(5)).Return(ports.IssueDetail{
372+ Issue: testIssue(t, 5, domain.IssueStateOpen, "bug", ""),
373+ }, nil)
374+ body := r.do("GET", "/proj/issues/5", false, nil).Body.String()
375+ assert.NotContains(t, body, "r-refs")
376+ assert.NotContains(t, body, "closed by")
377+}
@@ -309,6 +309,8 @@ func TestIssueLiveNotices(t *testing.T) {
309 {domain.IssueReopened{Issue: issue}, "issue", "5", "this issue was reopened"},309 {domain.IssueReopened{Issue: issue}, "issue", "5", "this issue was reopened"},
310 {domain.IssueCommented{Repo: issue.Repo, Number: 5}, "issue", "5", "a comment was added"},310 {domain.IssueCommented{Repo: issue.Repo, Number: 5}, "issue", "5", "a comment was added"},
311 {domain.IssueCommented{Repo: issue.Repo, Number: 6}, "issue", "5", ""},311 {domain.IssueCommented{Repo: issue.Repo, Number: 6}, "issue", "5", ""},
312+ {domain.IssueReferenced{Repo: issue.Repo, Issue: 5, MergeRequest: 2}, "issue", "5", "a merge request mentions this issue"},
313+ {domain.IssueReferenced{Repo: issue.Repo, Issue: 6, MergeRequest: 2}, "issue", "5", ""},
312 {domain.IssueClosed{Issue: issue}, "mrs", "", ""},314 {domain.IssueClosed{Issue: issue}, "mrs", "", ""},
313 {domain.IssueClosed{Issue: issue}, "summary", "", ""},315 {domain.IssueClosed{Issue: issue}, "summary", "", ""},
314 }316 }
@@ -330,3 +332,46 @@ func TestIssueLiveNotices(t *testing.T) {
330 require.NoError(t, err)332 require.NoError(t, err)
331 assert.Empty(t, got)333 assert.Empty(t, got)
332 }334 }
335+
336+func TestIssueShowReferences(t *testing.T) {
337+ t.Parallel()
338+ r := newIssueRig(t, domain.IssuesPublic, true)
339+ title, err := domain.ParseMRTitle("fix <b>it</b>")
340+ require.NoError(t, err)
341+ issue := testIssue(t, 5, domain.IssueStateClosed, "bug", "see #7 and #8")
342+ issue.ClosedByMergeRequest = 2
343+ r.issues.EXPECT().Get(mock.Anything, mock.Anything, repoName(t, "proj"), int64(5)).Return(ports.IssueDetail{
344+ Issue: issue,
345+ References: []ports.IssueReference{
346+ {Number: 2, Title: title, State: domain.MRStateMerged, Closes: true},
347+ {Number: 3, Title: title, State: domain.MRStateOpen},
348+ },
349+ }, nil)
350+ r.nums.EXPECT().Kind(mock.Anything, mock.Anything, repoName(t, "proj"), int64(7)).Return(domain.NumberMergeRequest, nil)
351+ r.nums.EXPECT().Kind(mock.Anything, mock.Anything, repoName(t, "proj"), int64(8)).Return(domain.NumberNone, nil)
352+
353+ rec := r.do("GET", "/proj/issues/5", false, nil)
354+ require.Equal(t, 200, rec.Code, rec.Body.String())
355+ body := rec.Body.String()
356+ assert.Contains(t, body, "closed by")
357+ assert.Contains(t, body, `href="/proj/merge-requests/2"`)
358+ assert.Contains(t, body, ">!2<")
359+ assert.Contains(t, body, ">!3<")
360+ assert.Contains(t, body, `id="r-refs"`)
361+ assert.Contains(t, body, "closes")
362+ assert.Contains(t, body, "fix &lt;b&gt;it&lt;/b&gt;")
363+ assert.Contains(t, body, `href="/proj/merge-requests/7"`, "#7 in the body")
364+ assert.Contains(t, body, `title="merge request 7"`)
365+ assert.NotContains(t, body, `/issues/8"`)
366+}
367+
368+func TestIssueShowWithoutReferences(t *testing.T) {
369+ t.Parallel()
370+ r := newIssueRig(t, domain.IssuesPublic, false)
371+ r.issues.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, int64(5)).Return(ports.IssueDetail{
372+ Issue: testIssue(t, 5, domain.IssueStateOpen, "bug", ""),
373+ }, nil)
374+ body := r.do("GET", "/proj/issues/5", false, nil).Body.String()
375+ assert.NotContains(t, body, "r-refs")
376+ assert.NotContains(t, body, "closed by")
377+}
M internal/adapters/web/markdown.go +72 -23
@@ -64,15 +64,32 @@ func (m *markdown) render(source []byte, repo, ref, dir string) (template.HTML,
6464 // #fragment links chosen, so that several documents on one page (the summaries
6565 // of a run) do not share ids.
6666 func (m *markdown) renderWithIDs(source []byte, repo, ref, dir, idPrefix string) (template.HTML, []Heading, error) {
67- return m.renderPolicy(source, linkPolicy{repo: repo, ref: ref, dir: dir, idPrefix: idPrefix}, nil)
67+ return m.renderPolicy(source, linkPolicy{repo: repo, ref: ref, dir: dir, idPrefix: idPrefix}, refResolver{})
6868 }
6969
70-// renderNote uses the repository Markdown policy, then adds local note links.
71-func (m *markdown) renderNote(source []byte, repo, ref, idPrefix string, notes func(int64) bool) (template.HTML, error) {
72- h, _, err := m.renderPolicy(source, linkPolicy{repo: repo, ref: ref, idPrefix: idPrefix}, notes)
70+// renderNote uses the repository Markdown policy, then adds local note links
71+// and links to issues and merge requests.
72+func (m *markdown) renderNote(source []byte, repo, ref, idPrefix string, refs refResolver) (template.HTML, error) {
73+ h, _, err := m.renderPolicy(source, linkPolicy{repo: repo, ref: ref, idPrefix: idPrefix}, refs)
7374 return h, err
7475 }
7576
77+// renderIssue renders the trusted body of an issue: the policy of render, plus
78+// links to issues and merge requests.
79+func (m *markdown) renderIssue(source []byte, repo, ref, idPrefix string, numbers func(int64) (string, bool)) (template.HTML, error) {
80+ h, _, err := m.renderPolicy(source, linkPolicy{repo: repo, ref: ref, idPrefix: idPrefix}, refResolver{numbers: numbers})
81+ return h, err
82+}
83+
84+// refResolver tells which references of a text are links.
85+type refResolver struct {
86+ // notes tells whether ^N names a note of the page (nil: no ^N links).
87+ notes func(int64) bool
88+ // numbers gives the page of the issue or merge request #N, when it exists
89+ // (nil: no #N links). The href is built by the server, never from the text.
90+ numbers func(int64) (href string, ok bool)
91+}
92+
7693 // renderUntrusted renders Markdown written by an anonymous visitor (an issue
7794 // approved from the public form). On top of the policy of render, which already
7895 // drops raw HTML and never loads an image:
@@ -80,19 +97,19 @@ func (m *markdown) renderNote(source []byte, repo, ref, idPrefix string, notes f
8097 // rel="nofollow noopener noreferrer ugc"; relative links, #fragments and
8198 // every other scheme lose their link and keep the text;
8299 // - nothing points into the repository.
83-func (m *markdown) renderUntrusted(source []byte, idPrefix string) (template.HTML, error) {
84- h, _, err := m.renderPolicy(source, linkPolicy{idPrefix: idPrefix, untrusted: true}, nil)
100+func (m *markdown) renderUntrusted(source []byte, idPrefix string, numbers func(int64) (string, bool)) (template.HTML, error) {
101+ h, _, err := m.renderPolicy(source, linkPolicy{idPrefix: idPrefix, untrusted: true}, refResolver{numbers: numbers})
85102 return h, err
86103 }
87104
88-func (m *markdown) renderPolicy(source []byte, pol linkPolicy, notes func(int64) bool) (template.HTML, []Heading, error) {
105+func (m *markdown) renderPolicy(source []byte, pol linkPolicy, res refResolver) (template.HTML, []Heading, error) {
89106 pc := parser.NewContext(parser.WithIDs(&prefixedIDs{prefix: pol.idPrefix, inner: parser.NewContext().IDs()}))
90107 doc := m.md.Parser().Parse(text.NewReader(source), parser.WithContext(pc))
91108 pol.source = source
92109 // Collect eligible text before the policy unwraps rejected links: their
93110 // labels must not accidentally become references either.
94111 var refs []*ast.Text
95- if notes != nil {
112+ if res.notes != nil || res.numbers != nil {
96113 _ = ast.Walk(doc, func(n ast.Node, entering bool) (ast.WalkStatus, error) {
97114 if !entering {
98115 return ast.WalkContinue, nil
@@ -108,7 +125,7 @@ func (m *markdown) renderPolicy(source []byte, pol linkPolicy, notes func(int64)
108125 }
109126 pol.apply(doc)
110127 for _, t := range refs {
111- noteReferences(t, source, notes)
128+ references(t, source, res)
112129 }
113130
114131 var buf bytes.Buffer
@@ -390,42 +407,74 @@ func (altRenderer) RegisterFuncs(r renderer.NodeRendererFuncRegisterer) {
390407 })
391408 }
392409
393-var noteReferencePattern = regexp.MustCompile(`(^|[^0-9A-Za-z_^])\^([1-9][0-9]{0,5})\b`)
410+// referencePattern matches ^N (groups 1-2) and #N (groups 3-4). The preceding
411+// character is not part of a word; #N also rejects & and # so that numeric
412+// entities such as &#39; stay text.
413+var referencePattern = regexp.MustCompile(`(^|[^0-9A-Za-z_^])\^([1-9][0-9]{0,5})\b|(^|[^0-9A-Za-z_&#])#([1-9][0-9]{0,9})\b`)
394414
395-// noteReferences changes only text segments; code and link labels never enter
396-// this function. New fragment links deliberately bypass heading-prefix rewriting.
397-func noteReferences(t *ast.Text, source []byte, exists func(int64) bool) {
415+// references changes only text segments; code and link labels never enter
416+// this function. New links deliberately bypass heading-prefix rewriting.
417+func references(t *ast.Text, source []byte, res refResolver) {
398418 parent := t.Parent()
399419 if parent == nil {
400420 return
401421 }
402422 value := t.Value(source)
403423 offset := 0
404- for _, match := range noteReferencePattern.FindAllSubmatchIndex(value, -1) {
405- id := string(value[match[4]:match[5]])
424+ for _, match := range referencePattern.FindAllSubmatchIndex(value, -1) {
425+ isNote := match[4] >= 0
426+ var from, to int
427+ if isNote {
428+ from, to = match[4], match[5]
429+ } else {
430+ from, to = match[8], match[9]
431+ }
432+ if (isNote && res.notes == nil) || (!isNote && res.numbers == nil) {
433+ continue
434+ }
435+ id := string(value[from:to])
406436 n, err := strconv.ParseInt(id, 10, 64)
407- if err != nil || !exists(n) {
437+ if err != nil {
408438 continue
409439 }
410- start := match[4] - 1
440+ start := from - 1
411441 // Markdown punctuation may split adjacent text into separate AST nodes.
412442 // Check the original preceding byte as well as the segment-local regex.
413443 if at := t.Segment.Start + start; at > 0 {
414444 prev := source[at-1]
415- if prev >= '0' && prev <= '9' || prev >= 'A' && prev <= 'Z' || prev >= 'a' && prev <= 'z' || prev == '_' || prev == '^' {
445+ if prev >= '0' && prev <= '9' || prev >= 'A' && prev <= 'Z' || prev >= 'a' && prev <= 'z' || prev == '_' {
446+ continue
447+ }
448+ if isNote && prev == '^' || !isNote && (prev == '&' || prev == '#') {
416449 continue
417450 }
418451 }
452+ link := ast.NewLink()
453+ if isNote {
454+ if !res.notes(n) {
455+ continue
456+ }
457+ link.Destination = []byte("#c" + id)
458+ link.Title = []byte("note " + id)
459+ } else {
460+ href, ok := res.numbers(n)
461+ if !ok {
462+ continue
463+ }
464+ link.Destination = []byte(href)
465+ title := "issue "
466+ if strings.Contains(href, "/merge-requests/") {
467+ title = "merge request "
468+ }
469+ link.Title = []byte(title + id)
470+ }
419471 if start > offset {
420472 parent.InsertBefore(parent, t, ast.NewTextSegment(text.NewSegment(t.Segment.Start+offset, t.Segment.Start+start)))
421473 }
422- link := ast.NewLink()
423- link.Destination = []byte("#c" + id)
424- link.Title = []byte("note " + id)
425474 link.SetAttributeString("class", []byte("ref"))
426- link.AppendChild(link, ast.NewString(value[start:match[5]]))
475+ link.AppendChild(link, ast.NewString(value[start:to]))
427476 parent.InsertBefore(parent, t, link)
428- offset = match[5]
477+ offset = to
429478 }
430479 if offset == 0 {
431480 return
@@ -64,15 +64,32 @@ func (m *markdown) render(source []byte, repo, ref, dir string) (template.HTML,
64 // #fragment links chosen, so that several documents on one page (the summaries64 // #fragment links chosen, so that several documents on one page (the summaries
65 // of a run) do not share ids.65 // of a run) do not share ids.
66 func (m *markdown) renderWithIDs(source []byte, repo, ref, dir, idPrefix string) (template.HTML, []Heading, error) {66 func (m *markdown) renderWithIDs(source []byte, repo, ref, dir, idPrefix string) (template.HTML, []Heading, error) {
67- return m.renderPolicy(source, linkPolicy{repo: repo, ref: ref, dir: dir, idPrefix: idPrefix}, nil)67+ return m.renderPolicy(source, linkPolicy{repo: repo, ref: ref, dir: dir, idPrefix: idPrefix}, refResolver{})
68 }68 }
69 69
70-// renderNote uses the repository Markdown policy, then adds local note links.70+// renderNote uses the repository Markdown policy, then adds local note links
71-func (m *markdown) renderNote(source []byte, repo, ref, idPrefix string, notes func(int64) bool) (template.HTML, error) {71+// and links to issues and merge requests.
72- h, _, err := m.renderPolicy(source, linkPolicy{repo: repo, ref: ref, idPrefix: idPrefix}, notes)72+func (m *markdown) renderNote(source []byte, repo, ref, idPrefix string, refs refResolver) (template.HTML, error) {
73+ h, _, err := m.renderPolicy(source, linkPolicy{repo: repo, ref: ref, idPrefix: idPrefix}, refs)
73 return h, err74 return h, err
74 }75 }
75 76
77+// renderIssue renders the trusted body of an issue: the policy of render, plus
78+// links to issues and merge requests.
79+func (m *markdown) renderIssue(source []byte, repo, ref, idPrefix string, numbers func(int64) (string, bool)) (template.HTML, error) {
80+ h, _, err := m.renderPolicy(source, linkPolicy{repo: repo, ref: ref, idPrefix: idPrefix}, refResolver{numbers: numbers})
81+ return h, err
82+}
83+
84+// refResolver tells which references of a text are links.
85+type refResolver struct {
86+ // notes tells whether ^N names a note of the page (nil: no ^N links).
87+ notes func(int64) bool
88+ // numbers gives the page of the issue or merge request #N, when it exists
89+ // (nil: no #N links). The href is built by the server, never from the text.
90+ numbers func(int64) (href string, ok bool)
91+}
92+
76 // renderUntrusted renders Markdown written by an anonymous visitor (an issue93 // renderUntrusted renders Markdown written by an anonymous visitor (an issue
77 // approved from the public form). On top of the policy of render, which already94 // approved from the public form). On top of the policy of render, which already
78 // drops raw HTML and never loads an image:95 // drops raw HTML and never loads an image:
@@ -80,19 +97,19 @@ func (m *markdown) renderNote(source []byte, repo, ref, idPrefix string, notes f
80 // rel="nofollow noopener noreferrer ugc"; relative links, #fragments and97 // rel="nofollow noopener noreferrer ugc"; relative links, #fragments and
81 // every other scheme lose their link and keep the text;98 // every other scheme lose their link and keep the text;
82 // - nothing points into the repository.99 // - nothing points into the repository.
83-func (m *markdown) renderUntrusted(source []byte, idPrefix string) (template.HTML, error) {100+func (m *markdown) renderUntrusted(source []byte, idPrefix string, numbers func(int64) (string, bool)) (template.HTML, error) {
84- h, _, err := m.renderPolicy(source, linkPolicy{idPrefix: idPrefix, untrusted: true}, nil)101+ h, _, err := m.renderPolicy(source, linkPolicy{idPrefix: idPrefix, untrusted: true}, refResolver{numbers: numbers})
85 return h, err102 return h, err
86 }103 }
87 104
88-func (m *markdown) renderPolicy(source []byte, pol linkPolicy, notes func(int64) bool) (template.HTML, []Heading, error) {105+func (m *markdown) renderPolicy(source []byte, pol linkPolicy, res refResolver) (template.HTML, []Heading, error) {
89 pc := parser.NewContext(parser.WithIDs(&prefixedIDs{prefix: pol.idPrefix, inner: parser.NewContext().IDs()}))106 pc := parser.NewContext(parser.WithIDs(&prefixedIDs{prefix: pol.idPrefix, inner: parser.NewContext().IDs()}))
90 doc := m.md.Parser().Parse(text.NewReader(source), parser.WithContext(pc))107 doc := m.md.Parser().Parse(text.NewReader(source), parser.WithContext(pc))
91 pol.source = source108 pol.source = source
92 // Collect eligible text before the policy unwraps rejected links: their109 // Collect eligible text before the policy unwraps rejected links: their
93 // labels must not accidentally become references either.110 // labels must not accidentally become references either.
94 var refs []*ast.Text111 var refs []*ast.Text
95- if notes != nil {112+ if res.notes != nil || res.numbers != nil {
96 _ = ast.Walk(doc, func(n ast.Node, entering bool) (ast.WalkStatus, error) {113 _ = ast.Walk(doc, func(n ast.Node, entering bool) (ast.WalkStatus, error) {
97 if !entering {114 if !entering {
98 return ast.WalkContinue, nil115 return ast.WalkContinue, nil
@@ -108,7 +125,7 @@ func (m *markdown) renderPolicy(source []byte, pol linkPolicy, notes func(int64)
108 }125 }
109 pol.apply(doc)126 pol.apply(doc)
110 for _, t := range refs {127 for _, t := range refs {
111- noteReferences(t, source, notes)128+ references(t, source, res)
112 }129 }
113 130
114 var buf bytes.Buffer131 var buf bytes.Buffer
@@ -390,42 +407,74 @@ func (altRenderer) RegisterFuncs(r renderer.NodeRendererFuncRegisterer) {
390 })407 })
391 }408 }
392 409
393-var noteReferencePattern = regexp.MustCompile(`(^|[^0-9A-Za-z_^])\^([1-9][0-9]{0,5})\b`)410+// referencePattern matches ^N (groups 1-2) and #N (groups 3-4). The preceding
411+// character is not part of a word; #N also rejects & and # so that numeric
412+// entities such as &#39; stay text.
413+var referencePattern = regexp.MustCompile(`(^|[^0-9A-Za-z_^])\^([1-9][0-9]{0,5})\b|(^|[^0-9A-Za-z_&#])#([1-9][0-9]{0,9})\b`)
394 414
395-// noteReferences changes only text segments; code and link labels never enter415+// references changes only text segments; code and link labels never enter
396-// this function. New fragment links deliberately bypass heading-prefix rewriting.416+// this function. New links deliberately bypass heading-prefix rewriting.
397-func noteReferences(t *ast.Text, source []byte, exists func(int64) bool) {417+func references(t *ast.Text, source []byte, res refResolver) {
398 parent := t.Parent()418 parent := t.Parent()
399 if parent == nil {419 if parent == nil {
400 return420 return
401 }421 }
402 value := t.Value(source)422 value := t.Value(source)
403 offset := 0423 offset := 0
404- for _, match := range noteReferencePattern.FindAllSubmatchIndex(value, -1) {424+ for _, match := range referencePattern.FindAllSubmatchIndex(value, -1) {
405- id := string(value[match[4]:match[5]])425+ isNote := match[4] >= 0
426+ var from, to int
427+ if isNote {
428+ from, to = match[4], match[5]
429+ } else {
430+ from, to = match[8], match[9]
431+ }
432+ if (isNote && res.notes == nil) || (!isNote && res.numbers == nil) {
433+ continue
434+ }
435+ id := string(value[from:to])
406 n, err := strconv.ParseInt(id, 10, 64)436 n, err := strconv.ParseInt(id, 10, 64)
407- if err != nil || !exists(n) {437+ if err != nil {
408 continue438 continue
409 }439 }
410- start := match[4] - 1440+ start := from - 1
411 // Markdown punctuation may split adjacent text into separate AST nodes.441 // Markdown punctuation may split adjacent text into separate AST nodes.
412 // Check the original preceding byte as well as the segment-local regex.442 // Check the original preceding byte as well as the segment-local regex.
413 if at := t.Segment.Start + start; at > 0 {443 if at := t.Segment.Start + start; at > 0 {
414 prev := source[at-1]444 prev := source[at-1]
415- if prev >= '0' && prev <= '9' || prev >= 'A' && prev <= 'Z' || prev >= 'a' && prev <= 'z' || prev == '_' || prev == '^' {445+ if prev >= '0' && prev <= '9' || prev >= 'A' && prev <= 'Z' || prev >= 'a' && prev <= 'z' || prev == '_' {
446+ continue
447+ }
448+ if isNote && prev == '^' || !isNote && (prev == '&' || prev == '#') {
416 continue449 continue
417 }450 }
418 }451 }
452+ link := ast.NewLink()
453+ if isNote {
454+ if !res.notes(n) {
455+ continue
456+ }
457+ link.Destination = []byte("#c" + id)
458+ link.Title = []byte("note " + id)
459+ } else {
460+ href, ok := res.numbers(n)
461+ if !ok {
462+ continue
463+ }
464+ link.Destination = []byte(href)
465+ title := "issue "
466+ if strings.Contains(href, "/merge-requests/") {
467+ title = "merge request "
468+ }
469+ link.Title = []byte(title + id)
470+ }
419 if start > offset {471 if start > offset {
420 parent.InsertBefore(parent, t, ast.NewTextSegment(text.NewSegment(t.Segment.Start+offset, t.Segment.Start+start)))472 parent.InsertBefore(parent, t, ast.NewTextSegment(text.NewSegment(t.Segment.Start+offset, t.Segment.Start+start)))
421 }473 }
422- link := ast.NewLink()
423- link.Destination = []byte("#c" + id)
424- link.Title = []byte("note " + id)
425 link.SetAttributeString("class", []byte("ref"))474 link.SetAttributeString("class", []byte("ref"))
426- link.AppendChild(link, ast.NewString(value[start:match[5]]))475+ link.AppendChild(link, ast.NewString(value[start:to]))
427 parent.InsertBefore(parent, t, link)476 parent.InsertBefore(parent, t, link)
428- offset = match[5]477+ offset = to
429 }478 }
430 if offset == 0 {479 if offset == 0 {
431 return480 return
M internal/adapters/web/markdown_test.go +79 -3
@@ -6,7 +6,10 @@ import (
66 "strings"
77 "testing"
88
9+ "bnema.dev/ermite/internal/domain"
10+ "bnema.dev/ermite/internal/ports/mocks"
911 "github.com/stretchr/testify/assert"
12+ "github.com/stretchr/testify/mock"
1013 "github.com/stretchr/testify/require"
1114 )
1215
@@ -139,7 +142,7 @@ func TestMarkdownGFM(t *testing.T) {
139142
140143 func renderUntrustedMD(t *testing.T, src string) string {
141144 t.Helper()
142- h, err := newMarkdown().renderUntrusted([]byte(src), "issue-desc-")
145+ h, err := newMarkdown().renderUntrusted([]byte(src), "issue-desc-", nil)
143146 require.NoError(t, err)
144147 return string(h)
145148 }
@@ -218,17 +221,90 @@ func TestNoteMarkdown(t *testing.T) {
218221 for _, tt := range tests {
219222 t.Run(tt.source, func(t *testing.T) {
220223 t.Parallel()
221- h, err := newMarkdown().renderNote([]byte(tt.source), "demo", "main", "c1-", func(n int64) bool { return n == 2 })
224+ h, err := newMarkdown().renderNote([]byte(tt.source), "demo", "main", "c1-", refResolver{notes: func(n int64) bool { return n == 2 }})
222225 require.NoError(t, err)
223226 assert.Equal(t, tt.links, strings.Count(string(h), `href="#c2"`), string(h))
224227 assert.NotContains(t, string(h), `href="#c99"`)
225228 assert.NotContains(t, string(h), "javascript:")
226229 })
227230 }
228- h, err := newMarkdown().renderNote([]byte("# Heading\n\n<script>alert(1)</script>\n\n**bold** ^2"), "demo", "main", "c1-", func(n int64) bool { return n == 2 })
231+ h, err := newMarkdown().renderNote([]byte("# Heading\n\n<script>alert(1)</script>\n\n**bold** ^2"), "demo", "main", "c1-", refResolver{notes: func(n int64) bool { return n == 2 }})
229232 require.NoError(t, err)
230233 assert.Contains(t, string(h), `id="c1-heading"`)
231234 assert.Contains(t, string(h), "<strong>bold</strong>")
232235 assert.NotContains(t, string(h), "<script")
233236 assert.Contains(t, string(h), `title="note 2"`)
234237 }
238+
239+func TestNumberReferences(t *testing.T) {
240+ t.Parallel()
241+ numbers := func(n int64) (string, bool) {
242+ switch n {
243+ case 3:
244+ return "/demo/issues/3", true
245+ case 4:
246+ return "/demo/merge-requests/4", true
247+ }
248+ return "", false
249+ }
250+ tests := []struct {
251+ source string
252+ want []string
253+ not []string
254+ }{
255+ {"fix #3 and #4, not #9", []string{`<a href="/demo/issues/3" title="issue 3" class="ref">#3</a>`, `title="merge request 4"`}, []string{"/issues/9", "#9</a>"}},
256+ {"abc#3 _#3 ##3 1#3", nil, []string{"<a "}},
257+ {"&#39; &#3;", nil, []string{"<a "}},
258+ {"`#3`\n\n```\n#3\n```", nil, []string{"<a "}},
259+ {"[#3](https://example.org)", []string{`href="https://example.org"`}, []string{"/demo/issues/3"}},
260+ {"#3\n#3", []string{`href="/demo/issues/3"`}, nil},
261+ {"#03 #12345678901", nil, []string{"<a "}},
262+ }
263+ for _, tt := range tests {
264+ t.Run(tt.source, func(t *testing.T) {
265+ t.Parallel()
266+ h, err := newMarkdown().renderIssue([]byte(tt.source), "demo", "main", "i-", numbers)
267+ require.NoError(t, err)
268+ for _, w := range tt.want {
269+ assert.Contains(t, string(h), w)
270+ }
271+ for _, w := range tt.not {
272+ assert.NotContains(t, string(h), w)
273+ }
274+ u, err := newMarkdown().renderUntrusted([]byte(tt.source), "i-", numbers)
275+ require.NoError(t, err)
276+ assert.Equal(t, strings.Count(string(h), `class="ref"`), strings.Count(string(u), `class="ref"`))
277+ })
278+ }
279+ // Without a resolver nothing links.
280+ h, err := newMarkdown().renderUntrusted([]byte("#3"), "i-", nil)
281+ require.NoError(t, err)
282+ assert.NotContains(t, string(h), "<a ")
283+}
284+
285+func TestNumberAndNoteReferencesTogether(t *testing.T) {
286+ t.Parallel()
287+ h, err := newMarkdown().renderNote([]byte("see ^2 and #3."), "demo", "main", "c1-", refResolver{
288+ notes: func(n int64) bool { return n == 2 },
289+ numbers: func(n int64) (string, bool) { return "/demo/issues/3", n == 3 },
290+ })
291+ require.NoError(t, err)
292+ assert.Contains(t, string(h), `href="#c2"`)
293+ assert.Contains(t, string(h), `href="/demo/issues/3"`)
294+ assert.Contains(t, string(h), "and ")
295+}
296+
297+func TestNumberResolverCapsLookups(t *testing.T) {
298+ t.Parallel()
299+ nums := mocks.NewIssueNumberResolver(t)
300+ nums.EXPECT().Kind(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(domain.NumberIssue, nil).Times(maxNumberLookups)
301+ srv := &Server{cfg: Config{IssueNumbers: nums}}
302+ resolve := srv.numberResolver(t.Context(), "demo")
303+ require.NotNil(t, resolve)
304+ for n := int64(1); n <= maxNumberLookups+10; n++ {
305+ _, ok := resolve(n)
306+ assert.Equal(t, n <= maxNumberLookups, ok, n)
307+ }
308+ _, ok := resolve(1) // memoized
309+ assert.True(t, ok)
310+}
@@ -6,7 +6,10 @@ import (
6 "strings"6 "strings"
7 "testing"7 "testing"
8 8
9+ "bnema.dev/ermite/internal/domain"
10+ "bnema.dev/ermite/internal/ports/mocks"
9 "github.com/stretchr/testify/assert"11 "github.com/stretchr/testify/assert"
12+ "github.com/stretchr/testify/mock"
10 "github.com/stretchr/testify/require"13 "github.com/stretchr/testify/require"
11 )14 )
12 15
@@ -139,7 +142,7 @@ func TestMarkdownGFM(t *testing.T) {
139 142
140 func renderUntrustedMD(t *testing.T, src string) string {143 func renderUntrustedMD(t *testing.T, src string) string {
141 t.Helper()144 t.Helper()
142- h, err := newMarkdown().renderUntrusted([]byte(src), "issue-desc-")145+ h, err := newMarkdown().renderUntrusted([]byte(src), "issue-desc-", nil)
143 require.NoError(t, err)146 require.NoError(t, err)
144 return string(h)147 return string(h)
145 }148 }
@@ -218,17 +221,90 @@ func TestNoteMarkdown(t *testing.T) {
218 for _, tt := range tests {221 for _, tt := range tests {
219 t.Run(tt.source, func(t *testing.T) {222 t.Run(tt.source, func(t *testing.T) {
220 t.Parallel()223 t.Parallel()
221- h, err := newMarkdown().renderNote([]byte(tt.source), "demo", "main", "c1-", func(n int64) bool { return n == 2 })224+ h, err := newMarkdown().renderNote([]byte(tt.source), "demo", "main", "c1-", refResolver{notes: func(n int64) bool { return n == 2 }})
222 require.NoError(t, err)225 require.NoError(t, err)
223 assert.Equal(t, tt.links, strings.Count(string(h), `href="#c2"`), string(h))226 assert.Equal(t, tt.links, strings.Count(string(h), `href="#c2"`), string(h))
224 assert.NotContains(t, string(h), `href="#c99"`)227 assert.NotContains(t, string(h), `href="#c99"`)
225 assert.NotContains(t, string(h), "javascript:")228 assert.NotContains(t, string(h), "javascript:")
226 })229 })
227 }230 }
228- h, err := newMarkdown().renderNote([]byte("# Heading\n\n<script>alert(1)</script>\n\n**bold** ^2"), "demo", "main", "c1-", func(n int64) bool { return n == 2 })231+ h, err := newMarkdown().renderNote([]byte("# Heading\n\n<script>alert(1)</script>\n\n**bold** ^2"), "demo", "main", "c1-", refResolver{notes: func(n int64) bool { return n == 2 }})
229 require.NoError(t, err)232 require.NoError(t, err)
230 assert.Contains(t, string(h), `id="c1-heading"`)233 assert.Contains(t, string(h), `id="c1-heading"`)
231 assert.Contains(t, string(h), "<strong>bold</strong>")234 assert.Contains(t, string(h), "<strong>bold</strong>")
232 assert.NotContains(t, string(h), "<script")235 assert.NotContains(t, string(h), "<script")
233 assert.Contains(t, string(h), `title="note 2"`)236 assert.Contains(t, string(h), `title="note 2"`)
234 }237 }
238+
239+func TestNumberReferences(t *testing.T) {
240+ t.Parallel()
241+ numbers := func(n int64) (string, bool) {
242+ switch n {
243+ case 3:
244+ return "/demo/issues/3", true
245+ case 4:
246+ return "/demo/merge-requests/4", true
247+ }
248+ return "", false
249+ }
250+ tests := []struct {
251+ source string
252+ want []string
253+ not []string
254+ }{
255+ {"fix #3 and #4, not #9", []string{`<a href="/demo/issues/3" title="issue 3" class="ref">#3</a>`, `title="merge request 4"`}, []string{"/issues/9", "#9</a>"}},
256+ {"abc#3 _#3 ##3 1#3", nil, []string{"<a "}},
257+ {"&#39; &#3;", nil, []string{"<a "}},
258+ {"`#3`\n\n```\n#3\n```", nil, []string{"<a "}},
259+ {"[#3](https://example.org)", []string{`href="https://example.org"`}, []string{"/demo/issues/3"}},
260+ {"#3\n#3", []string{`href="/demo/issues/3"`}, nil},
261+ {"#03 #12345678901", nil, []string{"<a "}},
262+ }
263+ for _, tt := range tests {
264+ t.Run(tt.source, func(t *testing.T) {
265+ t.Parallel()
266+ h, err := newMarkdown().renderIssue([]byte(tt.source), "demo", "main", "i-", numbers)
267+ require.NoError(t, err)
268+ for _, w := range tt.want {
269+ assert.Contains(t, string(h), w)
270+ }
271+ for _, w := range tt.not {
272+ assert.NotContains(t, string(h), w)
273+ }
274+ u, err := newMarkdown().renderUntrusted([]byte(tt.source), "i-", numbers)
275+ require.NoError(t, err)
276+ assert.Equal(t, strings.Count(string(h), `class="ref"`), strings.Count(string(u), `class="ref"`))
277+ })
278+ }
279+ // Without a resolver nothing links.
280+ h, err := newMarkdown().renderUntrusted([]byte("#3"), "i-", nil)
281+ require.NoError(t, err)
282+ assert.NotContains(t, string(h), "<a ")
283+}
284+
285+func TestNumberAndNoteReferencesTogether(t *testing.T) {
286+ t.Parallel()
287+ h, err := newMarkdown().renderNote([]byte("see ^2 and #3."), "demo", "main", "c1-", refResolver{
288+ notes: func(n int64) bool { return n == 2 },
289+ numbers: func(n int64) (string, bool) { return "/demo/issues/3", n == 3 },
290+ })
291+ require.NoError(t, err)
292+ assert.Contains(t, string(h), `href="#c2"`)
293+ assert.Contains(t, string(h), `href="/demo/issues/3"`)
294+ assert.Contains(t, string(h), "and ")
295+}
296+
297+func TestNumberResolverCapsLookups(t *testing.T) {
298+ t.Parallel()
299+ nums := mocks.NewIssueNumberResolver(t)
300+ nums.EXPECT().Kind(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(domain.NumberIssue, nil).Times(maxNumberLookups)
301+ srv := &Server{cfg: Config{IssueNumbers: nums}}
302+ resolve := srv.numberResolver(t.Context(), "demo")
303+ require.NotNil(t, resolve)
304+ for n := int64(1); n <= maxNumberLookups+10; n++ {
305+ _, ok := resolve(n)
306+ assert.Equal(t, n <= maxNumberLookups, ok, n)
307+ }
308+ _, ok := resolve(1) // memoized
309+ assert.True(t, ok)
310+}
M internal/adapters/web/mr.go +4 -4
@@ -616,7 +616,7 @@ func (s *Server) mrPage(ctx context.Context, r *http.Request, since bool) (templ
616616 if canWrite {
617617 on = parseOn(r.URL.Query().Get("on"))
618618 }
619- s.placeComments(&p, d.name(), n, res.Comments, head, !since, canWrite, on)
619+ s.placeComments(ctx, &p, d.name(), n, res.Comments, head, !since, canWrite, on)
620620 lines, invisible := diffLines(p.Files)
621621 p.Invisible = invisible
622622 p.DiffView = newDiffView(r, lines, withFrom(self+"?view=unified", from), withFrom(self+"?view=split", from))
@@ -680,7 +680,7 @@ func (s *Server) mrFacts(ctx context.Context, r *http.Request, d *repoData, p *M
680680 // Relative links point into the source branch as it is now (its head
681681 // commit); the heading ids carry their own prefix, apart from the
682682 // ids of the diff.
683- h, err := s.md.renderNote([]byte(desc), d.name(), head, "mr-desc-", noteIDs(res.Comments))
683+ h, err := s.md.renderNote([]byte(desc), d.name(), head, "mr-desc-", refResolver{notes: noteIDs(res.Comments), numbers: s.numberResolver(ctx, d.name())})
684684 if err != nil {
685685 return err
686686 }
@@ -905,7 +905,7 @@ func parseOn(s string) *lineRef {
905905 // as links to the form of their line; on designates the line whose form shows.
906906 //
907907 //hexcheck:ignore suspicious-business-logic-in-adapter places comments under the lines of the diff shown; display only
908-func (s *Server) placeComments(p *MRPage, repo string, n int64, comments []domain.Comment, head string, inline, canWrite bool, on *lineRef) {
908+func (s *Server) placeComments(ctx context.Context, p *MRPage, repo string, n int64, comments []domain.Comment, head string, inline, canWrite bool, on *lineRef) {
909909 now := s.cfg.Now()
910910 self := mrHref(repo, n)
911911 resolveAction := func(id string) string { return mrHref(repo, n, "comments", id, "resolve") }
@@ -953,7 +953,7 @@ func (s *Server) placeComments(p *MRPage, repo string, n int64, comments []domai
953953 return index[lineKey{path, side, line}]
954954 }
955955
956- exists := noteIDs(comments)
956+ exists := refResolver{notes: noteIDs(comments), numbers: s.numberResolver(ctx, repo)}
957957 children := map[int64][]domain.Comment{}
958958 for _, c := range comments {
959959 if c.ReplyTo != 0 && !c.Deleted {
@@ -616,7 +616,7 @@ func (s *Server) mrPage(ctx context.Context, r *http.Request, since bool) (templ
616 if canWrite {616 if canWrite {
617 on = parseOn(r.URL.Query().Get("on"))617 on = parseOn(r.URL.Query().Get("on"))
618 }618 }
619- s.placeComments(&p, d.name(), n, res.Comments, head, !since, canWrite, on)619+ s.placeComments(ctx, &p, d.name(), n, res.Comments, head, !since, canWrite, on)
620 lines, invisible := diffLines(p.Files)620 lines, invisible := diffLines(p.Files)
621 p.Invisible = invisible621 p.Invisible = invisible
622 p.DiffView = newDiffView(r, lines, withFrom(self+"?view=unified", from), withFrom(self+"?view=split", from))622 p.DiffView = newDiffView(r, lines, withFrom(self+"?view=unified", from), withFrom(self+"?view=split", from))
@@ -680,7 +680,7 @@ func (s *Server) mrFacts(ctx context.Context, r *http.Request, d *repoData, p *M
680 // Relative links point into the source branch as it is now (its head680 // Relative links point into the source branch as it is now (its head
681 // commit); the heading ids carry their own prefix, apart from the681 // commit); the heading ids carry their own prefix, apart from the
682 // ids of the diff.682 // ids of the diff.
683- h, err := s.md.renderNote([]byte(desc), d.name(), head, "mr-desc-", noteIDs(res.Comments))683+ h, err := s.md.renderNote([]byte(desc), d.name(), head, "mr-desc-", refResolver{notes: noteIDs(res.Comments), numbers: s.numberResolver(ctx, d.name())})
684 if err != nil {684 if err != nil {
685 return err685 return err
686 }686 }
@@ -905,7 +905,7 @@ func parseOn(s string) *lineRef {
905 // as links to the form of their line; on designates the line whose form shows.905 // as links to the form of their line; on designates the line whose form shows.
906 //906 //
907 //hexcheck:ignore suspicious-business-logic-in-adapter places comments under the lines of the diff shown; display only907 //hexcheck:ignore suspicious-business-logic-in-adapter places comments under the lines of the diff shown; display only
908-func (s *Server) placeComments(p *MRPage, repo string, n int64, comments []domain.Comment, head string, inline, canWrite bool, on *lineRef) {908+func (s *Server) placeComments(ctx context.Context, p *MRPage, repo string, n int64, comments []domain.Comment, head string, inline, canWrite bool, on *lineRef) {
909 now := s.cfg.Now()909 now := s.cfg.Now()
910 self := mrHref(repo, n)910 self := mrHref(repo, n)
911 resolveAction := func(id string) string { return mrHref(repo, n, "comments", id, "resolve") }911 resolveAction := func(id string) string { return mrHref(repo, n, "comments", id, "resolve") }
@@ -953,7 +953,7 @@ func (s *Server) placeComments(p *MRPage, repo string, n int64, comments []domai
953 return index[lineKey{path, side, line}]953 return index[lineKey{path, side, line}]
954 }954 }
955 955
956- exists := noteIDs(comments)956+ exists := refResolver{notes: noteIDs(comments), numbers: s.numberResolver(ctx, repo)}
957 children := map[int64][]domain.Comment{}957 children := map[int64][]domain.Comment{}
958 for _, c := range comments {958 for _, c := range comments {
959 if c.ReplyTo != 0 && !c.Deleted {959 if c.ReplyTo != 0 && !c.Deleted {
M internal/adapters/web/mr_files.go +1 -1
@@ -227,7 +227,7 @@ func (s *Server) mrFiles(ctx context.Context, r *http.Request) (templ.Component,
227227 } else {
228228 collapseFiles(p.Files, res.Diff.Files, links)
229229 }
230- s.placeComments(&p, d.name(), n, res.Comments, m.HeadID.String(), !since, canWrite, nil)
230+ s.placeComments(ctx, &p, d.name(), n, res.Comments, m.HeadID.String(), !since, canWrite, nil)
231231 window := p.Files[from:min(from+mrWindow, len(p.Files))]
232232 if one {
233233 window = p.Files[from : from+1]
@@ -227,7 +227,7 @@ func (s *Server) mrFiles(ctx context.Context, r *http.Request) (templ.Component,
227 } else {227 } else {
228 collapseFiles(p.Files, res.Diff.Files, links)228 collapseFiles(p.Files, res.Diff.Files, links)
229 }229 }
230- s.placeComments(&p, d.name(), n, res.Comments, m.HeadID.String(), !since, canWrite, nil)230+ s.placeComments(ctx, &p, d.name(), n, res.Comments, m.HeadID.String(), !since, canWrite, nil)
231 window := p.Files[from:min(from+mrWindow, len(p.Files))]231 window := p.Files[from:min(from+mrWindow, len(p.Files))]
232 if one {232 if one {
233 window = p.Files[from : from+1]233 window = p.Files[from : from+1]
M internal/adapters/web/mr_notes_test.go +3 -3
@@ -43,7 +43,7 @@ func TestNoteThreads(t *testing.T) {
4343 {ID: 6, Author: "alice", Text: text, Verdict: domain.VerdictApprove, Resolved: true},
4444 }
4545 p := MRPage{NoteAuthor: "alice"}
46- s.placeComments(&p, "demo", 1, comments, "", true, true, nil)
46+ s.placeComments(t.Context(), &p, "demo", 1, comments, "", true, true, nil)
4747 require.Len(t, p.General, 3)
4848 assert.Equal(t, 4, p.NComments)
4949 assert.Equal(t, 1, p.NUnresolved)
@@ -117,7 +117,7 @@ func TestDeletedNoteRendering(t *testing.T) {
117117 require.NoError(t, err)
118118 s := &Server{cfg: Config{Now: time.Now}, md: newMarkdown()}
119119 p := MRPage{}
120- s.placeComments(&p, "demo", 1, []domain.Comment{
120+ s.placeComments(t.Context(), &p, "demo", 1, []domain.Comment{
121121 {ID: 1, Author: "reviewer", Deleted: true, Verdict: domain.VerdictApprove},
122122 {ID: 2, Deleted: true},
123123 {ID: 3, ReplyTo: 2, Deleted: true},
@@ -147,7 +147,7 @@ func TestInlineNoteHeadingIDs(t *testing.T) {
147147 require.NoError(t, err)
148148 s := &Server{cfg: Config{Now: time.Now}, md: newMarkdown()}
149149 p := MRPage{Files: []DiffFile{{Key: "a", ID: "f1", Hunks: []DiffHunk{{Lines: []DiffLine{{Kind: "add", newN: 1}}}}}}}
150- s.placeComments(&p, "demo", 1, []domain.Comment{{ID: 12, Path: "a", Side: domain.SideNew, Line: 1, Text: text}}, "", true, true, nil)
150+ s.placeComments(t.Context(), &p, "demo", 1, []domain.Comment{{ID: 12, Path: "a", Side: domain.SideNew, Line: 1, Text: text}}, "", true, true, nil)
151151 notes := p.Files[0].Hunks[0].Lines[0].Notes
152152 require.Len(t, notes, 1)
153153 var b bytes.Buffer
@@ -43,7 +43,7 @@ func TestNoteThreads(t *testing.T) {
43 {ID: 6, Author: "alice", Text: text, Verdict: domain.VerdictApprove, Resolved: true},43 {ID: 6, Author: "alice", Text: text, Verdict: domain.VerdictApprove, Resolved: true},
44 }44 }
45 p := MRPage{NoteAuthor: "alice"}45 p := MRPage{NoteAuthor: "alice"}
46- s.placeComments(&p, "demo", 1, comments, "", true, true, nil)46+ s.placeComments(t.Context(), &p, "demo", 1, comments, "", true, true, nil)
47 require.Len(t, p.General, 3)47 require.Len(t, p.General, 3)
48 assert.Equal(t, 4, p.NComments)48 assert.Equal(t, 4, p.NComments)
49 assert.Equal(t, 1, p.NUnresolved)49 assert.Equal(t, 1, p.NUnresolved)
@@ -117,7 +117,7 @@ func TestDeletedNoteRendering(t *testing.T) {
117 require.NoError(t, err)117 require.NoError(t, err)
118 s := &Server{cfg: Config{Now: time.Now}, md: newMarkdown()}118 s := &Server{cfg: Config{Now: time.Now}, md: newMarkdown()}
119 p := MRPage{}119 p := MRPage{}
120- s.placeComments(&p, "demo", 1, []domain.Comment{120+ s.placeComments(t.Context(), &p, "demo", 1, []domain.Comment{
121 {ID: 1, Author: "reviewer", Deleted: true, Verdict: domain.VerdictApprove},121 {ID: 1, Author: "reviewer", Deleted: true, Verdict: domain.VerdictApprove},
122 {ID: 2, Deleted: true},122 {ID: 2, Deleted: true},
123 {ID: 3, ReplyTo: 2, Deleted: true},123 {ID: 3, ReplyTo: 2, Deleted: true},
@@ -147,7 +147,7 @@ func TestInlineNoteHeadingIDs(t *testing.T) {
147 require.NoError(t, err)147 require.NoError(t, err)
148 s := &Server{cfg: Config{Now: time.Now}, md: newMarkdown()}148 s := &Server{cfg: Config{Now: time.Now}, md: newMarkdown()}
149 p := MRPage{Files: []DiffFile{{Key: "a", ID: "f1", Hunks: []DiffHunk{{Lines: []DiffLine{{Kind: "add", newN: 1}}}}}}}149 p := MRPage{Files: []DiffFile{{Key: "a", ID: "f1", Hunks: []DiffHunk{{Lines: []DiffLine{{Kind: "add", newN: 1}}}}}}}
150- s.placeComments(&p, "demo", 1, []domain.Comment{{ID: 12, Path: "a", Side: domain.SideNew, Line: 1, Text: text}}, "", true, true, nil)150+ s.placeComments(t.Context(), &p, "demo", 1, []domain.Comment{{ID: 12, Path: "a", Side: domain.SideNew, Line: 1, Text: text}}, "", true, true, nil)
151 notes := p.Files[0].Hunks[0].Lines[0].Notes151 notes := p.Files[0].Hunks[0].Lines[0].Notes
152 require.Len(t, notes, 1)152 require.Len(t, notes, 1)
153 var b bytes.Buffer153 var b bytes.Buffer

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 2

Tags 41

↑↓ moveenter openesc close