From 5497252aec1a4f43c1d294503136d8a6f083d5d8 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 9 Aug 2026 10:20:34 +0800 Subject: [PATCH] feat(scm): read a whole commit in the panel's second-level view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The graph gives up text for shape: a 260px row has room for about 26 characters beside its lanes, and this repository's commit subjects run to a median of 64. This is where the rest comes back — the full subject, the body, every ref, the parents, and the files the commit touched. The file list is two commands, not one. Git accepts `--numstat` and `--name-status` together and then quietly drops the numstat half (measured on 2.50.1), so they run separately and join on the path. It is `log -1 --first-parent` rather than `diff-tree -m --first-parent`, which does not narrow a merge: on the same git it emits one diff per parent and concatenates them, so a two-parent merge came back with every path listed twice. `log` is also how `DiffSource::Commit` walks the patch, which is what makes this list and the overlay's cards agree file for file. `DiffSource::Commit` now carries an optional label, so the overlay's header can say what the commit was about instead of eight hex digits. The label deliberately takes no part in the source's identity: `PartialEq`, `Hash` and the overlay's string probe key are all derived from one `tag()` function, because the same commit opened with a subject in hand and without one has to stay one patch, one in-flight probe and one overlay. The key used to be built from `Debug`, which would have split the probe cache the moment a label arrived. Also lifts `local_branches` out of the panel's inline `for-each-ref`, and keeps the `%(upstream)` that `parse_refs` had been asking for and throwing away. --- crates/tty7-core/src/core/git/diff.rs | 150 ++++- crates/tty7-core/src/core/git/log.rs | 468 +++++++++++++- src/ui/diff_overlay.rs | 182 +++++- src/ui/i18n/en.rs | 4 + src/ui/i18n/ja.rs | 4 + src/ui/i18n/mod.rs | 10 +- src/ui/i18n/zh.rs | 4 + src/ui/scm/detail.rs | 885 +++++++++++++++++++++++++- src/ui/scm/mod.rs | 7 +- src/ui/scm/state.rs | 40 ++ 10 files changed, 1709 insertions(+), 45 deletions(-) diff --git a/crates/tty7-core/src/core/git/diff.rs b/crates/tty7-core/src/core/git/diff.rs index 209c2991..01761bd0 100644 --- a/crates/tty7-core/src/core/git/diff.rs +++ b/crates/tty7-core/src/core/git/diff.rs @@ -28,12 +28,29 @@ pub enum Truncation { Budget, } +/// What a commit is called, for a header that would otherwise have eight hex +/// digits and nothing else to say. +/// +/// Rides along on [`DiffSource::Commit`] and takes no part in its identity — +/// see [`DiffSource::tag`]. Defined here rather than in [`log`](super::log) so +/// the dependency between the two modules stays one-way: `log` reaches into +/// `diff` for [`FileStatus`], and nothing goes back. That is also why the +/// timestamp is a bare unix second rather than an +/// [`OffsetTs`](super::log::OffsetTs) — relative time is all the header shows. +#[derive(Clone, PartialEq, Eq, Debug, Default)] +pub struct CommitLabel { + pub subject: String, + pub author: String, + /// Author time, unix seconds. Zero where it is not known. + pub at: i64, +} + /// Which patch to ask git for. /// /// The three working-tree variants are the same three questions the SCM panel /// asks — `Worktree` is what is not staged, `Staged` is what is, and `Head` is /// both at once, which is what the overlay has always shown. -#[derive(Clone, PartialEq, Eq, Hash, Debug, Default)] +#[derive(Clone, Debug, Default)] pub enum DiffSource { /// `git diff` — unstaged changes. Worktree, @@ -42,13 +59,59 @@ pub enum DiffSource { /// `git diff HEAD` — staged and unstaged together. #[default] Head, - /// One commit against its first parent. - Commit { rev: String }, + /// One commit against its first parent, and optionally what to call it. + Commit { + rev: String, + label: Option, + }, /// `base...head`: what `head` added since the two diverged. Range { base: String, head: String }, } +impl PartialEq for DiffSource { + fn eq(&self, other: &DiffSource) -> bool { + self.tag() == other.tag() + } +} + +impl Eq for DiffSource {} + +impl std::hash::Hash for DiffSource { + fn hash(&self, state: &mut H) { + self.tag().hash(state); + } +} + impl DiffSource { + /// A commit with nothing known about it yet beyond which one it is. + pub fn commit(rev: impl Into) -> DiffSource { + DiffSource::Commit { + rev: rev.into(), + label: None, + } + } + + /// The identity of the patch, with nothing on it that only affects how the + /// patch is *labelled*. + /// + /// `PartialEq`, `Hash` and the overlay's string cache key are all defined + /// from this one function, so the three cannot drift apart. The rule it + /// exists to enforce: the same commit opened from the graph (with a + /// subject in hand) and from a keybinding (without one) is one patch, one + /// in-flight probe and one overlay. A derived `PartialEq` would make them + /// two, and the derived `Debug` the overlay's key used to be built from + /// would have split the probe cache the same way. + pub fn tag(&self) -> String { + match self { + DiffSource::Worktree => "worktree".to_string(), + DiffSource::Staged => "staged".to_string(), + DiffSource::Head => "head".to_string(), + // US, which can occur in neither a refname nor an object id. + DiffSource::Commit { rev, .. } => format!("commit\u{1f}{rev}"), + DiffSource::Range { base, head } => format!("range\u{1f}{base}\u{1f}{head}"), + } + } + /// The whole argv, minus pathspecs. Every diff tty7 runs is built here so /// there is one place to read, and one place to test, what git is asked. pub fn args(&self, context: u32, ignore_whitespace: bool) -> Vec { @@ -70,7 +133,7 @@ impl DiffSource { // is no special case and no need for `--root`. `--format=` empties // the commit header, at the cost of one blank line the parser // ignores. - DiffSource::Commit { rev } => { + DiffSource::Commit { rev, .. } => { argv.extend(strings(&["log", "-p", "-1", "--format=", "--first-parent"])); argv.push(rev.clone()); } @@ -767,9 +830,7 @@ Binary files a/img.png and b/img.png differ (DiffSource::Staged, vec!["diff", "--cached"]), (DiffSource::Head, vec!["diff", "HEAD"]), ( - DiffSource::Commit { - rev: "deadbeef".into(), - }, + DiffSource::commit("deadbeef"), vec!["log", "-p", "-1", "--format=", "--first-parent", "deadbeef"], ), ( @@ -791,6 +852,73 @@ Binary files a/img.png and b/img.png differ } } + /// The one property the whole label mechanism rests on. Break it and the + /// same commit becomes two probes, two overlays and two cache entries the + /// moment one of them learns its own subject. + #[test] + fn a_commits_label_is_not_part_of_which_commit_it_is() { + let bare = DiffSource::commit("deadbeef"); + let labelled = DiffSource::Commit { + rev: "deadbeef".into(), + label: Some(CommitLabel { + subject: "fix(scm): the thing".into(), + author: "Ada".into(), + at: 1_786_255_391, + }), + }; + let other = DiffSource::Commit { + rev: "deadbeef".into(), + label: Some(CommitLabel { + subject: "something else entirely".into(), + ..Default::default() + }), + }; + + assert_eq!(bare, labelled); + assert_eq!(labelled, other); + assert_eq!(bare.tag(), labelled.tag()); + assert_eq!(hash_of(&bare), hash_of(&labelled)); + assert_eq!(hash_of(&labelled), hash_of(&other)); + // …and the argv, which is the other thing a split cache would show up + // in: two "different" sources running the identical command. + assert_eq!(argv(bare.clone()), argv(labelled)); + + // Which commit it is still separates them, of course. + let elsewhere = DiffSource::commit("cafebabe"); + assert_ne!(bare, elsewhere); + assert_ne!(hash_of(&bare), hash_of(&elsewhere)); + assert_ne!(bare, DiffSource::Head); + assert_ne!( + DiffSource::Range { + base: "a".into(), + head: "b".into() + }, + DiffSource::Range { + base: "b".into(), + head: "a".into() + } + ); + // Every variant is still equal to itself, which `Eq` promises and a + // hand-written `PartialEq` is exactly where it could stop being true. + for source in [ + DiffSource::Worktree, + DiffSource::Staged, + DiffSource::Head, + bare, + elsewhere, + ] { + assert_eq!(source, source.clone(), "{source:?}"); + assert_eq!(hash_of(&source), hash_of(&source.clone())); + } + } + + fn hash_of(source: &DiffSource) -> u64 { + use std::hash::{Hash as _, Hasher as _}; + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + source.hash(&mut hasher); + hasher.finish() + } + #[test] fn quote_path_is_off_on_every_source() { // Left on, a non-ASCII path arrives as C octal escapes that nothing @@ -802,7 +930,7 @@ Binary files a/img.png and b/img.png differ DiffSource::Worktree, DiffSource::Staged, DiffSource::Head, - DiffSource::Commit { rev: "HEAD".into() }, + DiffSource::commit("HEAD"), DiffSource::Range { base: "a".into(), head: "b".into(), @@ -915,7 +1043,7 @@ Binary files a/img.png and b/img.png differ assert!(DiffSource::Worktree.lists_untracked()); assert!(DiffSource::Head.lists_untracked()); assert!(!DiffSource::Staged.lists_untracked()); - assert!(!DiffSource::Commit { rev: "x".into() }.lists_untracked()); + assert!(!DiffSource::commit("x").lists_untracked()); assert!( !DiffSource::Range { base: "a".into(), @@ -1284,9 +1412,7 @@ index 1..2 100644 fn commit_files(host: &dyn Host, dir: &Path, spec: &str) -> Vec { let req = DiffRequest { - source: DiffSource::Commit { - rev: rev(dir, spec), - }, + source: DiffSource::commit(rev(dir, spec)), ..Default::default() }; probe_diff(host, dir, &req) diff --git a/crates/tty7-core/src/core/git/log.rs b/crates/tty7-core/src/core/git/log.rs index b0079703..f584b5ad 100644 --- a/crates/tty7-core/src/core/git/log.rs +++ b/crates/tty7-core/src/core/git/log.rs @@ -13,6 +13,7 @@ use std::path::Path; use smallvec::SmallVec; use super::RecordSplitter; +use super::diff::FileStatus; use crate::host::Host; /// Full hex object id. Kept as `String` rather than `[u8; 20]` because sha256 @@ -33,6 +34,10 @@ pub const GRAPH_PAGE: usize = 200; pub const MAX_GRAPH_COMMITS: usize = 5_000; pub const MAX_LANES: Lane = 32; pub const MAX_REFS: usize = 2_000; +/// How many changed files one commit's detail view will hold. A vendored +/// dependency landing in a single commit is tens of thousands of paths, and +/// every one of them would become a row. +pub const MAX_COMMIT_FILES: usize = 1_000; pub const MAX_SUBJECT_BYTES: usize = 512; pub const MAX_BODY_BYTES: usize = 8 * 1024; pub const MAX_LOG_BYTES: usize = 16 * 1024 * 1024; @@ -79,6 +84,10 @@ pub struct RefDeco { pub short: String, /// Carried the `HEAD -> ` prefix in `%D`. pub is_head: bool, + /// The full refname this branch tracks, where it tracks one. Only + /// [`for_each_ref`] can fill it in — `%D` says nothing about upstreams — + /// so a decoration parsed out of a `log` record always leaves it `None`. + pub upstream: Option, } #[derive(Clone, PartialEq, Eq, Debug)] @@ -496,6 +505,7 @@ fn parse_deco(text: &str) -> Vec { full: "HEAD".to_string(), short: "HEAD".to_string(), is_head: true, + upstream: None, }); continue; } @@ -524,6 +534,7 @@ fn ref_deco(full: &str, is_head: bool) -> Option { full: full.to_string(), short: short.to_string(), is_head, + upstream: None, }) } @@ -538,11 +549,10 @@ pub fn parse_refs(stdout: &[u8]) -> HashMap> { let full = fields.next().unwrap_or_default().trim(); // `%(refname:short)` is skipped in favour of stripping the prefix here, // so a ref named the same way from `%D` and from here reads identically - // in the UI. `%(upstream)` and `%(objecttype)` are asked for because - // the branch switcher will want them off the same call; neither has a - // home on `RefDeco` yet. + // in the UI. `%(objecttype)` is asked for to keep the format one line + // rather than two; only `%(*objectname)` below acts on it. let _short = fields.next(); - let _upstream = fields.next(); + let upstream = fields.next().unwrap_or_default().trim(); let head = fields.next().unwrap_or_default().trim(); let _kind = fields.next(); let peeled = fields.next().unwrap_or_default().trim(); @@ -553,7 +563,8 @@ pub fn parse_refs(stdout: &[u8]) -> HashMap> { if full.is_empty() || !is_hex_oid(target) { continue; } - if let Some(deco) = ref_deco(full, head == "*") { + if let Some(mut deco) = ref_deco(full, head == "*") { + deco.upstream = (!upstream.is_empty()).then(|| upstream.to_string()); out.entry(target.to_string()).or_default().push(deco); } } @@ -577,6 +588,264 @@ pub fn for_each_ref(host: &dyn Host, root: &Path) -> HashMap> } } +/// The local branch names, one per line, in git's own refname order. +/// +/// `for-each-ref` rather than `branch`: no porcelain warnings, no column +/// layout, and one name per line whatever the user's config says. It is a +/// separate call from [`for_each_ref`] because that one groups by the commit a +/// ref points at, which is the wrong shape for a list of branches — the +/// switcher wants every branch, including the ones sharing a tip. +pub fn local_branches(host: &dyn Host, root: &Path) -> Vec { + let count = format!("--count={MAX_REFS}"); + let args = [ + "for-each-ref", + &count, + "--format=%(refname:short)", + "refs/heads", + ]; + match host.git(root, &args) { + Ok(out) if out.success() => parse_branch_names(&out.stdout), + _ => Vec::new(), + } +} + +pub fn parse_branch_names(stdout: &[u8]) -> Vec { + String::from_utf8_lossy(stdout) + .lines() + .map(str::trim) + .filter(|name| !name.is_empty()) + .take(MAX_REFS) + .map(str::to_string) + .collect() +} + +/// One commit's metadata, for a detail view that does not already have it. +/// +/// A commit that is on screen in the graph is already a [`Commit`] in +/// [`CommitPage::commits`], and the caller is expected to hand that over +/// instead of paying for this. What is left is the case the page cannot +/// answer: a commit reached from a parent link, or from anywhere outside the +/// window the graph happens to be holding. +pub fn load_commit(host: &dyn Host, root: &Path, rev: &str) -> Option { + if !is_rev(rev) { + return None; + } + let args = [ + "-c", + "log.showSignature=false", + "show", + "--no-patch", + // Without it `%D` prints short names, and `parse_deco` reads full + // ones — every chip would come back as `RefKind::Other`. + "--decorate=full", + "--no-color", + LOG_PRETTY, + rev, + ]; + let out = host.git(root, &args).ok()?; + if !out.success() { + return None; + } + parse_log(&out.stdout).into_iter().next() +} + +/// One path a commit touched, with the line counts beside it. +/// +/// The counts are `Option` rather than `0` because "git did not say" and +/// "nothing changed" are different answers: a binary file reports neither, and +/// a pure rename reports `0 0`. +#[derive(Clone, PartialEq, Eq, Debug)] +pub struct CommitFile { + /// Repository-relative, and for a rename the *new* name. + pub path: String, + /// Where a rename or a copy came from. + pub orig_path: Option, + pub status: FileStatus, + pub added: Option, + pub removed: Option, + pub binary: bool, +} + +/// The paths one commit touched, against its first parent. +/// +/// Two commands rather than one, because git will take `--numstat` and +/// `--name-status` together and then quietly drop the numstat half — measured +/// on 2.50.1, where the combined `-z` stream comes back as pure name-status. +/// So they are run separately and joined on the path. +/// +/// `log -1 --first-parent`, *not* `diff-tree -m --first-parent`: `diff-tree` +/// does not honour `--first-parent` as a narrowing of a merge. On the same git +/// it emits one diff per parent and concatenates them, so a two-parent merge +/// comes back with a file list twice as long as the merge really is. `log` is +/// also exactly how [`DiffSource::Commit`](super::diff::DiffSource) walks the +/// patch, which is what makes this list and the overlay's cards agree file for +/// file — and it needs no `--root`, because `log` shows a root commit's +/// contents as additions without being asked. +pub fn commit_files(host: &dyn Host, root: &Path, rev: &str) -> Option> { + let numstat = commit_diff(host, root, rev, "--numstat")?; + let name_status = commit_diff(host, root, rev, "--name-status")?; + Some(join_commit_files(&numstat, &name_status)) +} + +fn commit_diff(host: &dyn Host, root: &Path, rev: &str, what: &str) -> Option> { + if !is_rev(rev) { + return None; + } + let args = [ + "-c", + "log.showSignature=false", + // Without it a non-ASCII path comes back wrapped in quotes with its + // bytes spelled as C octal escapes, and nothing here decodes those. + "-c", + "core.quotePath=false", + "log", + "-1", + "--format=", + "--first-parent", + "--no-color", + "-z", + what, + "--find-renames", + rev, + ]; + let out = host.git(root, &args).ok()?; + out.success().then_some(out.stdout) +} + +/// A rev the caller made up is still a rev git will be handed, so anything +/// that could be read as an option is refused before it gets there. +fn is_rev(rev: &str) -> bool { + !rev.is_empty() && !rev.starts_with('-') && !rev.contains(|c: char| c.is_control()) +} + +fn records(stdout: &[u8]) -> Vec { + let mut out = Vec::new(); + let mut on_record = |record: &[u8]| out.push(String::from_utf8_lossy(record).into_owned()); + let mut split = RecordSplitter::new(0); + split.push(stdout, &mut on_record); + split.finish(&mut on_record); + out +} + +/// `-z --numstat`: `\t\t\0` per file — except for a +/// rename or a copy, where the third field is *empty* and the old and the new +/// path follow as two records of their own. A binary file reports `-\t-`. +fn parse_numstat(stdout: &[u8]) -> HashMap, Option, bool)> { + let mut out = HashMap::new(); + let records = records(stdout); + let mut at = 0usize; + while at < records.len() && out.len() < MAX_COMMIT_FILES { + let record = &records[at]; + at += 1; + let mut fields = record.splitn(3, '\t'); + let (Some(added), Some(removed), Some(path)) = + (fields.next(), fields.next(), fields.next()) + else { + continue; + }; + let binary = added == "-" && removed == "-"; + let counts = (added.parse::().ok(), removed.parse::().ok()); + let path = if path.is_empty() { + let new = records.get(at + 1).cloned(); + at += 2; + match new { + Some(new) => new, + // Truncated mid-rename. Nothing else can be read from here. + None => break, + } + } else { + path.to_string() + }; + out.insert(path, (counts.0, counts.1, binary)); + } + out +} + +/// `-z --name-status`: `\0\0`, and `R\0\0\0` +/// for the two statuses that name two paths. +fn parse_name_status(stdout: &[u8]) -> Vec<(String, Option, FileStatus)> { + let mut out = Vec::new(); + let records = records(stdout); + let mut at = 0usize; + while at < records.len() && out.len() < MAX_COMMIT_FILES { + let code = records[at].trim().to_string(); + at += 1; + let two_paths = matches!(code.as_bytes().first(), Some(b'R' | b'C')); + let taken = 1 + usize::from(two_paths); + let Some(paths) = records.get(at..at + taken) else { + break; + }; + at += taken; + let Some(status) = file_status(&code) else { + continue; + }; + match paths { + [path] => out.push((path.clone(), None, status)), + [old, new] => out.push((new.clone(), Some(old.clone()), status)), + _ => {} + } + } + out +} + +fn file_status(code: &str) -> Option { + match code.as_bytes().first()? { + b'A' => Some(FileStatus::Added), + b'M' => Some(FileStatus::Modified), + b'D' => Some(FileStatus::Deleted), + b'R' => Some(FileStatus::Renamed), + b'C' => Some(FileStatus::Copied), + b'T' => Some(FileStatus::TypeChanged), + // `X` is git's own "unknown"; `B` only appears under + // `--break-rewrites`, which nothing here passes. + b'U' => Some(FileStatus::Unmerged), + _ => None, + } +} + +/// Joins the two streams on the path. +/// +/// `--name-status` is the spine: it carries the letter every row is drawn +/// from, and it is in git's own order. `--numstat` only contributes counts, so +/// losing it costs the numbers and nothing else. Losing the other way round is +/// worse — a path with counts and no letter would vanish — so anything left +/// over is appended rather than dropped. +pub fn join_commit_files(numstat: &[u8], name_status: &[u8]) -> Vec { + let mut counts = parse_numstat(numstat); + let named = parse_name_status(name_status); + let mut out: Vec = Vec::with_capacity(named.len()); + for (path, orig_path, status) in named { + let (added, removed, binary) = counts.remove(&path).unwrap_or((None, None, false)); + out.push(CommitFile { + path, + orig_path, + status, + added, + removed, + binary, + }); + } + let mut leftover: Vec<_> = counts.into_iter().collect(); + // A `HashMap` has no order to preserve, and a file list that reshuffles + // itself between two reads of the same commit would be worse than a + // list that is merely not in git's order. + leftover.sort_by(|a, b| a.0.cmp(&b.0)); + for (path, (added, removed, binary)) in leftover { + if out.len() >= MAX_COMMIT_FILES { + break; + } + out.push(CommitFile { + path, + orig_path: None, + status: FileStatus::Modified, + added, + removed, + binary, + }); + } + out +} + /// Loads the newest `count` commits of `scope` and lays them out. /// /// Paging is a bigger `-n`, never `--skip`. `--skip=M` walks and discards M @@ -1282,6 +1551,195 @@ mod tests { assert_eq!(by_oid[SHA_C][0].short, "origin/dev"); } + #[test] + fn a_branch_keeps_the_upstream_it_tracks() { + let lines = [ + format!( + "{SHA_A}\x1frefs/heads/main\x1fmain\x1frefs/remotes/origin/main\x1f*\x1fcommit\x1f" + ), + // A branch nobody has published tracks nothing, and an empty + // `%(upstream)` has to stay `None` rather than become `Some("")`. + format!("{SHA_B}\x1frefs/heads/local-only\x1flocal-only\x1f\x1f \x1fcommit\x1f"), + format!("{SHA_C}\x1frefs/tags/v9\x1fv9\x1f\x1f \x1fcommit\x1f"), + ]; + let by_oid = parse_refs(lines.join("\n").as_bytes()); + + assert_eq!( + by_oid[SHA_A][0].upstream.as_deref(), + Some("refs/remotes/origin/main") + ); + assert_eq!(by_oid[SHA_B][0].upstream, None); + assert_eq!(by_oid[SHA_C][0].upstream, None); + // `%D` cannot carry an upstream at all, so a decoration parsed out of + // a log record must not claim one. + let logged = parse_log(one(SHA_A, "", "refs/heads/main", "s", "").as_bytes()); + assert_eq!(logged[0].refs[0].upstream, None); + } + + #[test] + fn branch_names_come_back_one_per_line() { + let out = b"main\nfeature/a\n\n spaced \n"; + assert_eq!( + parse_branch_names(out), + ["main", "feature/a", "spaced"], + "blank lines are not branches, and git pads nothing" + ); + assert!(parse_branch_names(b"").is_empty()); + let many: String = (0..MAX_REFS + 50) + .map(|i| format!("b{i}\n")) + .collect::>() + .concat(); + assert_eq!(parse_branch_names(many.as_bytes()).len(), MAX_REFS); + } + + /// Every shape `-z` can produce, from the streams git actually emits — + /// each of these was captured from git 2.50.1 rather than guessed. + #[test] + fn a_commits_file_list_joins_the_two_z_streams() { + // A rename, a path with a space, a path outside ASCII, and a binary. + let numstat = b"1\t0\tbin.dat\x000\t0\t\x00a.txt\x00renamed.txt\x001\t0\twith space.txt\x002\t3\t\xe4\xb8\xad\xe6\x96\x87\xe5\x90\x8d.txt\x00"; + let name_status = b"A\x00bin.dat\x00R100\x00a.txt\x00renamed.txt\x00A\x00with space.txt\x00M\x00\xe4\xb8\xad\xe6\x96\x87\xe5\x90\x8d.txt\x00"; + let files = join_commit_files(numstat, name_status); + + assert_eq!(files.len(), 4, "{files:?}"); + assert_eq!(files[0].path, "bin.dat"); + assert_eq!(files[0].status, FileStatus::Added); + assert_eq!((files[0].added, files[0].removed), (Some(1), Some(0))); + + // The rename: `--numstat` spells it as an empty third field followed + // by two records of its own, and the counts belong to the new name. + assert_eq!(files[1].path, "renamed.txt"); + assert_eq!(files[1].orig_path.as_deref(), Some("a.txt")); + assert_eq!(files[1].status, FileStatus::Renamed); + assert_eq!((files[1].added, files[1].removed), (Some(0), Some(0))); + + assert_eq!( + files[2].path, "with space.txt", + "a space is not a separator" + ); + assert_eq!(files[3].path, "中文名.txt"); + assert_eq!((files[3].added, files[3].removed), (Some(2), Some(3))); + assert!(files.iter().all(|f| !f.binary)); + } + + #[test] + fn a_binary_file_reports_no_counts_rather_than_zero() { + let files = join_commit_files(b"-\t-\tbin2.dat\x00", b"A\x00bin2.dat\x00"); + assert_eq!(files.len(), 1); + assert!(files[0].binary); + assert_eq!( + (files[0].added, files[0].removed), + (None, None), + "`0 0` is a real answer and `-\t-` is not, so they must not read alike" + ); + // A pure rename really does change nothing, and says so. + let renamed = join_commit_files(b"0\t0\t\x00a\x00b\x00", b"R100\x00a\x00b\x00"); + assert!(!renamed[0].binary); + assert_eq!((renamed[0].added, renamed[0].removed), (Some(0), Some(0))); + } + + #[test] + fn one_stream_going_missing_degrades_instead_of_emptying_the_list() { + // No counts: every row still knows what happened to it. + let no_numstat = join_commit_files(b"", b"M\x00a.txt\x00D\x00b.txt\x00"); + assert_eq!(no_numstat.len(), 2); + assert_eq!(no_numstat[1].status, FileStatus::Deleted); + assert!(no_numstat.iter().all(|f| f.added.is_none())); + + // No letters: the paths are the more important half, so they are kept + // and given the one status that claims the least. + let no_names = join_commit_files(b"1\t2\tb.txt\x003\t4\ta.txt\x00", b""); + assert_eq!( + no_names.iter().map(|f| f.path.as_str()).collect::>(), + ["a.txt", "b.txt"], + "with no order to inherit the leftovers are sorted, not shuffled" + ); + assert!(no_names.iter().all(|f| f.status == FileStatus::Modified)); + assert_eq!((no_names[0].added, no_names[0].removed), (Some(3), Some(4))); + + assert!(join_commit_files(b"", b"").is_empty()); + } + + #[test] + fn a_truncated_or_unknown_record_is_dropped_rather_than_shifting_the_parse() { + // A status letter with no path behind it ends the read; anything + // already parsed still stands. + let cut = join_commit_files(b"", b"M\x00a.txt\x00D\x00"); + assert_eq!(cut.len(), 1); + assert_eq!(cut[0].path, "a.txt"); + + // `X` is git's own "something went wrong". Its path is consumed so the + // records after it stay aligned. + let unknown = join_commit_files(b"", b"X\x00weird\x00A\x00good.txt\x00"); + assert_eq!(unknown.len(), 1); + assert_eq!(unknown[0].path, "good.txt"); + + // A rename cut off after its old name leaves nothing to attach to. + assert!(join_commit_files(b"0\t0\t\x00a\x00", b"R100\x00a\x00").is_empty()); + } + + #[test] + fn a_file_list_is_capped_without_losing_its_first_rows() { + let mut numstat = Vec::new(); + let mut name_status = Vec::new(); + for i in 0..MAX_COMMIT_FILES + 20 { + numstat.extend_from_slice(format!("1\t0\tf{i:05}.rs\0").as_bytes()); + name_status.extend_from_slice(format!("A\0f{i:05}.rs\0").as_bytes()); + } + let files = join_commit_files(&numstat, &name_status); + assert_eq!(files.len(), MAX_COMMIT_FILES); + assert_eq!(files[0].path, "f00000.rs"); + } + + #[test] + fn a_rev_that_could_be_read_as_an_option_never_reaches_git() { + assert!(is_rev("HEAD")); + assert!(is_rev(SHA_A)); + assert!(is_rev("v1.0^{commit}")); + assert!(!is_rev("")); + assert!(!is_rev("--upload-pack=touch /tmp/pwned")); + assert!(!is_rev("-n")); + assert!(!is_rev("HEAD\nrm -rf")); + } + + #[test] + fn this_repo_answers_for_one_commit_and_its_files() { + let host = crate::host::local::LocalHost::new(); + let here = Path::new(env!("CARGO_MANIFEST_DIR")); + // A source tarball is a legitimate place to run the tests from. + let Some(page) = load_page(&*host, here, &GraphScope::Head, 2) else { + return; + }; + let Some(head) = page.commits.first() else { + return; + }; + + let shown = load_commit(&*host, here, &head.oid).expect("HEAD is a commit"); + assert_eq!(shown.oid, head.oid); + assert_eq!(shown.summary, head.summary, "the two formats are the same"); + assert_eq!(shown.parents.as_slice(), head.parents.as_slice()); + assert_eq!(shown.author.at, head.author.at); + assert_eq!(load_commit(&*host, here, "-n"), None); + + let files = commit_files(&*host, here, &head.oid).expect("HEAD touched something"); + assert!(!files.is_empty(), "no commit in this repo is empty"); + assert!( + files.iter().all(|f| !f.path.is_empty()), + "an empty path means the join lost a record: {files:?}" + ); + // The whole reason the two commands are run separately. + assert!( + files + .iter() + .any(|f| f.added.is_some() || f.removed.is_some() || f.binary), + "not one row got its counts: {files:?}" + ); + + let branches = local_branches(&*host, here); + assert!(!branches.is_empty(), "this checkout is on a branch"); + assert!(branches.iter().all(|b| !b.starts_with("refs/heads/"))); + } + #[test] fn this_repo_lays_out_one_row_per_commit() { let host = crate::host::local::LocalHost::new(); diff --git a/src/ui/diff_overlay.rs b/src/ui/diff_overlay.rs index 3dc13f15..ab80d3e3 100644 --- a/src/ui/diff_overlay.rs +++ b/src/ui/diff_overlay.rs @@ -12,8 +12,8 @@ use gpui_component::{ActiveTheme as _, Icon, IconName, Sizable as _, h_flex, v_f use crate::core::config::{Config, DiffViewMode}; use crate::core::git::status::DecoStatus; use crate::terminal::git_diff::{ - self, AUTO_COLLAPSE_LINES, DiffSnapshot, DiffSource, DiffStats, FileDiff, FileStatus, LineKind, - MAX_RENDERED_FILES, Truncation, + self, AUTO_COLLAPSE_LINES, CommitLabel, DiffSnapshot, DiffSource, DiffStats, FileDiff, + FileStatus, LineKind, MAX_RENDERED_FILES, Truncation, }; use crate::ui::app::Tty7App; use crate::ui::diff_rows::{Side, SplitCell, SplitRow, UnifiedRow, split_hunk, unified_rows}; @@ -21,6 +21,7 @@ use crate::ui::i18n::{L10nKey, t, t_fmt, t_plural}; use crate::ui::right_panel::info_chip; use crate::ui::rounding; use crate::ui::rounding::RoundedCorners as _; +use crate::ui::scm::path::relative_time; use crate::ui::scm::status::{status_color, status_glyph}; pub(crate) enum DiffLoad { @@ -444,6 +445,27 @@ impl Tty7App { &mono, )) }) + // The subject takes the slack the spacer below would otherwise + // have, which is why that one is skipped when a label is present: + // two `flex_1` siblings split the line in half and the subject + // would truncate with empty space beside it. + .when_some(subject.label.as_ref(), |bar, label| { + bar.child( + div() + .flex_1() + .min_w_0() + .truncate() + .text_sm() + .child(SharedString::from(label.subject.clone())), + ) + .child( + div() + .flex_shrink_0() + .text_xs() + .text_color(cx.theme().muted_foreground) + .child(label_byline(label, now_unix())), + ) + }) .when_some(focused_name(overlay), |bar, name| { bar.child( div().occlude().flex_shrink_0().child( @@ -523,7 +545,7 @@ impl Tty7App { ) }, ) - .child(div().flex_1()) + .when(subject.label.is_none(), |bar| bar.child(div().flex_1())) .child(div().occlude().flex_shrink_0().child({ let sf = cx.global::().window; let selected = usize::from(view_mode(cx) == DiffViewMode::Unified); @@ -1062,7 +1084,7 @@ fn unified_marker(kind: LineKind) -> &'static str { /// `Copied` and `TypeChanged` have no decoration of their own — porcelain v2's /// index folds them the same way — so they take the nearest one rather than /// inventing a `C` and a `T` that appear in the overlay and nowhere else. -fn deco_status(status: FileStatus) -> DecoStatus { +pub(crate) fn deco_status(status: FileStatus) -> DecoStatus { match status { FileStatus::Added => DecoStatus::Added, FileStatus::Modified => DecoStatus::Modified, @@ -1089,6 +1111,10 @@ struct SourceSubject { /// Set only where the branch name alone would be ambiguous. chip: Option<&'static str>, is_rev: bool, + /// What the commit is *about*, where whoever opened it knew. An object id + /// is an address, not a name, and a header with nothing but eight hex + /// digits leaves the reader to remember which commit that was. + label: Option, } fn source_subject(source: &DiffSource, branch: String) -> SourceSubject { @@ -1097,6 +1123,7 @@ fn source_subject(source: &DiffSource, branch: String) -> SourceSubject { text: branch.clone(), chip, is_rev: false, + label: None, }; match source { // Worktree and Head are both "the branch, right now"; the header for @@ -1105,21 +1132,45 @@ fn source_subject(source: &DiffSource, branch: String) -> SourceSubject { // Staged is the branch too, but a patch that does not match the files // on disk — without the chip it is indistinguishable from the above. DiffSource::Staged => branch_of(Some("STAGED")), - DiffSource::Commit { rev } => SourceSubject { + DiffSource::Commit { rev, label } => SourceSubject { icon: "icons/git-commit.svg", text: short_rev(rev), chip: None, is_rev: true, + // An empty subject is no more use than no label at all, and a + // `Default::default()` that leaked through would render as one. + label: label.clone().filter(|l| !l.subject.is_empty()), }, DiffSource::Range { base, head } => SourceSubject { icon: "icons/git-commit.svg", text: format!("{}…{}", short_rev(base), short_rev(head)), chip: None, is_rev: true, + label: None, }, } } +/// `Ada · 2h`, the byline under a commit's subject. +/// +/// One string rather than two elements: the separator has to disappear along +/// with whichever half is missing, and a `when_some` chain around a middle dot +/// says less than this does. +fn label_byline(label: &CommitLabel, now: i64) -> String { + let when = (label.at > 0).then(|| relative_time(now, label.at)); + match (label.author.trim(), when) { + ("", Some(when)) => when, + (author, Some(when)) => format!("{author} · {when}"), + (author, None) => author.to_string(), + } +} + +fn now_unix() -> i64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_or(0, |d| d.as_secs() as i64) +} + /// Object ids get cut to eight characters; anything else is already a name a /// person chose, and cutting `origin/main` in half would only hide which it is. fn short_rev(rev: &str) -> String { @@ -1191,15 +1242,19 @@ fn oversized_summary(snap: &DiffSnapshot, stats: &DiffStats) -> String { /// The de-duplication sets on `Tty7App` are keyed by `(HostId, PathBuf)`, so /// the source rides along inside the path: two sources over one directory are -/// two independent probes and must not cancel one another. `Debug` is what -/// makes the tag unique — it carries the rev of a `Commit` and both ends of a -/// `Range` — and the separator is a byte no path contains. +/// two independent probes and must not cancel one another. +/// +/// `DiffSource::tag` rather than `Debug`, which is what this used to be built +/// from. `Debug` prints a commit's label too, so the same commit opened with a +/// subject in hand and without one would have been two keys and two probes for +/// one patch — the same split `DiffSource`'s own `PartialEq` is written to +/// avoid. The separator is a byte no path contains. fn probe_key( host: crate::ui::host_ops::HostId, cwd: &Path, source: &DiffSource, ) -> (crate::ui::host_ops::HostId, PathBuf) { - let mut tagged = std::ffi::OsString::from(format!("{source:?}\u{1}")); + let mut tagged = std::ffi::OsString::from(format!("{}\u{1}", source.tag())); tagged.push(cwd.as_os_str()); (host, PathBuf::from(tagged)) } @@ -1227,8 +1282,8 @@ mod tests { assert_ne!(worktree, probe_key(host, cwd, &DiffSource::Staged)); assert_ne!(worktree, probe_key(host, cwd, &DiffSource::Head)); assert_ne!( - probe_key(host, cwd, &DiffSource::Commit { rev: "a".into() }), - probe_key(host, cwd, &DiffSource::Commit { rev: "b".into() }), + probe_key(host, cwd, &DiffSource::commit("a")), + probe_key(host, cwd, &DiffSource::commit("b")), "two commits are two probes" ); assert_eq!(worktree, probe_key(host, cwd, &DiffSource::Worktree)); @@ -1236,6 +1291,24 @@ mod tests { worktree, probe_key(host, Path::new("/other"), &DiffSource::Worktree) ); + // …and one commit is one probe however much is known about it. Built + // from `Debug`, as this key once was, the labelled one would have been + // a second in-flight probe for a patch already being read. + assert_eq!( + probe_key(host, cwd, &DiffSource::commit("a")), + probe_key( + host, + cwd, + &DiffSource::Commit { + rev: "a".into(), + label: Some(CommitLabel { + subject: "s".into(), + author: "Ada".into(), + at: 1, + }), + } + ) + ); } #[test] @@ -1307,9 +1380,7 @@ mod tests { ); let commit = source_subject( - &DiffSource::Commit { - rev: "3f2a1b9c8d7e6f5a4b3c2d1e0f9a8b7c6d5e4f3a".into(), - }, + &DiffSource::commit("3f2a1b9c8d7e6f5a4b3c2d1e0f9a8b7c6d5e4f3a"), branch(), ); assert_eq!(commit.icon, COMMIT_ICON); @@ -1327,6 +1398,83 @@ mod tests { assert_eq!(range.text, "main…feature"); } + #[test] + fn a_labelled_commit_says_what_it_was_about() { + let label = CommitLabel { + subject: "fix(scm): stop the panel asking twice".into(), + author: "Ada".into(), + at: 1_786_255_391, + }; + let with = source_subject( + &DiffSource::Commit { + rev: "3f2a1b9c8d7e6f5a4b3c2d1e0f9a8b7c6d5e4f3a".into(), + label: Some(label.clone()), + }, + "main".to_string(), + ); + assert_eq!(with.text, "3f2a1b9c", "the sha is still the identifier"); + assert_eq!( + with.label.as_ref().map(|l| l.subject.as_str()), + Some(label.subject.as_str()) + ); + + // Nothing else grows a subject line, least of all a working-tree + // patch, whose "subject" would be a branch name repeated. + assert!( + source_subject(&DiffSource::Worktree, "main".into()) + .label + .is_none() + ); + assert!( + source_subject(&DiffSource::Head, "main".into()) + .label + .is_none() + ); + assert!( + source_subject(&DiffSource::commit("deadbeef"), "main".into()) + .label + .is_none(), + "a commit nobody has read yet has nothing to say" + ); + // A default-constructed label is indistinguishable from none, and must + // not paint an empty row where the subject would go. + let empty = source_subject( + &DiffSource::Commit { + rev: "deadbeef".into(), + label: Some(CommitLabel::default()), + }, + "main".into(), + ); + assert!(empty.label.is_none()); + } + + #[test] + fn the_byline_drops_the_separator_along_with_the_half_it_joined() { + let now = 1_786_255_391 + 7200; + let full = CommitLabel { + subject: "s".into(), + author: "Ada".into(), + at: 1_786_255_391, + }; + assert_eq!(label_byline(&full, now), "Ada · 2h"); + assert_eq!( + label_byline( + &CommitLabel { + author: String::new(), + ..full.clone() + }, + now + ), + "2h", + "a commit with no author is not `· 2h`" + ); + assert_eq!( + label_byline(&CommitLabel { at: 0, ..full }, now), + "Ada", + "and a timestamp that would not parse is not `Ada · 56y`" + ); + } + const BRANCH_ICON: &str = "icons/git-branch.svg"; const COMMIT_ICON: &str = "icons/git-commit.svg"; @@ -1868,8 +2016,14 @@ mod overlay_gpui_tests { DiffSource::Worktree, DiffSource::Staged, DiffSource::Head, + DiffSource::commit("3f2a1b9c8d7e6f5a4b3c2d1e0f9a8b7c6d5e4f3a"), DiffSource::Commit { rev: "3f2a1b9c8d7e6f5a4b3c2d1e0f9a8b7c6d5e4f3a".into(), + label: Some(CommitLabel { + subject: "fix(scm): read a commit's own header".into(), + author: "Ada".into(), + at: 1_786_255_391, + }), }, DiffSource::Range { base: "main".into(), diff --git a/src/ui/i18n/en.rs b/src/ui/i18n/en.rs index 6e1c8200..5317e04e 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -857,6 +857,10 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::ScmResetToCommit => "Reset to Commit", L10nKey::ScmRefresh => "Refresh", L10nKey::ScmBackToChanges => "Back", + L10nKey::ScmCommitParents => "Parents", + L10nKey::ScmShowMore => "Show more", + L10nKey::ScmShowLess => "Show less", + L10nKey::ScmCommitNotFound => "This commit is not in this repository.", L10nKey::ScmTooManyChanges => "Showing the first {shown} of {total} changes.", L10nKey::ScmOpenChanges => "Open Changes", L10nKey::ScmDiscardAllConfirm => { diff --git a/src/ui/i18n/ja.rs b/src/ui/i18n/ja.rs index 4a782126..7b1dbf32 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -907,6 +907,10 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { L10nKey::ScmResetToCommit => "このコミットにリセット", L10nKey::ScmRefresh => "更新", L10nKey::ScmBackToChanges => "戻る", + L10nKey::ScmCommitParents => "親コミット", + L10nKey::ScmShowMore => "続きを表示", + L10nKey::ScmShowLess => "折りたたむ", + L10nKey::ScmCommitNotFound => "このリポジトリにそのコミットはありません。", L10nKey::ScmTooManyChanges => { "変更が多いため、{total} 件のうち先頭 {shown} 件のみ表示しています。" } diff --git a/src/ui/i18n/mod.rs b/src/ui/i18n/mod.rs index 81cf061a..95df2ccd 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -683,6 +683,13 @@ pub enum L10nKey { ScmResetToCommit, ScmRefresh, ScmBackToChanges, + /// Header over the parent links in the commit detail view. + ScmCommitParents, + /// The fold toggle under a long commit body. + ScmShowMore, + ScmShowLess, + /// The detail view asked git for a commit and git did not have one. + ScmCommitNotFound, /// Shown when the working tree has more changes than the status parser /// keeps. The list is still useful; the count at the top would otherwise /// be a lie. @@ -1135,12 +1142,10 @@ const SCM_KEYS_AWAITING_A_CALLER: &[L10nKey] = &[ L10nKey::ScmGraphAllBranches, L10nKey::ScmGraphEmpty, L10nKey::ScmCommitDetailTitle, - L10nKey::ScmCopyCommitSha, L10nKey::ScmCherryPick, L10nKey::ScmRevertCommit, L10nKey::ScmResetToCommit, L10nKey::ScmRefresh, - L10nKey::ScmBackToChanges, L10nKey::ScmTooManyChanges, L10nKey::ScmOpenChanges, L10nKey::ScmDiscardAllConfirm, @@ -1152,7 +1157,6 @@ const SCM_KEYS_AWAITING_A_CALLER: &[L10nKey] = &[ L10nKey::ScmOpBisect, L10nKey::ScmOpAm, L10nKey::ScmSwitchRepository, - L10nKey::ScmFilesChanged, L10nKey::DiffViewSplit, L10nKey::DiffViewUnified, ]; diff --git a/src/ui/i18n/zh.rs b/src/ui/i18n/zh.rs index e23182a7..5d662d8d 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -830,6 +830,10 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::ScmResetToCommit => "重置到该提交", L10nKey::ScmRefresh => "刷新", L10nKey::ScmBackToChanges => "返回", + L10nKey::ScmCommitParents => "父提交", + L10nKey::ScmShowMore => "展开", + L10nKey::ScmShowLess => "收起", + L10nKey::ScmCommitNotFound => "本仓库中没有这个提交。", L10nKey::ScmTooManyChanges => "改动过多，仅显示前 {shown} 项（共 {total} 项）。", L10nKey::ScmOpenChanges => "查看改动", L10nKey::ScmDiscardAllConfirm => "放弃本仓库的全部改动？此操作无法撤销。", diff --git a/src/ui/scm/detail.rs b/src/ui/scm/detail.rs index ea6c972a..323f8939 100644 --- a/src/ui/scm/detail.rs +++ b/src/ui/scm/detail.rs @@ -8,20 +8,891 @@ //! //! The patch itself still goes to the full-screen overlay. 260px is not a //! place to read a diff. +//! +//! This is also where the panel pays back what the graph gave up. A history +//! row has about 26 characters beside its lanes and this repository's subjects +//! run to a median of 64, so the graph shows shape and this shows text: the +//! whole subject, the body, every ref, the parents, and the files. -use gpui::{AnyElement, Context}; +use std::sync::Arc; -use crate::ui::app::Tty7App; -use crate::ui::scm::state::CommitDetailView; +use gpui::{AnyElement, Context, SharedString, Window, div, prelude::*, px}; +use gpui_component::{ActiveTheme as _, Icon, IconName, Sizable as _, h_flex, v_flex}; + +use tty7_core::core::git::diff::CommitLabel; +use tty7_core::core::git::log::{Commit, CommitFile, RefKind}; +use tty7_core::core::git::status::DecoStatus; + +use crate::terminal::git_diff::DiffSource; +use crate::ui::app::{CONTENT_INSET, Tty7App}; +use crate::ui::i18n::{L10nKey, t, t_plural}; +use crate::ui::right_panel::{git_badge, info_chip}; +use crate::ui::scm::path::{relative_time, split_display_path}; +use crate::ui::scm::state::{CommitDetailView, RepoKey}; +use crate::ui::scm::status::{status_color, status_glyph}; + +/// A file row, the same height as the working tree's, and inset the same way. +/// The two lists sit in one column and have to read as one grid. +const ROW_H: f32 = 24.; +const ROW_INSET: f32 = 4.; + +/// How much of the body is shown before it folds. Four lines is a paragraph; +/// past that it is a changelog, and the file list is what the reader came for. +const BODY_LINES: usize = 4; + +/// And how much of the subject, which wraps rather than folding. Three lines +/// of 12px in 260px is around 90 characters — longer than every subject in +/// this repository but a handful, and a cap for the ones that are a paragraph. +const SUBJECT_LINES: usize = 3; impl Tty7App { + /// Show one commit, replacing the working tree in the panel body. + /// + /// `seed` is the commit the caller already has. The graph's page carries + /// every field this view renders, so a click on a row hands its own + /// [`Commit`] over and no `git show` is run at all; a parent link, or + /// anything else reaching a commit outside that window, passes `None` and + /// pays for the read. + pub(crate) fn open_commit_detail( + &mut self, + repo: RepoKey, + oid: String, + seed: Option, + cx: &mut Context, + ) { + self.scm.detail = Some(CommitDetailView::new(repo, oid, seed)); + cx.notify(); + } + + pub(crate) fn close_commit_detail(&mut self, cx: &mut Context) { + if self.scm.detail.take().is_some() { + cx.notify(); + } + } + /// The commit detail body, shown in place of the file groups. pub(crate) fn render_commit_detail( &mut self, - _detail: &CommitDetailView, - _window: &mut gpui::Window, - _cx: &mut Context, + detail: &CommitDetailView, + _window: &mut Window, + cx: &mut Context, ) -> Option { - None + // The detail names its own repository, and the panel may since have + // followed the active pane somewhere else. A commit from a repository + // nobody is looking at any more is not a second-level view of + // anything, so it goes rather than sitting on top of the wrong tree. + if self.scm.active_repo() != Some(&detail.repo) { + self.scm.detail = None; + return None; + } + self.load_commit_detail(detail, cx); + + let mono = cx.theme().mono_font_family.clone(); + let muted = cx.theme().muted_foreground; + // Each section insets itself rather than sharing one on the column: + // `panel_subtitle` applies `CONTENT_INSET` of its own, and an outer + // inset would push it eight pixels right of the rows beneath it. + let mut body = v_flex() + .py(px(2.)) + .child(self.detail_header_row(detail, &mono, cx)); + + match detail.commit.as_deref() { + Some(commit) => { + body = body + .child(self.detail_message(detail, commit, cx)) + .children(self.detail_refs(commit, &mono, cx)) + .children(self.detail_parents(detail, commit, &mono, cx)) + .child(self.detail_files(detail, commit, &mono, cx)); + } + // Nothing came back. `loaded` is what tells "still reading" apart + // from "git has no such commit here" — without it a bad oid would + // read as a spinner that never stops. + None => { + body = body.child( + div() + .px(px(CONTENT_INSET)) + .py(px(4.)) + .text_size(px(12.)) + .text_color(muted) + .child(if detail.loaded { + t(L10nKey::ScmCommitNotFound) + } else { + t(L10nKey::PanelLoading) + }), + ); + } + } + Some(body.into_any_element()) + } + + /// Read the commit and its file list, once. + /// + /// Runs from `render`, so it has to be idempotent in the strongest sense: + /// the panel is redrawn on every status change and a second dispatch would + /// mean a `git show` per frame. `loading` covers the window while a read + /// is out and `loaded` covers every frame after it lands, including the + /// ones where it landed with nothing. + fn load_commit_detail(&mut self, detail: &CommitDetailView, cx: &mut Context) { + if detail.loading || detail.loaded { + return; + } + let Some(host) = crate::ui::host_registry::HostRegistry::get(cx, detail.repo.host) else { + return; + }; + if let Some(open) = self.scm.detail.as_mut() { + open.loading = true; + } + let root = detail.repo.root.clone(); + let oid = detail.oid.clone(); + // A seeded view already has its metadata and only wants the files, so + // the `show` is skipped rather than run for an answer we hold. + let seeded = detail.commit.is_some(); + let key = (detail.repo.clone(), detail.oid.clone()); + crate::ui::host_ops::HostOps::run( + host, + cx, + move |h| { + use tty7_core::core::git::log; + let commit = (!seeded) + .then(|| log::load_commit(h, &root, &oid)) + .flatten(); + (commit, log::commit_files(h, &root, &oid)) + }, + move |this, (commit, files), cx| { + // The user may have gone back, or moved on to another commit, + // while the read was out. Landing it anywhere but on the view + // that asked would show one commit's files under another's + // message. + let Some(open) = this + .scm + .detail + .as_mut() + .filter(|d| (d.repo.clone(), d.oid.clone()) == key) + else { + return; + }; + open.loading = false; + open.loaded = true; + if let Some(commit) = commit { + open.commit = Some(Arc::new(commit)); + } + open.files = Some(Arc::new(files.unwrap_or_default())); + cx.notify(); + }, + ); + } + + /// The way back, and the object id. + /// + /// The back affordance belongs in `panel_title`'s trailing slot, where the + /// diff overlay puts its own. It is here instead because the title is + /// rendered by the panel and this function only produces the body — see + /// the note in `render_panel_scm`. Being the first row of the body it + /// scrolls with the content, which is the one thing lost by the move. + fn detail_header_row( + &self, + detail: &CommitDetailView, + mono: &SharedString, + cx: &mut Context, + ) -> AnyElement { + let oid = detail.oid.clone(); + h_flex() + .items_center() + .gap(px(4.)) + .h(px(ROW_H)) + .px(px(CONTENT_INSET - ROW_INSET)) + .child( + h_flex() + .id("scm-detail-back") + .items_center() + .gap(px(2.)) + .px(px(4.)) + .py(px(1.)) + .rounded_md() + .cursor_pointer() + .hover(|s| s.bg(cx.theme().list_hover)) + .on_click(cx.listener(|this, _, _window, cx| this.close_commit_detail(cx))) + .child( + Icon::new(IconName::ChevronLeft) + .small() + .text_color(cx.theme().muted_foreground), + ) + .child(div().text_xs().child(t(L10nKey::ScmBackToChanges))), + ) + .child(div().flex_1().min_w_0()) + .child( + div() + .id("scm-detail-sha") + .flex_none() + .px(px(4.)) + .py(px(1.)) + .rounded_md() + .cursor_pointer() + .hover(|s| s.bg(cx.theme().list_hover)) + .text_size(px(13.)) + .font_family(mono.clone()) + .tooltip(|window, cx| { + gpui_component::tooltip::Tooltip::new(t(L10nKey::ScmCopyCommitSha)) + .build(window, cx) + }) + .on_click(move |_, _window, cx| { + cx.write_to_clipboard(gpui::ClipboardItem::new_string(oid.clone())); + }) + .child(short_oid(&detail.oid).to_string()), + ) + .into_any_element() + } + + /// Subject, byline, body. + fn detail_message( + &self, + detail: &CommitDetailView, + commit: &Commit, + cx: &mut Context, + ) -> AnyElement { + let body = commit.body.trim(); + let lines = body.lines().count(); + let folded = !detail.body_expanded && lines > BODY_LINES; + v_flex() + .px(px(CONTENT_INSET)) + .pb(px(4.)) + .gap(px(3.)) + .child( + // Wrapping, not truncating: this view exists because the + // graph row could only show the first 26 characters. + div() + .text_size(px(12.)) + .font_weight(gpui::FontWeight::MEDIUM) + .line_clamp(SUBJECT_LINES) + .child(SharedString::from(commit.summary.clone())), + ) + .child( + div() + .text_size(px(11.)) + .text_color(cx.theme().muted_foreground) + .child(byline(commit, now_unix())), + ) + .when(!body.is_empty(), |this| { + this.child( + div() + .pt(px(2.)) + .text_size(px(11.5)) + .text_color(cx.theme().muted_foreground) + .when(folded, |d| d.line_clamp(BODY_LINES)) + .child(SharedString::from(body.to_string())), + ) + .when(lines > BODY_LINES, |this| { + this.child( + div() + .id("scm-detail-body-fold") + .w_full() + .py(px(1.)) + .cursor_pointer() + .text_size(px(11.)) + .text_color(cx.theme().info) + .on_click(cx.listener(|this, _, _window, cx| { + if let Some(open) = this.scm.detail.as_mut() { + open.body_expanded = !open.body_expanded; + cx.notify(); + } + })) + .child(t(if folded { + L10nKey::ScmShowMore + } else { + L10nKey::ScmShowLess + })), + ) + }) + }) + .into_any_element() + } + + /// Every ref pointing here, wrapped over as many lines as it takes. + /// + /// The graph row shows one chip and a `+N`; there is no reason to hide any + /// of them once there is a whole column to put them in. + fn detail_refs( + &self, + commit: &Commit, + mono: &SharedString, + cx: &mut Context, + ) -> Option { + if commit.refs.is_empty() { + return None; + } + let theme = cx.theme(); + let (accent, warning, fg, muted) = ( + theme.accent, + theme.warning, + theme.foreground, + theme.muted_foreground, + ); + let mut row = h_flex() + .flex_wrap() + .items_center() + .gap(px(4.)) + .px(px(CONTENT_INSET)) + .pb(px(6.)); + for deco in &commit.refs { + // The same three colours the graph's chips use: a tag is yellow + // because a tag is yellow everywhere in git, HEAD is emphasised, + // and everything else is quiet. + let (bg, color) = match deco.kind { + RefKind::Tag => (warning.opacity(0.16), warning), + _ if deco.is_head => (accent.opacity(0.28), fg), + _ => (accent, muted), + }; + row = row.child(info_chip(&deco.short, bg, color, mono)); + } + Some(row.into_any_element()) + } + + /// The parents, as links. Following one is the only way to walk history + /// backwards from a commit the graph's window does not reach. + fn detail_parents( + &self, + detail: &CommitDetailView, + commit: &Commit, + mono: &SharedString, + cx: &mut Context, + ) -> Option { + if commit.parents.is_empty() { + return None; + } + let mut row = h_flex() + .flex_wrap() + .items_center() + .gap(px(6.)) + .px(px(CONTENT_INSET)) + .pb(px(4.)) + .child( + div() + .text_size(px(11.)) + .text_color(cx.theme().muted_foreground) + .child(t(L10nKey::ScmCommitParents)), + ); + for parent in &commit.parents { + let repo = detail.repo.clone(); + let oid = parent.clone(); + row = row.child( + div() + .id(SharedString::from(format!("scm-detail-parent-{parent}"))) + .px(px(3.)) + .rounded(px(4.)) + .cursor_pointer() + .hover(|s| s.bg(cx.theme().list_hover)) + .text_size(px(11.)) + .font_family(mono.clone()) + .text_color(cx.theme().info) + .on_click(cx.listener(move |this, _, _window, cx| { + // No seed: a parent is by definition one step past + // whatever the caller had in hand. + this.open_commit_detail(repo.clone(), oid.clone(), None, cx); + })) + .child(short_oid(parent).to_string()), + ); + } + Some(row.into_any_element()) + } + + fn detail_files( + &self, + detail: &CommitDetailView, + commit: &Commit, + mono: &SharedString, + cx: &mut Context, + ) -> AnyElement { + let files = detail.files.clone().unwrap_or_default(); + let mut list = v_flex().child(self.panel_subtitle( + &t_plural(L10nKey::ScmFilesChanged, files.len(), &[]), + true, + None, + cx, + )); + if detail.files.is_none() { + return list + .child(self.detail_note(t(L10nKey::PanelLoading).to_string(), cx)) + .into_any_element(); + } + // The label rides along on the source so the overlay's header can say + // which commit it is showing, and it is deliberately not part of that + // source's identity — the same commit opened from here and from + // anywhere else has to stay one overlay. + let source = DiffSource::Commit { + rev: detail.oid.clone(), + label: Some(CommitLabel { + subject: commit.summary.clone(), + author: commit.author.name.clone(), + at: commit.author.at.unix, + }), + }; + // The rows sit in the working tree's own column: laid out one + // `ROW_INSET` short of `CONTENT_INSET` and padding themselves back + // out, so a hovered row's background is wider than its text. + let mut rows = v_flex().px(px(CONTENT_INSET - ROW_INSET)); + for file in files.iter() { + rows = rows.child(self.detail_file_row(detail, &source, file, mono, cx)); + } + list.child(rows).into_any_element() + } + + /// The working tree's file row, minus the hover buttons. + /// + /// A copy of `scm_file_row`, which is the wrong way round and known to be: + /// the two have to stay pixel-identical and nothing here enforces that. + /// They differ only in what they are built from — a `StatusEntry` against + /// a [`CommitFile`] — and in the buttons, so the shared version is a + /// function over `(letter, deco, path)` plus an optional trailing element. + fn detail_file_row( + &self, + detail: &CommitDetailView, + source: &DiffSource, + file: &CommitFile, + mono: &SharedString, + cx: &mut Context, + ) -> AnyElement { + let sf = cx.global::().sidebar; + let deco = crate::ui::diff_overlay::deco_status(file.status); + let (name, dir) = split_display_path(&file.path); + let selected = self.diff_overlay_focus(detail.repo.host, &detail.repo.root) + == Some(file.path.as_str()); + + h_flex() + .id(SharedString::from(format!("scm-detail-file-{}", file.path))) + .items_center() + .gap(px(8.)) + .h(px(ROW_H)) + .px(px(ROW_INSET)) + .py(px(3.)) + .rounded(px(5.)) + .cursor_pointer() + .hover(|s| s.bg(gpui::rgb(sf.hover))) + .when(selected, |s| s.bg(gpui::rgb(sf.selected))) + .on_click({ + let repo = detail.repo.clone(); + let source = source.clone(); + let path = file.path.clone(); + cx.listener(move |this, _, window, cx| { + // 260px cannot render a patch, so the file level is the + // full-screen overlay's job — the same one the working + // tree's rows open, pointed at a commit instead. + this.open_diff_overlay( + repo.host, + repo.root.clone(), + source.clone(), + Some(path.clone()), + window, + cx, + ); + }) + }) + .child(git_badge(status_glyph(deco), status_color(deco, cx), mono)) + .child( + div() + .flex_none() + .text_size(px(12.)) + .font_family(mono.clone()) + .text_color(if deco == DecoStatus::Deleted { + cx.theme().muted_foreground + } else { + cx.theme().foreground + }) + .when(deco == DecoStatus::Deleted, |s| s.line_through()) + .child(name.to_string()), + ) + .when(!dir.is_empty(), |this| { + this.child( + div() + .flex_1() + .min_w_0() + .truncate() + .text_size(px(11.)) + .text_color(cx.theme().muted_foreground.opacity(0.75)) + .child(dir.to_string()), + ) + }) + .into_any_element() + } + + fn detail_note(&self, text: String, cx: &mut Context) -> AnyElement { + div() + .px(px(CONTENT_INSET)) + .py(px(3.)) + .text_size(px(11.)) + .text_color(cx.theme().muted_foreground.opacity(0.75)) + .child(text) + .into_any_element() + } +} + +/// `Ada Lovelace · 2h`. Author, not committer: a rebase rewrites the second +/// one, and "who wrote this" is the question a reader is asking. +pub(crate) fn byline(commit: &Commit, now: i64) -> String { + let when = (commit.author.at.unix > 0).then(|| relative_time(now, commit.author.at.unix)); + match (commit.author.name.trim(), when) { + ("", Some(when)) => when, + (name, Some(when)) => format!("{name} · {when}"), + (name, None) => name.to_string(), + } +} + +/// Seven, which is what git itself prints and what the graph's rows use. +pub(crate) fn short_oid(oid: &str) -> &str { + &oid[..oid.len().min(7)] +} + +fn now_unix() -> i64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_or(0, |d| d.as_secs() as i64) +} + +#[cfg(test)] +mod tests { + use super::*; + use tty7_core::core::git::log::{OffsetTs, Signature}; + + fn commit(name: &str, at: i64) -> Commit { + Commit { + oid: "3f2a1b9c8d7e6f5a4b3c2d1e0f9a8b7c6d5e4f3a".into(), + parents: Default::default(), + author: Signature { + name: name.into(), + email: "ada@example.com".into(), + at: OffsetTs { + unix: at, + offset_minutes: 0, + }, + }, + committer: Signature { + name: "Grace".into(), + email: "grace@example.com".into(), + at: OffsetTs { + unix: at, + offset_minutes: 0, + }, + }, + summary: "s".into(), + body: String::new(), + refs: Vec::new(), + } + } + + #[test] + fn the_byline_drops_the_separator_along_with_the_half_it_joined() { + let now = 1_786_255_391 + 7200; + assert_eq!(byline(&commit("Ada", 1_786_255_391), now), "Ada · 2h"); + assert_eq!( + byline(&commit("", 1_786_255_391), now), + "2h", + "an unattributed commit is not `· 2h`" + ); + assert_eq!( + byline(&commit("Ada", 0), now), + "Ada", + "and a date that would not parse is not `Ada · 56y`" + ); + } + + #[test] + fn a_short_oid_is_the_seven_characters_git_itself_prints() { + assert_eq!( + short_oid("3f2a1b9c8d7e6f5a4b3c2d1e0f9a8b7c6d5e4f3a"), + "3f2a1b9" + ); + assert_eq!(short_oid("abc"), "abc", "a truncated oid is not padded"); + assert_eq!(short_oid(""), ""); + } +} + +/// The detail view against a real repository, drawn in a real window. +/// +/// Construction alone would prove very little: everything that can go wrong +/// here — a missing global, a theme token, a slice through the middle of a +/// character — goes wrong during layout and paint, so these arm the render +/// probe and insist something was actually drawn. +#[cfg(all(test, unix))] +mod detail_gpui_tests { + use super::*; + use crate::daemon::protocol::DaemonMsg; + use crate::ui::app::{render_probe, test_window}; + use crate::ui::host_ops::HostId; + use gpui::{Entity, TestAppContext, VisualTestContext}; + use std::path::{Path, PathBuf}; + use tty7_core::core::config::RightPanelTab; + + fn scratch(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("tty7-detail-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::canonicalize(&dir).unwrap() + } + + fn git(root: &Path, args: &[&str]) -> String { + let out = std::process::Command::new("git") + .args(args) + .current_dir(root) + .output() + .expect("git runs"); + assert!(out.status.success(), "git {args:?} failed"); + String::from_utf8_lossy(&out.stdout).trim().to_string() + } + + /// Two commits: a root, then one that renames a file, adds a path with a + /// space in it and writes a body long enough to fold. + fn two_commit_repo(name: &str) -> PathBuf { + let root = scratch(name); + git(&root, &["init", "--quiet"]); + git(&root, &["config", "user.email", "ada@example.com"]); + git(&root, &["config", "user.name", "Ada"]); + std::fs::write(root.join("a.txt"), "one\n").unwrap(); + git(&root, &["add", "a.txt"]); + git(&root, &["commit", "-qm", "root commit"]); + std::fs::rename(root.join("a.txt"), root.join("renamed.txt")).unwrap(); + std::fs::write(root.join("with space.txt"), "two\n").unwrap(); + std::fs::write(root.join("中文名.txt"), "three\n").unwrap(); + git(&root, &["add", "-A"]); + git( + &root, + &[ + "commit", + "-qm", + "feat(detail): a subject long enough that the graph row could never have shown it", + "-m", + "one\ntwo\nthree\nfour\nfive\nsix", + ], + ); + root + } + + fn panel_on( + cx: &mut TestAppContext, + root: &Path, + ) -> ( + Entity, + VisualTestContext, + std::os::unix::net::UnixStream, + ) { + let (app, mut vcx, mut pane) = test_window::harness_with_pane(cx); + DaemonMsg::Cwd(root.to_path_buf()) + .encode(&mut pane) + .expect("the pane's socket takes the cwd"); + app.update_in(&mut vcx, |app, _, cx| { + app.right_panel_visible = true; + app.right_panel_tab = RightPanelTab::Scm; + cx.notify(); + }); + let want = root.to_path_buf(); + settle(&app, &mut vcx, move |app, _| { + app.scm.repo.as_ref().is_some_and(|r| r.root == want) + }); + (app, vcx, pane) + } + + /// Pump frames until the panel has done what it was asked. The panel only + /// starts a read from `render`, so nothing here can be awaited directly. + fn settle( + app: &Entity, + vcx: &mut VisualTestContext, + done: impl Fn(&Tty7App, &gpui::App) -> bool, + ) { + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); + loop { + app.update_in(vcx, |_, _, cx| cx.notify()); + vcx.background_executor.run_until_parked(); + if app.update_in(vcx, |app, _, cx| done(app, cx)) { + return; + } + assert!( + std::time::Instant::now() < deadline, + "the panel never settled" + ); + std::thread::sleep(std::time::Duration::from_millis(20)); + } + } + + fn paths(app: &Entity, vcx: &mut VisualTestContext) -> Vec { + app.update_in(vcx, |app, _, _| { + app.scm + .detail + .as_ref() + .and_then(|d| d.files.clone()) + .map(|files| files.iter().map(|f| f.path.clone()).collect()) + .unwrap_or_default() + }) + } + + #[gpui::test] + fn a_commit_detail_reads_its_own_files_and_draws_them(cx: &mut TestAppContext) { + crate::core::config::pin_test_config_dir(); + let root = two_commit_repo("draws"); + let head = git(&root, &["rev-parse", "HEAD"]); + let (app, mut vcx, _pane) = panel_on(cx, &root); + + let repo = app.update_in(&mut vcx, |app, _, _| app.scm.repo.clone().unwrap()); + app.update_in(&mut vcx, |app, _, cx| { + app.open_commit_detail(repo.clone(), head.clone(), None, cx) + }); + settle(&app, &mut vcx, |app, _| { + app.scm.detail.as_ref().is_some_and(|d| d.loaded) + }); + + // Nothing was seeded, so the metadata came from `git show`. + app.update_in(&mut vcx, |app, _, _| { + let detail = app.scm.detail.as_ref().expect("the detail is open"); + let commit = detail.commit.as_ref().expect("git resolved the commit"); + assert_eq!(commit.oid, head); + assert!(commit.summary.starts_with("feat(detail):")); + assert_eq!(commit.parents.len(), 1); + assert_eq!(commit.body.lines().count(), 6, "long enough to fold"); + }); + let mut listed = paths(&app, &mut vcx); + listed.sort(); + assert_eq!( + listed, + ["renamed.txt", "with space.txt", "中文名.txt"], + "the two -z streams joined into one list" + ); + + // A real frame, so layout and paint run over every row above. + render_probe::arm(10_000); + app.update_in(&mut vcx, |_, _, cx| cx.notify()); + vcx.background_executor.run_until_parked(); + assert!( + render_probe::draws() > 0, + "nothing was drawn, so nothing was proved" + ); + + // Expanding the body is another branch of the same element. + app.update_in(&mut vcx, |app, _, cx| { + app.scm.detail.as_mut().unwrap().body_expanded = true; + cx.notify(); + }); + render_probe::arm(10_000); + app.update_in(&mut vcx, |_, _, cx| cx.notify()); + vcx.background_executor.run_until_parked(); + assert!(render_probe::draws() > 0); + + // The read runs from `render`, which is the shape that has spun this + // panel before: a dispatch that did not record itself would ask git + // for the same commit again on the frame its own answer caused. + assert_eq!(draws_while_idle(&mut vcx), 0); + + let _ = std::fs::remove_dir_all(&root); + } + + /// Copied from `panel.rs`'s own idle tests: arm the probe, let every timer + /// the panel owns fire, and count the frames nobody asked for. + fn draws_while_idle(vcx: &mut VisualTestContext) -> u64 { + render_probe::arm(200); + vcx.background_executor.run_until_parked(); + vcx.executor() + .advance_clock(std::time::Duration::from_secs(3)); + vcx.background_executor.run_until_parked(); + render_probe::arm(200); + vcx.executor() + .advance_clock(std::time::Duration::from_secs(9)); + vcx.background_executor.run_until_parked(); + render_probe::draws() + } + + #[gpui::test] + fn following_a_parent_swaps_the_commit_and_going_back_clears_it(cx: &mut TestAppContext) { + crate::core::config::pin_test_config_dir(); + let root = two_commit_repo("parent"); + let head = git(&root, &["rev-parse", "HEAD"]); + let parent = git(&root, &["rev-parse", "HEAD^"]); + let (app, mut vcx, _pane) = panel_on(cx, &root); + + let repo = app.update_in(&mut vcx, |app, _, _| app.scm.repo.clone().unwrap()); + app.update_in(&mut vcx, |app, _, cx| { + app.open_commit_detail(repo.clone(), head.clone(), None, cx) + }); + settle(&app, &mut vcx, |app, _| { + app.scm.detail.as_ref().is_some_and(|d| d.loaded) + }); + + // What the parent link does: the same call with the other oid, and + // nothing carried over from the commit that was on screen. + app.update_in(&mut vcx, |app, _, cx| { + app.open_commit_detail(repo.clone(), parent.clone(), None, cx) + }); + app.update_in(&mut vcx, |app, _, _| { + let detail = app.scm.detail.as_ref().unwrap(); + assert_eq!(detail.oid, parent); + assert!(detail.commit.is_none(), "the old commit did not linger"); + assert!(!detail.loaded); + }); + settle(&app, &mut vcx, |app, _| { + app.scm.detail.as_ref().is_some_and(|d| d.loaded) + }); + assert_eq!( + paths(&app, &mut vcx), + ["a.txt"], + "a root commit's files are what it added, with no --root needed" + ); + + app.update_in(&mut vcx, |app, _, cx| app.close_commit_detail(cx)); + assert!(app.update_in(&mut vcx, |app, _, _| app.scm.detail.is_none())); + let _ = std::fs::remove_dir_all(&root); + } + + #[gpui::test] + fn a_seeded_detail_only_asks_for_the_files(cx: &mut TestAppContext) { + crate::core::config::pin_test_config_dir(); + let root = two_commit_repo("seeded"); + let head = git(&root, &["rev-parse", "HEAD"]); + let (app, mut vcx, _pane) = panel_on(cx, &root); + let repo = app.update_in(&mut vcx, |app, _, _| app.scm.repo.clone().unwrap()); + + // What the graph hands over: a row it already holds. The subject is + // deliberately not the real one, so a `git show` behind our back would + // overwrite it and show up here. + let mut seed = tty7_core::core::git::log::load_commit( + &*tty7_core::host::local::LocalHost::new(), + &root, + &head, + ) + .expect("the scratch repo answers"); + seed.summary = "what the graph already knew".into(); + app.update_in(&mut vcx, |app, _, cx| { + app.open_commit_detail(repo.clone(), head.clone(), Some(seed), cx) + }); + settle(&app, &mut vcx, |app, _| { + app.scm.detail.as_ref().is_some_and(|d| d.loaded) + }); + + app.update_in(&mut vcx, |app, _, _| { + let detail = app.scm.detail.as_ref().unwrap(); + assert_eq!( + detail.commit.as_ref().unwrap().summary, + "what the graph already knew", + "the seed was kept, so no second read of the same commit happened" + ); + assert_eq!(detail.files.as_ref().unwrap().len(), 3); + }); + let _ = std::fs::remove_dir_all(&root); + } + + /// A commit from a repository the panel has since walked away from is not + /// a second-level view of anything. + #[gpui::test] + fn a_detail_from_another_repository_gives_the_body_back(cx: &mut TestAppContext) { + crate::core::config::pin_test_config_dir(); + let root = two_commit_repo("elsewhere"); + let head = git(&root, &["rev-parse", "HEAD"]); + let (app, mut vcx, _pane) = panel_on(cx, &root); + + app.update_in(&mut vcx, |app, window, cx| { + let stranger = RepoKey { + host: HostId::LOCAL, + root: PathBuf::from("/no/such/tty7/repo"), + }; + app.open_commit_detail(stranger.clone(), head.clone(), None, cx); + let detail = app.scm.detail.clone().unwrap(); + assert!(app.render_commit_detail(&detail, window, cx).is_none()); + assert!(app.scm.detail.is_none(), "and it does not come back"); + }); + let _ = std::fs::remove_dir_all(&root); } } diff --git a/src/ui/scm/mod.rs b/src/ui/scm/mod.rs index ff6da7c7..04d7ba65 100644 --- a/src/ui/scm/mod.rs +++ b/src/ui/scm/mod.rs @@ -4,11 +4,10 @@ //! `file_tree.rs` use. The directory only keeps the surface from piling into //! `right_panel.rs`. -// What is left unused is what the graph and the commit detail view will call: -// `relative_time` has no row to date yet, and `status_rank` is the file tree's -// to use. Both allows come off with the step that wires them up. +// What is left unused is what the graph will call, plus `status_rank`, which +// is the file tree's to use. Both allows come off with the step that wires +// them up. pub(crate) mod actions; -#[allow(dead_code)] pub(crate) mod detail; #[allow(dead_code)] pub(crate) mod graph; diff --git a/src/ui/scm/state.rs b/src/ui/scm/state.rs index 96880939..121fe514 100644 --- a/src/ui/scm/state.rs +++ b/src/ui/scm/state.rs @@ -12,9 +12,11 @@ use std::collections::{HashMap, HashSet}; use std::path::PathBuf; +use std::sync::Arc; use gpui::Entity; use gpui_component::input::InputState; +use tty7_core::core::git::log::{Commit, CommitFile}; use tty7_core::core::git::status::HeadState; use crate::ui::host_ops::HostId; @@ -174,11 +176,49 @@ pub(crate) struct GraphState { /// touched. A file-level diff is not shown here — that opens the full-screen /// overlay, because 260px cannot render a diff and pretending otherwise would /// mean inventing a third kind of container. +/// +/// The two loaded halves are behind `Arc` because the panel clones this whole +/// struct once per frame — `render_panel_scm` cannot hand `render_commit_ +/// detail` a borrow of `self.scm` and a `&mut self` at once — and a commit +/// that touched a thousand files would otherwise deep-copy a thousand paths +/// every time anything on the panel redrew. #[derive(Clone, PartialEq, Eq, Debug)] pub(crate) struct CommitDetailView { pub(crate) repo: RepoKey, pub(crate) oid: String, + /// A read is out. Set before it is dispatched, so the render that runs in + /// between does not ask for a second one. pub(crate) loading: bool, + /// Whether a read has ever come back. With `loading` it is what stops the + /// view asking again forever after a commit git could not resolve: the + /// pair says "nothing is in flight and nothing is coming". + pub(crate) loaded: bool, + /// `None` until a read lands, and still `None` afterwards for a commit + /// that is not in this repository. + pub(crate) commit: Option>, + pub(crate) files: Option>>, + /// A long body starts folded — a merge from a bot can run to fifty lines, + /// and the file list is what the reader came for. + pub(crate) body_expanded: bool, +} + +impl CommitDetailView { + /// A commit the panel is about to show. `seed` is the row the graph + /// already has in hand, where the caller came from the graph: the page + /// carries every field a detail view needs, so handing it over is what + /// keeps the common path from running `git show` for a commit that is + /// literally on screen. + pub(crate) fn new(repo: RepoKey, oid: String, seed: Option) -> CommitDetailView { + CommitDetailView { + repo, + oid, + loading: false, + loaded: false, + commit: seed.map(Arc::new), + files: None, + body_expanded: false, + } + } } #[cfg(test)]