fix(layouts): recover from resurrection crash and pick up swap layouts properly (#3249)

* fix(layouts): recover from issues in the constraint system

* fix(keybinds): pick up swap layouts for new tab keybinding
This commit is contained in:
Aram Drevekenin 2024-04-10 15:26:54 +02:00 committed by GitHub
parent 462239b535
commit 41dbe65e26
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 246 additions and 237 deletions

View File

@ -124,11 +124,19 @@ impl<'a> LayoutApplier<'a> {
refocus_pane: bool,
client_id: Option<ClientId>,
) -> Result<()> {
let err_context = || format!("failed to apply tiled panes layout");
let free_space = self.total_space_for_tiled_panes();
let tiled_panes_count = self.tiled_panes.visible_panes_count();
match layout.position_panes_in_space(&free_space, Some(tiled_panes_count)) {
Ok(positions_in_layout) => {
let positions_in_layout =
match layout.position_panes_in_space(&free_space, Some(tiled_panes_count), false) {
Ok(positions_in_layout) => positions_in_layout,
// in the error branch, we try to recover by positioning the panes in the space but
// ignoring the percentage sizes (passing true as the third argument), this is a hack
// around some issues with the constraint system that should be addressed in a systemic
// manner
Err(_e) => layout
.position_panes_in_space(&free_space, Some(tiled_panes_count), true)
.map_err(|e| anyhow!(e))?,
};
let currently_focused_pane_id =
client_id.and_then(|client_id| self.tiled_panes.focused_pane_id(client_id));
let mut existing_tab_state =
@ -149,8 +157,7 @@ impl<'a> LayoutApplier<'a> {
Some(position_and_size),
);
pane_focuser.set_pane_id_in_focused_location(layout.focus, &pane);
pane_focuser
.set_expanded_stacked_pane(layout.is_expanded_in_stack, &pane);
pane_focuser.set_expanded_stacked_pane(layout.is_expanded_in_stack, &pane);
resize_pty!(pane, self.os_api, self.senders, self.character_cell_size)?;
self.tiled_panes
.add_pane_with_existing_geom(pane.pid(), pane);
@ -168,11 +175,7 @@ impl<'a> LayoutApplier<'a> {
layout.focus.unwrap_or(false),
true,
) {
self.apply_layout_properties_to_pane(
&mut pane,
&layout,
Some(position_and_size),
);
self.apply_layout_properties_to_pane(&mut pane, &layout, Some(position_and_size));
pane_focuser.set_pane_id_in_focused_location(layout.focus, &pane);
pane_focuser.set_expanded_stacked_pane(layout.is_expanded_in_stack, &pane);
resize_pty!(pane, self.os_api, self.senders, self.character_cell_size)?;
@ -193,13 +196,6 @@ impl<'a> LayoutApplier<'a> {
self.tiled_panes,
self.draw_pane_frames,
);
},
Err(e) => {
Err::<(), _>(anyError::msg(e))
.with_context(err_context)
.non_fatal(); // TODO: propagate this to the user
},
};
Ok(())
}
fn apply_tiled_panes_layout(
@ -211,8 +207,17 @@ impl<'a> LayoutApplier<'a> {
) -> Result<()> {
let err_context = || format!("failed to apply tiled panes layout");
let free_space = self.total_space_for_tiled_panes();
match layout.position_panes_in_space(&free_space, None) {
Ok(mut positions_in_layout) => {
let mut positions_in_layout = match layout.position_panes_in_space(&free_space, None, false)
{
Ok(positions_in_layout) => positions_in_layout,
// in the error branch, we try to recover by positioning the panes in the space but
// ignoring the percentage sizes (passing true as the third argument), this is a hack
// around some issues with the constraint system that should be addressed in a systemic
// manner
Err(_e) => layout
.position_panes_in_space(&free_space, None, true)
.map_err(|e| anyhow!(e))?,
};
let mut run_instructions_to_ignore = layout.run_instructions_to_ignore.clone();
let mut new_terminal_ids = new_terminal_ids.iter();
@ -337,10 +342,8 @@ impl<'a> LayoutApplier<'a> {
if let Some(held_command) = hold_for_command {
new_pane.hold(None, true, held_command.clone());
}
self.tiled_panes.add_pane_with_existing_geom(
PaneId::Terminal(*pid),
Box::new(new_pane),
);
self.tiled_panes
.add_pane_with_existing_geom(PaneId::Terminal(*pid), Box::new(new_pane));
set_focus_pane_id(layout, PaneId::Terminal(*pid));
}
}
@ -352,18 +355,6 @@ impl<'a> LayoutApplier<'a> {
}
self.adjust_viewport().with_context(err_context)?;
self.set_focused_tiled_pane(focus_pane_id, client_id);
},
Err(e) => {
for (unused_pid, _) in new_terminal_ids {
self.senders
.send_to_pty(PtyInstruction::ClosePane(PaneId::Terminal(unused_pid)))
.with_context(err_context)?;
}
Err::<(), _>(anyError::msg(e))
.with_context(err_context)
.non_fatal(); // TODO: propagate this to the user
},
};
Ok(())
}
fn apply_floating_panes_layout(

View File

@ -249,7 +249,7 @@ impl SwapLayouts {
let pane_count = tiled_panes.visible_panes_count();
let display_area = PaneGeom::from(&*display_area);
if layout
.position_panes_in_space(&display_area, Some(pane_count))
.position_panes_in_space(&display_area, Some(pane_count), false)
.is_ok()
{
return Some(layout.clone());
@ -279,7 +279,7 @@ impl SwapLayouts {
let pane_count = tiled_panes.visible_panes_count();
let display_area = PaneGeom::from(&*display_area);
if layout
.position_panes_in_space(&display_area, Some(pane_count))
.position_panes_in_space(&display_area, Some(pane_count), false)
.is_ok()
{
return Some(layout.clone());

View File

@ -849,6 +849,7 @@ impl TiledPaneLayout {
&self,
space: &PaneGeom,
max_panes: Option<usize>,
ignore_percent_split_sizes: bool,
) -> Result<Vec<(TiledPaneLayout, PaneGeom)>, &'static str> {
let layouts = match max_panes {
Some(max_panes) => {
@ -874,9 +875,9 @@ impl TiledPaneLayout {
layout_to_split.focus_deepest_pane();
}
split_space(space, &layout_to_split, space)?
split_space(space, &layout_to_split, space, ignore_percent_split_sizes)?
},
None => split_space(space, self, space)?,
None => split_space(space, self, space, ignore_percent_split_sizes)?,
};
for (_pane_layout, pane_geom) in layouts.iter() {
if !pane_geom.is_at_least_minimum_size() {
@ -1426,6 +1427,7 @@ fn split_space(
space_to_split: &PaneGeom,
layout: &TiledPaneLayout,
total_space_to_split: &PaneGeom,
ignore_percent_split_sizes: bool,
) -> Result<Vec<(TiledPaneLayout, PaneGeom)>, &'static str> {
let sizes: Vec<Option<SplitSize>> = if layout.children_are_stacked {
let index_of_expanded_pane = layout.children.iter().position(|p| p.is_expanded_in_stack);
@ -1440,6 +1442,15 @@ fn split_space(
*last_size = None;
}
sizes
} else if ignore_percent_split_sizes {
layout
.children
.iter()
.map(|part| match part.split_size {
Some(SplitSize::Percent(_)) => None,
split_size => split_size,
})
.collect()
} else {
layout.children.iter().map(|part| part.split_size).collect()
};
@ -1539,8 +1550,12 @@ fn split_space(
for (i, part) in layout.children.iter().enumerate() {
let part_position_and_size = split_geom.get(i).unwrap();
if !part.children.is_empty() {
let mut part_positions =
split_space(part_position_and_size, part, total_space_to_split)?;
let mut part_positions = split_space(
part_position_and_size,
part,
total_space_to_split,
ignore_percent_split_sizes,
)?;
pane_positions.append(&mut part_positions);
} else {
let part = part.clone();

View File

@ -860,6 +860,9 @@ impl TryFrom<(&KdlNode, &Options)> for Action {
)
})?;
let swap_tiled_layouts = Some(layout.swap_tiled_layouts.clone());
let swap_floating_layouts = Some(layout.swap_floating_layouts.clone());
let mut tabs = layout.tabs();
if tabs.len() > 1 {
return Err(ConfigError::new_kdl_error(
@ -874,8 +877,8 @@ impl TryFrom<(&KdlNode, &Options)> for Action {
Ok(Action::NewTab(
Some(layout),
floating_panes_layout,
None,
None,
swap_tiled_layouts,
swap_floating_layouts,
name,
))
} else {
@ -884,8 +887,8 @@ impl TryFrom<(&KdlNode, &Options)> for Action {
Ok(Action::NewTab(
Some(layout),
floating_panes_layout,
None,
None,
swap_tiled_layouts,
swap_floating_layouts,
name,
))
}

View File

@ -122,7 +122,7 @@ impl Dimension {
self.inner += by;
}
pub fn decrease_inner(&mut self, by: usize) {
self.inner -= by;
self.inner = self.inner.saturating_sub(by);
}
pub fn is_fixed(&self) -> bool {