diff --git a/agents/format-projections.md b/agents/format-projections.md index a963f479..74c622b1 100644 --- a/agents/format-projections.md +++ b/agents/format-projections.md @@ -46,3 +46,4 @@ SwiftUI also offers an inline HTML preview button (same on-disk-file requirement 5. Projection failures should degrade to raw content or a binary placeholder with diagnostics. They should not make the whole file diff unusable. 6. Switching raw/processed for the same file should keep the current rendered diff visible until replacement content is ready. Switching to a different file resets rich-preview state so one file's processed view does not become another file's default. 7. Keep v1 static and narrow: no dynamic plugin ABI, no projected diff editing, and no virtual file tree formats such as XLSX/DOCX/ZIP until the product behavior is designed. +8. Per-file diff stats count the effective display mode: processed for formats that open processed by default (binary plists), raw otherwise. diff --git a/agents/shell-parity.md b/agents/shell-parity.md index 81277256..148e0ed1 100644 --- a/agents/shell-parity.md +++ b/agents/shell-parity.md @@ -26,7 +26,7 @@ Update this matrix when the user guide adds a feature, a shell closes a gap, or | Rich File Previews | Yes | Partial — Markdown images; inline HTML | Raw/processed projection modes and cache identity match. GPUI has projection controls, banners, HTML external open, native SVG preview, and a rendered Markdown preview (native block renderer, single post-change document with scrolling — same single-view model as SwiftUI). Exact gaps: Markdown image blocks render as placeholders instead of actual images; GPUI has no inline sandboxed HTML preview toggle, only external-open. | | Review Notes | Yes | Yes | GPUI supports add/edit/resolve/delete review notes, gutter dot markers, inline note rows, file-list badges, the noted-files filter, and a stale/orphaned banner. Inline note rendering is unified-view-only in both shells; side-by-side shows a note-count banner with "Show in Unified" in both, so that is not a GPUI gap. | | Compare Changes | Yes | Yes | Shift-click compare, bookmark diff, reverse compare, clear compare, and interdiff loading should use the same rev semantics. | -| Edit Diffs & Split Work | Yes | Yes | Both shells cover line-granularity discard from the normal diff gutter and a dedicated multi-file Diff Edit view with per-file cards, line/hunk/file selection, select-all, keep-only-selected Done, Move to Working Copy, New Child, and New Parallel. | +| Edit Diffs & Split Work | Yes | Yes | Both shells cover line-granularity discard from the normal diff gutter and a dedicated multi-file Diff Edit view with per-file cards, line/hunk/file selection, select-all, keep-only-selected Done, Move to Working Copy, New Child, and New Parallel. Both show per-file +/− line stats in card headers, per-file collapse/expand, Expand All / Collapse All buttons with `mod+alt+e` / `mod+alt+c`, auto-collapse for large diffs via the shared `diff_edit_auto_collapsed_paths` policy, and a keyboard focus model: `j`/`k` or `↑`/`↓` move between file cards, `←`/`→` collapse/expand, `Return` toggles, and `Space` selects the focused card. | | Change Operations | Yes | Partial — DAG rewrites; @ description prefill | GPUI covers new-change-on-top, abandon, operation-log restore, editing descriptions (including `jj describe` on @), committing, codex/claude AI messages, file multi-select split/commit and batch actions, and Diff Edit destinations including Move to Working Copy, New Child, and New Parallel. Exact gaps: no DAG actions for `jj edit`, squash into parent/selected, rebase selected, merge selected, duplicate, absorb, or revert/backout; the commit box also does not initially prefill from @'s existing description. | | Bookmarks, Git & Pull Requests | Yes | Partial — bookmark-manager mutations; post-drag Push | GPUI covers bookmark creation, DAG drag/move, tracking, push/delete, Git fetch/push, remote URL opening, GitHub/GitLab/Codeberg PR/MR opening, and selected-bookmark PR/MR plus CI status. Exact gaps: the Bookmark Manager does not expose rename, push, delete, PR/MR open, conflict resolution, per-remote ahead/behind/diverged details, remote choice when tracking, or the hide-by-default **Show deleted** plus per-row **Forget** flow; dragging a tracked bookmark also does not show SwiftUI's one-click sidebar **Push** affordance. | | Workspaces | Yes | Yes | Both shells create a workspace by name into a sibling directory, open it in its own window, show workspace context, and forget workspaces. GPUI entry points: Repository menu, status-bar workspace picker, and palette; switching lives in the status-bar picker where SwiftUI also offers palette "Switch to" entries — a presentation difference, not a workflow gap. | @@ -36,7 +36,7 @@ Update this matrix when the user guide adds a feature, a shell closes a gap, or | Command Palette | Yes | Yes | Action names, raw jj behavior, command output handling, and searchable help topics should stay aligned even if presentation differs. | | Tools & Settings | Yes | Partial — custom command editing; font-size setting | Shared config ids and option vocabularies match. Exact gaps: GPUI displays but cannot edit custom editor/terminal command values in Settings, and font size is changed only through zoom commands rather than the Settings control. GPUI includes codex/claude and jj/gh/glab detection plus a Linux `jayjay` installer (AppImage-aware symlink in `~/.local/bin`). SwiftUI-only macOS integrations are outside the GPUI Linux parity target. | | Help & User Guide Access | Yes | Yes | Both shells reach the same help content and expose Send Feedback from the Help menu and command palette. On Linux, GPUI uses the desktop's `xdg-open` handler for the guide and pre-addressed feedback email. Its palette lists the same searchable help topics (shared `HelpFeatures.json`) opening the guide at each topic's anchor. The bundled macOS Help Book is SwiftUI-only. | -| Keyboard Shortcuts | Yes | Partial — shortcut reference window | GPUI implements every shortcut published in the user guide: palette, find, refresh, open repository, zoom, Bookmark Manager, operation log/undo, file review, shift-click compare, `j`/`k`, and `Ctrl+N`/`Ctrl+P`. GPUI uses `Ctrl` as its primary modifier on Linux. Exact gap: GPUI does not provide SwiftUI's **Help → Keyboard Shortcuts** reference window. | +| Keyboard Shortcuts | Yes | Partial — shortcut reference window | GPUI implements every shortcut published in the user guide: palette, find, refresh, open repository, zoom, Bookmark Manager, operation log/undo, file review, shift-click compare, diff-edit expand/collapse all plus card navigation (`j`/`k`, arrows, `Return`), `j`/`k`, and `Ctrl+N`/`Ctrl+P`. GPUI uses `Ctrl` as its primary modifier on Linux. Exact gap: GPUI does not provide SwiftUI's **Help → Keyboard Shortcuts** reference window. | | GPUI Shell Alpha | N/A | Yes | The guide's GPUI alpha section is the source of truth for current GPUI coverage claims. Update it and this matrix together. | ## Parity Rules diff --git a/crates/jayjay-core/src/repo/diff.rs b/crates/jayjay-core/src/repo/diff.rs index 17856c64..1526f789 100644 --- a/crates/jayjay-core/src/repo/diff.rs +++ b/crates/jayjay-core/src/repo/diff.rs @@ -26,6 +26,27 @@ pub(super) struct TreePair { pub(super) after: MergedTree, } +fn hunk_line_stats(hunk: &DiffHunk, ignore_whitespace: bool) -> FileDiffStats { + let old = hunk.old.content.as_deref(); + let new = hunk.new.content.as_deref(); + // The canonical classifier covers every core placeholder (symlink, too-large, ...), unlike jj_diff's narrower display set. + let editable = |text: Option<&str>| text.is_none_or(crate::placeholder::is_editable_text); + let (insertions, deletions) = if editable(old) && editable(new) { + jj_diff::count_changed_lines( + old.unwrap_or_default(), + new.unwrap_or_default(), + ignore_whitespace, + ) + } else { + (0, 0) + }; + FileDiffStats { + path: hunk.path.clone(), + insertions, + deletions, + } +} + impl Repo { fn commit_tree_pair(&self, rev: &str) -> CoreResult { let repo = self.get_repo(); @@ -215,6 +236,16 @@ impl Repo { } } + /// Per-file line counts for the revision, matching what Diff Edit presents: raw (unprojected) text, rename-aware, placeholder-only sides counted as zero. + pub fn diff_file_stats( + &self, + rev: &str, + ignore_whitespace: bool, + ) -> CoreResult> { + let trees = self.commit_tree_pair(rev)?; + self.diff_file_stats_walk(&trees, ignore_whitespace) + } + /// Fast: file list between two arbitrary revisions WITHOUT content. pub fn interdiff_summary(&self, from_rev: &str, to_rev: &str) -> CoreResult { let (trees, info) = self.interdiff_trees(from_rev, to_rev)?; diff --git a/crates/jayjay-core/src/repo/diff/content.rs b/crates/jayjay-core/src/repo/diff/content.rs index 7957eda7..60256c33 100644 --- a/crates/jayjay-core/src/repo/diff/content.rs +++ b/crates/jayjay-core/src/repo/diff/content.rs @@ -77,6 +77,26 @@ impl Repo { Ok(diff) } + /// Per-file stats over the displayed card list: the content-free walk supplies the cards and their rename pairing, then each card's sides are materialized in its effective display mode and dropped after counting, so no blob outlives its own card. + pub(super) fn diff_file_stats_walk( + &self, + trees: &TreePair, + ignore_whitespace: bool, + ) -> CoreResult> { + let files = self.diff_file_list(trees)?; + let mut stats = Vec::with_capacity(files.len()); + for file in files { + let mode = crate::projection::request_mode(file.projection.as_ref(), false) + .unwrap_or(DiffProjectionMode::Raw); + let mut hunk = self.diff_single_file_with_mode(trees, &file.path, mode)?; + if let Some(old_path) = file.old_path.as_deref() { + hunk.old = self.diff_single_file_with_mode(trees, old_path, mode)?.old; + } + stats.push(super::hunk_line_stats(&hunk, ignore_whitespace)); + } + Ok(stats) + } + /// Materialize a single file between two trees. pub(super) fn diff_single_file(&self, trees: &TreePair, path: &str) -> CoreResult { self.diff_single_file_with_mode(trees, path, DiffProjectionMode::Processed) diff --git a/crates/jayjay-core/tests/diff_file_stats.rs b/crates/jayjay-core/tests/diff_file_stats.rs new file mode 100644 index 00000000..4a9d2bd9 --- /dev/null +++ b/crates/jayjay-core/tests/diff_file_stats.rs @@ -0,0 +1,144 @@ +use std::fs; + +use jayjay_core::Repo; +use jj_test::{FormatFixture, init_jj_repo, run_jj_in}; + +#[test] +fn diff_file_stats_reports_per_file_line_counts() { + let temp_dir = init_jj_repo(); + let repo_path = temp_dir.path().join("repo"); + run_jj_in(&repo_path, &["new", "-m", "edit files"]); + let repo = Repo::open(&repo_path).expect("open repo"); + + fs::write( + repo_path.join("hello.txt"), + "hello from tests\nsecond line\n", + ) + .expect("modify hello.txt"); + fs::write(repo_path.join("added.txt"), "one\ntwo\nthree\n").expect("write added.txt"); + repo.refresh_working_copy().expect("snapshot working copy"); + + let stats = repo.diff_file_stats("@", false).expect("diff file stats"); + let for_path = |path: &str| { + stats + .iter() + .find(|file| file.path == path) + .unwrap_or_else(|| panic!("missing stats for {path}")) + }; + + let added = for_path("added.txt"); + assert_eq!((added.insertions, added.deletions), (3, 0)); + + let modified = for_path("hello.txt"); + assert_eq!( + (modified.insertions, modified.deletions), + (2, 1), + "all stats: {stats:?}" + ); +} + +#[test] +fn diff_file_stats_pairs_same_basename_moves_like_the_card_list() { + let temp_dir = init_jj_repo(); + let repo_path = temp_dir.path().join("repo"); + run_jj_in(&repo_path, &["new", "-m", "move hello"]); + let repo = Repo::open(&repo_path).expect("open repo"); + + fs::create_dir(repo_path.join("moved")).expect("create dir"); + fs::rename( + repo_path.join("hello.txt"), + repo_path.join("moved/hello.txt"), + ) + .expect("move hello.txt"); + repo.refresh_working_copy().expect("snapshot working copy"); + + let stats = repo.diff_file_stats("@", false).expect("diff file stats"); + assert_eq!(stats.len(), 1, "same-basename move pairs: {stats:?}"); + assert_eq!(stats[0].path, "moved/hello.txt"); + assert_eq!((stats[0].insertions, stats[0].deletions), (0, 0)); +} + +#[test] +fn diff_file_stats_keeps_cross_basename_renames_as_two_entries() { + // diff_file_list pairs renames without content, so the cards show a deletion and an addition; stats must describe those cards, not a content-aware merge. + let temp_dir = init_jj_repo(); + let repo_path = temp_dir.path().join("repo"); + run_jj_in(&repo_path, &["new", "-m", "rename hello"]); + let repo = Repo::open(&repo_path).expect("open repo"); + + fs::rename(repo_path.join("hello.txt"), repo_path.join("greeting.txt")) + .expect("rename hello.txt"); + repo.refresh_working_copy().expect("snapshot working copy"); + + let stats = repo.diff_file_stats("@", false).expect("diff file stats"); + assert_eq!(stats.len(), 2, "cards stay split: {stats:?}"); + let removed = stats + .iter() + .find(|s| s.path == "hello.txt") + .expect("removed entry"); + let added = stats + .iter() + .find(|s| s.path == "greeting.txt") + .expect("added entry"); + assert_eq!((removed.insertions, removed.deletions), (0, 1)); + assert_eq!((added.insertions, added.deletions), (1, 0)); +} + +#[cfg(unix)] +#[test] +fn diff_file_stats_counts_symlinks_as_zero() { + let temp_dir = init_jj_repo(); + let repo_path = temp_dir.path().join("repo"); + let repo = Repo::open(&repo_path).expect("open repo"); + + std::os::unix::fs::symlink("hello.txt", repo_path.join("link.txt")).expect("create symlink"); + repo.refresh_working_copy().expect("snapshot working copy"); + + let stats = repo.diff_file_stats("@", false).expect("diff file stats"); + let link = stats + .iter() + .find(|file| file.path == "link.txt") + .expect("missing stats for link.txt"); + assert_eq!((link.insertions, link.deletions), (0, 0)); +} + +#[test] +fn diff_file_stats_counts_auto_projected_plists_in_processed_mode() { + let fixture = FormatFixture::build(); + let repo = Repo::open(&fixture.path).expect("open repo"); + repo.refresh_working_copy().expect("snapshot working copy"); + + let stats = repo.diff_file_stats("@", false).expect("diff file stats"); + let plist = stats + .iter() + .find(|file| file.path == FormatFixture::PLIST) + .expect("missing stats for the binary plist"); + let projected = repo + .show_file("@", FormatFixture::PLIST) + .expect("projected plist hunk"); + let xml_lines = projected + .new + .content + .as_deref() + .expect("projected content") + .lines() + .count() as u32; + assert_eq!((plist.insertions, plist.deletions), (xml_lines, 0)); +} + +#[test] +fn diff_file_stats_counts_binary_files_as_zero() { + let temp_dir = init_jj_repo(); + let repo_path = temp_dir.path().join("repo"); + let repo = Repo::open(&repo_path).expect("open repo"); + + fs::write(repo_path.join("blob.bin"), [0u8, 159, 146, 150, 0, 1]).expect("write binary file"); + repo.refresh_working_copy().expect("snapshot working copy"); + + let stats = repo.diff_file_stats("@", false).expect("diff file stats"); + let blob = stats + .iter() + .find(|file| file.path == "blob.bin") + .expect("missing stats for blob.bin"); + assert_eq!((blob.insertions, blob.deletions), (0, 0)); +} diff --git a/crates/jayjay-primitives/src/diff.rs b/crates/jayjay-primitives/src/diff.rs index 96e5f207..197e2fe4 100644 --- a/crates/jayjay-primitives/src/diff.rs +++ b/crates/jayjay-primitives/src/diff.rs @@ -97,3 +97,119 @@ pub struct DiffStats { pub insertions: u32, pub deletions: u32, } + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FileDiffStats { + pub path: String, + pub insertions: u32, + pub deletions: u32, +} + +impl FileDiffStats { + pub fn total(&self) -> u32 { + self.insertions + self.deletions + } +} + +pub const DIFF_EDIT_AUTO_EXPAND_MAX_FILES: usize = 30; +pub const DIFF_EDIT_AUTO_COLLAPSE_TOTAL_LINES: u64 = 1000; +pub const DIFF_EDIT_AUTO_COLLAPSE_FILE_LINES: u32 = 300; + +pub fn diff_edit_starts_collapsed(file_count: usize, total_changed_lines: u64) -> bool { + file_count > DIFF_EDIT_AUTO_EXPAND_MAX_FILES + && total_changed_lines > DIFF_EDIT_AUTO_COLLAPSE_TOTAL_LINES +} + +pub fn diff_edit_collapses_while_stats_pending(file_count: usize) -> bool { + file_count > DIFF_EDIT_AUTO_EXPAND_MAX_FILES +} + +pub fn diff_edit_auto_collapsed_paths(stats: &[FileDiffStats]) -> Vec { + if stats.len() <= DIFF_EDIT_AUTO_EXPAND_MAX_FILES { + return Vec::new(); + } + let total: u64 = stats.iter().map(|file| u64::from(file.total())).sum(); + stats + .iter() + .filter(|file| { + total > DIFF_EDIT_AUTO_COLLAPSE_TOTAL_LINES + || file.total() > DIFF_EDIT_AUTO_COLLAPSE_FILE_LINES + }) + .map(|file| file.path.clone()) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + fn stats(path: &str, insertions: u32, deletions: u32) -> FileDiffStats { + FileDiffStats { + path: path.to_owned(), + insertions, + deletions, + } + } + + fn small_files(count: usize) -> Vec { + (0..count).map(|i| stats(&i.to_string(), 5, 2)).collect() + } + + #[test] + fn diffs_at_the_file_cap_stay_expanded_even_when_huge() { + let mut files = small_files(DIFF_EDIT_AUTO_EXPAND_MAX_FILES - 1); + files.push(stats("huge", 5000, 5000)); + assert!(diff_edit_auto_collapsed_paths(&files).is_empty()); + } + + #[test] + fn medium_diff_collapses_only_oversized_files() { + let mut files = small_files(DIFF_EDIT_AUTO_EXPAND_MAX_FILES); + files.push(stats("big", 400, 0)); + assert_eq!(diff_edit_auto_collapsed_paths(&files), ["big"]); + } + + #[test] + fn large_total_collapses_every_file() { + let mut files = small_files(DIFF_EDIT_AUTO_EXPAND_MAX_FILES); + files.push(stats("big", 900, 200)); + let collapsed = diff_edit_auto_collapsed_paths(&files); + assert_eq!(collapsed.len(), files.len()); + } + + #[test] + fn small_diff_with_many_files_stays_expanded() { + assert!( + diff_edit_auto_collapsed_paths(&small_files(DIFF_EDIT_AUTO_EXPAND_MAX_FILES + 5)) + .is_empty() + ); + } + + #[test] + fn entry_decision_matches_the_collapse_all_rule() { + assert!(diff_edit_starts_collapsed( + DIFF_EDIT_AUTO_EXPAND_MAX_FILES + 1, + 1001 + )); + assert!(!diff_edit_starts_collapsed( + DIFF_EDIT_AUTO_EXPAND_MAX_FILES + 1, + 1000 + )); + assert!(!diff_edit_starts_collapsed( + DIFF_EDIT_AUTO_EXPAND_MAX_FILES, + 50_000 + )); + assert!(!diff_edit_starts_collapsed(0, 0)); + } + + #[test] + fn pending_stats_collapse_only_past_the_file_cap() { + assert!(diff_edit_collapses_while_stats_pending( + DIFF_EDIT_AUTO_EXPAND_MAX_FILES + 1 + )); + assert!(!diff_edit_collapses_while_stats_pending( + DIFF_EDIT_AUTO_EXPAND_MAX_FILES + )); + assert!(!diff_edit_collapses_while_stats_pending(0)); + } +} diff --git a/crates/jayjay-uniffi/src/diff.rs b/crates/jayjay-uniffi/src/diff.rs index ffe0c974..273168e1 100644 --- a/crates/jayjay-uniffi/src/diff.rs +++ b/crates/jayjay-uniffi/src/diff.rs @@ -1,8 +1,24 @@ +use jayjay_core::FileDiffStats; use jayjay_core::diff::{ self, ChangeGroup, ConflictLineKind, DiffLine, DiffSpan, SideBySideRow, WrappedDiffLine, WrappedSbsRow, }; +#[uniffi::export] +pub fn diff_edit_auto_collapsed_paths(stats: Vec) -> Vec { + jayjay_core::diff_edit_auto_collapsed_paths(&stats) +} + +#[uniffi::export] +pub fn diff_edit_starts_collapsed(file_count: u64, total_changed_lines: u64) -> bool { + jayjay_core::diff_edit_starts_collapsed(file_count as usize, total_changed_lines) +} + +#[uniffi::export] +pub fn diff_edit_collapses_while_stats_pending(file_count: u64) -> bool { + jayjay_core::diff_edit_collapses_while_stats_pending(file_count as usize) +} + #[uniffi::export] pub fn build_side_by_side_rows(lines: Vec) -> Vec { diff::build_side_by_side_rows(&lines) diff --git a/crates/jayjay-uniffi/src/repo.rs b/crates/jayjay-uniffi/src/repo.rs index a746419a..cf94c2d5 100644 --- a/crates/jayjay-uniffi/src/repo.rs +++ b/crates/jayjay-uniffi/src/repo.rs @@ -3,10 +3,10 @@ use std::sync::Arc; use jayjay_core::{ AnnotationLine, BookmarkInfo, ChangeDetail, ChangeInfo, CliStatus, DiffEditDestination, - DiffEditFileSelection, DiffHunk, DiffStats, EvologEntry, FetchResult, FileTreeEntry, - GitSubmoduleStatus, GraphEntry, JjCommand, JjCommandResult, OpLogEntry, PrInfo, Repo, - ReviewNoteOutputFormat, RevsetPreset, Stack, StackedPrResult, SubmitStackLayer, ToolsConfig, - WorkspaceInfo, + DiffEditFileSelection, DiffHunk, DiffStats, EvologEntry, FetchResult, FileDiffStats, + FileTreeEntry, GitSubmoduleStatus, GraphEntry, JjCommand, JjCommandResult, OpLogEntry, PrInfo, + Repo, ReviewNoteOutputFormat, RevsetPreset, Stack, StackedPrResult, SubmitStackLayer, + ToolsConfig, WorkspaceInfo, diff::{self, CollapsedDiff, FileDiff}, }; use jayjay_primitives::{NoteAnchor, NoteEntry, NoteSide, ReviewNoteStatus}; @@ -546,6 +546,14 @@ impl JayJayRepo { Ok(self.inner.diff_stats(&rev)?) } + pub fn diff_file_stats( + &self, + rev: String, + ignore_whitespace: bool, + ) -> Result, JayJayError> { + Ok(self.inner.diff_file_stats(&rev, ignore_whitespace)?) + } + pub fn annotate_file( &self, rev: String, @@ -784,6 +792,16 @@ impl JayJayRepo { diff::compute_file_diff_full(&path, &old_content, &new_content, ignore_whitespace) } + pub fn compute_native_diff_full_plain( + &self, + path: String, + old_content: String, + new_content: String, + ignore_whitespace: bool, + ) -> FileDiff { + diff::compute_file_diff_full_plain(&path, &old_content, &new_content, ignore_whitespace) + } + pub fn collapse_diff_with_mapping(&self, diff: FileDiff) -> CollapsedDiff { diff::collapse_context_with_mapping(&diff) } diff --git a/crates/jayjay-uniffi/src/types/diff.rs b/crates/jayjay-uniffi/src/types/diff.rs index 37c49320..7d0928f9 100644 --- a/crates/jayjay-uniffi/src/types/diff.rs +++ b/crates/jayjay-uniffi/src/types/diff.rs @@ -7,7 +7,7 @@ use jayjay_core::diff::{ use jayjay_core::syntax::SyntaxToken; use jayjay_core::{ DiffContent, DiffEditDestination, DiffEditFileSelection, DiffEditRange, DiffHunk, DiffPreview, - DiffProjection, DiffProjectionMode, DiffRenderKind, DiffStats, HunkType, + DiffProjection, DiffProjectionMode, DiffRenderKind, DiffStats, FileDiffStats, HunkType, }; #[uniffi::remote(Enum)] @@ -245,3 +245,10 @@ pub struct DiffStats { pub insertions: u32, pub deletions: u32, } + +#[uniffi::remote(Record)] +pub struct FileDiffStats { + pub path: String, + pub insertions: u32, + pub deletions: u32, +} diff --git a/crates/jj-diff/src/compute.rs b/crates/jj-diff/src/compute.rs index 0a1b48fd..36107305 100644 --- a/crates/jj-diff/src/compute.rs +++ b/crates/jj-diff/src/compute.rs @@ -33,7 +33,7 @@ pub fn highlight_file(path: &str, content: &str) -> Vec> { } pub fn compute_file_diff(path: &str, old: &str, new: &str, ignore_whitespace: bool) -> FileDiff { - compute_file_diff_impl(path, old, new, ignore_whitespace, true) + compute_file_diff_impl(path, old, new, ignore_whitespace, true, false) } pub fn compute_file_diff_full( @@ -42,7 +42,17 @@ pub fn compute_file_diff_full( new: &str, ignore_whitespace: bool, ) -> FileDiff { - compute_file_diff_impl(path, old, new, ignore_whitespace, false) + compute_file_diff_impl(path, old, new, ignore_whitespace, false, false) +} + +/// Full diff without syntax highlighting, for callers that only need line structure (e.g. selection sets) — tree-sitter setup costs tens of milliseconds per file. +pub fn compute_file_diff_full_plain( + path: &str, + old: &str, + new: &str, + ignore_whitespace: bool, +) -> FileDiff { + compute_file_diff_impl(path, old, new, ignore_whitespace, false, true) } /// File extensions that are generated/data — skip syntax highlighting. @@ -61,6 +71,7 @@ fn compute_file_diff_impl( new: &str, ignore_whitespace: bool, collapse: bool, + force_skip_highlight: bool, ) -> FileDiff { let language = syntax::language_for_path(path); @@ -75,7 +86,7 @@ fn compute_file_diff_impl( let old_line_map = LineMap::from_text(old); let new_line_map = LineMap::from_text(new); - let skip_highlight = should_skip_highlight(path); + let skip_highlight = force_skip_highlight || should_skip_highlight(path); let old_lines: Vec<&str> = old.lines().collect(); let new_lines: Vec<&str> = new.lines().collect(); diff --git a/crates/jj-diff/src/lib.rs b/crates/jj-diff/src/lib.rs index a89506d0..c4bb77a2 100644 --- a/crates/jj-diff/src/lib.rs +++ b/crates/jj-diff/src/lib.rs @@ -9,6 +9,7 @@ mod line_diff; pub mod placeholders; mod render_highlights; pub mod side_by_side; +mod stats; pub mod syntax; mod types; mod word_diff; @@ -27,7 +28,9 @@ pub(crate) fn text_diff_config() -> TextDiffConfig { mod tests; pub use change_groups::{anchor_side_and_number, change_group_for_anchor, change_groups}; -pub use compute::{compute_file_diff, compute_file_diff_full, highlight_file}; +pub use compute::{ + compute_file_diff, compute_file_diff_full, compute_file_diff_full_plain, highlight_file, +}; pub use conflicts::{ annotate_conflict_lines, build_diff_display_items, build_diff_display_lines, conflict_display_text, @@ -35,6 +38,7 @@ pub use conflicts::{ pub use context::collapse_context_with_mapping; pub use placeholders::{is_editable_text, is_git_lfs, is_git_submodule}; pub use side_by_side::{RowSide, SideBySideRow, build_side_by_side_rows}; +pub use stats::count_changed_lines; pub use syntax::{HighlightSpan, SyntaxToken, highlight, language_for_path}; pub use types::{ ChangeGroup, CollapsedDiff, ConflictBlock, ConflictBlockSection, ConflictLineKind, diff --git a/crates/jj-diff/src/stats.rs b/crates/jj-diff/src/stats.rs new file mode 100644 index 00000000..c529683d --- /dev/null +++ b/crates/jj-diff/src/stats.rs @@ -0,0 +1,28 @@ +use super::line_diff::{LineOp, line_diff}; + +/// Insertion/deletion counts matching the rendered diff rows, including the active whitespace mode. +pub fn count_changed_lines(old: &str, new: &str, ignore_whitespace: bool) -> (u32, u32) { + if old.is_empty() && new.is_empty() { + return (0, 0); + } + let old_lines: Vec<&str> = old.lines().collect(); + let new_lines: Vec<&str> = new.lines().collect(); + let ops = line_diff(&old_lines, &new_lines, ignore_whitespace); + let mut insertions = 0; + let mut deletions = 0; + for op in &ops { + match op { + LineOp::Add => insertions += 1, + LineOp::Remove => deletions += 1, + LineOp::Equal => {} + } + } + // The renderer splits a shared trailing context line into a removed/added pair when only EOF-newline presence differs; mirror that so counts match visible rows. + let no_eof_old = !old.is_empty() && !old.ends_with('\n'); + let no_eof_new = !new.is_empty() && !new.ends_with('\n'); + if no_eof_old != no_eof_new && ops.last() == Some(&LineOp::Equal) { + insertions += 1; + deletions += 1; + } + (insertions, deletions) +} diff --git a/crates/jj-diff/src/tests/basic.rs b/crates/jj-diff/src/tests/basic.rs index 66e71778..56789d68 100644 --- a/crates/jj-diff/src/tests/basic.rs +++ b/crates/jj-diff/src/tests/basic.rs @@ -251,3 +251,36 @@ fn highlighted_lines_reassemble_to_source_text() { ); } } + +#[test] +fn plain_full_diff_matches_highlighted_line_structure() { + let old = "fn main() {\n let x = 1;\n}\n"; + let new = "fn main() {\n let x = 2;\n}\n"; + let highlighted = compute_file_diff_full("t.rs", old, new, false); + let plain = compute_file_diff_full_plain("t.rs", old, new, false); + assert_eq!(plain.lines.len(), highlighted.lines.len()); + for (plain_line, highlighted_line) in plain.lines.iter().zip(&highlighted.lines) { + assert_eq!( + ( + plain_line.old_line_no, + plain_line.new_line_no, + plain_line.style + ), + ( + highlighted_line.old_line_no, + highlighted_line.new_line_no, + highlighted_line.style + ) + ); + } + assert!(plain.lines.iter().all(|line| { + line.spans + .iter() + .all(|span| span.token == SyntaxToken::Plain) + })); + assert!(highlighted.lines.iter().any(|line| { + line.spans + .iter() + .any(|span| span.token != SyntaxToken::Plain) + })); +} diff --git a/crates/jj-diff/src/tests/mod.rs b/crates/jj-diff/src/tests/mod.rs index 4b6e338f..a3299371 100644 --- a/crates/jj-diff/src/tests/mod.rs +++ b/crates/jj-diff/src/tests/mod.rs @@ -4,6 +4,7 @@ mod basic; mod collapse; mod eof; mod performance; +mod stats; mod trim; mod word; diff --git a/crates/jj-diff/src/tests/stats.rs b/crates/jj-diff/src/tests/stats.rs new file mode 100644 index 00000000..038f45ba --- /dev/null +++ b/crates/jj-diff/src/tests/stats.rs @@ -0,0 +1,51 @@ +use super::*; + +#[test] +fn eof_only_change_counts_the_split_context_pair() { + assert_eq!(count_changed_lines("a\nb", "a\nb\n", false), (1, 1)); + assert_eq!(count_changed_lines("a\nb\n", "a\nb", false), (1, 1)); +} + +#[test] +fn whitespace_only_changes_follow_the_active_mode() { + assert_eq!(count_changed_lines("a b\n", "a b\n", false), (1, 1)); + assert_eq!(count_changed_lines("a b\n", "a b\n", true), (0, 0)); +} + +#[test] +fn counts_match_rendered_diff_rows() { + // The renderer re-pairs the old last line on appends and splits it on EOF-only changes; stats must agree with those rows, not with `jj diff --stat`. + let cases = [ + ("", ""), + ("", "one\ntwo\n"), + ("one\ntwo", ""), + ("a\nb\nc\n", "a\nB\nc\n"), + ("a\nb\nc\n", "a\nB\nc\nd\n"), + ("a\nb", "a\nb\n"), + ("a\nb\n", "a\nb"), + ("a\n", "a"), + ("x\na\nb", "X\na\nb\n"), + ("a\nb", "a\nb\nc"), + ("a b\nc\n", "a b\nc\nd\n"), + ]; + for ignore_whitespace in [false, true] { + for (old, new) in cases { + let rendered = compute_file_diff_full("t.txt", old, new, ignore_whitespace); + let added = rendered + .lines + .iter() + .filter(|l| l.style == DiffSpanStyle::Added) + .count() as u32; + let removed = rendered + .lines + .iter() + .filter(|l| l.style == DiffSpanStyle::Removed) + .count() as u32; + assert_eq!( + count_changed_lines(old, new, ignore_whitespace), + (added, removed), + "case {old:?} -> {new:?} (ignore_whitespace: {ignore_whitespace})" + ); + } + } +} diff --git a/docs/guide.html b/docs/guide.html index 32908971..ad9c396b 100644 --- a/docs/guide.html +++ b/docs/guide.html @@ -191,6 +191,8 @@

Compare Changes #

Edit Diffs & Split Work #

  • Diff edit mode lets you select files, hunks, or line ranges from a change.
  • +
  • Each file card shows its +/− line counts and collapses in place. Large diffs start collapsed so the file list stays scannable, and Expand All / Collapse All (Cmd+Option+E / Cmd+Option+C) toggle every file at once.
  • +
  • The whole view is keyboard-driven: j / k or / move focus between file cards, / collapse or expand the focused card, Return toggles it, and Space selects its changed lines.
  • Selected edits can become a child change, a parallel change, or be moved into the working copy. Working-copy edits can be discarded at selected line granularity.
  • Batch split can use reviewed files as the selection model. Split supports a parallel option when the selected edits should become a sibling instead of a child.
  • Topology-aware destinations preserve the intended jj graph shape when moving edits.
  • @@ -362,7 +364,7 @@

    GPUI Shell Alpha #

  • Build and run it from source with just gpui or just gpui /path/to/repo.
  • GPUI's current parity target is Linux. Its macOS build is for development and is not expected to duplicate every SwiftUI integration; the released macOS app remains SwiftUI.
  • Current GPUI coverage includes graph browsing, diffs, file history, annotate, evolog, file review, review notes, bookmark manager, filesystem refresh, command palette, raw jj commands, searchable help topics that open this guide, native appearance tracking, SVG previews, rendered Markdown previews, HTML external open, diff text selection/copy, file multi-select in the file column, workspace windows, and a Linux jayjay command-line install in Settings > Tools.
  • -
  • Early write coverage includes editing descriptions, saving the working-copy description with the Describe button, committing from the commit box, AI-generated commit messages via the codex or claude CLIs, splitting or committing selected files into a new change, batch file actions on the selection (restore to parent, delete from disk, ignore & untrack, mark reviewed), creating and forgetting workspaces, abandoning selected working-copy lines from the diff gutter, and a dedicated multi-file Diff Edit view with per-file cards, keep-only-selected Done, move to working copy, new child, and new parallel destinations.
  • +
  • Early write coverage includes editing descriptions, saving the working-copy description with the Describe button, committing from the commit box, AI-generated commit messages via the codex or claude CLIs, splitting or committing selected files into a new change, batch file actions on the selection (restore to parent, delete from disk, ignore & untrack, mark reviewed), creating and forgetting workspaces, abandoning selected working-copy lines from the diff gutter, and a dedicated multi-file Diff Edit view with per-file cards, per-file line stats and collapse/expand with auto-collapse for large diffs, keep-only-selected Done, move to working copy, new child, and new parallel destinations.
  • Remaining GPUI work is tracked in the roadmap.
@@ -378,6 +380,8 @@

Keyboard Shortcuts #

Cmd+Shift+BBookmark Manager Cmd+Shift+UUndo from jj operation log SpaceToggle selected file reviewed + Cmd+Option+E / Cmd+Option+CDiff edit: expand or collapse all files + j/k or /, /, ReturnDiff edit: move between file cards; collapse, expand, or toggle the focused card; Space selects it Shift+ClickCompare two revisions j / kMove through graph rows Ctrl+N / Ctrl+PMove to next or previous item diff --git a/docs/llms.txt b/docs/llms.txt index 9ff0e4e3..1da66698 100644 --- a/docs/llms.txt +++ b/docs/llms.txt @@ -24,7 +24,7 @@ Anonymous build and OS statistics are enabled by default and can be disabled in - Repository list: persistent pinned repositories shared between SwiftUI and GPUI, shell-local recent history, and a repository title menu that activates open windows or opens closed pins without duplication - DAG graph: lane-based fork/merge rendering, bookmark/conflict/divergent markers, revset filtering - Diffs: unified and side-by-side, tree-sitter syntax highlighting, word-level changes, interdiff (compare any two revisions), rich previews for Markdown, SVG, notebooks, CSV/TSV tables, SARIF reports, and binary property lists -- Diff edit: extract files, hunks, or individual lines into a new change; review files and split with one click +- Diff edit: extract files, hunks, or individual lines into a new change; per-file +/− line stats, collapsible file cards with auto-collapse for large diffs; review files and split with one click - Review notes: line-anchored notes on the working-copy diff, readable and resolvable via the `jayjay` CLI for AI-agent feedback loops - Evolog: change evolution viewer - Bookmarks: manager with prune/forget, auto-track on fetch, and bookmark diff diff --git a/shell/gpui/assets/icons/check.svg b/shell/gpui/assets/icons/check.svg new file mode 100644 index 00000000..a5479d9f --- /dev/null +++ b/shell/gpui/assets/icons/check.svg @@ -0,0 +1,3 @@ + + + diff --git a/shell/gpui/src/app/actions.rs b/shell/gpui/src/app/actions.rs index b2c5df5b..a0920a4e 100644 --- a/shell/gpui/src/app/actions.rs +++ b/shell/gpui/src/app/actions.rs @@ -36,7 +36,9 @@ actions!( Quit, SaveNoteComposer, SubmitStackedPr, - NewWorkspace + NewWorkspace, + DiffEditExpandAll, + DiffEditCollapseAll ] ); @@ -81,6 +83,12 @@ pub fn app_key_bindings() -> Vec { ), gpui::KeyBinding::new(format!("{mod_key}-f").as_str(), OpenFind, None), gpui::KeyBinding::new(format!("{mod_key}-c").as_str(), CopyDiffSelection, None), + gpui::KeyBinding::new(format!("{mod_key}-alt-e").as_str(), DiffEditExpandAll, None), + gpui::KeyBinding::new( + format!("{mod_key}-alt-c").as_str(), + DiffEditCollapseAll, + None, + ), // Scoped to the "NoteComposer" key context, not bare "TextArea", so mod+Return saves the note without binding on every other TextArea (commit box, edit description, ...). gpui::KeyBinding::new( format!("{mod_key}-enter").as_str(), diff --git a/shell/gpui/src/diff/file_column/row.rs b/shell/gpui/src/diff/file_column/row.rs index 59b1c7a5..022cc5ec 100644 --- a/shell/gpui/src/diff/file_column/row.rs +++ b/shell/gpui/src/diff/file_column/row.rs @@ -1,5 +1,5 @@ use gpui::{ - AnyElement, App, ClickEvent, Div, InteractiveElement, IntoElement, ParentElement, SharedString, + AnyElement, App, ClickEvent, Div, IntoElement, ParentElement, SharedString, StatefulInteractiveElement, Styled, Window, div, px, rgb, rgba, }; use jayjay_core::DiffHunk; @@ -7,7 +7,7 @@ use jayjay_core::DiffHunk; use crate::app::fonts; use crate::app::theme::{Theme, with_alpha}; use crate::diff::file_status; -use crate::ui::icons::{self, glyph}; +use crate::ui::primitives::{CheckCircleState, check_circle}; pub(super) fn row_bg(is_selected: bool, t: &Theme) -> u32 { if is_selected { @@ -30,34 +30,17 @@ pub(super) fn review_checkbox( where FRev: Fn(&ClickEvent, &mut Window, &mut App) + 'static, { - let border = if reviewed { - t.file_added_color + let state = if reviewed { + CheckCircleState::On } else { - t.fg_faint + CheckCircleState::Off }; - let mut checkbox = div() - .id(id) - .flex_none() - .flex() - .items_center() - .justify_center() - .w(px(14.)) - .h(px(14.)) - .rounded_full() - .border_1() - .border_color(rgb(border)) - .cursor_pointer() + check_circle(id, state, t.file_added_color, t) .on_click(move |ev, w, cx| { cx.stop_propagation(); on_click(ev, w, cx); - }); - if reviewed { - checkbox = - checkbox - .bg(rgb(t.file_added_color)) - .child(icons::icon(glyph::CHECK, 9., 0xffffff)); - } - checkbox.into_any_element() + }) + .into_any_element() } pub(super) fn file_text_content( diff --git a/shell/gpui/src/main.rs b/shell/gpui/src/main.rs index b604a64b..c2ec6737 100644 --- a/shell/gpui/src/main.rs +++ b/shell/gpui/src/main.rs @@ -15,6 +15,7 @@ const LUCIDE_FONT: &[u8] = include_bytes!("../assets/fonts/Lucide.ttf"); const ARROW_DOWN_SVG: &[u8] = include_bytes!("../assets/icons/arrow-down.svg"); const ARROW_UP_SVG: &[u8] = include_bytes!("../assets/icons/arrow-up.svg"); const CIRCLE_SVG: &[u8] = include_bytes!("../assets/icons/circle.svg"); +const CHECK_SVG: &[u8] = include_bytes!("../assets/icons/check.svg"); const REFRESH_CW_SVG: &[u8] = include_bytes!("../assets/icons/refresh-cw.svg"); const LOGO_SVG: &[u8] = include_bytes!("../assets/icons/logo.svg"); @@ -26,6 +27,7 @@ impl AssetSource for GpuiAssets { jayjay_gpui::ui::icons::ARROW_DOWN_SVG => Ok(Some(Cow::Borrowed(ARROW_DOWN_SVG))), jayjay_gpui::ui::icons::ARROW_UP_SVG => Ok(Some(Cow::Borrowed(ARROW_UP_SVG))), jayjay_gpui::ui::icons::CIRCLE_SVG => Ok(Some(Cow::Borrowed(CIRCLE_SVG))), + jayjay_gpui::ui::icons::CHECK_SVG => Ok(Some(Cow::Borrowed(CHECK_SVG))), jayjay_gpui::ui::icons::REFRESH_CW_SVG => Ok(Some(Cow::Borrowed(REFRESH_CW_SVG))), jayjay_gpui::ui::icons::LOGO_SVG => Ok(Some(Cow::Borrowed(LOGO_SVG))), _ => Ok(None), diff --git a/shell/gpui/src/repo/window/actions.rs b/shell/gpui/src/repo/window/actions.rs index 375baead..fe175a88 100644 --- a/shell/gpui/src/repo/window/actions.rs +++ b/shell/gpui/src/repo/window/actions.rs @@ -100,23 +100,13 @@ impl RepoWindow { } pub(super) fn open_diff_edit_description(&mut self, cx: &mut Context) { - if !self.diff_edit.active || self.diff_edit.working_copy { + let Some((subtitle, message, session)) = self.diff_edit_description_context() else { return; - } - let subtitle = self - .diff_edit - .change_id - .as_deref() - .unwrap_or_default() - .chars() - .take(12) - .collect::(); + }; self.open_description_modal( subtitle, - self.diff_edit.message.clone(), - TextModalAction::DiffEditDescription { - session: self.diff_edit.session, - }, + message, + TextModalAction::DiffEditDescription { session }, cx, ); } @@ -188,9 +178,7 @@ impl RepoWindow { } TextModalAction::DiffEditDescription { session } => { self.text_modal = None; - if self.diff_edit.active && self.diff_edit.session == session { - self.diff_edit.message = text; - } + self.apply_diff_edit_description(session, text); } TextModalAction::CreateBookmark { rev } => { let name = text.trim().to_string(); diff --git a/shell/gpui/src/repo/window/diff_edit_apply.rs b/shell/gpui/src/repo/window/diff_edit/apply.rs similarity index 86% rename from shell/gpui/src/repo/window/diff_edit_apply.rs rename to shell/gpui/src/repo/window/diff_edit/apply.rs index 3a026ec8..8ba6e9e6 100644 --- a/shell/gpui/src/repo/window/diff_edit_apply.rs +++ b/shell/gpui/src/repo/window/diff_edit/apply.rs @@ -3,10 +3,10 @@ use std::collections::BTreeSet; use gpui::Context; use jayjay_core::{DiffEditDestination, DiffEditFileSelection, DiffEditRange, HunkType}; -use super::RepoWindow; -use super::diff_edit_state::hunk_supports_diff_edit; -use super::diff_edit_view::DiffEditSnapshot; +use super::state::hunk_supports_diff_edit; +use super::view::DiffEditSnapshot; use crate::repo::view_model::mutations::DiffEditApplyRequest; +use crate::repo::window::RepoWindow; const EMPTY_SELECTION_MESSAGE: &str = "Select at least one file, hunk, or line before applying diff edit."; @@ -69,6 +69,32 @@ impl RepoWindow { cx.notify(); } + /// (change-id subtitle, current message, session) for the description modal; None while inactive or on the working copy. + pub(crate) fn diff_edit_description_context(&self) -> Option<(String, String, u64)> { + if !self.diff_edit.active || self.diff_edit.working_copy { + return None; + } + let subtitle = self + .diff_edit + .change_id + .as_deref() + .unwrap_or_default() + .chars() + .take(12) + .collect::(); + Some(( + subtitle, + self.diff_edit.message.clone(), + self.diff_edit.session, + )) + } + + pub(crate) fn apply_diff_edit_description(&mut self, session: u64, text: String) { + if self.diff_edit.active && self.diff_edit.session == session { + self.diff_edit.message = text; + } + } + pub fn start_diff_edit_apply( &mut self, destination: DiffEditDestination, @@ -169,7 +195,7 @@ impl RepoWindow { fn file_selection( hunk: &jayjay_core::DiffHunk, - loaded: &super::diff_edit_state::DiffEditLoadedFile, + loaded: &super::state::DiffEditLoadedFile, lines: &BTreeSet, ) -> Option { (!lines.is_empty()).then(|| DiffEditFileSelection { diff --git a/shell/gpui/src/repo/window/diff_edit_cards.rs b/shell/gpui/src/repo/window/diff_edit/cards.rs similarity index 53% rename from shell/gpui/src/repo/window/diff_edit_cards.rs rename to shell/gpui/src/repo/window/diff_edit/cards.rs index 396bf2dd..af209c5e 100644 --- a/shell/gpui/src/repo/window/diff_edit_cards.rs +++ b/shell/gpui/src/repo/window/diff_edit/cards.rs @@ -1,18 +1,18 @@ use std::sync::Arc; use gpui::{ - AnyElement, Context, InteractiveElement, IntoElement, ParentElement, Pixels, SharedString, - StatefulInteractiveElement, Styled, div, px, rgb, rgba, uniform_list, + AnyElement, Context, InteractiveElement, IntoElement, ParentElement, Pixels, Styled, div, px, + rgb, rgba, uniform_list, }; -use super::RepoWindow; -use super::diff_edit_gutter::diff_edit_line_row; -use super::diff_edit_rows::{DiffEditCardFile, DiffEditRow, DiffEditRowModel}; +use super::gutter::diff_edit_line_row; +use super::header::{header_bg, header_row}; +use super::rows::{DiffEditRow, DiffEditRowModel}; use crate::app::fonts; -use crate::app::theme::{Theme, with_alpha}; +use crate::app::theme::Theme; use crate::diff::bounds_capture; -use crate::diff::file_status; use crate::diff::line::ROW_HEIGHT; +use crate::repo::window::RepoWindow; use crate::ui::icons::{self, glyph}; use crate::ui::scrollbar::vertical_uniform_scrollbar; @@ -124,10 +124,6 @@ fn notice_row(t: &Theme) -> AnyElement { .into_any_element() } -fn header_bg(t: &Theme) -> u32 { - with_alpha(t.fg, if t.is_dark { 0x12 } else { 0x0a }) -} - fn header_pad_row(top: bool, t: &Theme) -> AnyElement { let mut pad = div().size_full().bg(rgba(header_bg(t))); if top { @@ -143,91 +139,6 @@ fn header_pad_row(top: bool, t: &Theme) -> AnyElement { .into_any_element() } -fn header_row( - view: &RepoWindow, - card: &DiffEditCardFile, - t: &Theme, - cx: &mut Context, -) -> AnyElement { - let selected_count = view - .diff_edit - .selected - .get(card.path.as_ref()) - .map(|selected| selected.len()) - .unwrap_or(0); - let mut row = div() - .flex() - .flex_1() - .items_center() - .gap(px(8.)) - .h(px(ROW_HEIGHT)) - .px(px(14.)) - .bg(rgba(header_bg(t))); - if card.supported { - let path = card.path.to_string(); - let checked = selected_count > 0; - row = row.child( - div() - .id(SharedString::from(format!( - "diff-edit-file-checkbox-{}", - card.path - ))) - .font_family(fonts::mono()) - .font_weight(gpui::FontWeight::SEMIBOLD) - .text_size(px(11.)) - .text_color(rgb(if checked { t.selected_accent } else { t.fg_dim })) - .cursor_pointer() - .on_click(cx.listener(move |view, _, _, cx| view.toggle_diff_edit_file(&path, cx))) - .child(if checked { "[x]" } else { "[ ]" }), - ); - } - let (icon, color) = file_icon(card, t); - row = row.child(icons::icon(icon, 12., color)).child( - div() - .font_family(fonts::mono()) - .font_weight(gpui::FontWeight::SEMIBOLD) - .text_size(px(12.)) - .child(card.path.to_string()), - ); - if card.supported && card.changed_total > 0 { - let badge = if selected_count == card.changed_total { - "File".to_owned() - } else if selected_count == 0 { - "None".to_owned() - } else { - format!("{selected_count} / {} lines", card.changed_total) - }; - row = row.child( - div() - .px(px(6.)) - .rounded_full() - .bg(rgba(with_alpha(t.selected_accent, 0x24))) - .text_size(px(10.)) - .font_weight(gpui::FontWeight::SEMIBOLD) - .child(badge), - ); - } - let row = - row.child(div().flex_1()) - .child( - div() - .text_size(px(11.)) - .text_color(rgb(t.fg_dim)) - .child(if card.supported { - "Select files or lines to edit" - } else { - "Text edits not supported" - }), - ); - div() - .flex() - .w_full() - .h(px(ROW_HEIGHT)) - .px(px(18.)) - .child(row) - .into_any_element() -} - fn placeholder_row(text: &'static str, t: &Theme) -> AnyElement { div() .flex() @@ -240,13 +151,3 @@ fn placeholder_row(text: &'static str, t: &Theme) -> AnyElement { .child(text) .into_any_element() } - -fn file_icon(card: &DiffEditCardFile, t: &Theme) -> (&'static str, u32) { - let color = file_status::color_for_hunk_type(card.hunk_type, t); - match card.hunk_type { - jayjay_core::HunkType::Added => (glyph::PLUS_CIRCLE, color), - jayjay_core::HunkType::Removed => (glyph::MINUS_CIRCLE, color), - jayjay_core::HunkType::Modified => (glyph::PENCIL_CIRCLE, color), - jayjay_core::HunkType::Renamed => (glyph::ARROW_CIRCLE_RIGHT, color), - } -} diff --git a/shell/gpui/src/repo/window/diff_edit/collapse.rs b/shell/gpui/src/repo/window/diff_edit/collapse.rs new file mode 100644 index 00000000..ce06e1b7 --- /dev/null +++ b/shell/gpui/src/repo/window/diff_edit/collapse.rs @@ -0,0 +1,140 @@ +use gpui::{AppContext, Context}; +use jayjay_core::{ + CoreResult, FileDiffStats, diff_edit_auto_collapsed_paths, + diff_edit_collapses_while_stats_pending, diff_edit_starts_collapsed, +}; + +use crate::repo::window::RepoWindow; + +impl RepoWindow { + pub(super) fn selected_commit_id(&self, cx: &Context) -> Option { + self.vm + .read(cx) + .selected_change_for_file_ops() + .map(|change| change.commit_id.id.clone()) + } + + /// Seeds collapse from the already-loaded whole-change stats so a large diff never renders expanded while per-file stats compute; the per-file pass replaces this approximation with the precise policy. + pub(super) fn seed_diff_edit_collapse(&mut self, cx: &Context) { + let vm = self.vm.read(cx); + let Some(files) = vm.files.as_ref() else { + return; + }; + let collapse_all = match vm.change_stats.as_ref() { + Some(stats) => { + let total = u64::from(stats.insertions) + u64::from(stats.deletions); + diff_edit_starts_collapsed(files.len(), total) + } + None => diff_edit_collapses_while_stats_pending(files.len()), + }; + if collapse_all { + self.diff_edit.collapsed = files.iter().map(|hunk| hunk.path.clone()).collect(); + } + } + + pub fn toggle_diff_edit_collapse(&mut self, path: &str, cx: &mut Context) { + if !self.diff_edit.collapsed.remove(path) { + self.diff_edit.collapsed.insert(path.to_owned()); + } + self.diff_edit.collapse_touched = true; + self.diff_edit.rows = None; + cx.notify(); + } + + pub fn collapse_all_diff_edit(&mut self, cx: &mut Context) { + let Some(files) = self.vm.read(cx).files.clone() else { + return; + }; + self.diff_edit.collapsed = files.iter().map(|hunk| hunk.path.clone()).collect(); + self.diff_edit.collapse_touched = true; + self.diff_edit.rows = None; + cx.notify(); + } + + pub fn expand_all_diff_edit(&mut self, cx: &mut Context) { + self.diff_edit.collapsed.clear(); + self.diff_edit.collapse_touched = true; + self.diff_edit.rows = None; + cx.notify(); + } + + pub fn diff_edit_collapsed(&self, path: &str) -> bool { + self.diff_edit.collapsed.contains(path) + } + + pub fn diff_edit_stats_ready(&self) -> bool { + self.diff_edit.stats.is_some() + } + + pub(super) fn spawn_diff_edit_stats(&mut self, cx: &mut Context) { + let Some(repo) = self.vm.read(cx).repo.clone() else { + return; + }; + // The immutable commit id is the query revision: a change-id query could read an amended replacement that the completion guard still attributes to the on-screen commit. + let Some(commit) = self + .vm + .read(cx) + .selected_change_for_file_ops() + .map(|change| change.commit_id.id.clone()) + else { + return; + }; + self.diff_edit.stats_commit = Some(commit.clone()); + let session = self.diff_edit.session; + let ignore_whitespace = self.vm.read(cx).ignore_whitespace; + cx.spawn(async move |this, cx| { + let rev = commit.clone(); + let stats = cx + .background_spawn(async move { repo.diff_file_stats(&rev, ignore_whitespace) }) + .await; + let _ = this.update(cx, |view, cx| { + view.finish_diff_edit_stats(session, commit, ignore_whitespace, stats, cx) + }); + }) + .detach(); + } + + fn finish_diff_edit_stats( + &mut self, + session: u64, + commit: String, + ignore_whitespace: bool, + stats: CoreResult>, + cx: &mut Context, + ) { + if !self.diff_edit.active || self.diff_edit.session != session { + return; + } + // Same session and commit both survive a mode toggle and an amend keeps the change id; the commit id and captured mode together prove the stats describe what is on screen. + if self.selected_commit_id(cx).as_deref() != Some(commit.as_str()) + || self.vm.read(cx).ignore_whitespace != ignore_whitespace + { + return; + } + let Ok(stats) = stats else { + return; + }; + // The aggregate seed can overcount the displayed rows (placeholders, projections), so the precise pass recomputes the whole policy and replaces the seed outright. + if !self.diff_edit.collapse_touched { + let cards: Vec = self + .vm + .read(cx) + .files + .as_ref() + .map(|files| files.iter().map(|hunk| hunk.path.clone()).collect()) + .unwrap_or_else(|| stats.iter().map(|s| s.path.clone()).collect()); + let total: u64 = stats + .iter() + .map(|s| u64::from(s.insertions) + u64::from(s.deletions)) + .sum(); + self.diff_edit.collapsed = if diff_edit_starts_collapsed(cards.len(), total) { + cards.into_iter().collect() + } else { + diff_edit_auto_collapsed_paths(&stats).into_iter().collect() + }; + self.diff_edit.rows = None; + } + self.diff_edit.stats = Some(stats.into_iter().map(|s| (s.path.clone(), s)).collect()); + cx.notify(); + } +} diff --git a/shell/gpui/src/repo/window/diff_edit/focus.rs b/shell/gpui/src/repo/window/diff_edit/focus.rs new file mode 100644 index 00000000..56446194 --- /dev/null +++ b/shell/gpui/src/repo/window/diff_edit/focus.rs @@ -0,0 +1,123 @@ +use gpui::{Context, KeyDownEvent, ScrollStrategy}; + +use super::rows::DiffEditRow; +use crate::repo::window::RepoWindow; +use crate::ui::navigation::{self, ListNav, ListNavKeys}; + +impl RepoWindow { + pub(crate) fn diff_edit_take_pending_focus(&mut self) -> bool { + let pending = self.diff_edit.active && self.diff_edit.focus_pending; + if pending { + self.diff_edit.focus_pending = false; + } + pending + } + + pub fn diff_edit_focused(&self) -> Option { + self.diff_edit.focused.clone() + } + + pub(super) fn diff_edit_is_focused(&self, path: &str) -> bool { + self.diff_edit.focused.as_deref() == Some(path) + } + + pub(crate) fn handle_diff_edit_nav_key( + &mut self, + ev: &KeyDownEvent, + cx: &mut Context, + ) -> bool { + if ev.keystroke.key == "enter" && !ev.keystroke.modifiers.modified() { + let Some(path) = self.diff_edit.focused.clone() else { + return false; + }; + self.toggle_diff_edit_collapse(&path, cx); + self.scroll_diff_edit_focus_into_view(cx); + return true; + } + if !ev.keystroke.modifiers.modified() { + match ev.keystroke.key.as_str() { + "left" => return self.set_focused_diff_edit_collapsed(true, cx), + "right" => return self.set_focused_diff_edit_collapsed(false, cx), + "space" => { + // Consumed even unfocused; falling through would toggle the hidden file column's review mark. + if let Some(path) = self.diff_edit.focused.clone() { + self.toggle_diff_edit_file(&path, cx); + } + return true; + } + _ => {} + } + } + if let Some(direction) = navigation::list_nav_from_key(ev, ListNavKeys::CONTENT_LIST) { + self.move_diff_edit_focus(direction, cx); + return true; + } + false + } + + fn set_focused_diff_edit_collapsed(&mut self, collapsed: bool, cx: &mut Context) -> bool { + let Some(path) = self.diff_edit.focused.clone() else { + return false; + }; + if self.diff_edit.collapsed.contains(&path) == collapsed { + return false; + } + self.toggle_diff_edit_collapse(&path, cx); + self.scroll_diff_edit_focus_into_view(cx); + true + } + + fn move_diff_edit_focus(&mut self, direction: ListNav, cx: &mut Context) { + let model = self.diff_edit_row_model(cx); + let len = model.files.len(); + if len == 0 { + return; + } + let current = self + .diff_edit + .focused + .as_deref() + .and_then(|path| model.file_index(path)); + let next = match current { + Some(pos) => navigation::move_index(Some(pos), len, direction), + None => Some(match direction { + ListNav::Next => 0, + ListNav::Previous => len - 1, + }), + }; + let Some(next) = next else { + return; + }; + let path = model.files[next].path.to_string(); + if self.diff_edit.focused.as_deref() != Some(path.as_str()) { + self.diff_edit.focused = Some(path); + cx.notify(); + } + self.scroll_diff_edit_focus_into_view(cx); + } + + pub fn focus_and_toggle_diff_edit_collapse(&mut self, path: &str, cx: &mut Context) { + self.diff_edit.focused = Some(path.to_owned()); + self.toggle_diff_edit_collapse(path, cx); + } + + fn scroll_diff_edit_focus_into_view(&mut self, cx: &mut Context) { + let Some(path) = self.diff_edit.focused.clone() else { + return; + }; + let model = self.diff_edit_row_model(cx); + let Some(file_ix) = model.file_index(&path) else { + return; + }; + let Some(row) = model + .rows + .iter() + .position(|row| matches!(row, DiffEditRow::Header(ix) if *ix == file_ix)) + else { + return; + }; + self.diff_edit + .scroll + .scroll_to_item(row, ScrollStrategy::Top); + } +} diff --git a/shell/gpui/src/repo/window/diff_edit_gutter.rs b/shell/gpui/src/repo/window/diff_edit/gutter.rs similarity index 85% rename from shell/gpui/src/repo/window/diff_edit_gutter.rs rename to shell/gpui/src/repo/window/diff_edit/gutter.rs index e1922540..ecfcdb29 100644 --- a/shell/gpui/src/repo/window/diff_edit_gutter.rs +++ b/shell/gpui/src/repo/window/diff_edit/gutter.rs @@ -1,13 +1,13 @@ use gpui::{ AnyElement, Context, InteractiveElement, IntoElement, ParentElement, Pixels, SharedString, - StatefulInteractiveElement, Styled, div, px, rgb, + StatefulInteractiveElement, Styled, div, px, rgb, svg, }; use jayjay_core::diff::DiffLine; -use super::RepoWindow; use crate::app::fonts; use crate::app::theme::Theme; use crate::diff::line::{ROW_HEIGHT, content_row, gutter_cell, line_bg_color}; +use crate::repo::window::RepoWindow; const CHECKBOX_WIDTH: f32 = 18.; @@ -83,15 +83,20 @@ fn checkbox_cell( .w(px(CHECKBOX_WIDTH)) .h(px(ROW_HEIGHT)) .bg(rgb(bg)) - .text_color(rgb(if checked { - t.selected_accent - } else { - t.fg_faint - })) + .text_color(rgb(t.fg_faint)) .cursor_pointer() .on_click(cx.listener(move |view, _, _, cx| { view.toggle_diff_edit_display_line(&toggle_path, display_line, cx); })) - .child(if checked { "■" } else { "□" }) + .child(if checked { + svg() + .path(crate::ui::icons::CHECK_SVG) + .w(px(10.)) + .h(px(10.)) + .text_color(rgb(t.selected_accent)) + .into_any_element() + } else { + "□".into_any_element() + }) .into_any_element() } diff --git a/shell/gpui/src/repo/window/diff_edit/header.rs b/shell/gpui/src/repo/window/diff_edit/header.rs new file mode 100644 index 00000000..b5124807 --- /dev/null +++ b/shell/gpui/src/repo/window/diff_edit/header.rs @@ -0,0 +1,180 @@ +use gpui::{ + AnyElement, Context, InteractiveElement, IntoElement, ParentElement, SharedString, + StatefulInteractiveElement, Styled, div, px, rgb, rgba, +}; + +use super::rows::DiffEditCardFile; +use super::state::DiffEditCheckboxState; +use crate::app::fonts; +use crate::app::theme::{Theme, with_alpha}; +use crate::diff::file_status; +use crate::diff::line::ROW_HEIGHT; +use crate::repo::window::RepoWindow; +use crate::ui::icons::{self, glyph}; +use crate::ui::primitives::{CheckCircleState, check_circle}; + +pub(super) fn header_bg(t: &Theme) -> u32 { + with_alpha(t.fg, if t.is_dark { 0x12 } else { 0x0a }) +} + +pub(super) fn header_row( + view: &RepoWindow, + card: &DiffEditCardFile, + t: &Theme, + cx: &mut Context, +) -> AnyElement { + // Counted against the changed set, the same basis the checkbox state uses, so the badge and circle can never disagree. + let selected_count = view + .diff_edit + .loaded_files + .get(card.path.as_ref()) + .zip(view.diff_edit.selected.get(card.path.as_ref())) + .map(|(loaded, selected)| selected.intersection(&loaded.changed).count()) + .unwrap_or(0); + let header_fill = if view.diff_edit_is_focused(card.path.as_ref()) { + with_alpha(t.selected_bg, 0xff) + } else { + header_bg(t) + }; + let mut row = div() + .flex() + .flex_1() + .items_center() + .gap(px(8.)) + .h(px(ROW_HEIGHT)) + .px(px(14.)) + .bg(rgba(header_fill)); + row = row.child(collapse_chevron(view, card, t, cx)); + if card.supported { + let path = card.path.to_string(); + let state = match view.diff_edit_file_state(card.path.as_ref()) { + DiffEditCheckboxState::None => CheckCircleState::Off, + DiffEditCheckboxState::Some => CheckCircleState::Partial, + DiffEditCheckboxState::All => CheckCircleState::On, + }; + row = row.child( + check_circle( + SharedString::from(format!("diff-edit-file-checkbox-{}", card.path)), + state, + t.selected_accent, + t, + ) + .on_click(cx.listener(move |view, _, _, cx| view.toggle_diff_edit_file(&path, cx))), + ); + } + let (icon, color) = file_icon(card, t); + let path_toggle = card.path.to_string(); + row = row.child(icons::icon(icon, 12., color)).child( + div() + .id(SharedString::from(format!( + "diff-edit-file-path-{}", + card.path + ))) + .cursor_pointer() + .on_click(cx.listener(move |view, _, _, cx| { + view.focus_and_toggle_diff_edit_collapse(&path_toggle, cx) + })) + .font_family(fonts::mono()) + .font_weight(gpui::FontWeight::SEMIBOLD) + .text_size(px(12.)) + .child(card.path.to_string()), + ); + if let Some(badge) = stats_badge(view, card, t) { + row = row.child(badge); + } + if card.supported && selected_count > 0 && selected_count < card.changed_total { + row = row.child( + div() + .px(px(6.)) + .rounded_full() + .bg(rgba(with_alpha(t.selected_accent, 0x24))) + .text_size(px(10.)) + .font_weight(gpui::FontWeight::SEMIBOLD) + .child(format!("{selected_count} / {} lines", card.changed_total)), + ); + } + let row = + row.child(div().flex_1()) + .child( + div() + .text_size(px(11.)) + .text_color(rgb(t.fg_dim)) + .child(if card.supported { + "Select files or lines to edit" + } else { + "Text edits not supported" + }), + ); + div() + .flex() + .w_full() + .h(px(ROW_HEIGHT)) + .px(px(18.)) + .child(row) + .into_any_element() +} + +fn collapse_chevron( + view: &RepoWindow, + card: &DiffEditCardFile, + t: &Theme, + cx: &mut Context, +) -> AnyElement { + let glyph_str = if view.diff_edit_collapsed(card.path.as_ref()) { + glyph::CARET_RIGHT + } else { + glyph::CARET_DOWN + }; + let path = card.path.to_string(); + div() + .id(SharedString::from(format!( + "diff-edit-chevron-{}", + card.path + ))) + .cursor_pointer() + .on_click( + cx.listener(move |view, _, _, cx| view.focus_and_toggle_diff_edit_collapse(&path, cx)), + ) + .child(icons::icon(glyph_str, 10., t.fg_faint)) + .into_any_element() +} + +fn stats_badge(view: &RepoWindow, card: &DiffEditCardFile, t: &Theme) -> Option { + let stats = view.diff_edit.stats.as_ref()?.get(card.path.as_ref())?; + if stats.insertions == 0 && stats.deletions == 0 { + return None; + } + let mut badge = div() + .flex() + .flex_row() + .items_baseline() + .gap(px(4.)) + .font_family(fonts::mono()) + .font_weight(gpui::FontWeight::SEMIBOLD) + .text_size(px(10.)); + if stats.insertions > 0 { + badge = badge.child( + div() + .text_color(rgb(t.diff_gutter_added_fg)) + .child(format!("+{}", stats.insertions)), + ); + } + if stats.deletions > 0 { + badge = badge.child( + div() + .text_color(rgb(t.diff_gutter_removed_fg)) + .child(format!("-{}", stats.deletions)), + ); + } + Some(badge.into_any_element()) +} + +fn file_icon(card: &DiffEditCardFile, t: &Theme) -> (&'static str, u32) { + let color = file_status::color_for_hunk_type(card.hunk_type, t); + match card.hunk_type { + jayjay_core::HunkType::Added => (glyph::PLUS_CIRCLE, color), + jayjay_core::HunkType::Removed => (glyph::MINUS_CIRCLE, color), + jayjay_core::HunkType::Modified => (glyph::PENCIL_CIRCLE, color), + jayjay_core::HunkType::Renamed => (glyph::ARROW_CIRCLE_RIGHT, color), + } +} diff --git a/shell/gpui/src/repo/window/diff_edit/mod.rs b/shell/gpui/src/repo/window/diff_edit/mod.rs new file mode 100644 index 00000000..c84cb5e1 --- /dev/null +++ b/shell/gpui/src/repo/window/diff_edit/mod.rs @@ -0,0 +1,14 @@ +mod apply; +mod cards; +mod collapse; +mod focus; +mod gutter; +mod header; +mod rows; +mod session; +mod state; +mod view; + +pub use state::{DiffEditCheckboxState, DiffEditState}; +pub use view::DiffEditSnapshot; +pub(crate) use view::diff_edit_view; diff --git a/shell/gpui/src/repo/window/diff_edit_rows.rs b/shell/gpui/src/repo/window/diff_edit/rows.rs similarity index 62% rename from shell/gpui/src/repo/window/diff_edit_rows.rs rename to shell/gpui/src/repo/window/diff_edit/rows.rs index 0e146e09..1dae381a 100644 --- a/shell/gpui/src/repo/window/diff_edit_rows.rs +++ b/shell/gpui/src/repo/window/diff_edit/rows.rs @@ -4,8 +4,9 @@ use gpui::Context; use jayjay_core::diff::FileDiff; use jayjay_core::{DiffHunk, HunkType}; -use super::RepoWindow; -use super::diff_edit_state::hunk_supports_diff_edit; +use super::state::hunk_supports_diff_edit; +use crate::repo::view_model::DiffLoadState; +use crate::repo::window::RepoWindow; pub(super) struct DiffEditRowModel { pub(super) files: Vec, @@ -16,6 +17,14 @@ pub(super) struct DiffEditRowModel { diff_cache_count: usize, } +impl DiffEditRowModel { + pub(super) fn file_index(&self, path: &str) -> Option { + self.files + .iter() + .position(|file| file.path.as_ref() == path) + } +} + pub(super) struct DiffEditCardFile { pub(super) path: Arc, pub(super) hunk_type: HunkType, @@ -70,6 +79,18 @@ impl RepoWindow { .unwrap_or(0) } + pub fn diff_edit_line_rows(&mut self, path: &str, cx: &Context) -> usize { + let model = self.diff_edit_row_model(cx); + let Some(file_ix) = model.file_index(path) else { + return 0; + }; + model + .rows + .iter() + .filter(|row| matches!(row, DiffEditRow::Line { file, .. } if *file == file_ix)) + .count() + } + fn build_diff_edit_rows( &self, hunks: Arc>, @@ -87,36 +108,37 @@ impl RepoWindow { } let loaded = self.diff_edit.loaded_files.get(&hunk.path); let supported = hunk_supports_diff_edit(hunk) && loaded.is_some(); + // The keyed lookup pins the default projection mode; a path scan can surface the wrong entry and can never match processed rows stored under the projection's virtual path. let preview = loaded.map(|file| file.display_diff.clone()).or_else(|| { - self.vm - .read(cx) - .diff_cache - .values() - .find(|cached| cached.diff.path == hunk.path) - .map(|cached| cached.diff.clone()) + match self.vm.read(cx).diff_load_state(hunk) { + DiffLoadState::Loaded(cached) => Some(cached.diff.clone()), + _ => None, + } }); rows.push(DiffEditRow::HeaderPad { top: true }); rows.push(DiffEditRow::Header(file_ix)); rows.push(DiffEditRow::HeaderPad { top: false }); - match &preview { - Some(diff) if !diff.lines.is_empty() => { - let map = loaded.map(|file| file.display_to_full.clone()); - for line_ix in 0..diff.lines.len() as u32 { - let full_line = map - .as_ref() - .filter(|_| supported) - .and_then(|map| map.get(&(line_ix + 1)).copied()); - rows.push(DiffEditRow::Line { - file: file_ix, - line_ix, - full_line, - }); + if !self.diff_edit.collapsed.contains(&hunk.path) { + match &preview { + Some(diff) if !diff.lines.is_empty() => { + let map = loaded.map(|file| file.display_to_full.clone()); + for line_ix in 0..diff.lines.len() as u32 { + let full_line = map + .as_ref() + .filter(|_| supported) + .and_then(|map| map.get(&(line_ix + 1)).copied()); + rows.push(DiffEditRow::Line { + file: file_ix, + line_ix, + full_line, + }); + } } + Some(_) => rows.push(DiffEditRow::Placeholder { loading: false }), + None => rows.push(DiffEditRow::Placeholder { + loading: !self.diff_edit.known_unsupported.contains(&hunk.path), + }), } - Some(_) => rows.push(DiffEditRow::Placeholder { loading: false }), - None => rows.push(DiffEditRow::Placeholder { - loading: !self.diff_edit.known_unsupported.contains(&hunk.path), - }), } files.push(DiffEditCardFile { path: hunk.path.as_str().into(), diff --git a/shell/gpui/src/repo/window/diff_edit.rs b/shell/gpui/src/repo/window/diff_edit/session.rs similarity index 79% rename from shell/gpui/src/repo/window/diff_edit.rs rename to shell/gpui/src/repo/window/diff_edit/session.rs index 90320b2b..98b8fd9a 100644 --- a/shell/gpui/src/repo/window/diff_edit.rs +++ b/shell/gpui/src/repo/window/diff_edit/session.rs @@ -3,8 +3,9 @@ use std::collections::BTreeSet; use gpui::Context; use jayjay_core::diff::change_groups; -use super::diff_edit_state::{checkbox_state, hunk_supports_diff_edit, next_diff_edit_session}; -use super::{DiffEditCheckboxState, DiffEditState, RepoWindow, TextModalAction}; +use super::state::{DiffEditCheckboxState, DiffEditState}; +use super::state::{checkbox_state, hunk_supports_diff_edit, next_diff_edit_session}; +use crate::repo::window::{RepoWindow, TextModalAction}; impl RepoWindow { pub fn enter_diff_edit(&mut self, cx: &mut Context) { @@ -29,6 +30,10 @@ impl RepoWindow { self.diff_edit.change_id = Some(change_id); self.diff_edit.working_copy = working_copy; self.diff_edit.message = description; + self.diff_edit.loaded_ignore_whitespace = self.vm.read(cx).ignore_whitespace; + self.diff_edit.loaded_commit = self.selected_commit_id(cx); + self.seed_diff_edit_collapse(cx); + self.spawn_diff_edit_stats(cx); self.ensure_diff_edit_files(cx); if let Some(files) = self.vm.read(cx).files.clone() { self.vm @@ -212,6 +217,31 @@ impl RepoWindow { self.exit_diff_edit(cx); return; } + // A whitespace-mode change or an amend invalidates every loaded diff AND the selections: selected values are full-diff row indices that silently remap when rows shift, and apply would submit them under the new state. + let ignore_whitespace = self.vm.read(cx).ignore_whitespace; + let commit = self.selected_commit_id(cx); + if self.diff_edit.loaded_ignore_whitespace != ignore_whitespace + || (commit.is_some() && self.diff_edit.loaded_commit != commit) + { + self.diff_edit.loaded_ignore_whitespace = ignore_whitespace; + self.diff_edit.loaded_commit = commit; + // A new session token kills every in-flight completion (card loads, stats) so a superseded compute can't reinstall old-epoch state over the cleared maps. + self.diff_edit.session = next_diff_edit_session(); + // Old-epoch badges must not outlive the reset; the per-file pass rebuilds the folds from fresh stats. + self.diff_edit.stats = None; + self.diff_edit.loaded_files.clear(); + self.diff_edit.known_unsupported.clear(); + self.diff_edit.loading.clear(); + self.diff_edit.selected.clear(); + self.diff_edit.select_all_pending.clear(); + self.diff_edit.rows = None; + self.refresh_diff_edit_summary(); + self.spawn_diff_edit_stats(cx); + if let Some(files) = self.vm.read(cx).files.clone() { + self.vm + .update(cx, |vm, cx| vm.preload_diffs_async(files, cx)); + } + } self.ensure_diff_edit_files(cx); } diff --git a/shell/gpui/src/repo/window/diff_edit_state.rs b/shell/gpui/src/repo/window/diff_edit/state.rs similarity index 89% rename from shell/gpui/src/repo/window/diff_edit_state.rs rename to shell/gpui/src/repo/window/diff_edit/state.rs index 6c08eebe..389103c7 100644 --- a/shell/gpui/src/repo/window/diff_edit_state.rs +++ b/shell/gpui/src/repo/window/diff_edit/state.rs @@ -7,13 +7,13 @@ use jayjay_core::diff::{ CollapsedDiff, DiffSpanStyle, FileDiff, collapse_context_with_mapping, compute_file_diff_full, }; use jayjay_core::placeholder::is_editable_text; -use jayjay_core::{DiffHunk, HunkType}; +use jayjay_core::{DiffHunk, FileDiffStats, HunkType}; use crate::repo::view_model::DiffLoadState; use crate::ui::scrollbar::ScrollbarBoundsSlot; -use super::RepoWindow; -use super::diff_edit_rows::DiffEditRowModel; +use super::rows::DiffEditRowModel; +use crate::repo::window::RepoWindow; static NEXT_SESSION: AtomicU64 = AtomicU64::new(1); @@ -49,6 +49,13 @@ pub struct DiffEditState { pub(super) focus_pending: bool, pub(super) session: u64, pub(super) summary: (usize, usize), + pub(super) collapsed: HashSet, + pub(super) collapse_touched: bool, + pub(super) stats_commit: Option, + pub(super) loaded_ignore_whitespace: bool, + pub(super) loaded_commit: Option, + pub(super) focused: Option, + pub(super) stats: Option>, pub(super) rows: Option>, pub(super) message: String, pub(super) scroll: UniformListScrollHandle, @@ -69,6 +76,13 @@ impl Default for DiffEditState { focus_pending: false, session: 0, summary: (0, 0), + collapsed: HashSet::new(), + collapse_touched: false, + stats_commit: None, + loaded_ignore_whitespace: false, + loaded_commit: None, + focused: None, + stats: None, rows: None, message: String::new(), scroll: UniformListScrollHandle::new(), @@ -115,6 +129,14 @@ impl RepoWindow { let Some(hunks) = self.vm.read(cx).files.clone() else { return; }; + let focus_gone = self + .diff_edit + .focused + .as_deref() + .is_some_and(|focused| !hunks.iter().any(|hunk| hunk.path.as_str() == focused)); + if focus_gone { + self.diff_edit.focused = None; + } for hunk in hunks.iter() { let path = hunk.path.clone(); if self.diff_edit.loaded_files.contains_key(&path) diff --git a/shell/gpui/src/repo/window/diff_edit_view.rs b/shell/gpui/src/repo/window/diff_edit/view.rs similarity index 92% rename from shell/gpui/src/repo/window/diff_edit_view.rs rename to shell/gpui/src/repo/window/diff_edit/view.rs index 1a316f89..092ea13a 100644 --- a/shell/gpui/src/repo/window/diff_edit_view.rs +++ b/shell/gpui/src/repo/window/diff_edit/view.rs @@ -4,10 +4,10 @@ use gpui::{ }; use jayjay_core::DiffEditDestination; -use super::RepoWindow; -use super::diff_edit_cards::diff_edit_body; +use super::cards::diff_edit_body; use crate::app::fonts; use crate::app::theme::Theme; +use crate::repo::window::RepoWindow; use crate::ui::icons::{glyph, icon}; use crate::ui::primitives::{button, divider_h}; @@ -21,7 +21,7 @@ pub struct DiffEditSnapshot { pub selected_lines: usize, } -pub(super) fn diff_edit_view( +pub(crate) fn diff_edit_view( view: &mut RepoWindow, t: &Theme, cx: &mut Context, @@ -59,6 +59,10 @@ fn header(view: &RepoWindow, t: &Theme, cx: &mut Context) -> AnyElem if !selecting { toggle = toggle.on_click(cx.listener(|view, _, _, cx| view.toggle_diff_edit_all(cx))); } + let collapse_all = button("diff-edit-collapse-all", "Collapse All", t, false) + .on_click(cx.listener(|view, _, _, cx| view.collapse_all_diff_edit(cx))); + let expand_all = button("diff-edit-expand-all", "Expand All", t, false) + .on_click(cx.listener(|view, _, _, cx| view.expand_all_diff_edit(cx))); div() .flex() .items_center() @@ -86,6 +90,8 @@ fn header(view: &RepoWindow, t: &Theme, cx: &mut Context) -> AnyElem .text_color(rgb(t.fg_dim)) .child(summary), ) + .child(collapse_all) + .child(expand_all) .child(toggle) .child( button("diff-edit-cancel", "Cancel", t, false) diff --git a/shell/gpui/src/repo/window/mod.rs b/shell/gpui/src/repo/window/mod.rs index 3cef98a5..63a130d8 100644 --- a/shell/gpui/src/repo/window/mod.rs +++ b/shell/gpui/src/repo/window/mod.rs @@ -7,12 +7,6 @@ mod dag; mod dag_row; mod detail; mod diff_edit; -mod diff_edit_apply; -mod diff_edit_cards; -mod diff_edit_gutter; -mod diff_edit_rows; -mod diff_edit_state; -mod diff_edit_view; mod diff_rows; mod diff_select; mod drag; @@ -46,8 +40,7 @@ mod view; mod workspace; pub use commit_ai::CommitMessageProvider; -pub use diff_edit_state::{DiffEditCheckboxState, DiffEditState}; -pub use diff_edit_view::DiffEditSnapshot; +pub use diff_edit::{DiffEditCheckboxState, DiffEditSnapshot, DiffEditState}; pub use file_actions::SplitFilesRequest; pub use file_actions_batch::FileBatchAction; pub use open::open_repo_window; diff --git a/shell/gpui/src/repo/window/nav.rs b/shell/gpui/src/repo/window/nav.rs index 50ed2b8a..56e5576b 100644 --- a/shell/gpui/src/repo/window/nav.rs +++ b/shell/gpui/src/repo/window/nav.rs @@ -12,6 +12,9 @@ impl RepoWindow { if self.find.query.is_some() { return false; } + if self.diff_edit_active() && self.handle_diff_edit_nav_key(ev, cx) { + return true; + } if let Some(direction) = navigation::list_nav_from_key(ev, ListNavKeys::CONTENT_LIST) { self.move_selection(direction, cx); return true; diff --git a/shell/gpui/src/repo/window/render/mod.rs b/shell/gpui/src/repo/window/render/mod.rs index 162f841d..1462ef46 100644 --- a/shell/gpui/src/repo/window/render/mod.rs +++ b/shell/gpui/src/repo/window/render/mod.rs @@ -8,17 +8,17 @@ use gpui::{ }; use super::detail::detail_pane; -use super::diff_edit_view::diff_edit_view; +use super::diff_edit::diff_edit_view; use super::onboarding::onboarding_pane; use super::repo_switcher::render_repo_switcher; use super::sidebar::sidebar; use super::status_bar::status_bar; use super::{DragTarget, RepoWindow}; use crate::app::actions::{ - CopyDiffSelection, ForgetStaleBookmarks, GitFetchOrigin, GitPushDefault, NewWorkspace, - OpenAbout, OpenBookmarkManager, OpenCommandPalette, OpenFind, OpenOperationLog, - OpenRemoteRepository, OpenRepoInEditor, OpenRepoInTerminal, OpenSettings, Refresh, - SaveNoteComposer, ShowRepoInFileManager, + CopyDiffSelection, DiffEditCollapseAll, DiffEditExpandAll, ForgetStaleBookmarks, + GitFetchOrigin, GitPushDefault, NewWorkspace, OpenAbout, OpenBookmarkManager, + OpenCommandPalette, OpenFind, OpenOperationLog, OpenRemoteRepository, OpenRepoInEditor, + OpenRepoInTerminal, OpenSettings, Refresh, SaveNoteComposer, ShowRepoInFileManager, }; use crate::app::theme::theme; use crate::platform::append_menu_bar; @@ -155,6 +155,16 @@ impl Render for RepoWindow { view.submit_stacked_pr(cx); }, )) + .on_action(cx.listener(|view, _: &DiffEditExpandAll, _, cx| { + if view.diff_edit_active() { + view.expand_all_diff_edit(cx); + } + })) + .on_action(cx.listener(|view, _: &DiffEditCollapseAll, _, cx| { + if view.diff_edit_active() { + view.collapse_all_diff_edit(cx); + } + })) .on_key_down(cx.listener(|view, ev: &gpui::KeyDownEvent, window, cx| { if view.handle_stacked_pr_key(ev, cx) { return; @@ -271,8 +281,7 @@ impl Render for RepoWindow { if let Some(menu) = repo_switcher_overlay { root = root.child(menu); } - if self.diff_edit.active && self.diff_edit.focus_pending { - self.diff_edit.focus_pending = false; + if self.diff_edit_take_pending_focus() { window.focus(&self.focus_handle, cx); } if self.text_modal.as_ref().is_some_and(|m| m.focus_pending) { @@ -331,7 +340,7 @@ impl RepoWindow { self.close_find(cx); } else if self.revset_filter.is_some() { self.close_revset_filter(cx); - } else if self.diff_edit.active { + } else if self.diff_edit_active() { self.exit_diff_edit(cx); } else { return false; diff --git a/shell/gpui/src/ui/icons.rs b/shell/gpui/src/ui/icons.rs index 954cec81..66de88b3 100644 --- a/shell/gpui/src/ui/icons.rs +++ b/shell/gpui/src/ui/icons.rs @@ -10,6 +10,7 @@ pub const FONT: &str = "lucide"; pub const ARROW_DOWN_SVG: &str = "icons/arrow-down.svg"; pub const ARROW_UP_SVG: &str = "icons/arrow-up.svg"; pub const CIRCLE_SVG: &str = "icons/circle.svg"; +pub const CHECK_SVG: &str = "icons/check.svg"; pub const REFRESH_CW_SVG: &str = "icons/refresh-cw.svg"; pub const LOGO_SVG: &str = "icons/logo.svg"; diff --git a/shell/gpui/src/ui/primitives.rs b/shell/gpui/src/ui/primitives.rs index 3554075e..739b659a 100644 --- a/shell/gpui/src/ui/primitives.rs +++ b/shell/gpui/src/ui/primitives.rs @@ -1,12 +1,64 @@ use gpui::{ AnyElement, AnyView, App, AppContext, BoxShadow, ClickEvent, ClipboardItem, Context, Div, - InteractiveElement, IntoElement, ParentElement, Render, Role, SharedString, Stateful, - StatefulInteractiveElement, Styled, Toggled, UniformList, Window, div, hsla, px, rgb, + ElementId, InteractiveElement, IntoElement, ParentElement, Render, Role, SharedString, + Stateful, StatefulInteractiveElement, Styled, Toggled, UniformList, Window, div, hsla, px, rgb, + svg, }; use crate::app::theme::{Theme, theme}; use crate::ui::icons; +#[derive(Clone, Copy, PartialEq, Eq)] +pub enum CheckCircleState { + Off, + Partial, + On, +} + +/// Circular check shared by the file list's review mark and diff edit's selection checkbox; callers attach their own click handler. +pub fn check_circle( + id: impl Into, + state: CheckCircleState, + accent: u32, + t: &Theme, +) -> Stateful
{ + let mut circle = div() + .id(id) + .flex_none() + .flex() + .items_center() + .justify_center() + .w(px(14.)) + .h(px(14.)) + .rounded_full() + .border_1() + .border_color(rgb(if state == CheckCircleState::Off { + t.fg_faint + } else { + accent + })) + .cursor_pointer(); + match state { + CheckCircleState::Off => {} + CheckCircleState::Partial => { + circle = circle + .bg(rgb(accent)) + .child(div().w(px(6.)).h(px(2.)).rounded_full().bg(rgb(0xffffff))); + } + CheckCircleState::On => { + // An SVG check centers geometrically; the lucide text glyph sits visibly off-center in a 14px circle. + circle = circle.bg(rgb(accent)).child( + svg() + .path(icons::CHECK_SVG) + .w(px(8.)) + .h(px(8.)) + .text_color(rgb(0xffffff)), + ); + } + } + circle +} + pub fn toggle_button( glyph_str: &'static str, tooltip: &'static str, diff --git a/shell/gpui/tests/repo_diff_edit_cases/collapse.rs b/shell/gpui/tests/repo_diff_edit_cases/collapse.rs new file mode 100644 index 00000000..fb51f92f --- /dev/null +++ b/shell/gpui/tests/repo_diff_edit_cases/collapse.rs @@ -0,0 +1,420 @@ +use std::fs; + +use gpui::TestAppContext; +use jj_test::{FormatFixture, LinearFixture, run_jj_in}; + +use super::common::*; + +fn big_file_contents(lines: usize) -> String { + (0..lines).map(|i| format!("line {i}\n")).collect() +} + +fn one_big_working_copy_fixture(lines: usize, extra_small_files: usize) -> LinearFixture { + let fixture = LinearFixture::build(); + run_jj_in(&fixture.path, &["new"]); + fs::write(fixture.path.join("big.txt"), big_file_contents(lines)).unwrap(); + for ix in 0..extra_small_files { + fs::write(fixture.path.join(format!("small{ix}.txt")), "a\nb\n").unwrap(); + } + run_jj_in(&fixture.path, &["st"]); + fixture +} + +#[gpui::test] +fn toggling_collapse_hides_and_restores_line_rows_preserving_selection(cx: &mut TestAppContext) { + let fixture = two_file_edits_fixture(); + let (view, cx) = open_fixture(&fixture, cx); + select_file_by_path(&view, cx, "edit.txt"); + view.update_in(cx, |view, _, cx| view.enter_diff_edit(cx)); + settle_visual(cx); + + let (rows_before, selected) = view.update_in(cx, |view, _, cx| { + view.toggle_diff_edit_file("edit.txt", cx); + ( + view.diff_edit_line_rows("edit.txt", cx), + view.diff_edit_selected("edit.txt"), + ) + }); + assert!(rows_before > 0, "an expanded file lists its line rows"); + assert!(!selected.is_empty(), "the whole-file selection is recorded"); + + view.update_in(cx, |view, _, cx| { + view.toggle_diff_edit_collapse("edit.txt", cx) + }); + view.update_in(cx, |view, _, cx| { + assert_eq!( + view.diff_edit_line_rows("edit.txt", cx), + 0, + "a collapsed file drops its line rows" + ); + assert_eq!( + view.diff_edit_selected("edit.txt"), + selected, + "collapse leaves the selection intact" + ); + }); + + view.update_in(cx, |view, _, cx| { + view.toggle_diff_edit_collapse("edit.txt", cx) + }); + view.update_in(cx, |view, _, cx| { + assert_eq!( + view.diff_edit_line_rows("edit.txt", cx), + rows_before, + "expanding restores every line row" + ); + assert_eq!( + view.diff_edit_selected("edit.txt"), + selected, + "expand leaves the selection intact" + ); + }); +} + +#[gpui::test] +fn collapse_all_and_expand_all_toggle_every_file(cx: &mut TestAppContext) { + let fixture = two_file_edits_fixture(); + let (view, cx) = open_fixture(&fixture, cx); + view.update_in(cx, |view, _, cx| view.enter_diff_edit(cx)); + settle_visual(cx); + + let expanded = view.update_in(cx, |view, _, cx| { + ( + view.diff_edit_line_rows("edit.txt", cx), + view.diff_edit_line_rows("untouched.txt", cx), + ) + }); + assert!( + expanded.0 > 0 && expanded.1 > 0, + "both files start expanded" + ); + + view.update_in(cx, |view, _, cx| view.collapse_all_diff_edit(cx)); + view.update_in(cx, |view, _, cx| { + assert_eq!(view.diff_edit_line_rows("edit.txt", cx), 0); + assert_eq!(view.diff_edit_line_rows("untouched.txt", cx), 0); + assert!(view.diff_edit_collapsed("edit.txt")); + assert!(view.diff_edit_collapsed("untouched.txt")); + }); + + view.update_in(cx, |view, _, cx| view.expand_all_diff_edit(cx)); + view.update_in(cx, |view, _, cx| { + assert_eq!(view.diff_edit_line_rows("edit.txt", cx), expanded.0); + assert_eq!(view.diff_edit_line_rows("untouched.txt", cx), expanded.1); + assert!(!view.diff_edit_collapsed("edit.txt")); + assert!(!view.diff_edit_collapsed("untouched.txt")); + }); +} + +#[gpui::test] +fn collapse_all_keystroke_collapses_every_file(cx: &mut TestAppContext) { + let fixture = two_file_edits_fixture(); + let (view, cx) = open_fixture(&fixture, cx); + view.update_in(cx, |view, _, cx| view.enter_diff_edit(cx)); + settle_visual(cx); + + cx.simulate_keystrokes(&format!("{}-alt-c", jayjay_gpui::platform::MOD_KEY)); + settle_visual(cx); + assert!(view.read_with(cx, |view, _| view.diff_edit_collapsed("edit.txt"))); + + cx.simulate_keystrokes(&format!("{}-alt-e", jayjay_gpui::platform::MOD_KEY)); + settle_visual(cx); + assert!(view.read_with(cx, |view, _| !view.diff_edit_collapsed("edit.txt"))); +} + +#[gpui::test] +fn auto_collapse_hides_oversized_file_and_leaves_small_ones(cx: &mut TestAppContext) { + let fixture = one_big_working_copy_fixture(400, 30); + let (view, cx) = open_fixture(&fixture, cx); + view.update_in(cx, |view, _, cx| view.enter_diff_edit(cx)); + settle_visual(cx); + + view.update_in(cx, |view, _, _| { + assert!(view.diff_edit_stats_ready(), "per-file stats arrive"); + assert!( + view.diff_edit_collapsed("big.txt"), + "the oversized file starts collapsed" + ); + assert!(!view.diff_edit_collapsed("small0.txt")); + assert!(!view.diff_edit_collapsed("small1.txt")); + }); +} + +#[gpui::test] +fn auto_collapse_skips_diffs_at_the_file_cap(cx: &mut TestAppContext) { + let fixture = one_big_working_copy_fixture(1100, 28); + let (view, cx) = open_fixture(&fixture, cx); + view.update_in(cx, |view, _, cx| view.enter_diff_edit(cx)); + settle_visual(cx); + + view.update_in(cx, |view, _, _| { + assert!(view.diff_edit_stats_ready()); + assert!( + !view.diff_edit_collapsed("big.txt"), + "at or under the file cap nothing auto-collapses even when the total is large" + ); + assert!(!view.diff_edit_collapsed("small0.txt")); + }); +} + +#[gpui::test] +fn large_diff_seeds_collapse_before_per_file_stats_arrive(cx: &mut TestAppContext) { + let fixture = one_big_working_copy_fixture(1100, 30); + let (view, cx) = open_fixture(&fixture, cx); + settle_visual(cx); + + view.update_in(cx, |view, _, cx| { + view.enter_diff_edit(cx); + assert!( + !view.diff_edit_stats_ready(), + "per-file stats cannot have arrived inside the entry update" + ); + assert!( + view.diff_edit_collapsed("big.txt"), + "the whole-change stats seed collapse at entry" + ); + assert!(view.diff_edit_collapsed("small0.txt")); + assert!(view.diff_edit_collapsed("small1.txt")); + }); + + settle_visual(cx); + view.update_in(cx, |view, _, _| { + assert!(view.diff_edit_stats_ready()); + assert!( + view.diff_edit_collapsed("big.txt") && view.diff_edit_collapsed("small0.txt"), + "the per-file refinement never expands seeded folds" + ); + }); +} + +#[gpui::test] +fn cached_rich_preview_does_not_replace_the_raw_card(cx: &mut TestAppContext) { + let fixture = FormatFixture::build(); + let (view, cx) = open_repo(fixture.path.clone(), cx); + select_file_by_path(&view, cx, FormatFixture::NOTEBOOK); + view.update_in(cx, |view, _, cx| view.toggle_projection_rich_preview(cx)); + settle_visual(cx); + + view.update_in(cx, |view, _, cx| view.enter_diff_edit(cx)); + settle_visual(cx); + + let (notebook_lines, plist_lines) = view.update_in(cx, |view, _, cx| { + ( + view.diff_edit_preview_line_count(FormatFixture::NOTEBOOK, cx), + view.diff_edit_preview_line_count(FormatFixture::PLIST, cx), + ) + }); + let raw_lines = fs::read_to_string(fixture.path.join(FormatFixture::NOTEBOOK)) + .unwrap() + .lines() + .count(); + assert_eq!( + notebook_lines, raw_lines, + "the card shows the raw source rows its stats describe, not the cached processed preview" + ); + let xml_lines = jayjay_core::Repo::open(&fixture.path) + .expect("open repo") + .show_file("@", FormatFixture::PLIST) + .expect("projected plist") + .new + .content + .expect("projected content") + .lines() + .count(); + assert_eq!( + plist_lines, xml_lines, + "the auto-open plist card resolves its processed rows; the cache stores them under the virtual path a path scan can never match" + ); +} + +#[gpui::test] +fn whitespace_toggle_reloads_diff_edit_files(cx: &mut TestAppContext) { + let fixture = LinearFixture::build(); + fs::write(fixture.path.join("ws.txt"), "a b\nkeep\n").unwrap(); + run_jj_in(&fixture.path, &["describe", "-m", "base"]); + run_jj_in(&fixture.path, &["new"]); + fs::write(fixture.path.join("ws.txt"), "a b\nkeep\n").unwrap(); + run_jj_in(&fixture.path, &["st"]); + + let (view, cx) = open_fixture(&fixture, cx); + view.update_in(cx, |view, _, cx| view.enter_diff_edit(cx)); + settle_visual(cx); + + view.update_in(cx, |view, _, cx| { + view.toggle_diff_edit_file("ws.txt", cx); + assert_eq!( + view.diff_edit_file_state("ws.txt"), + jayjay_gpui::repo::window::DiffEditCheckboxState::All, + "the whitespace-only edit is selectable while the mode is exact" + ); + view.view_model() + .update(cx, |vm, cx| vm.toggle_ignore_whitespace(cx)); + }); + settle_visual(cx); + + view.update_in(cx, |view, _, cx| { + assert_eq!( + view.diff_edit_file_state("ws.txt"), + jayjay_gpui::repo::window::DiffEditCheckboxState::None, + "the mode change reloads the card, whose diff now has no changed lines" + ); + assert!( + view.diff_edit_selected("ws.txt").is_empty(), + "old-mode row indices are cleared with the loaded diffs" + ); + assert!( + view.diff_edit_line_rows("ws.txt", cx) > 0, + "the reload preloads every card instead of leaving it loading" + ); + }); +} + +#[gpui::test] +fn whitespace_toggle_replaces_stale_folds_with_fresh_stats(cx: &mut TestAppContext) { + let fixture = LinearFixture::build(); + let base: String = (0..400).map(|i| format!("line {i} x\n")).collect(); + fs::write(fixture.path.join("big.txt"), base).unwrap(); + run_jj_in(&fixture.path, &["describe", "-m", "base"]); + run_jj_in(&fixture.path, &["new"]); + let edited: String = (0..400).map(|i| format!("line {i} x\n")).collect(); + fs::write(fixture.path.join("big.txt"), edited).unwrap(); + for ix in 0..30 { + fs::write(fixture.path.join(format!("small{ix}.txt")), "a\nb\n").unwrap(); + } + run_jj_in(&fixture.path, &["st"]); + + let (view, cx) = open_fixture(&fixture, cx); + view.update_in(cx, |view, _, cx| view.enter_diff_edit(cx)); + settle_visual(cx); + + view.update_in(cx, |view, _, cx| { + assert!( + view.diff_edit_collapsed("big.txt"), + "exact mode folds the 400-line whitespace churn" + ); + view.view_model() + .update(cx, |vm, cx| vm.toggle_ignore_whitespace(cx)); + }); + settle_visual(cx); + + view.update_in(cx, |view, _, _| { + assert!( + view.diff_edit_stats_ready(), + "the reset respawns per-file stats" + ); + assert!( + !view.diff_edit_collapsed("big.txt"), + "fresh ignore-whitespace stats replace the stale exact-mode folds instead of unioning" + ); + }); +} + +#[gpui::test] +fn stats_pending_entry_collapses_provisionally_then_refines(cx: &mut TestAppContext) { + let fixture = one_big_working_copy_fixture(400, 30); + let (view, cx) = open_fixture(&fixture, cx); + settle_visual(cx); + + view.update_in(cx, |view, _, cx| { + view.view_model().update(cx, |vm, _| vm.change_stats = None); + view.enter_diff_edit(cx); + assert!( + view.diff_edit_collapsed("big.txt") && view.diff_edit_collapsed("small0.txt"), + "without whole-change stats a multi-file diff starts provisionally collapsed" + ); + }); + + settle_visual(cx); + view.update_in(cx, |view, _, _| { + assert!(view.diff_edit_stats_ready()); + assert!( + view.diff_edit_collapsed("big.txt"), + "the per-file policy keeps the oversized file folded" + ); + assert!( + !view.diff_edit_collapsed("small0.txt"), + "the provisional seed is replaced, so small files expand" + ); + }); +} + +#[gpui::test] +fn queued_stats_read_the_captured_commit_not_an_amended_replacement(cx: &mut TestAppContext) { + let fixture = one_big_working_copy_fixture(400, 30); + let (view, cx) = open_fixture(&fixture, cx); + view.update_in(cx, |view, _, cx| view.enter_diff_edit(cx)); + + // Amend through the shared core handle while the stats task is still queued and the vm lags; the query must stay pinned to the entry commit. + fs::write(fixture.path.join("big.txt"), "tiny\n").unwrap(); + view.update_in(cx, |view, _, cx| { + let repo = view.view_model().read(cx).repo.clone().expect("repo"); + repo.refresh_working_copy().expect("refresh"); + }); + settle_visual(cx); + + view.update_in(cx, |view, _, _| { + assert!(view.diff_edit_stats_ready()); + assert!( + view.diff_edit_collapsed("big.txt"), + "stats describe the on-screen commit's 400-line file, not the amended replacement" + ); + }); +} + +#[gpui::test] +fn refined_stats_reopen_folds_seeded_by_an_inflated_aggregate(cx: &mut TestAppContext) { + let fixture = one_big_working_copy_fixture(2, 30); + let (view, cx) = open_fixture(&fixture, cx); + settle_visual(cx); + + view.update_in(cx, |view, _, cx| { + // Models a >32 MiB text file: jj's aggregate counts its real lines while the displayed placeholder counts zero. + view.view_model().update(cx, |vm, _| { + vm.change_stats = Some(jayjay_core::DiffStats { + files_changed: 31, + insertions: 5000, + deletions: 5000, + }); + }); + view.enter_diff_edit(cx); + assert!( + view.diff_edit_collapsed("big.txt") && view.diff_edit_collapsed("small0.txt"), + "the inflated aggregate seeds every card collapsed" + ); + }); + + settle_visual(cx); + view.update_in(cx, |view, _, _| { + assert!(view.diff_edit_stats_ready()); + assert!( + !view.diff_edit_collapsed("big.txt") && !view.diff_edit_collapsed("small0.txt"), + "precise per-file stats replace the misleading aggregate seed and reopen the cards" + ); + }); +} + +#[gpui::test] +fn manual_toggle_before_stats_suppresses_auto_collapse(cx: &mut TestAppContext) { + let fixture = one_big_working_copy_fixture(400, 30); + let (view, cx) = open_fixture(&fixture, cx); + view.update_in(cx, |view, _, cx| { + view.enter_diff_edit(cx); + view.toggle_diff_edit_collapse("small0.txt", cx); + }); + settle_visual(cx); + + view.update_in(cx, |view, _, _| { + assert!( + view.diff_edit_stats_ready(), + "stats are still stored for badges" + ); + assert!( + !view.diff_edit_collapsed("big.txt"), + "a manual toggle before stats arrive blocks auto-collapse" + ); + assert!( + view.diff_edit_collapsed("small0.txt"), + "the user's manual collapse is preserved" + ); + }); +} diff --git a/shell/gpui/tests/repo_diff_edit_cases/common.rs b/shell/gpui/tests/repo_diff_edit_cases/common.rs index a24f9eb9..ced2f3ae 100644 --- a/shell/gpui/tests/repo_diff_edit_cases/common.rs +++ b/shell/gpui/tests/repo_diff_edit_cases/common.rs @@ -247,8 +247,15 @@ pub(super) fn open_fixture<'a>( fixture: &LinearFixture, cx: &'a mut TestAppContext, ) -> (Entity, &'a mut VisualTestContext) { + open_repo(fixture.path.clone(), cx) +} + +pub(super) fn open_repo( + path: std::path::PathBuf, + cx: &mut TestAppContext, +) -> (Entity, &mut VisualTestContext) { install_test_globals(cx); - let (view, cx) = cx.add_window_view(|_, cx| RepoWindow::new(fixture.path.clone(), cx)); + let (view, cx) = cx.add_window_view(|_, cx| RepoWindow::new(path, cx)); let cx: &mut VisualTestContext = cx; load_selected_change_files(&view, cx); settle_visual(cx); diff --git a/shell/gpui/tests/repo_diff_edit_cases/focus.rs b/shell/gpui/tests/repo_diff_edit_cases/focus.rs new file mode 100644 index 00000000..89d79afd --- /dev/null +++ b/shell/gpui/tests/repo_diff_edit_cases/focus.rs @@ -0,0 +1,205 @@ +use gpui::{Entity, Focusable, TestAppContext, VisualTestContext}; +use jayjay_gpui::repo::window::RepoWindow; + +use super::common::*; + +fn focus_window(view: &Entity, cx: &mut VisualTestContext) { + view.update_in(cx, |view, window, cx| { + view.focus_handle(cx).focus(window, cx); + }); +} + +fn ordered_paths(view: &Entity, cx: &mut VisualTestContext) -> Vec { + view.read_with(cx, |view, cx| { + view.view_model() + .read(cx) + .files + .as_ref() + .expect("files loaded") + .iter() + .map(|hunk| hunk.path.clone()) + .collect() + }) +} + +fn focused(view: &Entity, cx: &mut VisualTestContext) -> Option { + view.read_with(cx, |view, _| view.diff_edit_focused()) +} + +fn enter_focused_diff_edit( + cx: &mut TestAppContext, +) -> (Entity, &mut VisualTestContext) { + let fixture = two_file_edits_fixture(); + let (view, cx) = open_fixture(&fixture, cx); + view.update_in(cx, |view, _, cx| view.enter_diff_edit(cx)); + settle_visual(cx); + focus_window(&view, cx); + (view, cx) +} + +#[gpui::test] +fn entering_diff_edit_has_no_focus_until_first_keypress(cx: &mut TestAppContext) { + let (view, cx) = enter_focused_diff_edit(cx); + assert_eq!(focused(&view, cx), None); +} + +#[gpui::test] +fn j_and_k_move_focus_through_files_with_clamping(cx: &mut TestAppContext) { + let (view, cx) = enter_focused_diff_edit(cx); + let paths = ordered_paths(&view, cx); + assert!(paths.len() >= 2, "fixture exposes two edited files"); + + cx.simulate_keystrokes("j"); + assert_eq!(focused(&view, cx), Some(paths[0].clone())); + + cx.simulate_keystrokes("j"); + assert_eq!(focused(&view, cx), Some(paths[1].clone())); + + cx.simulate_keystrokes("j"); + assert_eq!( + focused(&view, cx), + Some(paths[1].clone()), + "j clamps at the last file" + ); + + cx.simulate_keystrokes("k"); + assert_eq!(focused(&view, cx), Some(paths[0].clone())); + + cx.simulate_keystrokes("k"); + assert_eq!( + focused(&view, cx), + Some(paths[0].clone()), + "k clamps at the first file" + ); +} + +#[gpui::test] +fn k_with_no_focus_lands_on_the_last_file(cx: &mut TestAppContext) { + let (view, cx) = enter_focused_diff_edit(cx); + let paths = ordered_paths(&view, cx); + cx.simulate_keystrokes("k"); + assert_eq!(focused(&view, cx), paths.last().cloned()); +} + +#[gpui::test] +fn enter_collapses_then_reexpands_the_focused_file(cx: &mut TestAppContext) { + let (view, cx) = enter_focused_diff_edit(cx); + let paths = ordered_paths(&view, cx); + let first = paths[0].clone(); + + cx.simulate_keystrokes("j"); + assert_eq!(focused(&view, cx), Some(first.clone())); + assert!(!view.read_with(cx, |view, _| view.diff_edit_collapsed(&first))); + + cx.simulate_keystrokes("enter"); + assert!(view.read_with(cx, |view, _| view.diff_edit_collapsed(&first))); + + cx.simulate_keystrokes("enter"); + assert!(!view.read_with(cx, |view, _| view.diff_edit_collapsed(&first))); +} + +#[gpui::test] +fn arrows_move_focus_and_left_right_collapse_and_expand(cx: &mut TestAppContext) { + let (view, cx) = enter_focused_diff_edit(cx); + let paths = ordered_paths(&view, cx); + let first = paths[0].clone(); + + cx.simulate_keystrokes("down"); + assert_eq!(focused(&view, cx), Some(first.clone())); + + cx.simulate_keystrokes("left"); + assert!(view.read_with(cx, |view, _| view.diff_edit_collapsed(&first))); + + cx.simulate_keystrokes("left"); + assert!( + view.read_with(cx, |view, _| view.diff_edit_collapsed(&first)), + "left on an already-collapsed card stays collapsed" + ); + + cx.simulate_keystrokes("right"); + assert!(!view.read_with(cx, |view, _| view.diff_edit_collapsed(&first))); + + cx.simulate_keystrokes("up"); + assert_eq!( + focused(&view, cx), + Some(first), + "up clamps at the first file" + ); +} + +#[gpui::test] +fn space_toggles_selection_of_the_focused_file(cx: &mut TestAppContext) { + let fixture = two_file_edits_fixture(); + let (view, cx) = open_fixture(&fixture, cx); + select_file_by_path(&view, cx, "edit.txt"); + view.update_in(cx, |view, _, cx| view.enter_diff_edit(cx)); + settle_visual(cx); + focus_window(&view, cx); + let paths = ordered_paths(&view, cx); + let first = paths[0].clone(); + let reviewed = |view: &Entity, cx: &mut VisualTestContext| { + view.read_with(cx, |view, cx| { + let vm = view.view_model().read(cx); + let change_id = vm.selected_change().expect("change").change_id.id.clone(); + let hunk = vm + .files + .as_ref() + .and_then(|files| files.iter().find(|h| h.path == "edit.txt").cloned()) + .expect("hunk"); + view.is_reviewed(&change_id, &hunk.path, &hunk.review_identity) + }) + }; + assert!(!reviewed(&view, cx)); + + cx.simulate_keystrokes("space"); + assert_eq!( + view.read_with(cx, |view, _| view.diff_edit_file_state(&first)), + jayjay_gpui::repo::window::DiffEditCheckboxState::None, + "space without focus selects nothing" + ); + assert!( + !reviewed(&view, cx), + "unfocused space is consumed; it must not toggle the file column's review mark behind the overlay" + ); + + cx.simulate_keystrokes("j"); + cx.simulate_keystrokes("space"); + assert_eq!( + view.read_with(cx, |view, _| view.diff_edit_file_state(&first)), + jayjay_gpui::repo::window::DiffEditCheckboxState::All, + "space selects the focused file's changed lines" + ); + + cx.simulate_keystrokes("space"); + assert_eq!( + view.read_with(cx, |view, _| view.diff_edit_file_state(&first)), + jayjay_gpui::repo::window::DiffEditCheckboxState::None, + "space toggles the selection off again" + ); +} + +#[gpui::test] +fn focus_keys_do_not_move_the_underlying_selection(cx: &mut TestAppContext) { + let (view, cx) = enter_focused_diff_edit(cx); + let (selected, selected_file_ix) = view.read_with(cx, |view, cx| { + let vm = view.view_model().read(cx); + (vm.selected, vm.selected_file_ix) + }); + + for key in ["j", "j", "k"] { + cx.simulate_keystrokes(key); + } + + view.read_with(cx, |view, cx| { + let vm = view.view_model().read(cx); + assert_eq!(vm.selected, selected, "DAG selection stays put"); + assert_eq!( + vm.selected_file_ix, selected_file_ix, + "file-list selection stays put" + ); + }); + assert!( + focused(&view, cx).is_some(), + "the focus keys were consumed by diff edit" + ); +} diff --git a/shell/gpui/tests/repo_diff_edit_cases/loading.rs b/shell/gpui/tests/repo_diff_edit_cases/loading.rs index 5c2ca203..11e4e065 100644 --- a/shell/gpui/tests/repo_diff_edit_cases/loading.rs +++ b/shell/gpui/tests/repo_diff_edit_cases/loading.rs @@ -116,67 +116,32 @@ fn same_path_cache_entry_from_another_revision_is_ignored(cx: &mut TestAppContex #[gpui::test] fn unsupported_preview_replaces_cached_placeholder_when_cache_grows(cx: &mut TestAppContext) { let fixture = LinearFixture::build(); - fs::create_dir(fixture.path.join("moved")).unwrap(); - fs::rename( - fixture.path.join("README.md"), - fixture.path.join("moved/README.md"), - ) - .unwrap(); - fs::rename( - fixture.path.join("feature.txt"), - fixture.path.join("moved/feature.txt"), - ) - .unwrap(); - fs::write( - fixture.path.join("moved/README.md"), - "# Sample project\nrenamed readme\n", - ) - .unwrap(); fs::write( - fixture.path.join("moved/feature.txt"), - "feature\nrenamed feature\n", + fixture.path.join("analysis.ipynb"), + "{\n \"cells\": [\n {\"cell_type\": \"markdown\", \"metadata\": {}, \"source\": [\"# Title\"]}\n ],\n \"metadata\": {},\n \"nbformat\": 4,\n \"nbformat_minor\": 5\n}\n", ) .unwrap(); + fs::write(fixture.path.join("plain.txt"), "keep\n").unwrap(); run_jj_in(&fixture.path, &["st"]); let (view, cx) = open_fixture(&fixture, cx); - select_file_by_path(&view, cx, "moved/README.md"); + select_file_by_path(&view, cx, "plain.txt"); view.update_in(cx, |view, _, cx| { view.view_model().update(cx, |vm, _| vm.diff_cache.clear()); view.enter_diff_edit(cx); + assert_eq!( + view.diff_edit_preview_line_count("analysis.ipynb", cx), + 0, + "the row model must begin with a placeholder" + ); }); settle_visual(cx); - let before = view.update_in(cx, |view, _, cx| { - view.diff_edit_preview_line_count("moved/README.md", cx) - }); - assert_eq!(before, 0, "the row model must begin with a placeholder"); - - view.update_in(cx, |view, _, cx| { - view.view_model().update(cx, |vm, _| { - vm.diff_cache.insert( - "arrived-preview".into(), - LoadedDiff { - diff: std::sync::Arc::new(compute_file_diff( - "moved/README.md", - "old preview\n", - "new preview\n", - false, - )), - projection: None, - svg_preview: None, - markdown_preview: None, - old_content: Some("old preview\n".into()), - new_content: Some("new preview\n".into()), - }, - ); - }); - }); let preview_lines = view.update_in(cx, |view, _, cx| { - view.diff_edit_preview_line_count("moved/README.md", cx) + view.diff_edit_preview_line_count("analysis.ipynb", cx) }); assert!( preview_lines > 0, - "the newly cached preview must replace its cached placeholder" + "the preload's arrival must replace the cached placeholder" ); } diff --git a/shell/gpui/tests/repo_diff_edit_cases/mod.rs b/shell/gpui/tests/repo_diff_edit_cases/mod.rs index 7145465c..3efd298c 100644 --- a/shell/gpui/tests/repo_diff_edit_cases/mod.rs +++ b/shell/gpui/tests/repo_diff_edit_cases/mod.rs @@ -1,5 +1,7 @@ mod apply; +mod collapse; mod common; +mod focus; mod loading; mod mode; mod select_all; diff --git a/shell/mac/Packages/JayJayDiffUI/Sources/JayJayDiffUI/NativeDiffView.swift b/shell/mac/Packages/JayJayDiffUI/Sources/JayJayDiffUI/NativeDiffView.swift index 1be3e6f4..585c483a 100644 --- a/shell/mac/Packages/JayJayDiffUI/Sources/JayJayDiffUI/NativeDiffView.swift +++ b/shell/mac/Packages/JayJayDiffUI/Sources/JayJayDiffUI/NativeDiffView.swift @@ -12,6 +12,8 @@ public struct NativeDiffView: NSViewRepresentable { public var displayGroups: [ChangeGroup]? public var reserveNoteColumn: Bool public var compactGutterWidth: Bool + /// When set, the view sizes to its full content: inner scrolling is disabled and every laid-out height change is reported so the host can match its frame. + public var onContentHeightChanged: ((CGFloat) -> Void)? @Environment(\.colorScheme) private var colorScheme @Environment(\.diffFontSize) private var fontSize @@ -24,7 +26,8 @@ public struct NativeDiffView: NSViewRepresentable { displayLines: [DiffLine]? = nil, displayGroups: [ChangeGroup]? = nil, reserveNoteColumn: Bool = false, - compactGutterWidth: Bool = false + compactGutterWidth: Bool = false, + onContentHeightChanged: ((CGFloat) -> Void)? = nil ) { self.diff = diff self.gutterActions = gutterActions @@ -33,6 +36,7 @@ public struct NativeDiffView: NSViewRepresentable { self.displayGroups = displayGroups self.reserveNoteColumn = reserveNoteColumn self.compactGutterWidth = compactGutterWidth + self.onContentHeightChanged = onContentHeightChanged } public func updateNSView(_ containerView: DiffTextContainerView, context: Context) { @@ -295,6 +299,9 @@ public struct NativeDiffView: NSViewRepresentable { } containerView.onContentLayoutChanged = renderGutter + containerView.onContentHeightChanged = onContentHeightChanged + containerView.setFitsContent(onContentHeightChanged != nil) renderGutter() + containerView.reportContentHeightIfNeeded() } } diff --git a/shell/mac/Packages/JayJayDiffUI/Sources/JayJayDiffUI/Text/DiffTextContainerView.swift b/shell/mac/Packages/JayJayDiffUI/Sources/JayJayDiffUI/Text/DiffTextContainerView.swift index dc69c936..56c2e26f 100644 --- a/shell/mac/Packages/JayJayDiffUI/Sources/JayJayDiffUI/Text/DiffTextContainerView.swift +++ b/shell/mac/Packages/JayJayDiffUI/Sources/JayJayDiffUI/Text/DiffTextContainerView.swift @@ -11,6 +11,8 @@ public final class DiffTextContainerView: NSView { private var needsWidthPassAfterResize = false private(set) var gutterWidth: CGFloat = 0 var onContentLayoutChanged: (() -> Void)? + var onContentHeightChanged: ((CGFloat) -> Void)? + private var lastReportedHeight: CGFloat = -1 /// SBS rows are pre-wrapped by `wrap_sbs_rows`, so SBS callers set this `false` /// to stop the text container from re-wrapping on top. var wrapsText: Bool = true @@ -115,6 +117,34 @@ public final class DiffTextContainerView: NSView { ) } onContentLayoutChanged?() + reportContentHeightIfNeeded() + } + + /// Sized-to-content hosts (Diff Edit cards) disable inner scrolling so the outer scroll view owns the wheel; the default scrolling host keeps its own scroller. + func setFitsContent(_ fitsContent: Bool) { + let elasticity: NSScrollView.Elasticity = fitsContent ? .none : .automatic + scrollView.verticalScrollElasticity = elasticity + gutterScrollView.verticalScrollElasticity = elasticity + scrollView.hasVerticalScroller = !fitsContent + } + + func reportContentHeightIfNeeded() { + guard let onContentHeightChanged else { return } + let height = max(documentHeight(of: textView), documentHeight(of: gutterTextView)) + guard height > 0, abs(height - lastReportedHeight) > 0.5 else { return } + lastReportedHeight = height + // Reported async: updateNSView runs inside a SwiftUI update pass where setting @State is illegal. + DispatchQueue.main.async { + onContentHeightChanged(height) + } + } + + private func documentHeight(of textView: NSTextView) -> CGFloat { + guard let layoutManager = textView.layoutManager, + let textContainer = textView.textContainer + else { return 0 } + layoutManager.ensureLayout(for: textContainer) + return layoutManager.usedRect(for: textContainer).height + textView.textContainerInset.height * 2 } @objc private func gutterScrolled(_ notification: Notification) { diff --git a/shell/mac/Resources/HelpFeatures.json b/shell/mac/Resources/HelpFeatures.json index d0c8c4a0..bac4265f 100644 --- a/shell/mac/Resources/HelpFeatures.json +++ b/shell/mac/Resources/HelpFeatures.json @@ -50,7 +50,7 @@ "title": "Split work from a change", "summary": "Use diff edit mode to select files, hunks, or line ranges and extract them into another change.", "category": "Changes", - "keywords": ["help", "split", "diff edit", "extract", "hunk", "line range", "parallel change", "child change", "move edits", "discard lines"], + "keywords": ["help", "split", "diff edit", "extract", "hunk", "line range", "parallel change", "child change", "move edits", "discard lines", "collapse", "expand all", "line stats"], "guideAnchor": "diff-edit" }, { diff --git a/shell/mac/Resources/JayJayHelpBook/Contents/Resources/English.lproj/topics/split.html b/shell/mac/Resources/JayJayHelpBook/Contents/Resources/English.lproj/topics/split.html index 2a6fcbc0..15f3515d 100644 --- a/shell/mac/Resources/JayJayHelpBook/Contents/Resources/English.lproj/topics/split.html +++ b/shell/mac/Resources/JayJayHelpBook/Contents/Resources/English.lproj/topics/split.html @@ -33,6 +33,7 @@

Split work from a change

Open diff edit mode from a change, then select files, hunks, or line ranges. Selected edits can become a child change, a parallel change, or move into the working copy.

    +
  • File cards show +/− line counts and collapse in place; large diffs start collapsed. Expand All and Collapse All (⌥⌘E / ⌥⌘C) toggle every file, while j / k or the arrow keys move focus between cards, ← / → collapse or expand the focused card, Return toggles it, and Space selects its changed lines.
  • Use reviewed files as the selection model for batch split.
  • Choose a parallel change when the extracted edits should become a sibling, not a child.
  • Working-copy edits can be discarded at selected line granularity.
  • diff --git a/shell/mac/Sources/JayJay/Detail/ChangeDetailView.swift b/shell/mac/Sources/JayJay/Detail/ChangeDetailView.swift index 1ffbf9bb..dec284fe 100644 --- a/shell/mac/Sources/JayJay/Detail/ChangeDetailView.swift +++ b/shell/mac/Sources/JayJay/Detail/ChangeDetailView.swift @@ -46,6 +46,7 @@ struct ChangeDetailView: View { @State var showNotedFilesOnly = false @State var diffStats: DiffStats? @State var paneMode: DetailPaneMode = .files + @State var paneBeforeDiffEdit: ActivePane? @State var conflictedPaths: Set = [] @State var trackedGitLfsPaths: Set = [] @State var reviewedPaths: Set = [] @@ -111,6 +112,8 @@ struct ChangeDetailView: View { repo: repo, diffStore: diffStore, actions: actions, + diffStats: diffStats, + settings: appSettings, onDone: { paneMode = .files } ) } else { @@ -123,6 +126,19 @@ struct ChangeDetailView: View { } } .onAppear { resetState() } + // Diff edit must own j/k: the DAG's earlier-installed key monitor would otherwise consume them whenever the DAG was the active pane. + .onChange(of: paneMode.isDiffEdit) { _, isDiffEdit in + if isDiffEdit { + paneBeforeDiffEdit = activePane + activePane = .fileColumn + } else { + // Restore only if the user didn't click another pane while diff edit was open. + if activePane == .fileColumn, let previous = paneBeforeDiffEdit { + activePane = previous + } + paneBeforeDiffEdit = nil + } + } .onChange(of: detail.info.commitId) { _, _ in resetState(preservingFileContext: detail.info.isWorkingCopy) } diff --git a/shell/mac/Sources/JayJay/Detail/FileList/FileColumn+Selection.swift b/shell/mac/Sources/JayJay/Detail/FileList/FileColumn+Selection.swift index 50a7d01a..578e4112 100644 --- a/shell/mac/Sources/JayJay/Detail/FileList/FileColumn+Selection.swift +++ b/shell/mac/Sources/JayJay/Detail/FileList/FileColumn+Selection.swift @@ -4,12 +4,12 @@ import SwiftUI extension ChangeDetailView { func handleFileColumnKey(_ event: NSEvent) -> Bool { - if event.keyCode == 49 { // Space + if event.keyCode == KeyCode.space { return toggleReviewOnSelection() } switch event.keyCode { - case 125: return moveFileSelection(by: 1) // Down arrow - case 126: return moveFileSelection(by: -1) // Up arrow + case KeyCode.downArrow: return moveFileSelection(by: 1) + case KeyCode.upArrow: return moveFileSelection(by: -1) default: break } let isCtrl = event.modifierFlags.intersection(.deviceIndependentFlagsMask) == .control diff --git a/shell/mac/Sources/JayJay/Diff/DiffStore+ContentLoading.swift b/shell/mac/Sources/JayJay/Diff/DiffStore+ContentLoading.swift new file mode 100644 index 00000000..51e1b810 --- /dev/null +++ b/shell/mac/Sources/JayJay/Diff/DiffStore+ContentLoading.swift @@ -0,0 +1,85 @@ +import JayJayCore + +extension DiffStore { + /// Throwing twin of the render path's load: the same fetch sequence, with primary-fetch errors propagated so destructive callers (Done's strict reload) can abort instead of seeing empty content. + func loadContentStrict( + hunk: DiffHunk, + rev: String, + repo: JayJayRepo + ) async throws -> DiffLoadedContent { + try await Self.fetchContent( + repo: repo, hunk: hunk, rev: rev, fromRev: nil, projectionMode: nil + ) + } + + func loadFileContent( + repo: JayJayRepo, + hunk: DiffHunk, + rev: String?, + fromRev: String?, + projectionMode: DiffProjectionMode? + ) async -> DiffLoadedContent { + await (try? Self.fetchContent( + repo: repo, hunk: hunk, rev: rev, fromRev: fromRev, projectionMode: projectionMode + )) ?? DiffLoadedContent(oldContent: "", newContent: "") + } + + private static func fetchContent( + repo: JayJayRepo, + hunk: DiffHunk, + rev: String?, + fromRev: String?, + projectionMode: DiffProjectionMode? + ) async throws -> DiffLoadedContent { + let path = hunk.path + let hunkType = hunk.hunkType + let oldPath = hunk.oldPath + let raw = effectiveProjectionMode(hunk: hunk, mode: projectionMode) == .raw + if let fromRev, let rev { + return try await loadedContent { + raw + ? try repo.interdiffFileRaw(fromRev: fromRev, toRev: rev, path: path) + : try repo.interdiffFile(fromRev: fromRev, toRev: rev, path: path) + } + } + if let rev, hunkType == .renamed, let oldPath { + return try await loadedContent { + raw + ? try repo.showFileRenameRaw(rev: rev, oldPath: oldPath, newPath: path) + : try repo.showFileRename(rev: rev, oldPath: oldPath, newPath: path) + } + } + if let rev { + let loaded = try await loadedContent { + raw + ? try repo.showFileRaw(rev: rev, path: path) + : try repo.showFile(rev: rev, path: path) + } + if loaded.oldText.isEmpty, loaded.newText.isEmpty { + // Best-effort enhancement, kept soft even for strict callers: image hunks land here with empty text sides, and a hard failure would drop their previews. + let content = await Task.detached { + try? repo.fileContent(rev: rev, path: path) + }.value + if let content, !content.isEmpty { + return DiffLoadedContent(oldContent: nil, newContent: content) + } + } + return loaded + } + return DiffLoadedContent(oldContent: "", newContent: "") + } + + private static func loadedContent( + loading: @escaping () throws -> DiffHunk + ) async throws -> DiffLoadedContent { + let hunk = try await Task.detached { try loading() }.value + // Absent sides stay nil: core's staleness guard materializes them as None, and Some("") would abort the apply as stale. + return DiffLoadedContent( + oldContent: hunk.oldContent, + newContent: hunk.newContent, + oldPreview: hunk.oldPreview, + newPreview: hunk.newPreview, + projection: hunk.projection + ) + } +} diff --git a/shell/mac/Sources/JayJay/Diff/DiffStore.swift b/shell/mac/Sources/JayJay/Diff/DiffStore.swift index f09619f1..14648fc7 100644 --- a/shell/mac/Sources/JayJay/Diff/DiffStore.swift +++ b/shell/mac/Sources/JayJay/Diff/DiffStore.swift @@ -7,11 +7,25 @@ final class DiffStore { let diff: FileDiff let content: DiffLoadedContent - var oldContent: String { content.oldText } - var newContent: String { content.newText } - var oldPreview: DiffPreview? { content.oldPreview } - var newPreview: DiffPreview? { content.newPreview } - var projection: DiffProjection? { content.projection } + var oldContent: String { + content.oldText + } + + var newContent: String { + content.newText + } + + var oldPreview: DiffPreview? { + content.oldPreview + } + + var newPreview: DiffPreview? { + content.newPreview + } + + var projection: DiffProjection? { + content.projection + } } struct CacheKeyParts { @@ -74,8 +88,8 @@ final class DiffStore { } var content = DiffLoadedContent( - oldContent: hunk.oldContent ?? "", - newContent: hunk.newContent ?? "", + oldContent: hunk.oldContent, + newContent: hunk.newContent, oldPreview: hunk.oldPreview, newPreview: hunk.newPreview, projection: hunk.projection @@ -132,7 +146,9 @@ final class DiffStore { preloadTask?.cancel() preloadTask = Task.detached(priority: .utility) { [weak self] in for hunk in hunks { - if Task.isCancelled { return } + if Task.isCancelled { + return + } _ = await self?.loadDiff( hunk: hunk, rev: rev, commitId: commitId, repo: repo, compareFromRev: compareFromRev, ignoreWhitespace: ignoreWhitespace, @@ -142,76 +158,6 @@ final class DiffStore { } } - // MARK: - Private - - private func loadFileContent( - repo: JayJayRepo, - hunk: DiffHunk, - rev: String?, - fromRev: String?, - projectionMode: DiffProjectionMode? - ) async -> DiffLoadedContent { - let path = hunk.path - let hunkType = hunk.hunkType - let oldPath = hunk.oldPath - let raw = Self.effectiveProjectionMode(hunk: hunk, mode: projectionMode) == .raw - if let fromRev, let rev { - return await loadRepositoryHunk(raw: raw) { - try repo.interdiffFileRaw(fromRev: fromRev, toRev: rev, path: path) - } processed: { - try repo.interdiffFile(fromRev: fromRev, toRev: rev, path: path) - } - } - if let rev, hunkType == .renamed, let oldPath { - return await loadRepositoryHunk(raw: raw) { - try repo.showFileRenameRaw(rev: rev, oldPath: oldPath, newPath: path) - } processed: { - try repo.showFileRename(rev: rev, oldPath: oldPath, newPath: path) - } - } - if let rev { - let loaded = await loadRepositoryHunk(raw: raw) { - try repo.showFileRaw(rev: rev, path: path) - } processed: { - try repo.showFile(rev: rev, path: path) - } - if loaded.oldText.isEmpty, loaded.newText.isEmpty { - let content = await Task.detached { - try? repo.fileContent(rev: rev, path: path) - }.value - if let content, !content.isEmpty { - return DiffLoadedContent( - oldContent: "", - newContent: content - ) - } - } - return loaded - } - return DiffLoadedContent(oldContent: "", newContent: "") - } - - private func loadRepositoryHunk( - raw: Bool, - raw rawLoad: @escaping () throws -> DiffHunk, - processed processedLoad: @escaping () throws -> DiffHunk - ) async -> DiffLoadedContent { - let hunk = await Task.detached { - raw ? (try? rawLoad()) : (try? processedLoad()) - }.value - return Self.loadedContent(from: hunk) - } - - private static func loadedContent(from hunk: DiffHunk?) -> DiffLoadedContent { - DiffLoadedContent( - oldContent: hunk?.oldContent ?? "", - newContent: hunk?.newContent ?? "", - oldPreview: hunk?.oldPreview, - newPreview: hunk?.newPreview, - projection: hunk?.projection - ) - } - nonisolated static func key(_ parts: CacheKeyParts) -> String { let base = (parts.commitId?.isEmpty == false ? parts.commitId : parts.rev) ?? "" let identity = parts.compareFromRev.map { "\($0)→\(base)" } ?? base diff --git a/shell/mac/Sources/JayJay/DiffEdit/DiffEditFileSection+Gutter.swift b/shell/mac/Sources/JayJay/DiffEdit/DiffEditFileSection+Gutter.swift new file mode 100644 index 00000000..47802abe --- /dev/null +++ b/shell/mac/Sources/JayJay/DiffEdit/DiffEditFileSection+Gutter.swift @@ -0,0 +1,39 @@ +import JayJayCore +import JayJayDiffUI +import SwiftUI + +extension DiffEditFileSection { + var currentSelectedLineRange: ClosedRange? { + nil + } + + func didSelectLines(_ lineRange: ClosedRange) {} + + func selectFile() { + onSelectFile() + } + + func selectChangeGroup(_ lineRange: ClosedRange) { + let mapped = ClosedRange( + uncheckedBounds: ( + displayToFullMap[lineRange.lowerBound] ?? lineRange.lowerBound, + displayToFullMap[lineRange.upperBound] ?? lineRange.upperBound + ) + ) + onSelectHunk(mapped) + } + + func lineCheckboxState(for lineNumber: Int) -> DiffGutterCheckboxState? { + guard let fileDiff, + let fullLine = displayToFullMap[lineNumber], + fileDiff.lines.indices.contains(fullLine - 1), + fileDiff.lines[fullLine - 1].isChanged + else { return nil } + return selectedChangedLines.contains(fullLine) ? .selected : .unselected + } + + func toggleLineCheckbox(_ lineNumber: Int) { + guard let fullLine = displayToFullMap[lineNumber] else { return } + onToggleLine(fullLine) + } +} diff --git a/shell/mac/Sources/JayJay/DiffEdit/DiffEditFileSection.swift b/shell/mac/Sources/JayJay/DiffEdit/DiffEditFileSection.swift index fa13c476..5b503f4c 100644 --- a/shell/mac/Sources/JayJay/DiffEdit/DiffEditFileSection.swift +++ b/shell/mac/Sources/JayJay/DiffEdit/DiffEditFileSection.swift @@ -9,20 +9,30 @@ struct DiffEditFileSection: View, DiffGutterSelectionActions { let repo: JayJayRepo? let diffStore: DiffStore let selectedChangedLines: Set + let stats: FileDiffStats? + let isCollapsed: Bool + let isFocused: Bool + let onToggleCollapse: () -> Void let onToggleFile: () -> Void let onSelectFile: () -> Void let onToggleLine: (Int) -> Void let onSelectHunk: (ClosedRange) -> Void let onLoaded: (DiffEditLoadedFile) -> Void - @State private var fileDiff: FileDiff? + @State var fileDiff: FileDiff? /// Collapsed version for display, with index map back to full diff. @State private var displayDiff: FileDiff? - @State private var displayToFullMap: [Int: Int] = [:] + @State var displayToFullMap: [Int: Int] = [:] @State private var oldContent: String? @State private var newContent: String? @State private var loadError: String? @State private var isLoading = false + @State private var measuredHeight: CGFloat? + @State private var loadedKey: String? + + private var loadKey: String { + "\(rev)|\(hunk.path)|\(settings.ignoreWhitespace)" + } @Environment(AppSettings.self) private var settings @Environment(\.jayjayFontSize) private var jayjayFontSize @@ -31,18 +41,17 @@ struct DiffEditFileSection: View, DiffGutterSelectionActions { var body: some View { VStack(alignment: .leading, spacing: 10) { header - content + if !isCollapsed { + content + } } .padding(14) .background(Color.primary.opacity(0.025), in: RoundedRectangle(cornerRadius: 14, style: .continuous)) .overlay( RoundedRectangle(cornerRadius: 14, style: .continuous) - .stroke( - selectedChangedLines.isEmpty ? Color.primary.opacity(0.08) : Color.accentColor.opacity(0.35), - lineWidth: 1 - ) + .stroke(borderColor, lineWidth: isFocused ? 2 : 1) ) - .task(id: "\(rev)|\(hunk.path)|\(settings.ignoreWhitespace)") { + .task(id: loadKey) { await loadDiff() } .environment(\.diffFontSize, jayjayFontSize) @@ -50,12 +59,23 @@ struct DiffEditFileSection: View, DiffGutterSelectionActions { } private var header: some View { - HStack(spacing: 8) { + let selection = headerSelection + return HStack(spacing: 8) { + Button(action: onToggleCollapse) { + Image(systemName: isCollapsed ? "chevron.right" : "chevron.down") + .jayjayFont(11) + .foregroundStyle(.secondary) + .frame(width: 12) + } + .buttonStyle(.plain) + .accessibilityIdentifier(AID.DiffEdit.fileToggle(hunk.path)) if supportsDiffEdit { Button(action: onToggleFile) { - Text(fileCheckboxText) - .jayjayFont(12, weight: .semibold, design: .monospaced) - .foregroundStyle(selectedChangedLines.isEmpty ? .secondary : Color.accentColor) + Image(systemName: selection.state.systemImage) + .foregroundStyle( + selection.state == .none ? Color.secondary.opacity(0.4) : Color.accentColor + ) + .jayjayFont(14) } .buttonStyle(.plain) } @@ -64,22 +84,51 @@ struct DiffEditFileSection: View, DiffGutterSelectionActions { Text(hunk.path) .jayjayFont(13, weight: .semibold, design: .monospaced) .textSelection(.enabled) - if supportsDiffEdit, let fileDiff { - Text(selectionBadgeText(fileDiff: fileDiff)) + if supportsDiffEdit, let partialText = selection.partialText { + Text(partialText) .jayjayFont(10, weight: .semibold) .padding(.horizontal, 8) .padding(.vertical, 4) .background(Color.accentColor.opacity(0.14), in: Capsule()) } - Spacer() - if supportsDiffEdit { - Text("Select files or lines to edit") - .jayjayFont(11) - .foregroundStyle(.secondary) - } else { - Text("Text edits not supported") - .jayjayFont(11) - .foregroundStyle(.secondary) + statsLabel + // Toggle lives on the filler, not the whole header, so the path's text selection and checkbox keep their own taps. + HStack(spacing: 8) { + Spacer(minLength: 12) + hintText + } + .contentShape(Rectangle()) + .onTapGesture(perform: onToggleCollapse) + } + } + + @ViewBuilder + private var hintText: some View { + if supportsDiffEdit { + Text("Select files or lines to edit") + .jayjayFont(11) + .foregroundStyle(.secondary) + } else { + Text("Text edits not supported") + .jayjayFont(11) + .foregroundStyle(.secondary) + } + } + + @ViewBuilder + private var statsLabel: some View { + if let stats, stats.insertions > 0 || stats.deletions > 0 { + HStack(spacing: 4) { + if stats.insertions > 0 { + Text("+\(stats.insertions)") + .jayjayFont(11, weight: .semibold, design: .monospaced) + .foregroundStyle(.green) + } + if stats.deletions > 0 { + Text("-\(stats.deletions)") + .jayjayFont(11, weight: .semibold, design: .monospaced) + .foregroundStyle(.red) + } } } } @@ -99,9 +148,14 @@ struct DiffEditFileSection: View, DiffGutterSelectionActions { diff: displayDiff, gutterActions: supportsDiffEdit ? self - : nil + : nil, + onContentHeightChanged: { height in + if abs((measuredHeight ?? 0) - height) > 0.5 { + measuredHeight = height + } + } ) - .frame(height: diffHeight(for: displayDiff)) + .frame(height: measuredHeight ?? estimatedHeight(for: displayDiff)) } else { Text("No textual preview available for this file.") .jayjayFont(12) @@ -117,43 +171,77 @@ struct DiffEditFileSection: View, DiffGutterSelectionActions { && DiffPlaceholder.isEditableText(newContent) } - private var fileCheckboxText: String { - selectedChangedLines.isEmpty ? "[ ]" : "[x]" + enum FileSelectionState { + case none, partial, all + + var systemImage: String { + switch self { + case .none: "circle" + case .partial: "minus.circle.fill" + case .all: "checkmark.circle.fill" + } + } + } + + /// One O(lines) pass shared by the checkbox and the partial badge; the header renders on every selection change. + private var headerSelection: (state: FileSelectionState, partialText: String?) { + guard let fileDiff else { + return (selectedChangedLines.isEmpty ? .none : .all, nil) + } + var changed = 0 + var selected = 0 + for (index, line) in fileDiff.lines.enumerated() where line.isChanged { + changed += 1 + if selectedChangedLines.contains(index + 1) { + selected += 1 + } + } + if selected == 0 || changed == 0 { + return (.none, nil) + } + if selected == changed { + return (.all, nil) + } + return (.partial, "\(selected) / \(changed) lines") + } + + private var borderColor: Color { + if isFocused { + return Color.accentColor.opacity(0.7) + } + return selectedChangedLines.isEmpty ? Color.primary.opacity(0.08) : Color.accentColor.opacity(0.35) } private func loadDiff() async { guard let repo else { return } - // Already loaded for this file — skip (LazyVStack re-triggers .task on scroll) - if fileDiff != nil, displayDiff != nil { return } + // Skip only when this exact rev+mode is loaded (LazyVStack re-triggers .task on scroll); a whitespace-mode change must reload so rows, badges, and apply agree. + let key = loadKey + if loadedKey == key { + return + } isLoading = true loadError = nil - // Reuse DiffStore for file content loading (cached if already loaded by DiffSection) + // Reuse DiffStore for file content loading (cached if already loaded by DiffSection); the request mode keeps auto-open formats on the processed rows their stats count. let cached = await diffStore.loadDiff( hunk: hunk, rev: rev, commitId: commitId, repo: repo, - ignoreWhitespace: settings.ignoreWhitespace + ignoreWhitespace: settings.ignoreWhitespace, + projectionMode: DiffProjectionDisplayPolicy.requestMode(for: hunk.projection, richView: false) ) - let old = cached?.oldContent - let new = cached?.newContent - let ignoreWhitespace = settings.ignoreWhitespace - let path = hunk.path - - // DiffEdit needs the full (uncollapsed) diff — line selection indices - // must match the full diff the Rust side computes when applying. - let diff = await Task.detached { - repo.computeNativeDiffFull( - path: path, oldContent: old ?? "", newContent: new ?? "", - ignoreWhitespace: ignoreWhitespace - ) - }.value + let loaded = await DiffEditLoadedFile.make( + hunk: hunk, oldContent: cached?.content.oldContent, newContent: cached?.content.newContent, + repo: repo, ignoreWhitespace: settings.ignoreWhitespace + ) + // The detached work outlives .task(id:) cancellation; a superseded mode's result must not install over the replacement's. + guard !Task.isCancelled, loadKey == key else { return } - oldContent = old - newContent = new - fileDiff = diff + oldContent = loaded.oldContent + newContent = loaded.newContent + fileDiff = loaded.diff // Collapse context for display, with mapping back to full diff line numbers - let collapsed = repo.collapseDiffWithMapping(diff: diff) + let collapsed = repo.collapseDiffWithMapping(diff: loaded.diff) displayDiff = collapsed.diff displayToFullMap = Dictionary( uniqueKeysWithValues: collapsed.displayToFull.map { @@ -162,73 +250,13 @@ struct DiffEditFileSection: View, DiffGutterSelectionActions { ) isLoading = false - onLoaded(DiffEditLoadedFile(hunk: hunk, oldContent: old, newContent: new, diff: diff)) + loadedKey = key + onLoaded(loaded) } - private func diffHeight(for diff: FileDiff) -> CGFloat { + /// Placeholder until the first real layout reports; full content height, never an inner-scroll cap. + private func estimatedHeight(for diff: FileDiff) -> CGFloat { let lineHeight = max(18, CGFloat(settings.fontSize) + 5) - return min(max(CGFloat(max(diff.lines.count, 4)) * lineHeight + 24, 120), 680) - } - - private func selectionBadgeText(fileDiff: FileDiff) -> String { - let changedLineCount = fileDiff.lines.filter(\.isChanged).count - let selectedLineCount = fileDiff.lines.enumerated().reduce(into: 0) { count, entry in - let lineNumber = entry.offset + 1 - if entry.element.isChanged, selectedChangedLines.contains(lineNumber) { - count += 1 - } - } - if selectedLineCount == changedLineCount { - return "File" - } - if selectedLineCount == 0 { - return "None" - } - return "\(selectedLineCount) / \(changedLineCount) lines" - } - - private func lineCheckboxState(fileDiff: FileDiff, lineNumber: Int) -> DiffGutterCheckboxState? { - let lineIndex = lineNumber - 1 - guard fileDiff.lines.indices.contains(lineIndex) else { return nil } - guard fileDiff.lines[lineIndex].isChanged else { return nil } - return selectedChangedLines.contains(lineNumber) ? .selected : .unselected - } - - var currentSelectedLineRange: ClosedRange? { - nil - } - - func didSelectLines(_ lineRange: ClosedRange) {} - - func selectFile() { - onSelectFile() - } - - func selectChangeGroup(_ lineRange: ClosedRange) { - let mapped = ClosedRange( - uncheckedBounds: ( - displayToFullMap[lineRange.lowerBound] ?? lineRange.lowerBound, - displayToFullMap[lineRange.upperBound] ?? lineRange.upperBound - ) - ) - onSelectHunk(mapped) - } - - func lineCheckboxState(for lineNumber: Int) -> DiffGutterCheckboxState? { - guard let fileDiff, - let fullLine = displayToFullMap[lineNumber] - else { return nil } - return lineCheckboxState(fileDiff: fileDiff, lineNumber: fullLine) - } - - func toggleLineCheckbox(_ lineNumber: Int) { - guard let fullLine = displayToFullMap[lineNumber] else { return } - onToggleLine(fullLine) - } -} - -private extension DiffLine { - var isChanged: Bool { - style == .added || style == .removed + return max(CGFloat(max(diff.lines.count, 1)) * lineHeight + 24, 44) } } diff --git a/shell/mac/Sources/JayJay/DiffEdit/DiffEditModels.swift b/shell/mac/Sources/JayJay/DiffEdit/DiffEditModels.swift index dc9ed6b2..8c72d06c 100644 --- a/shell/mac/Sources/JayJay/DiffEdit/DiffEditModels.swift +++ b/shell/mac/Sources/JayJay/DiffEdit/DiffEditModels.swift @@ -29,6 +29,35 @@ struct DiffEditLoadedFile { changedLineNumbers.filter(selectedLines.contains).count } + /// Content equality is sufficient: the diff is deterministic in (path, contents, whitespace mode), and comparing diffs would false-trip between highlighted and plain variants. + func hasSameSelectionBasis(as other: DiffEditLoadedFile) -> Bool { + oldContent == other.oldContent && newContent == other.newContent + } + + /// Always the full (uncollapsed) diff — line selection indices must match what the Rust side computes when applying. Selection-only callers pass highlight: false; tree-sitter setup costs tens of milliseconds per file. + static func make( + hunk: DiffHunk, + oldContent: String?, + newContent: String?, + repo: JayJayRepo, + ignoreWhitespace: Bool, + highlight: Bool = true + ) async -> DiffEditLoadedFile { + let path = hunk.path + let old = oldContent ?? "" + let new = newContent ?? "" + let diff = await Task.detached { + highlight + ? repo.computeNativeDiffFull( + path: path, oldContent: old, newContent: new, ignoreWhitespace: ignoreWhitespace + ) + : repo.computeNativeDiffFullPlain( + path: path, oldContent: old, newContent: new, ignoreWhitespace: ignoreWhitespace + ) + }.value + return DiffEditLoadedFile(hunk: hunk, oldContent: oldContent, newContent: newContent, diff: diff) + } + func makeSelection(selectedLines: Set) -> DiffEditFileSelection? { let lineNumbers = changedLineNumbers.filter(selectedLines.contains) let ranges = collapseRanges(lineNumbers) @@ -72,7 +101,7 @@ private func collapseRanges(_ lineNumbers: [Int]) -> [ClosedRange] { return ranges } -private extension DiffLine { +extension DiffLine { var isChanged: Bool { style == .added || style == .removed } diff --git a/shell/mac/Sources/JayJay/DiffEdit/DiffEditSession+Prepare.swift b/shell/mac/Sources/JayJay/DiffEdit/DiffEditSession+Prepare.swift new file mode 100644 index 00000000..0eb0a6bc --- /dev/null +++ b/shell/mac/Sources/JayJay/DiffEdit/DiffEditSession+Prepare.swift @@ -0,0 +1,101 @@ +import JayJayCore +import JayJayDiffUI + +extension DiffEditSession { + /// removeFromSource keeps only selected lines, so every editable hunk is strictly reloaded first — card loads go through DiffStore, which converts repository errors into empty content that would silently keep a file whole. + func prepareRemoveFromSource() { + guard let repo else { + finishApply(.removeFromSource) + return + } + removalTask?.cancel() + isPreparingRemoval = true + let hunks = detail.diff + let renderedByPath = loadedFiles + let ignoreWhitespace = settings.ignoreWhitespace + removalTask = Task { + defer { isPreparingRemoval = false } + let outcome = await strictReloadAll( + hunks: hunks, repo: repo, ignoreWhitespace: ignoreWhitespace + ) + // One guard for every published outcome: a mode toggle mid-preparation cancels this task and wipes the selections, so nothing from the old-mode reload — the failure alert included — may reach the replacement session. + guard !Task.isCancelled, settings.ignoreWhitespace == ignoreWhitespace else { return } + switch outcome { + case .canceled: + return + case let .failed(path): + applyLoadFailurePath = path + case let .loaded(strictByPath): + if let stalePath = Self.firstStalePath( + renderedByPath: renderedByPath, + strictByPath: strictByPath, + orderedPaths: hunks.map(\.path) + ) { + applyStalePath = stalePath + return + } + loadedFiles = strictByPath.merging(renderedByPath) { _, rendered in rendered } + finishApply(.removeFromSource) + } + } + } + + private enum StrictReloadOutcome { + case loaded([String: DiffEditLoadedFile]) + case failed(String) + case canceled + } + + nonisolated static func firstStalePath( + renderedByPath: [String: DiffEditLoadedFile], + strictByPath: [String: DiffEditLoadedFile], + orderedPaths: [String] + ) -> String? { + orderedPaths.first { path in + guard let rendered = renderedByPath[path], let strict = strictByPath[path] else { + return false + } + return !rendered.hasSameSelectionBasis(as: strict) + } + } + + private func strictReloadAll( + hunks: [DiffHunk], + repo: JayJayRepo, + ignoreWhitespace: Bool + ) async -> StrictReloadOutcome { + let rev = sessionCommit + var strictByPath: [String: DiffEditLoadedFile] = [:] + for hunk in hunks { + if Task.isCancelled { + return .canceled + } + do { + if let loaded = try await loadFileStrict( + hunk: hunk, rev: rev, ignoreWhitespace: ignoreWhitespace, repo: repo + ) { + strictByPath[hunk.path] = loaded + } + } catch { + return .failed(hunk.path) + } + } + return Task.isCancelled ? .canceled : .loaded(strictByPath) + } + + private func loadFileStrict( + hunk: DiffHunk, + rev: String, + ignoreWhitespace: Bool, + repo: JayJayRepo + ) async throws -> DiffEditLoadedFile? { + // Submodule placeholders are synthesized shell-side and absent from the jj tree diff; reloading one would fail and abort the whole apply. + guard hunk.projection == nil, hunk.hunkType != .renamed, !hunk.isSubmodulePlaceholder + else { return nil } + let content = try await diffStore.loadContentStrict(hunk: hunk, rev: rev, repo: repo) + return await DiffEditLoadedFile.make( + hunk: hunk, oldContent: content.oldContent, newContent: content.newContent, + repo: repo, ignoreWhitespace: ignoreWhitespace, highlight: false + ) + } +} diff --git a/shell/mac/Sources/JayJay/DiffEdit/DiffEditView+Selection.swift b/shell/mac/Sources/JayJay/DiffEdit/DiffEditSession+Selection.swift similarity index 83% rename from shell/mac/Sources/JayJay/DiffEdit/DiffEditView+Selection.swift rename to shell/mac/Sources/JayJay/DiffEdit/DiffEditSession+Selection.swift index 21cbbb70..273b32cb 100644 --- a/shell/mac/Sources/JayJay/DiffEdit/DiffEditView+Selection.swift +++ b/shell/mac/Sources/JayJay/DiffEdit/DiffEditSession+Selection.swift @@ -1,8 +1,7 @@ import JayJayCore import JayJayDiffUI -import SwiftUI -extension DiffEditView { +extension DiffEditSession { var selectionSummary: String { let selectedFiles = builtSelections().count let selectedLines = selectedChangedLinesByPath.reduce(into: 0) { count, entry in @@ -59,6 +58,14 @@ extension DiffEditView { showEmptySelectionAlert = true return } + if destination == .removeFromSource { + prepareRemoveFromSource() + return + } + finishApply(destination) + } + + func finishApply(_ destination: DiffEditDestination) { let selections = builtSelections(for: destination) guard !selections.isEmpty else { showEmptySelectionAlert = true @@ -74,7 +81,12 @@ extension DiffEditView { onDone() } - func syncSelection(path: String, loaded: DiffEditLoadedFile) { + func fileLoaded(path: String, loaded: DiffEditLoadedFile) { + loadedFiles[path] = loaded + syncSelection(path: path, loaded: loaded) + } + + private func syncSelection(path: String, loaded: DiffEditLoadedFile) { let changedLines = loaded.changedLineSet guard loaded.supportsDiffEdit, !changedLines.isEmpty else { selectedChangedLinesByPath.removeValue(forKey: path) @@ -142,8 +154,8 @@ extension DiffEditView { let hunks = detail.diff let request = DiffEditLoadRequest( - rev: detailRevision, - commitId: detail.info.commitId.id, + rev: sessionCommit, + commitId: sessionCommit, ignoreWhitespace: settings.ignoreWhitespace ) let diffStore = diffStore @@ -151,8 +163,10 @@ extension DiffEditView { bulkSelectionTask = Task { var loadedByPath: [String: DiffEditLoadedFile] = [:] for hunk in hunks { - if Task.isCancelled { return } - guard let loaded = await loadEditableFile( + if Task.isCancelled { + return + } + guard let loaded = await Self.loadEditableFile( hunk: hunk, request: request, repo: repo, @@ -163,7 +177,10 @@ extension DiffEditView { loadedByPath[hunk.path] = loaded } - if Task.isCancelled { return } + // The captured request outlives a whitespace-mode change; old-mode files and indices must not overwrite reloaded cards. + if Task.isCancelled || settings.ignoreWhitespace != request.ignoreWhitespace { + return + } loadedFiles.merge(loadedByPath) { _, new in new } for loaded in loadedByPath.values { selectedChangedLinesByPath[loaded.hunk.path] = loaded.changedLineSet @@ -183,7 +200,7 @@ extension DiffEditView { } } - private func loadEditableFile( + private static func loadEditableFile( hunk: DiffHunk, request: DiffEditLoadRequest, repo: JayJayRepo, @@ -198,27 +215,15 @@ extension DiffEditView { repo: repo, ignoreWhitespace: request.ignoreWhitespace ), - DiffPlaceholder.isEditableText(cached.oldContent), - DiffPlaceholder.isEditableText(cached.newContent) + DiffPlaceholder.isEditableText(cached.content.oldContent), + DiffPlaceholder.isEditableText(cached.content.newContent) else { return nil } - let path = hunk.path - let ignoreWhitespace = request.ignoreWhitespace - let diff = await Task.detached { - repo.computeNativeDiffFull( - path: path, - oldContent: cached.oldContent, - newContent: cached.newContent, - ignoreWhitespace: ignoreWhitespace - ) - }.value - let loaded = DiffEditLoadedFile( - hunk: hunk, - oldContent: cached.oldContent, - newContent: cached.newContent, - diff: diff + let loaded = await DiffEditLoadedFile.make( + hunk: hunk, oldContent: cached.content.oldContent, newContent: cached.content.newContent, + repo: repo, ignoreWhitespace: request.ignoreWhitespace, highlight: false ) return loaded.changedLineSet.isEmpty ? nil : loaded } diff --git a/shell/mac/Sources/JayJay/DiffEdit/DiffEditSession.swift b/shell/mac/Sources/JayJay/DiffEdit/DiffEditSession.swift new file mode 100644 index 00000000..4c6075ce --- /dev/null +++ b/shell/mac/Sources/JayJay/DiffEdit/DiffEditSession.swift @@ -0,0 +1,192 @@ +import AppKit +import JayJayCore +import Observation + +/// Owns all Diff Edit state and jj-facing operations; DiffEditView renders it and forwards user intent. +@MainActor +@Observable +final class DiffEditSession { + let detail: ChangeDetail + let repo: JayJayRepo? + let diffStore: DiffStore + let actions: (any ChangeActions)? + let settings: AppSettings + let onDone: () -> Void + + var loadedFiles: [String: DiffEditLoadedFile] = [:] + var selectedChangedLinesByPath: [String: Set] = [:] + var newChangeMessage: String + var showEmptySelectionAlert = false + var selectsNewlyLoadedFiles = false + var isSelectingAll = false + var bulkSelectionTask: Task? + var collapsedPaths: Set + var collapseTouched = false + var fileStats: [String: FileDiffStats] = [:] + var isPreparingRemoval = false + var removalTask: Task? + var applyLoadFailurePath: String? + var applyStalePath: String? + var focusedPath: String? + + init( + detail: ChangeDetail, + repo: JayJayRepo?, + diffStore: DiffStore, + actions: (any ChangeActions)?, + diffStats: DiffStats?, + settings: AppSettings, + onDone: @escaping () -> Void + ) { + self.detail = detail + self.repo = repo + self.diffStore = diffStore + self.actions = actions + self.settings = settings + self.onDone = onDone + newChangeMessage = detail.info.description + // Seeded before the first frame from the whole-change stats so a large diff never flashes expanded while per-file stats compute; the per-file pass replaces this approximation with the precise policy. + let fileCount = UInt64(detail.diff.count) + let startsCollapsed: Bool = if let diffStats { + diffEditStartsCollapsed( + fileCount: fileCount, + totalChangedLines: UInt64(diffStats.insertions) + UInt64(diffStats.deletions) + ) + } else { + diffEditCollapsesWhileStatsPending(fileCount: fileCount) + } + collapsedPaths = startsCollapsed ? Set(detail.diff.map(\.path)) : [] + } + + var detailRevision: String { + detail.info.selectionRevision + } + + /// Every session load resolves this immutable commit, never the floating revision, so mid-session drift fails the core staleness guard instead of being silently absorbed. + var sessionCommit: String { + detail.info.commitId.id + } + + func cancelTasks() { + bulkSelectionTask?.cancel() + removalTask?.cancel() + } + + /// Loaded diffs and selections are full-diff row indices under one whitespace mode; a toggle silently remaps them, so the session resets both instead of letting apply submit old-mode indices. + func whitespaceModeChanged() { + bulkSelectionTask?.cancel() + removalTask?.cancel() + isSelectingAll = false + isPreparingRemoval = false + selectsNewlyLoadedFiles = false + loadedFiles = [:] + selectedChangedLinesByPath = [:] + // Old-mode stats must not outlive the reset; the per-file pass rebuilds the folds from fresh stats. + fileStats = [:] + } + + func toggleCollapse(path: String) { + collapseTouched = true + if collapsedPaths.contains(path) { + collapsedPaths.remove(path) + } else { + collapsedPaths.insert(path) + } + } + + func expandAllFiles() { + collapseTouched = true + collapsedPaths = [] + } + + func collapseAllFiles() { + collapseTouched = true + collapsedPaths = Set(detail.diff.map(\.path)) + } + + func loadFileStats() async { + guard let repo else { return } + let rev = sessionCommit + let ignoreWhitespace = settings.ignoreWhitespace + let stats = await Task.detached { + try? repo.diffFileStats(rev: rev, ignoreWhitespace: ignoreWhitespace) + }.value + // The detached call outlives .task(id:) cancellation; a superseded mode's result must not overwrite the current one. + guard let stats, !Task.isCancelled, settings.ignoreWhitespace == ignoreWhitespace + else { return } + fileStats = Dictionary(uniqueKeysWithValues: stats.map { ($0.path, $0) }) + guard !collapseTouched else { return } + // Synthetic cards (dirty-only submodules) are absent from the jj tree diff; count them so the file threshold matches the cards on screen. + var policyStats = stats + let knownPaths = Set(stats.map(\.path)) + for hunk in detail.diff where !knownPaths.contains(hunk.path) { + policyStats.append(FileDiffStats(path: hunk.path, insertions: 0, deletions: 0)) + } + // The aggregate seed can overcount the displayed rows (placeholders, projections), so the precise pass recomputes the whole policy and replaces the seed outright. + let total = policyStats.reduce(UInt64(0)) { $0 + UInt64($1.insertions) + UInt64($1.deletions) } + collapsedPaths = diffEditStartsCollapsed( + fileCount: UInt64(policyStats.count), + totalChangedLines: total + ) + ? Set(detail.diff.map(\.path)) + : Set(diffEditAutoCollapsedPaths(stats: policyStats)) + } + + nonisolated static func nextFocusedPath(current: String?, paths: [String], forward: Bool) -> String? { + guard !paths.isEmpty else { return nil } + guard let current, let index = paths.firstIndex(of: current) else { + return forward ? paths.first : paths.last + } + let target = forward ? index + 1 : index - 1 + guard paths.indices.contains(target) else { return current } + return paths[target] + } + + func handleKey(_ event: NSEvent) -> Bool { + // Keypad Enter and arrows always carry numericPad/function flags, so only reject real modifiers. + let modifiers = event.modifierFlags.intersection(.deviceIndependentFlagsMask) + guard modifiers.subtracting([.numericPad, .function]).isEmpty else { return false } + let paths = detail.diff.map(\.path) + switch event.keyCode { + case KeyCode.returnKey, KeyCode.keypadEnter: + return withFocusedCard(in: paths) { toggleCollapse(path: $0) } + case KeyCode.space: + // Consumed even unfocused; falling through would toggle the file column's review mark. + _ = withFocusedCard(in: paths) { toggleFileSelection(path: $0) } + return true + case KeyCode.leftArrow: + return setFocusedCollapsed(true, paths: paths) + case KeyCode.rightArrow: + return setFocusedCollapsed(false, paths: paths) + default: + break + } + switch (event.keyCode, event.charactersIgnoringModifiers) { + case (KeyCode.downArrow, _), (_, "j"): + return moveFocus(forward: true, paths: paths) + case (KeyCode.upArrow, _), (_, "k"): + return moveFocus(forward: false, paths: paths) + default: + return false + } + } + + private func withFocusedCard(in paths: [String], _ action: (String) -> Void) -> Bool { + guard let focusedPath, paths.contains(focusedPath) else { return false } + action(focusedPath) + return true + } + + private func moveFocus(forward: Bool, paths: [String]) -> Bool { + focusedPath = Self.nextFocusedPath(current: focusedPath, paths: paths, forward: forward) + return true + } + + private func setFocusedCollapsed(_ collapsed: Bool, paths: [String]) -> Bool { + guard let focusedPath, paths.contains(focusedPath), + collapsedPaths.contains(focusedPath) != collapsed + else { return false } + toggleCollapse(path: focusedPath) + return true + } +} diff --git a/shell/mac/Sources/JayJay/DiffEdit/DiffEditView.swift b/shell/mac/Sources/JayJay/DiffEdit/DiffEditView.swift index e995d425..f37b5750 100644 --- a/shell/mac/Sources/JayJay/DiffEdit/DiffEditView.swift +++ b/shell/mac/Sources/JayJay/DiffEdit/DiffEditView.swift @@ -3,69 +3,123 @@ import JayJayDiffUI import SwiftUI struct DiffEditView: View { - let detail: ChangeDetail - let repo: JayJayRepo? - let diffStore: DiffStore - let actions: (any ChangeActions)? - let onDone: () -> Void + @State var session: DiffEditSession - @State var loadedFiles: [String: DiffEditLoadedFile] = [:] - @State var selectedChangedLinesByPath: [String: Set] = [:] - @State var newChangeMessage = "" - @State var showEmptySelectionAlert = false - @State var selectsNewlyLoadedFiles = false - @State var isSelectingAll = false - @State var bulkSelectionTask: Task? - @Environment(AppSettings.self) var settings - - var detailRevision: String { - detail.info.selectionRevision + init( + detail: ChangeDetail, + repo: JayJayRepo?, + diffStore: DiffStore, + actions: (any ChangeActions)?, + diffStats: DiffStats?, + settings: AppSettings, + onDone: @escaping () -> Void + ) { + _session = State(initialValue: DiffEditSession( + detail: detail, + repo: repo, + diffStore: diffStore, + actions: actions, + diffStats: diffStats, + settings: settings, + onDone: onDone + )) } var body: some View { VStack(spacing: 0) { header Divider() - ScrollView { - LazyVStack(alignment: .leading, spacing: 14) { - if hasUnsupportedFiles { - unsupportedNotice + ScrollViewReader { proxy in + ScrollView { + LazyVStack(alignment: .leading, spacing: 14) { + if hasUnsupportedFiles { + unsupportedNotice + } + ForEach(session.detail.diff, id: \.path) { hunk in + DiffEditFileSection( + hunk: hunk, + rev: session.sessionCommit, + commitId: session.sessionCommit, + repo: session.repo, + diffStore: session.diffStore, + selectedChangedLines: session.selectedChangedLinesByPath[hunk.path] ?? [], + stats: session.fileStats[hunk.path], + isCollapsed: session.collapsedPaths.contains(hunk.path), + isFocused: session.focusedPath == hunk.path, + onToggleCollapse: { + session.focusedPath = hunk.path + session.toggleCollapse(path: hunk.path) + }, + onToggleFile: { session.toggleFileSelection(path: hunk.path) }, + onSelectFile: { session.selectFile(path: hunk.path) }, + onToggleLine: { session.toggleLineSelection(path: hunk.path, lineNumber: $0) }, + onSelectHunk: { session.selectHunk(path: hunk.path, range: $0) }, + onLoaded: { session.fileLoaded(path: hunk.path, loaded: $0) } + ) + .id(hunk.path) + } } - ForEach(detail.diff, id: \.path) { hunk in - DiffEditFileSection( - hunk: hunk, - rev: detailRevision, - commitId: detail.info.commitId.id, - repo: repo, - diffStore: diffStore, - selectedChangedLines: selectedChangedLinesByPath[hunk.path] ?? [], - onToggleFile: { toggleFileSelection(path: hunk.path) }, - onSelectFile: { selectFile(path: hunk.path) }, - onToggleLine: { toggleLineSelection(path: hunk.path, lineNumber: $0) }, - onSelectHunk: { selectHunk(path: hunk.path, range: $0) }, - onLoaded: { loaded in - loadedFiles[hunk.path] = loaded - syncSelection(path: hunk.path, loaded: loaded) - } - ) + .padding(18) + } + .onChange(of: session.focusedPath) { _, path in + guard let path else { return } + withAnimation(.easeInOut(duration: 0.18)) { + proxy.scrollTo(path, anchor: .top) } } - .padding(18) } } + .background( + KeyDownMonitor(ignoresReadOnlyText: true, onKeyDown: { session.handleKey($0) }) + .frame(width: 0, height: 0) + .allowsHitTesting(false) + ) .safeAreaInset(edge: .bottom) { actionBar } - .alert("Nothing Selected", isPresented: $showEmptySelectionAlert) { + .alert("Nothing Selected", isPresented: Bindable(session).showEmptySelectionAlert) { Button("OK", role: .cancel) {} } message: { Text("Select at least one file, hunk, or line before applying diff edit.") } - .onAppear { - newChangeMessage = detail.info.description + .alert( + "Couldn't Load All Files", + isPresented: Binding( + get: { session.applyLoadFailurePath != nil }, + set: { + if !$0 { + session.applyLoadFailurePath = nil + } + } + ) + ) { + Button("OK", role: .cancel) {} + } message: { + Text("Failed to load \(session.applyLoadFailurePath ?? ""). Done was not applied — try again.") + } + .alert( + "Diff Changed", + isPresented: Binding( + get: { session.applyStalePath != nil }, + set: { + if !$0 { + session.applyStalePath = nil + } + } + ) + ) { + Button("OK", role: .cancel) {} + } message: { + Text("\(session.applyStalePath ?? "") changed after its selection was rendered. Done was not applied — refresh and try again.") + } + .task(id: "\(session.sessionCommit)|\(session.settings.ignoreWhitespace)") { + await session.loadFileStats() + } + .onChange(of: session.settings.ignoreWhitespace) { _, _ in + session.whitespaceModeChanged() } .onDisappear { - bulkSelectionTask?.cancel() + session.cancelTasks() } } @@ -73,21 +127,35 @@ struct DiffEditView: View { HStack(spacing: 12) { Label("Diff Edit", systemImage: "slider.horizontal.3") .jayjayFont(15, weight: .semibold) - Text(String(detailRevision.prefix(12))) + Text(String(session.detailRevision.prefix(12))) .jayjayFont(12, design: .monospaced) .foregroundStyle(.secondary) Spacer() - Text(selectionSummary) + Text(session.selectionSummary) .jayjayFont(11) .foregroundStyle(.secondary) Button { - toggleBulkSelection() + session.expandAllFiles() + } label: { + Label("Expand All", systemImage: "rectangle.expand.vertical") + } + .keyboardShortcut("e", modifiers: [.command, .option]) + .accessibilityIdentifier(AID.DiffEdit.expandAll) + Button { + session.collapseAllFiles() + } label: { + Label("Collapse All", systemImage: "rectangle.compress.vertical") + } + .keyboardShortcut("c", modifiers: [.command, .option]) + .accessibilityIdentifier(AID.DiffEdit.collapseAll) + Button { + session.toggleBulkSelection() } label: { - Label(selectionToggleTitle, systemImage: selectionToggleSystemImage) + Label(session.selectionToggleTitle, systemImage: session.selectionToggleSystemImage) } - .disabled(selectionToggleDisabled) + .disabled(session.selectionToggleDisabled) .controlSize(.small) - Button("Cancel", action: onDone) + Button("Cancel", action: session.onDone) .keyboardShortcut(.cancelAction) } .controlSize(.small) @@ -114,27 +182,28 @@ struct DiffEditView: View { VStack(spacing: 10) { Divider() HStack(spacing: 12) { - Text(selectionSummary) + Text(session.selectionSummary) .jayjayFont(12, weight: .medium) Spacer() - if !detail.info.isWorkingCopy { - TextField("New change description", text: $newChangeMessage) + if !session.detail.info.isWorkingCopy { + TextField("New change description", text: Bindable(session).newChangeMessage) .textFieldStyle(.roundedBorder) .frame(width: 260) } - if !detail.info.isWorkingCopy { - Button("Create New Child Change") { apply(.newChild) } + if !session.detail.info.isWorkingCopy { + Button("Create New Child Change") { session.apply(.newChild) } .buttonStyle(.borderedProminent) - Button("Create Parallel Change") { apply(.newParallel) } + Button("Create Parallel Change") { session.apply(.newParallel) } .buttonStyle(.bordered) - Button("Move to Working Copy") { apply(.moveToWorkingCopy) } + Button("Move to Working Copy") { session.apply(.moveToWorkingCopy) } .buttonStyle(.bordered) } Button("Done") { - apply(.removeFromSource) + session.apply(.removeFromSource) } .buttonStyle(.bordered) } + .disabled(session.isPreparingRemoval) .padding(.horizontal, 18) .padding(.bottom, 12) } @@ -142,7 +211,7 @@ struct DiffEditView: View { } private var hasUnsupportedFiles: Bool { - detail.diff.contains { hunk in + session.detail.diff.contains { hunk in hunk.projection != nil || hunk.hunkType == .renamed || !DiffPlaceholder.isEditableText(hunk.oldContent) diff --git a/shell/mac/Sources/JayJay/Shared/AccessibilityIdentifiers.swift b/shell/mac/Sources/JayJay/Shared/AccessibilityIdentifiers.swift index 712151f8..9e8095ea 100644 --- a/shell/mac/Sources/JayJay/Shared/AccessibilityIdentifiers.swift +++ b/shell/mac/Sources/JayJay/Shared/AccessibilityIdentifiers.swift @@ -78,4 +78,13 @@ enum AID { "conflict.useTheirs.\(path)" } } + + enum DiffEdit { + static let expandAll = "diffEdit.expandAll" + static let collapseAll = "diffEdit.collapseAll" + + static func fileToggle(_ path: String) -> String { + "diffEdit.fileToggle.\(path)" + } + } } diff --git a/shell/mac/Sources/JayJay/Shared/KeyCode.swift b/shell/mac/Sources/JayJay/Shared/KeyCode.swift new file mode 100644 index 00000000..130eba20 --- /dev/null +++ b/shell/mac/Sources/JayJay/Shared/KeyCode.swift @@ -0,0 +1,12 @@ +import Foundation + +/// Hardware key codes (Carbon's kVK_* values), which NSEvent still reports. +enum KeyCode { + static let returnKey: UInt16 = 36 + static let space: UInt16 = 49 + static let keypadEnter: UInt16 = 76 + static let leftArrow: UInt16 = 123 + static let rightArrow: UInt16 = 124 + static let downArrow: UInt16 = 125 + static let upArrow: UInt16 = 126 +} diff --git a/shell/mac/Sources/JayJay/Shared/KeyDownMonitor.swift b/shell/mac/Sources/JayJay/Shared/KeyDownMonitor.swift index ef94f9c0..4606807c 100644 --- a/shell/mac/Sources/JayJay/Shared/KeyDownMonitor.swift +++ b/shell/mac/Sources/JayJay/Shared/KeyDownMonitor.swift @@ -5,6 +5,8 @@ import SwiftUI /// text input owns focus. struct KeyDownMonitor: NSViewRepresentable { var isActive: () -> Bool = { true } + /// Diff views hold selectable read-only NSTextViews; clicking one must not disable list navigation, while editable inputs keep swallowing keys. + var ignoresReadOnlyText = false let onKeyDown: (NSEvent) -> Bool func makeNSView(context: Context) -> NSView { @@ -16,20 +18,29 @@ struct KeyDownMonitor: NSViewRepresentable { func updateNSView(_: NSView, context: Context) { context.coordinator.onKeyDown = onKeyDown context.coordinator.isActive = isActive + context.coordinator.ignoresReadOnlyText = ignoresReadOnlyText } func makeCoordinator() -> Coordinator { - Coordinator(isActive: isActive, onKeyDown: onKeyDown) + Coordinator( + isActive: isActive, ignoresReadOnlyText: ignoresReadOnlyText, onKeyDown: onKeyDown + ) } final class Coordinator { var isActive: () -> Bool + var ignoresReadOnlyText: Bool var onKeyDown: (NSEvent) -> Bool private weak var view: NSView? private var monitor: Any? - init(isActive: @escaping () -> Bool, onKeyDown: @escaping (NSEvent) -> Bool) { + init( + isActive: @escaping () -> Bool, + ignoresReadOnlyText: Bool, + onKeyDown: @escaping (NSEvent) -> Bool + ) { self.isActive = isActive + self.ignoresReadOnlyText = ignoresReadOnlyText self.onKeyDown = onKeyDown } @@ -44,8 +55,8 @@ struct KeyDownMonitor: NSViewRepresentable { else { return event } - if let responder = window.firstResponder, - responder is NSText || responder is NSTextView + if let text = window.firstResponder as? NSText, + text.isEditable || !ignoresReadOnlyText { return event } @@ -54,7 +65,9 @@ struct KeyDownMonitor: NSViewRepresentable { } deinit { - if let monitor { NSEvent.removeMonitor(monitor) } + if let monitor { + NSEvent.removeMonitor(monitor) + } } } } diff --git a/shell/mac/Sources/JayJay/Shared/ShortcutGuide.swift b/shell/mac/Sources/JayJay/Shared/ShortcutGuide.swift index e5c48805..6e2c8a56 100644 --- a/shell/mac/Sources/JayJay/Shared/ShortcutGuide.swift +++ b/shell/mac/Sources/JayJay/Shared/ShortcutGuide.swift @@ -46,7 +46,13 @@ enum ShortcutGuide { ShortcutSection(title: "Diff & Review", entries: [ ShortcutEntry(label: "Find in Diff", keys: ["⌘", "F"]), ShortcutEntry(label: "Mark File Reviewed", keys: ["Space"]), - ShortcutEntry(label: "Save Description", keys: ["⌘", "S"]) + ShortcutEntry(label: "Save Description", keys: ["⌘", "S"]), + ShortcutEntry(label: "Expand All Files", keys: ["⌥", "⌘", "E"]), + ShortcutEntry(label: "Collapse All Files", keys: ["⌥", "⌘", "C"]), + ShortcutEntry(label: "Next / Previous File Card", keys: ["J", "K"]), + ShortcutEntry(label: "Collapse / Expand File Card", keys: ["←", "→"]), + ShortcutEntry(label: "Select File Card", keys: ["Space"]), + ShortcutEntry(label: "Toggle File Card", keys: ["Return"]) ]), ShortcutSection(title: "Drag & Drop", entries: [ ShortcutEntry(label: "Confirm Drop", keys: ["Return"]), diff --git a/shell/mac/Tests/JayJayTests/DiffEditFocusTests.swift b/shell/mac/Tests/JayJayTests/DiffEditFocusTests.swift new file mode 100644 index 00000000..37eb8ef6 --- /dev/null +++ b/shell/mac/Tests/JayJayTests/DiffEditFocusTests.swift @@ -0,0 +1,43 @@ +@testable import JayJay +import XCTest + +final class DiffEditFocusTests: XCTestCase { + private let paths = ["a", "b", "c"] + + func testForwardFromNilFocusesFirst() { + XCTAssertEqual(DiffEditSession.nextFocusedPath(current: nil, paths: paths, forward: true), "a") + } + + func testBackwardFromNilFocusesLast() { + XCTAssertEqual(DiffEditSession.nextFocusedPath(current: nil, paths: paths, forward: false), "c") + } + + func testForwardMovesToNext() { + XCTAssertEqual(DiffEditSession.nextFocusedPath(current: "a", paths: paths, forward: true), "b") + } + + func testBackwardMovesToPrevious() { + XCTAssertEqual(DiffEditSession.nextFocusedPath(current: "c", paths: paths, forward: false), "b") + } + + func testForwardClampsAtLast() { + XCTAssertEqual(DiffEditSession.nextFocusedPath(current: "c", paths: paths, forward: true), "c") + } + + func testBackwardClampsAtFirst() { + XCTAssertEqual(DiffEditSession.nextFocusedPath(current: "a", paths: paths, forward: false), "a") + } + + func testStalePathForwardFocusesFirst() { + XCTAssertEqual(DiffEditSession.nextFocusedPath(current: "gone", paths: paths, forward: true), "a") + } + + func testStalePathBackwardFocusesLast() { + XCTAssertEqual(DiffEditSession.nextFocusedPath(current: "gone", paths: paths, forward: false), "c") + } + + func testEmptyPathsHasNoFocus() { + XCTAssertNil(DiffEditSession.nextFocusedPath(current: nil, paths: [], forward: true)) + XCTAssertNil(DiffEditSession.nextFocusedPath(current: "a", paths: [], forward: false)) + } +} diff --git a/shell/mac/Tests/JayJayTests/DiffEditPrepareTests.swift b/shell/mac/Tests/JayJayTests/DiffEditPrepareTests.swift new file mode 100644 index 00000000..bc3a76b5 --- /dev/null +++ b/shell/mac/Tests/JayJayTests/DiffEditPrepareTests.swift @@ -0,0 +1,72 @@ +@testable import JayJay +import JayJayCore +import XCTest + +final class DiffEditPrepareTests: XCTestCase { + func testMatchingStrictReloadPreservesSelectionBasis() { + let rendered = loaded(path: "a", old: "one\n", new: "two\n") + let strict = loaded(path: "a", old: "one\n", new: "two\n") + + XCTAssertNil(DiffEditSession.firstStalePath( + renderedByPath: ["a": rendered], + strictByPath: ["a": strict], + orderedPaths: ["a"] + )) + } + + func testChangedContentIsStale() { + let rendered = loaded(path: "a", old: "one\n", new: "two\n") + let strict = loaded(path: "a", old: "one\n", new: "three\n") + + XCTAssertEqual(DiffEditSession.firstStalePath( + renderedByPath: ["a": rendered], + strictByPath: ["a": strict], + orderedPaths: ["a"] + ), "a") + } + + func testDifferingDiffPresentationIsNotStaleForEqualContent() { + let rendered = loaded(path: "a", old: "one\n", new: "two\n") + let strict = loaded(path: "a", old: "one\n", new: "two\n", whitespaceOnlyHidden: true) + + XCTAssertNil(DiffEditSession.firstStalePath( + renderedByPath: ["a": rendered], + strictByPath: ["a": strict], + orderedPaths: ["a"] + )) + } + + func testPreviouslyUnloadedFileCanUseStrictSnapshot() { + let strict = loaded(path: "a", old: "one\n", new: "two\n") + + XCTAssertNil(DiffEditSession.firstStalePath( + renderedByPath: [:], + strictByPath: ["a": strict], + orderedPaths: ["a"] + )) + } + + private func loaded( + path: String, + old: String, + new: String, + whitespaceOnlyHidden: Bool = false + ) -> DiffEditLoadedFile { + DiffEditLoadedFile( + hunk: testHunk( + path: path, + oldContent: old, + newContent: new, + hunkType: .modified + ), + oldContent: old, + newContent: new, + diff: FileDiff( + path: path, + language: "", + lines: [], + whitespaceOnlyHidden: whitespaceOnlyHidden + ) + ) + } +}