Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/app/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1800,7 +1800,9 @@ impl AppState {
};

if let Some(focused) = panes.iter().find(|p| p.is_focused) {
if let Some(target) = find_in_direction(focused, direction, &panes) {
if let Some(target) =
find_in_direction(focused, direction, &panes, tab.layout.focus_history())
{
self.focus_pane_in_workspace(ws_idx, target);
}
}
Expand All @@ -1823,7 +1825,12 @@ impl AppState {
let Some(focused) = panes.iter().find(|p| p.is_focused) else {
return false;
};
let Some(target) = find_in_direction(focused, direction, &panes) else {
let Some(target) = find_in_direction(
focused,
direction,
&panes,
&crate::layout::FocusHistory::default(),
) else {
return false;
};
let source = focused.id;
Expand Down
131 changes: 119 additions & 12 deletions src/app/api/panes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::app::actions::{PaneZoomCommand, PaneZoomNoopReason};
use crate::app::App;
#[cfg(test)]
use crate::app::Mode;
use crate::layout::{find_in_direction, NavDirection, PaneId};
use crate::layout::{find_in_direction, FocusHistory, NavDirection, PaneId};

use super::super::api_helpers::{
detect_state_from_api, encode_api_keys, normalize_metadata_source, normalize_metadata_tokens,
Expand Down Expand Up @@ -470,7 +470,7 @@ impl App {
.unwrap_or_default(),
);
};
let target = self.directional_pane_target(ws_idx, tab_idx, source_pane_id, direction);
let target = self.directional_swap_target(ws_idx, tab_idx, source_pane_id, direction);
match target {
Some(target_pane_id) => {
(ws_idx, tab_idx, source_pane_id, Some(target_pane_id), None)
Expand Down Expand Up @@ -871,10 +871,6 @@ impl App {
self.recover_failed_pane_move(recovery_context, moved);
return encode_error(id, "pane_move_failed", "target tab disappeared");
};
let previous_target_focus = self.state.workspaces[target_ws_idx].tabs
[target_tab_idx]
.layout
.focused();
let direction = split_direction_to_layout(split);
let moved_pane_id = match self.state.workspaces[target_ws_idx]
.insert_moved_pane_into_tab(
Expand All @@ -883,6 +879,7 @@ impl App {
moved,
direction,
ratio,
focus,
) {
Ok(pane_id) => pane_id,
Err(moved) => {
Expand All @@ -894,11 +891,6 @@ impl App {
);
}
};
if !focus {
self.state.workspaces[target_ws_idx].tabs[target_tab_idx]
.layout
.focus_pane(previous_target_focus);
}
(target_ws_idx, target_tab_idx, moved_pane_id)
}
ResolvedPaneMoveDestination::NewTab {
Expand Down Expand Up @@ -1676,7 +1668,28 @@ impl App {
let tab = self.state.workspaces.get(ws_idx)?.tabs.get(tab_idx)?;
let panes = tab.layout.panes(self.state.view.terminal_area);
let source = panes.iter().find(|pane| pane.id == source_pane_id)?;
find_in_direction(source, direction.into(), &panes)
find_in_direction(source, direction.into(), &panes, tab.layout.focus_history())
}

/// Resolve a directional swap target using geometry only.
///
/// Unlike [`Self::directional_pane_target`], swaps deliberately ignore focus
/// history: a directional swap exchanges the source with its geometric
/// neighbor, matching the TUI swap path (`directional_pane_swap_from_view`).
/// Biasing swaps toward the most-recently-focused neighbor would make the
/// API diverge from the keyboard swap and make swap targets depend on
/// unrelated focus movements.
fn directional_swap_target(
&self,
ws_idx: usize,
tab_idx: usize,
source_pane_id: PaneId,
direction: PaneDirection,
) -> Option<PaneId> {
let tab = self.state.workspaces.get(ws_idx)?.tabs.get(tab_idx)?;
let panes = tab.layout.panes(self.state.view.terminal_area);
let source = panes.iter().find(|pane| pane.id == source_pane_id)?;
find_in_direction(source, direction.into(), &panes, &FocusHistory::default())
}

pub(super) fn pane_layout_snapshot(
Expand Down Expand Up @@ -2452,6 +2465,46 @@ mod tests {
assert!(app.event_hub.events_after(0).is_empty());
}

#[test]
fn api_pane_swap_direction_ignores_focus_history() {
// Layout: H(L, V(RT, RB)). RB is focused most recently among the right
// column, then focus returns to L. A directional swap must exchange L
// with its geometric neighbor (RT, the top pane), not the most-recently
// focused one (RB); swaps are geometry-only, unlike focus navigation.
let mut app = app_with_linked_worktree();
let left = app.state.workspaces[0].tabs[0].root_pane;
let right_top = app.state.workspaces[0].test_split(ratatui::layout::Direction::Horizontal);
let right_bottom = app.state.workspaces[0].test_split(ratatui::layout::Direction::Vertical);
// Make RB the most-recently-focused right pane, then return focus to L.
app.state.workspaces[0].tabs[0]
.layout
.focus_pane(right_bottom);
app.state.workspaces[0].tabs[0].layout.focus_pane(left);
crate::ui::compute_view(&mut app.state, ratatui::layout::Rect::new(0, 0, 100, 20));
let left_public = app.public_pane_id(0, left).unwrap();
let right_top_public = app.public_pane_id(0, right_top).unwrap();
let right_bottom_public = app.public_pane_id(0, right_bottom).unwrap();

let response = app.handle_pane_swap(
"req".into(),
PaneSwapParams {
pane_id: Some(left_public.clone()),
direction: Some(PaneDirection::Right),
..PaneSwapParams::default()
},
);

let success: SuccessResponse = serde_json::from_str(&response).unwrap();
let ResponseResult::PaneSwap { swap } = success.result else {
panic!("expected pane swap response");
};
assert!(swap.changed);
assert_eq!(swap.reason, None);
assert_eq!(swap.source_pane_id, left_public);
assert_eq!(swap.target_pane_id, Some(right_top_public));
assert_ne!(swap.target_pane_id, Some(right_bottom_public));
}

#[test]
fn api_pane_swap_explicit_missing_target_returns_not_found_noop() {
let mut app = app_with_linked_worktree();
Expand Down Expand Up @@ -3135,6 +3188,60 @@ mod tests {
);
}

#[test]
fn api_pane_move_no_focus_pane_is_not_a_directional_navigation_target() {
// Target tab: H(A, V(RT, RB)), focus A. Move `source` next to RT with a
// downward split and focus: false, producing H(A, V(V(RT, source), RB)).
// Directional navigation from A must not select the never-focused moved
// pane; it has no focus history and loses to the geometric winner (RB).
let mut app = app_with_linked_worktree();
let source = app.state.workspaces[0].tabs[0].root_pane;
let target_tab = app.state.workspaces[0].test_add_tab(Some("target"));
app.state.workspaces[0].active_tab = target_tab;
let a = app.state.workspaces[0].tabs[target_tab].root_pane;
let rt = app.state.workspaces[0].test_split(ratatui::layout::Direction::Horizontal);
let rb = app.state.workspaces[0].test_split(ratatui::layout::Direction::Vertical);
app.state.workspaces[0].tabs[target_tab]
.layout
.focus_pane(a);
seed_terminal_states(&mut app);
let source_public = app.public_pane_id(0, source).unwrap();
let target_tab_public = app.public_tab_id(0, target_tab).unwrap();
let rt_public = app.public_pane_id(0, rt).unwrap();

let response = app.handle_pane_move(
"req".into(),
PaneMoveParams {
pane_id: source_public,
destination: PaneMoveDestination::Tab {
tab_id: target_tab_public,
target_pane_id: Some(rt_public),
split: SplitDirection::Down,
ratio: None,
},
focus: false,
},
);

let success: SuccessResponse = serde_json::from_str(&response).unwrap();
let ResponseResult::PaneMove { move_result } = success.result else {
panic!("expected pane move response");
};
assert!(move_result.changed);
// Focus stayed on A; the moved pane was never focused. The source's
// emptied tab is removed by the move, so resolve the target tab by pane.
let target_tab_after = app.state.workspaces[0].find_tab_index_for_pane(a).unwrap();
let layout = &app.state.workspaces[0].tabs[target_tab_after].layout;
assert_eq!(layout.focused(), a);

let panes = layout.panes(ratatui::layout::Rect::new(0, 0, 100, 40));
let focused = panes.iter().find(|pane| pane.id == a).unwrap();
let nav_target =
find_in_direction(focused, NavDirection::Right, &panes, layout.focus_history());
assert_ne!(nav_target, Some(source));
assert_eq!(nav_target, Some(rb));
}

#[test]
fn api_pane_move_recovery_restores_removed_source_workspace() {
let mut app = app_with_linked_worktree();
Expand Down
24 changes: 20 additions & 4 deletions src/app/input/navigate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -689,8 +689,19 @@ impl App {
.pane_infos
.iter()
.find(|pane| pane.is_focused)?;
let target =
crate::layout::find_in_direction(focused, direction, &self.state.view.pane_infos)?;
let history = self
.state
.workspaces
.get(ws_idx)?
.active_tab()?
.layout
.focus_history();
let target = crate::layout::find_in_direction(
focused,
direction,
&self.state.view.pane_infos,
history,
)?;
Some((ws_idx, target))
}

Expand All @@ -705,8 +716,13 @@ impl App {
.pane_infos
.iter()
.find(|pane| pane.is_focused)?;
let target =
crate::layout::find_in_direction(focused, direction, &self.state.view.pane_infos)?;
let empty = crate::layout::FocusHistory::default();
let target = crate::layout::find_in_direction(
focused,
direction,
&self.state.view.pane_infos,
&empty,
)?;
Some((ws_idx, focused.id, target))
}

Expand Down
Loading
Loading