API: anchored comments carry their current line c0fac31d36fd
Listing a proposal's comments now places each anchored comment on its line of the projected state (by blame, so it follows the atom through later changes); a comment whose atom is gone has no line. A new comment comes back with the line it was posted on.
- Author
- Andy McCall @andymccall <[email protected]>
- When
- · 2026-09-27 23:40 UTC
- Signature
- ✔ signed by Andy McCall
- Hash
c0fac31d36fdfab3f7af0bcfc213767a973f016769e353538c30bfab90edd7fb- Depends on
- 3f02e5ea4f2d b64e2537d702 d23464c0ac60
- Depended on by
- 70c3d02998a2 3571c519ada6
- In bookmarks
- api-unknown-fields cancel-landed-runs clear-waiting close-proposal-140 diffusion-pin-bookmark draft-ready image-check job-waits main oh-comment-body-file proposal-list-fast run-one-job wiki-says-so
- In tags
- v0.1.0 v0.2.0 v0.3.0 v0.3.1 v0.4.0 v0.4.1 v0.4.2 v0.5.0 v0.5.1 v0.5.2 v0.6.0 v0.6.1 v0.7.0 v0.8.0 v0.9.0 v0.9.1 v0.9.11 v0.9.15 v0.9.16 v0.9.19 v0.9.2 v0.9.20 v0.9.21 v0.9.22 v0.9.23 v0.9.24 v0.9.3 v0.9.4 v0.9.5 v0.9.6 v0.9.7 v0.9.8 v0.9.9
- Edits
- 9 atoms, 0 tree operations, 4 files
Browse the tree at this change
modified crates/hub-api/src/lib.rs +4 −0
| … | 251 unchanged lines |
||
| 252 | 252 | /// For proposal comments: the file and the atom the comment anchors to. |
|
| 253 | 253 | pub path: Option<String>, |
|
| 254 | 254 | pub anchor: Option<Atom>, |
|
| 255 | + | /// Where the anchor sits now: its 1-based line in `path` in the |
|
| 256 | + | /// projected state, when the atom is still live there. |
|
| 257 | + | #[serde(default)] |
|
| 258 | + | pub line: Option<usize>, |
|
| 255 | 259 | pub resolved: bool, |
|
| 256 | 260 | } |
|
| 257 | 261 | ||
| … | 359 unchanged lines |
modified crates/hub-api/ts/index.d.ts +1 −0
| … | 150 unchanged lines |
||
| 151 | 151 | body: string; |
|
| 152 | 152 | created_ms: number; |
|
| 153 | 153 | id: number; |
|
| 154 | + | line?: number | null; |
|
| 154 | 155 | path?: string | null; |
|
| 155 | 156 | resolved: boolean; |
|
| 156 | 157 | }; |
|
| … | 206 unchanged lines |
modified crates/hub/src/api/mod.rs +41 −3
| … | 640 unchanged lines |
||
| 641 | 641 | } else { |
|
| 642 | 642 | Some(Atom { change: c.anchor_change.clone(), index: c.anchor_index as u32 }) |
|
| 643 | 643 | }, |
|
| 644 | + | line: None, |
|
| 644 | 645 | resolved: c.resolved, |
|
| 645 | 646 | } |
|
| 646 | 647 | } |
|
| … | 6 unchanged lines |
||
| 653 | 654 | let c = caller(&app, &headers).await?; |
|
| 654 | 655 | let (_, row, _) = reach(&app, &c, &owner, &name, Access::Read).await?; |
|
| 655 | 656 | let p = db::proposals::get(&app.db, row.id, number).await?.ok_or_else(|| not_found("no such proposal"))?; |
|
| 656 | - | Ok(Json(db::proposals::comments(&app.db, p.id).await?.iter().map(comment_of).collect())) |
|
| 657 | + | let mut out: Vec<Comment> = db::proposals::comments(&app.db, p.id).await?.iter().map(comment_of).collect(); |
|
| 658 | + | // Place anchored comments on their line of the projected state. |
|
| 659 | + | let paths: std::collections::BTreeSet<String> = out.iter().filter(|c| c.anchor.is_some()).filter_map(|c| c.path.clone()).collect(); |
|
| 660 | + | if !paths.is_empty() { |
|
| 661 | + | let repo = RepoName::new(&owner, &name).ok_or_else(|| not_found("no such repository"))?; |
|
| 662 | + | let heads = crate::web::proposals::load(&app, &repo, p).await?.heads; |
|
| 663 | + | let lines = app |
|
| 664 | + | .repos |
|
| 665 | + | .read(&repo, move |r, _| { |
|
| 666 | + | let mut at = std::collections::HashMap::new(); |
|
| 667 | + | for path in paths { |
|
| 668 | + | // A file the projection removed has no lines to place a comment on. |
|
| 669 | + | for l in r.blame_in(&path, &heads).unwrap_or_default() { |
|
| 670 | + | at.insert((path.clone(), l.hash.to_hex(), l.index), l.line); |
|
| 671 | + | } |
|
| 672 | + | } |
|
| 673 | + | Ok(at) |
|
| 674 | + | }) |
|
| 675 | + | .await?; |
|
| 676 | + | for c in &mut out { |
|
| 677 | + | if let (Some(path), Some(a)) = (&c.path, &c.anchor) { |
|
| 678 | + | c.line = lines.get(&(path.clone(), a.change.clone(), a.index)).copied(); |
|
| 679 | + | } |
|
| 680 | + | } |
|
| 681 | + | } |
|
| 682 | + | Ok(Json(out)) |
|
| 657 | 683 | } |
|
| 658 | 684 | ||
| 659 | 685 | async fn proposal_comment( |
|
| … | 32 unchanged lines |
||
| 692 | 718 | serde_json::json!({ "number": number, "comment": comment_of(&row_c) }), |
|
| 693 | 719 | ) |
|
| 694 | 720 | .await; |
|
| 695 | - | Ok((StatusCode::CREATED, Json(comment_of(&row_c)))) |
|
| 721 | + | let mut created = comment_of(&row_c); |
|
| 722 | + | created.line = created.anchor.as_ref().and(body.line); |
|
| 723 | + | Ok((StatusCode::CREATED, Json(created))) |
|
| 696 | 724 | } |
|
| 697 | 725 | ||
| 698 | 726 | async fn proposal_reviews( |
|
| … | 222 unchanged lines |
||
| 921 | 949 | created_ms: c.created_ms, |
|
| 922 | 950 | path: None, |
|
| 923 | 951 | anchor: None, |
|
| 952 | + | line: None, |
|
| 924 | 953 | resolved: false, |
|
| 925 | 954 | }) |
|
| 926 | 955 | .collect(), |
|
| … | 32 unchanged lines |
||
| 959 | 988 | }, |
|
| 960 | 989 | ) |
|
| 961 | 990 | .await; |
|
| 962 | - | let api = Comment { id: cm.id, author: cm.author, body: cm.body, created_ms: cm.created_ms, path: None, anchor: None, resolved: false }; |
|
| 991 | + | let api = Comment { |
|
| 992 | + | id: cm.id, |
|
| 993 | + | author: cm.author, |
|
| 994 | + | body: cm.body, |
|
| 995 | + | created_ms: cm.created_ms, |
|
| 996 | + | path: None, |
|
| 997 | + | anchor: None, |
|
| 998 | + | line: None, |
|
| 999 | + | resolved: false, |
|
| 1000 | + | }; |
|
| 963 | 1001 | crate::hooks::emit(&app, &row, "comment.issue", &me.username, serde_json::json!({ "number": number, "comment": api })).await; |
|
| 964 | 1002 | Ok((StatusCode::CREATED, Json(api))) |
|
| 965 | 1003 | } |
|
| … | 138 unchanged lines |
modified crates/hub/tests/api.rs +15 −0
| … | 280 unchanged lines |
||
| 281 | 281 | assert_eq!(call(app, "GET", &side("head", "b.txt"), None, None, None, None).await.body, "b\n"); |
|
| 282 | 282 | assert_eq!(call(app, "GET", &side("base", "b.txt"), None, None, None, None).await.status, StatusCode::NOT_FOUND, "b.txt is new"); |
|
| 283 | 283 | assert_eq!(call(app, "GET", &side("middle", "a.txt"), None, None, None, None).await.status, StatusCode::BAD_REQUEST); |
|
| 284 | + | // An anchored comment comes back with its line in the projected state. |
|
| 285 | + | let (st, v) = api( |
|
| 286 | + | app, |
|
| 287 | + | "POST", |
|
| 288 | + | "/api/v1/repos/andy/demo/proposals/2/comments", |
|
| 289 | + | Some(&bob_rw), |
|
| 290 | + | Some(r#"{"body":"why capital A?","path":"a.txt","line":2}"#), |
|
| 291 | + | ) |
|
| 292 | + | .await; |
|
| 293 | + | assert_eq!(st, StatusCode::CREATED, "{v}"); |
|
| 294 | + | assert_eq!(v["line"], 2); |
|
| 295 | + | let (_, v) = api(app, "GET", "/api/v1/repos/andy/demo/proposals/2/comments", None, None).await; |
|
| 296 | + | assert_eq!(v[0]["path"], "a.txt"); |
|
| 297 | + | assert_eq!(v[0]["line"], 2, "{v}"); |
|
| 298 | + | assert_eq!(v[0]["anchor"]["change"], feat.to_hex()); |
|
| 284 | 299 | // Require the "build" check. |
|
| 285 | 300 | let settings = call(app, "GET", "/andy/demo/settings", None, Some(&andy), None, None).await; |
|
| 286 | 301 | let csrf = csrf_of(&settings.body); |
|
| … | 46 unchanged lines |