max / makenotwork
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
7 files changed,
+113 insertions,
-11 deletions
| @@ -7183,6 +7183,16 @@ | |||
| 7183 | 7183 | } | |
| 7184 | 7184 | .git-note-namespace { font-family: var(--font-mono); } | |
| 7185 | 7185 | .git-note-attribution { opacity: 0.6; } | |
| 7186 | + | .git-note-badge { | |
| 7187 | + | font-size: var(--text-fine); | |
| 7188 | + | padding: var(--gap-bound) var(--gap-peer); | |
| 7189 | + | border: 1px solid var(--border); | |
| 7190 | + | border-radius: var(--radius-fine); | |
| 7191 | + | color: var(--content); | |
| 7192 | + | text-decoration: none; | |
| 7193 | + | opacity: 0.7; | |
| 7194 | + | } | |
| 7195 | + | .git-note-badge:hover { opacity: 1; } | |
| 7186 | 7196 | .git-note-body { padding: var(--gap-section); font-size: var(--text-note); } | |
| 7187 | 7197 | .git-note-body > :first-child { margin-top: 0; } | |
| 7188 | 7198 | .git-note-body > :last-child { margin-bottom: 0; } |
| @@ -331,16 +331,20 @@ | |||
| 331 | 331 | let offset = (page - 1).saturating_mul(limit); | |
| 332 | 332 | ||
| 333 | 333 | let git_ref_c = git_ref.clone(); | |
| 334 | - | let (refs, commits) = resolved | |
| 334 | + | let (refs, commits, has_more, annotated) = resolved | |
| 335 | 335 | .with_repo(move |gix_repo| { | |
| 336 | 336 | let refs = git::list_refs(gix_repo); | |
| 337 | 337 | let commit_oid = git::resolve_ref(gix_repo, &git_ref_c)?; | |
| 338 | - | let commits = git::commit_log(gix_repo, commit_oid, limit + 1, offset)?; | |
| 339 | - | Ok((refs, commits)) | |
| 338 | + | let mut commits = git::commit_log(gix_repo, commit_oid, limit + 1, offset)?; | |
| 339 | + | // The extra row exists only to answer "is there another page" and is | |
| 340 | + | // never rendered, so drop it before asking about notes. | |
| 341 | + | let has_more = commits.len() > limit; | |
| 342 | + | commits.truncate(limit); | |
| 343 | + | let page_oids: Vec<String> = commits.iter().map(|c| c.oid.clone()).collect(); | |
| 344 | + | let annotated = notes_view::annotation_counts(gix_repo, &page_oids); | |
| 345 | + | Ok((refs, commits, has_more, annotated)) | |
| 340 | 346 | }) | |
| 341 | 347 | .await?; | |
| 342 | - | let has_more = commits.len() > limit; | |
| 343 | - | let commits: Vec<_> = commits.into_iter().take(limit).collect(); | |
| 344 | 348 | ||
| 345 | 349 | let csrf_token = get_csrf_token(&session).await; | |
| 346 | 350 | let is_owner = maybe_user.as_ref().map(|u| u.id) == Some(resolved.db_user.id); | |
| @@ -357,6 +361,7 @@ | |||
| 357 | 361 | current_ref: git_ref, | |
| 358 | 362 | refs, | |
| 359 | 363 | commits, | |
| 364 | + | annotated, | |
| 360 | 365 | page, | |
| 361 | 366 | has_more, | |
| 362 | 367 | open_issue_count, | |
| @@ -579,11 +584,11 @@ | |||
| 579 | 584 | ||
| 580 | 585 | let git_ref_c = git_ref.clone(); | |
| 581 | 586 | let path_c = path.clone(); | |
| 582 | - | let (refs, commits) = resolved | |
| 587 | + | let (refs, commits, has_more, annotated) = resolved | |
| 583 | 588 | .with_repo(move |gix_repo| { | |
| 584 | 589 | let refs = git::list_refs(gix_repo); | |
| 585 | 590 | let commit_oid = git::resolve_ref(gix_repo, &git_ref_c)?; | |
| 586 | - | let commits = git::file_commit_log( | |
| 591 | + | let mut commits = git::file_commit_log( | |
| 587 | 592 | gix_repo, | |
| 588 | 593 | commit_oid, | |
| 589 | 594 | &path_c, | |
| @@ -591,11 +596,13 @@ | |||
| 591 | 596 | offset, | |
| 592 | 597 | constants::GIT_FILE_LOG_MAX_WALK, | |
| 593 | 598 | )?; | |
| 594 | - | Ok((refs, commits)) | |
| 599 | + | let has_more = commits.len() > limit; | |
| 600 | + | commits.truncate(limit); | |
| 601 | + | let page_oids: Vec<String> = commits.iter().map(|c| c.oid.clone()).collect(); | |
| 602 | + | let annotated = notes_view::annotation_counts(gix_repo, &page_oids); | |
| 603 | + | Ok((refs, commits, has_more, annotated)) | |
| 595 | 604 | }) | |
| 596 | 605 | .await?; | |
| 597 | - | let has_more = commits.len() > limit; | |
| 598 | - | let commits: Vec<_> = commits.into_iter().take(limit).collect(); | |
| 599 | 606 | ||
| 600 | 607 | let filename = path.rsplit('/').next().unwrap_or(&path).to_string(); | |
| 601 | 608 | let breadcrumbs = build_breadcrumbs(&path); | |
| @@ -616,6 +623,7 @@ | |||
| 616 | 623 | filename, | |
| 617 | 624 | breadcrumbs, | |
| 618 | 625 | commits, | |
| 626 | + | annotated, | |
| 619 | 627 | page, | |
| 620 | 628 | has_more, | |
| 621 | 629 | open_issue_count, |
| @@ -13,6 +13,8 @@ | |||
| 13 | 13 | //! note is decoration on a commit view; it must never be the reason the commit | |
| 14 | 14 | //! view 500s. | |
| 15 | 15 | ||
| 16 | + | use std::collections::HashMap; | |
| 17 | + | ||
| 16 | 18 | use crate::git::notes::{self, Attribution, GixEngine, Oid}; | |
| 17 | 19 | ||
| 18 | 20 | /// How far back the attribution walk may look on a detail view. Bounded because | |
| @@ -95,6 +97,57 @@ | |||
| 95 | 97 | out | |
| 96 | 98 | } | |
| 97 | 99 | ||
| 100 | + | /// How many namespaces annotate each of `targets`, keyed by the hex id the | |
| 101 | + | /// caller passed in. | |
| 102 | + | /// | |
| 103 | + | /// This is the log-page path, so it is one [`notes::notes_for_many`] call per | |
| 104 | + | /// namespace per page and never one lookup per row: `notes_for_many` flattens | |
| 105 | + | /// the notes tree once and answers every row from that. A repository with no | |
| 106 | + | /// notes costs one ref scan and stops there. | |
| 107 | + | /// | |
| 108 | + | /// Targets absent from the result carry no note; the templates ask with `get`. | |
| 109 | + | pub fn annotation_counts(repo: &gix::Repository, targets: &[String]) -> HashMap<String, usize> { | |
| 110 | + | let mut counts = HashMap::new(); | |
| 111 | + | if targets.is_empty() { | |
| 112 | + | return counts; | |
| 113 | + | } | |
| 114 | + | ||
| 115 | + | let engine = GixEngine::new(repo); | |
| 116 | + | let namespaces = match notes::list_namespaces(&engine) { | |
| 117 | + | Ok(ns) if !ns.is_empty() => ns, | |
| 118 | + | Ok(_) => return counts, | |
| 119 | + | Err(e) => { | |
| 120 | + | tracing::warn!(error = %e, "listing notes namespaces failed"); | |
| 121 | + | return counts; | |
| 122 | + | } | |
| 123 | + | }; | |
| 124 | + | ||
| 125 | + | // Keep the caller's spelling of each id: the templates look the count up by | |
| 126 | + | // the same string they printed, and re-deriving it from the Oid would make | |
| 127 | + | // that lookup depend on two hex encoders agreeing. | |
| 128 | + | let mut by_oid: HashMap<Oid, &String> = HashMap::with_capacity(targets.len()); | |
| 129 | + | for hex in targets { | |
| 130 | + | if let Ok(oid) = Oid::from_hex(hex.as_bytes()) { | |
| 131 | + | by_oid.insert(oid, hex); | |
| 132 | + | } | |
| 133 | + | } | |
| 134 | + | let oids: Vec<Oid> = by_oid.keys().copied().collect(); | |
| 135 | + | ||
| 136 | + | for ns in namespaces { | |
| 137 | + | match notes::notes_for_many(&engine, ns.tip, &oids) { | |
| 138 | + | Ok(found) => { | |
| 139 | + | for target in found.keys() { | |
| 140 | + | if let Some(hex) = by_oid.get(target) { | |
| 141 | + | *counts.entry((*hex).clone()).or_insert(0) += 1; | |
| 142 | + | } | |
| 143 | + | } | |
| 144 | + | } | |
| 145 | + | Err(e) => tracing::warn!(namespace = %ns.name, error = %e, "batch note lookup failed"), | |
| 146 | + | } | |
| 147 | + | } | |
| 148 | + | counts | |
| 149 | + | } | |
| 150 | + | ||
| 98 | 151 | fn render_attribution(a: Attribution) -> NoteAttribution { | |
| 99 | 152 | NoteAttribution { | |
| 100 | 153 | short_commit: a.note_commit.to_short_hex(SHORT_OID_LEN), | |
| @@ -172,6 +225,24 @@ | |||
| 172 | 225 | assert_eq!(a.by, "Fixture"); | |
| 173 | 226 | } | |
| 174 | 227 | ||
| 228 | + | #[test] | |
| 229 | + | fn the_badge_counts_every_namespace_annotating_a_commit() { | |
| 230 | + | let (_tmp, repo) = annotated_repo(); | |
| 231 | + | // Three ids, so this takes `notes_for_many`'s flattening branch — the | |
| 232 | + | // one the log page actually uses. | |
| 233 | + | let page = vec![TARGET.to_string(), "0".repeat(40), "1".repeat(40)]; | |
| 234 | + | let counts = annotation_counts(&repo, &page); | |
| 235 | + | ||
| 236 | + | assert_eq!(counts.get(TARGET).copied(), Some(2)); | |
| 237 | + | assert_eq!(counts.len(), 1, "unannotated commits carry no entry"); | |
| 238 | + | } | |
| 239 | + | ||
| 240 | + | #[test] | |
| 241 | + | fn a_page_of_nothing_asks_the_repository_nothing() { | |
| 242 | + | let (_tmp, repo) = annotated_repo(); | |
| 243 | + | assert!(annotation_counts(&repo, &[]).is_empty()); | |
| 244 | + | } | |
| 245 | + | ||
| 175 | 246 | #[test] | |
| 176 | 247 | fn an_unannotated_target_and_a_malformed_one_both_render_nothing() { | |
| 177 | 248 | let (_tmp, repo) = annotated_repo(); |
| @@ -1,5 +1,6 @@ | |||
| 1 | 1 | //! Templates for the public git source browser, issues, and repo settings. | |
| 2 | 2 | ||
| 3 | + | use std::collections::HashMap; | |
| 3 | 4 | use std::sync::Arc; | |
| 4 | 5 | ||
| 5 | 6 | use askama::Template; | |
| @@ -104,6 +105,9 @@ | |||
| 104 | 105 | pub current_ref: String, | |
| 105 | 106 | pub refs: Vec<git::RefInfo>, | |
| 106 | 107 | pub commits: Vec<git::CommitInfo>, | |
| 108 | + | /// Commits on this page that carry a note, mapped to how many namespaces | |
| 109 | + | /// annotate them. One batched lookup per page, never one per row. | |
| 110 | + | pub annotated: HashMap<String, usize>, | |
| 107 | 111 | pub page: usize, | |
| 108 | 112 | pub has_more: bool, | |
| 109 | 113 | pub open_issue_count: i64, | |
| @@ -191,6 +195,9 @@ | |||
| 191 | 195 | pub filename: String, | |
| 192 | 196 | pub breadcrumbs: Vec<git::Breadcrumb>, | |
| 193 | 197 | pub commits: Vec<git::CommitInfo>, | |
| 198 | + | /// Commits on this page that carry a note, mapped to how many namespaces | |
| 199 | + | /// annotate them. One batched lookup per page, never one per row. | |
| 200 | + | pub annotated: HashMap<String, usize>, | |
| 194 | 201 | pub page: usize, | |
| 195 | 202 | pub has_more: bool, | |
| 196 | 203 | pub open_issue_count: i64, |
| @@ -35,7 +35,7 @@ | |||
| 35 | 35 | </div> | |
| 36 | 36 | ||
| 37 | 37 | {% if !notes.is_empty() %} | |
| 38 | - | <div class="git-notes"> | |
| 38 | + | <div class="git-notes" id="notes"> | |
| 39 | 39 | {% for note in notes %} | |
| 40 | 40 | <div class="git-note"> | |
| 41 | 41 | <div class="git-note-header"> |
| @@ -27,6 +27,9 @@ | |||
| 27 | 27 | <span class="git-commit-oid"> | |
| 28 | 28 | <a href="/git/{{ owner }}/{{ repo_name }}/commit/{{ commit.oid }}">{{ commit.short_oid }}</a> | |
| 29 | 29 | </span> | |
| 30 | + | {% if let Some(n) = annotated.get(commit.oid) %} | |
| 31 | + | <a class="git-note-badge" href="/git/{{ owner }}/{{ repo_name }}/commit/{{ commit.oid }}#notes">{% if **n == 1 %}note{% else %}{{ n }} notes{% endif %}</a> | |
| 32 | + | {% endif %} | |
| 30 | 33 | </div> | |
| 31 | 34 | </li> | |
| 32 | 35 | {% endfor %} |
| @@ -43,6 +43,9 @@ | |||
| 43 | 43 | <span class="git-commit-oid"> | |
| 44 | 44 | <a href="/git/{{ owner }}/{{ repo_name }}/commit/{{ commit.oid }}">{{ commit.short_oid }}</a> | |
| 45 | 45 | </span> | |
| 46 | + | {% if let Some(n) = annotated.get(commit.oid) %} | |
| 47 | + | <a class="git-note-badge" href="/git/{{ owner }}/{{ repo_name }}/commit/{{ commit.oid }}#notes">{% if **n == 1 %}note{% else %}{{ n }} notes{% endif %}</a> | |
| 48 | + | {% endif %} | |
| 46 | 49 | </div> | |
| 47 | 50 | </li> | |
| 48 | 51 | {% endfor %} |