Commit `89a6bbd4c1`

89a6bbd4c1877d26528ef967a88c2e4c920a763a

parent: 94e954df77

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-29 05:02 UTC

web: suggestions render as a diff in their thread, with an apply button or the CLI command; line ranges in the compose form

Ref #288

Layout: unified · split

internal/httpd/mractions.go +18

		@@ -140,6 +140,13 @@ func (s Server) mrDiffCommentSubmit(w http.ResponseWriter, r http.Request, u s
140	140	return
141	141	}
142	142	extra = []string{"--path", path, "--line", line}
	143	if start := strings.TrimSpace(r.FormValue("start_line")); start != "" {
	144	if n, err := strconv.ParseInt(start, 10, 64); err != nil \|\| n < 1 {
	145	s.mrDiffRedirect(w, r, "the first line is a line number")
	146	return
	147	}
	148	extra = append(extra, "--start-line", start)
	149	}
143	150	if r.FormValue("side") == "old" {
144	151	extra = append(extra, "--old")
145	152	}
		@@ -179,6 +186,17 @@ func (s Server) mrThreadSubmit(w http.ResponseWriter, r http.Request, u store.
179	186	s.done(w, r, code, msg, s.mrRedirect)
180	187	}
181	188
	189	// mrSuggestionSubmit commits a thread's suggestion to the source branch.
	190	func (s Server) mrSuggestionSubmit(w http.ResponseWriter, r http.Request, u store.User) {
	191	id := strings.TrimSpace(r.FormValue("thread"))
	192	if _, err := strconv.ParseInt(id, 10, 64); err != nil {
	193	s.mrDiffRedirect(w, r, "bad thread id")
	194	return
	195	}
	196	_, msg, code := s.runControlCode(u, mrArgs(r, "apply-suggestion", id))
	197	s.done(w, r, code, msg, s.mrDiffRedirect)
	198	}
	199
182	200	// mrNewPage is the create form: branches to choose from, plus whatever
183	201	// the last attempt had in it so a refusal does not lose the draft.
184	202	type mrNewPage struct {

internal/httpd/routes.go +2

		@@ -225,6 +225,8 @@ func (s *Server) Routes() []Route {
225	225	Handler: s.checkOrigin(s.requireUser(s.mrThreadSubmit))},
226	226	Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/diff-comment", Mutating: true,
227	227	Handler: s.checkOrigin(s.requireUser(s.mrDiffCommentSubmit))},
	228	Route{Method: "POST", Pattern: "/{owner}/{repo}/mrs/{n}/suggestion", Mutating: true,
	229	Handler: s.checkOrigin(s.requireUser(s.mrSuggestionSubmit))},
