feat(basin): inline review comments on pull request diffs
- commit
a6405613762b332d422d52f2973d2c576757190dbrowse tree- parent
550ae5cf578ced20789e19bd65fb22f1fb39270f- author
- obscurejames <354576047+obscurejames@users.noreply.github.com>
- signature
- Signed by:
~james SHA256:KlzJ0P0gvJcnOYpaxD5yCUw52ImHttTKRjVAEnnYnKg
- date
- 2026-10-03 13:55 UTC
4 files changed, +160 −5
- README.md modified +2 −2
- pull.go modified +131 −0
- static/style.css modified +13 −0
- templates/pull.html modified +14 −3
README.md modified
| @@ -8,7 +8,7 @@ Small and lightweight git server for self hosting. Repositories are served over | ||
| 8 | 8 | - Tree and blob and log and commit and refs views, plus a case insensitive search across file contents at any ref backed by `git grep`. Chroma does the highlighting and goldmark rendering the Markdown before bluemonday cleans it. Anything in `$x$` or `$$x$$` gets typeset in the browser |
| 9 | 9 | - The last commit shown against every tree entry comes out of one `git log --name-only` walk per directory instead of a separate process per file |
| 10 | 10 | - Profiles have a display name and a Markdown bio along with location and workplace and website, plus an avatar that you crop in the browser and the server then resamples down to a 256px PNG |
| 11 | -- Forks and pull requests from either a branch or a fork. Comments are Markdown, merges take an editable message, and a request can be closed or reopened or brought up to date from its source at any point | |
| 11 | +- Forks and pull requests from either a branch or a fork. Discussion comments and inline review comments on any line of the diff are Markdown, review threads can be resolved and go to an outdated section when the line leaves the diff, merges take an editable message, and a request can be closed or reopened or brought up to date from its source at any point | |
| 12 | 12 | - SSH signature verification. Sign with `gpg.format ssh`, register the same key in settings, and your commits start showing `Verified ~you` |
| 13 | 13 | - SSH keys and access tokens live in settings. HTTPS auth is plain basic auth with a token or your password |
| 14 | 14 | - Registration is invite only unless you say otherwise, and there is a failure counter per address covering login and register and password changes and HTTPS auth |
| @@ -71,7 +71,7 @@ To send changes to something you do not own, fork it from the button beside the | ||
| 71 | 71 | |
| 72 | 72 | ## Limits |
| 73 | 73 | |
| 74 | -There are no inline review comments, no squash or rebase merges, no collaborators beyond the single owner of a repo, and no disk quota of any kind. Blobs over 1 MiB are not rendered and anything over 64 MiB is not served raw either, while highlighting gives up past 256 KiB. Signature checking is SSH only for now, so commits signed with GPG show up as unverified. | |
| 74 | +There are no squash or rebase merges, no collaborators beyond the single owner of a repo, and no disk quota of any kind. Blobs over 1 MiB are not rendered and anything over 64 MiB is not served raw either, while highlighting gives up past 256 KiB. Signature checking is SSH only for now, so commits signed with GPG show up as unverified. | |
| 75 | 75 | |
| 76 | 76 | ## Licence |
| 77 | 77 | |
pull.go modified
| @@ -30,6 +30,17 @@ create table if not exists pulls ( | ||
| 30 | 30 | merged text not null default '', |
| 31 | 31 | unique(repo, number) |
| 32 | 32 | ); |
| 33 | +create table if not exists reviews ( | |
| 34 | + id integer primary key, | |
| 35 | + pull integer not null references pulls(id) on delete cascade, | |
| 36 | + author integer not null references users(id) on delete cascade, | |
| 37 | + path text not null, | |
| 38 | + side text not null, | |
| 39 | + line integer not null, | |
| 40 | + body text not null, | |
| 41 | + resolved integer not null default 0, | |
| 42 | + created integer not null | |
| 43 | +); | |
| 33 | 44 | create table if not exists comments ( |
| 34 | 45 | id integer primary key, |
| 35 | 46 | pull integer not null references pulls(id) on delete cascade, |
| @@ -56,6 +67,24 @@ type Pull struct { | ||
| 56 | 67 | Comments int |
| 57 | 68 | } |
| 58 | 69 | |
| 70 | +type Review struct { | |
| 71 | + ID int64 | |
| 72 | + Author User | |
| 73 | + Path string | |
| 74 | + Side string | |
| 75 | + Line int | |
| 76 | + Body string | |
| 77 | + Resolved bool | |
| 78 | + Created time.Time | |
| 79 | + HTML template.HTML | |
| 80 | + URL string | |
| 81 | + CanEdit bool | |
| 82 | +} | |
| 83 | + | |
| 84 | +func (r Review) Key() string { | |
| 85 | + return fmt.Sprintf("%s:%s:%d", r.Path, r.Side, r.Line) | |
| 86 | +} | |
| 87 | + | |
| 59 | 88 | type Comment struct { |
| 60 | 89 | ID int64 |
| 61 | 90 | Author User |
| @@ -177,6 +206,54 @@ func (s *Store) AddComment(pull int64, author User, body string) error { | ||
| 177 | 206 | return err |
| 178 | 207 | } |
| 179 | 208 | |
| 209 | +func (s *Store) AddReview(pull int64, author User, path, side string, line int, body string) error { | |
| 210 | + body = strings.TrimSpace(body) | |
| 211 | + if body == "" || len(body) > 20000 { | |
| 212 | + return errors.New("Comment is empty or too long") | |
| 213 | + } | |
| 214 | + if side != "n" && side != "o" || line <= 0 || path == "" { | |
| 215 | + return errors.New("Bad line reference") | |
| 216 | + } | |
| 217 | + _, err := s.db.Exec(`insert into reviews(pull, author, path, side, line, body, created) values (?, ?, ?, ?, ?, ?, ?)`, pull, author.ID, path, side, line, body, time.Now().Unix()) | |
| 218 | + s.TouchPull(pull) | |
| 219 | + return err | |
| 220 | +} | |
| 221 | + | |
| 222 | +func (s *Store) Reviews(pull int64) ([]Review, error) { | |
| 223 | + rows, err := s.db.Query(`select r.id, r.path, r.side, r.line, r.body, r.resolved, r.created, u.id, u.name, u.created from reviews r join users u on u.id = r.author where r.pull = ? order by r.id`, pull) | |
| 224 | + if err != nil { | |
| 225 | + return nil, err | |
| 226 | + } | |
| 227 | + defer rows.Close() | |
| 228 | + var out []Review | |
| 229 | + for rows.Next() { | |
| 230 | + var r Review | |
| 231 | + var resolved int | |
| 232 | + var created, uCreated int64 | |
| 233 | + if err := rows.Scan(&r.ID, &r.Path, &r.Side, &r.Line, &r.Body, &resolved, &created, &r.Author.ID, &r.Author.Name, &uCreated); err != nil { | |
| 234 | + return nil, err | |
| 235 | + } | |
| 236 | + r.Resolved = resolved != 0 | |
| 237 | + r.Created, r.Author.Created = time.Unix(created, 0), time.Unix(uCreated, 0) | |
| 238 | + out = append(out, r) | |
| 239 | + } | |
| 240 | + return out, rows.Err() | |
| 241 | +} | |
| 242 | + | |
| 243 | +func (s *Store) ReviewAuthor(id int64) int64 { | |
| 244 | + var author int64 | |
| 245 | + s.db.QueryRow(`select author from reviews where id = ?`, id).Scan(&author) | |
| 246 | + return author | |
| 247 | +} | |
| 248 | + | |
| 249 | +func (s *Store) ResolveReview(id int64, resolved bool) { | |
| 250 | + s.db.Exec(`update reviews set resolved = ? where id = ?`, resolved, id) | |
| 251 | +} | |
| 252 | + | |
| 253 | +func (s *Store) DeleteReview(id int64) { | |
| 254 | + s.db.Exec(`delete from reviews where id = ?`, id) | |
| 255 | +} | |
| 256 | + | |
| 180 | 257 | func (s *Store) Comments(pull int64) ([]Comment, error) { |
| 181 | 258 | rows, err := s.db.Query(`select c.id, c.body, c.created, u.id, u.name, u.created from comments c join users u on u.id = c.author where c.pull = ? order by c.id`, pull) |
| 182 | 259 | if err != nil { |
| @@ -404,6 +481,29 @@ func (s *Server) pull(w http.ResponseWriter, r *http.Request, repo Repo, rest st | ||
| 404 | 481 | if (isOwner || isAuthor) && pull.State == "closed" { |
| 405 | 482 | s.store.SetPullState(pull.ID, "open", "") |
| 406 | 483 | } |
| 484 | + case "review": | |
| 485 | + line, _ := strconv.Atoi(r.FormValue("line")) | |
| 486 | + if err := s.store.AddReview(pull.ID, *user, r.FormValue("path"), r.FormValue("side"), line, r.FormValue("body")); err != nil { | |
| 487 | + http.Redirect(w, r, back+"?error="+template.URLQueryEscaper(err.Error()), http.StatusSeeOther) | |
| 488 | + return | |
| 489 | + } | |
| 490 | + back += "#r-" + r.FormValue("path") + ":" + r.FormValue("side") + ":" + r.FormValue("line") | |
| 491 | + case "resolve", "unresolve", "delete": | |
| 492 | + id, _ := strconv.ParseInt(r.FormValue("review"), 10, 64) | |
| 493 | + owner := s.store.ReviewAuthor(id) | |
| 494 | + if owner == 0 || (owner != user.ID && !isOwner) { | |
| 495 | + s.fail(w, r, 403, "Not your comment") | |
| 496 | + return | |
| 497 | + } | |
| 498 | + switch action { | |
| 499 | + case "resolve": | |
| 500 | + s.store.ResolveReview(id, true) | |
| 501 | + case "unresolve": | |
| 502 | + s.store.ResolveReview(id, false) | |
| 503 | + case "delete": | |
| 504 | + s.store.DeleteReview(id) | |
| 505 | + } | |
| 506 | + back += "#diff" | |
| 407 | 507 | case "update": |
| 408 | 508 | if (isOwner || isAuthor) && pull.State == "open" { |
| 409 | 509 | if err := g.Fetch(s.repoDir(pull.SourceRepo), "refs/heads/"+pull.SourceBranch, pull.Ref()); err != nil { |
| @@ -455,6 +555,37 @@ func (s *Server) pull(w http.ResponseWriter, r *http.Request, repo Repo, rest st | ||
| 455 | 555 | data["Commits"] = commits |
| 456 | 556 | files, _ := g.DiffRange(mb, head) |
| 457 | 557 | data["Files"] = files |
| 558 | + reviews, _ := s.store.Reviews(pull.ID) | |
| 559 | + present := map[string]bool{} | |
| 560 | + for _, f := range files { | |
| 561 | + for _, h := range f.Hunks { | |
| 562 | + for _, l := range h.Lines { | |
| 563 | + switch l.Kind { | |
| 564 | + case "add", "ctx": | |
| 565 | + present[fmt.Sprintf("%s:n:%d", f.New, l.New)] = true | |
| 566 | + } | |
| 567 | + if l.Kind == "del" || l.Kind == "ctx" { | |
| 568 | + present[fmt.Sprintf("%s:o:%d", f.New, l.Old)] = true | |
| 569 | + } | |
| 570 | + } | |
| 571 | + } | |
| 572 | + } | |
| 573 | + inline := map[string][]Review{} | |
| 574 | + var outdated []Review | |
| 575 | + for _, rv := range reviews { | |
| 576 | + rv.HTML = s.renderMarkdown([]byte(rv.Body)) | |
| 577 | + rv.URL = fmt.Sprintf("/%s/pulls/%d", repo.Path(), pull.Number) | |
| 578 | + rv.CanEdit = user != nil && (user.ID == rv.Author.ID || isOwner) | |
| 579 | + if present[rv.Key()] { | |
| 580 | + inline[rv.Key()] = append(inline[rv.Key()], rv) | |
| 581 | + } else { | |
| 582 | + outdated = append(outdated, rv) | |
| 583 | + } | |
| 584 | + } | |
| 585 | + data["Inline"] = inline | |
| 586 | + data["Outdated"] = outdated | |
| 587 | + data["ReviewCount"] = len(reviews) | |
| 588 | + data["CommentAt"] = r.URL.Query().Get("comment") | |
| 458 | 589 | added, removed := 0, 0 |
| 459 | 590 | for _, f := range files { |
| 460 | 591 | added += f.Added |
static/style.css modified
| @@ -238,3 +238,16 @@ form.stack.centred input{width:100%} | ||
| 238 | 238 | .hits h4{margin:0 0 .2rem} |
| 239 | 239 | .hits table.blob{border:1px solid var(--line)} |
| 240 | 240 | mark{background:#fff3b0;padding:0} |
| 241 | +.hunk td.n a{color:var(--link);text-decoration:none} | |
| 242 | +.hunk td.n a:hover{text-decoration:underline;font-weight:600} | |
| 243 | +.hunk tr.inline td{white-space:normal;padding:.4rem .6rem;background:#fff;border-top:1px solid var(--line);border-bottom:1px solid var(--line)} | |
| 244 | +.hunk tr.inline .comment{margin:0;max-width:60rem} | |
| 245 | +.hunk tr.inline.resolved .comment{opacity:.7} | |
| 246 | +.comment .who{display:flex;gap:.5rem;align-items:center} | |
| 247 | +.comment .tools{margin-left:auto;display:flex;gap:.6rem} | |
| 248 | +.comment .tools form{display:inline} | |
| 249 | +.comment .tools button{background:none;border:0;color:var(--muted);cursor:pointer;font:inherit;font-size:.85rem;padding:0} | |
| 250 | +.comment .tools button:hover{color:var(--fg);text-decoration:underline} | |
| 251 | +.reviewform{margin:0} | |
| 252 | +.reviewform textarea{width:100%} | |
| 253 | +.comment.outdated{opacity:.8} | |
templates/pull.html modified
| @@ -39,18 +39,29 @@ | ||
| 39 | 39 | <div class="log">{{range .Data.Commits}}<div class="event"><div><a class="hash" href="/{{$p.Repo.Path}}/commit/{{.Hash}}">{{short .Hash}}</a> <a class="subject" href="/{{$p.Repo.Path}}/commit/{{.Hash}}">{{.Subject}}</a>{{with signer .}}<span class="verified" title="Signed with a key registered by ~{{.}}">Verified ~{{.}}</span>{{end}}</div><div class="muted small">{{.Author}} · {{ago .Time}}</div></div>{{end}}</div> |
| 40 | 40 | {{end}} |
| 41 | 41 | {{if .Data.Files}} |
| 42 | -<h3>Changes <span class="muted">{{len .Data.Files}} files, <span class="add">+{{.Data.Added}}</span> <span class="del">−{{.Data.Removed}}</span></span></h3> | |
| 43 | -{{range .Data.Files}} | |
| 42 | +<h3 id="diff">Changes <span class="muted">{{len .Data.Files}} files, <span class="add">+{{.Data.Added}}</span> <span class="del">−{{.Data.Removed}}</span>{{if .Data.ReviewCount}}, {{.Data.ReviewCount}} review comments{{end}}</span></h3> | |
| 43 | +{{if .User}}<p class="muted small">Click a line number to comment on that line.</p>{{end}} | |
| 44 | +{{range $file := .Data.Files}} | |
| 44 | 45 | <section class="diff" id="{{.New}}"> |
| 45 | 46 | <h4><a href="/{{$p.Repo.Path}}/tree/{{$.Data.Head}}/{{.New}}">{{if eq .Status "renamed"}}{{.Old}} → {{end}}{{.New}}</a> <span class="muted">{{.Status}}</span></h4> |
| 46 | 47 | {{if .Binary}}<p class="muted">Binary file.</p>{{end}} |
| 47 | 48 | {{range .Hunks}} |
| 48 | 49 | <table class="hunk"> |
| 49 | 50 | <tr class="head"><td colspan="3">{{.Header}}</td></tr> |
| 50 | -{{range .Lines}}<tr class="{{.Kind}}"><td class="n">{{if .Old}}{{.Old}}{{end}}</td><td class="n">{{if .New}}{{.New}}{{end}}</td><td class="l">{{if eq .Kind "add"}}+{{else if eq .Kind "del"}}-{{else if eq .Kind "ctx"}} {{end}}{{.Text}}</td></tr>{{end}} | |
| 51 | +{{range .Lines}}{{$side := "n"}}{{$num := .New}}{{if eq .Kind "del"}}{{$side = "o"}}{{$num = .Old}}{{end}}{{$key := printf "%s:%s:%d" $file.New $side $num}} | |
| 52 | +<tr class="{{.Kind}}" id="r-{{$key}}"><td class="n">{{if .Old}}{{if and $.User (ne .Kind "meta")}}<a href="?comment={{$file.New}}:o:{{.Old}}#r-{{$file.New}}:o:{{.Old}}">{{.Old}}</a>{{else}}{{.Old}}{{end}}{{end}}</td><td class="n">{{if .New}}{{if and $.User (ne .Kind "meta")}}<a href="?comment={{$file.New}}:n:{{.New}}#r-{{$file.New}}:n:{{.New}}">{{.New}}</a>{{else}}{{.New}}{{end}}{{end}}</td><td class="l">{{if eq .Kind "add"}}+{{else if eq .Kind "del"}}-{{else if eq .Kind "ctx"}} {{end}}{{.Text}}</td></tr> | |
| 53 | +{{template "reviews" (index $.Data.Inline $key)}} | |
| 54 | +{{if eq $.Data.CommentAt $key}}<tr class="inline"><td colspan="3"><form method="post" action="/{{$p.Repo.Path}}/pulls/{{$pr.Number}}/review" class="stack wide reviewform"><input type="hidden" name="path" value="{{$file.New}}"><input type="hidden" name="side" value="{{$side}}"><input type="hidden" name="line" value="{{$num}}"><textarea name="body" rows="4" required autofocus placeholder="Markdown supported"></textarea><div class="actions left"><button class="btn primary">Comment ▸</button><a class="btn" href="/{{$p.Repo.Path}}/pulls/{{$pr.Number}}#diff">Cancel</a></div></form></td></tr>{{end}} | |
| 55 | +{{end}} | |
| 51 | 56 | </table> |
| 52 | 57 | {{end}} |
| 53 | 58 | </section> |
| 54 | 59 | {{end}} |
| 60 | +{{with .Data.Outdated}} | |
| 61 | +<h4>Outdated review comments</h4> | |
| 62 | +<p class="muted small">These lines are no longer in the diff.</p> | |
| 63 | +{{range .}}<div class="comment outdated"><div class="who"><a href="/~{{.Author.Name}}">~{{.Author.Name}}</a> <span class="muted">on {{.Path}} line {{.Line}} · {{ago .Created}}</span></div><div class="readme">{{.HTML}}</div></div>{{end}} | |
| 64 | +{{end}} | |
| 55 | 65 | {{end}} |
| 56 | 66 | {{end}} |
| 67 | +{{define "reviews"}}{{range .}}<tr class="inline{{if .Resolved}} resolved{{end}}"><td colspan="3"><div class="comment"><div class="who"><a href="/~{{.Author.Name}}">~{{.Author.Name}}</a> <span class="muted">{{ago .Created}}{{if .Resolved}} · resolved{{end}}</span>{{if .CanEdit}}<span class="tools"><form method="post" action="{{.URL}}/{{if .Resolved}}unresolve{{else}}resolve{{end}}"><input type="hidden" name="review" value="{{.ID}}"><button>{{if .Resolved}}Unresolve{{else}}Resolve{{end}}</button></form><form method="post" action="{{.URL}}/delete"><input type="hidden" name="review" value="{{.ID}}"><button>Delete</button></form></span>{{end}}</div>{{if not .Resolved}}<div class="readme">{{.HTML}}</div>{{end}}</div></td></tr>{{end}}{{end}} | |