fix(pane): only let a remembered pane settle a tie between neighbors (#738)

The remembered pane was accepted on the strength of lying that way with a
shared edge, which a pane two columns over also does. In a row of three,
moving left off the last column and then reaching the first by a click or
a cycle -- neither of which records anything -- left the origin naming the
column that had been skipped over, and the move back jumped straight to
it.

Being on the right side is now not enough: a remembered pane also has to
be no farther off than the nearest pane that way. That still settles the
T-junction, where the two candidates are equidistant and only the shared
edge separates them, while a pane with another one in front of it loses
to whatever is actually adjacent. A stale origin can therefore only ever
break a tie, which is what the comments claimed and now describe.
This commit is contained in:
l0ng-ai
2026-09-07 19:09:13 +08:00
parent f697b2f6ce
commit e711df7904
2 changed files with 69 additions and 22 deletions
+6 -3
View File
@@ -442,9 +442,12 @@ pub struct Tab {
pub(crate) last_used: std::cell::Cell<u64>,
/// 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.
/// geometry ranks first (#738). Per tab because the panes are.
///
/// Nothing clears these: a recorded pane only ever breaks a tie between the
/// panes already next to the one focus is leaving, so the worst an entry
/// left over from an older layout — or from before a click moved focus
/// somewhere else entirely — can do is lose to geometry.
focus_origin: [Option<gpui::EntityId>; 4],
}
+63 -19
View File
@@ -807,6 +807,12 @@ impl<L: Clone> Pane<L> {
pub fn neighbor_in_direction(&self, from: usize, dir: Dir) -> Option<usize> {
let rects = self.leaf_rects();
Self::ranked_neighbor(&rects, from, dir).map(|(i, _)| i)
}
/// The pane a move in `dir` lands on and how far off it sits: nearest wins,
/// and the widest shared edge breaks a tie.
fn ranked_neighbor(rects: &[(L, Rect)], from: usize, dir: Dir) -> Option<(usize, f32)> {
let f = rects.get(from)?.1;
const EPS: f32 = ADJACENCY_EPS;
let mut best: Option<(usize, f32, f32)> = None;
@@ -825,38 +831,51 @@ impl<L: Clone> Pane<L> {
best = Some((i, dist, overlap));
}
}
best.map(|(i, _, _)| i)
best.map(|(i, dist, _)| (i, dist))
}
/// 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 {
/// Whether a move in `dir` could land on `to` without stepping over
/// anything: `to` shares an edge with that side of `from`, and nothing in
/// that direction sits nearer.
///
/// Lying on the right side is not enough on its own. In a row of three
/// columns the far one also sits to the right of the first with a full edge
/// in common, and treating that as adjacent would skip the column between
/// them.
pub fn is_adjacent_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,
}
let (Some((_, f)), Some((_, c))) = (rects.get(from), rects.get(to)) else {
return false;
};
let Some((dist, _)) = adjacency(*f, *c, dir) else {
return false;
};
Self::ranked_neighbor(&rects, from, dir)
.is_some_and(|(_, nearest)| dist <= nearest + ADJACENCY_EPS)
}
/// 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.
/// move the other way started — as long as it is still adjacent.
///
/// 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.
/// Among the panes actually next to `from`, geometry can only rank by
/// shared edge, 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 settles that tie the only way the user can mean it.
///
/// It settles a tie and nothing more: `back` still has to be one of the
/// nearest panes that way, so a move can never step over the pane in
/// between, however out of date the caller's memory is.
pub fn focus_target_in_direction(
&self,
from: usize,
dir: Dir,
back: Option<usize>,
) -> Option<usize> {
back.filter(|&back| self.is_neighbor_in_direction(from, back, dir))
back.filter(|&back| self.is_adjacent_in_direction(from, back, dir))
.or_else(|| self.neighbor_in_direction(from, dir))
}
@@ -933,8 +952,8 @@ impl Pane<PaneSlot> {
/// 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.
/// left behind in another tab — is simply not found; one that is still here
/// but no longer next to `from` loses to the pane that is.
pub fn neighbor_in_dir(
&self,
dir: Dir,
@@ -1674,7 +1693,7 @@ mod tests {
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!(!pane.is_adjacent_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))
@@ -1691,6 +1710,31 @@ mod tests {
);
}
#[test]
fn a_remembered_pane_further_off_never_steps_over_the_one_between() {
// Three equal full-height columns, 0 | 1 | 2.
let pane = TestPane::split_node(
Axis::Horizontal,
1.0 / 3.0,
Pane::Leaf(0),
TestPane::split_node(Axis::Horizontal, 0.5, Pane::Leaf(1), Pane::Leaf(2)),
);
let idx = |id: u32| pane.leaves().iter().position(|v| *v == id).unwrap();
// Focus reaches 1 by moving left off 2, then leaves for 0 by a click or
// a cycle — neither of which records anything, so the origin still
// names 2 when the move back to the right happens from 0.
assert!(pane.is_adjacent_in_direction(idx(1), idx(2), Dir::Right));
assert!(!pane.is_adjacent_in_direction(idx(0), idx(2), Dir::Right));
// 2 does lie to the right of 0 with a full edge in common; only being
// farther off than 1 disqualifies it.
assert!(adjacency(rect_of(&pane, 0), rect_of(&pane, 2), Dir::Right).is_some());
assert_eq!(
pane.focus_target_in_direction(idx(0), Dir::Right, Some(idx(2))),
Some(idx(1)),
"a move right must land on the next column, not skip it"
);
}
#[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));