228	230	Route{Method: "GET", Pattern: "/{owner}/{repo}/edit/{ref}/{path...}",
229	231	Handler: s.requireUser(s.editForm)},
230	232	Route{Method: "POST", Pattern: "/{owner}/{repo}/edit/{ref}/{path...}", Mutating: true,

internal/httpd/suggestion_test.go added +74

		@@ -0,0 +1,74 @@
	1	package httpd
	2
	3	import (
	4	"html/template"
	5	"strings"
	6	"testing"
	7
	8	"gitbay.org/gitbay/internal/control"
	9	"gitbay.org/gitbay/internal/store"
	10	"gitbay.org/gitbay/internal/web"
	11	)
	12
	13	func renderSuggestion(t testing.T, sg control.SuggestionOut, canApply bool) string {
	14	t.Helper()
	15	view := newSuggestionView(sg, canApply, "gitbay mr apply-suggestion alice/app 1 7")
	16	var sb strings.Builder
	17	if err := web.Render(&sb, "mr.html", mrPageData{
	18	repoPage: testRepoPage(), MR: testMR("open"), View: "conversation",
	19	DetachedThreads: []diffThread{{ID: 7, Suggestion: view,
	20	Comments: []renderedComment{{Author: "bob", BodyHTML: template.HTML("<p>try</p>")}}}},
	21	}); err != nil {
	22	t.Fatalf("render: %v", err)
	23	}
	24	return sb.String()
	25	}
	26
	27	// A suggestion renders as the lines it replaces and the ones it proposes,
	28	// with a button for someone who can push to the source branch.
	29	func TestSuggestionRendersAsDiff(t *testing.T) {
	30	sg := &control.SuggestionOut{StartLine: 4, EndLine: 5, Original: "old a\r\nold b\r\n",
	31	Replacement: "new a\n", Apply: "server"}
	32	out := renderSuggestion(t, sg, true)
	33	for _, w := range []string{
	34	`<tr class="del"><td class="ln">4</td><td class="src">old a</td>`,
	35	`<tr class="del"><td class="ln">5</td><td class="src">old b</td>`,
	36	`<tr class="add"><td class="ln">4</td><td class="src">new a</td>`,
	37	`action="/krz/gitbay/mrs/42/suggestion"`, `name="thread" value="7"`, "Apply suggestion",
	38	} {
	39	if !strings.Contains(out, w) {
	40	t.Errorf("page lacks %q", w)
	41	}
	42	}
	43	if out := renderSuggestion(t, sg, false); strings.Contains(out, "Apply suggestion") {
	44	t.Error("apply button shown to someone who cannot push to the source branch")
	45	}
	46	}
	47
	48	// Where the server cannot sign, the page gives the command; an outdated
	49	// suggestion says why and offers neither.
	50	func TestSuggestionLocalAndOutdated(t *testing.T) {
	51	local := renderSuggestion(t, &control.SuggestionOut{StartLine: 1, EndLine: 1, Original: "a\n",
	52	Replacement: "b\n", Apply: "local"}, true)
	53	if !strings.Contains(local, "<code>gitbay mr apply-suggestion alice/app 1 7</code>") \|\| strings.Contains(local, "Apply suggestion</button>") {
	54	t.Error("require-signed suggestion does not give the CLI command in place of the button")
	55	}
	56	stale := renderSuggestion(t, &control.SuggestionOut{StartLine: 1, EndLine: 1, Original: "a\n",
	57	Replacement: "b\n", Apply: "server", Outdated: true, Reason: "the lines it replaces have changed"}, true)
	58	if !strings.Contains(stale, "outdated suggestion: the lines it replaces have changed") \|\| strings.Contains(stale, "Apply suggestion</button>") {
	59	t.Error("outdated suggestion is not marked, or still offers the button")
	60	}
	61	}
	62
	63	// The thread body renders without the raw block the diff stands in for.
	64	func TestAttachThreadsStripsSuggestionBlock(t *testing.T) {
	65	md := func(src, _ string) template.HTML { return template.HTML(src) }
	66	cm := store.DiffComment{ID: 3, Author: "bob", HeadSHA: "h", Path: "a.go", Side: "new", Line: 2,
	67	Body: "try this\n```suggestion\nx\n```\n"}
	68	view := &suggestionView{}
	69	_, detached := attachThreads(nil, []store.DiffComment{cm}, "h", md, reviewRights{},
	70	map[int64]*suggestionView{3: view})
	71	if len(detached) != 1 \|\| detached[0].Suggestion != view \|\| string(detached[0].Comments[0].BodyHTML) != "try this" {
	72	t.Fatalf("thread = %+v", detached)
	73	}
	74	}

internal/httpd/web.go +60 −4

		@@ -39,6 +39,7 @@ import (
39	39	"gitbay.org/gitbay/internal/gitutil"
40	40	"gitbay.org/gitbay/internal/sig"
41	41	"gitbay.org/gitbay/internal/store"
	42	"gitbay.org/gitbay/internal/suggest"
42	43	"gitbay.org/gitbay/internal/web"
43	44	)
44	45
		@@ -1494,6 +1495,38 @@ type diffThread struct {
1494	1495	Pending bool
1495	1496	CanResolve bool
1496	1497	Comments []renderedComment
	1498	Suggestion *suggestionView
	1499	}
	1500
	1501	// suggestionView is a thread's suggestion as the page shows it: the lines
	1502	// it replaces and the ones it proposes, numbered from Start, and whether
	1503	// the viewer can apply it here or needs the CLI.
	1504	type suggestionView struct {
	1505	Start int64
	1506	Old, New []suggestionLine
	1507	Outdated bool
	1508	Reason string
	1509	Local bool // the repositories require signed commits: apply from a clone
	1510	CanApply bool // the viewer can push to the source branch of an open MR
	1511	Command string // the CLI command that applies it
	1512	}
	1513
	1514	type suggestionLine struct {
	1515	N int64
	1516	Text string
	1517	}
	1518
	1519	// newSuggestionView lays out s for the page.
	1520	func newSuggestionView(s control.SuggestionOut, canApply bool, command string) suggestionView {
	1521	v := &suggestionView{Start: s.StartLine, Outdated: s.Outdated, Reason: s.Reason,
	1522	Local: s.Apply == "local", CanApply: canApply, Command: command}
	1523	for i, l := range suggest.FromText(strings.ReplaceAll(s.Original, "\r\n", "\n")) {
	1524	v.Old = append(v.Old, suggestionLine{s.StartLine + int64(i), l})
	1525	}
	1526	for i, l := range suggest.FromText(s.Replacement) {
	1527	v.New = append(v.New, suggestionLine{s.StartLine + int64(i), l})
	1528	}
	1529	return v
1497	1530	}
1498	1531
1499	1532	// reviewRights decides which thread controls a viewer sees. mr resolve
		@@ -1511,8 +1544,10 @@ func (r reviewRights) canResolve(threadAuthor string) bool {
1511	1544
1512	1545	// attachThreads injects review threads under their anchored diff lines;
1513	1546	// threads whose anchor no longer appears (stale after force-push, or on a
1514		// context line outside the current diff) are returned separately.
1515		func attachThreads(files []diffFile, comments []store.DiffComment, headSHA string, md ugcRenderer, rights reviewRights) ([]diffFile, []diffThread) {
	1547	// context line outside the current diff) are returned separately. A
	1548	// thread root in suggestions renders its suggestion as a diff, and its
	1549	// body without the block.
	1550	func attachThreads(files []diffFile, comments []store.DiffComment, headSHA string, md ugcRenderer, rights reviewRights, suggestions map[int64]*suggestionView) ([]diffFile, []diffThread) {
1516	1551	type anchor struct {
1517	1552	path string
1518	1553	side string
		@@ -1525,10 +1560,15 @@ func attachThreads(files []diffFile, comments []store.DiffComment, headSHA strin
1525	1560	var order []int64
1526	1561	for _, cm := range comments {
1527	1562	if cm.ReplyTo == 0 {
	1563	body := cm.Body
	1564	if suggestions[cm.ID] != nil {
	1565	body = suggest.Strip(body)
	1566	}
1528	1567	threads[cm.ID] = &diffThread{ID: cm.ID, Resolved: cm.ResolvedBy, Stale: cm.HeadSHA != headSHA,
1529	1568	Pending: cm.Pending,
1530	1569	CanResolve: rights.canResolve(cm.Author),
1531		Comments: []renderedComment{{Author: cm.Author, CreatedAt: cm.CreatedAt, BodyHTML: md(cm.Body, "md")}}}
	1570	Suggestion: suggestions[cm.ID],
	1571	Comments: []renderedComment{{Author: cm.Author, CreatedAt: cm.CreatedAt, BodyHTML: md(body, "md")}}}
1532	1572	anchors[cm.ID] = anchor{cm.Path, cm.Side, cm.Line}
1533	1573	order = append(order, cm.ID)
1534	1574	} else if th, ok := threads[cm.ReplyTo]; ok {
		@@ -2186,9 +2226,25 @@ func (s Server) mrPage(w http.ResponseWriter, r http.Request, previewForm stri
2186	2226	}
2187	2227	md := s.ugcFor(r, p.Repo)
2188	2228	canWrite := s.canWriteRepo(r, p.Repo)
	2229	// Applying a suggestion pushes to the source branch, so the button
	2230	// follows write on the source repository, which for a fork is not
	2231	// the one this page is in.
	2232	canApply := false
	2233	if p.Viewer != "" && m.State == "open" {
	2234	if src, err := s.st.RepoByID(m.SourceRepoID); err == nil {
	2235	canApply = s.canWriteRepo(r, src)
	2236	}
	2237	}
	2238	suggestions := map[int64]*suggestionView{}
	2239	for _, cm := range diffComments {
	2240	if sg := control.ThreadSuggestion(s.st, s.cfg.Server.Root, p.Repo, m, cm); sg != nil {
	2241	suggestions[cm.ID] = newSuggestionView(sg, canApply && !cm.Pending,
	2242	fmt.Sprintf("gitbay mr apply-suggestion %s %d %d", p.Repo.Path(), m.Number, cm.ID))
	2243	}
	2244	}
2189	2245	var detachedThreads []diffThread
2190	2246	files, detachedThreads = attachThreads(files, diffComments, m.HeadSHA, md,
2191		reviewRights{Viewer: p.Viewer, MRAuthor: m.Author, Write: canWrite})
	2247	reviewRights{Viewer: p.Viewer, MRAuthor: m.Author, Write: canWrite}, suggestions)
2192	2248	if p.Viewer != "" {
2193	2249	markCompose(files, r.URL.Query())
2194	2250	}

internal/web/static/style.css +2

		@@ -1293,6 +1293,8 @@ a.authorlink:hover { color: var(--link); }
1293	1293	.thread textarea { width: 100%; }
1294	1294	.thread.composing p, .thread details.threadreply p { margin: var(--sp-2) 0 0; }
1295	1295	form.threadact { margin: var(--sp-1) 0 0; padding: 0 var(--sp-3) var(--sp-2); }
	1296	.thread .suggestion { margin: var(--sp-2) 0 0; }
	1297	.thread .suggestion table.difftable { margin: var(--sp-1) 0 0; }
1296	1298
1297	1299	nav.subtabs {
1298	1300	display: flex;

internal/web/templates/layout.html +15 −1

		@@ -235,6 +235,7 @@
235	235	<input type="hidden" name="path" value="{{.Path}}">
236	236	<input type="hidden" name="line" value="{{if eq .Class "del"}}{{.OldLine}}{{else}}{{.NewLine}}{{end}}">
237	237	<input type="hidden" name="side" value="{{if eq .Class "del"}}old{{else}}new{{end}}">
	238	{{if ne .Class "del"}}<p><label>From line <input type="number" name="start_line" min="1" max="{{.NewLine}}" placeholder="{{.NewLine}}"></label> to {{.NewLine}}; a <code>```suggestion</code> block proposes replacement lines</p>{{end}}
238	239	<p><textarea name="body" aria-label="Comment on {{.Path}}" rows="3" placeholder="Comment on this line" autofocus></textarea></p>
239	240	<p><button type="submit" class="btn">Comment</button>
240	241	<button type="submit" name="pending" value="on" class="btn">Add to review</button>
		@@ -255,7 +256,7 @@
255	256	{{define "thread"}}{{$t := .T}}<div class="thread{{if $t.Resolved}} resolved{{end}}{{if $t.Pending}} pending{{end}}{{if .Class}} {{.Class}}{{end}}" id="thread-{{$t.ID}}">
256	257	{{if $t.Pending}}<p class="threadstate">pending — only you can see this until you submit your review</p>{{end}}
257	258	{{if or $t.Resolved (and .Class $t.Stale)}}<p class="threadstate">{{if and .Class $t.Stale}}stale{{end}}{{if $t.Resolved}}{{if and .Class $t.Stale}} · {{end}}resolved by {{$t.Resolved}}{{end}}</p>{{end}}
258		{{range $t.Comments}}<p class="commenthead"><strong>{{.Author}}</strong> <span class="when">{{when .CreatedAt}}</span></p><div class="rendered">{{.BodyHTML}}</div>{{end}}
	259	{{range $i, $c := $t.Comments}}<p class="commenthead"><strong>{{.Author}}</strong> <span class="when">{{when .CreatedAt}}</span></p><div class="rendered">{{.BodyHTML}}</div>{{if and (eq $i 0) $t.Suggestion}}{{template "suggestion" dict "S" $t.Suggestion "T" $t.ID "Base" $.Base}}{{end}}{{end}}
259	260	{{if .Viewer}}<details class="threadreply"><summary>Reply</summary>
260	261	<form method="post" action="{{.Base}}/diff-comment">
261	262	<input type="hidden" name="reply" value="{{$t.ID}}">
		@@ -264,3 +265,16 @@
264	265	</form></details>{{end}}
265	266	{{if $t.CanResolve}}<form method="post" action="{{.Base}}/thread" class="threadact"><input type="hidden" name="thread" value="{{$t.ID}}"><button type="submit" name="action" value="{{if $t.Resolved}}unresolve{{else}}resolve{{end}}" class="linklike">{{if $t.Resolved}}Reopen thread{{else}}Resolve thread{{end}}</button></form>{{end}}
266	267	</div>{{end}}
	268
	269	{{/* suggestion renders the lines a thread's suggestion replaces and the
	270	ones it proposes, with the apply button, or the command to run where
	271	the server cannot sign the commit. */}}
	272	{{define "suggestion"}}{{$s := .S}}<div class="suggestion">
	273	<p class="threadstate">{{if $s.Outdated}}outdated suggestion: {{$s.Reason}}{{else}}suggested change{{end}}</p>
	274	<div class="tablewrap"><table class="difftable">
	275	{{range $s.Old}}<tr class="del"><td class="ln">{{.N}}</td><td class="src">{{.Text}}</td></tr>
	276	{{end}}{{range $s.New}}<tr class="add"><td class="ln">{{.N}}</td><td class="src">{{.Text}}</td></tr>
	277	{{end}}</table></div>
	278	{{if and $s.CanApply (not $s.Outdated)}}{{if $s.Local}}<p class="threadstate">This repository requires signed commits, which the server cannot make. Apply it from a clone: <code>{{$s.Command}}</code></p>
	279	{{else}}<form method="post" action="{{.Base}}/suggestion" class="threadact"><input type="hidden" name="thread" value="{{.T}}"><button type="submit" class="btn">Apply suggestion</button></form>
	280	{{end}}{{end}}</div>{{end}}

Commit 89a6bbd4c1

Commit `89a6bbd4c1`