From a3615c9b05f64ed96429f89959a84916b1480898 Mon Sep 17 00:00:00 2001 From: Daniel Kurzynski Date: Mon, 31 Aug 2026 16:15:48 +0200 Subject: [PATCH 01/10] feat(ui_gpui): review sidebar with two-column layout and multi-repo support Add a Review right-sidebar that shows changed files and their diffs: - Two-column layout: unified diff on the left, per-repo changed-file trees on the right, separated by a draggable divider (h_resizable). - Multi-repo support: when the project root isn't itself a git repo but contains multiple repos, discover immediate-subfolder repos and show each with its own base-branch selector. - Resizable sidebar width via a left-edge drag handle. - Diff pane reuses the chat's unified-diff rendering (monospace body, red/green rows) and scrolls via a native ScrollHandle. - Persist sidebar width, tree-column width, and default base globally. Backend: git changed-file/diff support (working tree and branch-vs-base), repo discovery, and per-repo review listing in SessionService. --- crates/code_assistant_core/src/session/mod.rs | 1 + .../src/session/service.rs | 321 +++++++- .../code_assistant_core/src/ui/ui_events.rs | 14 + crates/git/Cargo.toml | 2 +- crates/git/src/binary.rs | 35 + crates/git/src/diff.rs | 581 ++++++++++++++ crates/git/src/lib.rs | 2 + crates/ui_acp/src/ui.rs | 3 + crates/ui_gpui/src/app/commands.rs | 68 ++ crates/ui_gpui/src/app/event_loop.rs | 43 + crates/ui_gpui/src/lib.rs | 69 ++ crates/ui_gpui/src/main_screen/mod.rs | 427 +++++++--- .../src/main_screen/right_panel/mod.rs | 88 +++ .../main_screen/right_panel/review_view.rs | 733 ++++++++++++++++++ crates/ui_gpui/src/shared/file_icons.rs | 38 + crates/ui_gpui/src/shared/file_tree.rs | 429 ++++++++++ crates/ui_gpui/src/shared/mod.rs | 1 + crates/ui_gpui/src/shared/settings.rs | 16 + crates/ui_gpui/src/shared/ui_state.rs | 61 +- crates/ui_gpui/src/tool_cards/diff_card.rs | 14 +- 20 files changed, 2843 insertions(+), 103 deletions(-) create mode 100644 crates/git/src/diff.rs create mode 100644 crates/ui_gpui/src/main_screen/right_panel/mod.rs create mode 100644 crates/ui_gpui/src/main_screen/right_panel/review_view.rs create mode 100644 crates/ui_gpui/src/shared/file_tree.rs diff --git a/crates/code_assistant_core/src/session/mod.rs b/crates/code_assistant_core/src/session/mod.rs index 9a1405de..06ae6951 100644 --- a/crates/code_assistant_core/src/session/mod.rs +++ b/crates/code_assistant_core/src/session/mod.rs @@ -24,6 +24,7 @@ pub mod watcher; pub use event_stream::{EventPayload, EventStream, SessionEvent, StreamError, Subscription}; pub use manager::SessionManager; pub use service::SessionService; +pub use service::{RepoReview, ReviewListing, ReviewMode, WorktreeListing}; pub use turn::{ ResourceRef, ToolRecord, TurnDispatch, TurnHandle, TurnOutcome, TurnRequest, TurnStatus, TurnUsage, diff --git a/crates/code_assistant_core/src/session/service.rs b/crates/code_assistant_core/src/session/service.rs index 98bb438a..738d2b96 100644 --- a/crates/code_assistant_core/src/session/service.rs +++ b/crates/code_assistant_core/src/session/service.rs @@ -30,8 +30,9 @@ use command_executor::CommandExecutor; use llm::factory::create_llm_client_from_model; use llm::provider_config::ConfigurationSystem; use sandbox::SandboxPolicy; +use std::collections::HashMap; use std::future::Future; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::pin::Pin; use std::sync::Arc; use tokio::sync::Mutex; @@ -135,6 +136,45 @@ pub struct WorktreeListing { pub is_git_repo: bool, } +/// Which changes the Review panel should compare. +#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +pub enum ReviewMode { + /// Local working-tree changes (staged, unstaged, untracked) vs `HEAD`. + WorkingTree, + /// The current branch vs a base branch, using merge-base (PR) semantics. + BranchVsBase, +} + +/// Changed files for a single git repository within the reviewed project, +/// plus the per-repo metadata the UI needs to render its selectors. +#[derive(Debug, Clone)] +pub struct RepoReview { + /// Absolute path to the repository's working directory. + pub repo_root: PathBuf, + /// Human-readable label (the repo directory's name). + pub label: String, + pub current_branch: Option, + /// Candidate base refs (local + remote branches) for `BranchVsBase` mode. + pub base_candidates: Vec, + /// The base ref actually used in `BranchVsBase` mode (may have been + /// defaulted from `None`). + pub base: Option, + pub files: Vec, +} + +/// Listing of changed files for the Review panel across one or more +/// repositories discovered under the project root. +#[derive(Debug, Clone)] +pub struct ReviewListing { + /// One entry per discovered git repository (0 when the project is not, + /// and contains no, git repos). + pub repos: Vec, + /// `true` when at least one git repository was found. + pub is_git_repo: bool, + /// The mode this listing was produced for (echoed back for the UI). + pub mode: ReviewMode, +} + /// A git worktree the session was switched to. #[derive(Debug, Clone)] pub struct CreatedWorktree { @@ -1008,6 +1048,121 @@ impl SessionService { .await } + /// List changed files for the Review panel in the requested `mode`. + /// + /// Resolves the session's on-disk directory with `effective_project_path` + /// (worktree-aware). Returns `is_git_repo: false` early for non-git dirs. + /// In `BranchVsBase` mode a repo without an override in `base_overrides` + /// defaults to the current branch's upstream, else the first remote + /// candidate. + pub async fn list_review_files( + &self, + session_id: String, + mode: ReviewMode, + base_overrides: HashMap, + ) -> Result { + self.call(move |ctx| async move { + let project_root = { + let manager = ctx.manager.lock().await; + session_effective_path(&manager, &session_id)? + }; + + let discovered = discover_review_repos(&project_root); + if discovered.is_empty() { + return Ok(ReviewListing { + repos: Vec::new(), + is_git_repo: false, + mode, + }); + } + + let mut repos = Vec::with_capacity(discovered.len()); + for (repo_root, label) in discovered { + let repo = match git::GitRepository::open(&repo_root) { + Ok(r) => r, + Err(e) => { + warn!("Skipping repo {}: {e:#}", repo_root.display()); + continue; + } + }; + let current_branch = repo.current_branch(); + let base_candidates = repo.list_base_candidates().unwrap_or_default(); + let override_base = base_overrides.get(&repo_root).cloned(); + + let (files, resolved_base) = match mode { + ReviewMode::WorkingTree => { + let files = repo + .changed_files_working_tree() + .await + .context("Failed to list working-tree changes")?; + (files, None) + } + ReviewMode::BranchVsBase => { + let resolved = + resolve_review_base(&repo, override_base, &base_candidates); + match &resolved { + Some(b) => { + let files = repo + .changed_files_vs_base(b) + .await + .with_context(|| format!("Failed to diff against {b}"))?; + (files, resolved) + } + None => (Vec::new(), None), + } + } + }; + + repos.push(RepoReview { + repo_root, + label, + current_branch, + base_candidates, + base: resolved_base, + files, + }); + } + + Ok(ReviewListing { + is_git_repo: !repos.is_empty(), + repos, + mode, + }) + }) + .await + } + + /// Load both sides of the diff for a single file in the Review panel. + pub async fn get_review_file_diff( + &self, + _session_id: String, + repo_root: PathBuf, + mode: ReviewMode, + base: Option, + file: git::ChangedFile, + ) -> Result { + self.call(move |_ctx| async move { + let repo = + git::GitRepository::open(&repo_root).context("Failed to open git repository")?; + + match mode { + ReviewMode::WorkingTree => repo + .file_diff_working_tree(&file) + .await + .context("Failed to load working-tree diff"), + ReviewMode::BranchVsBase => { + let base_candidates = repo.list_base_candidates().unwrap_or_default(); + let resolved = resolve_review_base(&repo, base, &base_candidates) + .ok_or_else(|| anyhow!("No base branch available for comparison"))?; + repo.file_diff_vs_base(&resolved, &file) + .await + .context("Failed to load branch-vs-base diff") + } + } + }) + .await + } + pub async fn switch_worktree( &self, session_id: String, @@ -1186,6 +1341,170 @@ fn session_project_root(manager: &SessionManager, session_id: &str) -> Result Result { + let session = manager + .get_session(session_id) + .ok_or_else(|| anyhow!("Session {session_id} not found"))?; + session + .session + .config + .effective_project_path() + .cloned() + .ok_or_else(|| anyhow!("Session has no project path configured")) +} + +/// Choose the base ref for `BranchVsBase` mode. Prefers an explicit `base` +/// (validated against candidates), then the current branch's upstream, then +/// the first `origin/*` candidate, then any candidate that is not the current +/// branch. +fn resolve_review_base( + repo: &git::GitRepository, + base: Option, + candidates: &[String], +) -> Option { + if let Some(base) = base + && candidates.iter().any(|c| c == &base) + { + return Some(base); + } + + let current = repo.current_branch(); + + if let Some(branch) = repo.list_branches().ok().and_then(|branches| { + branches + .into_iter() + .find(|b| b.is_head) + .and_then(|b| b.upstream) + }) && candidates.iter().any(|c| c == &branch) + { + return Some(branch); + } + + if let Some(origin) = candidates.iter().find(|c| c.starts_with("origin/")) { + return Some(origin.clone()); + } + + candidates + .iter() + .find(|c| current.as_deref() != Some(c.as_str())) + .cloned() +} + +/// Discover the git repositories to review under `root`. +/// +/// Returns `(repo workdir, display label)` pairs. If `root` is itself a repo +/// root, that single repo is returned (preserving single-repo behavior). +/// Otherwise `root`'s immediate subdirectories are scanned and each one that is +/// its own repo root is included — no deep recursion. Results are sorted by +/// label. +/// +/// `GitRepository::open` discovers *upward*, so a plain child folder inside an +/// upward repo would resolve to that ancestor; the `workdir() == dir` check +/// (both canonicalized) is what pins each entry to an actual repo root. +fn discover_review_repos(root: &Path) -> Vec<(PathBuf, String)> { + fn is_repo_root(dir: &Path) -> bool { + let Ok(repo) = git::GitRepository::open(dir) else { + return false; + }; + let workdir = repo.workdir(); + match (workdir.canonicalize(), dir.canonicalize()) { + (Ok(a), Ok(b)) => a == b, + _ => workdir == dir, + } + } + + fn label_for(dir: &Path) -> String { + dir.file_name() + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_else(|| dir.display().to_string()) + } + + if is_repo_root(root) { + return vec![(root.to_path_buf(), label_for(root))]; + } + + let mut repos: Vec<(PathBuf, String)> = Vec::new(); + if let Ok(entries) = std::fs::read_dir(root) { + for entry in entries.flatten() { + let path = entry.path(); + if !path.is_dir() { + continue; + } + if is_repo_root(&path) { + let label = label_for(&path); + repos.push((path, label)); + } + } + } + repos.sort_by_key(|(_, label)| label.to_lowercase()); + repos +} + +#[cfg(test)] +mod discover_tests { + use super::discover_review_repos; + use std::process::Command; + + fn git_init(dir: &std::path::Path) { + let ok = Command::new("git") + .args(["init", "-q"]) + .current_dir(dir) + .status() + .expect("run git init") + .success(); + assert!(ok, "git init failed in {}", dir.display()); + } + + #[test] + fn root_is_repo_returns_single_entry() { + let tmp = tempfile::tempdir().unwrap(); + git_init(tmp.path()); + let repos = discover_review_repos(tmp.path()); + assert_eq!(repos.len(), 1); + assert_eq!(repos[0].0.canonicalize().unwrap(), tmp.path().canonicalize().unwrap()); + } + + #[test] + fn plain_folder_with_two_child_repos() { + let tmp = tempfile::tempdir().unwrap(); + let a = tmp.path().join("alpha"); + let b = tmp.path().join("beta"); + std::fs::create_dir(&a).unwrap(); + std::fs::create_dir(&b).unwrap(); + std::fs::create_dir(tmp.path().join("plain")).unwrap(); + git_init(&a); + git_init(&b); + let repos = discover_review_repos(tmp.path()); + assert_eq!(repos.len(), 2); + assert_eq!(repos[0].1, "alpha"); + assert_eq!(repos[1].1, "beta"); + } + + #[test] + fn nested_non_root_child_is_ignored() { + // A repo at root/outer, with root/outer/inner a plain subdir. Scanning + // root sees only `outer`; `inner` is never a scanned top-level child, + // and a plain folder with no repos yields nothing. + let tmp = tempfile::tempdir().unwrap(); + let outer = tmp.path().join("outer"); + std::fs::create_dir(&outer).unwrap(); + git_init(&outer); + std::fs::create_dir(outer.join("inner")).unwrap(); + + // Scanning the repo root itself → single entry (the root). + let repos = discover_review_repos(&outer); + assert_eq!(repos.len(), 1); + + // A sibling plain folder with no git repos → empty. + let empty = tmp.path().join("empty"); + std::fs::create_dir(&empty).unwrap(); + assert!(discover_review_repos(&empty).is_empty()); + } +} + async fn send_user_message_impl( ctx: &ServiceCtx, session_id: &str, diff --git a/crates/code_assistant_core/src/ui/ui_events.rs b/crates/code_assistant_core/src/ui/ui_events.rs index 9b5f84ef..6536478d 100644 --- a/crates/code_assistant_core/src/ui/ui_events.rs +++ b/crates/code_assistant_core/src/ui/ui_events.rs @@ -318,6 +318,20 @@ pub enum UiEvent { is_git_repo: bool, }, + // === Review Panel Events === + /// Updated list of changed files for the Review panel, grouped per repo. + UpdateReviewFiles { + repos: Vec, + is_git_repo: bool, + mode: crate::session::ReviewMode, + }, + /// The loaded diff for a single file selected in the Review panel. + UpdateReviewDiff { + repo_root: PathBuf, + path: String, + diff: git::FileDiffContent, + }, + // === Configuration Events === /// Configuration files (providers.json / models.json) were changed on disk. /// The UI should reload model lists, settings sections, etc. diff --git a/crates/git/Cargo.toml b/crates/git/Cargo.toml index 269d0a53..702200da 100644 --- a/crates/git/Cargo.toml +++ b/crates/git/Cargo.toml @@ -6,7 +6,7 @@ edition = "2024" [dependencies] anyhow = "1.0.95" serde = { version = "1.0.215", features = ["derive"] } -tokio = { version = "1.40.0", features = ["process"] } +tokio = { version = "1.40.0", features = ["fs", "process"] } tracing = "0.1.40" which = "7.0" diff --git a/crates/git/src/binary.rs b/crates/git/src/binary.rs index 10872412..e3aec00e 100644 --- a/crates/git/src/binary.rs +++ b/crates/git/src/binary.rs @@ -75,4 +75,39 @@ impl GitBinary { Ok(stdout) } + + /// Run a git command and return raw stdout bytes on success. + /// + /// Unlike [`run`](Self::run), this does not require the output to be valid + /// UTF-8 and does not strip any trailing newline. Use it for commands whose + /// output may be binary (e.g. `git show :` on a binary blob). + pub async fn run_bytes>( + &self, + working_dir: &Path, + args: &[S], + ) -> Result> { + let mut cmd = self.command(working_dir); + cmd.args(args); + + let output = cmd + .output() + .await + .context("Failed to execute git command")?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + let args_display: Vec<_> = args + .iter() + .map(|a| a.as_ref().to_string_lossy().to_string()) + .collect(); + bail!( + "git {} failed (exit {}): {}", + args_display.join(" "), + output.status.code().unwrap_or(-1), + stderr.trim() + ); + } + + Ok(output.stdout) + } } diff --git a/crates/git/src/diff.rs b/crates/git/src/diff.rs new file mode 100644 index 00000000..2825bfbd --- /dev/null +++ b/crates/git/src/diff.rs @@ -0,0 +1,581 @@ +use crate::repository::GitRepository; +use anyhow::{Context, Result}; +use serde::{Deserialize, Serialize}; + +/// Upper bound on the size (in bytes) of a single side of a diff we will load +/// into memory and hand to the UI. Larger blobs are reported as `too_large` +/// and their text is omitted. +const MAX_DIFF_BYTES: usize = 1_500_000; + +/// How a file changed relative to the comparison base. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum ChangeStatus { + Added, + Modified, + Deleted, + Renamed, + Copied, + TypeChanged, + Untracked, +} + +/// A single changed file in a review listing. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ChangedFile { + /// Repo-relative path of the file in its new/current location + /// (forward-slash separated). + pub path: String, + /// For renames/copies, the original repo-relative path; otherwise `None`. + pub orig_path: Option, + /// The kind of change. + pub status: ChangeStatus, +} + +/// The two whole-file sides of a diff, ready to be fed to the UI's unified +/// diff renderer. Either side may be `None` (pure add or delete). When +/// `is_binary` or `too_large` is set, the text sides are omitted and the UI +/// should show a placeholder instead. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct FileDiffContent { + /// The old (base) content, or `None` if the file was added. + pub old_text: Option, + /// The new (current) content, or `None` if the file was deleted. + pub new_text: Option, + /// True if either side is not valid UTF-8 (i.e. a binary blob). + pub is_binary: bool, + /// True if either side exceeds [`MAX_DIFF_BYTES`]. + pub too_large: bool, +} + +/// One side of a diff, before decoding. +enum Side { + /// The side does not exist (pure add or delete). + Absent, + /// The side exists but is larger than [`MAX_DIFF_BYTES`]. + TooLarge, + /// The raw bytes of the side. + Bytes(Vec), +} + +impl Side { + fn from_bytes(bytes: Vec) -> Self { + if bytes.len() > MAX_DIFF_BYTES { + Side::TooLarge + } else { + Side::Bytes(bytes) + } + } +} + +impl GitRepository { + /// List files that differ between the working tree (including staged + /// changes and untracked files) and `HEAD`. + pub async fn changed_files_working_tree(&self) -> Result> { + let out = self + .git + .run_bytes( + self.workdir(), + &[ + "-c", + "core.quotepath=false", + "status", + "--porcelain=v1", + "-z", + "--untracked-files=all", + ], + ) + .await?; + Ok(parse_status_z(&out)) + } + + /// List files that differ between `base` and `HEAD` using three-dot + /// (merge-base) semantics, matching what a pull request would show. + pub async fn changed_files_vs_base(&self, base: &str) -> Result> { + let range = format!("{base}...HEAD"); + let out = self + .git + .run_bytes( + self.workdir(), + &[ + "-c", + "core.quotepath=false", + "diff", + "--name-status", + "-M", + "-z", + &range, + ], + ) + .await?; + Ok(parse_diff_name_status_z(&out)) + } + + /// Load both sides of the diff for `file` in working-tree mode: + /// old = the `HEAD` version, new = the current working-tree file. + pub async fn file_diff_working_tree(&self, file: &ChangedFile) -> Result { + let old_ref_path = file.orig_path.as_deref().unwrap_or(&file.path); + let old = if matches!(file.status, ChangeStatus::Added | ChangeStatus::Untracked) { + Side::Absent + } else { + let bytes = self + .git + .run_bytes(self.workdir(), &["show", &format!("HEAD:{old_ref_path}")]) + .await + .with_context(|| format!("reading HEAD:{old_ref_path}"))?; + Side::from_bytes(bytes) + }; + + let new = if matches!(file.status, ChangeStatus::Deleted) { + Side::Absent + } else { + self.read_workdir_file(&file.path).await? + }; + + Ok(build_diff_content(old, new)) + } + + /// Load both sides of the diff for `file` in branch-vs-base mode: + /// old = the version at `merge-base(base, HEAD)`, new = the `HEAD` version. + pub async fn file_diff_vs_base( + &self, + base: &str, + file: &ChangedFile, + ) -> Result { + let merge_base = self + .git + .run(self.workdir(), &["merge-base", base, "HEAD"]) + .await + .with_context(|| format!("merge-base {base} HEAD"))?; + let merge_base = merge_base.trim(); + + let old_ref_path = file.orig_path.as_deref().unwrap_or(&file.path); + let old = if matches!(file.status, ChangeStatus::Added) { + Side::Absent + } else { + let bytes = self + .git + .run_bytes( + self.workdir(), + &["show", &format!("{merge_base}:{old_ref_path}")], + ) + .await + .with_context(|| format!("reading {merge_base}:{old_ref_path}"))?; + Side::from_bytes(bytes) + }; + + let new = if matches!(file.status, ChangeStatus::Deleted) { + Side::Absent + } else { + let bytes = self + .git + .run_bytes(self.workdir(), &["show", &format!("HEAD:{}", file.path)]) + .await + .with_context(|| format!("reading HEAD:{}", file.path))?; + Side::from_bytes(bytes) + }; + + Ok(build_diff_content(old, new)) + } + + /// Candidate base refs for branch-vs-base comparison: local branches plus + /// remote-tracking branches (excluding `*/HEAD`), sorted and de-duplicated. + pub fn list_base_candidates(&self) -> Result> { + let repo = self.repo.to_thread_local(); + let mut out = Vec::new(); + + for reference in repo.references()?.local_branches()? { + let reference = reference.map_err(|e| anyhow::anyhow!("{e}"))?; + let name = reference.name().shorten().to_string(); + if !name.is_empty() { + out.push(name); + } + } + + for reference in repo.references()?.remote_branches()? { + let reference = reference.map_err(|e| anyhow::anyhow!("{e}"))?; + let name = reference.name().shorten().to_string(); + if name.is_empty() || name.ends_with("/HEAD") { + continue; + } + out.push(name); + } + + out.sort(); + out.dedup(); + Ok(out) + } + + /// Read a working-tree file as a diff `Side`, mapping a missing file to + /// `Absent` and an oversized file to `TooLarge` (without reading it). + async fn read_workdir_file(&self, rel_path: &str) -> Result { + let full = self.workdir().join(rel_path); + match tokio::fs::metadata(&full).await { + Ok(meta) if meta.len() as usize > MAX_DIFF_BYTES => Ok(Side::TooLarge), + Ok(_) => match tokio::fs::read(&full).await { + Ok(bytes) => Ok(Side::from_bytes(bytes)), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(Side::Absent), + Err(e) => Err(e).with_context(|| format!("reading {}", full.display())), + }, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(Side::Absent), + Err(e) => Err(e).with_context(|| format!("stat {}", full.display())), + } + } +} + +/// Classify a porcelain `XY` status pair into a single net `ChangeStatus`. +fn classify_status(x: char, y: char) -> ChangeStatus { + if x == '?' || y == '?' { + return ChangeStatus::Untracked; + } + let has = |c: char| x == c || y == c; + if has('R') { + ChangeStatus::Renamed + } else if has('C') { + ChangeStatus::Copied + } else if has('A') { + ChangeStatus::Added + } else if has('D') { + ChangeStatus::Deleted + } else if has('T') { + ChangeStatus::TypeChanged + } else { + ChangeStatus::Modified + } +} + +/// Parse the output of `git status --porcelain=v1 -z`. +/// +/// Records are NUL-terminated. Each record is `XYPATH`. For renames and +/// copies the `-z` format omits the ` -> ` and reverses the field order, so the +/// new path is in the record itself and the original path is the *next* +/// NUL-terminated field. +fn parse_status_z(bytes: &[u8]) -> Vec { + let mut files = Vec::new(); + let mut chunks = bytes.split(|&b| b == 0); + while let Some(chunk) = chunks.next() { + if chunk.len() < 4 { + // Empty trailing chunk or malformed record; skip. + continue; + } + let x = chunk[0] as char; + let y = chunk[1] as char; + // chunk[2] is the separating space; path starts at index 3. + let path = String::from_utf8_lossy(&chunk[3..]).into_owned(); + let status = classify_status(x, y); + let orig_path = if matches!(status, ChangeStatus::Renamed | ChangeStatus::Copied) { + chunks + .next() + .map(|c| String::from_utf8_lossy(c).into_owned()) + } else { + None + }; + files.push(ChangedFile { + path, + orig_path, + status, + }); + } + files +} + +/// Parse the output of `git diff --name-status -M -z`. +/// +/// Fields are NUL-terminated. A regular record is `STATUS`, then `PATH`. A +/// rename/copy record is `STATUS`, then the source path, then the destination +/// path. +fn parse_diff_name_status_z(bytes: &[u8]) -> Vec { + let mut files = Vec::new(); + let mut chunks = bytes.split(|&b| b == 0).filter(|c| !c.is_empty()); + while let Some(status_chunk) = chunks.next() { + let code = status_chunk[0] as char; + let status = match code { + 'A' => ChangeStatus::Added, + 'D' => ChangeStatus::Deleted, + 'R' => ChangeStatus::Renamed, + 'C' => ChangeStatus::Copied, + 'T' => ChangeStatus::TypeChanged, + _ => ChangeStatus::Modified, + }; + if matches!(status, ChangeStatus::Renamed | ChangeStatus::Copied) { + let Some(old) = chunks.next() else { break }; + let Some(new) = chunks.next() else { break }; + files.push(ChangedFile { + path: String::from_utf8_lossy(new).into_owned(), + orig_path: Some(String::from_utf8_lossy(old).into_owned()), + status, + }); + } else { + let Some(path) = chunks.next() else { break }; + files.push(ChangedFile { + path: String::from_utf8_lossy(path).into_owned(), + orig_path: None, + status, + }); + } + } + files +} + +/// Decode both raw sides into a [`FileDiffContent`], flagging binary and +/// oversized content. +fn build_diff_content(old: Side, new: Side) -> FileDiffContent { + let mut is_binary = false; + let mut too_large = false; + + let mut decode = |side: Side| -> Option { + match side { + Side::Absent => None, + Side::TooLarge => { + too_large = true; + None + } + Side::Bytes(bytes) => match String::from_utf8(bytes) { + Ok(text) => Some(text), + Err(_) => { + is_binary = true; + None + } + }, + } + }; + + let old_text = decode(old); + let new_text = decode(new); + + FileDiffContent { + old_text, + new_text, + is_binary, + too_large, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::testutil::init_repo_with_commit; + use std::path::Path; + use tempfile::TempDir; + + /// Run a git command synchronously for test setup (staging, committing). + fn git(dir: &Path, args: &[&str]) { + let status = std::process::Command::new("git") + .args(args) + .current_dir(dir) + .status() + .unwrap_or_else(|e| panic!("git {args:?}: {e}")); + assert!(status.success(), "git {args:?} failed"); + } + + fn write(dir: &Path, rel: &str, content: &[u8]) { + let full = dir.join(rel); + if let Some(parent) = full.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(full, content).unwrap(); + } + + fn find<'a>(files: &'a [ChangedFile], path: &str) -> &'a ChangedFile { + files + .iter() + .find(|f| f.path == path) + .unwrap_or_else(|| panic!("no changed file with path {path} in {files:?}")) + } + + #[test] + fn parse_status_handles_rename_pair() { + // Rename record: new path in-record, old path in following field. + let raw = b"R new_name.txt\0old_name.txt\0M other.txt\0"; + let files = parse_status_z(raw); + assert_eq!(files.len(), 2); + assert_eq!(files[0].status, ChangeStatus::Renamed); + assert_eq!(files[0].path, "new_name.txt"); + assert_eq!(files[0].orig_path.as_deref(), Some("old_name.txt")); + assert_eq!(files[1].status, ChangeStatus::Modified); + assert_eq!(files[1].path, "other.txt"); + assert_eq!(files[1].orig_path, None); + } + + #[test] + fn parse_status_handles_untracked_and_paths_with_spaces() { + let raw = b"?? a file.txt\0 M tracked.rs\0"; + let files = parse_status_z(raw); + assert_eq!(files.len(), 2); + assert_eq!(files[0].status, ChangeStatus::Untracked); + assert_eq!(files[0].path, "a file.txt"); + assert_eq!(files[1].status, ChangeStatus::Modified); + assert_eq!(files[1].path, "tracked.rs"); + } + + #[test] + fn parse_diff_name_status_handles_rename() { + let raw = b"M\0a.txt\0R100\0old.txt\0new.txt\0A\0added.txt\0"; + let files = parse_diff_name_status_z(raw); + assert_eq!(files.len(), 3); + assert_eq!(files[0].status, ChangeStatus::Modified); + assert_eq!(files[0].path, "a.txt"); + assert_eq!(files[1].status, ChangeStatus::Renamed); + assert_eq!(files[1].orig_path.as_deref(), Some("old.txt")); + assert_eq!(files[1].path, "new.txt"); + assert_eq!(files[2].status, ChangeStatus::Added); + assert_eq!(files[2].path, "added.txt"); + } + + #[tokio::test] + async fn working_tree_add_modify_delete_untracked() { + let dir = TempDir::new().unwrap(); + init_repo_with_commit(dir.path()); + + // Seed two tracked files and commit them. + write(dir.path(), "keep.txt", b"one\ntwo\n"); + write(dir.path(), "gone.txt", b"delete me\n"); + git(dir.path(), &["add", "."]); + git(dir.path(), &["commit", "-m", "seed"]); + + // Modify keep.txt, delete gone.txt, add an untracked new.txt. + write(dir.path(), "keep.txt", b"one\nchanged\n"); + std::fs::remove_file(dir.path().join("gone.txt")).unwrap(); + write(dir.path(), "new.txt", b"brand new\n"); + + let repo = GitRepository::open(dir.path()).unwrap(); + let files = repo.changed_files_working_tree().await.unwrap(); + + assert_eq!(find(&files, "keep.txt").status, ChangeStatus::Modified); + assert_eq!(find(&files, "gone.txt").status, ChangeStatus::Deleted); + assert_eq!(find(&files, "new.txt").status, ChangeStatus::Untracked); + + // Modified: both sides present. + let d = repo + .file_diff_working_tree(find(&files, "keep.txt")) + .await + .unwrap(); + assert_eq!(d.old_text.as_deref(), Some("one\ntwo\n")); + assert_eq!(d.new_text.as_deref(), Some("one\nchanged\n")); + assert!(!d.is_binary && !d.too_large); + + // Deleted: no new side. + let d = repo + .file_diff_working_tree(find(&files, "gone.txt")) + .await + .unwrap(); + assert_eq!(d.old_text.as_deref(), Some("delete me\n")); + assert_eq!(d.new_text, None); + + // Untracked: no old side. + let d = repo + .file_diff_working_tree(find(&files, "new.txt")) + .await + .unwrap(); + assert_eq!(d.old_text, None); + assert_eq!(d.new_text.as_deref(), Some("brand new\n")); + } + + #[tokio::test] + async fn working_tree_rename() { + let dir = TempDir::new().unwrap(); + init_repo_with_commit(dir.path()); + + write(dir.path(), "original.txt", b"stable content\n"); + git(dir.path(), &["add", "."]); + git(dir.path(), &["commit", "-m", "seed"]); + + // Stage a rename so git detects it as R. + git(dir.path(), &["mv", "original.txt", "renamed.txt"]); + + let repo = GitRepository::open(dir.path()).unwrap(); + let files = repo.changed_files_working_tree().await.unwrap(); + + let renamed = find(&files, "renamed.txt"); + assert_eq!(renamed.status, ChangeStatus::Renamed); + assert_eq!(renamed.orig_path.as_deref(), Some("original.txt")); + + let d = repo.file_diff_working_tree(renamed).await.unwrap(); + assert_eq!(d.old_text.as_deref(), Some("stable content\n")); + assert_eq!(d.new_text.as_deref(), Some("stable content\n")); + } + + #[tokio::test] + async fn working_tree_binary_and_too_large() { + let dir = TempDir::new().unwrap(); + init_repo_with_commit(dir.path()); + + // Binary file (invalid UTF-8 bytes). + write(dir.path(), "blob.bin", &[0u8, 159, 146, 150, 255]); + // Oversized text file. + let big = vec![b'a'; MAX_DIFF_BYTES + 10]; + write(dir.path(), "big.txt", &big); + + let repo = GitRepository::open(dir.path()).unwrap(); + let files = repo.changed_files_working_tree().await.unwrap(); + + let d = repo + .file_diff_working_tree(find(&files, "blob.bin")) + .await + .unwrap(); + assert!(d.is_binary); + assert_eq!(d.new_text, None); + + let d = repo + .file_diff_working_tree(find(&files, "big.txt")) + .await + .unwrap(); + assert!(d.too_large); + assert_eq!(d.new_text, None); + } + + #[tokio::test] + async fn branch_vs_base_three_dot() { + let dir = TempDir::new().unwrap(); + init_repo_with_commit(dir.path()); + + // Base commit on the default branch. + write(dir.path(), "shared.txt", b"base line\n"); + git(dir.path(), &["add", "."]); + git(dir.path(), &["commit", "-m", "base"]); + let base_branch = { + let repo = GitRepository::open(dir.path()).unwrap(); + repo.current_branch().unwrap() + }; + + // Diverge onto a feature branch: modify shared, add a file. + git(dir.path(), &["checkout", "-b", "feature"]); + write(dir.path(), "shared.txt", b"feature line\n"); + write(dir.path(), "feature_only.txt", b"new on feature\n"); + git(dir.path(), &["add", "."]); + git(dir.path(), &["commit", "-m", "feature work"]); + + let repo = GitRepository::open(dir.path()).unwrap(); + let files = repo.changed_files_vs_base(&base_branch).await.unwrap(); + + assert_eq!(find(&files, "shared.txt").status, ChangeStatus::Modified); + assert_eq!( + find(&files, "feature_only.txt").status, + ChangeStatus::Added + ); + + let d = repo + .file_diff_vs_base(&base_branch, find(&files, "shared.txt")) + .await + .unwrap(); + assert_eq!(d.old_text.as_deref(), Some("base line\n")); + assert_eq!(d.new_text.as_deref(), Some("feature line\n")); + + let d = repo + .file_diff_vs_base(&base_branch, find(&files, "feature_only.txt")) + .await + .unwrap(); + assert_eq!(d.old_text, None); + assert_eq!(d.new_text.as_deref(), Some("new on feature\n")); + } + + #[tokio::test] + async fn base_candidates_include_branches() { + let dir = TempDir::new().unwrap(); + init_repo_with_commit(dir.path()); + git(dir.path(), &["checkout", "-b", "extra-branch"]); + + let repo = GitRepository::open(dir.path()).unwrap(); + let candidates = repo.list_base_candidates().unwrap(); + assert!(candidates.iter().any(|c| c == "extra-branch")); + } +} diff --git a/crates/git/src/lib.rs b/crates/git/src/lib.rs index 54a3cbc5..54abc87e 100644 --- a/crates/git/src/lib.rs +++ b/crates/git/src/lib.rs @@ -1,11 +1,13 @@ mod binary; mod branch; +mod diff; mod repository; mod types; pub mod worktree; pub use binary::GitBinary; pub use branch::BranchNotMerged; +pub use diff::*; pub use repository::GitRepository; pub use types::*; diff --git a/crates/ui_acp/src/ui.rs b/crates/ui_acp/src/ui.rs index bc37781d..7700c94b 100644 --- a/crates/ui_acp/src/ui.rs +++ b/crates/ui_acp/src/ui.rs @@ -972,6 +972,9 @@ impl UserInterface for ACPUserUI { UiEvent::UpdateWorktreeData { .. } => { // Worktree management not supported in ACP UI } + UiEvent::UpdateReviewFiles { .. } | UiEvent::UpdateReviewDiff { .. } => { + // Review panel is GPUI-specific. + } UiEvent::UpdateAllowedModels { .. } => { // Model dropdown filtering is GPUI-specific. } diff --git a/crates/ui_gpui/src/app/commands.rs b/crates/ui_gpui/src/app/commands.rs index 006cb66a..e4b3cd5d 100644 --- a/crates/ui_gpui/src/app/commands.rs +++ b/crates/ui_gpui/src/app/commands.rs @@ -587,6 +587,74 @@ impl Gpui { }); } + // ======================================================================== + // Review panel + // ======================================================================== + + /// Fetch the changed-files listing for the Review panel in the given mode. + pub(crate) fn cmd_list_review_files( + &self, + session_id: String, + mode: code_assistant_core::session::ReviewMode, + base_overrides: std::collections::HashMap, + ) { + let Some(service) = self.session_service() else { + return; + }; + let gpui = self.clone(); + self.dispatch(async move { + match service + .list_review_files(session_id.clone(), mode, base_overrides) + .await + { + Ok(listing) => { + if gpui.is_current_session(&session_id) { + gpui.push_event(UiEvent::UpdateReviewFiles { + repos: listing.repos, + is_git_repo: listing.is_git_repo, + mode: listing.mode, + }); + } + } + Err(e) => debug!("Failed to list review files: {e:#}"), + } + }); + } + + /// Fetch the diff for a single file selected in the Review panel. + pub(crate) fn cmd_get_review_file_diff( + &self, + session_id: String, + repo_root: PathBuf, + mode: code_assistant_core::session::ReviewMode, + base: Option, + file: git::ChangedFile, + ) { + let Some(service) = self.session_service() else { + return; + }; + let gpui = self.clone(); + let path = file.path.clone(); + let event_repo_root = repo_root.clone(); + self.dispatch(async move { + match service + .get_review_file_diff(session_id.clone(), repo_root, mode, base, file) + .await + { + Ok(diff) => { + if gpui.is_current_session(&session_id) { + gpui.push_event(UiEvent::UpdateReviewDiff { + repo_root: event_repo_root, + path, + diff, + }); + } + } + Err(e) => gpui.display_error(format!("Failed to load diff: {e:#}")), + } + }); + } + // ======================================================================== // Projects // ======================================================================== diff --git a/crates/ui_gpui/src/app/event_loop.rs b/crates/ui_gpui/src/app/event_loop.rs index b89ba5ca..73390522 100644 --- a/crates/ui_gpui/src/app/event_loop.rs +++ b/crates/ui_gpui/src/app/event_loop.rs @@ -834,6 +834,49 @@ impl Gpui { cx.refresh(); } + UiEvent::UpdateReviewFiles { + repos, + is_git_repo, + mode, + } => { + debug!( + "UI: UpdateReviewFiles event — {} repos, is_git_repo={}", + repos.len(), + is_git_repo + ); + let repos = repos + .into_iter() + .map(|r| RepoReviewData { + repo_root: r.repo_root, + label: r.label, + current_branch: r.current_branch, + base_candidates: r.base_candidates, + base: r.base, + files: r.files, + }) + .collect(); + *self.current_review_listing.lock().unwrap() = Some(ReviewData { + repos, + is_git_repo, + mode, + }); + cx.refresh(); + } + + UiEvent::UpdateReviewDiff { + repo_root, + path, + diff, + } => { + debug!("UI: UpdateReviewDiff event — path={path}"); + *self.current_review_diff.lock().unwrap() = Some(ReviewDiff { + repo_root, + path, + diff, + }); + cx.refresh(); + } + UiEvent::RefreshCurrentSession { session_id } => { // Another process modified the session file on disk. // Use incremental refresh which diffs the active path and only diff --git a/crates/ui_gpui/src/lib.rs b/crates/ui_gpui/src/lib.rs index 7cc8f7a4..92e6a424 100644 --- a/crates/ui_gpui/src/lib.rs +++ b/crates/ui_gpui/src/lib.rs @@ -46,6 +46,22 @@ pub struct UiSettingsGlobal(pub shared::settings::UiSettings); impl Global for UiSettingsGlobal {} +/// Mutate the global [`UiSettings`] and persist to disk on a background thread. +/// +/// Callable anywhere with access to `&mut App` (including via `Context` deref), +/// so both `MainScreen` handlers and `ReviewView` resize callbacks can use it. +pub fn update_ui_settings(cx: &mut App, f: impl FnOnce(&mut shared::settings::UiSettings)) { + if cx.has_global::() { + let global = cx.global_mut::(); + f(&mut global.0); + let settings = global.0.clone(); + cx.background_spawn(async move { + settings.save(); + }) + .detach(); + } +} + /// Snapshot of worktree/branch data for the active session, kept in `Gpui` /// so that `RootView::render()` can push it into the `WorktreeSelector`. #[derive(Debug, Clone, PartialEq)] @@ -55,6 +71,34 @@ pub struct WorktreeData { pub is_git_repo: bool, } +/// Latest changed-files listing for the Review panel, mirrored from the +/// backend into a global the `ReviewView` reads during render. +#[derive(Debug, Clone, PartialEq)] +pub struct ReviewData { + pub repos: Vec, + pub is_git_repo: bool, + pub mode: code_assistant_core::session::ReviewMode, +} + +/// Per-repo changed-files data within a [`ReviewData`] listing. +#[derive(Debug, Clone, PartialEq)] +pub struct RepoReviewData { + pub repo_root: std::path::PathBuf, + pub label: String, + pub current_branch: Option, + pub base_candidates: Vec, + pub base: Option, + pub files: Vec, +} + +/// Latest loaded diff for a single file in the Review panel. +#[derive(Debug, Clone, PartialEq)] +pub struct ReviewDiff { + pub repo_root: std::path::PathBuf, + pub path: String, + pub diff: git::FileDiffContent, +} + // Our main UI struct that implements the UserInterface trait #[derive(Clone)] pub struct Gpui { @@ -108,6 +152,11 @@ pub struct Gpui { // Current worktree state (branches + worktrees listing from backend) current_worktree_data: Arc>>, + // Review panel state (changed-files listing + selected-file diff) mirrored + // from the backend for the ReviewView to read during render. + current_review_listing: Arc>>, + current_review_diff: Arc>>, + // Last usage from the active session's most recent assistant message. // Stored separately from chat_sessions so it cannot be overwritten by // stale metadata loaded from disk (via UpdateChatList / ListSessions). @@ -335,6 +384,8 @@ impl Gpui { *self.current_permission_tier.lock().unwrap() = None; self.pending_permission_requests.lock().unwrap().clear(); *self.current_worktree_data.lock().unwrap() = None; + *self.current_review_listing.lock().unwrap() = None; + *self.current_review_diff.lock().unwrap() = None; *self.current_session_last_usage.lock().unwrap() = None; *self.current_session_total_usage.lock().unwrap() = None; } @@ -433,6 +484,8 @@ impl Gpui { // Current worktree state current_worktree_data: Arc::new(Mutex::new(None)), + current_review_listing: Arc::new(Mutex::new(None)), + current_review_diff: Arc::new(Mutex::new(None)), // Current session last usage current_session_last_usage: Arc::new(Mutex::new(None)), @@ -733,6 +786,22 @@ impl Gpui { self.current_worktree_data.lock().unwrap().clone() } + pub fn get_current_review_listing(&self) -> Option { + self.current_review_listing.lock().unwrap().clone() + } + + pub fn set_current_review_listing(&self, data: Option) { + *self.current_review_listing.lock().unwrap() = data; + } + + pub fn get_current_review_diff(&self) -> Option { + self.current_review_diff.lock().unwrap().clone() + } + + pub fn set_current_review_diff(&self, diff: Option) { + *self.current_review_diff.lock().unwrap() = diff; + } + pub fn get_current_session_last_usage(&self) -> Option { self.current_session_last_usage.lock().unwrap().clone() } diff --git a/crates/ui_gpui/src/main_screen/mod.rs b/crates/ui_gpui/src/main_screen/mod.rs index 6b93a903..ee56aa05 100644 --- a/crates/ui_gpui/src/main_screen/mod.rs +++ b/crates/ui_gpui/src/main_screen/mod.rs @@ -1,5 +1,6 @@ mod about_dialog; pub mod project_dialog; +pub mod right_panel; mod status_popover; use crate::sidebar::{SessionSidebar, SessionSidebarEvent}; @@ -37,6 +38,9 @@ use tracing::{debug, error, warn}; const SIDEBAR_ANIMATION_DURATION_MS: f32 = 250.0; const SIDEBAR_ANIMATION_FRAME_MS: u64 = 8; // ~120 FPS +/// Fixed width of the right (review) sidebar when fully expanded. +const RIGHT_SIDEBAR_WIDTH: f32 = 440.0; + /// Return the argument text for a syntactically separate `/goal` command. /// Prefixes such as `/goals` remain ordinary input. fn goal_command_args(input: &str) -> Option<&str> { @@ -62,6 +66,103 @@ enum SidebarAnimationState { }, } +/// Drives the width animation for one collapsible sidebar. Multiple animators +/// can share a single ticking task on [`MainScreen`]; each tracks its own +/// easing state independently. +#[derive(Clone, Debug)] +struct SidebarAnimator { + state: SidebarAnimationState, +} + +impl SidebarAnimator { + fn new() -> Self { + Self { + state: SidebarAnimationState::Idle, + } + } + + /// Begin animating towards expanded (`true`) or collapsed (`false`). + /// Reverses smoothly if an animation is already in flight. + fn start(&mut self, should_expand: bool) { + let target = if should_expand { 1.0 } else { 0.0 }; + let now = Instant::now(); + + match &self.state { + SidebarAnimationState::Animating { + width_scale, + target: current_target, + .. + } if *current_target != target => { + // Reverse mid-animation: keep current scale, adjust start for smooth transition + let current_progress = if target == 1.0 { + *width_scale + } else { + 1.0 - *width_scale + }; + let adjusted_start = now + - Duration::from_millis( + (current_progress * SIDEBAR_ANIMATION_DURATION_MS) as u64, + ); + self.state = SidebarAnimationState::Animating { + width_scale: *width_scale, + target, + start_time: adjusted_start, + }; + } + _ => { + let initial = if should_expand { 0.0 } else { 1.0 }; + self.state = SidebarAnimationState::Animating { + width_scale: initial, + target, + start_time: now, + }; + } + } + } + + /// Advance one frame. Returns `true` while still animating. + fn tick(&mut self) -> bool { + match &mut self.state { + SidebarAnimationState::Animating { + width_scale, + target, + start_time, + } => { + let elapsed = start_time.elapsed().as_millis() as f32; + let progress = (elapsed / SIDEBAR_ANIMATION_DURATION_MS).min(1.0); + // ease-out cubic + let eased = 1.0 - (1.0 - progress).powi(3); + + *width_scale = if *target == 1.0 { eased } else { 1.0 - eased }; + + if progress >= 1.0 { + *width_scale = *target; + self.state = SidebarAnimationState::Idle; + false + } else { + true + } + } + SidebarAnimationState::Idle => false, + } + } + + /// Current animation scale: 0.0 = fully collapsed, 1.0 = fully expanded. + /// `collapsed` supplies the resting value when no animation is running. + fn scale(&self, collapsed: bool) -> f32 { + match &self.state { + SidebarAnimationState::Animating { width_scale, .. } => *width_scale, + SidebarAnimationState::Idle => { + if collapsed { + 0.0 + } else { + 1.0 + } + } + } + } +} + /// Events emitted by MainScreen to its parent (the AppShell/RootView). #[derive(Clone, Debug)] pub enum MainScreenEvent { @@ -94,13 +195,27 @@ pub struct MainScreen { /// Pending folder path from the file picker, waiting to create the dialog in render pending_project_path: Option, + // Right (review) sidebar state + right_sidebar_collapsed: bool, + right_panel: Entity, + /// Session id last pushed into the right panel (change detection). + right_panel_session_id: Option, + // Sidebar animation - sidebar_animation_state: SidebarAnimationState, + left_animator: SidebarAnimator, + right_animator: SidebarAnimator, sidebar_content_width: Rc>, sidebar_animation_task: Option>, /// UI zoom scale factor (1.0 = 100%, multiplied with the base font size) ui_scale: f32, + /// Current (persisted) width of the right review sidebar when expanded. + right_sidebar_width: Pixels, + /// Whether the user is currently dragging the sidebar's resize handle. + right_sidebar_resizing: bool, + /// Drag anchor: pointer x and sidebar width captured at mouse-down. + resize_start_x: f32, + resize_start_width: f32, /// Cached context token limit for the current model (model_name, limit). /// Reloaded when the model changes. context_limit_cache: Option<(String, u32)>, @@ -144,10 +259,19 @@ impl MainScreen { .map(|s| s.0.ui_scale) .unwrap_or(1.0); + // Restore the persisted right-sidebar width (default fallback). + let initial_sidebar_width = cx + .try_global::() + .and_then(|s| s.0.right_sidebar_width) + .unwrap_or(RIGHT_SIDEBAR_WIDTH); + // Watch for window move / resize so we can persist bounds. let window_bounds_subscription = cx.observe_window_bounds(window, Self::on_window_bounds_changed); + // Create the right (review) sidebar panel. + let right_panel = cx.new(|cx| right_panel::RightPanel::new(window, cx)); + let mut root_view = Self { input_area, project_sidebar, @@ -166,11 +290,20 @@ impl MainScreen { about_dialog: None, pending_project_path: None, - sidebar_animation_state: SidebarAnimationState::Idle, + right_sidebar_collapsed: true, // Review sidebar hidden by default + right_panel, + right_panel_session_id: None, + + left_animator: SidebarAnimator::new(), + right_animator: SidebarAnimator::new(), sidebar_content_width: Rc::new(Cell::new(px(0.0))), sidebar_animation_task: None, ui_scale: initial_scale, + right_sidebar_width: px(initial_sidebar_width), + right_sidebar_resizing: false, + resize_start_x: 0.0, + resize_start_width: 0.0, context_limit_cache: None, _input_area_subscription: input_area_subscription, _plan_banner_subscription: plan_banner_subscription, @@ -194,10 +327,50 @@ impl MainScreen { ) { let should_expand = self.sidebar_collapsed; self.sidebar_collapsed = !self.sidebar_collapsed; - self.start_sidebar_animation(should_expand, cx); + self.left_animator.start(should_expand); + self.ensure_sidebar_animation_task(cx); cx.notify(); } + pub fn on_toggle_right_sidebar( + &mut self, + _: &ClickEvent, + _window: &mut gpui::Window, + cx: &mut Context, + ) { + let should_expand = self.right_sidebar_collapsed; + self.right_sidebar_collapsed = !self.right_sidebar_collapsed; + self.right_animator.start(should_expand); + self.ensure_sidebar_animation_task(cx); + + // When opening, make sure the panel reflects the current session and + // has fresh data. + if should_expand { + let session_id = self.current_session_id.clone(); + self.right_panel.update(cx, |panel, cx| { + panel.set_session(session_id, cx); + }); + } + + // Persist the open/closed state for the active session. + if let Some(session_id) = &self.current_session_id { + if let Ok(mut store) = crate::shared::ui_state::UiStateStore::global().lock() { + store.set_right_panel_open(session_id, !self.right_sidebar_collapsed); + } + if let Some(sender) = cx.try_global::() { + let _ = sender.0.try_send(UiEvent::PersistUiState); + } + } + + cx.notify(); + } + + fn ensure_sidebar_animation_task(&mut self, cx: &mut Context) { + if self.sidebar_animation_task.is_none() { + self.start_sidebar_animation_task(cx); + } + } + fn on_open_settings( &mut self, _: &ClickEvent, @@ -233,47 +406,8 @@ impl MainScreen { // ── Sidebar animation ───────────────────────────────────────────────── - fn start_sidebar_animation(&mut self, should_expand: bool, cx: &mut Context) { - let target = if should_expand { 1.0 } else { 0.0 }; - let now = Instant::now(); - - match &self.sidebar_animation_state { - SidebarAnimationState::Animating { - width_scale, - target: current_target, - .. - } if *current_target != target => { - // Reverse mid-animation: keep current scale, adjust start for smooth transition - let current_progress = if target == 1.0 { - *width_scale - } else { - 1.0 - *width_scale - }; - let adjusted_start = now - - Duration::from_millis( - (current_progress * SIDEBAR_ANIMATION_DURATION_MS) as u64, - ); - self.sidebar_animation_state = SidebarAnimationState::Animating { - width_scale: *width_scale, - target, - start_time: adjusted_start, - }; - } - _ => { - let initial = if should_expand { 0.0 } else { 1.0 }; - self.sidebar_animation_state = SidebarAnimationState::Animating { - width_scale: initial, - target, - start_time: now, - }; - } - } - - if self.sidebar_animation_task.is_none() { - self.start_sidebar_animation_task(cx); - } - } - + /// Ticking task shared by both sidebar animators. Runs until neither the + /// left nor the right animator is still animating. fn start_sidebar_animation_task(&mut self, cx: &mut Context) { let task = cx.spawn(async move |weak_entity, async_cx| { loop { @@ -283,14 +417,13 @@ impl MainScreen { .await; let should_continue = weak_entity.update(async_cx, |view, cx| { - view.update_sidebar_animation(); - match &view.sidebar_animation_state { - SidebarAnimationState::Idle => false, - _ => { - cx.notify(); - true - } + let left = view.left_animator.tick(); + let right = view.right_animator.tick(); + let cont = left || right; + if cont { + cx.notify(); } + cont }); if let Ok(should_continue) = should_continue { @@ -308,41 +441,14 @@ impl MainScreen { self.sidebar_animation_task = Some(task); } - fn update_sidebar_animation(&mut self) { - match &mut self.sidebar_animation_state { - SidebarAnimationState::Animating { - width_scale, - target, - start_time, - } => { - let elapsed = start_time.elapsed().as_millis() as f32; - let progress = (elapsed / SIDEBAR_ANIMATION_DURATION_MS).min(1.0); - // ease-out cubic - let eased = 1.0 - (1.0 - progress).powi(3); - - *width_scale = if *target == 1.0 { eased } else { 1.0 - eased }; - - if progress >= 1.0 { - *width_scale = *target; - self.sidebar_animation_state = SidebarAnimationState::Idle; - } - } - SidebarAnimationState::Idle => {} - } + /// Current left (project) sidebar animation scale. + fn sidebar_animation_scale(&self) -> f32 { + self.left_animator.scale(self.sidebar_collapsed) } - /// Current sidebar animation scale: 0.0 = fully collapsed, 1.0 = fully expanded - fn sidebar_animation_scale(&self) -> f32 { - match &self.sidebar_animation_state { - SidebarAnimationState::Animating { width_scale, .. } => *width_scale, - SidebarAnimationState::Idle => { - if self.sidebar_collapsed { - 0.0 - } else { - 1.0 - } - } - } + /// Current right (review) sidebar animation scale. + fn right_sidebar_animation_scale(&self) -> f32 { + self.right_animator.scale(self.right_sidebar_collapsed) } fn on_plan_banner_event( @@ -446,15 +552,7 @@ impl MainScreen { /// Update the global [`UiSettings`], persist to disk on a background thread. fn update_settings(cx: &mut Context, f: impl FnOnce(&mut settings::UiSettings)) { - if cx.has_global::() { - let global = cx.global_mut::(); - f(&mut global.0); - let settings = global.0.clone(); - cx.background_spawn(async move { - settings.save(); - }) - .detach(); - } + crate::update_ui_settings(cx, f); } /// Called when the window is moved or resized. @@ -1114,9 +1212,11 @@ impl MainScreen { ("".to_string(), Vec::new(), None) }; - // Clear worktree data while we hold the ref + // Clear worktree + review data while we hold the ref let gpui_handle = if let Some(gpui) = &gpui { *gpui.current_worktree_data.lock().unwrap() = None; + gpui.set_current_review_listing(None); + gpui.set_current_review_diff(None); Some((*gpui).clone()) } else { None @@ -1159,6 +1259,31 @@ impl MainScreen { sel.set_local(window, cx); }); }); + + // Restore the right (review) sidebar's open state for the new session. + let restored_open = new_session_id + .as_ref() + .and_then(|id| { + crate::shared::ui_state::UiStateStore::try_global() + .and_then(|store| store.lock().ok()) + .map(|mut store| store.get_right_panel_open(id)) + }) + .unwrap_or(false); + + // Animate to the restored state if it differs from the current one. + if restored_open == self.right_sidebar_collapsed { + // (collapsed == !open) — they differ, so kick the animation. + self.right_animator.start(restored_open); + self.ensure_sidebar_animation_task(cx); + } + self.right_sidebar_collapsed = !restored_open; + + // Point the panel at the new session (clears stale tree/diff) and, when + // open, request fresh data. + self.right_panel_session_id = new_session_id.clone(); + self.right_panel.update(cx, |panel, cx| { + panel.set_session(new_session_id.clone(), cx); + }); } } @@ -1429,6 +1554,7 @@ impl Render for MainScreen { let new_project_dialog = self.new_project_dialog.clone(); let about_dialog = self.about_dialog.clone(); let sidebar_scale = self.sidebar_animation_scale(); + let right_sidebar_scale = self.right_sidebar_animation_scale(); let permission_prompts = self.render_permission_prompts(cx); // Main container with titlebar and content @@ -1570,6 +1696,31 @@ impl Render for MainScreen { .flex() .items_center() .gap_1() + // Review (right) sidebar toggle button + .child( + div() + .id("toggle-right-sidebar-btn") + .size(px(28.)) + .rounded_sm() + .flex() + .items_center() + .justify_center() + .cursor_pointer() + .hover(|s| s.bg(cx.theme().muted)) + .child( + Icon::default() + .path(SharedString::from( + if self.right_sidebar_collapsed { + "icons/panel_right_open.svg" + } else { + "icons/panel_right_close.svg" + }, + )) + .with_size(Size::Small) + .text_color(cx.theme().muted_foreground), + ) + .on_click(cx.listener(Self::on_toggle_right_sidebar)), + ) .child( div() .id("about-btn") @@ -1694,12 +1845,98 @@ impl Render for MainScreen { .border_color(cx.theme().border) .child(self.input_area.clone()), ), - ), + ) + // Right sidebar: Review panel (animated width, resizable) + .when(right_sidebar_scale > 0.0, { + let right_panel = self.right_panel.clone(); + let border = cx.theme().border; + let handle_color = if self.right_sidebar_resizing { + cx.theme().drag_border + } else { + cx.theme().border + }; + let sidebar_width = self.right_sidebar_width * right_sidebar_scale; + let resizing = self.right_sidebar_resizing; + let handle_mouse_down = cx.listener( + |this, ev: &gpui::MouseDownEvent, _window, cx| { + this.right_sidebar_resizing = true; + this.resize_start_x = f32::from(ev.position.x); + this.resize_start_width = f32::from(this.right_sidebar_width); + cx.notify(); + }, + ); + move |el| { + el.child( + div() + .relative() + .flex_none() + .h_full() + .overflow_hidden() + .border_l_1() + .border_color(border) + .w(sidebar_width) + .child(right_panel) + // Left-edge drag handle to resize the sidebar. + .child( + div() + .id("right-sidebar-resize-handle") + .absolute() + .top_0() + .left_0() + .h_full() + .w(px(6.)) + .cursor_col_resize() + .bg(handle_color) + .opacity(if resizing { 1.0 } else { 0.0 }) + .hover(|s| s.opacity(1.0)) + .on_mouse_down( + gpui::MouseButton::Left, + handle_mouse_down, + ), + ), + ) + } + }), ) // Modal dialog overlay for new project creation .when_some(new_project_dialog, |el, dialog| el.child(dialog)) // Modal "About" dialog overlay .when_some(about_dialog, |el, dialog| el.child(dialog)) + // While resizing the sidebar, a transparent full-window overlay + // captures pointer motion so the drag tracks even over child views. + .when(self.right_sidebar_resizing, |el| { + el.child( + div() + .absolute() + .inset_0() + .cursor_col_resize() + .on_mouse_move(cx.listener(|this, ev: &gpui::MouseMoveEvent, _window, cx| { + if !this.right_sidebar_resizing { + return; + } + // Dragging the left edge leftward widens the sidebar. + let delta = f32::from(ev.position.x) - this.resize_start_x; + let new_width = + (this.resize_start_width - delta).clamp(320.0, 1800.0); + this.right_sidebar_width = px(new_width); + cx.notify(); + })) + .on_mouse_up( + gpui::MouseButton::Left, + cx.listener(|this, _ev: &gpui::MouseUpEvent, _window, cx| { + if !this.right_sidebar_resizing { + return; + } + this.right_sidebar_resizing = false; + let width = f32::from(this.right_sidebar_width); + crate::update_ui_settings(cx, |s| { + s.right_sidebar_width = Some(width); + }); + cx.notify(); + }), + ), + ) + }) } } diff --git a/crates/ui_gpui/src/main_screen/right_panel/mod.rs b/crates/ui_gpui/src/main_screen/right_panel/mod.rs new file mode 100644 index 00000000..099f258c --- /dev/null +++ b/crates/ui_gpui/src/main_screen/right_panel/mod.rs @@ -0,0 +1,88 @@ +//! The right sidebar's view switcher. +//! +//! For now the sidebar hosts a single view — [`review_view::ReviewView`] — but +//! it is structured as a switcher so additional views (e.g. an outline or a +//! terminal) can be added without reworking the [`crate::main_screen::MainScreen`] +//! shell that owns it. + +pub mod review_view; + +use gpui::{Context, Entity, FocusHandle, Focusable, Render, Window, div, prelude::*}; +use review_view::ReviewView; + +/// Which view the right panel is currently showing. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum RightPanelView { + Review, +} + +impl RightPanelView { + /// Stable string used for persistence. + pub fn as_str(self) -> &'static str { + match self { + RightPanelView::Review => "review", + } + } + + /// Parse a persisted string back into a view (defaults to `Review`). + #[allow(clippy::should_implement_trait)] + pub fn from_str(_s: &str) -> Self { + // Only one view exists today; everything maps to Review. + RightPanelView::Review + } +} + +pub struct RightPanel { + active_view: RightPanelView, + review_view: Entity, + focus_handle: FocusHandle, +} + +impl RightPanel { + pub fn new(window: &mut Window, cx: &mut Context) -> Self { + let review_view = cx.new(|cx| ReviewView::new(window, cx)); + Self { + active_view: RightPanelView::Review, + review_view, + focus_handle: cx.focus_handle(), + } + } + + #[allow(dead_code)] + pub fn active_view(&self) -> RightPanelView { + self.active_view + } + + #[allow(dead_code)] + pub fn set_active_view(&mut self, view: RightPanelView, cx: &mut Context) { + if self.active_view != view { + self.active_view = view; + cx.notify(); + } + } + + /// Point the active view(s) at a session. + pub fn set_session(&mut self, session_id: Option, cx: &mut Context) { + self.review_view.update(cx, |v, cx| v.set_session(session_id, cx)); + } + + /// Re-request data for the active view. + pub fn reload(&mut self, cx: &mut Context) { + self.review_view.update(cx, |v, cx| v.reload(cx)); + } +} + +impl Focusable for RightPanel { + fn focus_handle(&self, _cx: &gpui::App) -> FocusHandle { + self.focus_handle.clone() + } +} + +impl Render for RightPanel { + fn render(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { + let body = match self.active_view { + RightPanelView::Review => self.review_view.clone().into_any_element(), + }; + div().size_full().child(body) + } +} diff --git a/crates/ui_gpui/src/main_screen/right_panel/review_view.rs b/crates/ui_gpui/src/main_screen/right_panel/review_view.rs new file mode 100644 index 00000000..5f3b62c6 --- /dev/null +++ b/crates/ui_gpui/src/main_screen/right_panel/review_view.rs @@ -0,0 +1,733 @@ +//! The "Review" view for the right sidebar: a compare-mode selector plus a +//! two-column body — the unified diff on the **left** and, on the **right**, a +//! per-repo set of changed-file trees (each repo gets its own base-branch +//! selector in "Branch vs base" mode). The two columns are separated by a +//! draggable divider (`h_resizable`). +//! +//! Backend data arrives through the `current_review_listing` / `current_review_diff` +//! globals on [`Gpui`]; this view consumes them in `render` by diffing against +//! cached copies (the same "sync-in-render" technique the worktree selector uses). + +use crate::shared::file_tree::{ChangedFilesTree, ChangedFilesTreeEvent}; +use crate::tool_cards::diff_card::render_unified_diff; +use crate::{Gpui, ReviewData}; +use code_assistant_core::session::ReviewMode; +use gpui::{ + Context, Entity, EventEmitter, FocusHandle, Focusable, FontWeight, Render, ScrollHandle, + Subscription, Window, div, prelude::*, px, rems, +}; +use gpui_component::{ + ActiveTheme, Icon, Sizable, Size, + resizable::{ResizableState, h_resizable, resizable_panel}, + scroll::ScrollableElement, + select::{Select, SelectEvent, SelectItem, SelectState}, + v_flex, +}; +use std::collections::HashMap; +use std::path::PathBuf; + +/// Default width (px) of the file-tree column when nothing is persisted. +const DEFAULT_TREE_WIDTH: f32 = 240.0; + +// --------------------------------------------------------------------------- +// Compare-mode dropdown +// --------------------------------------------------------------------------- + +#[derive(Clone, Debug)] +struct ModeOption { + label: String, + value: ReviewMode, +} + +impl SelectItem for ModeOption { + type Value = ReviewMode; + fn title(&self) -> gpui::SharedString { + self.label.clone().into() + } + fn display_title(&self) -> Option { + None + } + fn value(&self) -> &Self::Value { + &self.value + } +} + +fn mode_options() -> Vec { + vec![ + ModeOption { + label: "Working tree".to_string(), + value: ReviewMode::WorkingTree, + }, + ModeOption { + label: "Branch vs base".to_string(), + value: ReviewMode::BranchVsBase, + }, + ] +} + +// --------------------------------------------------------------------------- +// Base-branch dropdown +// --------------------------------------------------------------------------- + +#[derive(Clone, Debug)] +struct BaseOption { + branch: String, +} + +impl SelectItem for BaseOption { + type Value = String; + fn title(&self) -> gpui::SharedString { + self.branch.clone().into() + } + fn display_title(&self) -> Option { + None + } + fn value(&self) -> &Self::Value { + &self.branch + } +} + +// --------------------------------------------------------------------------- +// Per-repo section +// --------------------------------------------------------------------------- + +/// One git repo's changed-files tree plus its base selector, rendered as a +/// collapsible section in the right column. +struct RepoSection { + repo_root: PathBuf, + label: String, + tree: Entity, + base_state: Entity>>, + base_candidates: Vec, + base: Option, + collapsed: bool, + _tree_sub: Subscription, + _base_sub: Subscription, +} + +// --------------------------------------------------------------------------- +// ReviewView +// --------------------------------------------------------------------------- + +pub struct ReviewView { + session_id: Option, + mode_state: Entity>>, + + /// Current compare mode (drives requests). Base is tracked per repo. + mode: ReviewMode, + is_git_repo: bool, + + /// Per-repo sections in the right column. + repos: Vec, + /// Currently selected file as `(repo_root, repo-relative path)`. + selected: Option<(PathBuf, String)>, + /// User's explicit per-repo base choices for this session. + base_overrides: HashMap, + /// Persisted default base ref, seeds a repo's base when it has no override. + default_base: Option, + + /// Two-column split state (LEFT = diff, RIGHT = tree column). + split_state: Entity, + /// Persisted tree-column width, used as the initial panel size. + tree_width: f32, + /// Scroll position of the diff pane. + diff_scroll: ScrollHandle, + + /// Last listing consumed from the global (change detection). + last_listing: Option, + + focus_handle: FocusHandle, + _mode_sub: Subscription, +} + +impl EventEmitter<()> for ReviewView {} + +impl ReviewView { + pub fn new(window: &mut Window, cx: &mut Context) -> Self { + let mode_state = cx.new(|cx| { + let mut state = SelectState::new(mode_options(), None, window, cx); + state.set_selected_value(&ReviewMode::WorkingTree, window, cx); + state + }); + let mode_sub = cx.subscribe_in(&mode_state, window, Self::on_mode_event); + + // Seed persisted preferences (default base + tree width) from settings. + let (default_base, tree_width) = cx + .try_global::() + .map(|g| { + ( + g.0.review_default_base.clone(), + g.0.review_tree_width.unwrap_or(DEFAULT_TREE_WIDTH), + ) + }) + .unwrap_or((None, DEFAULT_TREE_WIDTH)); + + let split_state = cx.new(|_| ResizableState::default()); + + Self { + session_id: None, + mode_state, + mode: ReviewMode::WorkingTree, + is_git_repo: false, + repos: Vec::new(), + selected: None, + base_overrides: HashMap::new(), + default_base, + split_state, + tree_width, + diff_scroll: ScrollHandle::new(), + last_listing: None, + focus_handle: cx.focus_handle(), + _mode_sub: mode_sub, + } + } + + /// Point the view at a session and request its changed files. + pub fn set_session(&mut self, session_id: Option, cx: &mut Context) { + self.session_id = session_id; + // Reset per-session state; fresh data will arrive via the global. + self.selected = None; + self.last_listing = None; + self.repos.clear(); + self.base_overrides.clear(); + + // Restore the persisted compare mode for this session. The selector + // resyncs from the echoed listing on the next render. + if let Some(id) = &self.session_id { + if let Some(store) = crate::shared::ui_state::UiStateStore::try_global() + && let Ok(mut store) = store.lock() + { + let (mode, _base) = store.get_review_settings(id); + self.mode = match mode.as_deref() { + Some("branch_vs_base") => ReviewMode::BranchVsBase, + _ => ReviewMode::WorkingTree, + }; + } + } else { + self.mode = ReviewMode::WorkingTree; + } + + self.request_listing(cx); + } + + /// Persist the current compare mode for the active session. + fn persist_mode(&self, cx: &mut Context) { + let Some(session_id) = &self.session_id else { + return; + }; + let mode = match self.mode { + ReviewMode::WorkingTree => "working_tree", + ReviewMode::BranchVsBase => "branch_vs_base", + }; + if let Ok(mut store) = crate::shared::ui_state::UiStateStore::global().lock() { + store.set_review_settings(session_id, Some(mode.to_string()), None); + } + if let Some(sender) = cx.try_global::() { + let _ = sender + .0 + .try_send(code_assistant_core::ui::ui_events::UiEvent::PersistUiState); + } + } + + /// Re-request the changed-files listing for the current mode. + pub fn reload(&mut self, cx: &mut Context) { + self.request_listing(cx); + } + + fn request_listing(&self, cx: &mut Context) { + let Some(session_id) = self.session_id.clone() else { + return; + }; + if let Some(gpui) = cx.try_global::() { + gpui.cmd_list_review_files(session_id, self.mode, self.base_overrides.clone()); + } + } + + fn request_diff(&self, repo_root: &PathBuf, path: &str, cx: &mut Context) { + let Some(session_id) = self.session_id.clone() else { + return; + }; + let Some(listing) = &self.last_listing else { + return; + }; + let Some(repo) = listing.repos.iter().find(|r| &r.repo_root == repo_root) else { + return; + }; + let Some(file) = repo.files.iter().find(|f| f.path == path).cloned() else { + return; + }; + let base = repo.base.clone(); + if let Some(gpui) = cx.try_global::() { + gpui.cmd_get_review_file_diff(session_id, repo_root.clone(), self.mode, base, file); + } + } + + fn on_file_selected(&mut self, repo_root: PathBuf, path: String, cx: &mut Context) { + // Clear selection highlight in sibling repos' trees. + for section in &self.repos { + if section.repo_root != repo_root { + section.tree.update(cx, |t, cx| t.set_selected(None, cx)); + } + } + self.request_diff(&repo_root, &path, cx); + self.selected = Some((repo_root, path)); + cx.notify(); + } + + fn on_repo_base_changed(&mut self, repo_root: PathBuf, branch: String, cx: &mut Context) { + self.base_overrides.insert(repo_root, branch.clone()); + // Remember this as the global default for future repos/sessions. + self.default_base = Some(branch.clone()); + crate::update_ui_settings(cx, |s| s.review_default_base = Some(branch)); + self.selected = None; + self.request_listing(cx); + cx.notify(); + } + + fn on_mode_event( + &mut self, + _: &Entity>>, + event: &SelectEvent>, + _window: &mut Window, + cx: &mut Context, + ) { + if let SelectEvent::Confirm(Some(mode)) = event + && *mode != self.mode + { + self.mode = *mode; + // Selecting a new mode invalidates the current diff selection. + self.selected = None; + self.persist_mode(cx); + self.request_listing(cx); + cx.notify(); + } + } + + /// Consume the latest listing from the global if it changed. + fn sync_listing(&mut self, window: &mut Window, cx: &mut Context) { + let listing = cx + .try_global::() + .and_then(|g| g.get_current_review_listing()); + + if listing == self.last_listing { + return; + } + self.last_listing = listing.clone(); + + let Some(listing) = listing else { + // Cleared (e.g. session change): drop all sections. + self.repos.clear(); + return; + }; + + self.is_git_repo = listing.is_git_repo; + self.mode = listing.mode; + + // Sync the mode selector to the echoed mode. + self.mode_state.update(cx, |state, cx| { + state.set_selected_value(&listing.mode, window, cx); + }); + + // Rebuild sections when the set of repos changes; otherwise update the + // existing sections in place (preserving expansion / selection). + let incoming_roots: Vec = + listing.repos.iter().map(|r| r.repo_root.clone()).collect(); + let current_roots: Vec = + self.repos.iter().map(|r| r.repo_root.clone()).collect(); + + if incoming_roots != current_roots { + self.repos = listing + .repos + .iter() + .map(|r| self.build_section(r, window, cx)) + .collect(); + } else { + for (section, data) in self.repos.iter_mut().zip(listing.repos.iter()) { + Self::update_section(section, data, window, cx); + } + } + + // Apply the persisted default base to any repo that has no explicit + // override yet and whose resolved base differs. Seeding the override + // and re-requesting makes the default actually take effect. This + // terminates: once the backend echoes the default as the repo's base, + // the `base != default` guard stops further seeding. + if let Some(default) = self.default_base.clone() { + let mut seeded = false; + for data in &listing.repos { + if !self.base_overrides.contains_key(&data.repo_root) + && data.base_candidates.iter().any(|c| c == &default) + && data.base.as_deref() != Some(default.as_str()) + { + self.base_overrides + .insert(data.repo_root.clone(), default.clone()); + seeded = true; + } + } + if seeded { + self.request_listing(cx); + } + } + + // Reconcile the selection against the fresh listing. + let still_present = self.selected.as_ref().is_some_and(|(root, path)| { + listing + .repos + .iter() + .any(|r| &r.repo_root == root && r.files.iter().any(|f| &f.path == path)) + }); + if !still_present { + self.selected = None; + } + let selected = self.selected.clone(); + for section in &self.repos { + let sel = selected + .as_ref() + .filter(|(root, _)| root == §ion.repo_root) + .map(|(_, path)| path.clone()); + section.tree.update(cx, |t, cx| t.set_selected(sel, cx)); + } + } + + /// Create a fresh [`RepoSection`] for `data`, wiring per-repo subscriptions + /// that capture the repo root so events identify their origin. + fn build_section( + &self, + data: &crate::RepoReviewData, + window: &mut Window, + cx: &mut Context, + ) -> RepoSection { + let tree = cx.new(ChangedFilesTree::new); + tree.update(cx, |t, cx| t.set_files(&data.files, cx)); + + let root_for_tree = data.repo_root.clone(); + let tree_sub = cx.subscribe_in( + &tree, + window, + move |this, _tree, event, _window, cx| { + let ChangedFilesTreeEvent::FileSelected(path) = event; + this.on_file_selected(root_for_tree.clone(), path.clone(), cx); + }, + ); + + let items: Vec = data + .base_candidates + .iter() + .map(|b| BaseOption { branch: b.clone() }) + .collect(); + let effective_base = self.effective_base(data); + let base_state = cx.new(|cx| { + let mut state = SelectState::new(items, None, window, cx); + if let Some(base) = &effective_base { + state.set_selected_value(base, window, cx); + } + state + }); + + let root_for_base = data.repo_root.clone(); + let base_sub = cx.subscribe_in( + &base_state, + window, + move |this, _state, event, _window, cx| { + if let SelectEvent::Confirm(Some(branch)) = event { + this.on_repo_base_changed(root_for_base.clone(), branch.clone(), cx); + } + }, + ); + + RepoSection { + repo_root: data.repo_root.clone(), + label: data.label.clone(), + tree, + base_state, + base_candidates: data.base_candidates.clone(), + base: effective_base, + collapsed: false, + _tree_sub: tree_sub, + _base_sub: base_sub, + } + } + + /// Update an existing section's tree + base selector in place. + fn update_section( + section: &mut RepoSection, + data: &crate::RepoReviewData, + window: &mut Window, + cx: &mut Context, + ) { + section.label = data.label.clone(); + section.tree.update(cx, |t, cx| t.set_files(&data.files, cx)); + + if section.base_candidates != data.base_candidates { + section.base_candidates = data.base_candidates.clone(); + let items: Vec = data + .base_candidates + .iter() + .map(|b| BaseOption { branch: b.clone() }) + .collect(); + section.base_state.update(cx, |state, cx| { + state.set_items(items, window, cx); + }); + } + section.base = data.base.clone(); + if let Some(base) = &data.base { + section.base_state.update(cx, |state, cx| { + state.set_selected_value(base, window, cx); + }); + } + } + + /// The base ref to preselect for a repo: an explicit session override, else + /// the persisted default (when it is a candidate), else the resolved base. + fn effective_base(&self, data: &crate::RepoReviewData) -> Option { + if let Some(base) = self.base_overrides.get(&data.repo_root) { + return Some(base.clone()); + } + if let Some(default) = &self.default_base + && data.base_candidates.iter().any(|c| c == default) + { + return Some(default.clone()); + } + data.base.clone() + } + + fn render_diff_pane(&self, window: &mut Window, cx: &mut Context) -> gpui::AnyElement { + let muted = cx.theme().muted_foreground; + + let placeholder = |msg: &str| { + div() + .size_full() + .flex() + .items_center() + .justify_center() + .p_4() + .text_sm() + .text_color(muted) + .child(msg.to_string()) + .into_any_element() + }; + + let Some((sel_root, sel_path)) = self.selected.clone() else { + return placeholder("Select a file to view its diff"); + }; + + let diff = cx + .try_global::() + .and_then(|g| g.get_current_review_diff()); + + let Some(diff) = diff.filter(|d| d.repo_root == sel_root && d.path == sel_path) else { + return placeholder("Loading diff…"); + }; + + if diff.diff.is_binary { + return placeholder("Binary file — no text diff"); + } + if diff.diff.too_large { + return placeholder("File too large to display"); + } + + let old = diff.diff.old_text.clone().unwrap_or_default(); + let new = diff.diff.new_text.clone().unwrap_or_default(); + if old.is_empty() && new.is_empty() { + return placeholder("No changes to display"); + } + + let rem_size = window.rem_size(); + let theme = cx.theme(); + // Match the chat diff card's body styling so the unified diff (with its + // red/green row backgrounds) renders identically here. + let is_dark = theme.background.l < 0.5; + let body_bg = if is_dark { + gpui::hsla(0.0, 0.0, 0.08, 1.0) + } else { + gpui::hsla(0.0, 0.0, 0.97, 1.0) + }; + let line_height_px = rems(1.25).to_pixels(rem_size).round(); + let diff = render_unified_diff(&old, &new, theme, Some(1), rem_size); + + div() + .id("review-diff-scroll") + .size_full() + .overflow_scroll() + .track_scroll(&self.diff_scroll) + .child( + div() + .w_full() + .py_1() + .bg(body_bg) + .flex() + .flex_col() + .text_size(rems(0.78125)) + .line_height(line_height_px) + .font_family("Menlo") + .font_weight(FontWeight(400.0)) + .child(diff), + ) + .into_any_element() + } + + /// Render the right column: a scrollable stack of per-repo sections. + fn render_tree_column(&self, cx: &mut Context) -> gpui::AnyElement { + let muted = cx.theme().muted_foreground; + let fg = cx.theme().foreground; + let border = cx.theme().border; + let branch_mode = matches!(self.mode, ReviewMode::BranchVsBase); + // A single repo whose label matches the project needn't show its header + // chrome; but keeping it uniform is simpler and clarifies multi-repo. + let show_headers = self.repos.len() > 1 || branch_mode; + + let mut column = v_flex().size_full().overflow_y_scrollbar(); + + for (ix, section) in self.repos.iter().enumerate() { + let repo_root = section.repo_root.clone(); + let collapsed = section.collapsed; + + let mut section_el = v_flex().w_full(); + + if show_headers { + let chevron = if collapsed { + "icons/chevron_right.svg" + } else { + "icons/chevron_down.svg" + }; + let header = div() + .id(gpui::SharedString::from(format!("repo-header-{ix}"))) + .flex() + .flex_row() + .items_center() + .gap_1p5() + .px_2() + .py_1() + .border_b_1() + .border_color(border) + .cursor_pointer() + .hover(|s| s.bg(cx.theme().muted)) + .child(gpui::svg().size(px(12.)).path(chevron).text_color(muted)) + .child( + div() + .flex_1() + .text_sm() + .font_weight(gpui::FontWeight::MEDIUM) + .text_color(fg) + .child(section.label.clone()), + ) + .on_click(cx.listener(move |this, _ev, _window, cx| { + if let Some(s) = + this.repos.iter_mut().find(|s| s.repo_root == repo_root) + { + s.collapsed = !s.collapsed; + cx.notify(); + } + })); + section_el = section_el.child(header); + } + + if !collapsed { + if branch_mode { + section_el = section_el.child( + div().px_2().py_1().child( + Select::new(§ion.base_state) + .placeholder("Base") + .with_size(Size::XSmall) + .icon( + Icon::default() + .path("icons/chevron_up_down.svg") + .with_size(Size::XSmall) + .text_color(muted), + ) + .w_full(), + ), + ); + } + section_el = section_el.child(section.tree.clone()); + } + + column = column.child(section_el); + } + + column.into_any_element() + } +} + +impl Focusable for ReviewView { + fn focus_handle(&self, _cx: &gpui::App) -> FocusHandle { + self.focus_handle.clone() + } +} + +impl Render for ReviewView { + fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { + // Pull fresh backend data before laying out. + self.sync_listing(window, cx); + + let muted = cx.theme().muted_foreground; + let border = cx.theme().border; + + if !self.is_git_repo && self.last_listing.is_some() { + return v_flex() + .size_full() + .items_center() + .justify_center() + .p_4() + .text_sm() + .text_color(muted) + .child("Not a git repository") + .into_any_element(); + } + + // Header: compare-mode selector only (base selectors live per repo). + let header = div() + .flex() + .flex_row() + .items_center() + .gap_2() + .p_2() + .border_b_1() + .border_color(border) + .child( + Select::new(&self.mode_state) + .placeholder("Compare") + .with_size(Size::XSmall) + .icon( + Icon::default() + .path("icons/chevron_up_down.svg") + .with_size(Size::XSmall) + .text_color(muted), + ) + .min_w(px(130.)), + ); + + // Two-column body: diff (LEFT, grows) | tree column (RIGHT, sized). + let diff_pane = self.render_diff_pane(window, cx); + let tree_column = self.render_tree_column(cx); + + let body = h_resizable("review-split") + .with_state(&self.split_state) + .on_resize(|state, _window, cx| { + if let Some(width) = state.read(cx).sizes().get(1).copied() { + let w = f32::from(width); + crate::update_ui_settings(cx, |s| s.review_tree_width = Some(w)); + } + }) + .child(resizable_panel().child(diff_pane)) + .child( + resizable_panel() + .size(px(self.tree_width)) + .size_range(px(180.)..px(600.)) + .flex_none() + .child( + div() + .size_full() + .border_l_1() + .border_color(border) + .child(tree_column), + ), + ); + + v_flex() + .size_full() + .child(header) + .child(div().flex_1().min_h_0().child(body)) + .into_any_element() + } +} diff --git a/crates/ui_gpui/src/shared/file_icons.rs b/crates/ui_gpui/src/shared/file_icons.rs index 332f92a5..07343248 100644 --- a/crates/ui_gpui/src/shared/file_icons.rs +++ b/crates/ui_gpui/src/shared/file_icons.rs @@ -188,6 +188,44 @@ impl FileIcons { } } + /// Get an icon for a file given its name (or path). + /// + /// Resolution order mirrors editor conventions: an exact stem match + /// (e.g. `Cargo.toml`, `Dockerfile`) wins, then the longest matching + /// suffix (e.g. `rs`, `tar.gz`), then a generic file icon fallback. + pub fn get_icon_for_filename(&self, name: &str) -> Option { + // Use just the final path component for matching. + let file_name = name + .rsplit(['/', '\\']) + .next() + .unwrap_or(name) + .to_lowercase(); + + // 1) Exact stem (full filename) match. + if let Some(typ) = self.config.stems.get(&file_name) + && let Some(icon) = self.get_type_icon(typ) + { + return Some(icon); + } + + // 2) Suffix match. Try progressively shorter suffixes so multi-part + // extensions (`tar.gz`) are preferred over their tail (`gz`). + let parts: Vec<&str> = file_name.split('.').collect(); + if parts.len() > 1 { + for start in 1..parts.len() { + let suffix = parts[start..].join("."); + if let Some(typ) = self.config.suffixes.get(&suffix) + && let Some(icon) = self.get_type_icon(typ) + { + return Some(icon); + } + } + } + + // 3) Generic fallback. + self.get_type_icon(TOOL_GENERIC) + } + /// Get tool-specific icon based on tool name pub fn get_tool_icon(&self, tool_name: &str) -> Option { // MCP server tools (`mcp____`) share one generic icon; diff --git a/crates/ui_gpui/src/shared/file_tree.rs b/crates/ui_gpui/src/shared/file_tree.rs new file mode 100644 index 00000000..5eb72c09 --- /dev/null +++ b/crates/ui_gpui/src/shared/file_tree.rs @@ -0,0 +1,429 @@ +//! Nested, collapsible tree of changed files for the Review panel. +//! +//! [`build_tree`] is a pure function (unit-tested) that turns a flat list of +//! [`git::ChangedFile`]s into a nested [`TreeNode`] structure. [`ChangedFilesTree`] +//! is the GPUI entity that renders it, tracks expansion/selection state, and +//! emits [`ChangedFilesTreeEvent::FileSelected`] when the user clicks a file. + +use crate::shared::file_icons; +use git::{ChangeStatus, ChangedFile}; +use gpui::{ + Context, EventEmitter, FocusHandle, Focusable, Render, Window, div, prelude::*, px, svg, +}; +use gpui_component::ActiveTheme; +use std::collections::HashSet; + +/// A node in the changed-files tree: either a directory (with children) or a +/// leaf file carrying its change status. +#[derive(Debug, Clone, PartialEq)] +pub enum TreeNode { + Dir { + /// Last path segment (display name). + name: String, + /// Full relative path from the repo root (unique key). + path: String, + children: Vec, + }, + File { + name: String, + path: String, + status: ChangeStatus, + }, +} + +/// Build a nested tree from a flat list of changed files. +/// +/// Paths are split on `/`; files sharing a directory prefix are merged under a +/// single directory node. Siblings are sorted directories-first, then +/// case-insensitively by name. +pub fn build_tree(files: &[ChangedFile]) -> Vec { + let mut roots: Vec = Vec::new(); + for f in files { + insert_path(&mut roots, &f.path, f.status, ""); + } + sort_nodes(&mut roots); + roots +} + +fn insert_path(nodes: &mut Vec, rel: &str, status: ChangeStatus, prefix: &str) { + let mut parts = rel.splitn(2, '/'); + let head = match parts.next() { + Some(h) if !h.is_empty() => h, + // Empty or leading-slash segment: skip it. + _ => return, + }; + let rest = parts.next(); + let full = if prefix.is_empty() { + head.to_string() + } else { + format!("{prefix}/{head}") + }; + + match rest { + None => { + nodes.push(TreeNode::File { + name: head.to_string(), + path: full, + status, + }); + } + Some(rest) => { + let idx = nodes + .iter() + .position(|n| matches!(n, TreeNode::Dir { name, .. } if name == head)); + let idx = match idx { + Some(i) => i, + None => { + nodes.push(TreeNode::Dir { + name: head.to_string(), + path: full.clone(), + children: Vec::new(), + }); + nodes.len() - 1 + } + }; + if let TreeNode::Dir { children, .. } = &mut nodes[idx] { + insert_path(children, rest, status, &full); + } + } + } +} + +fn node_name(node: &TreeNode) -> &str { + match node { + TreeNode::Dir { name, .. } | TreeNode::File { name, .. } => name, + } +} + +fn sort_nodes(nodes: &mut [TreeNode]) { + nodes.sort_by(|a, b| { + // Directories sort before files. + let rank = |n: &TreeNode| matches!(n, TreeNode::File { .. }) as u8; + rank(a) + .cmp(&rank(b)) + .then_with(|| node_name(a).to_lowercase().cmp(&node_name(b).to_lowercase())) + }); + for n in nodes.iter_mut() { + if let TreeNode::Dir { children, .. } = n { + sort_nodes(children); + } + } +} + +// --------------------------------------------------------------------------- +// Entity +// --------------------------------------------------------------------------- + +/// Events emitted by [`ChangedFilesTree`]. +#[derive(Clone, Debug)] +pub enum ChangedFilesTreeEvent { + /// The user selected a file. Carries the file's repo-relative path. + FileSelected(String), +} + +/// A flattened, renderable row (computed each render from the tree + expansion). +struct Row { + depth: usize, + is_dir: bool, + name: String, + path: String, + status: Option, + expanded: bool, +} + +/// Nested collapsible tree of changed files. +pub struct ChangedFilesTree { + nodes: Vec, + /// Full paths of directories that are currently expanded. + expanded: HashSet, + /// Currently selected file path. + selected: Option, + focus_handle: FocusHandle, +} + +impl EventEmitter for ChangedFilesTree {} + +impl ChangedFilesTree { + pub fn new(cx: &mut Context) -> Self { + Self { + nodes: Vec::new(), + expanded: HashSet::new(), + selected: None, + focus_handle: cx.focus_handle(), + } + } + + /// Replace the file list, rebuilding the tree. All directories start + /// expanded. Preserves the current selection if it still exists. + pub fn set_files(&mut self, files: &[ChangedFile], cx: &mut Context) { + self.nodes = build_tree(files); + self.expanded.clear(); + collect_dir_paths(&self.nodes, &mut self.expanded); + // Drop selection if the selected file is gone. + if let Some(sel) = &self.selected + && !files.iter().any(|f| &f.path == sel) + { + self.selected = None; + } + cx.notify(); + } + + /// Set the selected file path (without emitting an event). + pub fn set_selected(&mut self, path: Option, cx: &mut Context) { + self.selected = path; + cx.notify(); + } + + /// The currently selected file path, if any. + pub fn selected(&self) -> Option<&str> { + self.selected.as_deref() + } + + fn toggle_dir(&mut self, path: &str, cx: &mut Context) { + if !self.expanded.remove(path) { + self.expanded.insert(path.to_string()); + } + cx.notify(); + } + + fn on_file_click(&mut self, path: String, cx: &mut Context) { + self.selected = Some(path.clone()); + cx.notify(); + cx.emit(ChangedFilesTreeEvent::FileSelected(path)); + } + + /// Walk the tree honoring expansion state, producing a flat list of rows. + fn visible_rows(&self) -> Vec { + let mut rows = Vec::new(); + self.push_rows(&self.nodes, 0, &mut rows); + rows + } + + fn push_rows(&self, nodes: &[TreeNode], depth: usize, out: &mut Vec) { + for node in nodes { + match node { + TreeNode::Dir { + name, + path, + children, + } => { + let expanded = self.expanded.contains(path); + out.push(Row { + depth, + is_dir: true, + name: name.clone(), + path: path.clone(), + status: None, + expanded, + }); + if expanded { + self.push_rows(children, depth + 1, out); + } + } + TreeNode::File { name, path, status } => { + out.push(Row { + depth, + is_dir: false, + name: name.clone(), + path: path.clone(), + status: Some(*status), + expanded: false, + }); + } + } + } + } +} + +/// Recursively collect the full paths of all directory nodes. +fn collect_dir_paths(nodes: &[TreeNode], out: &mut HashSet) { + for n in nodes { + if let TreeNode::Dir { path, children, .. } = n { + out.insert(path.clone()); + collect_dir_paths(children, out); + } + } +} + +/// Single-letter badge and color for a change status. +fn status_badge(status: ChangeStatus) -> (&'static str, gpui::Hsla) { + match status { + ChangeStatus::Added | ChangeStatus::Untracked => ("A", gpui::rgb(0x3f_a5_5a).into()), + ChangeStatus::Modified => ("M", gpui::rgb(0xc7_9a_3a).into()), + ChangeStatus::Deleted => ("D", gpui::rgb(0xc7_4a_4a).into()), + ChangeStatus::Renamed => ("R", gpui::rgb(0x4a_82_c7).into()), + ChangeStatus::Copied => ("C", gpui::rgb(0x4a_82_c7).into()), + ChangeStatus::TypeChanged => ("T", gpui::rgb(0x8a_6a_c7).into()), + } +} + +impl Focusable for ChangedFilesTree { + fn focus_handle(&self, _cx: &gpui::App) -> FocusHandle { + self.focus_handle.clone() + } +} + +impl Render for ChangedFilesTree { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + let rows = self.visible_rows(); + let muted = cx.theme().muted_foreground; + let fg = cx.theme().foreground; + let accent = cx.theme().accent; + let selected = self.selected.clone(); + + if rows.is_empty() { + return div() + .p_3() + .text_sm() + .text_color(muted) + .child("No changes") + .into_any_element(); + } + + div() + .flex() + .flex_col() + .children(rows.into_iter().map(|row| { + let indent = px(8.0 + row.depth as f32 * 12.0); + let is_selected = !row.is_dir && selected.as_deref() == Some(row.path.as_str()); + let row_path = row.path.clone(); + + let mut container = div() + .id(gpui::SharedString::from(format!("tree-{}", row.path))) + .flex() + .flex_row() + .items_center() + .gap_1p5() + .pl(indent) + .pr_2() + .py_0p5() + .text_sm() + .cursor_pointer() + .hover(|s| s.bg(cx.theme().muted)); + + if is_selected { + container = container.bg(accent); + } + + // Leading glyph: chevron + folder for dirs, spacer + file icon for files. + if row.is_dir { + let chevron = if row.expanded { + "icons/chevron_down.svg" + } else { + "icons/chevron_right.svg" + }; + let folder = if row.expanded { + "icons/file_icons/folder_open.svg" + } else { + "icons/file_icons/folder.svg" + }; + container = container + .child(svg().size(px(12.)).path(chevron).text_color(muted)) + .child(svg().size(px(14.)).path(folder).text_color(muted)) + .child(div().text_color(fg).child(row.name.clone())); + } else { + let (badge, badge_color) = + row.status.map(status_badge).unwrap_or((" ", muted)); + let icon = file_icons::get().get_icon_for_filename(&row.name); + container = container + // Align file rows under the folder glyph (skip chevron slot). + .child(div().size(px(12.))) + .child(file_icons::render_icon(&icon, 14.0, muted, "📄")) + .child(div().flex_1().text_color(fg).child(row.name.clone())) + .child( + div() + .w(px(14.)) + .flex() + .justify_center() + .text_color(badge_color) + .child(badge), + ); + } + + let is_dir = row.is_dir; + container.on_click(cx.listener(move |this, _ev, _window, cx| { + if is_dir { + this.toggle_dir(&row_path, cx); + } else { + this.on_file_click(row_path.clone(), cx); + } + })) + })) + .into_any_element() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn file(path: &str, status: ChangeStatus) -> ChangedFile { + ChangedFile { + path: path.to_string(), + orig_path: None, + status, + } + } + + #[test] + fn build_tree_nests_and_sorts_dirs_first() { + let files = vec![ + file("z.rs", ChangeStatus::Modified), + file("a/c.rs", ChangeStatus::Added), + file("a/b.rs", ChangeStatus::Modified), + file("a/sub/d.rs", ChangeStatus::Deleted), + ]; + let tree = build_tree(&files); + + // Root: dir "a" first, then file "z.rs". + assert_eq!(tree.len(), 2); + match &tree[0] { + TreeNode::Dir { name, path, children } => { + assert_eq!(name, "a"); + assert_eq!(path, "a"); + // Inside "a": dir "sub" first, then files b.rs, c.rs (sorted). + assert_eq!(children.len(), 3); + assert!(matches!(&children[0], TreeNode::Dir { name, .. } if name == "sub")); + assert!(matches!(&children[1], TreeNode::File { name, .. } if name == "b.rs")); + assert!(matches!(&children[2], TreeNode::File { name, .. } if name == "c.rs")); + // Nested file path is fully qualified. + if let TreeNode::Dir { children: sub, .. } = &children[0] { + assert!(matches!(&sub[0], TreeNode::File { path, .. } if path == "a/sub/d.rs")); + } + } + other => panic!("expected dir 'a', got {other:?}"), + } + assert!(matches!(&tree[1], TreeNode::File { name, .. } if name == "z.rs")); + } + + #[test] + fn build_tree_merges_shared_prefix() { + let files = vec![ + file("src/main.rs", ChangeStatus::Modified), + file("src/lib.rs", ChangeStatus::Modified), + ]; + let tree = build_tree(&files); + assert_eq!(tree.len(), 1); + match &tree[0] { + TreeNode::Dir { name, children, .. } => { + assert_eq!(name, "src"); + assert_eq!(children.len(), 2); + } + other => panic!("expected single 'src' dir, got {other:?}"), + } + } + + #[test] + fn collect_dir_paths_gathers_all_dirs() { + let files = vec![ + file("a/b/c.rs", ChangeStatus::Modified), + file("d.rs", ChangeStatus::Added), + ]; + let tree = build_tree(&files); + let mut dirs = HashSet::new(); + collect_dir_paths(&tree, &mut dirs); + assert!(dirs.contains("a")); + assert!(dirs.contains("a/b")); + assert_eq!(dirs.len(), 2); + } +} diff --git a/crates/ui_gpui/src/shared/mod.rs b/crates/ui_gpui/src/shared/mod.rs index 7323b984..851b95fb 100644 --- a/crates/ui_gpui/src/shared/mod.rs +++ b/crates/ui_gpui/src/shared/mod.rs @@ -3,6 +3,7 @@ pub mod auto_scroll; pub mod context_breakdown; pub mod context_indicator; pub mod file_icons; +pub mod file_tree; pub mod image; pub mod plan_banner; pub mod settings; diff --git a/crates/ui_gpui/src/shared/settings.rs b/crates/ui_gpui/src/shared/settings.rs index 7c1fc69c..a6fb6089 100644 --- a/crates/ui_gpui/src/shared/settings.rs +++ b/crates/ui_gpui/src/shared/settings.rs @@ -72,6 +72,19 @@ pub struct UiSettings { /// Used when no `--model` CLI argument is given. #[serde(default, skip_serializing_if = "Option::is_none")] pub default_model: Option, + + /// Persisted width (px) of the right Review sidebar. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub right_sidebar_width: Option, + + /// Persisted width (px) of the Review panel's file-tree column. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub review_tree_width: Option, + + /// Persisted default base ref for "Branch vs base" review mode; seeds each + /// repo's per-session base override. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub review_default_base: Option, } fn default_theme_mode() -> ThemeModeSetting { @@ -89,6 +102,9 @@ impl Default for UiSettings { ui_scale: default_ui_scale(), window_bounds: None, default_model: None, + right_sidebar_width: None, + review_tree_width: None, + review_default_base: None, } } } diff --git a/crates/ui_gpui/src/shared/ui_state.rs b/crates/ui_gpui/src/shared/ui_state.rs index bea3d817..73128050 100644 --- a/crates/ui_gpui/src/shared/ui_state.rs +++ b/crates/ui_gpui/src/shared/ui_state.rs @@ -72,6 +72,22 @@ pub struct UiSessionState { /// exactly as it was left across app restarts. #[serde(default, skip_serializing_if = "Option::is_none")] pub scroll: Option, + + /// Whether the right (review) sidebar is open for this session. + #[serde(default)] + pub right_panel_open: bool, + + /// Which view the right panel last showed (e.g. "review"). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub right_panel_view: Option, + + /// Last review compare mode ("working_tree" or "branch_vs_base"). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub review_compare_mode: Option, + + /// Last review base branch (only meaningful in branch-vs-base mode). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub review_base_branch: Option, } // --------------------------------------------------------------------------- @@ -194,9 +210,49 @@ impl UiStateStore { true } + /// Return whether the right (review) sidebar is open for a session. + /// Loads from disk if the session hasn't been loaded yet. + pub fn get_right_panel_open(&mut self, session_id: &str) -> bool { + self.get(session_id).right_panel_open + } + + /// Set whether the right (review) sidebar is open for a session. + pub fn set_right_panel_open(&mut self, session_id: &str, open: bool) { + let state = self.states.entry(session_id.to_owned()).or_default(); + if state.right_panel_open == open { + return; + } + state.right_panel_open = open; + self.dirty.insert(session_id.to_owned()); + } + + /// Return the persisted review compare mode / base branch for a session. + pub fn get_review_settings( + &mut self, + session_id: &str, + ) -> (Option, Option) { + let state = self.get(session_id); + (state.review_compare_mode, state.review_base_branch) + } + + /// Persist the review compare mode / base branch for a session. + pub fn set_review_settings( + &mut self, + session_id: &str, + mode: Option, + base: Option, + ) { + let state = self.states.entry(session_id.to_owned()).or_default(); + if state.review_compare_mode == mode && state.review_base_branch == base { + return; + } + state.review_compare_mode = mode; + state.review_base_branch = base; + self.dirty.insert(session_id.to_owned()); + } + /// Remove the in-memory state and on-disk file for a deleted session. - pub fn remove_session(&mut self, session_id: &str) { - self.states.remove(session_id); + pub fn remove_session(&mut self, session_id: &str) { self.states.remove(session_id); self.dirty.remove(session_id); let path = self.file_path(session_id); if path.exists() @@ -425,6 +481,7 @@ mod tests { tool_collapse_overrides: HashMap::from([("t1".to_owned(), false)]), tool_diff_mode_overrides: HashMap::new(), scroll: None, + ..Default::default() }; let path = dir.path().join("s1.ui_state.json"); let mut f = std::fs::File::create(&path).unwrap(); diff --git a/crates/ui_gpui/src/tool_cards/diff_card.rs b/crates/ui_gpui/src/tool_cards/diff_card.rs index 4ca4d36a..07a0409f 100644 --- a/crates/ui_gpui/src/tool_cards/diff_card.rs +++ b/crates/ui_gpui/src/tool_cards/diff_card.rs @@ -537,7 +537,7 @@ fn normalize_for_diff(text: &str) -> String { format!("{trimmed}\n") } -fn render_unified_diff( +pub(crate) fn render_unified_diff( old_text: &str, new_text: &str, theme: &gpui_component::theme::Theme, @@ -878,7 +878,9 @@ fn rgba_color(r: u8, g: u8, b: u8, a: u8) -> gpui::Hsla { .into() } -fn deleted_row_colors(theme: &gpui_component::theme::Theme) -> (Option, gpui::Hsla) { +pub(crate) fn deleted_row_colors( + theme: &gpui_component::theme::Theme, +) -> (Option, gpui::Hsla) { if theme.is_dark() { ( Some(rgba_color(0x80, 0x20, 0x20, 0x60)), @@ -892,7 +894,9 @@ fn deleted_row_colors(theme: &gpui_component::theme::Theme) -> (Option (Option, gpui::Hsla) { +pub(crate) fn added_row_colors( + theme: &gpui_component::theme::Theme, +) -> (Option, gpui::Hsla) { if theme.is_dark() { ( Some(rgba_color(0x20, 0x60, 0x20, 0x60)), @@ -906,7 +910,9 @@ fn added_row_colors(theme: &gpui_component::theme::Theme) -> (Option } } -fn unchanged_row_colors(theme: &gpui_component::theme::Theme) -> (Option, gpui::Hsla) { +pub(crate) fn unchanged_row_colors( + theme: &gpui_component::theme::Theme, +) -> (Option, gpui::Hsla) { if theme.is_dark() { (None, rgba_color(0xFF, 0xFF, 0xFF, 0x99)) } else { From 332e01703d933649d0dc59da6ec1ab33b3cf2383 Mon Sep 17 00:00:00 2001 From: Daniel Kurzynski Date: Mon, 31 Aug 2026 16:19:39 +0200 Subject: [PATCH 02/10] style: apply cargo fmt --- .../src/session/service.rs | 8 +++-- crates/git/src/diff.rs | 5 +-- crates/ui_gpui/src/main_screen/mod.rs | 31 ++++++++++--------- .../src/main_screen/right_panel/mod.rs | 3 +- .../main_screen/right_panel/review_view.rs | 23 ++++++-------- crates/ui_gpui/src/shared/file_tree.rs | 17 ++++++---- crates/ui_gpui/src/shared/ui_state.rs | 8 ++--- 7 files changed, 47 insertions(+), 48 deletions(-) diff --git a/crates/code_assistant_core/src/session/service.rs b/crates/code_assistant_core/src/session/service.rs index 738d2b96..6338a52f 100644 --- a/crates/code_assistant_core/src/session/service.rs +++ b/crates/code_assistant_core/src/session/service.rs @@ -1098,8 +1098,7 @@ impl SessionService { (files, None) } ReviewMode::BranchVsBase => { - let resolved = - resolve_review_base(&repo, override_base, &base_candidates); + let resolved = resolve_review_base(&repo, override_base, &base_candidates); match &resolved { Some(b) => { let files = repo @@ -1464,7 +1463,10 @@ mod discover_tests { git_init(tmp.path()); let repos = discover_review_repos(tmp.path()); assert_eq!(repos.len(), 1); - assert_eq!(repos[0].0.canonicalize().unwrap(), tmp.path().canonicalize().unwrap()); + assert_eq!( + repos[0].0.canonicalize().unwrap(), + tmp.path().canonicalize().unwrap() + ); } #[test] diff --git a/crates/git/src/diff.rs b/crates/git/src/diff.rs index 2825bfbd..4e963a4b 100644 --- a/crates/git/src/diff.rs +++ b/crates/git/src/diff.rs @@ -548,10 +548,7 @@ mod tests { let files = repo.changed_files_vs_base(&base_branch).await.unwrap(); assert_eq!(find(&files, "shared.txt").status, ChangeStatus::Modified); - assert_eq!( - find(&files, "feature_only.txt").status, - ChangeStatus::Added - ); + assert_eq!(find(&files, "feature_only.txt").status, ChangeStatus::Added); let d = repo .file_diff_vs_base(&base_branch, find(&files, "shared.txt")) diff --git a/crates/ui_gpui/src/main_screen/mod.rs b/crates/ui_gpui/src/main_screen/mod.rs index ee56aa05..4546ed1d 100644 --- a/crates/ui_gpui/src/main_screen/mod.rs +++ b/crates/ui_gpui/src/main_screen/mod.rs @@ -1857,14 +1857,13 @@ impl Render for MainScreen { }; let sidebar_width = self.right_sidebar_width * right_sidebar_scale; let resizing = self.right_sidebar_resizing; - let handle_mouse_down = cx.listener( - |this, ev: &gpui::MouseDownEvent, _window, cx| { + let handle_mouse_down = + cx.listener(|this, ev: &gpui::MouseDownEvent, _window, cx| { this.right_sidebar_resizing = true; this.resize_start_x = f32::from(ev.position.x); this.resize_start_width = f32::from(this.right_sidebar_width); cx.notify(); - }, - ); + }); move |el| { el.child( div() @@ -1910,17 +1909,19 @@ impl Render for MainScreen { .absolute() .inset_0() .cursor_col_resize() - .on_mouse_move(cx.listener(|this, ev: &gpui::MouseMoveEvent, _window, cx| { - if !this.right_sidebar_resizing { - return; - } - // Dragging the left edge leftward widens the sidebar. - let delta = f32::from(ev.position.x) - this.resize_start_x; - let new_width = - (this.resize_start_width - delta).clamp(320.0, 1800.0); - this.right_sidebar_width = px(new_width); - cx.notify(); - })) + .on_mouse_move(cx.listener( + |this, ev: &gpui::MouseMoveEvent, _window, cx| { + if !this.right_sidebar_resizing { + return; + } + // Dragging the left edge leftward widens the sidebar. + let delta = f32::from(ev.position.x) - this.resize_start_x; + let new_width = + (this.resize_start_width - delta).clamp(320.0, 1800.0); + this.right_sidebar_width = px(new_width); + cx.notify(); + }, + )) .on_mouse_up( gpui::MouseButton::Left, cx.listener(|this, _ev: &gpui::MouseUpEvent, _window, cx| { diff --git a/crates/ui_gpui/src/main_screen/right_panel/mod.rs b/crates/ui_gpui/src/main_screen/right_panel/mod.rs index 099f258c..e4a699ee 100644 --- a/crates/ui_gpui/src/main_screen/right_panel/mod.rs +++ b/crates/ui_gpui/src/main_screen/right_panel/mod.rs @@ -63,7 +63,8 @@ impl RightPanel { /// Point the active view(s) at a session. pub fn set_session(&mut self, session_id: Option, cx: &mut Context) { - self.review_view.update(cx, |v, cx| v.set_session(session_id, cx)); + self.review_view + .update(cx, |v, cx| v.set_session(session_id, cx)); } /// Re-request data for the active view. diff --git a/crates/ui_gpui/src/main_screen/right_panel/review_view.rs b/crates/ui_gpui/src/main_screen/right_panel/review_view.rs index 5f3b62c6..4d83ae77 100644 --- a/crates/ui_gpui/src/main_screen/right_panel/review_view.rs +++ b/crates/ui_gpui/src/main_screen/right_panel/review_view.rs @@ -332,8 +332,7 @@ impl ReviewView { // existing sections in place (preserving expansion / selection). let incoming_roots: Vec = listing.repos.iter().map(|r| r.repo_root.clone()).collect(); - let current_roots: Vec = - self.repos.iter().map(|r| r.repo_root.clone()).collect(); + let current_roots: Vec = self.repos.iter().map(|r| r.repo_root.clone()).collect(); if incoming_roots != current_roots { self.repos = listing @@ -401,14 +400,10 @@ impl ReviewView { tree.update(cx, |t, cx| t.set_files(&data.files, cx)); let root_for_tree = data.repo_root.clone(); - let tree_sub = cx.subscribe_in( - &tree, - window, - move |this, _tree, event, _window, cx| { - let ChangedFilesTreeEvent::FileSelected(path) = event; - this.on_file_selected(root_for_tree.clone(), path.clone(), cx); - }, - ); + let tree_sub = cx.subscribe_in(&tree, window, move |this, _tree, event, _window, cx| { + let ChangedFilesTreeEvent::FileSelected(path) = event; + this.on_file_selected(root_for_tree.clone(), path.clone(), cx); + }); let items: Vec = data .base_candidates @@ -456,7 +451,9 @@ impl ReviewView { cx: &mut Context, ) { section.label = data.label.clone(); - section.tree.update(cx, |t, cx| t.set_files(&data.files, cx)); + section + .tree + .update(cx, |t, cx| t.set_files(&data.files, cx)); if section.base_candidates != data.base_candidates { section.base_candidates = data.base_candidates.clone(); @@ -612,9 +609,7 @@ impl ReviewView { .child(section.label.clone()), ) .on_click(cx.listener(move |this, _ev, _window, cx| { - if let Some(s) = - this.repos.iter_mut().find(|s| s.repo_root == repo_root) - { + if let Some(s) = this.repos.iter_mut().find(|s| s.repo_root == repo_root) { s.collapsed = !s.collapsed; cx.notify(); } diff --git a/crates/ui_gpui/src/shared/file_tree.rs b/crates/ui_gpui/src/shared/file_tree.rs index 5eb72c09..e885ba3b 100644 --- a/crates/ui_gpui/src/shared/file_tree.rs +++ b/crates/ui_gpui/src/shared/file_tree.rs @@ -99,9 +99,11 @@ fn sort_nodes(nodes: &mut [TreeNode]) { nodes.sort_by(|a, b| { // Directories sort before files. let rank = |n: &TreeNode| matches!(n, TreeNode::File { .. }) as u8; - rank(a) - .cmp(&rank(b)) - .then_with(|| node_name(a).to_lowercase().cmp(&node_name(b).to_lowercase())) + rank(a).cmp(&rank(b)).then_with(|| { + node_name(a) + .to_lowercase() + .cmp(&node_name(b).to_lowercase()) + }) }); for n in nodes.iter_mut() { if let TreeNode::Dir { children, .. } = n { @@ -322,8 +324,7 @@ impl Render for ChangedFilesTree { .child(svg().size(px(14.)).path(folder).text_color(muted)) .child(div().text_color(fg).child(row.name.clone())); } else { - let (badge, badge_color) = - row.status.map(status_badge).unwrap_or((" ", muted)); + let (badge, badge_color) = row.status.map(status_badge).unwrap_or((" ", muted)); let icon = file_icons::get().get_icon_for_filename(&row.name); container = container // Align file rows under the folder glyph (skip chevron slot). @@ -378,7 +379,11 @@ mod tests { // Root: dir "a" first, then file "z.rs". assert_eq!(tree.len(), 2); match &tree[0] { - TreeNode::Dir { name, path, children } => { + TreeNode::Dir { + name, + path, + children, + } => { assert_eq!(name, "a"); assert_eq!(path, "a"); // Inside "a": dir "sub" first, then files b.rs, c.rs (sorted). diff --git a/crates/ui_gpui/src/shared/ui_state.rs b/crates/ui_gpui/src/shared/ui_state.rs index 73128050..243f953f 100644 --- a/crates/ui_gpui/src/shared/ui_state.rs +++ b/crates/ui_gpui/src/shared/ui_state.rs @@ -227,10 +227,7 @@ impl UiStateStore { } /// Return the persisted review compare mode / base branch for a session. - pub fn get_review_settings( - &mut self, - session_id: &str, - ) -> (Option, Option) { + pub fn get_review_settings(&mut self, session_id: &str) -> (Option, Option) { let state = self.get(session_id); (state.review_compare_mode, state.review_base_branch) } @@ -252,7 +249,8 @@ impl UiStateStore { } /// Remove the in-memory state and on-disk file for a deleted session. - pub fn remove_session(&mut self, session_id: &str) { self.states.remove(session_id); + pub fn remove_session(&mut self, session_id: &str) { + self.states.remove(session_id); self.dirty.remove(session_id); let path = self.file_path(session_id); if path.exists() From a312891b2af691bf77762e731f6f5f641224b2a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Tue, 1 Sep 2026 11:18:53 +0200 Subject: [PATCH 03/10] fix(ui_gpui): debounce UI-settings writes and flush persisted state on quit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit update_ui_settings previously cloned the settings at call time and spawned a detached background write per call. Two in-flight writes to ui-settings.json could complete out of order, letting an older snapshot clobber a newer one (hot path: per-event window-bounds saves during a window drag). The write is now debounced like the UiStateStore flush, and the snapshot is taken when the timer fires, so the last write always carries the latest state. An on_app_quit hook flushes both the pending settings write and any dirty per-session UI state, closing the debounce window on exit (previously the UiStateStore could silently drop up to 500ms of changes on quit). Also drop UiSessionState::review_base_branch, which was only ever written as None and never read — the base preference is persisted globally via review_default_base — and narrow the accessors to the compare mode. --- crates/ui_gpui/src/lib.rs | 53 ++++++++++++++++--- .../main_screen/right_panel/review_view.rs | 4 +- crates/ui_gpui/src/shared/ui_state.rs | 43 ++++++++------- 3 files changed, 73 insertions(+), 27 deletions(-) diff --git a/crates/ui_gpui/src/lib.rs b/crates/ui_gpui/src/lib.rs index 92e6a424..9dd2da53 100644 --- a/crates/ui_gpui/src/lib.rs +++ b/crates/ui_gpui/src/lib.rs @@ -46,20 +46,40 @@ pub struct UiSettingsGlobal(pub shared::settings::UiSettings); impl Global for UiSettingsGlobal {} -/// Mutate the global [`UiSettings`] and persist to disk on a background thread. +/// Delay after the last [`update_ui_settings`] call before writing to disk. +const UI_SETTINGS_SAVE_DEBOUNCE: std::time::Duration = std::time::Duration::from_millis(500); + +/// Pending debounced settings-save task. Replacing it drops (and thereby +/// cancels) the previous timer, so rapid updates coalesce into a single write. +struct UiSettingsSaveTask(#[allow(dead_code)] gpui::Task<()>); + +impl Global for UiSettingsSaveTask {} + +/// Mutate the global [`UiSettings`] and persist to disk, debounced. +/// +/// The disk snapshot is taken when the debounce timer fires — not at call time +/// — so concurrent writes cannot land out of order and clobber a newer state. +/// A quit within the debounce window is covered by the `on_app_quit` flush. /// /// Callable anywhere with access to `&mut App` (including via `Context` deref), /// so both `MainScreen` handlers and `ReviewView` resize callbacks can use it. pub fn update_ui_settings(cx: &mut App, f: impl FnOnce(&mut shared::settings::UiSettings)) { - if cx.has_global::() { - let global = cx.global_mut::(); - f(&mut global.0); - let settings = global.0.clone(); + if !cx.has_global::() { + return; + } + f(&mut cx.global_mut::().0); + + let task = cx.spawn(async move |cx: &mut AsyncApp| { + cx.background_executor() + .timer(UI_SETTINGS_SAVE_DEBOUNCE) + .await; + let settings = cx.update(|cx| cx.global::().0.clone()); cx.background_spawn(async move { settings.save(); }) - .detach(); - } + .await; + }); + cx.set_global(UiSettingsSaveTask(task)); } /// Snapshot of worktree/branch data for the active session, kept in `Gpui` @@ -573,6 +593,25 @@ impl Gpui { // Store settings as a GPUI global so entities can access/update them cx.set_global(UiSettingsGlobal(ui_settings.clone())); + // Flush pending debounced writes (UI settings + per-session UI + // state) before the process exits, closing the debounce window. + cx.on_app_quit(|cx| { + let settings = cx + .try_global::() + .map(|global| global.0.clone()); + let files = shared::ui_state::UiStateStore::try_global() + .and_then(|store| store.lock().ok()) + .map(|mut store| store.take_dirty()) + .unwrap_or_default(); + async move { + if let Some(settings) = settings { + settings.save(); + } + shared::ui_state::write_ui_state_files(files); + } + }) + .detach(); + init(cx); // Spawn task to receive UiEvents diff --git a/crates/ui_gpui/src/main_screen/right_panel/review_view.rs b/crates/ui_gpui/src/main_screen/right_panel/review_view.rs index 4d83ae77..78e99dfe 100644 --- a/crates/ui_gpui/src/main_screen/right_panel/review_view.rs +++ b/crates/ui_gpui/src/main_screen/right_panel/review_view.rs @@ -197,7 +197,7 @@ impl ReviewView { if let Some(store) = crate::shared::ui_state::UiStateStore::try_global() && let Ok(mut store) = store.lock() { - let (mode, _base) = store.get_review_settings(id); + let mode = store.get_review_compare_mode(id); self.mode = match mode.as_deref() { Some("branch_vs_base") => ReviewMode::BranchVsBase, _ => ReviewMode::WorkingTree, @@ -220,7 +220,7 @@ impl ReviewView { ReviewMode::BranchVsBase => "branch_vs_base", }; if let Ok(mut store) = crate::shared::ui_state::UiStateStore::global().lock() { - store.set_review_settings(session_id, Some(mode.to_string()), None); + store.set_review_compare_mode(session_id, mode.to_string()); } if let Some(sender) = cx.try_global::() { let _ = sender diff --git a/crates/ui_gpui/src/shared/ui_state.rs b/crates/ui_gpui/src/shared/ui_state.rs index 243f953f..18eb6cb5 100644 --- a/crates/ui_gpui/src/shared/ui_state.rs +++ b/crates/ui_gpui/src/shared/ui_state.rs @@ -84,10 +84,6 @@ pub struct UiSessionState { /// Last review compare mode ("working_tree" or "branch_vs_base"). #[serde(default, skip_serializing_if = "Option::is_none")] pub review_compare_mode: Option, - - /// Last review base branch (only meaningful in branch-vs-base mode). - #[serde(default, skip_serializing_if = "Option::is_none")] - pub review_base_branch: Option, } // --------------------------------------------------------------------------- @@ -226,25 +222,18 @@ impl UiStateStore { self.dirty.insert(session_id.to_owned()); } - /// Return the persisted review compare mode / base branch for a session. - pub fn get_review_settings(&mut self, session_id: &str) -> (Option, Option) { - let state = self.get(session_id); - (state.review_compare_mode, state.review_base_branch) + /// Return the persisted review compare mode for a session. + pub fn get_review_compare_mode(&mut self, session_id: &str) -> Option { + self.get(session_id).review_compare_mode } - /// Persist the review compare mode / base branch for a session. - pub fn set_review_settings( - &mut self, - session_id: &str, - mode: Option, - base: Option, - ) { + /// Persist the review compare mode for a session. + pub fn set_review_compare_mode(&mut self, session_id: &str, mode: String) { let state = self.states.entry(session_id.to_owned()).or_default(); - if state.review_compare_mode == mode && state.review_base_branch == base { + if state.review_compare_mode.as_deref() == Some(mode.as_str()) { return; } - state.review_compare_mode = mode; - state.review_base_branch = base; + state.review_compare_mode = Some(mode); self.dirty.insert(session_id.to_owned()); } @@ -470,6 +459,24 @@ mod tests { assert_eq!(parsed.tool_collapse_overrides.get("t1"), Some(&true)); } + #[test] + fn test_set_review_compare_mode_roundtrip_and_dirty() { + let (mut store, _dir) = test_store(); + assert_eq!(store.get_review_compare_mode("s1"), None); + + store.set_review_compare_mode("s1", "branch_vs_base".to_owned()); + assert!(store.dirty.contains("s1")); + assert_eq!( + store.get_review_compare_mode("s1").as_deref(), + Some("branch_vs_base") + ); + + // Setting the identical value must not re-dirty the session. + store.dirty.clear(); + store.set_review_compare_mode("s1", "branch_vs_base".to_owned()); + assert!(!store.dirty.contains("s1")); + } + #[test] fn test_load_from_disk() { let (mut store, dir) = test_store(); From 81873646ca6ffedee29623c20a9dbb101b45292b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Tue, 1 Sep 2026 11:29:21 +0200 Subject: [PATCH 04/10] perf(ui_gpui): make ReviewView renders cheap at animation frame rates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ReviewView renders on every MainScreen notify — ~120fps during sidebar animations and resize drags, plus every cx.refresh() from the event loop. Each render deep-cloned the changed-files listing out of its mutex global and deep-compared it for change detection, cloned the selected file's FileDiffContent (up to ~3MB of text, twice), and re-ran the full Myers line diff via render_unified_diff. That per-frame cost scales with diff size and threatens the frame budget. - The review globals on Gpui now carry a generation counter bumped on every write; the view's per-frame unchanged path is a mutex lock plus an integer compare (review_listing_if_newer / review_diff_if_newer). - The line diff is computed once when UpdateReviewDiff arrives (compute_diff_lines) and cached as PreparedDiff; render only builds elements from the cached lines (render_diff_lines, SharedString rows). render_unified_diff keeps its signature for the chat diff cards. --- crates/ui_gpui/src/app/event_loop.rs | 8 +- crates/ui_gpui/src/lib.rs | 38 +++--- .../main_screen/right_panel/review_view.rs | 110 ++++++++++++++---- crates/ui_gpui/src/tool_cards/diff_card.rs | 65 +++++++---- 4 files changed, 160 insertions(+), 61 deletions(-) diff --git a/crates/ui_gpui/src/app/event_loop.rs b/crates/ui_gpui/src/app/event_loop.rs index 73390522..41b14960 100644 --- a/crates/ui_gpui/src/app/event_loop.rs +++ b/crates/ui_gpui/src/app/event_loop.rs @@ -855,11 +855,11 @@ impl Gpui { files: r.files, }) .collect(); - *self.current_review_listing.lock().unwrap() = Some(ReviewData { + self.set_current_review_listing(Some(ReviewData { repos, is_git_repo, mode, - }); + })); cx.refresh(); } @@ -869,11 +869,11 @@ impl Gpui { diff, } => { debug!("UI: UpdateReviewDiff event — path={path}"); - *self.current_review_diff.lock().unwrap() = Some(ReviewDiff { + self.set_current_review_diff(Some(ReviewDiff { repo_root, path, diff, - }); + })); cx.refresh(); } diff --git a/crates/ui_gpui/src/lib.rs b/crates/ui_gpui/src/lib.rs index 9dd2da53..c97efa39 100644 --- a/crates/ui_gpui/src/lib.rs +++ b/crates/ui_gpui/src/lib.rs @@ -173,9 +173,11 @@ pub struct Gpui { current_worktree_data: Arc>>, // Review panel state (changed-files listing + selected-file diff) mirrored - // from the backend for the ReviewView to read during render. - current_review_listing: Arc>>, - current_review_diff: Arc>>, + // from the backend for the ReviewView to read during render. The u64 is a + // generation counter bumped on every write so the view can detect changes + // with a plain integer compare instead of deep-cloning per frame. + current_review_listing: Arc)>>, + current_review_diff: Arc)>>, // Last usage from the active session's most recent assistant message. // Stored separately from chat_sessions so it cannot be overwritten by @@ -404,8 +406,8 @@ impl Gpui { *self.current_permission_tier.lock().unwrap() = None; self.pending_permission_requests.lock().unwrap().clear(); *self.current_worktree_data.lock().unwrap() = None; - *self.current_review_listing.lock().unwrap() = None; - *self.current_review_diff.lock().unwrap() = None; + self.set_current_review_listing(None); + self.set_current_review_diff(None); *self.current_session_last_usage.lock().unwrap() = None; *self.current_session_total_usage.lock().unwrap() = None; } @@ -504,8 +506,8 @@ impl Gpui { // Current worktree state current_worktree_data: Arc::new(Mutex::new(None)), - current_review_listing: Arc::new(Mutex::new(None)), - current_review_diff: Arc::new(Mutex::new(None)), + current_review_listing: Arc::new(Mutex::new((0, None))), + current_review_diff: Arc::new(Mutex::new((0, None))), // Current session last usage current_session_last_usage: Arc::new(Mutex::new(None)), @@ -825,20 +827,30 @@ impl Gpui { self.current_worktree_data.lock().unwrap().clone() } - pub fn get_current_review_listing(&self) -> Option { - self.current_review_listing.lock().unwrap().clone() + /// Return the review listing and its generation, but only if it changed + /// since `seen`. The unchanged case — the per-frame hot path — costs a + /// mutex lock and an integer compare; the data is only cloned on change. + pub fn review_listing_if_newer(&self, seen: u64) -> Option<(u64, Option)> { + let slot = self.current_review_listing.lock().unwrap(); + (slot.0 != seen).then(|| (slot.0, slot.1.clone())) } pub fn set_current_review_listing(&self, data: Option) { - *self.current_review_listing.lock().unwrap() = data; + let mut slot = self.current_review_listing.lock().unwrap(); + slot.0 = slot.0.wrapping_add(1); + slot.1 = data; } - pub fn get_current_review_diff(&self) -> Option { - self.current_review_diff.lock().unwrap().clone() + /// Generation-gated access to the review diff; see [`Self::review_listing_if_newer`]. + pub fn review_diff_if_newer(&self, seen: u64) -> Option<(u64, Option)> { + let slot = self.current_review_diff.lock().unwrap(); + (slot.0 != seen).then(|| (slot.0, slot.1.clone())) } pub fn set_current_review_diff(&self, diff: Option) { - *self.current_review_diff.lock().unwrap() = diff; + let mut slot = self.current_review_diff.lock().unwrap(); + slot.0 = slot.0.wrapping_add(1); + slot.1 = diff; } pub fn get_current_session_last_usage(&self) -> Option { diff --git a/crates/ui_gpui/src/main_screen/right_panel/review_view.rs b/crates/ui_gpui/src/main_screen/right_panel/review_view.rs index 78e99dfe..a98af38b 100644 --- a/crates/ui_gpui/src/main_screen/right_panel/review_view.rs +++ b/crates/ui_gpui/src/main_screen/right_panel/review_view.rs @@ -5,11 +5,13 @@ //! draggable divider (`h_resizable`). //! //! Backend data arrives through the `current_review_listing` / `current_review_diff` -//! globals on [`Gpui`]; this view consumes them in `render` by diffing against -//! cached copies (the same "sync-in-render" technique the worktree selector uses). +//! globals on [`Gpui`]; this view consumes them in `render` (the "sync-in-render" +//! technique the worktree selector uses). Change detection is generation-based: +//! the per-frame unchanged case costs an integer compare, and the expensive +//! line diff is computed once per content change, never during render. use crate::shared::file_tree::{ChangedFilesTree, ChangedFilesTreeEvent}; -use crate::tool_cards::diff_card::render_unified_diff; +use crate::tool_cards::diff_card::{DiffLine, compute_diff_lines, render_diff_lines}; use crate::{Gpui, ReviewData}; use code_assistant_core::session::ReviewMode; use gpui::{ @@ -109,6 +111,20 @@ struct RepoSection { // ReviewView // --------------------------------------------------------------------------- +/// A diff prepared for per-frame rendering: the expensive line diff is +/// computed once when the backend data changes, never during render. +struct PreparedDiff { + repo_root: PathBuf, + path: String, + is_binary: bool, + too_large: bool, + lines: Vec, +} + +/// Sentinel for "never synced": guarantees the first generation compare +/// mismatches, whatever the global's current generation is. +const GENERATION_UNSEEN: u64 = u64::MAX; + pub struct ReviewView { session_id: Option, mode_state: Entity>>, @@ -133,8 +149,16 @@ pub struct ReviewView { /// Scroll position of the diff pane. diff_scroll: ScrollHandle, - /// Last listing consumed from the global (change detection). + /// Last listing consumed from the global; needed to resolve file lookups. last_listing: Option, + /// Generation of `last_listing`. Change detection per frame is a plain + /// integer compare against the global's generation — no clones. + listing_generation: u64, + + /// The selected file's diff, with its line diff computed once on arrival. + prepared_diff: Option, + /// Generation of `prepared_diff` (see `listing_generation`). + diff_generation: u64, focus_handle: FocusHandle, _mode_sub: Subscription, @@ -177,6 +201,9 @@ impl ReviewView { tree_width, diff_scroll: ScrollHandle::new(), last_listing: None, + listing_generation: GENERATION_UNSEEN, + prepared_diff: None, + diff_generation: GENERATION_UNSEEN, focus_handle: cx.focus_handle(), _mode_sub: mode_sub, } @@ -188,6 +215,9 @@ impl ReviewView { // Reset per-session state; fresh data will arrive via the global. self.selected = None; self.last_listing = None; + self.listing_generation = GENERATION_UNSEEN; + self.prepared_diff = None; + self.diff_generation = GENERATION_UNSEEN; self.repos.clear(); self.base_overrides.clear(); @@ -303,15 +333,16 @@ impl ReviewView { } } - /// Consume the latest listing from the global if it changed. + /// Consume the latest listing from the global if it changed. The per-frame + /// unchanged case is a generation compare — no clone, no deep equality. fn sync_listing(&mut self, window: &mut Window, cx: &mut Context) { - let listing = cx + let Some((generation, listing)) = cx .try_global::() - .and_then(|g| g.get_current_review_listing()); - - if listing == self.last_listing { + .and_then(|g| g.review_listing_if_newer(self.listing_generation)) + else { return; - } + }; + self.listing_generation = generation; self.last_listing = listing.clone(); let Some(listing) = listing else { @@ -488,6 +519,38 @@ impl ReviewView { data.base.clone() } + /// Consume the latest diff from the global if it changed, computing the + /// line diff exactly once. Rendering then reuses the prepared lines. + fn sync_diff(&mut self, cx: &mut Context) { + let Some((generation, diff)) = cx + .try_global::() + .and_then(|g| g.review_diff_if_newer(self.diff_generation)) + else { + return; + }; + self.diff_generation = generation; + self.prepared_diff = diff.map(|d| { + let lines = if d.diff.is_binary || d.diff.too_large { + Vec::new() + } else { + let old = d.diff.old_text.as_deref().unwrap_or_default(); + let new = d.diff.new_text.as_deref().unwrap_or_default(); + if old.is_empty() && new.is_empty() { + Vec::new() + } else { + compute_diff_lines(old, new) + } + }; + PreparedDiff { + repo_root: d.repo_root, + path: d.path, + is_binary: d.diff.is_binary, + too_large: d.diff.too_large, + lines, + } + }); + } + fn render_diff_pane(&self, window: &mut Window, cx: &mut Context) -> gpui::AnyElement { let muted = cx.theme().muted_foreground; @@ -504,28 +567,25 @@ impl ReviewView { .into_any_element() }; - let Some((sel_root, sel_path)) = self.selected.clone() else { + let Some((sel_root, sel_path)) = self.selected.as_ref() else { return placeholder("Select a file to view its diff"); }; - let diff = cx - .try_global::() - .and_then(|g| g.get_current_review_diff()); - - let Some(diff) = diff.filter(|d| d.repo_root == sel_root && d.path == sel_path) else { + let Some(prepared) = self + .prepared_diff + .as_ref() + .filter(|p| &p.repo_root == sel_root && &p.path == sel_path) + else { return placeholder("Loading diff…"); }; - if diff.diff.is_binary { + if prepared.is_binary { return placeholder("Binary file — no text diff"); } - if diff.diff.too_large { + if prepared.too_large { return placeholder("File too large to display"); } - - let old = diff.diff.old_text.clone().unwrap_or_default(); - let new = diff.diff.new_text.clone().unwrap_or_default(); - if old.is_empty() && new.is_empty() { + if prepared.lines.is_empty() { return placeholder("No changes to display"); } @@ -540,7 +600,7 @@ impl ReviewView { gpui::hsla(0.0, 0.0, 0.97, 1.0) }; let line_height_px = rems(1.25).to_pixels(rem_size).round(); - let diff = render_unified_diff(&old, &new, theme, Some(1), rem_size); + let diff = render_diff_lines(&prepared.lines, theme, Some(1), rem_size); div() .id("review-diff-scroll") @@ -652,8 +712,10 @@ impl Focusable for ReviewView { impl Render for ReviewView { fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { - // Pull fresh backend data before laying out. + // Pull fresh backend data before laying out. Both syncs are a cheap + // generation compare when nothing changed. self.sync_listing(window, cx); + self.sync_diff(cx); let muted = cx.theme().muted_foreground; let border = cx.theme().border; diff --git a/crates/ui_gpui/src/tool_cards/diff_card.rs b/crates/ui_gpui/src/tool_cards/diff_card.rs index 07a0409f..678c7590 100644 --- a/crates/ui_gpui/src/tool_cards/diff_card.rs +++ b/crates/ui_gpui/src/tool_cards/diff_card.rs @@ -537,13 +537,18 @@ fn normalize_for_diff(text: &str) -> String { format!("{trimmed}\n") } -pub(crate) fn render_unified_diff( - old_text: &str, - new_text: &str, - theme: &gpui_component::theme::Theme, - start_line: Option, - rem_size: gpui::Pixels, -) -> gpui::AnyElement { +/// One line of a computed unified diff. `text` is a [`SharedString`] so cached +/// diffs can be re-rendered every frame with cheap clones. +pub(crate) struct DiffLine { + pub tag: ChangeTag, + pub text: SharedString, +} + +/// Run the line diff (the expensive part: normalization + Myers diff + per-line +/// allocations). Callers that render on every frame — like the Review panel — +/// should call this once per content change, cache the result, and feed it to +/// [`render_diff_lines`] per frame. +pub(crate) fn compute_diff_lines(old_text: &str, new_text: &str) -> Vec { let old_norm = normalize_for_diff(old_text); let new_norm = normalize_for_diff(new_text); @@ -551,19 +556,39 @@ pub(crate) fn render_unified_diff( .newline_terminated(true) .diff_lines(&old_norm, &new_norm); - // Collect individual lines with their tags for line-number rendering - struct DiffLine { - tag: ChangeTag, - text: String, - } - let mut diff_lines: Vec = Vec::new(); - for change in diff.iter_all_changes() { - diff_lines.push(DiffLine { + diff.iter_all_changes() + .map(|change| DiffLine { tag: change.tag(), - text: change.value().trim_end().to_string(), - }); - } + text: change.value().trim_end().to_string().into(), + }) + .collect() +} +/// Compute and render a unified diff in one go. For per-frame rendering of +/// unchanged content, prefer caching [`compute_diff_lines`]'s result and +/// calling [`render_diff_lines`] instead. +pub(crate) fn render_unified_diff( + old_text: &str, + new_text: &str, + theme: &gpui_component::theme::Theme, + start_line: Option, + rem_size: gpui::Pixels, +) -> gpui::AnyElement { + render_diff_lines( + &compute_diff_lines(old_text, new_text), + theme, + start_line, + rem_size, + ) +} + +/// Build the element tree for already-computed diff lines. +pub(crate) fn render_diff_lines( + diff_lines: &[DiffLine], + theme: &gpui_component::theme::Theme, + start_line: Option, + rem_size: gpui::Pixels, +) -> gpui::AnyElement { // Compute the gutter width (number of digits) based on new-file line numbers let gutter_width = if let Some(start) = start_line { let new_count = diff_lines @@ -589,7 +614,7 @@ pub(crate) fn render_unified_diff( div() .flex() .flex_col() - .children(diff_lines.into_iter().map(|dl| { + .children(diff_lines.iter().map(|dl| { let (row_bg, text_color) = match dl.tag { ChangeTag::Equal => unchanged_row_colors(theme), ChangeTag::Delete => deleted_row_colors(theme), @@ -645,7 +670,7 @@ pub(crate) fn render_unified_diff( .when(start_line.is_none(), |d| d.px_3()) .when(start_line.is_some(), |d| d.pl_1().pr_3()) .text_color(text_color) - .child(dl.text), + .child(dl.text.clone()), ); row.into_any() From 8849779a8875867823fac27a5808223e89991753 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Tue, 1 Sep 2026 15:06:12 +0200 Subject: [PATCH 05/10] feat(ui_gpui): incremental review scanning with per-repo progress and disk cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opening the Review panel on a project with many root-level repos scanned everything in one blocking service call — the panel stayed empty until the last repo finished, with no indication anything was happening. - Discovery and scanning are now separate service calls: list_review_repos returns the repo stubs immediately, scan_review_repo handles one repo. The Gpui command layer streams a listing update after every state change, so the panel shows all repos at once — one Scanning (spinner), the rest Pending — and results fill in repo by repo. An epoch counter supersedes in-flight scans when a newer request starts. - Scan results are cached per (repo, mode) under /review-cache/; on open the last known files and stats show instantly (marked with a trailing wait indicator) while the background rescan refreshes them. - Each repo header shows a +adds/−dels summary from git diff --numstat (untracked files are listed but not line-counted). - Section separators now sit above each section instead of between a header and its content, the tree no longer claims 'No changes' before a repo has ever been scanned, and the panel shows a spinner before the first discovery response instead of rendering empty. --- crates/code_assistant_core/src/session/mod.rs | 2 +- .../src/session/service.rs | 166 +++++++++--------- crates/git/src/diff.rs | 88 ++++++++++ crates/ui_gpui/src/app/commands.rs | 80 +++++++-- crates/ui_gpui/src/app/event_loop.rs | 2 + crates/ui_gpui/src/lib.rs | 10 ++ .../main_screen/right_panel/review_view.rs | 166 +++++++++++++----- crates/ui_gpui/src/shared/mod.rs | 1 + crates/ui_gpui/src/shared/review_cache.rs | 142 +++++++++++++++ 9 files changed, 521 insertions(+), 136 deletions(-) create mode 100644 crates/ui_gpui/src/shared/review_cache.rs diff --git a/crates/code_assistant_core/src/session/mod.rs b/crates/code_assistant_core/src/session/mod.rs index 06ae6951..8f8655f3 100644 --- a/crates/code_assistant_core/src/session/mod.rs +++ b/crates/code_assistant_core/src/session/mod.rs @@ -24,7 +24,7 @@ pub mod watcher; pub use event_stream::{EventPayload, EventStream, SessionEvent, StreamError, Subscription}; pub use manager::SessionManager; pub use service::SessionService; -pub use service::{RepoReview, ReviewListing, ReviewMode, WorktreeListing}; +pub use service::{RepoReview, ReviewMode, ReviewScanState, WorktreeListing}; pub use turn::{ ResourceRef, ToolRecord, TurnDispatch, TurnHandle, TurnOutcome, TurnRequest, TurnStatus, TurnUsage, diff --git a/crates/code_assistant_core/src/session/service.rs b/crates/code_assistant_core/src/session/service.rs index 6338a52f..0048ad22 100644 --- a/crates/code_assistant_core/src/session/service.rs +++ b/crates/code_assistant_core/src/session/service.rs @@ -30,7 +30,6 @@ use command_executor::CommandExecutor; use llm::factory::create_llm_client_from_model; use llm::provider_config::ConfigurationSystem; use sandbox::SandboxPolicy; -use std::collections::HashMap; use std::future::Future; use std::path::{Path, PathBuf}; use std::pin::Pin; @@ -137,7 +136,7 @@ pub struct WorktreeListing { } /// Which changes the Review panel should compare. -#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)] pub enum ReviewMode { /// Local working-tree changes (staged, unstaged, untracked) vs `HEAD`. WorkingTree, @@ -145,6 +144,18 @@ pub enum ReviewMode { BranchVsBase, } +/// Progress of one repo's background review scan, carried alongside the data +/// so the UI can render an activity indicator per repo. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ReviewScanState { + /// Not scanned yet in this round (data, if any, is from the cache). + Pending, + /// Currently being scanned in the background. + Scanning, + /// Scan finished; data is fresh. + Done, +} + /// Changed files for a single git repository within the reviewed project, /// plus the per-repo metadata the UI needs to render its selectors. #[derive(Debug, Clone)] @@ -160,19 +171,26 @@ pub struct RepoReview { /// defaulted from `None`). pub base: Option, pub files: Vec, + /// Aggregate added/deleted line counts (untracked files not included). + pub stats: git::DiffStats, + /// Scan progress for this repo in the current listing round. + pub scan_state: ReviewScanState, } -/// Listing of changed files for the Review panel across one or more -/// repositories discovered under the project root. -#[derive(Debug, Clone)] -pub struct ReviewListing { - /// One entry per discovered git repository (0 when the project is not, - /// and contains no, git repos). - pub repos: Vec, - /// `true` when at least one git repository was found. - pub is_git_repo: bool, - /// The mode this listing was produced for (echoed back for the UI). - pub mode: ReviewMode, +impl RepoReview { + /// An entry for a discovered-but-not-yet-scanned repo. + pub fn pending(repo_root: PathBuf, label: String) -> Self { + Self { + repo_root, + label, + current_branch: None, + base_candidates: Vec::new(), + base: None, + files: Vec::new(), + stats: git::DiffStats::default(), + scan_state: ReviewScanState::Pending, + } + } } /// A git worktree the session was switched to. @@ -1048,84 +1066,74 @@ impl SessionService { .await } - /// List changed files for the Review panel in the requested `mode`. + /// Discover the git repositories to review for a session — fast, no + /// per-repo scanning. Returns `(repo workdir, display label)` pairs. /// /// Resolves the session's on-disk directory with `effective_project_path` - /// (worktree-aware). Returns `is_git_repo: false` early for non-git dirs. - /// In `BranchVsBase` mode a repo without an override in `base_overrides` - /// defaults to the current branch's upstream, else the first remote - /// candidate. - pub async fn list_review_files( - &self, - session_id: String, - mode: ReviewMode, - base_overrides: HashMap, - ) -> Result { + /// (worktree-aware). An empty result means "not a git project". + pub async fn list_review_repos(&self, session_id: String) -> Result> { self.call(move |ctx| async move { let project_root = { let manager = ctx.manager.lock().await; session_effective_path(&manager, &session_id)? }; + Ok(discover_review_repos(&project_root)) + }) + .await + } - let discovered = discover_review_repos(&project_root); - if discovered.is_empty() { - return Ok(ReviewListing { - repos: Vec::new(), - is_git_repo: false, - mode, - }); - } + /// Scan a single repo for the Review panel: changed files, diff stats, and + /// the selector metadata. In `BranchVsBase` mode a `None` `base_override` + /// defaults to the current branch's upstream, else the first remote + /// candidate. + pub async fn scan_review_repo( + &self, + repo_root: PathBuf, + label: String, + mode: ReviewMode, + base_override: Option, + ) -> Result { + self.call(move |_ctx| async move { + let repo = + git::GitRepository::open(&repo_root).context("Failed to open git repository")?; + let current_branch = repo.current_branch(); + let base_candidates = repo.list_base_candidates().unwrap_or_default(); - let mut repos = Vec::with_capacity(discovered.len()); - for (repo_root, label) in discovered { - let repo = match git::GitRepository::open(&repo_root) { - Ok(r) => r, - Err(e) => { - warn!("Skipping repo {}: {e:#}", repo_root.display()); - continue; - } - }; - let current_branch = repo.current_branch(); - let base_candidates = repo.list_base_candidates().unwrap_or_default(); - let override_base = base_overrides.get(&repo_root).cloned(); - - let (files, resolved_base) = match mode { - ReviewMode::WorkingTree => { - let files = repo - .changed_files_working_tree() - .await - .context("Failed to list working-tree changes")?; - (files, None) - } - ReviewMode::BranchVsBase => { - let resolved = resolve_review_base(&repo, override_base, &base_candidates); - match &resolved { - Some(b) => { - let files = repo - .changed_files_vs_base(b) - .await - .with_context(|| format!("Failed to diff against {b}"))?; - (files, resolved) - } - None => (Vec::new(), None), + let (files, stats, resolved_base) = match mode { + ReviewMode::WorkingTree => { + let files = repo + .changed_files_working_tree() + .await + .context("Failed to list working-tree changes")?; + // Stats are best-effort (e.g. unborn HEAD has no diff base). + let stats = repo.diff_stats_working_tree().await.unwrap_or_default(); + (files, stats, None) + } + ReviewMode::BranchVsBase => { + let resolved = resolve_review_base(&repo, base_override, &base_candidates); + match &resolved { + Some(b) => { + let files = repo + .changed_files_vs_base(b) + .await + .with_context(|| format!("Failed to diff against {b}"))?; + let stats = repo.diff_stats_vs_base(b).await.unwrap_or_default(); + (files, stats, resolved) } + None => (Vec::new(), git::DiffStats::default(), None), } - }; - - repos.push(RepoReview { - repo_root, - label, - current_branch, - base_candidates, - base: resolved_base, - files, - }); - } + } + }; - Ok(ReviewListing { - is_git_repo: !repos.is_empty(), - repos, - mode, + Ok(RepoReview { + repo_root, + label, + current_branch, + base_candidates, + base: resolved_base, + files, + stats, + scan_state: ReviewScanState::Done, }) }) .await diff --git a/crates/git/src/diff.rs b/crates/git/src/diff.rs index 4e963a4b..55c3fa6b 100644 --- a/crates/git/src/diff.rs +++ b/crates/git/src/diff.rs @@ -31,6 +31,16 @@ pub struct ChangedFile { pub status: ChangeStatus, } +/// Aggregate line-change counts for a review listing (à la `git diff --stat`). +/// +/// Untracked files are not included — `git diff --numstat` only covers +/// tracked content. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Deserialize)] +pub struct DiffStats { + pub additions: usize, + pub deletions: usize, +} + /// The two whole-file sides of a diff, ready to be fed to the UI's unified /// diff renderer. Either side may be `None` (pure add or delete). When /// `is_binary` or `too_large` is set, the text sides are omitted and the UI @@ -177,6 +187,27 @@ impl GitRepository { Ok(build_diff_content(old, new)) } + /// Aggregate added/deleted line counts of the working tree (staged + + /// unstaged) vs `HEAD`. Untracked files are not counted. + pub async fn diff_stats_working_tree(&self) -> Result { + let out = self + .git + .run_bytes(self.workdir(), &["diff", "--numstat", "-M", "HEAD"]) + .await?; + Ok(parse_numstat(&out)) + } + + /// Aggregate added/deleted line counts of `base...HEAD` (merge-base + /// semantics, matching [`Self::changed_files_vs_base`]). + pub async fn diff_stats_vs_base(&self, base: &str) -> Result { + let range = format!("{base}...HEAD"); + let out = self + .git + .run_bytes(self.workdir(), &["diff", "--numstat", "-M", &range]) + .await?; + Ok(parse_numstat(&out)) + } + /// Candidate base refs for branch-vs-base comparison: local branches plus /// remote-tracking branches (excluding `*/HEAD`), sorted and de-duplicated. pub fn list_base_candidates(&self) -> Result> { @@ -316,6 +347,26 @@ fn parse_diff_name_status_z(bytes: &[u8]) -> Vec { files } +/// Parse `git diff --numstat` output and sum the per-file counts. +/// +/// Each line is `ADDEDDELETEDPATH`; binary files report `-` in the +/// numeric columns and are skipped. +fn parse_numstat(bytes: &[u8]) -> DiffStats { + let mut stats = DiffStats::default(); + for line in bytes.split(|&b| b == b'\n') { + let mut fields = line.split(|&b| b == b'\t'); + let (Some(add), Some(del)) = (fields.next(), fields.next()) else { + continue; + }; + let parse = |f: &[u8]| std::str::from_utf8(f).ok()?.parse::().ok(); + if let (Some(add), Some(del)) = (parse(add), parse(del)) { + stats.additions += add; + stats.deletions += del; + } + } + stats +} + /// Decode both raw sides into a [`FileDiffContent`], flagging binary and /// oversized content. fn build_diff_content(old: Side, new: Side) -> FileDiffContent { @@ -421,6 +472,43 @@ mod tests { assert_eq!(files[2].path, "added.txt"); } + #[test] + fn parse_numstat_sums_and_skips_binary() { + let raw = b"3\t1\tsrc/a.rs\n-\t-\tblob.bin\n10\t0\tnew.txt\n"; + let stats = parse_numstat(raw); + assert_eq!( + stats, + DiffStats { + additions: 13, + deletions: 1 + } + ); + assert_eq!(parse_numstat(b""), DiffStats::default()); + } + + #[tokio::test] + async fn diff_stats_working_tree_counts_lines() { + let dir = TempDir::new().unwrap(); + init_repo_with_commit(dir.path()); + + write(dir.path(), "a.txt", b"one\ntwo\nthree\n"); + git(dir.path(), &["add", "."]); + git(dir.path(), &["commit", "-m", "seed"]); + + // Replace one line and add one (net: +2 -1). + write(dir.path(), "a.txt", b"one\nTWO\nthree\nfour\n"); + + let repo = GitRepository::open(dir.path()).unwrap(); + let stats = repo.diff_stats_working_tree().await.unwrap(); + assert_eq!( + stats, + DiffStats { + additions: 2, + deletions: 1 + } + ); + } + #[tokio::test] async fn working_tree_add_modify_delete_untracked() { let dir = TempDir::new().unwrap(); diff --git a/crates/ui_gpui/src/app/commands.rs b/crates/ui_gpui/src/app/commands.rs index e4b3cd5d..7c47428f 100644 --- a/crates/ui_gpui/src/app/commands.rs +++ b/crates/ui_gpui/src/app/commands.rs @@ -591,32 +591,86 @@ impl Gpui { // Review panel // ======================================================================== - /// Fetch the changed-files listing for the Review panel in the given mode. + /// Refresh the Review panel: discover repos immediately (showing cached + /// results where available), then scan them one at a time in the + /// background, streaming a listing update after every state change. pub(crate) fn cmd_list_review_files( &self, session_id: String, mode: code_assistant_core::session::ReviewMode, base_overrides: std::collections::HashMap, ) { + use code_assistant_core::session::{RepoReview, ReviewScanState}; + use std::sync::atomic::Ordering; + let Some(service) = self.session_service() else { return; }; let gpui = self.clone(); + // Supersede any scan still streaming for an older request. + let epoch = self.review_scan_epoch.fetch_add(1, Ordering::SeqCst) + 1; + self.dispatch(async move { - match service - .list_review_files(session_id.clone(), mode, base_overrides) - .await - { - Ok(listing) => { - if gpui.is_current_session(&session_id) { - gpui.push_event(UiEvent::UpdateReviewFiles { - repos: listing.repos, - is_git_repo: listing.is_git_repo, - mode: listing.mode, - }); + let push = |repos: &[RepoReview], is_git_repo: bool| { + if gpui.review_scan_epoch.load(Ordering::SeqCst) == epoch + && gpui.is_current_session(&session_id) + { + gpui.push_event(UiEvent::UpdateReviewFiles { + repos: repos.to_vec(), + is_git_repo, + mode, + }); + } + }; + + // Phase 1: fast discovery — show every repo right away, seeded + // from the on-disk cache where a previous scan exists. + let discovered = match service.list_review_repos(session_id.clone()).await { + Ok(d) => d, + Err(e) => { + debug!("Failed to discover review repos: {e:#}"); + return; + } + }; + let is_git_repo = !discovered.is_empty(); + let mut repos: Vec = discovered + .into_iter() + .map(|(root, label)| { + crate::shared::review_cache::load(&root, &label, mode) + .unwrap_or_else(|| RepoReview::pending(root, label)) + }) + .collect(); + push(&repos, is_git_repo); + + // Phase 2: scan repo by repo, streaming each result. + for ix in 0..repos.len() { + if gpui.review_scan_epoch.load(Ordering::SeqCst) != epoch { + return; + } + repos[ix].scan_state = ReviewScanState::Scanning; + push(&repos, is_git_repo); + + let root = repos[ix].repo_root.clone(); + let label = repos[ix].label.clone(); + let base_override = base_overrides.get(&root).cloned(); + match service + .scan_review_repo(root, label, mode, base_override) + .await + { + Ok(review) => { + crate::shared::review_cache::store(&review, mode); + repos[ix] = review; + } + Err(e) => { + debug!( + "Failed to scan review repo {}: {e:#}", + repos[ix].repo_root.display() + ); + // Keep the cached/empty data; just stop indicating. + repos[ix].scan_state = ReviewScanState::Done; } } - Err(e) => debug!("Failed to list review files: {e:#}"), + push(&repos, is_git_repo); } }); } diff --git a/crates/ui_gpui/src/app/event_loop.rs b/crates/ui_gpui/src/app/event_loop.rs index 41b14960..b4e72e3c 100644 --- a/crates/ui_gpui/src/app/event_loop.rs +++ b/crates/ui_gpui/src/app/event_loop.rs @@ -853,6 +853,8 @@ impl Gpui { base_candidates: r.base_candidates, base: r.base, files: r.files, + stats: r.stats, + scan_state: r.scan_state, }) .collect(); self.set_current_review_listing(Some(ReviewData { diff --git a/crates/ui_gpui/src/lib.rs b/crates/ui_gpui/src/lib.rs index c97efa39..2840b003 100644 --- a/crates/ui_gpui/src/lib.rs +++ b/crates/ui_gpui/src/lib.rs @@ -109,6 +109,10 @@ pub struct RepoReviewData { pub base_candidates: Vec, pub base: Option, pub files: Vec, + /// Aggregate added/deleted line counts (untracked files not included). + pub stats: git::DiffStats, + /// Background-scan progress for this repo (drives the activity indicator). + pub scan_state: code_assistant_core::session::ReviewScanState, } /// Latest loaded diff for a single file in the Review panel. @@ -179,6 +183,11 @@ pub struct Gpui { current_review_listing: Arc)>>, current_review_diff: Arc)>>, + // Monotonic id of the latest review-scan request. A streaming scan task + // compares its captured epoch against this and stops when superseded, so + // stale scans never overwrite fresher listings. + review_scan_epoch: Arc, + // Last usage from the active session's most recent assistant message. // Stored separately from chat_sessions so it cannot be overwritten by // stale metadata loaded from disk (via UpdateChatList / ListSessions). @@ -508,6 +517,7 @@ impl Gpui { current_worktree_data: Arc::new(Mutex::new(None)), current_review_listing: Arc::new(Mutex::new((0, None))), current_review_diff: Arc::new(Mutex::new((0, None))), + review_scan_epoch: Arc::new(std::sync::atomic::AtomicU64::new(0)), // Current session last usage current_session_last_usage: Arc::new(Mutex::new(None)), diff --git a/crates/ui_gpui/src/main_screen/right_panel/review_view.rs b/crates/ui_gpui/src/main_screen/right_panel/review_view.rs index a98af38b..5f97524c 100644 --- a/crates/ui_gpui/src/main_screen/right_panel/review_view.rs +++ b/crates/ui_gpui/src/main_screen/right_panel/review_view.rs @@ -11,9 +11,11 @@ //! line diff is computed once per content change, never during render. use crate::shared::file_tree::{ChangedFilesTree, ChangedFilesTreeEvent}; -use crate::tool_cards::diff_card::{DiffLine, compute_diff_lines, render_diff_lines}; +use crate::tool_cards::diff_card::{ + DiffLine, added_row_colors, compute_diff_lines, deleted_row_colors, render_diff_lines, +}; use crate::{Gpui, ReviewData}; -use code_assistant_core::session::ReviewMode; +use code_assistant_core::session::{ReviewMode, ReviewScanState}; use gpui::{ Context, Entity, EventEmitter, FocusHandle, Focusable, FontWeight, Render, ScrollHandle, Subscription, Window, div, prelude::*, px, rems, @@ -23,6 +25,7 @@ use gpui_component::{ resizable::{ResizableState, h_resizable, resizable_panel}, scroll::ScrollableElement, select::{Select, SelectEvent, SelectItem, SelectState}, + spinner::Spinner, v_flex, }; use std::collections::HashMap; @@ -102,6 +105,9 @@ struct RepoSection { base_state: Entity>>, base_candidates: Vec, base: Option, + has_files: bool, + stats: git::DiffStats, + scan_state: ReviewScanState, collapsed: bool, _tree_sub: Subscription, _base_sub: Subscription, @@ -468,6 +474,9 @@ impl ReviewView { base_state, base_candidates: data.base_candidates.clone(), base: effective_base, + has_files: !data.files.is_empty(), + stats: data.stats, + scan_state: data.scan_state, collapsed: false, _tree_sub: tree_sub, _base_sub: base_sub, @@ -482,6 +491,9 @@ impl ReviewView { cx: &mut Context, ) { section.label = data.label.clone(); + section.has_files = !data.files.is_empty(); + section.stats = data.stats; + section.scan_state = data.scan_state; section .tree .update(cx, |t, cx| t.set_files(&data.files, cx)); @@ -623,15 +635,59 @@ impl ReviewView { .into_any_element() } + /// The header's right-hand slot: a spinner while a repo is being scanned, + /// a muted dash while it waits its turn, and a `+adds −dels` summary once + /// its (possibly cached) result is in. + fn render_scan_indicator(&self, section: &RepoSection, cx: &Context) -> gpui::AnyElement { + let theme = cx.theme(); + let muted = theme.muted_foreground; + + if matches!(section.scan_state, ReviewScanState::Scanning) { + return Spinner::new() + .with_size(Size::XSmall) + .color(muted) + .into_any_element(); + } + + let pending = matches!(section.scan_state, ReviewScanState::Pending); + let has_data = section.has_files || section.stats != git::DiffStats::default(); + if !has_data { + // Nothing (yet) to summarize: a queued repo shows a wait marker, + // a scanned clean repo shows no indicator at all. + return if pending { + div().text_color(muted).child("⋯").into_any_element() + } else { + div().into_any_element() + }; + } + + // Stats badge; a trailing wait marker means "cached, refresh queued". + div() + .flex() + .flex_row() + .items_center() + .gap_1() + .text_xs() + .child( + div() + .text_color(added_row_colors(theme).1) + .child(format!("+{}", section.stats.additions)), + ) + .child( + div() + .text_color(deleted_row_colors(theme).1) + .child(format!("−{}", section.stats.deletions)), + ) + .when(pending, |el| el.child(div().text_color(muted).child("⋯"))) + .into_any_element() + } + /// Render the right column: a scrollable stack of per-repo sections. fn render_tree_column(&self, cx: &mut Context) -> gpui::AnyElement { let muted = cx.theme().muted_foreground; let fg = cx.theme().foreground; let border = cx.theme().border; let branch_mode = matches!(self.mode, ReviewMode::BranchVsBase); - // A single repo whose label matches the project needn't show its header - // chrome; but keeping it uniform is simpler and clarifies multi-repo. - let show_headers = self.repos.len() > 1 || branch_mode; let mut column = v_flex().size_full().overflow_y_scrollbar(); @@ -639,43 +695,44 @@ impl ReviewView { let repo_root = section.repo_root.clone(); let collapsed = section.collapsed; - let mut section_el = v_flex().w_full(); + // Sections are separated by a line ABOVE each section (not by a + // line between a section's header and its content). + let mut section_el = v_flex() + .w_full() + .when(ix > 0, |s| s.border_t_1().border_color(border)); - if show_headers { - let chevron = if collapsed { - "icons/chevron_right.svg" - } else { - "icons/chevron_down.svg" - }; - let header = div() - .id(gpui::SharedString::from(format!("repo-header-{ix}"))) - .flex() - .flex_row() - .items_center() - .gap_1p5() - .px_2() - .py_1() - .border_b_1() - .border_color(border) - .cursor_pointer() - .hover(|s| s.bg(cx.theme().muted)) - .child(gpui::svg().size(px(12.)).path(chevron).text_color(muted)) - .child( - div() - .flex_1() - .text_sm() - .font_weight(gpui::FontWeight::MEDIUM) - .text_color(fg) - .child(section.label.clone()), - ) - .on_click(cx.listener(move |this, _ev, _window, cx| { - if let Some(s) = this.repos.iter_mut().find(|s| s.repo_root == repo_root) { - s.collapsed = !s.collapsed; - cx.notify(); - } - })); - section_el = section_el.child(header); - } + let chevron = if collapsed { + "icons/chevron_right.svg" + } else { + "icons/chevron_down.svg" + }; + let header = div() + .id(gpui::SharedString::from(format!("repo-header-{ix}"))) + .flex() + .flex_row() + .items_center() + .gap_1p5() + .px_2() + .py_1() + .cursor_pointer() + .hover(|s| s.bg(cx.theme().muted)) + .child(gpui::svg().size(px(12.)).path(chevron).text_color(muted)) + .child( + div() + .flex_1() + .text_sm() + .font_weight(gpui::FontWeight::MEDIUM) + .text_color(fg) + .child(section.label.clone()), + ) + .child(self.render_scan_indicator(section, cx)) + .on_click(cx.listener(move |this, _ev, _window, cx| { + if let Some(s) = this.repos.iter_mut().find(|s| s.repo_root == repo_root) { + s.collapsed = !s.collapsed; + cx.notify(); + } + })); + section_el = section_el.child(header); if !collapsed { if branch_mode { @@ -694,7 +751,14 @@ impl ReviewView { ), ); } - section_el = section_el.child(section.tree.clone()); + // While a repo waits for its first-ever scan there is nothing + // meaningful to show — suppress the tree so it can't claim + // "No changes" prematurely. + let awaiting_first_data = + !section.has_files && !matches!(section.scan_state, ReviewScanState::Done); + if !awaiting_first_data { + section_el = section_el.child(section.tree.clone()); + } } column = column.child(section_el); @@ -720,7 +784,23 @@ impl Render for ReviewView { let muted = cx.theme().muted_foreground; let border = cx.theme().border; - if !self.is_git_repo && self.last_listing.is_some() { + // Before the first (fast) discovery response there is nothing to lay + // out yet — show explicit activity instead of an empty panel. + if self.last_listing.is_none() { + return v_flex() + .size_full() + .items_center() + .justify_center() + .gap_2() + .p_4() + .text_sm() + .text_color(muted) + .child(Spinner::new().with_size(Size::Small).color(muted)) + .child("Looking for repositories…") + .into_any_element(); + } + + if !self.is_git_repo { return v_flex() .size_full() .items_center() diff --git a/crates/ui_gpui/src/shared/mod.rs b/crates/ui_gpui/src/shared/mod.rs index 851b95fb..213680b2 100644 --- a/crates/ui_gpui/src/shared/mod.rs +++ b/crates/ui_gpui/src/shared/mod.rs @@ -6,6 +6,7 @@ pub mod file_icons; pub mod file_tree; pub mod image; pub mod plan_banner; +pub mod review_cache; pub mod settings; pub mod theme; pub mod ui_state; diff --git a/crates/ui_gpui/src/shared/review_cache.rs b/crates/ui_gpui/src/shared/review_cache.rs new file mode 100644 index 00000000..d5f05088 --- /dev/null +++ b/crates/ui_gpui/src/shared/review_cache.rs @@ -0,0 +1,142 @@ +//! On-disk cache for Review panel scan results. +//! +//! One JSON file per `(repo, mode)` under `/review-cache/`. The +//! cache exists purely so the panel can show the last known state instantly +//! while a fresh background scan runs — entries are never trusted as current +//! and are always refreshed after being displayed. + +use code_assistant_core::session::{RepoReview, ReviewMode, ReviewScanState}; +use serde::{Deserialize, Serialize}; +use std::hash::{Hash, Hasher}; +use std::path::{Path, PathBuf}; +use tracing::{debug, warn}; + +/// Serialized form of one repo's last scan result. `repo_root` and `mode` are +/// stored so a hash collision can be detected on load. +#[derive(Debug, Serialize, Deserialize)] +struct CachedRepoScan { + repo_root: PathBuf, + mode: ReviewMode, + current_branch: Option, + base_candidates: Vec, + base: Option, + files: Vec, + stats: git::DiffStats, +} + +fn cache_dir() -> PathBuf { + code_assistant_core::config_dir::config_dir().join("review-cache") +} + +fn cache_path(repo_root: &Path, mode: ReviewMode) -> PathBuf { + let mut hasher = std::hash::DefaultHasher::new(); + repo_root.hash(&mut hasher); + mode.hash(&mut hasher); + cache_dir().join(format!("{:016x}.json", hasher.finish())) +} + +/// Load the cached scan for `(repo_root, mode)`, returned as a +/// [`ReviewScanState::Pending`] entry (data present, but not fresh). +/// Returns `None` when there is no valid cache entry. +pub fn load(repo_root: &Path, label: &str, mode: ReviewMode) -> Option { + let path = cache_path(repo_root, mode); + let json = std::fs::read_to_string(&path).ok()?; + let cached: CachedRepoScan = match serde_json::from_str(&json) { + Ok(c) => c, + Err(e) => { + warn!("Ignoring corrupt review cache {}: {}", path.display(), e); + return None; + } + }; + // Guard against hash collisions and stale mode mixups. + if cached.repo_root != repo_root || cached.mode != mode { + return None; + } + Some(RepoReview { + repo_root: cached.repo_root, + label: label.to_owned(), + current_branch: cached.current_branch, + base_candidates: cached.base_candidates, + base: cached.base, + files: cached.files, + stats: cached.stats, + scan_state: ReviewScanState::Pending, + }) +} + +/// Persist a finished scan for later instant display. Errors are logged only — +/// the cache is an optimization, never a requirement. +pub fn store(review: &RepoReview, mode: ReviewMode) { + let cached = CachedRepoScan { + repo_root: review.repo_root.clone(), + mode, + current_branch: review.current_branch.clone(), + base_candidates: review.base_candidates.clone(), + base: review.base.clone(), + files: review.files.clone(), + stats: review.stats, + }; + let path = cache_path(&review.repo_root, mode); + match code_assistant_core::utils::file_utils::atomic_write_json(&path, &cached) { + Ok(()) => debug!("Cached review scan to {}", path.display()), + Err(e) => warn!("Failed to write review cache {}: {}", path.display(), e), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Point the config dir at a temp dir for the duration of a test. + /// Serialized via a lock because the env var is process-global. + fn with_temp_config_dir(f: impl FnOnce() -> R) -> R { + static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + let _guard = LOCK.lock().unwrap(); + let dir = tempfile::tempdir().unwrap(); + // SAFETY: guarded by LOCK; tests touching this var run serialized. + unsafe { std::env::set_var("CODE_ASSISTANT_CONFIG_DIR", dir.path()) }; + let result = f(); + unsafe { std::env::remove_var("CODE_ASSISTANT_CONFIG_DIR") }; + result + } + + fn sample_review(root: &Path) -> RepoReview { + RepoReview { + repo_root: root.to_path_buf(), + label: "alpha".into(), + current_branch: Some("feature".into()), + base_candidates: vec!["main".into(), "origin/main".into()], + base: Some("origin/main".into()), + files: vec![git::ChangedFile { + path: "src/lib.rs".into(), + orig_path: None, + status: git::ChangeStatus::Modified, + }], + stats: git::DiffStats { + additions: 12, + deletions: 3, + }, + scan_state: ReviewScanState::Done, + } + } + + #[test] + fn store_load_roundtrip_marks_pending() { + with_temp_config_dir(|| { + let root = PathBuf::from("/tmp/some/repo"); + let review = sample_review(&root); + store(&review, ReviewMode::WorkingTree); + + let loaded = load(&root, "alpha", ReviewMode::WorkingTree).expect("cache hit"); + assert_eq!(loaded.scan_state, ReviewScanState::Pending); + assert_eq!(loaded.files, review.files); + assert_eq!(loaded.stats, review.stats); + assert_eq!(loaded.base.as_deref(), Some("origin/main")); + + // Different mode is a separate cache entry. + assert!(load(&root, "alpha", ReviewMode::BranchVsBase).is_none()); + // Unknown repo misses. + assert!(load(Path::new("/tmp/other"), "x", ReviewMode::WorkingTree).is_none()); + }); + } +} From cce6d16e1ce9e06970b5fc0377e46a5a0729a36c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Tue, 1 Sep 2026 16:29:31 +0200 Subject: [PATCH 06/10] style(ui_gpui): session-style scan spinner and silent clean repos in review panel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-repo activity indicator now uses the same rotating double-arrow (icons/arrow_circle.svg) as active sessions, in muted grey — including the pre-discovery placeholder. Repos without changes render only their header: the tree is omitted entirely and the 'No changes' placeholder is gone; the absent +/− badge already communicates a clean repo. --- .../main_screen/right_panel/review_view.rs | 38 ++++++++++++------- crates/ui_gpui/src/shared/file_tree.rs | 9 ++--- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/crates/ui_gpui/src/main_screen/right_panel/review_view.rs b/crates/ui_gpui/src/main_screen/right_panel/review_view.rs index 5f97524c..cf83b256 100644 --- a/crates/ui_gpui/src/main_screen/right_panel/review_view.rs +++ b/crates/ui_gpui/src/main_screen/right_panel/review_view.rs @@ -17,15 +17,14 @@ use crate::tool_cards::diff_card::{ use crate::{Gpui, ReviewData}; use code_assistant_core::session::{ReviewMode, ReviewScanState}; use gpui::{ - Context, Entity, EventEmitter, FocusHandle, Focusable, FontWeight, Render, ScrollHandle, - Subscription, Window, div, prelude::*, px, rems, + AnimationExt, Context, Entity, EventEmitter, FocusHandle, Focusable, FontWeight, Render, + ScrollHandle, Subscription, Window, div, prelude::*, px, rems, }; use gpui_component::{ ActiveTheme, Icon, Sizable, Size, resizable::{ResizableState, h_resizable, resizable_panel}, scroll::ScrollableElement, select::{Select, SelectEvent, SelectItem, SelectState}, - spinner::Spinner, v_flex, }; use std::collections::HashMap; @@ -635,6 +634,23 @@ impl ReviewView { .into_any_element() } + /// The rotating double-arrow used on active sessions, in grey — shown on + /// the repo whose scan is currently running. `id` keys the animation. + fn scan_spinner(id: impl std::fmt::Display, muted: gpui::Hsla) -> gpui::AnyElement { + gpui::svg() + .size(px(12.)) + .path("icons/arrow_circle.svg") + .text_color(muted) + .with_animation( + gpui::SharedString::from(format!("review-scan-spin-{id}")), + gpui::Animation::new(std::time::Duration::from_secs(2)).repeat(), + |svg, delta| { + svg.with_transformation(gpui::Transformation::rotate(gpui::percentage(delta))) + }, + ) + .into_any_element() + } + /// The header's right-hand slot: a spinner while a repo is being scanned, /// a muted dash while it waits its turn, and a `+adds −dels` summary once /// its (possibly cached) result is in. @@ -643,10 +659,7 @@ impl ReviewView { let muted = theme.muted_foreground; if matches!(section.scan_state, ReviewScanState::Scanning) { - return Spinner::new() - .with_size(Size::XSmall) - .color(muted) - .into_any_element(); + return Self::scan_spinner(section.repo_root.display(), muted); } let pending = matches!(section.scan_state, ReviewScanState::Pending); @@ -751,12 +764,9 @@ impl ReviewView { ), ); } - // While a repo waits for its first-ever scan there is nothing - // meaningful to show — suppress the tree so it can't claim - // "No changes" prematurely. - let awaiting_first_data = - !section.has_files && !matches!(section.scan_state, ReviewScanState::Done); - if !awaiting_first_data { + // A repo without changes shows just its header — the missing + // +/− badge already says "clean", so no placeholder text. + if section.has_files { section_el = section_el.child(section.tree.clone()); } } @@ -795,7 +805,7 @@ impl Render for ReviewView { .p_4() .text_sm() .text_color(muted) - .child(Spinner::new().with_size(Size::Small).color(muted)) + .child(Self::scan_spinner("discovery", muted)) .child("Looking for repositories…") .into_any_element(); } diff --git a/crates/ui_gpui/src/shared/file_tree.rs b/crates/ui_gpui/src/shared/file_tree.rs index e885ba3b..b1a641d3 100644 --- a/crates/ui_gpui/src/shared/file_tree.rs +++ b/crates/ui_gpui/src/shared/file_tree.rs @@ -273,13 +273,10 @@ impl Render for ChangedFilesTree { let accent = cx.theme().accent; let selected = self.selected.clone(); + // An empty tree renders nothing — callers hide the tree for clean + // repos, and a repo's missing +/− badge already communicates "clean". if rows.is_empty() { - return div() - .p_3() - .text_sm() - .text_color(muted) - .child("No changes") - .into_any_element(); + return div().into_any_element(); } div() From c03d7555e8cbac0fe165df7d6de33f3cf5f5c0f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Tue, 1 Sep 2026 16:31:29 +0200 Subject: [PATCH 07/10] style(ui_gpui): faded static double-arrow as pending marker in review panel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Queued repos now show the same arrow_circle icon as the running scan, static and at 40% of the muted color, replacing the '⋯' text marker — both standalone and trailing a cached stats badge. --- .../src/main_screen/right_panel/review_view.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/crates/ui_gpui/src/main_screen/right_panel/review_view.rs b/crates/ui_gpui/src/main_screen/right_panel/review_view.rs index cf83b256..e15598fb 100644 --- a/crates/ui_gpui/src/main_screen/right_panel/review_view.rs +++ b/crates/ui_gpui/src/main_screen/right_panel/review_view.rs @@ -651,9 +651,19 @@ impl ReviewView { .into_any_element() } + /// The same double-arrow, static and faded — marks a repo that is queued + /// for scanning but not yet running. + fn pending_marker(muted: gpui::Hsla) -> gpui::AnyElement { + gpui::svg() + .size(px(12.)) + .path("icons/arrow_circle.svg") + .text_color(muted.opacity(0.4)) + .into_any_element() + } + /// The header's right-hand slot: a spinner while a repo is being scanned, - /// a muted dash while it waits its turn, and a `+adds −dels` summary once - /// its (possibly cached) result is in. + /// a faded static one while it waits its turn, and a `+adds −dels` + /// summary once its (possibly cached) result is in. fn render_scan_indicator(&self, section: &RepoSection, cx: &Context) -> gpui::AnyElement { let theme = cx.theme(); let muted = theme.muted_foreground; @@ -668,7 +678,7 @@ impl ReviewView { // Nothing (yet) to summarize: a queued repo shows a wait marker, // a scanned clean repo shows no indicator at all. return if pending { - div().text_color(muted).child("⋯").into_any_element() + Self::pending_marker(muted) } else { div().into_any_element() }; @@ -691,7 +701,7 @@ impl ReviewView { .text_color(deleted_row_colors(theme).1) .child(format!("−{}", section.stats.deletions)), ) - .when(pending, |el| el.child(div().text_color(muted).child("⋯"))) + .when(pending, |el| el.child(Self::pending_marker(muted))) .into_any_element() } From 2f5c3a34ff44cd59210797d1377cf28d9670ad81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Sun, 6 Sep 2026 11:13:42 +0200 Subject: [PATCH 08/10] feat(ui_gpui): stacked per-file diffs in the review panel, hunk-based and off-thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the two-column tree/diff layout with a single scrollable stack, the way Zed's Project Diff (and most coding-agent UIs) present changes: each file is a collapsible header (icon, path, status letter, per-file +/− counts) with its diff directly below, then the next file. The file tree, the resizable split, and the review_tree_width setting are gone. Rendering and loading follow what makes Zed's diff view fast: - Diffs show only hunks — changed lines plus 3 context lines (similar::grouped_ops) with a '⋯' separator between hunks — instead of the whole file, so element counts scale with changed lines, not file sizes. Line numbers stay real per hunk, with a shared gutter width. - Hunk computation runs in the command layer on the background executor; the UI thread only builds elements from prepared hunks and never runs a Myers diff. Pure adds/deletes skip diffing entirely (this also fixes the phantom blank 'deleted' line on added files). - Diffs load lazily, one file at a time: the view keeps a single request in flight and asks for the next missing visible diff after each arrival; collapsed repos and files are skipped until expanded. - UpdateReviewFiles/UpdateReviewDiff are now payload-free notifications; the command layer mirrors listing and prepared diffs straight into the generation-counted Gpui globals. --- .../code_assistant_core/src/ui/ui_events.rs | 17 +- crates/ui_acp/src/ui.rs | 2 +- crates/ui_gpui/src/app/commands.rs | 60 +- crates/ui_gpui/src/app/event_loop.rs | 45 +- crates/ui_gpui/src/lib.rs | 70 +- .../main_screen/right_panel/review_view.rs | 623 ++++++++++-------- crates/ui_gpui/src/shared/file_tree.rs | 431 ------------ crates/ui_gpui/src/shared/mod.rs | 1 - crates/ui_gpui/src/shared/settings.rs | 5 - crates/ui_gpui/src/tool_cards/diff_card.rs | 157 ++++- 10 files changed, 615 insertions(+), 796 deletions(-) delete mode 100644 crates/ui_gpui/src/shared/file_tree.rs diff --git a/crates/code_assistant_core/src/ui/ui_events.rs b/crates/code_assistant_core/src/ui/ui_events.rs index 682694bb..40155b55 100644 --- a/crates/code_assistant_core/src/ui/ui_events.rs +++ b/crates/code_assistant_core/src/ui/ui_events.rs @@ -334,18 +334,11 @@ pub enum UiEvent { }, // === Review Panel Events === - /// Updated list of changed files for the Review panel, grouped per repo. - UpdateReviewFiles { - repos: Vec, - is_git_repo: bool, - mode: crate::session::ReviewMode, - }, - /// The loaded diff for a single file selected in the Review panel. - UpdateReviewDiff { - repo_root: PathBuf, - path: String, - diff: git::FileDiffContent, - }, + /// The Review panel's changed-files listing changed. Pure notification — + /// the data itself is mirrored into the UI layer's state by the sender. + UpdateReviewFiles, + /// A prepared file diff for the Review panel arrived (same mirror scheme). + UpdateReviewDiff, // === Configuration Events === /// Configuration files (providers.json / models.json) were changed on disk. diff --git a/crates/ui_acp/src/ui.rs b/crates/ui_acp/src/ui.rs index b2d56253..04bdbc5d 100644 --- a/crates/ui_acp/src/ui.rs +++ b/crates/ui_acp/src/ui.rs @@ -973,7 +973,7 @@ impl UserInterface for ACPUserUI { UiEvent::UpdateWorktreeData { .. } => { // Worktree management not supported in ACP UI } - UiEvent::UpdateReviewFiles { .. } | UiEvent::UpdateReviewDiff { .. } => { + UiEvent::UpdateReviewFiles | UiEvent::UpdateReviewDiff => { // Review panel is GPUI-specific. } UiEvent::UpdateAllowedModels { .. } => { diff --git a/crates/ui_gpui/src/app/commands.rs b/crates/ui_gpui/src/app/commands.rs index e0e4ffd7..d8f7da5f 100644 --- a/crates/ui_gpui/src/app/commands.rs +++ b/crates/ui_gpui/src/app/commands.rs @@ -638,15 +638,31 @@ impl Gpui { let epoch = self.review_scan_epoch.fetch_add(1, Ordering::SeqCst) + 1; self.dispatch(async move { + // Mirror the listing into the Gpui global and notify the UI. The + // event itself carries no data (see UiEvent::UpdateReviewFiles). let push = |repos: &[RepoReview], is_git_repo: bool| { if gpui.review_scan_epoch.load(Ordering::SeqCst) == epoch && gpui.is_current_session(&session_id) { - gpui.push_event(UiEvent::UpdateReviewFiles { - repos: repos.to_vec(), + let repos = repos + .iter() + .map(|r| crate::RepoReviewData { + repo_root: r.repo_root.clone(), + label: r.label.clone(), + current_branch: r.current_branch.clone(), + base_candidates: r.base_candidates.clone(), + base: r.base.clone(), + files: r.files.clone(), + stats: r.stats, + scan_state: r.scan_state, + }) + .collect(); + gpui.set_current_review_listing(Some(crate::ReviewData { + repos, is_git_repo, mode, - }); + })); + gpui.push_event(UiEvent::UpdateReviewFiles); } }; @@ -718,20 +734,32 @@ impl Gpui { let path = file.path.clone(); let event_repo_root = repo_root.clone(); self.dispatch(async move { - match service + let result = service .get_review_file_diff(session_id.clone(), repo_root, mode, base, file) - .await - { - Ok(diff) => { - if gpui.is_current_session(&session_id) { - gpui.push_event(UiEvent::UpdateReviewDiff { - repo_root: event_repo_root, - path, - diff, - }); - } - } - Err(e) => gpui.display_error(format!("Failed to load diff: {e:#}")), + .await; + // Hunks are computed HERE, on the background executor — the UI + // thread only ever renders prepared hunks (never runs a diff). + let prepared = match &result { + Ok(diff) => crate::PreparedReviewDiff::from_content(diff), + // An empty prepared diff still completes the view's one-at-a- + // time request pipeline; the error itself is surfaced below. + Err(_) => crate::PreparedReviewDiff::from_content(&git::FileDiffContent { + old_text: None, + new_text: None, + is_binary: false, + too_large: false, + }), + }; + if let Err(e) = &result { + gpui.display_error(format!("Failed to load diff: {e:#}")); + } + if gpui.is_current_session(&session_id) { + gpui.set_current_review_diff(Some(crate::ReviewDiff { + repo_root: event_repo_root, + path, + prepared, + })); + gpui.push_event(UiEvent::UpdateReviewDiff); } }); } diff --git a/crates/ui_gpui/src/app/event_loop.rs b/crates/ui_gpui/src/app/event_loop.rs index 1a1a49c3..84502d2d 100644 --- a/crates/ui_gpui/src/app/event_loop.rs +++ b/crates/ui_gpui/src/app/event_loop.rs @@ -842,48 +842,9 @@ impl Gpui { cx.refresh(); } - UiEvent::UpdateReviewFiles { - repos, - is_git_repo, - mode, - } => { - debug!( - "UI: UpdateReviewFiles event — {} repos, is_git_repo={}", - repos.len(), - is_git_repo - ); - let repos = repos - .into_iter() - .map(|r| RepoReviewData { - repo_root: r.repo_root, - label: r.label, - current_branch: r.current_branch, - base_candidates: r.base_candidates, - base: r.base, - files: r.files, - stats: r.stats, - scan_state: r.scan_state, - }) - .collect(); - self.set_current_review_listing(Some(ReviewData { - repos, - is_git_repo, - mode, - })); - cx.refresh(); - } - - UiEvent::UpdateReviewDiff { - repo_root, - path, - diff, - } => { - debug!("UI: UpdateReviewDiff event — path={path}"); - self.set_current_review_diff(Some(ReviewDiff { - repo_root, - path, - diff, - })); + // Pure notifications — the command layer already mirrored the + // review data (listing / prepared diff) into the Gpui globals. + UiEvent::UpdateReviewFiles | UiEvent::UpdateReviewDiff => { cx.refresh(); } diff --git a/crates/ui_gpui/src/lib.rs b/crates/ui_gpui/src/lib.rs index d4b51427..4ef33e7d 100644 --- a/crates/ui_gpui/src/lib.rs +++ b/crates/ui_gpui/src/lib.rs @@ -115,12 +115,76 @@ pub struct RepoReviewData { pub scan_state: code_assistant_core::session::ReviewScanState, } -/// Latest loaded diff for a single file in the Review panel. -#[derive(Debug, Clone, PartialEq)] +/// Latest loaded diff for a single file in the Review panel, already prepared +/// for rendering (see [`PreparedReviewDiff`]). +#[derive(Debug, Clone)] pub struct ReviewDiff { pub repo_root: std::path::PathBuf, pub path: String, - pub diff: git::FileDiffContent, + pub prepared: PreparedReviewDiff, +} + +/// Context lines around each review diff hunk (matches `git diff`'s default). +const REVIEW_HUNK_CONTEXT_LINES: usize = 3; + +/// A file diff reduced to renderable hunks. The expensive line diff runs once +/// on a background thread (in the command layer) — the UI only builds elements +/// from the prepared hunks, so element counts scale with changed lines and the +/// UI thread never runs a Myers diff. +#[derive(Debug, Clone)] +pub struct PreparedReviewDiff { + pub is_binary: bool, + pub too_large: bool, + pub hunks: Vec, + pub additions: usize, + pub deletions: usize, +} + +impl PreparedReviewDiff { + /// Compute hunks (changed lines + context) from raw diff content. + /// CPU-heavy for large files — call on a background thread. + pub fn from_content(diff: &git::FileDiffContent) -> Self { + if diff.is_binary || diff.too_large { + return Self { + is_binary: diff.is_binary, + too_large: diff.too_large, + hunks: Vec::new(), + additions: 0, + deletions: 0, + }; + } + let old = diff.old_text.as_deref().filter(|t| !t.is_empty()); + let new = diff.new_text.as_deref().filter(|t| !t.is_empty()); + let hunks = match (old, new) { + (None, None) => Vec::new(), + (Some(old), Some(new)) => { + tool_cards::diff_card::compute_diff_hunks(old, new, REVIEW_HUNK_CONTEXT_LINES) + } + // Pure add / pure delete: the whole file is the hunk. + (None, Some(new)) => { + tool_cards::diff_card::single_sided_hunk(new, similar::ChangeTag::Insert) + } + (Some(old), None) => { + tool_cards::diff_card::single_sided_hunk(old, similar::ChangeTag::Delete) + } + }; + let mut additions = 0; + let mut deletions = 0; + for line in hunks.iter().flat_map(|h| h.lines.iter()) { + match line.tag { + similar::ChangeTag::Insert => additions += 1, + similar::ChangeTag::Delete => deletions += 1, + similar::ChangeTag::Equal => {} + } + } + Self { + is_binary: false, + too_large: false, + hunks, + additions, + deletions, + } + } } // Our main UI struct that implements the UserInterface trait diff --git a/crates/ui_gpui/src/main_screen/right_panel/review_view.rs b/crates/ui_gpui/src/main_screen/right_panel/review_view.rs index e15598fb..1feeffd0 100644 --- a/crates/ui_gpui/src/main_screen/right_panel/review_view.rs +++ b/crates/ui_gpui/src/main_screen/right_panel/review_view.rs @@ -1,38 +1,37 @@ -//! The "Review" view for the right sidebar: a compare-mode selector plus a -//! two-column body — the unified diff on the **left** and, on the **right**, a -//! per-repo set of changed-file trees (each repo gets its own base-branch -//! selector in "Branch vs base" mode). The two columns are separated by a -//! draggable divider (`h_resizable`). +//! The "Review" view for the right sidebar: a compare-mode selector above a +//! single scrollable column of per-repo sections. Within a repo, changed files +//! are **stacked**: each file has a collapsible header (icon, path, status, +//! per-file `+/−`) with its diff hunks directly below — no separate tree/diff +//! split. //! //! Backend data arrives through the `current_review_listing` / `current_review_diff` //! globals on [`Gpui`]; this view consumes them in `render` (the "sync-in-render" //! technique the worktree selector uses). Change detection is generation-based: -//! the per-frame unchanged case costs an integer compare, and the expensive -//! line diff is computed once per content change, never during render. - -use crate::shared::file_tree::{ChangedFilesTree, ChangedFilesTreeEvent}; -use crate::tool_cards::diff_card::{ - DiffLine, added_row_colors, compute_diff_lines, deleted_row_colors, render_diff_lines, -}; -use crate::{Gpui, ReviewData}; +//! the per-frame unchanged case costs an integer compare. +//! +//! Diffs load lazily, one file at a time: after each arrival the next visible +//! file without a diff is requested. Hunks (changed lines + a few context +//! lines) are computed once on arrival and cached — rendering never diffs, and +//! the element count scales with changed lines, not file sizes. + +use crate::shared::file_icons; +use crate::tool_cards::diff_card::{added_row_colors, deleted_row_colors, render_diff_hunks}; +use crate::{Gpui, PreparedReviewDiff, RepoReviewData}; use code_assistant_core::session::{ReviewMode, ReviewScanState}; +use git::{ChangeStatus, ChangedFile}; use gpui::{ AnimationExt, Context, Entity, EventEmitter, FocusHandle, Focusable, FontWeight, Render, - ScrollHandle, Subscription, Window, div, prelude::*, px, rems, + Subscription, Window, div, prelude::*, px, rems, }; use gpui_component::{ ActiveTheme, Icon, Sizable, Size, - resizable::{ResizableState, h_resizable, resizable_panel}, scroll::ScrollableElement, select::{Select, SelectEvent, SelectItem, SelectState}, v_flex, }; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::PathBuf; -/// Default width (px) of the file-tree column when nothing is persisted. -const DEFAULT_TREE_WIDTH: f32 = 240.0; - // --------------------------------------------------------------------------- // Compare-mode dropdown // --------------------------------------------------------------------------- @@ -95,36 +94,39 @@ impl SelectItem for BaseOption { // Per-repo section // --------------------------------------------------------------------------- -/// One git repo's changed-files tree plus its base selector, rendered as a -/// collapsible section in the right column. +/// One git repo's stacked file list plus its base selector, rendered as a +/// collapsible section. struct RepoSection { repo_root: PathBuf, label: String, - tree: Entity, base_state: Entity>>, base_candidates: Vec, base: Option, - has_files: bool, + files: Vec, stats: git::DiffStats, scan_state: ReviewScanState, collapsed: bool, - _tree_sub: Subscription, _base_sub: Subscription, } +/// Single-letter status badge (same colors the old file tree used). +fn status_badge(status: ChangeStatus) -> (&'static str, gpui::Hsla) { + match status { + ChangeStatus::Added | ChangeStatus::Untracked => ("A", gpui::rgb(0x3f_a5_5a).into()), + ChangeStatus::Modified => ("M", gpui::rgb(0xc7_9a_3a).into()), + ChangeStatus::Deleted => ("D", gpui::rgb(0xc7_4a_4a).into()), + ChangeStatus::Renamed => ("R", gpui::rgb(0x4a_82_c7).into()), + ChangeStatus::Copied => ("C", gpui::rgb(0x4a_82_c7).into()), + ChangeStatus::TypeChanged => ("T", gpui::rgb(0x8a_6a_c7).into()), + } +} + // --------------------------------------------------------------------------- // ReviewView // --------------------------------------------------------------------------- -/// A diff prepared for per-frame rendering: the expensive line diff is -/// computed once when the backend data changes, never during render. -struct PreparedDiff { - repo_root: PathBuf, - path: String, - is_binary: bool, - too_large: bool, - lines: Vec, -} +/// Identifies one changed file across repos. +type FileKey = (PathBuf, String); /// Sentinel for "never synced": guarantees the first generation compare /// mismatches, whatever the global's current generation is. @@ -137,32 +139,28 @@ pub struct ReviewView { /// Current compare mode (drives requests). Base is tracked per repo. mode: ReviewMode, is_git_repo: bool, + /// Whether the first discovery response has arrived. + has_listing: bool, - /// Per-repo sections in the right column. + /// Per-repo sections, in listing order. repos: Vec, - /// Currently selected file as `(repo_root, repo-relative path)`. - selected: Option<(PathBuf, String)>, /// User's explicit per-repo base choices for this session. base_overrides: HashMap, /// Persisted default base ref, seeds a repo's base when it has no override. default_base: Option, - /// Two-column split state (LEFT = diff, RIGHT = tree column). - split_state: Entity, - /// Persisted tree-column width, used as the initial panel size. - tree_width: f32, - /// Scroll position of the diff pane. - diff_scroll: ScrollHandle, - - /// Last listing consumed from the global; needed to resolve file lookups. - last_listing: Option, - /// Generation of `last_listing`. Change detection per frame is a plain - /// integer compare against the global's generation — no clones. - listing_generation: u64, + /// Prepared diffs by file, filled lazily one request at a time. + file_diffs: HashMap, + /// Files the user collapsed (default is expanded). + collapsed_files: HashSet, + /// The single outstanding diff request; arrivals for anything else are + /// stale (e.g. from before a mode/base change) and dropped. + in_flight: Option, - /// The selected file's diff, with its line diff computed once on arrival. - prepared_diff: Option, - /// Generation of `prepared_diff` (see `listing_generation`). + /// Generation of the consumed listing. Change detection per frame is a + /// plain integer compare against the global's generation — no clones. + listing_generation: u64, + /// Generation of the last consumed diff (see `listing_generation`). diff_generation: u64, focus_handle: FocusHandle, @@ -180,34 +178,24 @@ impl ReviewView { }); let mode_sub = cx.subscribe_in(&mode_state, window, Self::on_mode_event); - // Seed persisted preferences (default base + tree width) from settings. - let (default_base, tree_width) = cx + // Seed the persisted default base from settings. + let default_base = cx .try_global::() - .map(|g| { - ( - g.0.review_default_base.clone(), - g.0.review_tree_width.unwrap_or(DEFAULT_TREE_WIDTH), - ) - }) - .unwrap_or((None, DEFAULT_TREE_WIDTH)); - - let split_state = cx.new(|_| ResizableState::default()); + .and_then(|g| g.0.review_default_base.clone()); Self { session_id: None, mode_state, mode: ReviewMode::WorkingTree, is_git_repo: false, + has_listing: false, repos: Vec::new(), - selected: None, base_overrides: HashMap::new(), default_base, - split_state, - tree_width, - diff_scroll: ScrollHandle::new(), - last_listing: None, + file_diffs: HashMap::new(), + collapsed_files: HashSet::new(), + in_flight: None, listing_generation: GENERATION_UNSEEN, - prepared_diff: None, diff_generation: GENERATION_UNSEEN, focus_handle: cx.focus_handle(), _mode_sub: mode_sub, @@ -218,13 +206,14 @@ impl ReviewView { pub fn set_session(&mut self, session_id: Option, cx: &mut Context) { self.session_id = session_id; // Reset per-session state; fresh data will arrive via the global. - self.selected = None; - self.last_listing = None; + self.has_listing = false; self.listing_generation = GENERATION_UNSEEN; - self.prepared_diff = None; self.diff_generation = GENERATION_UNSEEN; self.repos.clear(); self.base_overrides.clear(); + self.file_diffs.clear(); + self.collapsed_files.clear(); + self.in_flight = None; // Restore the persisted compare mode for this session. The selector // resyncs from the echoed listing on the next render. @@ -278,43 +267,53 @@ impl ReviewView { } } - fn request_diff(&self, repo_root: &PathBuf, path: &str, cx: &mut Context) { - let Some(session_id) = self.session_id.clone() else { - return; - }; - let Some(listing) = &self.last_listing else { - return; - }; - let Some(repo) = listing.repos.iter().find(|r| &r.repo_root == repo_root) else { + /// Request the next visible file that has no prepared diff yet. At most + /// one request is in flight; collapsed repos and files are skipped, which + /// keeps loading lazy. + fn ensure_diff_request(&mut self, cx: &mut Context) { + if self.in_flight.is_some() { return; - }; - let Some(file) = repo.files.iter().find(|f| f.path == path).cloned() else { + } + let Some(session_id) = self.session_id.clone() else { return; }; - let base = repo.base.clone(); - if let Some(gpui) = cx.try_global::() { - gpui.cmd_get_review_file_diff(session_id, repo_root.clone(), self.mode, base, file); + + let mut next: Option<(PathBuf, Option, ChangedFile)> = None; + 'outer: for section in &self.repos { + if section.collapsed { + continue; + } + for file in §ion.files { + let key = (section.repo_root.clone(), file.path.clone()); + if self.collapsed_files.contains(&key) || self.file_diffs.contains_key(&key) { + continue; + } + next = Some(( + section.repo_root.clone(), + section.base.clone(), + file.clone(), + )); + break 'outer; + } } - } - fn on_file_selected(&mut self, repo_root: PathBuf, path: String, cx: &mut Context) { - // Clear selection highlight in sibling repos' trees. - for section in &self.repos { - if section.repo_root != repo_root { - section.tree.update(cx, |t, cx| t.set_selected(None, cx)); + if let Some((repo_root, base, file)) = next { + self.in_flight = Some((repo_root.clone(), file.path.clone())); + if let Some(gpui) = cx.try_global::() { + gpui.cmd_get_review_file_diff(session_id, repo_root, self.mode, base, file); } } - self.request_diff(&repo_root, &path, cx); - self.selected = Some((repo_root, path)); - cx.notify(); } fn on_repo_base_changed(&mut self, repo_root: PathBuf, branch: String, cx: &mut Context) { + // Diffs of this repo were computed against the old base. + self.file_diffs.retain(|(root, _), _| root != &repo_root); + self.in_flight = None; + self.base_overrides.insert(repo_root, branch.clone()); // Remember this as the global default for future repos/sessions. self.default_base = Some(branch.clone()); crate::update_ui_settings(cx, |s| s.review_default_base = Some(branch)); - self.selected = None; self.request_listing(cx); cx.notify(); } @@ -330,8 +329,9 @@ impl ReviewView { && *mode != self.mode { self.mode = *mode; - // Selecting a new mode invalidates the current diff selection. - self.selected = None; + // A new mode invalidates every prepared diff. + self.file_diffs.clear(); + self.in_flight = None; self.persist_mode(cx); self.request_listing(cx); cx.notify(); @@ -348,14 +348,15 @@ impl ReviewView { return; }; self.listing_generation = generation; - self.last_listing = listing.clone(); let Some(listing) = listing else { // Cleared (e.g. session change): drop all sections. + self.has_listing = false; self.repos.clear(); return; }; + self.has_listing = true; self.is_git_repo = listing.is_git_repo; self.mode = listing.mode; @@ -365,7 +366,7 @@ impl ReviewView { }); // Rebuild sections when the set of repos changes; otherwise update the - // existing sections in place (preserving expansion / selection). + // existing sections in place (preserving expansion state). let incoming_roots: Vec = listing.repos.iter().map(|r| r.repo_root.clone()).collect(); let current_roots: Vec = self.repos.iter().map(|r| r.repo_root.clone()).collect(); @@ -382,6 +383,25 @@ impl ReviewView { } } + // Drop prepared diffs and collapse state of files that vanished from + // the listing. + let live: HashSet = self + .repos + .iter() + .flat_map(|s| { + s.files + .iter() + .map(|f| (s.repo_root.clone(), f.path.clone())) + }) + .collect(); + self.file_diffs.retain(|key, _| live.contains(key)); + self.collapsed_files.retain(|key| live.contains(key)); + if let Some(in_flight) = &self.in_flight + && !live.contains(in_flight) + { + self.in_flight = None; + } + // Apply the persisted default base to any repo that has no explicit // override yet and whose resolved base differs. Seeding the override // and re-requesting makes the default actually take effect. This @@ -404,43 +424,39 @@ impl ReviewView { } } - // Reconcile the selection against the fresh listing. - let still_present = self.selected.as_ref().is_some_and(|(root, path)| { - listing - .repos - .iter() - .any(|r| &r.repo_root == root && r.files.iter().any(|f| &f.path == path)) - }); - if !still_present { - self.selected = None; - } - let selected = self.selected.clone(); - for section in &self.repos { - let sel = selected - .as_ref() - .filter(|(root, _)| root == §ion.repo_root) - .map(|(_, path)| path.clone()); - section.tree.update(cx, |t, cx| t.set_selected(sel, cx)); + self.ensure_diff_request(cx); + } + + /// Consume the latest diff arrival from the global if it changed. Only the + /// response to the outstanding request is accepted; the hunks are computed + /// once here, then the next missing diff is requested. + fn sync_diff(&mut self, cx: &mut Context) { + let Some((generation, diff)) = cx + .try_global::() + .and_then(|g| g.review_diff_if_newer(self.diff_generation)) + else { + return; + }; + self.diff_generation = generation; + + if let Some(d) = diff { + let key = (d.repo_root, d.path); + if self.in_flight.as_ref() == Some(&key) { + self.in_flight = None; + self.file_diffs.insert(key, d.prepared); + } } + self.ensure_diff_request(cx); } - /// Create a fresh [`RepoSection`] for `data`, wiring per-repo subscriptions - /// that capture the repo root so events identify their origin. + /// Create a fresh [`RepoSection`] for `data`, wiring the base-selector + /// subscription so events identify their repo. fn build_section( &self, - data: &crate::RepoReviewData, + data: &RepoReviewData, window: &mut Window, cx: &mut Context, ) -> RepoSection { - let tree = cx.new(ChangedFilesTree::new); - tree.update(cx, |t, cx| t.set_files(&data.files, cx)); - - let root_for_tree = data.repo_root.clone(); - let tree_sub = cx.subscribe_in(&tree, window, move |this, _tree, event, _window, cx| { - let ChangedFilesTreeEvent::FileSelected(path) = event; - this.on_file_selected(root_for_tree.clone(), path.clone(), cx); - }); - let items: Vec = data .base_candidates .iter() @@ -469,33 +485,28 @@ impl ReviewView { RepoSection { repo_root: data.repo_root.clone(), label: data.label.clone(), - tree, base_state, base_candidates: data.base_candidates.clone(), base: effective_base, - has_files: !data.files.is_empty(), + files: data.files.clone(), stats: data.stats, scan_state: data.scan_state, collapsed: false, - _tree_sub: tree_sub, _base_sub: base_sub, } } - /// Update an existing section's tree + base selector in place. + /// Update an existing section's data + base selector in place. fn update_section( section: &mut RepoSection, - data: &crate::RepoReviewData, + data: &RepoReviewData, window: &mut Window, cx: &mut Context, ) { section.label = data.label.clone(); - section.has_files = !data.files.is_empty(); + section.files = data.files.clone(); section.stats = data.stats; section.scan_state = data.scan_state; - section - .tree - .update(cx, |t, cx| t.set_files(&data.files, cx)); if section.base_candidates != data.base_candidates { section.base_candidates = data.base_candidates.clone(); @@ -518,7 +529,7 @@ impl ReviewView { /// The base ref to preselect for a repo: an explicit session override, else /// the persisted default (when it is a candidate), else the resolved base. - fn effective_base(&self, data: &crate::RepoReviewData) -> Option { + fn effective_base(&self, data: &RepoReviewData) -> Option { if let Some(base) = self.base_overrides.get(&data.repo_root) { return Some(base.clone()); } @@ -530,110 +541,6 @@ impl ReviewView { data.base.clone() } - /// Consume the latest diff from the global if it changed, computing the - /// line diff exactly once. Rendering then reuses the prepared lines. - fn sync_diff(&mut self, cx: &mut Context) { - let Some((generation, diff)) = cx - .try_global::() - .and_then(|g| g.review_diff_if_newer(self.diff_generation)) - else { - return; - }; - self.diff_generation = generation; - self.prepared_diff = diff.map(|d| { - let lines = if d.diff.is_binary || d.diff.too_large { - Vec::new() - } else { - let old = d.diff.old_text.as_deref().unwrap_or_default(); - let new = d.diff.new_text.as_deref().unwrap_or_default(); - if old.is_empty() && new.is_empty() { - Vec::new() - } else { - compute_diff_lines(old, new) - } - }; - PreparedDiff { - repo_root: d.repo_root, - path: d.path, - is_binary: d.diff.is_binary, - too_large: d.diff.too_large, - lines, - } - }); - } - - fn render_diff_pane(&self, window: &mut Window, cx: &mut Context) -> gpui::AnyElement { - let muted = cx.theme().muted_foreground; - - let placeholder = |msg: &str| { - div() - .size_full() - .flex() - .items_center() - .justify_center() - .p_4() - .text_sm() - .text_color(muted) - .child(msg.to_string()) - .into_any_element() - }; - - let Some((sel_root, sel_path)) = self.selected.as_ref() else { - return placeholder("Select a file to view its diff"); - }; - - let Some(prepared) = self - .prepared_diff - .as_ref() - .filter(|p| &p.repo_root == sel_root && &p.path == sel_path) - else { - return placeholder("Loading diff…"); - }; - - if prepared.is_binary { - return placeholder("Binary file — no text diff"); - } - if prepared.too_large { - return placeholder("File too large to display"); - } - if prepared.lines.is_empty() { - return placeholder("No changes to display"); - } - - let rem_size = window.rem_size(); - let theme = cx.theme(); - // Match the chat diff card's body styling so the unified diff (with its - // red/green row backgrounds) renders identically here. - let is_dark = theme.background.l < 0.5; - let body_bg = if is_dark { - gpui::hsla(0.0, 0.0, 0.08, 1.0) - } else { - gpui::hsla(0.0, 0.0, 0.97, 1.0) - }; - let line_height_px = rems(1.25).to_pixels(rem_size).round(); - let diff = render_diff_lines(&prepared.lines, theme, Some(1), rem_size); - - div() - .id("review-diff-scroll") - .size_full() - .overflow_scroll() - .track_scroll(&self.diff_scroll) - .child( - div() - .w_full() - .py_1() - .bg(body_bg) - .flex() - .flex_col() - .text_size(rems(0.78125)) - .line_height(line_height_px) - .font_family("Menlo") - .font_weight(FontWeight(400.0)) - .child(diff), - ) - .into_any_element() - } - /// The rotating double-arrow used on active sessions, in grey — shown on /// the repo whose scan is currently running. `id` keys the animation. fn scan_spinner(id: impl std::fmt::Display, muted: gpui::Hsla) -> gpui::AnyElement { @@ -661,8 +568,8 @@ impl ReviewView { .into_any_element() } - /// The header's right-hand slot: a spinner while a repo is being scanned, - /// a faded static one while it waits its turn, and a `+adds −dels` + /// The repo header's right-hand slot: a spinner while a repo is being + /// scanned, a faded static one while it waits its turn, and a `+adds −dels` /// summary once its (possibly cached) result is in. fn render_scan_indicator(&self, section: &RepoSection, cx: &Context) -> gpui::AnyElement { let theme = cx.theme(); @@ -673,7 +580,7 @@ impl ReviewView { } let pending = matches!(section.scan_state, ReviewScanState::Pending); - let has_data = section.has_files || section.stats != git::DiffStats::default(); + let has_data = !section.files.is_empty() || section.stats != git::DiffStats::default(); if !has_data { // Nothing (yet) to summarize: a queued repo shows a wait marker, // a scanned clean repo shows no indicator at all. @@ -705,8 +612,165 @@ impl ReviewView { .into_any_element() } - /// Render the right column: a scrollable stack of per-repo sections. - fn render_tree_column(&self, cx: &mut Context) -> gpui::AnyElement { + /// One stacked file: collapsible header (icon, path, status, `+/−`) with + /// the file's diff hunks directly below. + fn render_file_entry( + &self, + repo_root: &std::path::Path, + file: &ChangedFile, + window: &Window, + cx: &mut Context, + ) -> gpui::AnyElement { + let theme = cx.theme(); + let muted = theme.muted_foreground; + let fg = theme.foreground; + let border = theme.border; + + let key: FileKey = (repo_root.to_path_buf(), file.path.clone()); + let collapsed = self.collapsed_files.contains(&key); + let entry = self.file_diffs.get(&key); + let loading = self.in_flight.as_ref() == Some(&key); + + // Right-hand slot of the file header. + let indicator: gpui::AnyElement = match entry { + Some(e) if e.is_binary => div() + .text_xs() + .text_color(muted) + .child("binary") + .into_any_element(), + Some(e) if e.too_large => div() + .text_xs() + .text_color(muted) + .child("too large") + .into_any_element(), + Some(e) => div() + .flex() + .flex_row() + .items_center() + .gap_1() + .text_xs() + .child( + div() + .text_color(added_row_colors(theme).1) + .child(format!("+{}", e.additions)), + ) + .child( + div() + .text_color(deleted_row_colors(theme).1) + .child(format!("−{}", e.deletions)), + ) + .into_any_element(), + None if loading => { + Self::scan_spinner(format!("{}:{}", repo_root.display(), file.path), muted) + } + None => Self::pending_marker(muted), + }; + + let chevron = if collapsed { + "icons/chevron_right.svg" + } else { + "icons/chevron_down.svg" + }; + let (status_letter, status_color) = status_badge(file.status); + let file_name = file.path.rsplit('/').next().unwrap_or(&file.path); + let icon = file_icons::get().get_icon_for_filename(file_name); + + let toggle_key = key.clone(); + let header = div() + .id(gpui::SharedString::from(format!( + "review-file-{}:{}", + repo_root.display(), + file.path + ))) + .flex() + .flex_row() + .items_center() + .gap_1p5() + .pl_3() + .pr_2() + .py_0p5() + .border_t_1() + .border_color(border) + .cursor_pointer() + .hover(|s| s.bg(theme.muted)) + .child(gpui::svg().size(px(10.)).path(chevron).text_color(muted)) + .child(file_icons::render_icon(&icon, 14.0, muted, "📄")) + .child( + div() + .flex_1() + .min_w_0() + .overflow_hidden() + .text_ellipsis() + .text_xs() + .text_color(fg) + .child(file.path.clone()), + ) + .child( + div() + .text_xs() + .font_weight(FontWeight::MEDIUM) + .text_color(status_color) + .child(status_letter), + ) + .child(indicator) + .on_click(cx.listener(move |this, _ev, _window, cx| { + if !this.collapsed_files.remove(&toggle_key) { + this.collapsed_files.insert(toggle_key.clone()); + } + // Expanding may unlock a diff that was skipped while collapsed. + this.ensure_diff_request(cx); + cx.notify(); + })); + + let mut container = v_flex().w_full().child(header); + + if !collapsed && let Some(entry) = entry { + let body: Option = if entry.is_binary || entry.too_large { + None // The header badge already says why there is no diff. + } else if entry.hunks.is_empty() { + Some( + div() + .px_3() + .py_1() + .text_xs() + .text_color(muted) + .child("No content changes") + .into_any_element(), + ) + } else { + let rem_size = window.rem_size(); + let is_dark = theme.background.l < 0.5; + let body_bg = if is_dark { + gpui::hsla(0.0, 0.0, 0.08, 1.0) + } else { + gpui::hsla(0.0, 0.0, 0.97, 1.0) + }; + let line_height_px = rems(1.25).to_pixels(rem_size).round(); + Some( + div() + .w_full() + .py_1() + .bg(body_bg) + .flex() + .flex_col() + .text_size(rems(0.78125)) + .line_height(line_height_px) + .font_family("Menlo") + .font_weight(FontWeight(400.0)) + .child(render_diff_hunks(&entry.hunks, theme, rem_size)) + .into_any_element(), + ) + }; + if let Some(body) = body { + container = container.child(body); + } + } + + container.into_any_element() + } + + /// The scrollable stack of per-repo sections with their stacked files. + fn render_sections(&mut self, window: &Window, cx: &mut Context) -> gpui::AnyElement { let muted = cx.theme().muted_foreground; let fg = cx.theme().foreground; let border = cx.theme().border; @@ -714,9 +778,20 @@ impl ReviewView { let mut column = v_flex().size_full().overflow_y_scrollbar(); - for (ix, section) in self.repos.iter().enumerate() { - let repo_root = section.repo_root.clone(); - let collapsed = section.collapsed; + // Snapshot the per-section data needed while building children, so the + // listener closures (which borrow `this`) don't fight the loop borrow. + let section_count = self.repos.len(); + for ix in 0..section_count { + let (repo_root, label, collapsed, scan_state) = { + let s = &self.repos[ix]; + ( + s.repo_root.clone(), + s.label.clone(), + s.collapsed, + s.scan_state, + ) + }; + let _ = scan_state; // Sections are separated by a line ABOVE each section (not by a // line between a section's header and its content). @@ -729,6 +804,7 @@ impl ReviewView { } else { "icons/chevron_down.svg" }; + let toggle_root = repo_root.clone(); let header = div() .id(gpui::SharedString::from(format!("repo-header-{ix}"))) .flex() @@ -744,14 +820,16 @@ impl ReviewView { div() .flex_1() .text_sm() - .font_weight(gpui::FontWeight::MEDIUM) + .font_weight(FontWeight::MEDIUM) .text_color(fg) - .child(section.label.clone()), + .child(label), ) - .child(self.render_scan_indicator(section, cx)) + .child(self.render_scan_indicator(&self.repos[ix], cx)) .on_click(cx.listener(move |this, _ev, _window, cx| { - if let Some(s) = this.repos.iter_mut().find(|s| s.repo_root == repo_root) { + if let Some(s) = this.repos.iter_mut().find(|s| s.repo_root == toggle_root) { s.collapsed = !s.collapsed; + // Expanding may unlock diffs skipped while collapsed. + this.ensure_diff_request(cx); cx.notify(); } })); @@ -761,7 +839,7 @@ impl ReviewView { if branch_mode { section_el = section_el.child( div().px_2().py_1().child( - Select::new(§ion.base_state) + Select::new(&self.repos[ix].base_state) .placeholder("Base") .with_size(Size::XSmall) .icon( @@ -775,9 +853,11 @@ impl ReviewView { ); } // A repo without changes shows just its header — the missing - // +/− badge already says "clean", so no placeholder text. - if section.has_files { - section_el = section_el.child(section.tree.clone()); + // +/− badge already says "clean". + let files = self.repos[ix].files.clone(); + for file in &files { + section_el = + section_el.child(self.render_file_entry(&repo_root, file, window, cx)); } } @@ -806,7 +886,7 @@ impl Render for ReviewView { // Before the first (fast) discovery response there is nothing to lay // out yet — show explicit activity instead of an empty panel. - if self.last_listing.is_none() { + if !self.has_listing { return v_flex() .size_full() .items_center() @@ -854,32 +934,7 @@ impl Render for ReviewView { .min_w(px(130.)), ); - // Two-column body: diff (LEFT, grows) | tree column (RIGHT, sized). - let diff_pane = self.render_diff_pane(window, cx); - let tree_column = self.render_tree_column(cx); - - let body = h_resizable("review-split") - .with_state(&self.split_state) - .on_resize(|state, _window, cx| { - if let Some(width) = state.read(cx).sizes().get(1).copied() { - let w = f32::from(width); - crate::update_ui_settings(cx, |s| s.review_tree_width = Some(w)); - } - }) - .child(resizable_panel().child(diff_pane)) - .child( - resizable_panel() - .size(px(self.tree_width)) - .size_range(px(180.)..px(600.)) - .flex_none() - .child( - div() - .size_full() - .border_l_1() - .border_color(border) - .child(tree_column), - ), - ); + let body = self.render_sections(window, cx); v_flex() .size_full() diff --git a/crates/ui_gpui/src/shared/file_tree.rs b/crates/ui_gpui/src/shared/file_tree.rs deleted file mode 100644 index b1a641d3..00000000 --- a/crates/ui_gpui/src/shared/file_tree.rs +++ /dev/null @@ -1,431 +0,0 @@ -//! Nested, collapsible tree of changed files for the Review panel. -//! -//! [`build_tree`] is a pure function (unit-tested) that turns a flat list of -//! [`git::ChangedFile`]s into a nested [`TreeNode`] structure. [`ChangedFilesTree`] -//! is the GPUI entity that renders it, tracks expansion/selection state, and -//! emits [`ChangedFilesTreeEvent::FileSelected`] when the user clicks a file. - -use crate::shared::file_icons; -use git::{ChangeStatus, ChangedFile}; -use gpui::{ - Context, EventEmitter, FocusHandle, Focusable, Render, Window, div, prelude::*, px, svg, -}; -use gpui_component::ActiveTheme; -use std::collections::HashSet; - -/// A node in the changed-files tree: either a directory (with children) or a -/// leaf file carrying its change status. -#[derive(Debug, Clone, PartialEq)] -pub enum TreeNode { - Dir { - /// Last path segment (display name). - name: String, - /// Full relative path from the repo root (unique key). - path: String, - children: Vec, - }, - File { - name: String, - path: String, - status: ChangeStatus, - }, -} - -/// Build a nested tree from a flat list of changed files. -/// -/// Paths are split on `/`; files sharing a directory prefix are merged under a -/// single directory node. Siblings are sorted directories-first, then -/// case-insensitively by name. -pub fn build_tree(files: &[ChangedFile]) -> Vec { - let mut roots: Vec = Vec::new(); - for f in files { - insert_path(&mut roots, &f.path, f.status, ""); - } - sort_nodes(&mut roots); - roots -} - -fn insert_path(nodes: &mut Vec, rel: &str, status: ChangeStatus, prefix: &str) { - let mut parts = rel.splitn(2, '/'); - let head = match parts.next() { - Some(h) if !h.is_empty() => h, - // Empty or leading-slash segment: skip it. - _ => return, - }; - let rest = parts.next(); - let full = if prefix.is_empty() { - head.to_string() - } else { - format!("{prefix}/{head}") - }; - - match rest { - None => { - nodes.push(TreeNode::File { - name: head.to_string(), - path: full, - status, - }); - } - Some(rest) => { - let idx = nodes - .iter() - .position(|n| matches!(n, TreeNode::Dir { name, .. } if name == head)); - let idx = match idx { - Some(i) => i, - None => { - nodes.push(TreeNode::Dir { - name: head.to_string(), - path: full.clone(), - children: Vec::new(), - }); - nodes.len() - 1 - } - }; - if let TreeNode::Dir { children, .. } = &mut nodes[idx] { - insert_path(children, rest, status, &full); - } - } - } -} - -fn node_name(node: &TreeNode) -> &str { - match node { - TreeNode::Dir { name, .. } | TreeNode::File { name, .. } => name, - } -} - -fn sort_nodes(nodes: &mut [TreeNode]) { - nodes.sort_by(|a, b| { - // Directories sort before files. - let rank = |n: &TreeNode| matches!(n, TreeNode::File { .. }) as u8; - rank(a).cmp(&rank(b)).then_with(|| { - node_name(a) - .to_lowercase() - .cmp(&node_name(b).to_lowercase()) - }) - }); - for n in nodes.iter_mut() { - if let TreeNode::Dir { children, .. } = n { - sort_nodes(children); - } - } -} - -// --------------------------------------------------------------------------- -// Entity -// --------------------------------------------------------------------------- - -/// Events emitted by [`ChangedFilesTree`]. -#[derive(Clone, Debug)] -pub enum ChangedFilesTreeEvent { - /// The user selected a file. Carries the file's repo-relative path. - FileSelected(String), -} - -/// A flattened, renderable row (computed each render from the tree + expansion). -struct Row { - depth: usize, - is_dir: bool, - name: String, - path: String, - status: Option, - expanded: bool, -} - -/// Nested collapsible tree of changed files. -pub struct ChangedFilesTree { - nodes: Vec, - /// Full paths of directories that are currently expanded. - expanded: HashSet, - /// Currently selected file path. - selected: Option, - focus_handle: FocusHandle, -} - -impl EventEmitter for ChangedFilesTree {} - -impl ChangedFilesTree { - pub fn new(cx: &mut Context) -> Self { - Self { - nodes: Vec::new(), - expanded: HashSet::new(), - selected: None, - focus_handle: cx.focus_handle(), - } - } - - /// Replace the file list, rebuilding the tree. All directories start - /// expanded. Preserves the current selection if it still exists. - pub fn set_files(&mut self, files: &[ChangedFile], cx: &mut Context) { - self.nodes = build_tree(files); - self.expanded.clear(); - collect_dir_paths(&self.nodes, &mut self.expanded); - // Drop selection if the selected file is gone. - if let Some(sel) = &self.selected - && !files.iter().any(|f| &f.path == sel) - { - self.selected = None; - } - cx.notify(); - } - - /// Set the selected file path (without emitting an event). - pub fn set_selected(&mut self, path: Option, cx: &mut Context) { - self.selected = path; - cx.notify(); - } - - /// The currently selected file path, if any. - pub fn selected(&self) -> Option<&str> { - self.selected.as_deref() - } - - fn toggle_dir(&mut self, path: &str, cx: &mut Context) { - if !self.expanded.remove(path) { - self.expanded.insert(path.to_string()); - } - cx.notify(); - } - - fn on_file_click(&mut self, path: String, cx: &mut Context) { - self.selected = Some(path.clone()); - cx.notify(); - cx.emit(ChangedFilesTreeEvent::FileSelected(path)); - } - - /// Walk the tree honoring expansion state, producing a flat list of rows. - fn visible_rows(&self) -> Vec { - let mut rows = Vec::new(); - self.push_rows(&self.nodes, 0, &mut rows); - rows - } - - fn push_rows(&self, nodes: &[TreeNode], depth: usize, out: &mut Vec) { - for node in nodes { - match node { - TreeNode::Dir { - name, - path, - children, - } => { - let expanded = self.expanded.contains(path); - out.push(Row { - depth, - is_dir: true, - name: name.clone(), - path: path.clone(), - status: None, - expanded, - }); - if expanded { - self.push_rows(children, depth + 1, out); - } - } - TreeNode::File { name, path, status } => { - out.push(Row { - depth, - is_dir: false, - name: name.clone(), - path: path.clone(), - status: Some(*status), - expanded: false, - }); - } - } - } - } -} - -/// Recursively collect the full paths of all directory nodes. -fn collect_dir_paths(nodes: &[TreeNode], out: &mut HashSet) { - for n in nodes { - if let TreeNode::Dir { path, children, .. } = n { - out.insert(path.clone()); - collect_dir_paths(children, out); - } - } -} - -/// Single-letter badge and color for a change status. -fn status_badge(status: ChangeStatus) -> (&'static str, gpui::Hsla) { - match status { - ChangeStatus::Added | ChangeStatus::Untracked => ("A", gpui::rgb(0x3f_a5_5a).into()), - ChangeStatus::Modified => ("M", gpui::rgb(0xc7_9a_3a).into()), - ChangeStatus::Deleted => ("D", gpui::rgb(0xc7_4a_4a).into()), - ChangeStatus::Renamed => ("R", gpui::rgb(0x4a_82_c7).into()), - ChangeStatus::Copied => ("C", gpui::rgb(0x4a_82_c7).into()), - ChangeStatus::TypeChanged => ("T", gpui::rgb(0x8a_6a_c7).into()), - } -} - -impl Focusable for ChangedFilesTree { - fn focus_handle(&self, _cx: &gpui::App) -> FocusHandle { - self.focus_handle.clone() - } -} - -impl Render for ChangedFilesTree { - fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { - let rows = self.visible_rows(); - let muted = cx.theme().muted_foreground; - let fg = cx.theme().foreground; - let accent = cx.theme().accent; - let selected = self.selected.clone(); - - // An empty tree renders nothing — callers hide the tree for clean - // repos, and a repo's missing +/− badge already communicates "clean". - if rows.is_empty() { - return div().into_any_element(); - } - - div() - .flex() - .flex_col() - .children(rows.into_iter().map(|row| { - let indent = px(8.0 + row.depth as f32 * 12.0); - let is_selected = !row.is_dir && selected.as_deref() == Some(row.path.as_str()); - let row_path = row.path.clone(); - - let mut container = div() - .id(gpui::SharedString::from(format!("tree-{}", row.path))) - .flex() - .flex_row() - .items_center() - .gap_1p5() - .pl(indent) - .pr_2() - .py_0p5() - .text_sm() - .cursor_pointer() - .hover(|s| s.bg(cx.theme().muted)); - - if is_selected { - container = container.bg(accent); - } - - // Leading glyph: chevron + folder for dirs, spacer + file icon for files. - if row.is_dir { - let chevron = if row.expanded { - "icons/chevron_down.svg" - } else { - "icons/chevron_right.svg" - }; - let folder = if row.expanded { - "icons/file_icons/folder_open.svg" - } else { - "icons/file_icons/folder.svg" - }; - container = container - .child(svg().size(px(12.)).path(chevron).text_color(muted)) - .child(svg().size(px(14.)).path(folder).text_color(muted)) - .child(div().text_color(fg).child(row.name.clone())); - } else { - let (badge, badge_color) = row.status.map(status_badge).unwrap_or((" ", muted)); - let icon = file_icons::get().get_icon_for_filename(&row.name); - container = container - // Align file rows under the folder glyph (skip chevron slot). - .child(div().size(px(12.))) - .child(file_icons::render_icon(&icon, 14.0, muted, "📄")) - .child(div().flex_1().text_color(fg).child(row.name.clone())) - .child( - div() - .w(px(14.)) - .flex() - .justify_center() - .text_color(badge_color) - .child(badge), - ); - } - - let is_dir = row.is_dir; - container.on_click(cx.listener(move |this, _ev, _window, cx| { - if is_dir { - this.toggle_dir(&row_path, cx); - } else { - this.on_file_click(row_path.clone(), cx); - } - })) - })) - .into_any_element() - } -} - -#[cfg(test)] -mod tests { - use super::*; - - fn file(path: &str, status: ChangeStatus) -> ChangedFile { - ChangedFile { - path: path.to_string(), - orig_path: None, - status, - } - } - - #[test] - fn build_tree_nests_and_sorts_dirs_first() { - let files = vec![ - file("z.rs", ChangeStatus::Modified), - file("a/c.rs", ChangeStatus::Added), - file("a/b.rs", ChangeStatus::Modified), - file("a/sub/d.rs", ChangeStatus::Deleted), - ]; - let tree = build_tree(&files); - - // Root: dir "a" first, then file "z.rs". - assert_eq!(tree.len(), 2); - match &tree[0] { - TreeNode::Dir { - name, - path, - children, - } => { - assert_eq!(name, "a"); - assert_eq!(path, "a"); - // Inside "a": dir "sub" first, then files b.rs, c.rs (sorted). - assert_eq!(children.len(), 3); - assert!(matches!(&children[0], TreeNode::Dir { name, .. } if name == "sub")); - assert!(matches!(&children[1], TreeNode::File { name, .. } if name == "b.rs")); - assert!(matches!(&children[2], TreeNode::File { name, .. } if name == "c.rs")); - // Nested file path is fully qualified. - if let TreeNode::Dir { children: sub, .. } = &children[0] { - assert!(matches!(&sub[0], TreeNode::File { path, .. } if path == "a/sub/d.rs")); - } - } - other => panic!("expected dir 'a', got {other:?}"), - } - assert!(matches!(&tree[1], TreeNode::File { name, .. } if name == "z.rs")); - } - - #[test] - fn build_tree_merges_shared_prefix() { - let files = vec![ - file("src/main.rs", ChangeStatus::Modified), - file("src/lib.rs", ChangeStatus::Modified), - ]; - let tree = build_tree(&files); - assert_eq!(tree.len(), 1); - match &tree[0] { - TreeNode::Dir { name, children, .. } => { - assert_eq!(name, "src"); - assert_eq!(children.len(), 2); - } - other => panic!("expected single 'src' dir, got {other:?}"), - } - } - - #[test] - fn collect_dir_paths_gathers_all_dirs() { - let files = vec![ - file("a/b/c.rs", ChangeStatus::Modified), - file("d.rs", ChangeStatus::Added), - ]; - let tree = build_tree(&files); - let mut dirs = HashSet::new(); - collect_dir_paths(&tree, &mut dirs); - assert!(dirs.contains("a")); - assert!(dirs.contains("a/b")); - assert_eq!(dirs.len(), 2); - } -} diff --git a/crates/ui_gpui/src/shared/mod.rs b/crates/ui_gpui/src/shared/mod.rs index 213680b2..94e9134b 100644 --- a/crates/ui_gpui/src/shared/mod.rs +++ b/crates/ui_gpui/src/shared/mod.rs @@ -3,7 +3,6 @@ pub mod auto_scroll; pub mod context_breakdown; pub mod context_indicator; pub mod file_icons; -pub mod file_tree; pub mod image; pub mod plan_banner; pub mod review_cache; diff --git a/crates/ui_gpui/src/shared/settings.rs b/crates/ui_gpui/src/shared/settings.rs index a6fb6089..cb1483d7 100644 --- a/crates/ui_gpui/src/shared/settings.rs +++ b/crates/ui_gpui/src/shared/settings.rs @@ -77,10 +77,6 @@ pub struct UiSettings { #[serde(default, skip_serializing_if = "Option::is_none")] pub right_sidebar_width: Option, - /// Persisted width (px) of the Review panel's file-tree column. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub review_tree_width: Option, - /// Persisted default base ref for "Branch vs base" review mode; seeds each /// repo's per-session base override. #[serde(default, skip_serializing_if = "Option::is_none")] @@ -103,7 +99,6 @@ impl Default for UiSettings { window_bounds: None, default_model: None, right_sidebar_width: None, - review_tree_width: None, review_default_base: None, } } diff --git a/crates/ui_gpui/src/tool_cards/diff_card.rs b/crates/ui_gpui/src/tool_cards/diff_card.rs index 678c7590..df7720fe 100644 --- a/crates/ui_gpui/src/tool_cards/diff_card.rs +++ b/crates/ui_gpui/src/tool_cards/diff_card.rs @@ -539,7 +539,8 @@ fn normalize_for_diff(text: &str) -> String { /// One line of a computed unified diff. `text` is a [`SharedString`] so cached /// diffs can be re-rendered every frame with cheap clones. -pub(crate) struct DiffLine { +#[derive(Debug, Clone)] +pub struct DiffLine { pub tag: ChangeTag, pub text: SharedString, } @@ -564,6 +565,107 @@ pub(crate) fn compute_diff_lines(old_text: &str, new_text: &str) -> Vec, +} + +/// Like [`compute_diff_lines`], but grouped into hunks with `context` lines +/// of surrounding context (à la `git diff`) — unchanged stretches between +/// hunks are dropped entirely, which keeps the element count proportional to +/// the *changed* lines instead of the file size. +pub fn compute_diff_hunks(old_text: &str, new_text: &str, context: usize) -> Vec { + let old_norm = normalize_for_diff(old_text); + let new_norm = normalize_for_diff(new_text); + + let diff = TextDiff::configure() + .newline_terminated(true) + .diff_lines(&old_norm, &new_norm); + + diff.grouped_ops(context) + .iter() + .map(|ops| DiffHunk { + new_start: ops.first().map(|op| op.new_range().start + 1).unwrap_or(1), + lines: ops + .iter() + .flat_map(|op| diff.iter_changes(op)) + .map(|change| DiffLine { + tag: change.tag(), + text: change.value().trim_end().to_string().into(), + }) + .collect(), + }) + .collect() +} + +/// A whole file as one one-sided hunk (pure add or pure delete). No diff +/// computation — diffing against an empty side would only produce a phantom +/// deleted/inserted blank line (`normalize_for_diff` maps "" to "\n"). +pub fn single_sided_hunk(text: &str, tag: ChangeTag) -> Vec { + let norm = normalize_for_diff(text); + let lines: Vec = norm + .lines() + .map(|l| DiffLine { + tag, + text: l.trim_end().to_string().into(), + }) + .collect(); + if lines.is_empty() { + return Vec::new(); + } + vec![DiffHunk { + new_start: 1, + lines, + }] +} + +/// Render already-computed hunks with real new-file line numbers, a shared +/// gutter width, and a slim "⋯" separator between hunks. +pub(crate) fn render_diff_hunks( + hunks: &[DiffHunk], + theme: &gpui_component::theme::Theme, + rem_size: gpui::Pixels, +) -> gpui::AnyElement { + let max_line = hunks + .iter() + .map(|h| { + h.new_start + + h.lines + .iter() + .filter(|l| l.tag != ChangeTag::Delete) + .count() + }) + .max() + .unwrap_or(1); + let gutter_width = max_line.to_string().len(); + + let mut column = div().flex().flex_col(); + for (ix, hunk) in hunks.iter().enumerate() { + if ix > 0 { + let (_, ctx_color) = unchanged_row_colors(theme); + column = column.child( + div() + .w_full() + .flex() + .justify_center() + .text_color(ctx_color.opacity(0.5)) + .child("⋯"), + ); + } + column = column.child(render_diff_rows( + &hunk.lines, + theme, + Some(hunk.new_start), + gutter_width, + rem_size, + )); + } + column.into_any() +} + /// Compute and render a unified diff in one go. For per-frame rendering of /// unchanged content, prefer caching [`compute_diff_lines`]'s result and /// calling [`render_diff_lines`] instead. @@ -600,7 +702,18 @@ pub(crate) fn render_diff_lines( } else { 0 }; + render_diff_rows(diff_lines, theme, start_line, gutter_width, rem_size) +} +/// Shared row builder: renders diff rows with numbering from `start_line` +/// (when given) into a fixed `gutter_width`-digit gutter. +fn render_diff_rows( + diff_lines: &[DiffLine], + theme: &gpui_component::theme::Theme, + start_line: Option, + gutter_width: usize, + rem_size: gpui::Pixels, +) -> gpui::AnyElement { // Track both old and new line numbers let mut old_line_num = start_line.unwrap_or(1); let mut new_line_num = start_line.unwrap_or(1); @@ -949,6 +1062,48 @@ pub(crate) fn unchanged_row_colors( mod tests { use super::*; + #[test] + fn compute_diff_hunks_groups_changes_with_context() { + let old: String = (1..=20).map(|i| format!("line {i}\n")).collect(); + let mut new_lines: Vec = (1..=20).map(|i| format!("line {i}\n")).collect(); + new_lines[2] = "changed 3\n".into(); + new_lines[15] = "changed 16\n".into(); + let new: String = new_lines.concat(); + + let hunks = compute_diff_hunks(&old, &new, 3); + assert_eq!(hunks.len(), 2, "two distant changes → two hunks"); + assert_eq!(hunks[0].new_start, 1); + assert_eq!(hunks[1].new_start, 13); + for hunk in &hunks { + let deletes = hunk + .lines + .iter() + .filter(|l| l.tag == ChangeTag::Delete) + .count(); + let inserts = hunk + .lines + .iter() + .filter(|l| l.tag == ChangeTag::Insert) + .count(); + let equals = hunk + .lines + .iter() + .filter(|l| l.tag == ChangeTag::Equal) + .count(); + assert_eq!((deletes, inserts), (1, 1)); + assert!(equals <= 6, "at most 3 context lines per side"); + } + } + + #[test] + fn single_sided_hunk_is_one_pure_hunk() { + let hunks = single_sided_hunk("a\nb\nc\n", ChangeTag::Insert); + assert_eq!(hunks.len(), 1); + assert_eq!(hunks[0].new_start, 1); + assert!(hunks[0].lines.iter().all(|l| l.tag == ChangeTag::Insert)); + assert_eq!(hunks[0].lines.len(), 3); + } + #[test] fn test_parse_single_section() { let diff = "<<<<<<< SEARCH\nold line\n=======\nnew line\n>>>>>>> REPLACE"; From cab0bef74f3f289cf044a7512bb7767e1b3fd158 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Sun, 6 Sep 2026 11:45:28 +0200 Subject: [PATCH 09/10] feat(ui_gpui): word-level diff emphasis and persisted repo-section collapse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Diff rows in the chat tool cards and the review sidebar now highlight the words that changed within a line: collect_change_lines uses similar's iter_inline_changes (unicode word segmentation enabled) and records emphasis byte ranges per DiffLine; rendering layers a stronger add/delete tint over those ranges via StyledText highlights, so the emphasis wraps with the text. Replace blocks over 16 lines skip the word diff — pairing lines across big rewrites is noise — and similar itself falls back to plain lines when the block similarity is too low. - Review repo sections now default to collapsed; expanding one is persisted in ui-settings.json as review_expanded_repos, keyed by absolute repo root (naturally per project). Combined with the lazy pipeline, a freshly opened panel loads no diffs until a section is expanded. --- crates/ui_gpui/Cargo.toml | 2 +- .../main_screen/right_panel/review_view.rs | 35 ++++- crates/ui_gpui/src/shared/settings.rs | 6 + crates/ui_gpui/src/tool_cards/diff_card.rs | 138 +++++++++++++++--- 4 files changed, 154 insertions(+), 27 deletions(-) diff --git a/crates/ui_gpui/Cargo.toml b/crates/ui_gpui/Cargo.toml index 18b68819..4af05aac 100644 --- a/crates/ui_gpui/Cargo.toml +++ b/crates/ui_gpui/Cargo.toml @@ -37,7 +37,7 @@ dirs = "5.0" open = "5" # Diff visualization -similar = { version = "2.7.0", features = ["inline"] } +similar = { version = "2.7.0", features = ["inline", "unicode"] } # Image handling (pasted/attached images) image = "0.25" diff --git a/crates/ui_gpui/src/main_screen/right_panel/review_view.rs b/crates/ui_gpui/src/main_screen/right_panel/review_view.rs index 1feeffd0..677cb6c4 100644 --- a/crates/ui_gpui/src/main_screen/right_panel/review_view.rs +++ b/crates/ui_gpui/src/main_screen/right_panel/review_view.rs @@ -482,6 +482,12 @@ impl ReviewView { }, ); + // Sections default to collapsed; only repos the user expanded (stored + // by absolute root path in the UI settings) start open. + let expanded = cx + .try_global::() + .is_some_and(|g| g.0.review_expanded_repos.contains(&data.repo_root)); + RepoSection { repo_root: data.repo_root.clone(), label: data.label.clone(), @@ -491,7 +497,7 @@ impl ReviewView { files: data.files.clone(), stats: data.stats, scan_state: data.scan_state, - collapsed: false, + collapsed: !expanded, _base_sub: base_sub, } } @@ -826,12 +832,27 @@ impl ReviewView { ) .child(self.render_scan_indicator(&self.repos[ix], cx)) .on_click(cx.listener(move |this, _ev, _window, cx| { - if let Some(s) = this.repos.iter_mut().find(|s| s.repo_root == toggle_root) { - s.collapsed = !s.collapsed; - // Expanding may unlock diffs skipped while collapsed. - this.ensure_diff_request(cx); - cx.notify(); - } + let Some(s) = this.repos.iter_mut().find(|s| s.repo_root == toggle_root) else { + return; + }; + s.collapsed = !s.collapsed; + let expanded = !s.collapsed; + + // Persist per repo root (sections default to collapsed). + let root = toggle_root.clone(); + crate::update_ui_settings(cx, move |settings| { + if expanded { + if !settings.review_expanded_repos.contains(&root) { + settings.review_expanded_repos.push(root); + } + } else { + settings.review_expanded_repos.retain(|r| r != &root); + } + }); + + // Expanding may unlock diffs skipped while collapsed. + this.ensure_diff_request(cx); + cx.notify(); })); section_el = section_el.child(header); diff --git a/crates/ui_gpui/src/shared/settings.rs b/crates/ui_gpui/src/shared/settings.rs index cb1483d7..609fe33f 100644 --- a/crates/ui_gpui/src/shared/settings.rs +++ b/crates/ui_gpui/src/shared/settings.rs @@ -81,6 +81,11 @@ pub struct UiSettings { /// repo's per-session base override. #[serde(default, skip_serializing_if = "Option::is_none")] pub review_default_base: Option, + + /// Repo roots whose Review section the user expanded — sections default + /// to collapsed. Absolute paths, so the state is per project/repo. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub review_expanded_repos: Vec, } fn default_theme_mode() -> ThemeModeSetting { @@ -100,6 +105,7 @@ impl Default for UiSettings { default_model: None, right_sidebar_width: None, review_default_base: None, + review_expanded_repos: Vec::new(), } } } diff --git a/crates/ui_gpui/src/tool_cards/diff_card.rs b/crates/ui_gpui/src/tool_cards/diff_card.rs index df7720fe..1376b93e 100644 --- a/crates/ui_gpui/src/tool_cards/diff_card.rs +++ b/crates/ui_gpui/src/tool_cards/diff_card.rs @@ -538,11 +538,61 @@ fn normalize_for_diff(text: &str) -> String { } /// One line of a computed unified diff. `text` is a [`SharedString`] so cached -/// diffs can be re-rendered every frame with cheap clones. +/// diffs can be re-rendered every frame with cheap clones. `emphasis` marks +/// the byte ranges within `text` that changed *within* the line (word diff); +/// they get a stronger background on top of the row color. #[derive(Debug, Clone)] pub struct DiffLine { pub tag: ChangeTag, pub text: SharedString, + pub emphasis: Vec>, +} + +/// Replace blocks larger than this skip the word-level diff — pairing lines +/// across big rewrites produces noise, not signal (Zed caps similarly). +const MAX_WORD_DIFF_LINES: usize = 16; + +/// Expand one diff op into [`DiffLine`]s, with word-level emphasis for small +/// replace blocks. `iter_inline_changes` falls back to plain changes on its +/// own when the block's similarity ratio is too low for a useful word diff. +fn collect_change_lines<'a>( + diff: &'a TextDiff<'a, 'a, 'a, str>, + op: &similar::DiffOp, + out: &mut Vec, +) { + let block_lines = op.old_range().len().max(op.new_range().len()); + if block_lines <= MAX_WORD_DIFF_LINES { + for change in diff.iter_inline_changes(op) { + let mut text = String::new(); + let mut emphasis = Vec::new(); + for (emphasized, piece) in change.iter_strings_lossy() { + let start = text.len(); + text.push_str(&piece); + if emphasized { + emphasis.push(start..text.len()); + } + } + let trimmed_len = text.trim_end().len(); + text.truncate(trimmed_len); + emphasis.retain_mut(|r| { + r.end = r.end.min(trimmed_len); + r.start < r.end + }); + out.push(DiffLine { + tag: change.tag(), + text: text.into(), + emphasis, + }); + } + } else { + for change in diff.iter_changes(op) { + out.push(DiffLine { + tag: change.tag(), + text: change.value().trim_end().to_string().into(), + emphasis: Vec::new(), + }); + } + } } /// Run the line diff (the expensive part: normalization + Myers diff + per-line @@ -557,12 +607,11 @@ pub(crate) fn compute_diff_lines(old_text: &str, new_text: &str) -> Vec Vec diff.grouped_ops(context) .iter() - .map(|ops| DiffHunk { - new_start: ops.first().map(|op| op.new_range().start + 1).unwrap_or(1), - lines: ops - .iter() - .flat_map(|op| diff.iter_changes(op)) - .map(|change| DiffLine { - tag: change.tag(), - text: change.value().trim_end().to_string().into(), - }) - .collect(), + .map(|ops| { + let mut lines = Vec::new(); + for op in ops { + collect_change_lines(&diff, op, &mut lines); + } + DiffHunk { + new_start: ops.first().map(|op| op.new_range().start + 1).unwrap_or(1), + lines, + } }) .collect() } @@ -611,6 +659,7 @@ pub fn single_sided_hunk(text: &str, tag: ChangeTag) -> Vec { .map(|l| DiffLine { tag, text: l.trim_end().to_string().into(), + emphasis: Vec::new(), }) .collect(); if lines.is_empty() { @@ -775,7 +824,25 @@ fn render_diff_rows( } // Content — overflow_x_hidden enables min-width:0 in flex so text - // wraps instead of pushing the row wider than the card. + // wraps instead of pushing the row wider than the card. Word-level + // changes get a stronger background via text-run highlights, which + // wrap with the text (unlike per-span elements). + let content: gpui::AnyElement = if dl.emphasis.is_empty() { + dl.text.clone().into_any_element() + } else { + let word_bg = word_emphasis_bg(dl.tag, theme); + gpui::StyledText::new(dl.text.clone()) + .with_highlights(dl.emphasis.iter().map(|range| { + ( + range.clone(), + gpui::HighlightStyle { + background_color: Some(word_bg), + ..Default::default() + }, + ) + })) + .into_any_element() + }; row = row.child( div() .flex_grow(1.0) @@ -783,7 +850,7 @@ fn render_diff_rows( .when(start_line.is_none(), |d| d.px_3()) .when(start_line.is_some(), |d| d.pl_1().pr_3()) .text_color(text_color) - .child(dl.text.clone()), + .child(content), ); row.into_any() @@ -1048,6 +1115,18 @@ pub(crate) fn added_row_colors( } } +/// Background for word-level (intra-line) changes: a stronger tint layered on +/// top of the row's add/delete background. +fn word_emphasis_bg(tag: ChangeTag, theme: &gpui_component::theme::Theme) -> gpui::Hsla { + match (tag, theme.is_dark()) { + (ChangeTag::Delete, true) => rgba_color(0xC0, 0x38, 0x38, 0x70), + (ChangeTag::Delete, false) => rgba_color(0xE0, 0x60, 0x60, 0x60), + (ChangeTag::Insert, true) => rgba_color(0x38, 0xA0, 0x38, 0x70), + (ChangeTag::Insert, false) => rgba_color(0x40, 0xB8, 0x40, 0x50), + (ChangeTag::Equal, _) => gpui::transparent_black(), + } +} + pub(crate) fn unchanged_row_colors( theme: &gpui_component::theme::Theme, ) -> (Option, gpui::Hsla) { @@ -1095,6 +1174,27 @@ mod tests { } } + #[test] + fn compute_diff_lines_marks_word_level_changes() { + let lines = compute_diff_lines("fn foo(alpha: u32) {}\n", "fn foo(beta: u32) {}\n"); + let del = lines.iter().find(|l| l.tag == ChangeTag::Delete).unwrap(); + let ins = lines.iter().find(|l| l.tag == ChangeTag::Insert).unwrap(); + + // The changed identifier is emphasized — not the whole line. + assert_eq!(del.emphasis.len(), 1); + assert_eq!(&del.text[del.emphasis[0].clone()], "alpha"); + assert_eq!(ins.emphasis.len(), 1); + assert_eq!(&ins.text[ins.emphasis[0].clone()], "beta"); + + // Unchanged context lines carry no emphasis. + assert!( + lines + .iter() + .filter(|l| l.tag == ChangeTag::Equal) + .all(|l| l.emphasis.is_empty()) + ); + } + #[test] fn single_sided_hunk_is_one_pure_hunk() { let hunks = single_sided_hunk("a\nb\nc\n", ChangeTag::Insert); From 3fb27259835ce954239486d34efe290fffa4c1d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20A=C3=9Fmus?= Date: Sun, 6 Sep 2026 12:33:58 +0200 Subject: [PATCH 10/10] chore: update Cargo.lock for similar unicode feature --- Cargo.lock | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index beb0e863..9b18671b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10069,6 +10069,10 @@ name = "similar" version = "2.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bbbb5d9659141646ae647b42fe094daf6c6192d1620870b449d9557f748b2daa" +dependencies = [ + "bstr", + "unicode-segmentation", +] [[package]] name = "simplecss"