From 45e5bde4ed0294028a6fddccab3d252c496bf0d8 Mon Sep 17 00:00:00 2001 From: Adam Hitchcock Date: Thu, 8 Oct 2026 19:49:39 -0700 Subject: [PATCH] fix(diff): a new or deleted file draws unified in the split view (#1138) A new or deleted file has one side only, so in the split view half its card was empty. Such a file now draws unified whatever the view, and the others still split. A drag takes its mode from the row it starts on, so selecting and copying in one of these files works in either view. `file_mode` in `diff_list.rs` picks the mode, with tests; `cargo test --bin tty7-app diff` gives 90 passed, 1 ignored. Co-authored-by: Claude Opus 5.5 --- src/ui/diff_list.rs | 38 ++++++++++++++++++++++++++++++++++++++ src/ui/diff_overlay.rs | 42 +++++++++++++++++++++++++++--------------- 2 files changed, 65 insertions(+), 15 deletions(-) diff --git a/src/ui/diff_list.rs b/src/ui/diff_list.rs index ef474f29..15c2acf4 100644 --- a/src/ui/diff_list.rs +++ b/src/ui/diff_list.rs @@ -155,7 +155,17 @@ pub(crate) fn preview_rows(file: &FileDiff, mode: DiffViewMode) -> Vec file_rows(usize::MAX, file, true, mode) } +/// The view `file` draws in: a new or deleted file has one side only, so +/// split would be half empty. +pub(crate) fn file_mode(file: &FileDiff, mode: DiffViewMode) -> DiffViewMode { + match file.status { + FileStatus::Added | FileStatus::Deleted => DiffViewMode::Unified, + _ => mode, + } +} + fn file_rows(index: usize, file: &FileDiff, expanded: bool, mode: DiffViewMode) -> Vec { + let mode = file_mode(file, mode); let expandable = !file.binary && (!file.hunks.is_empty() || file.truncated == Some(Truncation::Budget)); let shown_path = match &file.old_path { @@ -382,6 +392,34 @@ mod tests { ); } + /// A new or deleted file has nothing to set beside it, so split draws it + /// unified; a modified file in the same list still splits. + #[test] + fn split_draws_a_new_or_deleted_file_unified() { + let added = FileDiff { + status: FileStatus::Added, + ..file("new.rs", 1) + }; + let deleted = FileDiff { + status: FileStatus::Deleted, + ..file("gone.rs", 1) + }; + for one_sided in [added, deleted] { + let snap = snapshot(vec![one_sided, file("a.rs", 1)]); + let rows = build_rows(&snap, &open(["new.rs"]), None, DiffViewMode::Split, false); + let kinds: Vec<_> = shape(&rows) + .into_iter() + .filter(|k| matches!(*k, "split" | "unified")) + .collect(); + assert_eq!( + kinds, + [ + "unified", "unified", "unified", "unified", "split", "split", "split" + ] + ); + } + } + #[test] fn only_the_first_hunk_of_a_file_follows_its_header() { let rows = build_rows( diff --git a/src/ui/diff_overlay.rs b/src/ui/diff_overlay.rs index ecfc9f4d..46594094 100644 --- a/src/ui/diff_overlay.rs +++ b/src/ui/diff_overlay.rs @@ -1046,16 +1046,6 @@ impl Tty7App { let active = self.active; let overlay = self.tabs.get_mut(active)?.diff_overlay.as_mut()?; - // Forget a range the rows under it no longer answer to. The two views - // pair the same lines differently, so a row range drawn in one of them - // points at other code in the other. Here rather than beside the - // switch that flips the mode: this is the one place that knows which - // rows are about to be drawn. - if overlay.selection.as_ref().is_some_and(|s| s.mode != mode) { - overlay.selection = None; - overlay.selecting = false; - } - let snap = match &overlay.load { DiffLoad::Loading => return Some(DiffBody::Message(t(L10nKey::DiffReading))), DiffLoad::NotARepo => return Some(DiffBody::Message(t(L10nKey::DiffNotARepo))), @@ -1081,6 +1071,27 @@ impl Tty7App { None => None, }; + // Forget a range the rows under it no longer answer to. The two views + // pair the same lines differently, so a row range drawn in one of them + // points at other code in the other. Here rather than beside the + // switch that flips the mode: this is the one place that knows which + // rows are about to be drawn. + let drawn = |path: &str| { + preview + .as_deref() + .filter(|f| f.path == path) + .or_else(|| snap.files.iter().find(|f| f.path == path)) + .map_or(mode, |f| crate::ui::diff_list::file_mode(f, mode)) + }; + if overlay + .selection + .as_ref() + .is_some_and(|s| s.mode != drawn(&s.path)) + { + overlay.selection = None; + overlay.selecting = false; + } + let focused = focused_file(&snap, overlay); let from = RowsFrom { snap: &snap, @@ -1135,7 +1146,6 @@ impl Tty7App { let drag = Drag { sel: overlay.selection.clone().map(Rc::new), selecting: overlay.selecting, - mode: view_mode(cx), }; let body = gpui::list(list.clone(), move |ix, _window, cx| { #[cfg(test)] @@ -1197,9 +1207,6 @@ struct Drag { /// while one is: a diff runs to thousands of rows, and a listener each is /// worth paying for during a drag and not otherwise. selecting: bool, - /// The view the rows on screen are drawn in, which is the view a press - /// starts its selection in. - mode: DiffViewMode, } impl Drag { @@ -1535,7 +1542,12 @@ fn diff_row_drag( drag: &Drag, app: &gpui::WeakEntity, ) -> E { - let mode = drag.mode; + // From the row, not the overlay's view: a new or deleted file draws + // unified whatever the view (`diff_list::file_mode`). + let mode = match side { + Some(_) => DiffViewMode::Split, + None => DiffViewMode::Unified, + }; // The I-beam is the only standing sign that this text can be taken; // nothing else about a row says so until one is dragged. let el = el.cursor_text().on_mouse_down(MouseButton::Left, {