Compare page: bookmark and tag pickers, swap, /compare picks a sensible pair, and Propose this for writers 3d1e74cdac76
closes #7
- Author
- Andy McCall @andymccall <[email protected]>
- When
- · 2026-09-28 12:36 UTC
- Signature
- ✔ signed by Andy McCall
- Hash
3d1e74cdac76b716b793ea7d5f20f365e4bce8889818e7678be4a64d626a84bc- Depends on
- 42fa95540468 93be9ce4653e 9df855f29948 d23464c0ac60 de505df5cf8d
- Depended on by
- e271a37b629a 34beb1a7e376 43e8ca33bc5d
- 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
- 14 atoms, 0 tree operations, 7 files
Browse the tree at this change
modified crates/hub/src/web/history.rs +75 −1
| … | 6 unchanged lines |
||
| 7 | 7 | use crate::Shared; |
|
| 8 | 8 | use askama::Template; |
|
| 9 | 9 | use axum::extract::{Path, Query, State}; |
|
| 10 | + | use axum::response::{IntoResponse, Response}; |
|
| 10 | 11 | use oxy_core::repo::FileDiff; |
|
| 11 | 12 | use serde::Deserialize; |
|
| 12 | 13 | use std::collections::BTreeMap; |
|
| … | 142 unchanged lines |
||
| 155 | 156 | cmp: view::Compare, |
|
| 156 | 157 | files: Vec<FileRows>, |
|
| 157 | 158 | ignore_ws: bool, |
|
| 159 | + | /// The two sides as given, for the pickers. |
|
| 160 | + | base_spec: String, |
|
| 161 | + | head_spec: String, |
|
| 162 | + | /// Bookmarks and tags to pick from. |
|
| 163 | + | bookmarks: Vec<String>, |
|
| 164 | + | tags: Vec<String>, |
|
| 165 | + | propose: Option<Propose>, |
|
| 166 | + | } |
|
| 167 | + | ||
| 168 | + | /// "Propose this": head into base, when both are bookmarks and the viewer may write. |
|
| 169 | + | pub struct Propose { |
|
| 170 | + | pub url: String, |
|
| 171 | + | /// An open proposal of head into base already. |
|
| 172 | + | pub existing: Option<i64>, |
|
| 173 | + | } |
|
| 174 | + | ||
| 175 | + | #[derive(Deserialize)] |
|
| 176 | + | pub struct CompareForm { |
|
| 177 | + | base: Option<String>, |
|
| 178 | + | head: Option<String>, |
|
| 179 | + | } |
|
| 180 | + | ||
| 181 | + | /// `/compare`: the pickers' form target, or a sensible first comparison: |
|
| 182 | + | /// the most recently changed other bookmark against the default. |
|
| 183 | + | pub async fn compare_index( |
|
| 184 | + | State(app): State<Shared>, |
|
| 185 | + | viewer: Viewer, |
|
| 186 | + | Path((owner, name)): Path<(String, String)>, |
|
| 187 | + | Query(q): Query<CompareForm>, |
|
| 188 | + | ) -> Result<Response, Error> { |
|
| 189 | + | let shell = Shell::new(&app, &format!("/{owner}/{name}/compare"), &viewer); |
|
| 190 | + | let (repo, row, _role) = lookup(&app, &shell, &viewer, &owner, &name).await?; |
|
| 191 | + | let clean = |s: Option<String>| s.map(|x| x.trim().to_string()).filter(|x| !x.is_empty() && !x.contains("...")); |
|
| 192 | + | let (base, head) = match (clean(q.base), clean(q.head)) { |
|
| 193 | + | (Some(b), Some(h)) => (b, h), |
|
| 194 | + | (b, h) => { |
|
| 195 | + | let default = row.default_bookmark.clone(); |
|
| 196 | + | let d = default.clone(); |
|
| 197 | + | let newest = app |
|
| 198 | + | .repos |
|
| 199 | + | .read(&repo, move |r, cache| { |
|
| 200 | + | Ok(view::bookmarks(r, cache, &d)?.into_iter().filter(|b| !b.default).max_by_key(|b| b.latest_ms).map(|b| b.name)) |
|
| 201 | + | }) |
|
| 202 | + | .await |
|
| 203 | + | .ok() |
|
| 204 | + | .flatten(); |
|
| 205 | + | (b.unwrap_or_else(|| default.clone()), h.unwrap_or_else(|| newest.unwrap_or(default))) |
|
| 206 | + | } |
|
| 207 | + | }; |
|
| 208 | + | let enc = |s: &str| s.replace('%', "%25").replace(' ', "%20").replace('?', "%3F").replace('#', "%23"); |
|
| 209 | + | Ok(axum::response::Redirect::to(&format!("/{owner}/{name}/compare/{}...{}", enc(&base), enc(&head))).into_response()) |
|
| 158 | 210 | } |
|
| 159 | 211 | ||
| 160 | 212 | /// `/compare/{base}...{head}`. |
|
| … | 22 unchanged lines |
||
| 183 | 235 | super::attribute(&app, &mut cmp.added.iter_mut().collect::<Vec<_>>()).await; |
|
| 184 | 236 | let files = render_files_with(&cmp.files, &cmp.bases, q.options()); |
|
| 185 | 237 | let ignore_ws = q.options().ignore_whitespace; |
|
| 186 | - | Ok(Page(ComparePage { shell, repo: RepoShell::new(&app, &row, "changes").with_role(role), cmp, files, ignore_ws })) |
|
| 238 | + | let rs = RepoShell::new(&app, &row, "changes").with_role(role); |
|
| 239 | + | let (bookmarks, tags) = app |
|
| 240 | + | .repos |
|
| 241 | + | .read(&repo, |r, _| { |
|
| 242 | + | let b: Vec<String> = r.bookmarks()?.into_iter().map(|b| b.name).collect(); |
|
| 243 | + | let t: Vec<String> = view::tags(r)?.into_iter().map(|t| t.name).collect(); |
|
| 244 | + | Ok((b, t)) |
|
| 245 | + | }) |
|
| 246 | + | .await |
|
| 247 | + | .unwrap_or_default(); |
|
| 248 | + | let both_bookmarks = cmp.base.kind == view::RefKind::Bookmark && cmp.head.kind == view::RefKind::Bookmark; |
|
| 249 | + | let propose = if both_bookmarks && cmp.base.label != cmp.head.label && !cmp.added.is_empty() && rs.can(crate::auth::Access::Write) { |
|
| 250 | + | let open = crate::db::proposals::list(&app.db, row.id, &["open"]).await.unwrap_or_default(); |
|
| 251 | + | let existing = open |
|
| 252 | + | .iter() |
|
| 253 | + | .find(|p| p.source_kind == "bookmark" && p.source_bookmark == cmp.head.label && p.target == cmp.base.label) |
|
| 254 | + | .map(|p| p.number); |
|
| 255 | + | Some(Propose { url: format!("{}/proposals/new?source={}&target={}", rs.url, cmp.head.label, cmp.base.label), existing }) |
|
| 256 | + | } else { |
|
| 257 | + | None |
|
| 258 | + | }; |
|
| 259 | + | let (base_spec, head_spec) = spec.split_once("...").map(|(a, b)| (a.to_string(), b.to_string())).unwrap_or_default(); |
|
| 260 | + | Ok(Page(ComparePage { shell, repo: rs, cmp, files, ignore_ws, base_spec, head_spec, bookmarks, tags, propose })) |
|
| 187 | 261 | } |
modified crates/hub/src/web/mod.rs +1 −0
| … | 85 unchanged lines |
||
| 86 | 86 | .route("/{owner}/{name}/last/{r}", get(repo::last_changes)) |
|
| 87 | 87 | .route("/{owner}/{name}/changes/{r}", get(history::history)) |
|
| 88 | 88 | .route("/{owner}/{name}/change/{hash}", get(history::change)) |
|
| 89 | + | .route("/{owner}/{name}/compare", get(history::compare_index)) |
|
| 89 | 90 | .route("/{owner}/{name}/compare/{spec}", get(history::compare)) |
|
| 90 | 91 | .route("/{owner}/{name}/bookmarks", get(refs::bookmarks)) |
|
| 91 | 92 | .route("/{owner}/{name}/tags", get(refs::tags)) |
|
| … | 465 unchanged lines |
modified crates/hub/tests/browse.rs +19 −0
| … | 175 unchanged lines |
||
| 176 | 176 | let cmp = page(&h.app, "/andy/demo/compare/feature...main").await; |
|
| 177 | 177 | assert!(cmp.contains("1 change") && cmp.contains("Greet properly")); |
|
| 178 | 178 | assert_eq!(get(&h.app, "/andy/demo/compare/feature").await.0, StatusCode::BAD_REQUEST); |
|
| 179 | + | // Pickers, swap and the /compare form (hub spec §6.3). |
|
| 180 | + | assert!( |
|
| 181 | + | cmp.contains("<datalist id=\"compare-refs\">") |
|
| 182 | + | && cmp.contains("<option value=\"feature\">bookmark</option>") |
|
| 183 | + | && cmp.contains("<option value=\"v0.1\">tag</option>") |
|
| 184 | + | ); |
|
| 185 | + | assert!(cmp.contains("href=\"/andy/demo/compare/main...feature\""), "swap"); |
|
| 186 | + | assert!(!cmp.contains("Propose this"), "anonymous viewers cannot propose"); |
|
| 187 | + | let redirect = |path: &'static str| { |
|
| 188 | + | let app = h.app.clone(); |
|
| 189 | + | async move { |
|
| 190 | + | let res = oxygenhub::router(app).oneshot(Request::get(path).body(Body::empty()).unwrap()).await.unwrap(); |
|
| 191 | + | (res.status(), res.headers().get("location").and_then(|v| v.to_str().ok()).unwrap_or("").to_string()) |
|
| 192 | + | } |
|
| 193 | + | }; |
|
| 194 | + | let (st, loc) = redirect("/andy/demo/compare").await; |
|
| 195 | + | assert!(st.is_redirection() && loc == "/andy/demo/compare/main...feature", "the newest other bookmark against the default: {loc}"); |
|
| 196 | + | let (_, loc) = redirect("/andy/demo/compare?base=v0.1&head=+feature+").await; |
|
| 197 | + | assert_eq!(loc, "/andy/demo/compare/v0.1...feature"); |
|
| 179 | 198 | ||
| 180 | 199 | // Bookmarks, tags, conflicts (none), trust (no policy), pushes. |
|
| 181 | 200 | let bm = page(&h.app, "/andy/demo/bookmarks").await; |
|
| … | 60 unchanged lines |
modified crates/hub/tests/proposals.rs +11 −0
| … | 132 unchanged lines |
||
| 133 | 133 | let feat = client.commit("add three", &[]).unwrap(); |
|
| 134 | 134 | assert!(client.push_bookmark("origin", "feature", false).unwrap().iter().all(|r| r.ok)); |
|
| 135 | 135 | ||
| 136 | + | // The compare page offers the proposal to writers only (hub spec §6.3). |
|
| 137 | + | let cmp = get(app, "/andy/demo/compare/main...feature", Some(&andy)).await; |
|
| 138 | + | assert!(cmp.body.contains("href=\"/andy/demo/proposals/new?source=feature&target=main\">Propose this"), "{}", cmp.body); |
|
| 139 | + | assert!(!get(app, "/andy/demo/compare/main...feature", Some(&bob)).await.body.contains("Propose this")); |
|
| 140 | + | assert!( |
|
| 141 | + | !get(app, "/andy/demo/compare/feature...main", Some(&andy)).await.body.contains("Propose this"), |
|
| 142 | + | "nothing to propose the other way" |
|
| 143 | + | ); |
|
| 144 | + | ||
| 136 | 145 | // Bob previews and opens a proposal feature → main. |
|
| 137 | 146 | let preview = get(app, "/andy/demo/proposals/new?source=feature&target=main", Some(&bob)).await; |
|
| 138 | 147 | assert_eq!(preview.status, StatusCode::OK); |
|
| … | 8 unchanged lines |
||
| 147 | 156 | assert!(page.body.contains("Add three") && page.body.contains("1 change"), "{}", page.body); |
|
| 148 | 157 | assert!(page.body.contains("Please")); |
|
| 149 | 158 | assert!(get(app, "/andy/demo/proposals", None).await.body.contains("#1")); |
|
| 159 | + | let cmp = get(app, "/andy/demo/compare/main...feature", Some(&andy)).await; |
|
| 160 | + | assert!(cmp.body.contains("proposal #1 is open") && !cmp.body.contains("Propose this"), "the open proposal instead"); |
|
| 150 | 161 | ||
| 151 | 162 | // A comment anchored to line 3 of a.txt lands in the thread on the files tab. |
|
| 152 | 163 | let files = get(app, "/andy/demo/proposal/1?tab=files", Some(&bob)).await; |
|
| … | 108 unchanged lines |
modified docs/spec.md +1 −1
| … | 233 unchanged lines |
||
| 234 | 234 | | `/{owner}/{name}/file/{ref}/{path}` | File view, `?plain=1` raw, `?blame=1` blame (§6.6) | |
|
| 235 | 235 | | `/{owner}/{name}/changes/{ref}` | History (§6.7) | |
|
| 236 | 236 | | `/{owner}/{name}/change/{hash}` | Change page (§6.8) | |
|
| 237 | - | | `/{owner}/{name}/compare/{a}...{b}` | Changes in `b` not in `a`, and the diff between the two states | |
|
| 237 | + | | `/{owner}/{name}/compare/{a}...{b}` | Changes in `b` not in `a`, and the diff between the two states. Each side is picked from the bookmarks and tags, or typed as a change or state; a swap link reverses them. When both are bookmarks, `b` has changes `a` lacks and the viewer can write, **Propose this** opens a new proposal of `b` into `a` (or links the one already open). `/compare` alone compares the most recently changed other bookmark against the default one; `/compare?base=…&head=…` is the pickers' form | |
|
| 238 | 238 | | `/{owner}/{name}/bookmarks` | Bookmarks with heads, ahead/behind the default, last update, attestation status | |
|
| 239 | 239 | | `/{owner}/{name}/conflicts` | Every bookmark whose state carries conflicts, each with its regions (§6.10) | |
|
| 240 | 240 | | `/{owner}/{name}/proposals`, `/proposal/{n}` | §7. The list filters by state and by `q` (`author:`, `label:`, `target:` and words), sorts by recently updated (default), newest or oldest, and pages 25 at a time | |
|
| … | 314 unchanged lines |
modified templates/compare.html +10 −2
| 1 | 1 | {% extends "base.html" %} |
|
| 2 | 2 | {% import "repo_tabs.html" as m %} |
|
| 3 | - | {% block title %}Compare {{ cmp.base.label|short }}...{{ cmp.head.label|short }} · {{ repo.owner }}/{{ repo.name }}{% endblock %} |
|
| 3 | + | {% block title %}Compare {{ cmp.base.short_label() }}...{{ cmp.head.short_label() }} · {{ repo.owner }}/{{ repo.name }}{% endblock %} |
|
| 4 | 4 | {% block bodyattrs %} data-repo="{{ repo.url }}"{% endblock %} |
|
| 5 | 5 | {% block tabs %}{% call m::tabs(repo, "changes") %}{% endblock %} |
|
| 6 | 6 | {% block mainclass %} class="wide"{% endblock %} |
|
| 7 | 7 | {% block content %} |
|
| 8 | - | <div class="head"><h1>Compare <small>{{ cmp.base.label }} … {{ cmp.head.label }}</small></h1></div> |
|
| 8 | + | <div class="head"><h1>Compare <small>{{ cmp.base.short_label() }} … {{ cmp.head.short_label() }}</small></h1></div> |
|
| 9 | + | <form class="compare-bar" method="get" action="{{ repo.url }}/compare"> |
|
| 10 | + | <label>base <input type="text" name="base" value="{{ base_spec }}" list="compare-refs" aria-label="Base: bookmark, tag, change or state" required></label> |
|
| 11 | + | <a class="swap" href="{{ repo.url }}/compare/{{ head_spec }}...{{ base_spec }}" title="Swap the two sides" aria-label="Swap the two sides">⇄</a> |
|
| 12 | + | <label>head <input type="text" name="head" value="{{ head_spec }}" list="compare-refs" aria-label="Head: bookmark, tag, change or state" required></label> |
|
| 13 | + | <button type="submit">COMPARE</button> |
|
| 14 | + | {% if let Some(p) = propose %}{% if let Some(n) = p.existing %}<a class="go small" href="{{ repo.url }}/proposal/{{ n }}">proposal #{{ n }} is open</a>{% else %}<a class="go" href="{{ p.url }}">Propose this</a>{% endif %}{% endif %} |
|
| 15 | + | <datalist id="compare-refs">{% for b in bookmarks %}<option value="{{ b }}">bookmark</option>{% endfor %}{% for t in tags %}<option value="{{ t }}">tag</option>{% endfor %}</datalist> |
|
| 16 | + | </form> |
|
| 9 | 17 | <p class="lead">{{ cmp.added.len()|plural("change", "changes") }} in <strong>{{ cmp.head.label }}</strong> not in <strong>{{ cmp.base.label }}</strong>{% if cmp.missing > 0 %}; {{ cmp.missing }} the other way{% endif %}. {{ files.len()|plural("file", "files") }} differ between the two states.</p> |
|
| 10 | 18 | {% for c in cmp.added %}{% call m::change_row(repo, c) %}{% endfor %} |
|
| 11 | 19 | {% call m::diff(files, ignore_ws, "") %} |
|
| … | 1 unchanged lines |
modified templates/style.css +8 −0
| … | 444 unchanged lines |
||
| 445 | 445 | pre.flow-log .s-err { color: color-mix(in srgb, var(--bad) 75%, var(--text)); } |
|
| 446 | 446 | pre.flow-log .s-flow, pre.flow-log .s-console { color: var(--dim); font-style: italic; } |
|
| 447 | 447 | table.flow-runs td.st { width: 1.5em; } |
|
| 448 | + | ||
| 449 | + | /* Compare pickers (hub spec §6.3). */ |
|
| 450 | + | .compare-bar { display: flex; flex-wrap: wrap; align-items: center; gap: 10px; margin: 0 0 14px; } |
|
| 451 | + | .compare-bar label { display: flex; align-items: center; gap: 6px; font-size: 13px; color: var(--mute); margin: 0; } |
|
| 452 | + | .compare-bar input { width: 16em; margin: 0; } |
|
| 453 | + | .compare-bar .swap { font-size: 18px; padding: 0 4px; } |
|
| 454 | + | .compare-bar .swap:hover { text-decoration: none; color: var(--ink); } |
|
| 455 | + | .compare-bar button { margin: 0; } |