diff --git a/src/ui/app.rs b/src/ui/app.rs index dea55890..cfc429db 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -440,6 +440,12 @@ pub struct Tab { /// Monotonic stamp of when this tab was last activated, used to order the /// switcher's tab column most-recently-used first. Zero means never. pub(crate) last_used: std::cell::Cell, + /// Where the last directional focus move started, indexed by the direction + /// that undoes it, so reversing a move comes back here instead of wherever + /// geometry ranks first (#738). Per tab because the panes are; the ids are + /// only ever trusted after the current layout confirms them, so a split, + /// close or swap needs no bookkeeping of its own. + focus_origin: [Option; 4], } #[derive(Clone, Copy, PartialEq, Eq, Debug, Default)] @@ -463,6 +469,7 @@ impl Tab { sidebar_group: std::cell::RefCell::new(None), tree_id: std::cell::Cell::new(tty7_core::core::machine::TabId::new()), last_used: std::cell::Cell::new(0), + focus_origin: Default::default(), } } @@ -481,6 +488,7 @@ impl Tab { ), tree_id: std::cell::Cell::new(tree.id), last_used: std::cell::Cell::new(0), + focus_origin: Default::default(), } } @@ -491,6 +499,16 @@ impl Tab { } } + /// The pane a move in `dir` should return to, if it reverses the last move. + fn focus_origin(&self, dir: Dir) -> Option { + self.focus_origin[dir as usize] + } + + /// Remember that a move in `dir` left `from`, so the move back returns. + fn remember_focus_origin(&mut self, dir: Dir, from: gpui::EntityId) { + self.focus_origin[dir.opposite() as usize] = Some(from); + } + pub(crate) fn detail_pane( &self, window: &Window, @@ -1656,6 +1674,7 @@ impl Tty7App { sidebar_group: std::cell::RefCell::new(st.sidebar_group), tree_id: std::cell::Cell::new(tty7_core::core::machine::TabId::new()), last_used: std::cell::Cell::new(0), + focus_origin: Default::default(), }, ); self.active = insert_at; @@ -3592,13 +3611,22 @@ impl Tty7App { } fn focus_pane_dir(&mut self, dir: Dir, window: &mut Window, cx: &mut Context) { - let Some(target) = self - .tabs - .get(self.active) - .and_then(|tab| tab.pane.neighbor_in_dir(dir, window, cx)) + let Some(tab) = self.tabs.get(self.active) else { + return; + }; + let Some(from) = tab.pane.focused_leaf(window, cx) else { + return; + }; + let Some(target) = tab + .pane + .neighbor_in_dir(dir, tab.focus_origin(dir), window, cx) else { return; }; + let active = self.active; + if let Some(tab) = self.tabs.get_mut(active) { + tab.remember_focus_origin(dir, from.entity_id()); + } self.maximized = None; self.focus_leaf(&target, window, cx); cx.notify(); @@ -7915,6 +7943,7 @@ fn tabs_from_session( .unwrap_or_else(tty7_core::core::machine::TabId::new), ), last_used: std::cell::Cell::new(0), + focus_origin: Default::default(), }); } let active = session.active.min(tabs.len().saturating_sub(1)); diff --git a/src/ui/pane.rs b/src/ui/pane.rs index 497570b1..c7da3793 100644 --- a/src/ui/pane.rs +++ b/src/ui/pane.rs @@ -107,6 +107,16 @@ impl Dir { fn grows(self) -> bool { matches!(self, Dir::Right | Dir::Down) } + + /// The direction that undoes a move this way. + pub fn opposite(self) -> Dir { + match self { + Dir::Left => Dir::Right, + Dir::Right => Dir::Left, + Dir::Up => Dir::Down, + Dir::Down => Dir::Up, + } + } } /// What a tab wants drawn around its panes this frame. @@ -137,6 +147,22 @@ fn overlap_1d(a0: f32, alen: f32, b0: f32, blen: f32) -> f32 { ((a0 + alen).min(b0 + blen) - a0.max(b0)).max(0.0) } +/// Slack for the arithmetic behind `leaf_rects`: ratios multiply out down the +/// tree, so edges that meet exactly in the layout can differ in the last bits. +const ADJACENCY_EPS: f32 = 1e-4; + +/// How far `c` lies from `f` in `dir` and how much edge the two share, or +/// `None` when `c` is not on that side of `f` or only touches it at a corner. +fn adjacency(f: Rect, c: Rect, dir: Dir) -> Option<(f32, f32)> { + let (dist, overlap) = match dir { + Dir::Left => (f.x - (c.x + c.w), overlap_1d(f.y, f.h, c.y, c.h)), + Dir::Right => (c.x - (f.x + f.w), overlap_1d(f.y, f.h, c.y, c.h)), + Dir::Up => (f.y - (c.y + c.h), overlap_1d(f.x, f.w, c.x, c.w)), + Dir::Down => (c.y - (f.y + f.h), overlap_1d(f.x, f.w, c.x, c.w)), + }; + (dist >= -ADJACENCY_EPS && overlap > ADJACENCY_EPS).then_some((dist, overlap)) +} + pub enum CloseOutcome { NotFound, Collapsed, @@ -782,21 +808,15 @@ impl Pane { pub fn neighbor_in_direction(&self, from: usize, dir: Dir) -> Option { let rects = self.leaf_rects(); let f = rects.get(from)?.1; - const EPS: f32 = 1e-4; + const EPS: f32 = ADJACENCY_EPS; let mut best: Option<(usize, f32, f32)> = None; for (i, (_, c)) in rects.iter().enumerate() { if i == from { continue; } - let (dist, overlap) = match dir { - Dir::Left => (f.x - (c.x + c.w), overlap_1d(f.y, f.h, c.y, c.h)), - Dir::Right => (c.x - (f.x + f.w), overlap_1d(f.y, f.h, c.y, c.h)), - Dir::Up => (f.y - (c.y + c.h), overlap_1d(f.x, f.w, c.x, c.w)), - Dir::Down => (c.y - (f.y + f.h), overlap_1d(f.x, f.w, c.x, c.w)), - }; - if dist < -EPS || overlap <= EPS { + let Some((dist, overlap)) = adjacency(f, *c, dir) else { continue; - } + }; let better = match best { None => true, Some((_, bd, bo)) => dist < bd - EPS || (dist <= bd + EPS && overlap > bo + EPS), @@ -808,6 +828,38 @@ impl Pane { best.map(|(i, _, _)| i) } + /// Whether a move in `dir` could legally land on `to`: it sits on that side + /// of `from` and the two share an edge. + pub fn is_neighbor_in_direction(&self, from: usize, to: usize, dir: Dir) -> bool { + if from == to { + return false; + } + let rects = self.leaf_rects(); + match (rects.get(from), rects.get(to)) { + (Some((_, f)), Some((_, c))) => adjacency(*f, *c, dir).is_some(), + _ => false, + } + } + + /// The pane a move in `dir` lands on, preferring `back` — where the last + /// move the other way started — as long as it is still a neighbor. + /// + /// Geometry alone can only rank candidates by overlap, so at a T-junction + /// (one tall pane facing a stack) the reverse move lands on the same member + /// of the stack whichever one you left, and going back and forth drifts + /// (#738). Preferring where you came from makes reversing a move undo it. + /// A `back` the layout has since closed, moved or walled off fails the + /// neighbor test, so geometry decides exactly as it did before. + pub fn focus_target_in_direction( + &self, + from: usize, + dir: Dir, + back: Option, + ) -> Option { + back.filter(|&back| self.is_neighbor_in_direction(from, back, dir)) + .or_else(|| self.neighbor_in_direction(from, dir)) + } + pub fn resize_focused(&self, is_focused: &impl Fn(&L) -> bool, dir: Dir, step: f32) -> bool { let mut path: Vec<(&Pane, bool)> = Vec::new(); if !self.focus_path(is_focused, &mut path) { @@ -879,13 +931,24 @@ impl Pane { .collect() } - pub fn neighbor_in_dir(&self, dir: Dir, window: &Window, cx: &App) -> Option { + /// The pane focus moves to, `back` naming the pane the last move the other + /// way started from. A `back` that is no longer a leaf here — closed, or + /// left behind in another tab — is simply not found, which is what keeps a + /// stale id from ever winning. + pub fn neighbor_in_dir( + &self, + dir: Dir, + back: Option, + window: &Window, + cx: &App, + ) -> Option { let focused = self.focused_leaf(window, cx)?; let leaves = self.leaves(); let from = leaves .iter() .position(|l| l.entity_id() == focused.entity_id())?; - let target = self.neighbor_in_direction(from, dir)?; + let back = back.and_then(|id| leaves.iter().position(|l| l.entity_id() == id)); + let target = self.focus_target_in_direction(from, dir, back)?; leaves.get(target).cloned() } @@ -1562,6 +1625,72 @@ mod tests { assert_eq!(pane.neighbor_in_direction(idx(0), Dir::Right), Some(idx(1))); } + /// The layout from #738: one full-height pane facing a stack of two. + fn t_junction() -> TestPane { + TestPane::split_node( + Axis::Horizontal, + 0.5, + Pane::Leaf(0), + TestPane::split_node(Axis::Vertical, 0.5, Pane::Leaf(4), Pane::Leaf(6)), + ) + } + + #[test] + fn reversing_a_move_at_a_t_junction_returns_to_where_it_started() { + let pane = t_junction(); + let idx = |id: u32| pane.leaves().iter().position(|v| *v == id).unwrap(); + assert_eq!( + pane.focus_target_in_direction(idx(6), Dir::Left, None), + Some(idx(0)) + ); + // Geometry alone ties on overlap here and hands back the top pane. + assert_eq!(pane.neighbor_in_direction(idx(0), Dir::Right), Some(idx(4))); + assert_eq!( + pane.focus_target_in_direction(idx(0), Dir::Right, Some(idx(6))), + Some(idx(6)) + ); + assert_eq!( + pane.focus_target_in_direction(idx(0), Dir::Right, Some(idx(4))), + Some(idx(4)) + ); + } + + #[test] + fn a_recorded_pane_the_layout_moved_on_from_falls_back_to_geometry() { + let pane = t_junction(); + let idx = |id: u32| pane.leaves().iter().position(|v| *v == id).unwrap(); + // Gone entirely: the caller maps a missing pane to no index at all, and + // an index the tree no longer has must not resolve to whoever took it. + assert_eq!( + pane.focus_target_in_direction(idx(0), Dir::Right, None), + Some(idx(4)) + ); + assert_eq!( + pane.focus_target_in_direction(idx(0), Dir::Right, Some(99)), + Some(idx(4)) + ); + // Still there, but no longer reachable that way: splitting the tall + // pane leaves its top half facing only the top of the stack. + let mut pane = t_junction(); + split(&mut pane, 0, Axis::Vertical, 1); + let idx = |id: u32| pane.leaves().iter().position(|v| *v == id).unwrap(); + assert!(!pane.is_neighbor_in_direction(idx(0), idx(6), Dir::Right)); + assert_eq!( + pane.focus_target_in_direction(idx(0), Dir::Right, Some(idx(6))), + Some(idx(4)) + ); + // And the direction still has to match: the pane below is never the + // answer to a move right, however recently focus came from there. + assert_eq!( + pane.focus_target_in_direction(idx(0), Dir::Right, Some(idx(1))), + Some(idx(4)) + ); + assert_eq!( + pane.focus_target_in_direction(idx(0), Dir::Right, Some(idx(0))), + Some(idx(4)) + ); + } + #[test] fn resize_grows_the_focused_pane_from_either_side() { let build = || TestPane::split_node(Axis::Horizontal, 0.5, Pane::Leaf(0), Pane::Leaf(1));