fix(panes): adding panes to lone stack (#2298)

zellij-server/src/panes/floating_panes/mod.rs

@@ -532,7 +532,7 @@ impl FloatingPanes {
     pub fn focus_pane_on_edge(&mut self, direction: Direction, client_id: ClientId) {
         let display_area = *self.display_area.borrow();
         let viewport = *self.viewport.borrow();
-        let mut floating_pane_grid = FloatingPaneGrid::new(
+        let floating_pane_grid = FloatingPaneGrid::new(
             &mut self.panes,
             &mut self.desired_pane_positions,
             display_area,

zellij-server/src/panes/tiled_panes/mod.rs

@@ -176,13 +176,14 @@ impl TiledPanes {
             *self.display_area.borrow(),
             *self.viewport.borrow(),
         );
-        pane_grid
+        let has_room_for_new_pane = pane_grid
             .find_room_for_new_pane(cursor_height_width_ratio)
-            .is_some()
+            .is_some();
+        has_room_for_new_pane || pane_grid.has_room_for_new_stacked_pane()
     }
     fn add_pane(&mut self, pane_id: PaneId, mut pane: Box<dyn Pane>, should_relayout: bool) {
         let cursor_height_width_ratio = self.cursor_height_width_ratio();
-        let pane_grid = TiledPaneGrid::new(
+        let mut pane_grid = TiledPaneGrid::new(
             &mut self.panes,
             &self.panes_to_hide,
             *self.display_area.borrow(),
@@ -190,18 +191,34 @@ impl TiledPanes {
         );
         let pane_id_and_split_direction =
             pane_grid.find_room_for_new_pane(cursor_height_width_ratio);
-        if let Some((pane_id_to_split, split_direction)) = pane_id_and_split_direction {
+        match pane_id_and_split_direction {
-            // this unwrap is safe because floating panes should not be visible if there are no floating panes
+            Some((pane_id_to_split, split_direction)) => {
-            let pane_to_split = self.panes.get_mut(&pane_id_to_split).unwrap();
+                // this unwrap is safe because floating panes should not be visible if there are no floating panes
-            let size_of_both_panes = pane_to_split.position_and_size();
+                let pane_to_split = self.panes.get_mut(&pane_id_to_split).unwrap();
-            if let Some((first_geom, second_geom)) = split(split_direction, &size_of_both_panes) {
+                let size_of_both_panes = pane_to_split.position_and_size();
-                pane_to_split.set_geom(first_geom);
+                if let Some((first_geom, second_geom)) = split(split_direction, &size_of_both_panes)
-                pane.set_geom(second_geom);
+                {
-                self.panes.insert(pane_id, pane);
+                    pane_to_split.set_geom(first_geom);
-                if should_relayout {
+                    pane.set_geom(second_geom);
-                    self.relayout(!split_direction);
+                    self.panes.insert(pane_id, pane);
+                    if should_relayout {
+                        self.relayout(!split_direction);
+                    }
                 }
-            }
+            },
+            None => {
+                // we couldn't add the pane normally, let's see if there's room in one of the
+                // stacks...
+                match pane_grid.make_room_in_stack_for_pane() {
+                    Ok(new_pane_geom) => {
+                        pane.set_geom(new_pane_geom);
+                        self.panes.insert(pane_id, pane); // TODO: is set_geom the right one?
+                    },
+                    Err(e) => {
+                        log::error!("Failed to add pane to stack: {:?}", e);
+                    },
+                }
+            },
         }
     }
     pub fn fixed_pane_geoms(&self) -> Vec<Viewport> {

zellij-server/src/panes/tiled_panes/stacked_panes.rs

@@ -430,6 +430,58 @@ impl<'a> StackedPanes<'a> {
             stacked_pane_ids_over_flexible_panes,
         ))
     }
+    pub fn make_room_for_new_pane(&mut self) -> Result<PaneGeom> {
+        let err_context = || format!("Failed to add pane to stack");
+        let all_stacks = self.get_all_stacks()?;
+        for stack in all_stacks {
+            if let Some((id_of_flexible_pane_in_stack, _flexible_pane_in_stack)) = stack
+                .iter()
+                .find(|(_p_id, p)| !p.rows.is_fixed() && p.rows.as_usize() > 1)
+            {
+                self.make_lowest_pane_in_stack_flexible(id_of_flexible_pane_in_stack)?;
+                let all_stacked_pane_positions =
+                    self.positions_in_stack(id_of_flexible_pane_in_stack)?;
+                let position_of_flexible_pane =
+                    self.position_of_flexible_pane(&all_stacked_pane_positions)?;
+                let (flexible_pane_id, mut flexible_pane_geom) = *all_stacked_pane_positions
+                    .iter()
+                    .nth(position_of_flexible_pane)
+                    .with_context(err_context)?;
+                let mut position_for_new_pane = flexible_pane_geom.clone();
+                position_for_new_pane
+                    .rows
+                    .set_inner(position_for_new_pane.rows.as_usize() - 1);
+                position_for_new_pane.y = position_for_new_pane.y + 1;
+                flexible_pane_geom.rows = Dimension::fixed(1);
+                self.panes
+                    .borrow_mut()
+                    .get_mut(&flexible_pane_id)
+                    .with_context(err_context)?
+                    .set_geom(flexible_pane_geom);
+                return Ok(position_for_new_pane);
+            }
+        }
+        Err(anyhow!("Not enough room for another pane!"))
+    }
+    fn get_all_stacks(&self) -> Result<Vec<Vec<(PaneId, PaneGeom)>>> {
+        let err_context = || "Failed to get positions in stack";
+        let panes = self.panes.borrow();
+        let all_flexible_panes_in_stack: Vec<PaneId> = panes
+            .iter()
+            .filter(|(_pid, p)| {
+                p.position_and_size().is_stacked && !p.position_and_size().rows.is_fixed()
+            })
+            .map(|(pid, _p)| *pid)
+            .collect();
+        let mut stacks = vec![];
+        for pane_id in all_flexible_panes_in_stack {
+            stacks.push(
+                self.positions_in_stack(&pane_id)
+                    .with_context(err_context)?,
+            );
+        }
+        Ok(stacks)
+    }
     fn fill_space_over_one_liner_pane(&mut self, id: &PaneId) -> Result<bool> {
         let (position_of_current_pane, position_of_flexible_pane) =
             self.position_of_current_and_flexible_pane(id)?;

zellij-server/src/panes/tiled_panes/tiled_pane_grid.rs

@@ -1366,6 +1366,21 @@ impl<'a> TiledPaneGrid<'a> {
             direction.map(|direction| (*t_id_to_split, direction))
         })
     }
+    pub fn has_room_for_new_stacked_pane(&self) -> bool {
+        let panes = self.panes.borrow();
+        let flexible_pane_in_stack: Vec<(&PaneId, &&mut Box<dyn Pane>)> = panes
+            .iter()
+            .filter(|(_, p)| {
+                p.selectable() && p.current_geom().is_stacked && !p.current_geom().rows.is_fixed()
+            })
+            .collect();
+        flexible_pane_in_stack
+            .iter()
+            .any(|(_p_id, p)| p.current_geom().rows.as_usize() > 1)
+    }
+    pub fn make_room_in_stack_for_pane(&mut self) -> Result<PaneGeom> {
+        StackedPanes::new(self.panes.clone()).make_room_for_new_pane()
+    }
 }
 
 pub fn split(direction: SplitDirection, rect: &PaneGeom) -> Option<(PaneGeom, PaneGeom)> {

zellij-server/src/tab/mod.rs

@@ -1916,8 +1916,6 @@ impl Tab {
         self.tiled_panes.focus_previous_pane(client_id);
     }
     pub fn focus_pane_on_edge(&mut self, direction: Direction, client_id: ClientId) {
-        let err_context = || format!("failed to move focus left for client {}", client_id);
-
         if self.floating_panes.panes_are_visible() {
             self.floating_panes.focus_pane_on_edge(direction, client_id);
         } else if self.has_selectable_panes() && !self.tiled_panes.fullscreen_is_active() {

zellij-server/src/tab/unit/snapshots/zellij_server__tab__tab_integration_tests__swap_tiled_layout_with_only_stacked_children.snap

@@ -0,0 +1,26 @@
+---
+source: zellij-server/src/tab/./unit/tab_integration_tests.rs
+assertion_line: 3253
+expression: snapshot
+---
+00 (C): ┌ Pane #1 ──────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
+01 (C): ┌ Pane #2 ──────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
+02 (C): ┌ Pane #3 ──────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
+03 (C): ┌ Pane #4 ──────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
+04 (C): │                                                                                                                       │
+05 (C): │                                                                                                                       │
+06 (C): │                                                                                                                       │
+07 (C): │                                                                                                                       │
+08 (C): │                                                                                                                       │
+09 (C): │                                                                                                                       │
+10 (C): │                                                                                                                       │
+11 (C): │                                                                                                                       │
+12 (C): │                                                                                                                       │
+13 (C): │                                                                                                                       │
+14 (C): │                                                                                                                       │
+15 (C): │                                                                                                                       │
+16 (C): │                                                                                                                       │
+17 (C): │                                                                                                                       │
+18 (C): │                                                                                                                       │
+19 (C): └───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘
+

zellij-server/src/tab/unit/tab_integration_tests.rs

@@ -3206,6 +3206,53 @@ fn swap_tiled_layout_with_stacked_children() {
     assert_snapshot!(snapshot);
 }
 
+#[test]
+fn swap_tiled_layout_with_only_stacked_children() {
+    let size = Size {
+        cols: 121,
+        rows: 20,
+    };
+    let client_id = 1;
+    let mut output = Output::default();
+    let swap_layouts = r#"
+        layout {
+            swap_tiled_layout {
+                tab {
+                    pane stacked=true { children; }
+                }
+            }
+        }
+    "#;
+    let layout = Layout::from_kdl(swap_layouts, "file_name.kdl".into(), None, None).unwrap();
+    let swap_tiled_layouts = layout.swap_tiled_layouts.clone();
+    let swap_floating_layouts = layout.swap_floating_layouts.clone();
+    let mut tab = create_new_tab_with_swap_layouts(
+        size,
+        ModeInfo::default(),
+        (swap_tiled_layouts, swap_floating_layouts),
+        None,
+        true,
+    );
+    let new_pane_id_1 = PaneId::Terminal(2);
+    let new_pane_id_2 = PaneId::Terminal(3);
+    let new_pane_id_3 = PaneId::Terminal(4);
+
+    tab.new_pane(new_pane_id_1, None, None, Some(client_id))
+        .unwrap();
+    tab.new_pane(new_pane_id_2, None, None, Some(client_id))
+        .unwrap();
+    tab.new_pane(new_pane_id_3, None, None, Some(client_id))
+        .unwrap();
+    tab.render(&mut output, None).unwrap();
+    let snapshot = take_snapshot(
+        output.serialize().unwrap().get(&client_id).unwrap(),
+        size.rows,
+        size.cols,
+        Palette::default(),
+    );
+    assert_snapshot!(snapshot);
+}
+
 #[test]
 fn swap_tiled_layout_with_stacked_children_and_no_pane_frames() {
     let size = Size {