feat(web): threads, line ranges, Markdown notes, edits and verdicts on merge requests
- commit
- aec37bf928efb0d664dcec3f34f8fe5e39ab8539
- author
- brice · · 2026-10-19 05:47 UTC
- parent
- b31a354
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
18 files · +1311 -527
M internal/adapters/web/diff.go +1 -0
| @@ -30,6 +30,7 @@ type Seg struct { | |||
| 30 | 30 | // DiffLine is one line of a hunk, in the unified view and, paired, in the | |
| 31 | 31 | // split view. | |
| 32 | 32 | type DiffLine struct { | |
| 33 | + | Selected bool | |
| 33 | 34 | Kind string // "ctx", "add", "del" | |
| 34 | 35 | Mark string // " ", "+", "-": every change keeps its marker, not only a colour | |
| 35 | 36 | Old, New string // line numbers, "" when the side has none | |
| @@ -30,6 +30,7 @@ type Seg struct { | |||||
| 30 | // DiffLine is one line of a hunk, in the unified view and, paired, in the | 30 | // DiffLine is one line of a hunk, in the unified view and, paired, in the | ||
| 31 | // split view. | 31 | // split view. | ||
| 32 | type DiffLine struct { | 32 | type DiffLine struct { | ||
| 33 | + | Selected bool | |||
| 33 | Kind string // "ctx", "add", "del" | 34 | Kind string // "ctx", "add", "del" | ||
| 34 | Mark string // " ", "+", "-": every change keeps its marker, not only a colour | 35 | Mark string // " ", "+", "-": every change keeps its marker, not only a colour | ||
| 35 | Old, New string // line numbers, "" when the side has none | 36 | Old, New string // line numbers, "" when the side has none | ||
M internal/adapters/web/diff.templ +4 -4
| @@ -232,7 +232,7 @@ templ DiffFileView(f DiffFile, p DiffView) { | |||
| 232 | 232 | } | |
| 233 | 233 | <tr class="hunk"><td colspan="4">{ h.Header }</td></tr> | |
| 234 | 234 | for _, ln := range h.Lines { | |
| 235 | - | <tr class={ ln.Kind }> | |
| 235 | + | <tr class={ ln.Kind, templ.KV("sel", ln.Selected) }> | |
| 236 | 236 | <td class="ln">@gutter(ln.Old, ln.OldHref)</td> | |
| 237 | 237 | <td class="ln">@gutter(ln.New, ln.NewHref)</td> | |
| 238 | 238 | <td class="m">{ ln.Mark }</td> | |
| @@ -369,15 +369,15 @@ templ splitSide(ln *DiffLine, old bool) { | |||
| 369 | 369 | <td class="m none"></td> | |
| 370 | 370 | <td class="tx none"></td> | |
| 371 | 371 | } else { | |
| 372 | - | <td class={ "ln", ln.Kind }> | |
| 372 | + | <td class={ "ln", ln.Kind, templ.KV("sel", ln.Selected) }> | |
| 373 | 373 | if old { | |
| 374 | 374 | @gutter(ln.Old, ln.OldHref) | |
| 375 | 375 | } else { | |
| 376 | 376 | @gutter(ln.New, ln.NewHref) | |
| 377 | 377 | } | |
| 378 | 378 | </td> | |
| 379 | - | <td class={ "m", ln.Kind }>{ ln.Mark }</td> | |
| 380 | - | <td class={ "tx", "chroma", ln.Kind }>@codeLine(*ln)</td> | |
| 379 | + | <td class={ "m", ln.Kind, templ.KV("sel", ln.Selected) }>{ ln.Mark }</td> | |
| 380 | + | <td class={ "tx", "chroma", ln.Kind, templ.KV("sel", ln.Selected) }>@codeLine(*ln)</td> | |
| 381 | 381 | } | |
| 382 | 382 | } | |
| 383 | 383 | ||
| @@ -232,7 +232,7 @@ templ DiffFileView(f DiffFile, p DiffView) { | |||||
| 232 | } | 232 | } | ||
| 233 | <tr class="hunk"><td colspan="4">{ h.Header }</td></tr> | 233 | <tr class="hunk"><td colspan="4">{ h.Header }</td></tr> | ||
| 234 | for _, ln := range h.Lines { | 234 | for _, ln := range h.Lines { | ||
| 235 | - | <tr class={ ln.Kind }> | 235 | + | <tr class={ ln.Kind, templ.KV("sel", ln.Selected) }> |
| 236 | <td class="ln">@gutter(ln.Old, ln.OldHref)</td> | 236 | <td class="ln">@gutter(ln.Old, ln.OldHref)</td> | ||
| 237 | <td class="ln">@gutter(ln.New, ln.NewHref)</td> | 237 | <td class="ln">@gutter(ln.New, ln.NewHref)</td> | ||
| 238 | <td class="m">{ ln.Mark }</td> | 238 | <td class="m">{ ln.Mark }</td> | ||
| @@ -369,15 +369,15 @@ templ splitSide(ln *DiffLine, old bool) { | |||||
| 369 | <td class="m none"></td> | 369 | <td class="m none"></td> | ||
| 370 | <td class="tx none"></td> | 370 | <td class="tx none"></td> | ||
| 371 | } else { | 371 | } else { | ||
| 372 | - | <td class={ "ln", ln.Kind }> | 372 | + | <td class={ "ln", ln.Kind, templ.KV("sel", ln.Selected) }> |
| 373 | if old { | 373 | if old { | ||
| 374 | @gutter(ln.Old, ln.OldHref) | 374 | @gutter(ln.Old, ln.OldHref) | ||
| 375 | } else { | 375 | } else { | ||
| 376 | @gutter(ln.New, ln.NewHref) | 376 | @gutter(ln.New, ln.NewHref) | ||
| 377 | } | 377 | } | ||
| 378 | </td> | 378 | </td> | ||
| 379 | - | <td class={ "m", ln.Kind }>{ ln.Mark }</td> | 379 | + | <td class={ "m", ln.Kind, templ.KV("sel", ln.Selected) }>{ ln.Mark }</td> |
| 380 | - | <td class={ "tx", "chroma", ln.Kind }>@codeLine(*ln)</td> | 380 | + | <td class={ "tx", "chroma", ln.Kind, templ.KV("sel", ln.Selected) }>@codeLine(*ln)</td> |
| 381 | } | 381 | } | ||
| 382 | } | 382 | } | ||
| 383 | 383 | ||||
M internal/adapters/web/diff_templ.go +5 -5 generated file: collapsed
M internal/adapters/web/fragments.go +7 -1
| @@ -58,7 +58,7 @@ func liveEvent(we ports.WatchEvent) bool { | |||
| 58 | 58 | } | |
| 59 | 59 | switch we.Event.(type) { | |
| 60 | 60 | case domain.RepoCreated, domain.RepoUpdated, domain.RepoDeleted, domain.PushEvent, | |
| 61 | - | domain.MergeRequestOpened, domain.MergeRequestMerged, domain.MergeRequestClosed, domain.MergeRequestReady, domain.CommentAdded, | |
| 61 | + | domain.MergeRequestOpened, domain.MergeRequestMerged, domain.MergeRequestClosed, domain.MergeRequestReady, domain.CommentAdded, domain.CommentEdited, domain.CommentDeleted, domain.CommentResolved, | |
| 62 | 62 | domain.RunUpdated, domain.JobUpdated: | |
| 63 | 63 | return true | |
| 64 | 64 | } | |
| @@ -164,6 +164,12 @@ func (l *liveStream) repoPage(ctx context.Context, we ports.WatchEvent) ([]fragm | |||
| 164 | 164 | return l.mrNotice(ctx, ev.MergeRequest.Number, "this merge request was closed — reload") | |
| 165 | 165 | case domain.MergeRequestReady: | |
| 166 | 166 | return l.mrNotice(ctx, ev.MergeRequest.Number, "this merge request is ready for review — reload") | |
| 167 | + | case domain.CommentEdited: | |
| 168 | + | return l.mrNotice(ctx, ev.Number, "a note was edited — reload") | |
| 169 | + | case domain.CommentDeleted: | |
| 170 | + | return l.mrNotice(ctx, ev.Number, "a note was deleted — reload") | |
| 171 | + | case domain.CommentResolved: | |
| 172 | + | return l.mrNotice(ctx, ev.Number, "a thread was resolved — reload") | |
| 167 | 173 | case domain.CommentAdded: | |
| 168 | 174 | return l.mrNotice(ctx, ev.Number, "a comment was added — reload") | |
| 169 | 175 | case domain.PushEvent: | |
| @@ -58,7 +58,7 @@ func liveEvent(we ports.WatchEvent) bool { | |||||
| 58 | } | 58 | } | ||
| 59 | switch we.Event.(type) { | 59 | switch we.Event.(type) { | ||
| 60 | case domain.RepoCreated, domain.RepoUpdated, domain.RepoDeleted, domain.PushEvent, | 60 | case domain.RepoCreated, domain.RepoUpdated, domain.RepoDeleted, domain.PushEvent, | ||
| 61 | - | domain.MergeRequestOpened, domain.MergeRequestMerged, domain.MergeRequestClosed, domain.MergeRequestReady, domain.CommentAdded, | 61 | + | domain.MergeRequestOpened, domain.MergeRequestMerged, domain.MergeRequestClosed, domain.MergeRequestReady, domain.CommentAdded, domain.CommentEdited, domain.CommentDeleted, domain.CommentResolved, |
| 62 | domain.RunUpdated, domain.JobUpdated: | 62 | domain.RunUpdated, domain.JobUpdated: | ||
| 63 | return true | 63 | return true | ||
| 64 | } | 64 | } | ||
| @@ -164,6 +164,12 @@ func (l *liveStream) repoPage(ctx context.Context, we ports.WatchEvent) ([]fragm | |||||
| 164 | return l.mrNotice(ctx, ev.MergeRequest.Number, "this merge request was closed — reload") | 164 | return l.mrNotice(ctx, ev.MergeRequest.Number, "this merge request was closed — reload") | ||
| 165 | case domain.MergeRequestReady: | 165 | case domain.MergeRequestReady: | ||
| 166 | return l.mrNotice(ctx, ev.MergeRequest.Number, "this merge request is ready for review — reload") | 166 | return l.mrNotice(ctx, ev.MergeRequest.Number, "this merge request is ready for review — reload") | ||
| 167 | + | case domain.CommentEdited: | |||
| 168 | + | return l.mrNotice(ctx, ev.Number, "a note was edited — reload") | |||
| 169 | + | case domain.CommentDeleted: | |||
| 170 | + | return l.mrNotice(ctx, ev.Number, "a note was deleted — reload") | |||
| 171 | + | case domain.CommentResolved: | |||
| 172 | + | return l.mrNotice(ctx, ev.Number, "a thread was resolved — reload") | |||
| 167 | case domain.CommentAdded: | 173 | case domain.CommentAdded: | ||
| 168 | return l.mrNotice(ctx, ev.Number, "a comment was added — reload") | 174 | return l.mrNotice(ctx, ev.Number, "a comment was added — reload") | ||
| 169 | case domain.PushEvent: | 175 | case domain.PushEvent: | ||
M internal/adapters/web/jstest/mr.test.mjs +26 -0
| @@ -36,3 +36,29 @@ test('step: stays in the list', () => { | |||
| 36 | 36 | assert.equal(C.step(-1, -1, 3), 2); | |
| 37 | 37 | assert.equal(C.step(0, 1, 0), -1); | |
| 38 | 38 | }); | |
| 39 | + | ||
| 40 | + | test('rangeURL requires the same file, side and MR and sorts endpoints', () => { | |
| 41 | + | const base = 'https://forge.test/demo/merge-requests/1'; | |
| 42 | + | const a = base+'?on=new:24:dir%2Fa.go#f1'; | |
| 43 | + | const b = base+'?from=25&on=new:10:dir%2Fa.go#f1'; | |
| 44 | + | const u = new URL(C.rangeURL(a,b,base)); | |
| 45 | + | assert.equal(u.searchParams.get('on'),'new:10-24:dir/a.go'); | |
| 46 | + | assert.equal(u.searchParams.get('from'),'25'); | |
| 47 | + | assert.equal(u.hash,'#f1'); | |
| 48 | + | assert.equal(C.rangeURL(a,a,base),null); | |
| 49 | + | assert.equal(C.rangeURL(a,b.replace('new:','old:'),base),null); | |
| 50 | + | assert.equal(C.rangeURL(a,b.replace('a.go','b.go'),base),null); | |
| 51 | + | assert.equal(C.rangeURL(a,b.replace('/1?','/2?'),base),null); | |
| 52 | + | }); | |
| 53 | + | ||
| 54 | + | test('quote opens the thread reply and prefills and focuses its textarea', () => { | |
| 55 | + | let focused=false; | |
| 56 | + | const field={value:'',focus(){focused=true;}}; | |
| 57 | + | const details={open:false,querySelector(){return field;}}; | |
| 58 | + | const thread={querySelector(){return details;}}; | |
| 59 | + | const button={dataset:{quote:'> source\n\n^2 '},closest(){return {closest(){return thread;}};}}; | |
| 60 | + | assert.equal(C.quote(button),true); | |
| 61 | + | assert.equal(details.open,true); | |
| 62 | + | assert.equal(field.value,'> source\n\n^2 '); | |
| 63 | + | assert.equal(focused,true); | |
| 64 | + | }); | |
| @@ -36,3 +36,29 @@ test('step: stays in the list', () => { | |||||
| 36 | assert.equal(C.step(-1, -1, 3), 2); | 36 | assert.equal(C.step(-1, -1, 3), 2); | ||
| 37 | assert.equal(C.step(0, 1, 0), -1); | 37 | assert.equal(C.step(0, 1, 0), -1); | ||
| 38 | }); | 38 | }); | ||
| 39 | + | ||||
| 40 | + | test('rangeURL requires the same file, side and MR and sorts endpoints', () => { | |||
| 41 | + | const base = 'https://forge.test/demo/merge-requests/1'; | |||
| 42 | + | const a = base+'?on=new:24:dir%2Fa.go#f1'; | |||
| 43 | + | const b = base+'?from=25&on=new:10:dir%2Fa.go#f1'; | |||
| 44 | + | const u = new URL(C.rangeURL(a,b,base)); | |||
| 45 | + | assert.equal(u.searchParams.get('on'),'new:10-24:dir/a.go'); | |||
| 46 | + | assert.equal(u.searchParams.get('from'),'25'); | |||
| 47 | + | assert.equal(u.hash,'#f1'); | |||
| 48 | + | assert.equal(C.rangeURL(a,a,base),null); | |||
| 49 | + | assert.equal(C.rangeURL(a,b.replace('new:','old:'),base),null); | |||
| 50 | + | assert.equal(C.rangeURL(a,b.replace('a.go','b.go'),base),null); | |||
| 51 | + | assert.equal(C.rangeURL(a,b.replace('/1?','/2?'),base),null); | |||
| 52 | + | }); | |||
| 53 | + | ||||
| 54 | + | test('quote opens the thread reply and prefills and focuses its textarea', () => { | |||
| 55 | + | let focused=false; | |||
| 56 | + | const field={value:'',focus(){focused=true;}}; | |||
| 57 | + | const details={open:false,querySelector(){return field;}}; | |||
| 58 | + | const thread={querySelector(){return details;}}; | |||
| 59 | + | const button={dataset:{quote:'> source\n\n^2 '},closest(){return {closest(){return thread;}};}}; | |||
| 60 | + | assert.equal(C.quote(button),true); | |||
| 61 | + | assert.equal(details.open,true); | |||
| 62 | + | assert.equal(field.value,'> source\n\n^2 '); | |||
| 63 | + | assert.equal(focused,true); | |||
| 64 | + | }); | |||
M internal/adapters/web/markdown.go +79 -0
| @@ -7,6 +7,8 @@ import ( | |||
| 7 | 7 | "html/template" | |
| 8 | 8 | "net/url" | |
| 9 | 9 | "path" | |
| 10 | + | "regexp" | |
| 11 | + | "strconv" | |
| 10 | 12 | "strings" | |
| 11 | 13 | ||
| 12 | 14 | "github.com/yuin/goldmark" | |
| @@ -62,10 +64,40 @@ func (m *markdown) render(source []byte, repo, ref, dir string) (template.HTML, | |||
| 62 | 64 | // #fragment links chosen, so that several documents on one page (the summaries | |
| 63 | 65 | // of a run) do not share ids. | |
| 64 | 66 | func (m *markdown) renderWithIDs(source []byte, repo, ref, dir, idPrefix string) (template.HTML, []Heading, error) { | |
| 67 | + | return m.renderDocument(source, repo, ref, dir, idPrefix, nil) | |
| 68 | + | } | |
| 69 | + | ||
| 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.renderDocument(source, repo, ref, "", idPrefix, notes) | |
| 73 | + | return h, err | |
| 74 | + | } | |
| 75 | + | ||
| 76 | + | func (m *markdown) renderDocument(source []byte, repo, ref, dir, idPrefix string, notes func(int64) bool) (template.HTML, []Heading, error) { | |
| 65 | 77 | pc := parser.NewContext(parser.WithIDs(&prefixedIDs{prefix: idPrefix, inner: parser.NewContext().IDs()})) | |
| 66 | 78 | doc := m.md.Parser().Parse(text.NewReader(source), parser.WithContext(pc)) | |
| 67 | 79 | pol := linkPolicy{repo: repo, ref: ref, dir: dir, source: source, idPrefix: idPrefix} | |
| 80 | + | // Collect eligible text before the policy unwraps rejected links: their | |
| 81 | + | // labels must not accidentally become references either. | |
| 82 | + | var refs []*ast.Text | |
| 83 | + | if notes != nil { | |
| 84 | + | _ = ast.Walk(doc, func(n ast.Node, entering bool) (ast.WalkStatus, error) { | |
| 85 | + | if !entering { | |
| 86 | + | return ast.WalkContinue, nil | |
| 87 | + | } | |
| 88 | + | switch n := n.(type) { | |
| 89 | + | case *ast.CodeSpan, *ast.Link, *ast.AutoLink, *ast.Image, *ast.CodeBlock, *ast.FencedCodeBlock: | |
| 90 | + | return ast.WalkSkipChildren, nil | |
| 91 | + | case *ast.Text: | |
| 92 | + | refs = append(refs, n) | |
| 93 | + | } | |
| 94 | + | return ast.WalkContinue, nil | |
| 95 | + | }) | |
| 96 | + | } | |
| 68 | 97 | pol.apply(doc) | |
| 98 | + | for _, t := range refs { | |
| 99 | + | noteReferences(t, source, notes) | |
| 100 | + | } | |
| 69 | 101 | ||
| 70 | 102 | var buf bytes.Buffer | |
| 71 | 103 | if err := m.md.Renderer().Render(&buf, source, doc); err != nil { | |
| @@ -291,3 +323,50 @@ func (altRenderer) RegisterFuncs(r renderer.NodeRendererFuncRegisterer) { | |||
| 291 | 323 | return ast.WalkContinue, nil | |
| 292 | 324 | }) | |
| 293 | 325 | } | |
| 326 | + | ||
| 327 | + | var noteReferencePattern = regexp.MustCompile(`(^|[^0-9A-Za-z_^])\^([1-9][0-9]{0,5})\b`) | |
| 328 | + | ||
| 329 | + | // noteReferences changes only text segments; code and link labels never enter | |
| 330 | + | // this function. New fragment links deliberately bypass heading-prefix rewriting. | |
| 331 | + | func noteReferences(t *ast.Text, source []byte, exists func(int64) bool) { | |
| 332 | + | parent := t.Parent() | |
| 333 | + | if parent == nil { | |
| 334 | + | return | |
| 335 | + | } | |
| 336 | + | value := t.Value(source) | |
| 337 | + | offset := 0 | |
| 338 | + | for _, match := range noteReferencePattern.FindAllSubmatchIndex(value, -1) { | |
| 339 | + | id := string(value[match[4]:match[5]]) | |
| 340 | + | n, err := strconv.ParseInt(id, 10, 64) | |
| 341 | + | if err != nil || !exists(n) { | |
| 342 | + | continue | |
| 343 | + | } | |
| 344 | + | start := match[4] - 1 | |
| 345 | + | // Markdown punctuation may split adjacent text into separate AST nodes. | |
| 346 | + | // Check the original preceding byte as well as the segment-local regex. | |
| 347 | + | if at := t.Segment.Start + start; at > 0 { | |
| 348 | + | prev := source[at-1] | |
| 349 | + | if prev >= '0' && prev <= '9' || prev >= 'A' && prev <= 'Z' || prev >= 'a' && prev <= 'z' || prev == '_' || prev == '^' { | |
| 350 | + | continue | |
| 351 | + | } | |
| 352 | + | } | |
| 353 | + | if start > offset { | |
| 354 | + | parent.InsertBefore(parent, t, ast.NewTextSegment(text.NewSegment(t.Segment.Start+offset, t.Segment.Start+start))) | |
| 355 | + | } | |
| 356 | + | link := ast.NewLink() | |
| 357 | + | link.Destination = []byte("#c" + id) | |
| 358 | + | link.Title = []byte("note " + id) | |
| 359 | + | link.SetAttributeString("class", []byte("ref")) | |
| 360 | + | link.AppendChild(link, ast.NewString(value[start:match[5]])) | |
| 361 | + | parent.InsertBefore(parent, t, link) | |
| 362 | + | offset = match[5] | |
| 363 | + | } | |
| 364 | + | if offset == 0 { | |
| 365 | + | return | |
| 366 | + | } | |
| 367 | + | tail := ast.NewTextSegment(text.NewSegment(t.Segment.Start+offset, t.Segment.Stop)) | |
| 368 | + | tail.SetSoftLineBreak(t.SoftLineBreak()) | |
| 369 | + | tail.SetHardLineBreak(t.HardLineBreak()) | |
| 370 | + | parent.InsertBefore(parent, t, tail) | |
| 371 | + | parent.RemoveChild(parent, t) | |
| 372 | + | } | |
| @@ -7,6 +7,8 @@ import ( | |||||
| 7 | "html/template" | 7 | "html/template" | ||
| 8 | "net/url" | 8 | "net/url" | ||
| 9 | "path" | 9 | "path" | ||
| 10 | + | "regexp" | |||
| 11 | + | "strconv" | |||
| 10 | "strings" | 12 | "strings" | ||
| 11 | 13 | ||||
| 12 | "github.com/yuin/goldmark" | 14 | "github.com/yuin/goldmark" | ||
| @@ -62,10 +64,40 @@ func (m *markdown) render(source []byte, repo, ref, dir string) (template.HTML, | |||||
| 62 | // #fragment links chosen, so that several documents on one page (the summaries | 64 | // #fragment links chosen, so that several documents on one page (the summaries | ||
| 63 | // of a run) do not share ids. | 65 | // of a run) do not share ids. | ||
| 64 | 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.renderDocument(source, repo, ref, dir, idPrefix, nil) | |||
| 68 | + | } | |||
| 69 | + | ||||
| 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.renderDocument(source, repo, ref, "", idPrefix, notes) | |||
| 73 | + | return h, err | |||
| 74 | + | } | |||
| 75 | + | ||||
| 76 | + | func (m *markdown) renderDocument(source []byte, repo, ref, dir, idPrefix string, notes func(int64) bool) (template.HTML, []Heading, error) { | |||
| 65 | pc := parser.NewContext(parser.WithIDs(&prefixedIDs{prefix: idPrefix, inner: parser.NewContext().IDs()})) | 77 | pc := parser.NewContext(parser.WithIDs(&prefixedIDs{prefix: idPrefix, inner: parser.NewContext().IDs()})) | ||
| 66 | doc := m.md.Parser().Parse(text.NewReader(source), parser.WithContext(pc)) | 78 | doc := m.md.Parser().Parse(text.NewReader(source), parser.WithContext(pc)) | ||
| 67 | pol := linkPolicy{repo: repo, ref: ref, dir: dir, source: source, idPrefix: idPrefix} | 79 | pol := linkPolicy{repo: repo, ref: ref, dir: dir, source: source, idPrefix: idPrefix} | ||
| 80 | + | // Collect eligible text before the policy unwraps rejected links: their | |||
| 81 | + | // labels must not accidentally become references either. | |||
| 82 | + | var refs []*ast.Text | |||
| 83 | + | if notes != nil { | |||
| 84 | + | _ = ast.Walk(doc, func(n ast.Node, entering bool) (ast.WalkStatus, error) { | |||
| 85 | + | if !entering { | |||
| 86 | + | return ast.WalkContinue, nil | |||
| 87 | + | } | |||
| 88 | + | switch n := n.(type) { | |||
| 89 | + | case *ast.CodeSpan, *ast.Link, *ast.AutoLink, *ast.Image, *ast.CodeBlock, *ast.FencedCodeBlock: | |||
| 90 | + | return ast.WalkSkipChildren, nil | |||
| 91 | + | case *ast.Text: | |||
| 92 | + | refs = append(refs, n) | |||
| 93 | + | } | |||
| 94 | + | return ast.WalkContinue, nil | |||
| 95 | + | }) | |||
| 96 | + | } | |||
| 68 | pol.apply(doc) | 97 | pol.apply(doc) | ||
| 98 | + | for _, t := range refs { | |||
| 99 | + | noteReferences(t, source, notes) | |||
| 100 | + | } | |||
| 69 | 101 | ||||
| 70 | var buf bytes.Buffer | 102 | var buf bytes.Buffer | ||
| 71 | if err := m.md.Renderer().Render(&buf, source, doc); err != nil { | 103 | if err := m.md.Renderer().Render(&buf, source, doc); err != nil { | ||
| @@ -291,3 +323,50 @@ func (altRenderer) RegisterFuncs(r renderer.NodeRendererFuncRegisterer) { | |||||
| 291 | return ast.WalkContinue, nil | 323 | return ast.WalkContinue, nil | ||
| 292 | }) | 324 | }) | ||
| 293 | } | 325 | } | ||
| 326 | + | ||||
| 327 | + | var noteReferencePattern = regexp.MustCompile(`(^|[^0-9A-Za-z_^])\^([1-9][0-9]{0,5})\b`) | |||
| 328 | + | ||||
| 329 | + | // noteReferences changes only text segments; code and link labels never enter | |||
| 330 | + | // this function. New fragment links deliberately bypass heading-prefix rewriting. | |||
| 331 | + | func noteReferences(t *ast.Text, source []byte, exists func(int64) bool) { | |||
| 332 | + | parent := t.Parent() | |||
| 333 | + | if parent == nil { | |||
| 334 | + | return | |||
| 335 | + | } | |||
| 336 | + | value := t.Value(source) | |||
| 337 | + | offset := 0 | |||
| 338 | + | for _, match := range noteReferencePattern.FindAllSubmatchIndex(value, -1) { | |||
| 339 | + | id := string(value[match[4]:match[5]]) | |||
| 340 | + | n, err := strconv.ParseInt(id, 10, 64) | |||
| 341 | + | if err != nil || !exists(n) { | |||
| 342 | + | continue | |||
| 343 | + | } | |||
| 344 | + | start := match[4] - 1 | |||
| 345 | + | // Markdown punctuation may split adjacent text into separate AST nodes. | |||
| 346 | + | // Check the original preceding byte as well as the segment-local regex. | |||
| 347 | + | if at := t.Segment.Start + start; at > 0 { | |||
| 348 | + | prev := source[at-1] | |||
| 349 | + | if prev >= '0' && prev <= '9' || prev >= 'A' && prev <= 'Z' || prev >= 'a' && prev <= 'z' || prev == '_' || prev == '^' { | |||
| 350 | + | continue | |||
| 351 | + | } | |||
| 352 | + | } | |||
| 353 | + | if start > offset { | |||
| 354 | + | parent.InsertBefore(parent, t, ast.NewTextSegment(text.NewSegment(t.Segment.Start+offset, t.Segment.Start+start))) | |||
| 355 | + | } | |||
| 356 | + | link := ast.NewLink() | |||
| 357 | + | link.Destination = []byte("#c" + id) | |||
| 358 | + | link.Title = []byte("note " + id) | |||
| 359 | + | link.SetAttributeString("class", []byte("ref")) | |||
| 360 | + | link.AppendChild(link, ast.NewString(value[start:match[5]])) | |||
| 361 | + | parent.InsertBefore(parent, t, link) | |||
| 362 | + | offset = match[5] | |||
| 363 | + | } | |||
| 364 | + | if offset == 0 { | |||
| 365 | + | return | |||
| 366 | + | } | |||
| 367 | + | tail := ast.NewTextSegment(text.NewSegment(t.Segment.Start+offset, t.Segment.Stop)) | |||
| 368 | + | tail.SetSoftLineBreak(t.SoftLineBreak()) | |||
| 369 | + | tail.SetHardLineBreak(t.HardLineBreak()) | |||
| 370 | + | parent.InsertBefore(parent, t, tail) | |||
| 371 | + | parent.RemoveChild(parent, t) | |||
| 372 | + | } | |||
M internal/adapters/web/markdown_test.go +29 -0
| @@ -136,3 +136,32 @@ func TestMarkdownGFM(t *testing.T) { | |||
| 136 | 136 | assert.Contains(t, out, "<del>gone</del>") | |
| 137 | 137 | assert.Contains(t, out, `type="checkbox"`) | |
| 138 | 138 | } | |
| 139 | + | ||
| 140 | + | func TestNoteMarkdown(t *testing.T) { | |
| 141 | + | t.Parallel() | |
| 142 | + | tests := []struct { | |
| 143 | + | source string | |
| 144 | + | links int | |
| 145 | + | }{ | |
| 146 | + | {"^2 ^99 a^2 _^2 ^^2 1^2", 1}, | |
| 147 | + | {"`^2` [^2](https://example.org) [^2](javascript:alert(1))", 0}, | |
| 148 | + | {"```\n^2\n```", 0}, | |
| 149 | + | {"^2, ^2\n^2", 3}, | |
| 150 | + | } | |
| 151 | + | for _, tt := range tests { | |
| 152 | + | t.Run(tt.source, func(t *testing.T) { | |
| 153 | + | t.Parallel() | |
| 154 | + | h, err := newMarkdown().renderNote([]byte(tt.source), "demo", "main", "c1-", func(n int64) bool { return n == 2 }) | |
| 155 | + | require.NoError(t, err) | |
| 156 | + | assert.Equal(t, tt.links, strings.Count(string(h), `href="#c2"`), string(h)) | |
| 157 | + | assert.NotContains(t, string(h), `href="#c99"`) | |
| 158 | + | assert.NotContains(t, string(h), "javascript:") | |
| 159 | + | }) | |
| 160 | + | } | |
| 161 | + | 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 }) | |
| 162 | + | require.NoError(t, err) | |
| 163 | + | assert.Contains(t, string(h), `id="c1-heading"`) | |
| 164 | + | assert.Contains(t, string(h), "<strong>bold</strong>") | |
| 165 | + | assert.NotContains(t, string(h), "<script") | |
| 166 | + | assert.Contains(t, string(h), `title="note 2"`) | |
| 167 | + | } | |
| @@ -136,3 +136,32 @@ func TestMarkdownGFM(t *testing.T) { | |||||
| 136 | assert.Contains(t, out, "<del>gone</del>") | 136 | assert.Contains(t, out, "<del>gone</del>") | ||
| 137 | assert.Contains(t, out, `type="checkbox"`) | 137 | assert.Contains(t, out, `type="checkbox"`) | ||
| 138 | } | 138 | } | ||
| 139 | + | ||||
| 140 | + | func TestNoteMarkdown(t *testing.T) { | |||
| 141 | + | t.Parallel() | |||
| 142 | + | tests := []struct { | |||
| 143 | + | source string | |||
| 144 | + | links int | |||
| 145 | + | }{ | |||
| 146 | + | {"^2 ^99 a^2 _^2 ^^2 1^2", 1}, | |||
| 147 | + | {"`^2` [^2](https://example.org) [^2](javascript:alert(1))", 0}, | |||
| 148 | + | {"```\n^2\n```", 0}, | |||
| 149 | + | {"^2, ^2\n^2", 3}, | |||
| 150 | + | } | |||
| 151 | + | for _, tt := range tests { | |||
| 152 | + | t.Run(tt.source, func(t *testing.T) { | |||
| 153 | + | t.Parallel() | |||
| 154 | + | h, err := newMarkdown().renderNote([]byte(tt.source), "demo", "main", "c1-", func(n int64) bool { return n == 2 }) | |||
| 155 | + | require.NoError(t, err) | |||
| 156 | + | assert.Equal(t, tt.links, strings.Count(string(h), `href="#c2"`), string(h)) | |||
| 157 | + | assert.NotContains(t, string(h), `href="#c99"`) | |||
| 158 | + | assert.NotContains(t, string(h), "javascript:") | |||
| 159 | + | }) | |||
| 160 | + | } | |||
| 161 | + | 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 }) | |||
| 162 | + | require.NoError(t, err) | |||
| 163 | + | assert.Contains(t, string(h), `id="c1-heading"`) | |||
| 164 | + | assert.Contains(t, string(h), "<strong>bold</strong>") | |||
| 165 | + | assert.NotContains(t, string(h), "<script") | |||
| 166 | + | assert.Contains(t, string(h), `title="note 2"`) | |||
| 167 | + | } | |||
M internal/adapters/web/mr.go +125 -25
| @@ -101,13 +101,19 @@ type MRListPage struct { | |||
| 101 | 101 | ||
| 102 | 102 | // CommentView is a comment of a merge request, ready to show. | |
| 103 | 103 | type CommentView struct { | |
| 104 | - | ID string | |
| 105 | - | Author string | |
| 106 | - | When string | |
| 107 | - | WhenAt string | |
| 108 | - | Abs string | |
| 109 | - | Parts []TextPart | |
| 110 | - | Resolved bool | |
| 104 | + | ID string | |
| 105 | + | Author string | |
| 106 | + | When string | |
| 107 | + | WhenAt string | |
| 108 | + | Abs string | |
| 109 | + | Body template.HTML | |
| 110 | + | Replies []CommentView | |
| 111 | + | Edited, Deleted bool | |
| 112 | + | Verdict, Range, Quote string | |
| 113 | + | Edit *EditForm | |
| 114 | + | Delete *DeleteForm | |
| 115 | + | Reply *CommentForm | |
| 116 | + | Resolved bool | |
| 111 | 117 | // Outdated is true when the head moved since the comment was written, so | |
| 112 | 118 | // its line may have moved too. | |
| 113 | 119 | Outdated bool | |
| @@ -119,6 +125,19 @@ type CommentView struct { | |||
| 119 | 125 | Resolve *ResolveForm | |
| 120 | 126 | } | |
| 121 | 127 | ||
| 128 | + | type EditForm struct { | |
| 129 | + | Action, Text string | |
| 130 | + | From int | |
| 131 | + | } | |
| 132 | + | type DeleteForm struct { | |
| 133 | + | Action string | |
| 134 | + | From int | |
| 135 | + | } | |
| 136 | + | type VerdictView struct { | |
| 137 | + | Author, Label, Href, Short string | |
| 138 | + | Stale bool | |
| 139 | + | } | |
| 140 | + | ||
| 122 | 141 | // ResolveForm is the POST that changes the resolved state of a comment. | |
| 123 | 142 | type ResolveForm struct { | |
| 124 | 143 | Action string | |
| @@ -135,9 +154,10 @@ type CommentForm struct { | |||
| 135 | 154 | Action string | |
| 136 | 155 | Title string | |
| 137 | 156 | // Path, Line and Side designate the line; empty for a general comment. | |
| 138 | - | Path, Line, Side string | |
| 139 | - | Cancel string | |
| 140 | - | Focus bool | |
| 157 | + | Path, Line, Side string | |
| 158 | + | StartLine, ReplyTo string | |
| 159 | + | Cancel string | |
| 160 | + | Focus bool | |
| 141 | 161 | // From is the window of the page the form is on (see mrWindow), sent | |
| 142 | 162 | // back so that the answer returns to it; 0 for the first. | |
| 143 | 163 | From int | |
| @@ -192,6 +212,7 @@ type MRPage struct { | |||
| 192 | 212 | State string | |
| 193 | 213 | Open bool | |
| 194 | 214 | Signed bool | |
| 215 | + | NoteAuthor string | |
| 195 | 216 | Source, Target string | |
| 196 | 217 | SourceHref string | |
| 197 | 218 | TargetHref string | |
| @@ -226,6 +247,7 @@ type MRPage struct { | |||
| 226 | 247 | NComments int | |
| 227 | 248 | NUnresolved int | |
| 228 | 249 | GeneralForm *CommentForm | |
| 250 | + | Verdicts []VerdictView | |
| 229 | 251 | ||
| 230 | 252 | Commits []MRCommit | |
| 231 | 253 | CommitsNote string | |
| @@ -471,7 +493,7 @@ func (s *Server) mrPage(ctx context.Context, r *http.Request, since bool) (templ | |||
| 471 | 493 | ||
| 472 | 494 | p := MRPage{ | |
| 473 | 495 | Number: "#" + strconv.FormatInt(n, 10), Title: visibleText(m.Title.String()), | |
| 474 | - | State: mrStateName(m), Open: open, Signed: signed, | |
| 496 | + | State: mrStateName(m), Open: open, Signed: signed, NoteAuthor: domain.AuthorName(principal(ctx)), | |
| 475 | 497 | Source: visibleText(m.Source), Target: visibleText(m.Target), | |
| 476 | 498 | Author: s.who(m.Author), | |
| 477 | 499 | Head: head, HeadShort: short(head), | |
| @@ -485,12 +507,15 @@ func (s *Server) mrPage(ctx context.Context, r *http.Request, since bool) (templ | |||
| 485 | 507 | // Relative links point into the source branch as it is now (its head | |
| 486 | 508 | // commit); the heading ids carry their own prefix, apart from the | |
| 487 | 509 | // ids of the diff. | |
| 488 | - | h, _, err := s.md.renderWithIDs([]byte(desc), d.name(), head, "", "mr-desc-") | |
| 510 | + | h, err := s.md.renderNote([]byte(desc), d.name(), head, "mr-desc-", noteIDs(res.Comments)) | |
| 489 | 511 | if err != nil { | |
| 490 | 512 | return nil, 0, err | |
| 491 | 513 | } | |
| 492 | 514 | p.Desc = h | |
| 493 | 515 | } | |
| 516 | + | for _, v := range res.Verdicts { | |
| 517 | + | p.Verdicts = append(p.Verdicts, VerdictView{Author: s.who(v.Author), Label: v.Verdict.String(), Stale: v.Stale, Href: "#c" + strconv.FormatInt(v.CommentID, 10), Short: short(v.CommitID.String())}) | |
| 518 | + | } | |
| 494 | 519 | p.Opened.Name = p.Author | |
| 495 | 520 | p.Opened.When, p.Opened.WhenAt = whenOf(now, m.CreatedAt) | |
| 496 | 521 | if !m.CreatedAt.IsZero() { | |
| @@ -684,9 +709,10 @@ func shellWord(s string) string { | |||
| 684 | 709 | // lineRef designates a line of the diff in the query of the comment form: | |
| 685 | 710 | // "?on=new:12:path/to/file". | |
| 686 | 711 | type lineRef struct { | |
| 687 | - | side string | |
| 688 | - | line int | |
| 689 | - | path string | |
| 712 | + | side string | |
| 713 | + | line int | |
| 714 | + | start int | |
| 715 | + | path string | |
| 690 | 716 | } | |
| 691 | 717 | ||
| 692 | 718 | func parseOn(s string) *lineRef { | |
| @@ -695,11 +721,21 @@ func parseOn(s string) *lineRef { | |||
| 695 | 721 | return nil | |
| 696 | 722 | } | |
| 697 | 723 | num, path, ok := strings.Cut(rest, ":") | |
| 724 | + | start := 0 | |
| 725 | + | if a, b, rangeOK := strings.Cut(num, "-"); rangeOK { | |
| 726 | + | var e error | |
| 727 | + | start, e = strconv.Atoi(a) | |
| 728 | + | end, e2 := strconv.Atoi(b) | |
| 729 | + | if e != nil || e2 != nil || start < 1 || start >= end { | |
| 730 | + | return nil | |
| 731 | + | } | |
| 732 | + | num = b | |
| 733 | + | } | |
| 698 | 734 | line, err := strconv.Atoi(num) | |
| 699 | 735 | if !ok || err != nil || line < 1 || path == "" { | |
| 700 | 736 | return nil | |
| 701 | 737 | } | |
| 702 | - | return &lineRef{side: side, line: line, path: path} | |
| 738 | + | return &lineRef{side: side, line: line, start: start, path: path} | |
| 703 | 739 | } | |
| 704 | 740 | ||
| 705 | 741 | // placeComments puts every comment under its line when the diff shown has it, | |
| @@ -757,21 +793,62 @@ func (s *Server) placeComments(p *MRPage, repo string, n int64, comments []domai | |||
| 757 | 793 | return index[lineKey{path, side, line}] | |
| 758 | 794 | } | |
| 759 | 795 | ||
| 796 | + | exists := noteIDs(comments) | |
| 797 | + | children := map[int64][]domain.Comment{} | |
| 760 | 798 | for _, c := range comments { | |
| 799 | + | if c.ReplyTo != 0 { | |
| 800 | + | children[c.ReplyTo] = append(children[c.ReplyTo], c) | |
| 801 | + | } | |
| 802 | + | if !c.Deleted { | |
| 803 | + | p.NComments++ | |
| 804 | + | } | |
| 805 | + | if c.ReplyTo == 0 && !c.Deleted && !c.Resolved { | |
| 806 | + | p.NUnresolved++ | |
| 807 | + | } | |
| 808 | + | } | |
| 809 | + | var view func(domain.Comment) CommentView | |
| 810 | + | view = func(c domain.Comment) CommentView { | |
| 761 | 811 | cid := strconv.FormatInt(c.ID, 10) | |
| 762 | - | v := CommentView{ | |
| 763 | - | ID: cid, Author: s.who(c.Author), | |
| 764 | - | Parts: plainParts(c.Text.String()), Resolved: c.Resolved, | |
| 765 | - | Outdated: !c.CommitID.IsZero() && c.CommitID.String() != head, | |
| 812 | + | body, err := s.md.renderNote([]byte(c.Text.String()), repo, head, "c"+cid+"-", exists) | |
| 813 | + | if err != nil { | |
| 814 | + | body = "" | |
| 766 | 815 | } | |
| 816 | + | v := CommentView{ID: cid, Author: s.who(c.Author), Body: body, Resolved: c.Resolved, Deleted: c.Deleted, Edited: !c.EditedAt.IsZero(), Verdict: c.Verdict.String(), Outdated: !c.CommitID.IsZero() && c.CommitID.String() != head} | |
| 767 | 817 | v.When, v.WhenAt = whenOf(now, c.CreatedAt) | |
| 768 | 818 | if !c.CreatedAt.IsZero() { | |
| 769 | 819 | v.Abs = absolute(c.CreatedAt) | |
| 770 | 820 | } | |
| 771 | - | p.NComments++ | |
| 772 | - | if !c.Resolved { | |
| 773 | - | p.NUnresolved++ | |
| 821 | + | if c.StartLine > 0 { | |
| 822 | + | v.Range = "lines " + itoa(c.StartLine) + "–" + itoa(c.Line) | |
| 774 | 823 | } | |
| 824 | + | if canWrite && !c.Deleted { | |
| 825 | + | from := windowStart(fileAt[c.Path]) | |
| 826 | + | root := cid | |
| 827 | + | if c.ReplyTo != 0 { | |
| 828 | + | root = strconv.FormatInt(c.ReplyTo, 10) | |
| 829 | + | } | |
| 830 | + | v.Quote = "> " + strings.ReplaceAll(c.Text.String(), "\n", "\n> ") + "\n\n^" + cid + " " | |
| 831 | + | if c.ReplyTo == 0 { | |
| 832 | + | v.Reply = &CommentForm{ID: "reply-" + cid, Action: mrHref(repo, n, "comments"), Title: "Reply", ReplyTo: root, From: from} | |
| 833 | + | } | |
| 834 | + | if c.Author == p.NoteAuthor { | |
| 835 | + | v.Edit = &EditForm{Action: mrHref(repo, n, "comments", cid, "edit"), Text: c.Text.String(), From: from} | |
| 836 | + | v.Delete = &DeleteForm{Action: mrHref(repo, n, "comments", cid, "delete"), From: from} | |
| 837 | + | } | |
| 838 | + | } | |
| 839 | + | for _, reply := range children[c.ID] { | |
| 840 | + | if !reply.Deleted { | |
| 841 | + | v.Replies = append(v.Replies, view(reply)) | |
| 842 | + | } | |
| 843 | + | } | |
| 844 | + | return v | |
| 845 | + | } | |
| 846 | + | for _, c := range comments { | |
| 847 | + | if c.ReplyTo != 0 || c.Deleted && len(children[c.ID]) == 0 { | |
| 848 | + | continue | |
| 849 | + | } | |
| 850 | + | cid := strconv.FormatInt(c.ID, 10) | |
| 851 | + | v := view(c) | |
| 775 | 852 | general := c.Path == "" | |
| 776 | 853 | fi, listed := fileAt[c.Path] | |
| 777 | 854 | if !general && listed && p.Files[fi].Lazy { | |
| @@ -789,7 +866,7 @@ func (s *Server) placeComments(p *MRPage, repo string, n int64, comments []domai | |||
| 789 | 866 | v.Where = visibleText(c.Path) + ":" + itoa(c.Line) + " (" + side + ")" | |
| 790 | 867 | } | |
| 791 | 868 | } | |
| 792 | - | if canWrite { | |
| 869 | + | if canWrite && !c.Deleted { | |
| 793 | 870 | v.Resolve = &ResolveForm{Action: resolveAction(cid), Value: !c.Resolved, Label: "Resolve"} | |
| 794 | 871 | if !general && listed { | |
| 795 | 872 | v.Resolve.From = windowStart(fi) | |
| @@ -815,9 +892,18 @@ func (s *Server) placeComments(p *MRPage, repo string, n int64, comments []domai | |||
| 815 | 892 | if on.side == "old" { | |
| 816 | 893 | side = domain.SideOld | |
| 817 | 894 | } | |
| 895 | + | start := on.start | |
| 896 | + | if start == 0 { | |
| 897 | + | start = on.line | |
| 898 | + | } | |
| 899 | + | for key, ln := range index { | |
| 900 | + | if key.path == on.path && key.side == side && key.line >= start && key.line <= on.line { | |
| 901 | + | ln.Selected = true | |
| 902 | + | } | |
| 903 | + | } | |
| 818 | 904 | if ln := find(on.path, side, on.line); ln != nil { | |
| 819 | 905 | ln.Form = &CommentForm{ | |
| 820 | - | ID: "cf-line", Action: mrHref(repo, n, "comments"), Path: on.path, Line: itoa(on.line), Side: on.side, | |
| 906 | + | StartLine: optionalLine(on.start), ID: "cf-line", Action: mrHref(repo, n, "comments"), Path: on.path, Line: itoa(on.line), Side: on.side, | |
| 821 | 907 | Title: "comment on " + visibleText(on.path) + ":" + itoa(on.line) + " (" + on.side + ")", | |
| 822 | 908 | Cancel: withFrom(self, windowStart(fileAt[on.path])) + "#" + fileIDs[on.path], Focus: true, | |
| 823 | 909 | From: windowStart(fileAt[on.path]), | |
| @@ -842,3 +928,17 @@ func userText(parts []TextPart) templ.Component { | |||
| 842 | 928 | return err | |
| 843 | 929 | }) | |
| 844 | 930 | } | |
| 931 | + | ||
| 932 | + | func noteIDs(comments []domain.Comment) func(int64) bool { | |
| 933 | + | ids := make(map[int64]bool, len(comments)) | |
| 934 | + | for _, c := range comments { | |
| 935 | + | ids[c.ID] = true | |
| 936 | + | } | |
| 937 | + | return func(n int64) bool { return ids[n] } | |
| 938 | + | } | |
| 939 | + | func optionalLine(n int) string { | |
| 940 | + | if n == 0 { | |
| 941 | + | return "" | |
| 942 | + | } | |
| 943 | + | return itoa(n) | |
| 944 | + | } | |
| @@ -101,13 +101,19 @@ type MRListPage struct { | |||||
| 101 | 101 | ||||
| 102 | // CommentView is a comment of a merge request, ready to show. | 102 | // CommentView is a comment of a merge request, ready to show. | ||
| 103 | type CommentView struct { | 103 | type CommentView struct { | ||
| 104 | - | ID string | 104 | + | ID string |
| 105 | - | Author string | 105 | + | Author string |
| 106 | - | When string | 106 | + | When string |
| 107 | - | WhenAt string | 107 | + | WhenAt string |
| 108 | - | Abs string | 108 | + | Abs string |
| 109 | - | Parts []TextPart | 109 | + | Body template.HTML |
| 110 | - | Resolved bool | 110 | + | Replies []CommentView |
| 111 | + | Edited, Deleted bool | |||
| 112 | + | Verdict, Range, Quote string | |||
| 113 | + | Edit *EditForm | |||
| 114 | + | Delete *DeleteForm | |||
| 115 | + | Reply *CommentForm | |||
| 116 | + | Resolved bool | |||
| 111 | // Outdated is true when the head moved since the comment was written, so | 117 | // Outdated is true when the head moved since the comment was written, so | ||
| 112 | // its line may have moved too. | 118 | // its line may have moved too. | ||
| 113 | Outdated bool | 119 | Outdated bool | ||
| @@ -119,6 +125,19 @@ type CommentView struct { | |||||
| 119 | Resolve *ResolveForm | 125 | Resolve *ResolveForm | ||
| 120 | } | 126 | } | ||
| 121 | 127 | ||||
| 128 | + | type EditForm struct { | |||
| 129 | + | Action, Text string | |||
| 130 | + | From int | |||
| 131 | + | } | |||
| 132 | + | type DeleteForm struct { | |||
| 133 | + | Action string | |||
| 134 | + | From int | |||
| 135 | + | } | |||
| 136 | + | type VerdictView struct { | |||
| 137 | + | Author, Label, Href, Short string | |||
| 138 | + | Stale bool | |||
| 139 | + | } | |||
| 140 | + | ||||
| 122 | // ResolveForm is the POST that changes the resolved state of a comment. | 141 | // ResolveForm is the POST that changes the resolved state of a comment. | ||
| 123 | type ResolveForm struct { | 142 | type ResolveForm struct { | ||
| 124 | Action string | 143 | Action string | ||
| @@ -135,9 +154,10 @@ type CommentForm struct { | |||||
| 135 | Action string | 154 | Action string | ||
| 136 | Title string | 155 | Title string | ||
| 137 | // Path, Line and Side designate the line; empty for a general comment. | 156 | // Path, Line and Side designate the line; empty for a general comment. | ||
| 138 | - | Path, Line, Side string | 157 | + | Path, Line, Side string |
| 139 | - | Cancel string | 158 | + | StartLine, ReplyTo string |
| 140 | - | Focus bool | 159 | + | Cancel string |
| 160 | + | Focus bool | |||
| 141 | // From is the window of the page the form is on (see mrWindow), sent | 161 | // From is the window of the page the form is on (see mrWindow), sent | ||
| 142 | // back so that the answer returns to it; 0 for the first. | 162 | // back so that the answer returns to it; 0 for the first. | ||
| 143 | From int | 163 | From int | ||
| @@ -192,6 +212,7 @@ type MRPage struct { | |||||
| 192 | State string | 212 | State string | ||
| 193 | Open bool | 213 | Open bool | ||
| 194 | Signed bool | 214 | Signed bool | ||
| 215 | + | NoteAuthor string | |||
| 195 | Source, Target string | 216 | Source, Target string | ||
| 196 | SourceHref string | 217 | SourceHref string | ||
| 197 | TargetHref string | 218 | TargetHref string | ||
| @@ -226,6 +247,7 @@ type MRPage struct { | |||||
| 226 | NComments int | 247 | NComments int | ||
| 227 | NUnresolved int | 248 | NUnresolved int | ||
| 228 | GeneralForm *CommentForm | 249 | GeneralForm *CommentForm | ||
| 250 | + | Verdicts []VerdictView | |||
| 229 | 251 | ||||
| 230 | Commits []MRCommit | 252 | Commits []MRCommit | ||
| 231 | CommitsNote string | 253 | CommitsNote string | ||
| @@ -471,7 +493,7 @@ func (s *Server) mrPage(ctx context.Context, r *http.Request, since bool) (templ | |||||
| 471 | 493 | ||||
| 472 | p := MRPage{ | 494 | p := MRPage{ | ||
| 473 | Number: "#" + strconv.FormatInt(n, 10), Title: visibleText(m.Title.String()), | 495 | Number: "#" + strconv.FormatInt(n, 10), Title: visibleText(m.Title.String()), | ||
| 474 | - | State: mrStateName(m), Open: open, Signed: signed, | 496 | + | State: mrStateName(m), Open: open, Signed: signed, NoteAuthor: domain.AuthorName(principal(ctx)), |
| 475 | Source: visibleText(m.Source), Target: visibleText(m.Target), | 497 | Source: visibleText(m.Source), Target: visibleText(m.Target), | ||
| 476 | Author: s.who(m.Author), | 498 | Author: s.who(m.Author), | ||
| 477 | Head: head, HeadShort: short(head), | 499 | Head: head, HeadShort: short(head), | ||
| @@ -485,12 +507,15 @@ func (s *Server) mrPage(ctx context.Context, r *http.Request, since bool) (templ | |||||
| 485 | // Relative links point into the source branch as it is now (its head | 507 | // Relative links point into the source branch as it is now (its head | ||
| 486 | // commit); the heading ids carry their own prefix, apart from the | 508 | // commit); the heading ids carry their own prefix, apart from the | ||
| 487 | // ids of the diff. | 509 | // ids of the diff. | ||
| 488 | - | h, _, err := s.md.renderWithIDs([]byte(desc), d.name(), head, "", "mr-desc-") | 510 | + | h, err := s.md.renderNote([]byte(desc), d.name(), head, "mr-desc-", noteIDs(res.Comments)) |
| 489 | if err != nil { | 511 | if err != nil { | ||
| 490 | return nil, 0, err | 512 | return nil, 0, err | ||
| 491 | } | 513 | } | ||
| 492 | p.Desc = h | 514 | p.Desc = h | ||
| 493 | } | 515 | } | ||
| 516 | + | for _, v := range res.Verdicts { | |||
| 517 | + | p.Verdicts = append(p.Verdicts, VerdictView{Author: s.who(v.Author), Label: v.Verdict.String(), Stale: v.Stale, Href: "#c" + strconv.FormatInt(v.CommentID, 10), Short: short(v.CommitID.String())}) | |||
| 518 | + | } | |||
| 494 | p.Opened.Name = p.Author | 519 | p.Opened.Name = p.Author | ||
| 495 | p.Opened.When, p.Opened.WhenAt = whenOf(now, m.CreatedAt) | 520 | p.Opened.When, p.Opened.WhenAt = whenOf(now, m.CreatedAt) | ||
| 496 | if !m.CreatedAt.IsZero() { | 521 | if !m.CreatedAt.IsZero() { | ||
| @@ -684,9 +709,10 @@ func shellWord(s string) string { | |||||
| 684 | // lineRef designates a line of the diff in the query of the comment form: | 709 | // lineRef designates a line of the diff in the query of the comment form: | ||
| 685 | // "?on=new:12:path/to/file". | 710 | // "?on=new:12:path/to/file". | ||
| 686 | type lineRef struct { | 711 | type lineRef struct { | ||
| 687 | - | side string | 712 | + | side string |
| 688 | - | line int | 713 | + | line int |
| 689 | - | path string | 714 | + | start int |
| 715 | + | path string | |||
| 690 | } | 716 | } | ||
| 691 | 717 | ||||
| 692 | func parseOn(s string) *lineRef { | 718 | func parseOn(s string) *lineRef { | ||
| @@ -695,11 +721,21 @@ func parseOn(s string) *lineRef { | |||||
| 695 | return nil | 721 | return nil | ||
| 696 | } | 722 | } | ||
| 697 | num, path, ok := strings.Cut(rest, ":") | 723 | num, path, ok := strings.Cut(rest, ":") | ||
| 724 | + | start := 0 | |||
| 725 | + | if a, b, rangeOK := strings.Cut(num, "-"); rangeOK { | |||
| 726 | + | var e error | |||
| 727 | + | start, e = strconv.Atoi(a) | |||
| 728 | + | end, e2 := strconv.Atoi(b) | |||
| 729 | + | if e != nil || e2 != nil || start < 1 || start >= end { | |||
| 730 | + | return nil | |||
| 731 | + | } | |||
| 732 | + | num = b | |||
| 733 | + | } | |||
| 698 | line, err := strconv.Atoi(num) | 734 | line, err := strconv.Atoi(num) | ||
| 699 | if !ok || err != nil || line < 1 || path == "" { | 735 | if !ok || err != nil || line < 1 || path == "" { | ||
| 700 | return nil | 736 | return nil | ||
| 701 | } | 737 | } | ||
| 702 | - | return &lineRef{side: side, line: line, path: path} | 738 | + | return &lineRef{side: side, line: line, start: start, path: path} |
| 703 | } | 739 | } | ||
| 704 | 740 | ||||
| 705 | // placeComments puts every comment under its line when the diff shown has it, | 741 | // placeComments puts every comment under its line when the diff shown has it, | ||
| @@ -757,21 +793,62 @@ func (s *Server) placeComments(p *MRPage, repo string, n int64, comments []domai | |||||
| 757 | return index[lineKey{path, side, line}] | 793 | return index[lineKey{path, side, line}] | ||
| 758 | } | 794 | } | ||
| 759 | 795 | ||||
| 796 | + | exists := noteIDs(comments) | |||
| 797 | + | children := map[int64][]domain.Comment{} | |||
| 760 | for _, c := range comments { | 798 | for _, c := range comments { | ||
| 799 | + | if c.ReplyTo != 0 { | |||
| 800 | + | children[c.ReplyTo] = append(children[c.ReplyTo], c) | |||
| 801 | + | } | |||
| 802 | + | if !c.Deleted { | |||
| 803 | + | p.NComments++ | |||
| 804 | + | } | |||
| 805 | + | if c.ReplyTo == 0 && !c.Deleted && !c.Resolved { | |||
| 806 | + | p.NUnresolved++ | |||
| 807 | + | } | |||
| 808 | + | } | |||
| 809 | + | var view func(domain.Comment) CommentView | |||
| 810 | + | view = func(c domain.Comment) CommentView { | |||
| 761 | cid := strconv.FormatInt(c.ID, 10) | 811 | cid := strconv.FormatInt(c.ID, 10) | ||
| 762 | - | v := CommentView{ | 812 | + | body, err := s.md.renderNote([]byte(c.Text.String()), repo, head, "c"+cid+"-", exists) |
| 763 | - | ID: cid, Author: s.who(c.Author), | 813 | + | if err != nil { |
| 764 | - | Parts: plainParts(c.Text.String()), Resolved: c.Resolved, | 814 | + | body = "" |
| 765 | - | Outdated: !c.CommitID.IsZero() && c.CommitID.String() != head, | |||
| 766 | } | 815 | } | ||
| 816 | + | v := CommentView{ID: cid, Author: s.who(c.Author), Body: body, Resolved: c.Resolved, Deleted: c.Deleted, Edited: !c.EditedAt.IsZero(), Verdict: c.Verdict.String(), Outdated: !c.CommitID.IsZero() && c.CommitID.String() != head} | |||
| 767 | v.When, v.WhenAt = whenOf(now, c.CreatedAt) | 817 | v.When, v.WhenAt = whenOf(now, c.CreatedAt) | ||
| 768 | if !c.CreatedAt.IsZero() { | 818 | if !c.CreatedAt.IsZero() { | ||
| 769 | v.Abs = absolute(c.CreatedAt) | 819 | v.Abs = absolute(c.CreatedAt) | ||
| 770 | } | 820 | } | ||
| 771 | - | p.NComments++ | 821 | + | if c.StartLine > 0 { |
| 772 | - | if !c.Resolved { | 822 | + | v.Range = "lines " + itoa(c.StartLine) + "–" + itoa(c.Line) |
| 773 | - | p.NUnresolved++ | |||
| 774 | } | 823 | } | ||
| 824 | + | if canWrite && !c.Deleted { | |||
| 825 | + | from := windowStart(fileAt[c.Path]) | |||
| 826 | + | root := cid | |||
| 827 | + | if c.ReplyTo != 0 { | |||
| 828 | + | root = strconv.FormatInt(c.ReplyTo, 10) | |||
| 829 | + | } | |||
| 830 | + | v.Quote = "> " + strings.ReplaceAll(c.Text.String(), "\n", "\n> ") + "\n\n^" + cid + " " | |||
| 831 | + | if c.ReplyTo == 0 { | |||
| 832 | + | v.Reply = &CommentForm{ID: "reply-" + cid, Action: mrHref(repo, n, "comments"), Title: "Reply", ReplyTo: root, From: from} | |||
| 833 | + | } | |||
| 834 | + | if c.Author == p.NoteAuthor { | |||
| 835 | + | v.Edit = &EditForm{Action: mrHref(repo, n, "comments", cid, "edit"), Text: c.Text.String(), From: from} | |||
| 836 | + | v.Delete = &DeleteForm{Action: mrHref(repo, n, "comments", cid, "delete"), From: from} | |||
| 837 | + | } | |||
| 838 | + | } | |||
| 839 | + | for _, reply := range children[c.ID] { | |||
| 840 | + | if !reply.Deleted { | |||
| 841 | + | v.Replies = append(v.Replies, view(reply)) | |||
| 842 | + | } | |||
| 843 | + | } | |||
| 844 | + | return v | |||
| 845 | + | } | |||
| 846 | + | for _, c := range comments { | |||
| 847 | + | if c.ReplyTo != 0 || c.Deleted && len(children[c.ID]) == 0 { | |||
| 848 | + | continue | |||
| 849 | + | } | |||
| 850 | + | cid := strconv.FormatInt(c.ID, 10) | |||
| 851 | + | v := view(c) | |||
| 775 | general := c.Path == "" | 852 | general := c.Path == "" | ||
| 776 | fi, listed := fileAt[c.Path] | 853 | fi, listed := fileAt[c.Path] | ||
| 777 | if !general && listed && p.Files[fi].Lazy { | 854 | if !general && listed && p.Files[fi].Lazy { | ||
| @@ -789,7 +866,7 @@ func (s *Server) placeComments(p *MRPage, repo string, n int64, comments []domai | |||||
| 789 | v.Where = visibleText(c.Path) + ":" + itoa(c.Line) + " (" + side + ")" | 866 | v.Where = visibleText(c.Path) + ":" + itoa(c.Line) + " (" + side + ")" | ||
| 790 | } | 867 | } | ||
| 791 | } | 868 | } | ||
| 792 | - | if canWrite { | 869 | + | if canWrite && !c.Deleted { |
| 793 | v.Resolve = &ResolveForm{Action: resolveAction(cid), Value: !c.Resolved, Label: "Resolve"} | 870 | v.Resolve = &ResolveForm{Action: resolveAction(cid), Value: !c.Resolved, Label: "Resolve"} | ||
| 794 | if !general && listed { | 871 | if !general && listed { | ||
| 795 | v.Resolve.From = windowStart(fi) | 872 | v.Resolve.From = windowStart(fi) | ||
| @@ -815,9 +892,18 @@ func (s *Server) placeComments(p *MRPage, repo string, n int64, comments []domai | |||||
| 815 | if on.side == "old" { | 892 | if on.side == "old" { | ||
| 816 | side = domain.SideOld | 893 | side = domain.SideOld | ||
| 817 | } | 894 | } | ||
| 895 | + | start := on.start | |||
| 896 | + | if start == 0 { | |||
| 897 | + | start = on.line | |||
| 898 | + | } | |||
| 899 | + | for key, ln := range index { | |||
| 900 | + | if key.path == on.path && key.side == side && key.line >= start && key.line <= on.line { | |||
| 901 | + | ln.Selected = true | |||
| 902 | + | } | |||
| 903 | + | } | |||
| 818 | if ln := find(on.path, side, on.line); ln != nil { | 904 | if ln := find(on.path, side, on.line); ln != nil { | ||
| 819 | ln.Form = &CommentForm{ | 905 | ln.Form = &CommentForm{ | ||
| 820 | - | ID: "cf-line", Action: mrHref(repo, n, "comments"), Path: on.path, Line: itoa(on.line), Side: on.side, | 906 | + | StartLine: optionalLine(on.start), ID: "cf-line", Action: mrHref(repo, n, "comments"), Path: on.path, Line: itoa(on.line), Side: on.side, |
| 821 | Title: "comment on " + visibleText(on.path) + ":" + itoa(on.line) + " (" + on.side + ")", | 907 | Title: "comment on " + visibleText(on.path) + ":" + itoa(on.line) + " (" + on.side + ")", | ||
| 822 | Cancel: withFrom(self, windowStart(fileAt[on.path])) + "#" + fileIDs[on.path], Focus: true, | 908 | Cancel: withFrom(self, windowStart(fileAt[on.path])) + "#" + fileIDs[on.path], Focus: true, | ||
| 823 | From: windowStart(fileAt[on.path]), | 909 | From: windowStart(fileAt[on.path]), | ||
| @@ -842,3 +928,17 @@ func userText(parts []TextPart) templ.Component { | |||||
| 842 | return err | 928 | return err | ||
| 843 | }) | 929 | }) | ||
| 844 | } | 930 | } | ||
| 931 | + | ||||
| 932 | + | func noteIDs(comments []domain.Comment) func(int64) bool { | |||
| 933 | + | ids := make(map[int64]bool, len(comments)) | |||
| 934 | + | for _, c := range comments { | |||
| 935 | + | ids[c.ID] = true | |||
| 936 | + | } | |||
| 937 | + | return func(n int64) bool { return ids[n] } | |||
| 938 | + | } | |||
| 939 | + | func optionalLine(n int) string { | |||
| 940 | + | if n == 0 { | |||
| 941 | + | return "" | |||
| 942 | + | } | |||
| 943 | + | return itoa(n) | |||
| 944 | + | } | |||
M internal/adapters/web/mr.templ +58 -6
| @@ -229,10 +229,9 @@ templ lineNotes(notes []CommentView, form *CommentForm, suffix string) { | |||
| 229 | 229 | } | |
| 230 | 230 | ||
| 231 | 231 | // commentView is a note: a ruled block, under its line in the diff or in the | |
| 232 | - | // list of comments. The text is plain: escaped, its line breaks kept, its | |
| 233 | - | // http(s) addresses linked. | |
| 232 | + | // list of comments. Body comes only from the safe Markdown renderer. | |
| 234 | 233 | templ commentView(c CommentView, suffix string) { | |
| 235 | - | <div class="cmt" id={ "c" + c.ID + suffix } data-resolved={ boolText(c.Resolved) }> | |
| 234 | + | <div class="cmt" id={ "c" + c.ID + suffix } data-resolved={ boolText(c.Resolved) } data-thread?={ c.Reply != nil }> | |
| 236 | 235 | <p class="c-head"> | |
| 237 | 236 | <span class="c-who">{ c.Author }</span> | |
| 238 | 237 | if c.WhenAt != "" { | |
| @@ -248,7 +247,39 @@ templ commentView(c CommentView, suffix string) { | |||
| 248 | 247 | { " " }<span class="c-where mute">{ c.Where }</span> | |
| 249 | 248 | } | |
| 250 | 249 | </p> | |
| 251 | - | <p class="c-text">@userText(c.Parts)</p> | |
| 250 | + | if c.Deleted { | |
| 251 | + | <p class="mute">deleted note</p> | |
| 252 | + | } else { | |
| 253 | + | <div class="c-body prose"> | |
| 254 | + | @templ.Raw(string(c.Body)) | |
| 255 | + | </div> | |
| 256 | + | } | |
| 257 | + | if c.Edited { | |
| 258 | + | <span class="mute">edited</span> | |
| 259 | + | } | |
| 260 | + | if c.Verdict != "" { | |
| 261 | + | <span class="c-state">{ c.Verdict }</span> | |
| 262 | + | } | |
| 263 | + | if c.Range != "" { | |
| 264 | + | <span class="mute">{ c.Range }</span> | |
| 265 | + | } | |
| 266 | + | if c.Quote != "" { | |
| 267 | + | <button type="button" class="btn" data-js data-quote={ c.Quote }>Quote</button> | |
| 268 | + | } | |
| 269 | + | if c.Edit != nil { | |
| 270 | + | <details><summary>Edit</summary><form method="post" action={ templ.SafeURL(c.Edit.Action) }><input type="hidden" name="from" value={ itoa(c.Edit.From) }/><textarea name="text" maxlength="8000">{ c.Edit.Text }</textarea><button class="btn" type="submit">Save</button></form></details> | |
| 271 | + | } | |
| 272 | + | if c.Delete != nil { | |
| 273 | + | <details><summary>Delete</summary><form method="post" action={ templ.SafeURL(c.Delete.Action) }><input type="hidden" name="from" value={ itoa(c.Delete.From) }/><p>Delete this note?</p><button class="btn" type="submit">Confirm delete</button></form></details> | |
| 274 | + | } | |
| 275 | + | for _, reply := range c.Replies { | |
| 276 | + | @commentView(reply, suffix) | |
| 277 | + | } | |
| 278 | + | if c.Reply != nil { | |
| 279 | + | <details data-reply> | |
| 280 | + | <summary>Reply</summary>@commentForm(*c.Reply, suffix) | |
| 281 | + | </details> | |
| 282 | + | } | |
| 252 | 283 | if c.Resolve != nil { | |
| 253 | 284 | <form class="c-act" method="post" action={ templ.SafeURL(c.Resolve.Action) }> | |
| 254 | 285 | <input type="hidden" name="resolved" value={ boolText(c.Resolve.Value) }/> | |
| @@ -277,8 +308,7 @@ templ commentForm(f CommentForm, suffix string) { | |||
| 277 | 308 | name="text" | |
| 278 | 309 | rows="3" | |
| 279 | 310 | maxlength="8000" | |
| 280 | - | required | |
| 281 | - | placeholder="plain text, 8000 characters at most" | |
| 311 | + | placeholder="Markdown, 8000 characters at most" | |
| 282 | 312 | if f.Focus { | |
| 283 | 313 | autofocus | |
| 284 | 314 | } | |
| @@ -287,6 +317,13 @@ templ commentForm(f CommentForm, suffix string) { | |||
| 287 | 317 | <input type="hidden" name="path" value={ f.Path }/> | |
| 288 | 318 | <input type="hidden" name="line" value={ f.Line }/> | |
| 289 | 319 | <input type="hidden" name="side" value={ f.Side }/> | |
| 320 | + | <label>from line <input type="number" name="start_line" min="1" value={ f.StartLine }/></label> | |
| 321 | + | } | |
| 322 | + | if f.ReplyTo != "" { | |
| 323 | + | <input type="hidden" name="reply_to" value={ f.ReplyTo }/> | |
| 324 | + | } | |
| 325 | + | if f.Path == "" && f.ReplyTo == "" { | |
| 326 | + | <fieldset><legend>Verdict</legend><label><input type="radio" name="verdict" value="" checked/>Note</label><label><input type="radio" name="verdict" value="approve"/>Approve</label><label><input type="radio" name="verdict" value="changes_requested"/>Request changes</label></fieldset> | |
| 290 | 327 | } | |
| 291 | 328 | if f.From > 0 { | |
| 292 | 329 | <input type="hidden" name="from" value={ itoa(f.From) }/> | |
| @@ -310,6 +347,21 @@ templ mrRail(p MRPage) { | |||
| 310 | 347 | </h2> | |
| 311 | 348 | @FileList(p.Files) | |
| 312 | 349 | </section> | |
| 350 | + | if len(p.Verdicts) > 0 { | |
| 351 | + | <section> | |
| 352 | + | <h2>Verdicts</h2> | |
| 353 | + | <ul> | |
| 354 | + | for _, v := range p.Verdicts { | |
| 355 | + | <li> | |
| 356 | + | <a href={ templ.SafeURL(v.Href) }>{ v.Author }: { v.Label }</a> | |
| 357 | + | if v.Stale { | |
| 358 | + | <span class="mute">(stale)</span> | |
| 359 | + | } | |
| 360 | + | </li> | |
| 361 | + | } | |
| 362 | + | </ul> | |
| 363 | + | </section> | |
| 364 | + | } | |
| 313 | 365 | @MRChecks(p.Checks) | |
| 314 | 366 | <section aria-labelledby="r-commits"> | |
| 315 | 367 | <h2 id="r-commits">Commits <span class="count">{ itoa(len(p.Commits)) }</span></h2> | |
| @@ -229,10 +229,9 @@ templ lineNotes(notes []CommentView, form *CommentForm, suffix string) { | |||||
| 229 | } | 229 | } | ||
| 230 | 230 | ||||
| 231 | // commentView is a note: a ruled block, under its line in the diff or in the | 231 | // commentView is a note: a ruled block, under its line in the diff or in the | ||
| 232 | - | // list of comments. The text is plain: escaped, its line breaks kept, its | 232 | + | // list of comments. Body comes only from the safe Markdown renderer. |
| 233 | - | // http(s) addresses linked. | |||
| 234 | templ commentView(c CommentView, suffix string) { | 233 | templ commentView(c CommentView, suffix string) { | ||
| 235 | - | <div class="cmt" id={ "c" + c.ID + suffix } data-resolved={ boolText(c.Resolved) }> | 234 | + | <div class="cmt" id={ "c" + c.ID + suffix } data-resolved={ boolText(c.Resolved) } data-thread?={ c.Reply != nil }> |
| 236 | <p class="c-head"> | 235 | <p class="c-head"> | ||
| 237 | <span class="c-who">{ c.Author }</span> | 236 | <span class="c-who">{ c.Author }</span> | ||
| 238 | if c.WhenAt != "" { | 237 | if c.WhenAt != "" { | ||
| @@ -248,7 +247,39 @@ templ commentView(c CommentView, suffix string) { | |||||
| 248 | { " " }<span class="c-where mute">{ c.Where }</span> | 247 | { " " }<span class="c-where mute">{ c.Where }</span> | ||
| 249 | } | 248 | } | ||
| 250 | </p> | 249 | </p> | ||
| 251 | - | <p class="c-text">@userText(c.Parts)</p> | 250 | + | if c.Deleted { |
| 251 | + | <p class="mute">deleted note</p> | |||
| 252 | + | } else { | |||
| 253 | + | <div class="c-body prose"> | |||
| 254 | + | @templ.Raw(string(c.Body)) | |||
| 255 | + | </div> | |||
| 256 | + | } | |||
| 257 | + | if c.Edited { | |||
| 258 | + | <span class="mute">edited</span> | |||
| 259 | + | } | |||
| 260 | + | if c.Verdict != "" { | |||
| 261 | + | <span class="c-state">{ c.Verdict }</span> | |||
| 262 | + | } | |||
| 263 | + | if c.Range != "" { | |||
| 264 | + | <span class="mute">{ c.Range }</span> | |||
| 265 | + | } | |||
| 266 | + | if c.Quote != "" { | |||
| 267 | + | <button type="button" class="btn" data-js data-quote={ c.Quote }>Quote</button> | |||
| 268 | + | } | |||
| 269 | + | if c.Edit != nil { | |||
| 270 | + | <details><summary>Edit</summary><form method="post" action={ templ.SafeURL(c.Edit.Action) }><input type="hidden" name="from" value={ itoa(c.Edit.From) }/><textarea name="text" maxlength="8000">{ c.Edit.Text }</textarea><button class="btn" type="submit">Save</button></form></details> | |||
| 271 | + | } | |||
| 272 | + | if c.Delete != nil { | |||
| 273 | + | <details><summary>Delete</summary><form method="post" action={ templ.SafeURL(c.Delete.Action) }><input type="hidden" name="from" value={ itoa(c.Delete.From) }/><p>Delete this note?</p><button class="btn" type="submit">Confirm delete</button></form></details> | |||
| 274 | + | } | |||
| 275 | + | for _, reply := range c.Replies { | |||
| 276 | + | @commentView(reply, suffix) | |||
| 277 | + | } | |||
| 278 | + | if c.Reply != nil { | |||
| 279 | + | <details data-reply> | |||
| 280 | + | <summary>Reply</summary>@commentForm(*c.Reply, suffix) | |||
| 281 | + | </details> | |||
| 282 | + | } | |||
| 252 | if c.Resolve != nil { | 283 | if c.Resolve != nil { | ||
| 253 | <form class="c-act" method="post" action={ templ.SafeURL(c.Resolve.Action) }> | 284 | <form class="c-act" method="post" action={ templ.SafeURL(c.Resolve.Action) }> | ||
| 254 | <input type="hidden" name="resolved" value={ boolText(c.Resolve.Value) }/> | 285 | <input type="hidden" name="resolved" value={ boolText(c.Resolve.Value) }/> | ||
| @@ -277,8 +308,7 @@ templ commentForm(f CommentForm, suffix string) { | |||||
| 277 | name="text" | 308 | name="text" | ||
| 278 | rows="3" | 309 | rows="3" | ||
| 279 | maxlength="8000" | 310 | maxlength="8000" | ||
| 280 | - | required | 311 | + | placeholder="Markdown, 8000 characters at most" |
| 281 | - | placeholder="plain text, 8000 characters at most" | |||
| 282 | if f.Focus { | 312 | if f.Focus { | ||
| 283 | autofocus | 313 | autofocus | ||
| 284 | } | 314 | } | ||
| @@ -287,6 +317,13 @@ templ commentForm(f CommentForm, suffix string) { | |||||
| 287 | <input type="hidden" name="path" value={ f.Path }/> | 317 | <input type="hidden" name="path" value={ f.Path }/> | ||
| 288 | <input type="hidden" name="line" value={ f.Line }/> | 318 | <input type="hidden" name="line" value={ f.Line }/> | ||
| 289 | <input type="hidden" name="side" value={ f.Side }/> | 319 | <input type="hidden" name="side" value={ f.Side }/> | ||
| 320 | + | <label>from line <input type="number" name="start_line" min="1" value={ f.StartLine }/></label> | |||
| 321 | + | } | |||
| 322 | + | if f.ReplyTo != "" { | |||
| 323 | + | <input type="hidden" name="reply_to" value={ f.ReplyTo }/> | |||
| 324 | + | } | |||
| 325 | + | if f.Path == "" && f.ReplyTo == "" { | |||
| 326 | + | <fieldset><legend>Verdict</legend><label><input type="radio" name="verdict" value="" checked/>Note</label><label><input type="radio" name="verdict" value="approve"/>Approve</label><label><input type="radio" name="verdict" value="changes_requested"/>Request changes</label></fieldset> | |||
| 290 | } | 327 | } | ||
| 291 | if f.From > 0 { | 328 | if f.From > 0 { | ||
| 292 | <input type="hidden" name="from" value={ itoa(f.From) }/> | 329 | <input type="hidden" name="from" value={ itoa(f.From) }/> | ||
| @@ -310,6 +347,21 @@ templ mrRail(p MRPage) { | |||||
| 310 | </h2> | 347 | </h2> | ||
| 311 | @FileList(p.Files) | 348 | @FileList(p.Files) | ||
| 312 | </section> | 349 | </section> | ||
| 350 | + | if len(p.Verdicts) > 0 { | |||
| 351 | + | <section> | |||
| 352 | + | <h2>Verdicts</h2> | |||
| 353 | + | <ul> | |||
| 354 | + | for _, v := range p.Verdicts { | |||
| 355 | + | <li> | |||
| 356 | + | <a href={ templ.SafeURL(v.Href) }>{ v.Author }: { v.Label }</a> | |||
| 357 | + | if v.Stale { | |||
| 358 | + | <span class="mute">(stale)</span> | |||
| 359 | + | } | |||
| 360 | + | </li> | |||
| 361 | + | } | |||
| 362 | + | </ul> | |||
| 363 | + | </section> | |||
| 364 | + | } | |||
| 313 | @MRChecks(p.Checks) | 365 | @MRChecks(p.Checks) | ||
| 314 | <section aria-labelledby="r-commits"> | 366 | <section aria-labelledby="r-commits"> | ||
| 315 | <h2 id="r-commits">Commits <span class="count">{ itoa(len(p.Commits)) }</span></h2> | 367 | <h2 id="r-commits">Commits <span class="count">{ itoa(len(p.Commits)) }</span></h2> | ||
M internal/adapters/web/mr_files.go +5 -3
| @@ -131,8 +131,10 @@ func countNotes(files []DiffFile, comments []domain.Comment) { | |||
| 131 | 131 | if c.Path == "" || !ok { | |
| 132 | 132 | continue | |
| 133 | 133 | } | |
| 134 | - | files[i].Notes++ | |
| 135 | - | if !c.Resolved { | |
| 134 | + | if c.ReplyTo == 0 && !c.Deleted { | |
| 135 | + | files[i].Notes++ | |
| 136 | + | } | |
| 137 | + | if c.ReplyTo == 0 && !c.Deleted && !c.Resolved { | |
| 136 | 138 | files[i].OpenNotes++ | |
| 137 | 139 | } | |
| 138 | 140 | ids[i] = append(ids[i], "c"+strconv.FormatInt(c.ID, 10)) | |
| @@ -196,7 +198,7 @@ func (s *Server) mrFiles(ctx context.Context, r *http.Request) (templ.Component, | |||
| 196 | 198 | m := res.MergeRequest | |
| 197 | 199 | signed := SignedIn(ctx) | |
| 198 | 200 | canWrite := signed && m.State == domain.MRStateOpen && !since | |
| 199 | - | p := MRPage{Files: s.hl.files(ctx, res.Diff.Files), LooseInFiles: true} | |
| 201 | + | p := MRPage{Files: s.hl.files(ctx, res.Diff.Files), LooseInFiles: true, NoteAuthor: domain.AuthorName(principal(ctx))} | |
| 200 | 202 | contextGaps(p.Files, res.Diff.Files, repoHref(d.name(), "context", head.String())) | |
| 201 | 203 | s.placeComments(&p, d.name(), n, res.Comments, m.HeadID.String(), !since, canWrite, nil) | |
| 202 | 204 | window := p.Files[from:min(from+mrWindow, len(p.Files))] | |
| @@ -131,8 +131,10 @@ func countNotes(files []DiffFile, comments []domain.Comment) { | |||||
| 131 | if c.Path == "" || !ok { | 131 | if c.Path == "" || !ok { | ||
| 132 | continue | 132 | continue | ||
| 133 | } | 133 | } | ||
| 134 | - | files[i].Notes++ | 134 | + | if c.ReplyTo == 0 && !c.Deleted { |
| 135 | - | if !c.Resolved { | 135 | + | files[i].Notes++ |
| 136 | + | } | |||
| 137 | + | if c.ReplyTo == 0 && !c.Deleted && !c.Resolved { | |||
| 136 | files[i].OpenNotes++ | 138 | files[i].OpenNotes++ | ||
| 137 | } | 139 | } | ||
| 138 | ids[i] = append(ids[i], "c"+strconv.FormatInt(c.ID, 10)) | 140 | ids[i] = append(ids[i], "c"+strconv.FormatInt(c.ID, 10)) | ||
| @@ -196,7 +198,7 @@ func (s *Server) mrFiles(ctx context.Context, r *http.Request) (templ.Component, | |||||
| 196 | m := res.MergeRequest | 198 | m := res.MergeRequest | ||
| 197 | signed := SignedIn(ctx) | 199 | signed := SignedIn(ctx) | ||
| 198 | canWrite := signed && m.State == domain.MRStateOpen && !since | 200 | canWrite := signed && m.State == domain.MRStateOpen && !since | ||
| 199 | - | p := MRPage{Files: s.hl.files(ctx, res.Diff.Files), LooseInFiles: true} | 201 | + | p := MRPage{Files: s.hl.files(ctx, res.Diff.Files), LooseInFiles: true, NoteAuthor: domain.AuthorName(principal(ctx))} |
| 200 | contextGaps(p.Files, res.Diff.Files, repoHref(d.name(), "context", head.String())) | 202 | contextGaps(p.Files, res.Diff.Files, repoHref(d.name(), "context", head.String())) | ||
| 201 | s.placeComments(&p, d.name(), n, res.Comments, m.HeadID.String(), !since, canWrite, nil) | 203 | s.placeComments(&p, d.name(), n, res.Comments, m.HeadID.String(), !since, canWrite, nil) | ||
| 202 | window := p.Files[from:min(from+mrWindow, len(p.Files))] | 204 | window := p.Files[from:min(from+mrWindow, len(p.Files))] | ||
A internal/adapters/web/mr_notes_test.go +112 -0
| @@ -0,0 +1,112 @@ | |||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | + | ||
| 3 | + | package web | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "bnema.dev/ermite/internal/ports/mocks" | |
| 7 | + | "bytes" | |
| 8 | + | "context" | |
| 9 | + | "github.com/stretchr/testify/mock" | |
| 10 | + | "net/http" | |
| 11 | + | "net/http/httptest" | |
| 12 | + | "net/url" | |
| 13 | + | "strings" | |
| 14 | + | "testing" | |
| 15 | + | "time" | |
| 16 | + | ||
| 17 | + | "bnema.dev/ermite/internal/domain" | |
| 18 | + | "bnema.dev/ermite/internal/ports" | |
| 19 | + | "github.com/stretchr/testify/assert" | |
| 20 | + | "github.com/stretchr/testify/require" | |
| 21 | + | ) | |
| 22 | + | ||
| 23 | + | func TestParseOnRange(t *testing.T) { | |
| 24 | + | t.Parallel() | |
| 25 | + | for _, raw := range []string{"new:0-2:a", "new:2-2:a", "new:3-2:a", "new:1-x:a", "new:1-2:", "other:1-2:a"} { | |
| 26 | + | assert.Nil(t, parseOn(raw), raw) | |
| 27 | + | } | |
| 28 | + | assert.Equal(t, &lineRef{side: "new", start: 10, line: 24, path: "dir:a.go"}, parseOn("new:10-24:dir:a.go")) | |
| 29 | + | assert.Equal(t, &lineRef{side: "old", line: 10, path: "a"}, parseOn("old:10:a")) | |
| 30 | + | } | |
| 31 | + | ||
| 32 | + | func TestNoteThreads(t *testing.T) { | |
| 33 | + | t.Parallel() | |
| 34 | + | text, err := domain.ParsePlainText("**hello** ^2") | |
| 35 | + | require.NoError(t, err) | |
| 36 | + | s := &Server{cfg: Config{Now: time.Now}, md: newMarkdown()} | |
| 37 | + | comments := []domain.Comment{ | |
| 38 | + | {ID: 1, Author: "alice", Text: text}, | |
| 39 | + | {ID: 2, Author: "bob", ReplyTo: 1, Text: text}, | |
| 40 | + | {ID: 3, Author: "alice", Deleted: true}, | |
| 41 | + | {ID: 4, Author: "alice", Deleted: true}, | |
| 42 | + | {ID: 5, Author: "bob", ReplyTo: 4, Text: text}, | |
| 43 | + | {ID: 6, Author: "alice", Text: text, Verdict: domain.VerdictApprove, Resolved: true}, | |
| 44 | + | } | |
| 45 | + | p := MRPage{NoteAuthor: "alice"} | |
| 46 | + | s.placeComments(&p, "demo", 1, comments, "", true, true, nil) | |
| 47 | + | require.Len(t, p.General, 3) | |
| 48 | + | assert.Equal(t, 4, p.NComments) | |
| 49 | + | assert.Equal(t, 1, p.NUnresolved) | |
| 50 | + | require.Len(t, p.General[0].Replies, 1) | |
| 51 | + | assert.NotNil(t, p.General[0].Edit) | |
| 52 | + | assert.Nil(t, p.General[0].Replies[0].Edit) | |
| 53 | + | assert.Nil(t, p.General[0].Replies[0].Resolve) | |
| 54 | + | assert.Contains(t, string(p.General[0].Body), "<strong>hello</strong>") | |
| 55 | + | assert.Contains(t, string(p.General[0].Body), `href="#c2"`) | |
| 56 | + | assert.True(t, p.General[1].Deleted) | |
| 57 | + | var b bytes.Buffer | |
| 58 | + | require.NoError(t, commentView(p.General[0], "").Render(context.Background(), &b)) | |
| 59 | + | assert.Contains(t, b.String(), `name="reply_to" value="1"`) | |
| 60 | + | assert.Contains(t, b.String(), `data-js data-quote=`) | |
| 61 | + | assert.Contains(t, b.String(), `/comments/1/edit`) | |
| 62 | + | assert.NotContains(t, b.String(), `/comments/2/edit`) | |
| 63 | + | } | |
| 64 | + | ||
| 65 | + | func TestNoteLiveEvents(t *testing.T) { | |
| 66 | + | t.Parallel() | |
| 67 | + | l := liveStream{view: "mr", n: "1"} | |
| 68 | + | events := []domain.Event{domain.CommentEdited{Number: 1}, domain.CommentDeleted{Number: 1}, domain.CommentResolved{Number: 1}} | |
| 69 | + | for _, ev := range events { | |
| 70 | + | we := ports.WatchEvent{Event: ev} | |
| 71 | + | assert.True(t, liveEvent(we)) | |
| 72 | + | fs, err := l.repoPage(context.Background(), we) | |
| 73 | + | require.NoError(t, err) | |
| 74 | + | require.Len(t, fs, 1) | |
| 75 | + | assert.Contains(t, fs[0].html, "reload") | |
| 76 | + | } | |
| 77 | + | } | |
| 78 | + | ||
| 79 | + | func TestNotePosts(t *testing.T) { | |
| 80 | + | t.Parallel() | |
| 81 | + | svc := mocks.NewMergeRequestService(t) | |
| 82 | + | repo := repoName(t, "demo") | |
| 83 | + | s := &Server{cfg: Config{MergeRequests: svc}} | |
| 84 | + | ctx := domain.ContextWithPrincipal(context.Background(), domain.Owner()) | |
| 85 | + | request := func(values url.Values) *http.Request { | |
| 86 | + | r := httptest.NewRequest("POST", "/demo/merge-requests/1/comments", strings.NewReader(values.Encode())).WithContext(ctx) | |
| 87 | + | r.Header.Set("Content-Type", "application/x-www-form-urlencoded") | |
| 88 | + | r.SetPathValue("repo", "demo") | |
| 89 | + | r.SetPathValue("n", "1") | |
| 90 | + | r.SetPathValue("id", "2") | |
| 91 | + | require.NoError(t, r.ParseForm()) | |
| 92 | + | return r | |
| 93 | + | } | |
| 94 | + | svc.EXPECT().AddComment(mock.Anything, mock.Anything, repo, int64(1), mock.MatchedBy(func(p ports.AddCommentParams) bool { return p.ReplyTo == 2 && p.Text == "reply" })).Return(domain.Comment{ID: 7}, nil).Once() | |
| 95 | + | to, _, err := s.mrComment(ctx, request(url.Values{"reply_to": {"2"}, "text": {"reply"}})) | |
| 96 | + | require.NoError(t, err) | |
| 97 | + | assert.Equal(t, "/demo/merge-requests/1#c7", to) | |
| 98 | + | svc.EXPECT().AddComment(mock.Anything, mock.Anything, repo, int64(1), mock.MatchedBy(func(p ports.AddCommentParams) bool { | |
| 99 | + | return p.StartLine == 10 && p.Line == 24 && p.Path == "a.go" && p.Verdict == domain.VerdictApprove | |
| 100 | + | })).Return(domain.Comment{ID: 8}, nil).Once() | |
| 101 | + | to, _, err = s.mrComment(ctx, request(url.Values{"path": {"a.go"}, "line": {"24"}, "start_line": {"10"}, "side": {"new"}, "verdict": {"approve"}})) | |
| 102 | + | require.NoError(t, err) | |
| 103 | + | assert.Equal(t, "/demo/merge-requests/1#c8", to) | |
| 104 | + | svc.EXPECT().EditComment(mock.Anything, mock.Anything, repo, int64(1), int64(2), "new\ntext").Return(domain.Comment{ID: 2}, nil).Once() | |
| 105 | + | to, _, err = s.mrEditComment(ctx, request(url.Values{"text": {"new\r\ntext"}})) | |
| 106 | + | require.NoError(t, err) | |
| 107 | + | assert.Equal(t, "/demo/merge-requests/1#c2", to) | |
| 108 | + | svc.EXPECT().DeleteComment(mock.Anything, mock.Anything, repo, int64(1), int64(2)).Return(domain.Comment{ID: 2}, nil).Once() | |
| 109 | + | to, _, err = s.mrDeleteComment(ctx, request(nil)) | |
| 110 | + | require.NoError(t, err) | |
| 111 | + | assert.Equal(t, "/demo/merge-requests/1#c2", to) | |
| 112 | + | } | |
| @@ -0,0 +1,112 @@ | |||||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |||
| 2 | + | ||||
| 3 | + | package web | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "bnema.dev/ermite/internal/ports/mocks" | |||
| 7 | + | "bytes" | |||
| 8 | + | "context" | |||
| 9 | + | "github.com/stretchr/testify/mock" | |||
| 10 | + | "net/http" | |||
| 11 | + | "net/http/httptest" | |||
| 12 | + | "net/url" | |||
| 13 | + | "strings" | |||
| 14 | + | "testing" | |||
| 15 | + | "time" | |||
| 16 | + | ||||
| 17 | + | "bnema.dev/ermite/internal/domain" | |||
| 18 | + | "bnema.dev/ermite/internal/ports" | |||
| 19 | + | "github.com/stretchr/testify/assert" | |||
| 20 | + | "github.com/stretchr/testify/require" | |||
| 21 | + | ) | |||
| 22 | + | ||||
| 23 | + | func TestParseOnRange(t *testing.T) { | |||
| 24 | + | t.Parallel() | |||
| 25 | + | for _, raw := range []string{"new:0-2:a", "new:2-2:a", "new:3-2:a", "new:1-x:a", "new:1-2:", "other:1-2:a"} { | |||
| 26 | + | assert.Nil(t, parseOn(raw), raw) | |||
| 27 | + | } | |||
| 28 | + | assert.Equal(t, &lineRef{side: "new", start: 10, line: 24, path: "dir:a.go"}, parseOn("new:10-24:dir:a.go")) | |||
| 29 | + | assert.Equal(t, &lineRef{side: "old", line: 10, path: "a"}, parseOn("old:10:a")) | |||
| 30 | + | } | |||
| 31 | + | ||||
| 32 | + | func TestNoteThreads(t *testing.T) { | |||
| 33 | + | t.Parallel() | |||
| 34 | + | text, err := domain.ParsePlainText("**hello** ^2") | |||
| 35 | + | require.NoError(t, err) | |||
| 36 | + | s := &Server{cfg: Config{Now: time.Now}, md: newMarkdown()} | |||
| 37 | + | comments := []domain.Comment{ | |||
| 38 | + | {ID: 1, Author: "alice", Text: text}, | |||
| 39 | + | {ID: 2, Author: "bob", ReplyTo: 1, Text: text}, | |||
| 40 | + | {ID: 3, Author: "alice", Deleted: true}, | |||
| 41 | + | {ID: 4, Author: "alice", Deleted: true}, | |||
| 42 | + | {ID: 5, Author: "bob", ReplyTo: 4, Text: text}, | |||
| 43 | + | {ID: 6, Author: "alice", Text: text, Verdict: domain.VerdictApprove, Resolved: true}, | |||
| 44 | + | } | |||
| 45 | + | p := MRPage{NoteAuthor: "alice"} | |||
| 46 | + | s.placeComments(&p, "demo", 1, comments, "", true, true, nil) | |||
| 47 | + | require.Len(t, p.General, 3) | |||
| 48 | + | assert.Equal(t, 4, p.NComments) | |||
| 49 | + | assert.Equal(t, 1, p.NUnresolved) | |||
| 50 | + | require.Len(t, p.General[0].Replies, 1) | |||
| 51 | + | assert.NotNil(t, p.General[0].Edit) | |||
| 52 | + | assert.Nil(t, p.General[0].Replies[0].Edit) | |||
| 53 | + | assert.Nil(t, p.General[0].Replies[0].Resolve) | |||
| 54 | + | assert.Contains(t, string(p.General[0].Body), "<strong>hello</strong>") | |||
| 55 | + | assert.Contains(t, string(p.General[0].Body), `href="#c2"`) | |||
| 56 | + | assert.True(t, p.General[1].Deleted) | |||
| 57 | + | var b bytes.Buffer | |||
| 58 | + | require.NoError(t, commentView(p.General[0], "").Render(context.Background(), &b)) | |||
| 59 | + | assert.Contains(t, b.String(), `name="reply_to" value="1"`) | |||
| 60 | + | assert.Contains(t, b.String(), `data-js data-quote=`) | |||
| 61 | + | assert.Contains(t, b.String(), `/comments/1/edit`) | |||
| 62 | + | assert.NotContains(t, b.String(), `/comments/2/edit`) | |||
| 63 | + | } | |||
| 64 | + | ||||
| 65 | + | func TestNoteLiveEvents(t *testing.T) { | |||
| 66 | + | t.Parallel() | |||
| 67 | + | l := liveStream{view: "mr", n: "1"} | |||
| 68 | + | events := []domain.Event{domain.CommentEdited{Number: 1}, domain.CommentDeleted{Number: 1}, domain.CommentResolved{Number: 1}} | |||
| 69 | + | for _, ev := range events { | |||
| 70 | + | we := ports.WatchEvent{Event: ev} | |||
| 71 | + | assert.True(t, liveEvent(we)) | |||
| 72 | + | fs, err := l.repoPage(context.Background(), we) | |||
| 73 | + | require.NoError(t, err) | |||
| 74 | + | require.Len(t, fs, 1) | |||
| 75 | + | assert.Contains(t, fs[0].html, "reload") | |||
| 76 | + | } | |||
| 77 | + | } | |||
| 78 | + | ||||
| 79 | + | func TestNotePosts(t *testing.T) { | |||
| 80 | + | t.Parallel() | |||
| 81 | + | svc := mocks.NewMergeRequestService(t) | |||
| 82 | + | repo := repoName(t, "demo") | |||
| 83 | + | s := &Server{cfg: Config{MergeRequests: svc}} | |||
| 84 | + | ctx := domain.ContextWithPrincipal(context.Background(), domain.Owner()) | |||
| 85 | + | request := func(values url.Values) *http.Request { | |||
| 86 | + | r := httptest.NewRequest("POST", "/demo/merge-requests/1/comments", strings.NewReader(values.Encode())).WithContext(ctx) | |||
| 87 | + | r.Header.Set("Content-Type", "application/x-www-form-urlencoded") | |||
| 88 | + | r.SetPathValue("repo", "demo") | |||
| 89 | + | r.SetPathValue("n", "1") | |||
| 90 | + | r.SetPathValue("id", "2") | |||
| 91 | + | require.NoError(t, r.ParseForm()) | |||
| 92 | + | return r | |||
| 93 | + | } | |||
| 94 | + | svc.EXPECT().AddComment(mock.Anything, mock.Anything, repo, int64(1), mock.MatchedBy(func(p ports.AddCommentParams) bool { return p.ReplyTo == 2 && p.Text == "reply" })).Return(domain.Comment{ID: 7}, nil).Once() | |||
| 95 | + | to, _, err := s.mrComment(ctx, request(url.Values{"reply_to": {"2"}, "text": {"reply"}})) | |||
| 96 | + | require.NoError(t, err) | |||
| 97 | + | assert.Equal(t, "/demo/merge-requests/1#c7", to) | |||
| 98 | + | svc.EXPECT().AddComment(mock.Anything, mock.Anything, repo, int64(1), mock.MatchedBy(func(p ports.AddCommentParams) bool { | |||
| 99 | + | return p.StartLine == 10 && p.Line == 24 && p.Path == "a.go" && p.Verdict == domain.VerdictApprove | |||
| 100 | + | })).Return(domain.Comment{ID: 8}, nil).Once() | |||
| 101 | + | to, _, err = s.mrComment(ctx, request(url.Values{"path": {"a.go"}, "line": {"24"}, "start_line": {"10"}, "side": {"new"}, "verdict": {"approve"}})) | |||
| 102 | + | require.NoError(t, err) | |||
| 103 | + | assert.Equal(t, "/demo/merge-requests/1#c8", to) | |||
| 104 | + | svc.EXPECT().EditComment(mock.Anything, mock.Anything, repo, int64(1), int64(2), "new\ntext").Return(domain.Comment{ID: 2}, nil).Once() | |||
| 105 | + | to, _, err = s.mrEditComment(ctx, request(url.Values{"text": {"new\r\ntext"}})) | |||
| 106 | + | require.NoError(t, err) | |||
| 107 | + | assert.Equal(t, "/demo/merge-requests/1#c2", to) | |||
| 108 | + | svc.EXPECT().DeleteComment(mock.Anything, mock.Anything, repo, int64(1), int64(2)).Return(domain.Comment{ID: 2}, nil).Once() | |||
| 109 | + | to, _, err = s.mrDeleteComment(ctx, request(nil)) | |||
| 110 | + | require.NoError(t, err) | |||
| 111 | + | assert.Equal(t, "/demo/merge-requests/1#c2", to) | |||
| 112 | + | } | |||
M internal/adapters/web/mr_post.go +47 -0
| @@ -177,6 +177,23 @@ func (s *Server) mrComment(ctx context.Context, r *http.Request) (string, string | |||
| 177 | 177 | return "", back, formError("side must be new or old") | |
| 178 | 178 | } | |
| 179 | 179 | } | |
| 180 | + | if raw := r.PostFormValue("reply_to"); raw != "" { | |
| 181 | + | params.ReplyTo = commentID(raw) | |
| 182 | + | if params.ReplyTo == 0 { | |
| 183 | + | return "", back, formError("reply_to is not a note id") | |
| 184 | + | } | |
| 185 | + | } | |
| 186 | + | if raw := r.PostFormValue("start_line"); raw != "" { | |
| 187 | + | v, e := strconv.ParseInt(raw, 10, 32) | |
| 188 | + | if e != nil { | |
| 189 | + | return "", back, formError("start_line is not a number") | |
| 190 | + | } | |
| 191 | + | params.StartLine = int(v) | |
| 192 | + | } | |
| 193 | + | params.Verdict, err = domain.ParseVerdict(r.PostFormValue("verdict")) | |
| 194 | + | if err != nil { | |
| 195 | + | return "", back, err | |
| 196 | + | } | |
| 180 | 197 | name, err := domain.ParseRepoName(repo) | |
| 181 | 198 | if err != nil { | |
| 182 | 199 | return "", back, err | |
| @@ -356,3 +373,33 @@ func formBool(s string) (bool, error) { | |||
| 356 | 373 | func normalizeNewlines(s string) string { | |
| 357 | 374 | return strings.ReplaceAll(s, "\r\n", "\n") | |
| 358 | 375 | } | |
| 376 | + | ||
| 377 | + | // mrEditComment passes source text to the author-checked use case. | |
| 378 | + | func (s *Server) mrEditComment(ctx context.Context, r *http.Request) (string, string, error) { | |
| 379 | + | n, err := number(r) | |
| 380 | + | if err != nil { | |
| 381 | + | return "", "", err | |
| 382 | + | } | |
| 383 | + | back := withFrom(mrHref(r.PathValue("repo"), n), formFrom(r)) | |
| 384 | + | name, err := domain.ParseRepoName(r.PathValue("repo")) | |
| 385 | + | if err != nil { | |
| 386 | + | return "", back, err | |
| 387 | + | } | |
| 388 | + | c, err := s.cfg.MergeRequests.EditComment(ctx, principal(ctx), name, n, commentID(r.PathValue("id")), normalizeNewlines(r.PostFormValue("text"))) | |
| 389 | + | return back + "#c" + strconv.FormatInt(c.ID, 10), back, err | |
| 390 | + | } | |
| 391 | + | ||
| 392 | + | // mrDeleteComment passes the note id to the author-checked use case. | |
| 393 | + | func (s *Server) mrDeleteComment(ctx context.Context, r *http.Request) (string, string, error) { | |
| 394 | + | n, err := number(r) | |
| 395 | + | if err != nil { | |
| 396 | + | return "", "", err | |
| 397 | + | } | |
| 398 | + | back := withFrom(mrHref(r.PathValue("repo"), n), formFrom(r)) | |
| 399 | + | name, err := domain.ParseRepoName(r.PathValue("repo")) | |
| 400 | + | if err != nil { | |
| 401 | + | return "", back, err | |
| 402 | + | } | |
| 403 | + | c, err := s.cfg.MergeRequests.DeleteComment(ctx, principal(ctx), name, n, commentID(r.PathValue("id"))) | |
| 404 | + | return back + "#c" + strconv.FormatInt(c.ID, 10), back, err | |
| 405 | + | } | |
| @@ -177,6 +177,23 @@ func (s *Server) mrComment(ctx context.Context, r *http.Request) (string, string | |||||
| 177 | return "", back, formError("side must be new or old") | 177 | return "", back, formError("side must be new or old") | ||
| 178 | } | 178 | } | ||
| 179 | } | 179 | } | ||
| 180 | + | if raw := r.PostFormValue("reply_to"); raw != "" { | |||
| 181 | + | params.ReplyTo = commentID(raw) | |||
| 182 | + | if params.ReplyTo == 0 { | |||
| 183 | + | return "", back, formError("reply_to is not a note id") | |||
| 184 | + | } | |||
| 185 | + | } | |||
| 186 | + | if raw := r.PostFormValue("start_line"); raw != "" { | |||
| 187 | + | v, e := strconv.ParseInt(raw, 10, 32) | |||
| 188 | + | if e != nil { | |||
| 189 | + | return "", back, formError("start_line is not a number") | |||
| 190 | + | } | |||
| 191 | + | params.StartLine = int(v) | |||
| 192 | + | } | |||
| 193 | + | params.Verdict, err = domain.ParseVerdict(r.PostFormValue("verdict")) | |||
| 194 | + | if err != nil { | |||
| 195 | + | return "", back, err | |||
| 196 | + | } | |||
| 180 | name, err := domain.ParseRepoName(repo) | 197 | name, err := domain.ParseRepoName(repo) | ||
| 181 | if err != nil { | 198 | if err != nil { | ||
| 182 | return "", back, err | 199 | return "", back, err | ||
| @@ -356,3 +373,33 @@ func formBool(s string) (bool, error) { | |||||
| 356 | func normalizeNewlines(s string) string { | 373 | func normalizeNewlines(s string) string { | ||
| 357 | return strings.ReplaceAll(s, "\r\n", "\n") | 374 | return strings.ReplaceAll(s, "\r\n", "\n") | ||
| 358 | } | 375 | } | ||
| 376 | + | ||||
| 377 | + | // mrEditComment passes source text to the author-checked use case. | |||
| 378 | + | func (s *Server) mrEditComment(ctx context.Context, r *http.Request) (string, string, error) { | |||
| 379 | + | n, err := number(r) | |||
| 380 | + | if err != nil { | |||
| 381 | + | return "", "", err | |||
| 382 | + | } | |||
| 383 | + | back := withFrom(mrHref(r.PathValue("repo"), n), formFrom(r)) | |||
| 384 | + | name, err := domain.ParseRepoName(r.PathValue("repo")) | |||
| 385 | + | if err != nil { | |||
| 386 | + | return "", back, err | |||
| 387 | + | } | |||
| 388 | + | c, err := s.cfg.MergeRequests.EditComment(ctx, principal(ctx), name, n, commentID(r.PathValue("id")), normalizeNewlines(r.PostFormValue("text"))) | |||
| 389 | + | return back + "#c" + strconv.FormatInt(c.ID, 10), back, err | |||
| 390 | + | } | |||
| 391 | + | ||||
| 392 | + | // mrDeleteComment passes the note id to the author-checked use case. | |||
| 393 | + | func (s *Server) mrDeleteComment(ctx context.Context, r *http.Request) (string, string, error) { | |||
| 394 | + | n, err := number(r) | |||
| 395 | + | if err != nil { | |||
| 396 | + | return "", "", err | |||
| 397 | + | } | |||
| 398 | + | back := withFrom(mrHref(r.PathValue("repo"), n), formFrom(r)) | |||
| 399 | + | name, err := domain.ParseRepoName(r.PathValue("repo")) | |||
| 400 | + | if err != nil { | |||
| 401 | + | return "", back, err | |||
| 402 | + | } | |||
| 403 | + | c, err := s.cfg.MergeRequests.DeleteComment(ctx, principal(ctx), name, n, commentID(r.PathValue("id"))) | |||
| 404 | + | return back + "#c" + strconv.FormatInt(c.ID, 10), back, err | |||
| 405 | + | } | |||
M internal/adapters/web/mr_templ.go +763 -480 generated file: collapsed
M internal/adapters/web/static/css/55-diff.css +4 -0
| @@ -6,3 +6,7 @@ | |||
| 6 | 6 | } | |
| 7 | 7 | .eol { color: var(--ink-mute); padding-left: 1ch; user-select: none; } | |
| 8 | 8 | } | |
| 9 | + | ||
| 10 | + | @layer components { | |
| 11 | + | .sel { outline: 1px solid var(--ink-dim); outline-offset: -1px; } | |
| 12 | + | } | |
| @@ -6,3 +6,7 @@ | |||||
| 6 | } | 6 | } | ||
| 7 | .eol { color: var(--ink-mute); padding-left: 1ch; user-select: none; } | 7 | .eol { color: var(--ink-mute); padding-left: 1ch; user-select: none; } | ||
| 8 | } | 8 | } | ||
| 9 | + | ||||
| 10 | + | @layer components { | |||
| 11 | + | .sel { outline: 1px solid var(--ink-dim); outline-offset: -1px; } | |||
| 12 | + | } | |||
M internal/adapters/web/static/css/60-mr.css +7 -0
| @@ -190,3 +190,10 @@ | |||
| 190 | 190 | .list.mrs .c-who { display: none; } | |
| 191 | 191 | } | |
| 192 | 192 | } | |
| 193 | + | ||
| 194 | + | @layer components { | |
| 195 | + | .c-body { white-space: normal; overflow-wrap: anywhere; } | |
| 196 | + | .c-body.prose > * { margin-block: .5em; } | |
| 197 | + | .c-body .ref { font-variant-numeric: tabular-nums; text-decoration: underline; } | |
| 198 | + | .cmt > .cmt { margin-inline-start: 2ch; } | |
| 199 | + | } | |
| @@ -190,3 +190,10 @@ | |||||
| 190 | .list.mrs .c-who { display: none; } | 190 | .list.mrs .c-who { display: none; } | ||
| 191 | } | 191 | } | ||
| 192 | } | 192 | } | ||
| 193 | + | ||||
| 194 | + | @layer components { | |||
| 195 | + | .c-body { white-space: normal; overflow-wrap: anywhere; } | |||
| 196 | + | .c-body.prose > * { margin-block: .5em; } | |||
| 197 | + | .c-body .ref { font-variant-numeric: tabular-nums; text-decoration: underline; } | |||
| 198 | + | .cmt > .cmt { margin-inline-start: 2ch; } | |||
| 199 | + | } | |||
M internal/adapters/web/static/js/mr.js +31 -0
| @@ -18,6 +18,25 @@ | |||
| 18 | 18 | window.ermiteMR = (() => { | |
| 19 | 19 | // core: the decisions, without the DOM. | |
| 20 | 20 | const core = { | |
| 21 | + | rangeURL(previous, current, base) { | |
| 22 | + | const a = new URL(previous, base), b = new URL(current, base); | |
| 23 | + | const parse = (u) => /^(new|old):(\d+):(.+)$/.exec(u.searchParams.get('on') || ''); | |
| 24 | + | const x = parse(a), y = parse(b); | |
| 25 | + | if (!x || !y || a.pathname !== b.pathname || x[1] !== y[1] || x[3] !== y[3] || x[2] === y[2]) return null; | |
| 26 | + | const nums = [Number(x[2]), Number(y[2])].sort((a,b) => a-b); | |
| 27 | + | b.searchParams.set('on', `${y[1]}:${nums[0]}-${nums[1]}:${y[3]}`); | |
| 28 | + | return b.href; | |
| 29 | + | }, | |
| 30 | + | quote(button) { | |
| 31 | + | const thread = button.closest('.cmt').closest('[data-thread]'); | |
| 32 | + | const details = thread?.querySelector(':scope > details[data-reply]'); | |
| 33 | + | const field = details?.querySelector('textarea'); | |
| 34 | + | if (!field) return false; | |
| 35 | + | details.open = true; | |
| 36 | + | field.value = button.dataset.quote; | |
| 37 | + | field.focus(); | |
| 38 | + | return true; | |
| 39 | + | }, | |
| 21 | 40 | // windowOf is the start of the window of file i (files are f1, f2, ...). | |
| 22 | 41 | windowOf(id, size) { | |
| 23 | 42 | const n = Number(String(id).replace(/^f/, '')) - 1; | |
| @@ -262,6 +281,18 @@ window.ermiteMR = (() => { | |||
| 262 | 281 | }; | |
| 263 | 282 | addEventListener('hashchange', hash); | |
| 264 | 283 | hash(); | |
| 284 | + | let lastLine = null; | |
| 285 | + | try { lastLine = sessionStorage.getItem('mr-last-line'); } catch { /* storage optional */ } | |
| 286 | + | addEventListener('click', (e) => { | |
| 287 | + | const quote = e.target.closest?.('[data-quote]'); | |
| 288 | + | if (quote) { core.quote(quote); return; } | |
| 289 | + | const link = e.target.closest?.('a[href*="on="]'); | |
| 290 | + | if (!link || e.ctrlKey || e.metaKey || e.altKey) return; | |
| 291 | + | const next = e.shiftKey && lastLine && core.rangeURL(lastLine, link.href, location.href); | |
| 292 | + | lastLine = link.href; | |
| 293 | + | try { sessionStorage.setItem('mr-last-line', lastLine); } catch { /* storage optional */ } | |
| 294 | + | if (next) { e.preventDefault(); location.assign(next); } | |
| 295 | + | }); | |
| 265 | 296 | // A click outside an open menu closes it. | |
| 266 | 297 | addEventListener('click', (e) => { | |
| 267 | 298 | for (const d of $$('details.menu[open]')) if (!d.contains(e.target)) d.open = false; | |
| @@ -18,6 +18,25 @@ | |||||
| 18 | window.ermiteMR = (() => { | 18 | window.ermiteMR = (() => { | ||
| 19 | // core: the decisions, without the DOM. | 19 | // core: the decisions, without the DOM. | ||
| 20 | const core = { | 20 | const core = { | ||
| 21 | + | rangeURL(previous, current, base) { | |||
| 22 | + | const a = new URL(previous, base), b = new URL(current, base); | |||
| 23 | + | const parse = (u) => /^(new|old):(\d+):(.+)$/.exec(u.searchParams.get('on') || ''); | |||
| 24 | + | const x = parse(a), y = parse(b); | |||
| 25 | + | if (!x || !y || a.pathname !== b.pathname || x[1] !== y[1] || x[3] !== y[3] || x[2] === y[2]) return null; | |||
| 26 | + | const nums = [Number(x[2]), Number(y[2])].sort((a,b) => a-b); | |||
| 27 | + | b.searchParams.set('on', `${y[1]}:${nums[0]}-${nums[1]}:${y[3]}`); | |||
| 28 | + | return b.href; | |||
| 29 | + | }, | |||
| 30 | + | quote(button) { | |||
| 31 | + | const thread = button.closest('.cmt').closest('[data-thread]'); | |||
| 32 | + | const details = thread?.querySelector(':scope > details[data-reply]'); | |||
| 33 | + | const field = details?.querySelector('textarea'); | |||
| 34 | + | if (!field) return false; | |||
| 35 | + | details.open = true; | |||
| 36 | + | field.value = button.dataset.quote; | |||
| 37 | + | field.focus(); | |||
| 38 | + | return true; | |||
| 39 | + | }, | |||
| 21 | // windowOf is the start of the window of file i (files are f1, f2, ...). | 40 | // windowOf is the start of the window of file i (files are f1, f2, ...). | ||
| 22 | windowOf(id, size) { | 41 | windowOf(id, size) { | ||
| 23 | const n = Number(String(id).replace(/^f/, '')) - 1; | 42 | const n = Number(String(id).replace(/^f/, '')) - 1; | ||
| @@ -262,6 +281,18 @@ window.ermiteMR = (() => { | |||||
| 262 | }; | 281 | }; | ||
| 263 | addEventListener('hashchange', hash); | 282 | addEventListener('hashchange', hash); | ||
| 264 | hash(); | 283 | hash(); | ||
| 284 | + | let lastLine = null; | |||
| 285 | + | try { lastLine = sessionStorage.getItem('mr-last-line'); } catch { /* storage optional */ } | |||
| 286 | + | addEventListener('click', (e) => { | |||
| 287 | + | const quote = e.target.closest?.('[data-quote]'); | |||
| 288 | + | if (quote) { core.quote(quote); return; } | |||
| 289 | + | const link = e.target.closest?.('a[href*="on="]'); | |||
| 290 | + | if (!link || e.ctrlKey || e.metaKey || e.altKey) return; | |||
| 291 | + | const next = e.shiftKey && lastLine && core.rangeURL(lastLine, link.href, location.href); | |||
| 292 | + | lastLine = link.href; | |||
| 293 | + | try { sessionStorage.setItem('mr-last-line', lastLine); } catch { /* storage optional */ } | |||
| 294 | + | if (next) { e.preventDefault(); location.assign(next); } | |||
| 295 | + | }); | |||
| 265 | // A click outside an open menu closes it. | 296 | // A click outside an open menu closes it. | ||
| 266 | addEventListener('click', (e) => { | 297 | addEventListener('click', (e) => { | ||
| 267 | for (const d of $$('details.menu[open]')) if (!d.contains(e.target)) d.open = false; | 298 | for (const d of $$('details.menu[open]')) if (!d.contains(e.target)) d.open = false; | ||
M internal/adapters/web/web.go +2 -0
| @@ -190,6 +190,8 @@ func (s *Server) routes() *http.ServeMux { | |||
| 190 | 190 | mux.HandleFunc("POST /{repo}/merge-requests", s.mrAction(s.mrCreate)) | |
| 191 | 191 | mux.HandleFunc("POST /{repo}/merge-requests/{n}/comments", s.mrAction(s.mrComment)) | |
| 192 | 192 | mux.HandleFunc("POST /{repo}/merge-requests/{n}/comments/{id}/resolve", s.mrAction(s.mrResolve)) | |
| 193 | + | mux.HandleFunc("POST /{repo}/merge-requests/{n}/comments/{id}/edit", s.mrAction(s.mrEditComment)) | |
| 194 | + | mux.HandleFunc("POST /{repo}/merge-requests/{n}/comments/{id}/delete", s.mrAction(s.mrDeleteComment)) | |
| 193 | 195 | mux.HandleFunc("POST /{repo}/merge-requests/{n}/merge", s.mrAction(s.mrMerge)) | |
| 194 | 196 | mux.HandleFunc("POST /{repo}/merge-requests/{n}/close", s.mrAction(s.mrClose)) | |
| 195 | 197 | mux.HandleFunc("POST /{repo}/merge-requests/{n}/ready", s.mrAction(s.mrReady)) | |
| @@ -190,6 +190,8 @@ func (s *Server) routes() *http.ServeMux { | |||||
| 190 | mux.HandleFunc("POST /{repo}/merge-requests", s.mrAction(s.mrCreate)) | 190 | mux.HandleFunc("POST /{repo}/merge-requests", s.mrAction(s.mrCreate)) | ||
| 191 | mux.HandleFunc("POST /{repo}/merge-requests/{n}/comments", s.mrAction(s.mrComment)) | 191 | mux.HandleFunc("POST /{repo}/merge-requests/{n}/comments", s.mrAction(s.mrComment)) | ||
| 192 | mux.HandleFunc("POST /{repo}/merge-requests/{n}/comments/{id}/resolve", s.mrAction(s.mrResolve)) | 192 | mux.HandleFunc("POST /{repo}/merge-requests/{n}/comments/{id}/resolve", s.mrAction(s.mrResolve)) | ||
| 193 | + | mux.HandleFunc("POST /{repo}/merge-requests/{n}/comments/{id}/edit", s.mrAction(s.mrEditComment)) | |||
| 194 | + | mux.HandleFunc("POST /{repo}/merge-requests/{n}/comments/{id}/delete", s.mrAction(s.mrDeleteComment)) | |||
| 193 | mux.HandleFunc("POST /{repo}/merge-requests/{n}/merge", s.mrAction(s.mrMerge)) | 195 | mux.HandleFunc("POST /{repo}/merge-requests/{n}/merge", s.mrAction(s.mrMerge)) | ||
| 194 | mux.HandleFunc("POST /{repo}/merge-requests/{n}/close", s.mrAction(s.mrClose)) | 196 | mux.HandleFunc("POST /{repo}/merge-requests/{n}/close", s.mrAction(s.mrClose)) | ||
| 195 | mux.HandleFunc("POST /{repo}/merge-requests/{n}/ready", s.mrAction(s.mrReady)) | 197 | mux.HandleFunc("POST /{repo}/merge-requests/{n}/ready", s.mrAction(s.mrReady)) | ||
M internal/app/web_mr_test.go +6 -3
| @@ -284,7 +284,7 @@ func TestWebMergeRequestReviewAndFastForward(t *testing.T) { | |||
| 284 | 284 | require.GreaterOrEqual(t, i, 0, "a note row under the line") | |
| 285 | 285 | note := anon.body[i:] | |
| 286 | 286 | assert.Less(t, strings.Index(note, "Why <i>empty</i>?"), strings.Index(note, "</tr>"), "the note sits in the row under its line") | |
| 287 | - | assert.Contains(t, anon.body, `<a href="https://example.org/a" rel="noopener nofollow ugc">https://example.org/a</a>.`) | |
| 287 | + | assert.Contains(t, anon.body, `<a href="https://example.org/a" rel="nofollow noopener noreferrer">https://example.org/a</a>.`) | |
| 288 | 288 | assert.NotContains(t, anon.body, "<i>empty</i>") | |
| 289 | 289 | assert.Contains(t, anon.body, "Looks fine overall.") | |
| 290 | 290 | assert.Equal(t, 2, len(reNoteID.FindAllString(anon.body, -1))) | |
| @@ -338,7 +338,8 @@ func TestWebMergeRequestReviewAndFastForward(t *testing.T) { | |||
| 338 | 338 | assert.Contains(t, since.body, "changes since "+head1[:7]) | |
| 339 | 339 | assert.Contains(t, since.body, "extra</td>") | |
| 340 | 340 | assert.NotContains(t, since.body, `<span class="kn">package</span> search</td>`, "only what changed after the read head") | |
| 341 | - | assert.Contains(t, since.body, "Why <i>empty</i>?", "the notes are listed below, their lines are not in this diff") | |
| 341 | + | assert.Contains(t, since.body, "empty", "the notes are listed below, their lines are not in this diff") | |
| 342 | + | assert.NotContains(t, since.body, "<i>", "raw HTML is dropped") | |
| 342 | 343 | assert.Contains(t, since.body, "search.go:3 (new)") | |
| 343 | 344 | assert.NotContains(t, since.body, `<form class="cform"`, "no comment form on this diff") | |
| 344 | 345 | ||
| @@ -554,7 +555,9 @@ func TestWebMergeRequestEscapesText(t *testing.T) { | |||
| 554 | 555 | assert.NotContains(t, body, "<img src=x") | |
| 555 | 556 | assert.NotContains(t, body, "<script>x</script>") | |
| 556 | 557 | assert.Contains(t, html.UnescapeString(body), `"><script>alert(1)</script>`) | |
| 557 | - | assert.Contains(t, body, "<script>x</script> & more") | |
| 558 | + | // A note is safe Markdown: a line that opens with an HTML tag is an HTML | |
| 559 | + | // block, dropped whole. | |
| 560 | + | assert.Contains(t, body, `class="c-body prose"><!-- raw HTML omitted -->`) | |
| 558 | 561 | assertNoInline(t, body) | |
| 559 | 562 | assert.NotContains(t, body, `onmouseover="alert`, "an address stops at a quote") | |
| 560 | 563 | } | |
| @@ -284,7 +284,7 @@ func TestWebMergeRequestReviewAndFastForward(t *testing.T) { | |||||
| 284 | require.GreaterOrEqual(t, i, 0, "a note row under the line") | 284 | require.GreaterOrEqual(t, i, 0, "a note row under the line") | ||
| 285 | note := anon.body[i:] | 285 | note := anon.body[i:] | ||
| 286 | assert.Less(t, strings.Index(note, "Why <i>empty</i>?"), strings.Index(note, "</tr>"), "the note sits in the row under its line") | 286 | assert.Less(t, strings.Index(note, "Why <i>empty</i>?"), strings.Index(note, "</tr>"), "the note sits in the row under its line") | ||
| 287 | - | assert.Contains(t, anon.body, `<a href="https://example.org/a" rel="noopener nofollow ugc">https://example.org/a</a>.`) | 287 | + | assert.Contains(t, anon.body, `<a href="https://example.org/a" rel="nofollow noopener noreferrer">https://example.org/a</a>.`) |
| 288 | assert.NotContains(t, anon.body, "<i>empty</i>") | 288 | assert.NotContains(t, anon.body, "<i>empty</i>") | ||
| 289 | assert.Contains(t, anon.body, "Looks fine overall.") | 289 | assert.Contains(t, anon.body, "Looks fine overall.") | ||
| 290 | assert.Equal(t, 2, len(reNoteID.FindAllString(anon.body, -1))) | 290 | assert.Equal(t, 2, len(reNoteID.FindAllString(anon.body, -1))) | ||
| @@ -338,7 +338,8 @@ func TestWebMergeRequestReviewAndFastForward(t *testing.T) { | |||||
| 338 | assert.Contains(t, since.body, "changes since "+head1[:7]) | 338 | assert.Contains(t, since.body, "changes since "+head1[:7]) | ||
| 339 | assert.Contains(t, since.body, "extra</td>") | 339 | assert.Contains(t, since.body, "extra</td>") | ||
| 340 | assert.NotContains(t, since.body, `<span class="kn">package</span> search</td>`, "only what changed after the read head") | 340 | assert.NotContains(t, since.body, `<span class="kn">package</span> search</td>`, "only what changed after the read head") | ||
| 341 | - | assert.Contains(t, since.body, "Why <i>empty</i>?", "the notes are listed below, their lines are not in this diff") | 341 | + | assert.Contains(t, since.body, "empty", "the notes are listed below, their lines are not in this diff") |
| 342 | + | assert.NotContains(t, since.body, "<i>", "raw HTML is dropped") | |||
| 342 | assert.Contains(t, since.body, "search.go:3 (new)") | 343 | assert.Contains(t, since.body, "search.go:3 (new)") | ||
| 343 | assert.NotContains(t, since.body, `<form class="cform"`, "no comment form on this diff") | 344 | assert.NotContains(t, since.body, `<form class="cform"`, "no comment form on this diff") | ||
| 344 | 345 | ||||
| @@ -554,7 +555,9 @@ func TestWebMergeRequestEscapesText(t *testing.T) { | |||||
| 554 | assert.NotContains(t, body, "<img src=x") | 555 | assert.NotContains(t, body, "<img src=x") | ||
| 555 | assert.NotContains(t, body, "<script>x</script>") | 556 | assert.NotContains(t, body, "<script>x</script>") | ||
| 556 | assert.Contains(t, html.UnescapeString(body), `"><script>alert(1)</script>`) | 557 | assert.Contains(t, html.UnescapeString(body), `"><script>alert(1)</script>`) | ||
| 557 | - | assert.Contains(t, body, "<script>x</script> & more") | 558 | + | // A note is safe Markdown: a line that opens with an HTML tag is an HTML |
| 559 | + | // block, dropped whole. | |||
| 560 | + | assert.Contains(t, body, `class="c-body prose"><!-- raw HTML omitted -->`) | |||
| 558 | assertNoInline(t, body) | 561 | assertNoInline(t, body) | ||
| 559 | assert.NotContains(t, body, `onmouseover="alert`, "an address stops at a quote") | 562 | assert.NotContains(t, body, `onmouseover="alert`, "an address stops at a quote") | ||
| 560 | } | 563 | } | ||