fix(panes): show visual error when unable to split panes vertically/horizontally (#2025)

* fix(panes): show visual error when failing to split pane vertically/horizontally

* fix: lockfile

2 files changed, 160 insertions, 4 deletions

diff --git a/zellij-server/src/tab/mod.rs b/zellij-server/src/tab/mod.rs
index 63573a08e..a841b39ae 100644
--- a/zellij-server/src/tab/mod.rs
+++ b/zellij-server/src/tab/mod.rs
@@ -13,6 +13,7 @@ use zellij_utils::input::command::RunCommand;
 use zellij_utils::position::{Column, Line};
 use zellij_utils::{position::Position, serde};
 
+use crate::background_jobs::BackgroundJob;
 use crate::pty_writer::PtyWriteInstruction;
 use crate::screen::CopyOptions;
 use crate::ui::pane_boundaries_frame::FrameParams;
@@ -386,6 +387,9 @@ pub trait Pane {
     fn hold(&mut self, _exit_status: Option<i32>, _is_first_run: bool, _run_command: RunCommand) {
         // No-op by default, only terminal panes support holding
     }
+    fn add_red_pane_frame_color_override(&mut self, _error_text: Option<String>);
+    fn clear_pane_frame_color_override(&mut self);
+    fn frame_color_override(&self) -> Option<PaletteColor>;
 }
 
 #[derive(Clone, Debug)]
@@ -618,14 +622,18 @@ impl Tab {
                         .send_to_pty(PtyInstruction::ClosePane(PaneId::Terminal(*unused_pid)))
                         .with_context(err_context)?;
                 }
-                // FIXME: This is another hack to crop the viewport to fixed-size panes. Once you can have
-                // non-fixed panes that are part of the viewport, get rid of this!
+
+                // here we offset the viewport from borderless panes that are on the edges of the
+                // screen, this is so that when we don't have pane boundaries (eg. when they were
+                // disabled by the user) boundaries won't be drawn around these panes
+                // geometrically, we can only do this with panes that are on the edges of the
+                // screen - so it's mostly a best-effort thing
                 let display_area = {
                     let display_area = self.display_area.borrow();
                     *display_area
                 };
                 self.resize_whole_tab(display_area);
-                let boundary_geoms = self.tiled_panes.fixed_pane_geoms();
+                let boundary_geoms = self.tiled_panes.borderless_pane_geoms();
                 for geom in boundary_geoms {
                     self.offset_viewport(&geom)
                 }
@@ -1035,6 +1043,20 @@ impl Tab {
                 self.should_clear_display_before_rendering = true;
                 self.tiled_panes.focus_pane(pid, client_id);
             }
+        } else {
+            log::error!("No room to split pane horizontally");
+            if let Some(active_pane_id) = self.tiled_panes.get_active_pane_id(client_id) {
+                self.senders
+                    .send_to_background_jobs(BackgroundJob::DisplayPaneError(
+                        active_pane_id,
+                        "TOO SMALL!".into(),
+                    ))
+                    .with_context(err_context)?;
+            }
+            self.senders
+                .send_to_pty(PtyInstruction::ClosePane(pid))
+                .with_context(err_context)?;
+            return Ok(());
         }
         Ok(())
     }
@@ -1075,6 +1097,20 @@ impl Tab {
                 self.should_clear_display_before_rendering = true;
                 self.tiled_panes.focus_pane(pid, client_id);
             }
+        } else {
+            log::error!("No room to split pane vertically");
+            if let Some(active_pane_id) = self.tiled_panes.get_active_pane_id(client_id) {
+                self.senders
+                    .send_to_background_jobs(BackgroundJob::DisplayPaneError(
+                        active_pane_id,
+                        "TOO SMALL!".into(),
+                    ))
+                    .with_context(err_context)?;
+            }
+            self.senders
+                .send_to_pty(PtyInstruction::ClosePane(pid))
+                .with_context(err_context)?;
+            return Ok(());
         }
         Ok(())
     }
@@ -1139,6 +1175,11 @@ impl Tab {
                 .values()
                 .any(|s_p| s_p.pid() == PaneId::Plugin(plugin_id))
     }
+    pub fn has_pane_with_pid(&self, pid: &PaneId) -> bool {
+        self.tiled_panes.panes_contain(pid)
+            || self.floating_panes.panes_contain(pid)
+            || self.suppressed_panes.values().any(|s_p| s_p.pid() == *pid)
+    }
     pub fn handle_pty_bytes(&mut self, pid: u32, bytes: VteBytes) -> Result<()> {
         if self.is_pending {
             self.pending_instructions
@@ -2809,6 +2850,39 @@ impl Tab {
         self.is_pending
     }
 
+    pub fn add_red_pane_frame_color_override(
+        &mut self,
+        pane_id: PaneId,
+        error_text: Option<String>,
+    ) {
+        if let Some(pane) = self
+            .tiled_panes
+            .get_pane_mut(pane_id)
+            .or_else(|| self.floating_panes.get_pane_mut(pane_id))
+            .or_else(|| {
+                self.suppressed_panes
+                    .values_mut()
+                    .find(|s_p| s_p.pid() == pane_id)
+            })
+        {
+            pane.add_red_pane_frame_color_override(error_text);
+        }
+    }
+    pub fn clear_pane_frame_color_override(&mut self, pane_id: PaneId) {
+        if let Some(pane) = self
+            .tiled_panes
+            .get_pane_mut(pane_id)
+            .or_else(|| self.floating_panes.get_pane_mut(pane_id))
+            .or_else(|| {
+                self.suppressed_panes
+                    .values_mut()
+                    .find(|s_p| s_p.pid() == pane_id)
+            })
+        {
+            pane.clear_pane_frame_color_override();
+        }
+    }
+
     fn show_floating_panes(&mut self) {
         // this function is to be preferred to directly invoking floating_panes.toggle_show_panes(true)
         self.floating_panes.toggle_show_panes(true);
diff --git a/zellij-server/src/tab/unit/tab_tests.rs b/zellij-server/src/tab/unit/tab_tests.rs
index a676f3c68..192c55e3e 100644
--- a/zellij-server/src/tab/unit/tab_tests.rs
+++ b/zellij-server/src/tab/unit/tab_tests.rs
@@ -10,7 +10,7 @@ use crate::{
 use std::path::PathBuf;
 use zellij_utils::data::{Direction, Resize, ResizeStrategy};
 use zellij_utils::errors::prelude::*;
-use zellij_utils::input::layout::PaneLayout;
+use zellij_utils::input::layout::{PaneLayout, SplitDirection, SplitSize};
 use zellij_utils::ipc::IpcReceiverWithContext;
 use zellij_utils::pane_size::{Size, SizeInPixels};
 
@@ -187,6 +187,62 @@ fn create_new_tab(size: Size) -> Tab {
     tab
 }
 
+fn create_new_tab_with_layout(size: Size, layout: PaneLayout) -> Tab {
+    let index = 0;
+    let position = 0;
+    let name = String::new();
+    let os_api = Box::new(FakeInputOutput {});
+    let senders = ThreadSenders::default().silently_fail_on_send();
+    let max_panes = None;
+    let mode_info = ModeInfo::default();
+    let style = Style::default();
+    let draw_pane_frames = true;
+    let client_id = 1;
+    let session_is_mirrored = true;
+    let mut connected_clients = HashSet::new();
+    let character_cell_info = Rc::new(RefCell::new(None));
+    connected_clients.insert(client_id);
+    let connected_clients = Rc::new(RefCell::new(connected_clients));
+    let terminal_emulator_colors = Rc::new(RefCell::new(Palette::default()));
+    let copy_options = CopyOptions::default();
+    let sixel_image_store = Rc::new(RefCell::new(SixelImageStore::default()));
+    let terminal_emulator_color_codes = Rc::new(RefCell::new(HashMap::new()));
+    let mut tab = Tab::new(
+        index,
+        position,
+        name,
+        size,
+        character_cell_info,
+        sixel_image_store,
+        os_api,
+        senders,
+        max_panes,
+        style,
+        mode_info,
+        draw_pane_frames,
+        connected_clients,
+        session_is_mirrored,
+        client_id,
+        copy_options,
+        terminal_emulator_colors,
+        terminal_emulator_color_codes,
+    );
+    let mut new_terminal_ids = vec![];
+    for i in 0..layout.extract_run_instructions().len() {
+        new_terminal_ids.push((i as u32, None));
+    }
+    tab.apply_layout(
+        layout,
+        // vec![(1, None), (2, None)],
+        new_terminal_ids,
+        HashMap::new(),
+        index,
+        client_id,
+    )
+    .unwrap();
+    tab
+}
+
 fn create_new_tab_with_cell_size(
     size: Size,
     character_cell_size: Rc<RefCell<Option<SizeInPixels>>>,
@@ -678,6 +734,32 @@ pub fn cannot_split_largest_pane_when_there_is_no_room() {
 }
 
 #[test]
+pub fn cannot_split_panes_vertically_when_active_pane_has_fixed_columns() {
+    let size = Size { cols: 50, rows: 20 };
+    let mut initial_layout = PaneLayout::default();
+    initial_layout.children_split_direction = SplitDirection::Vertical;
+    let mut fixed_child = PaneLayout::default();
+    fixed_child.split_size = Some(SplitSize::Fixed(30));
+    initial_layout.children = vec![fixed_child, PaneLayout::default()];
+    let mut tab = create_new_tab_with_layout(size, initial_layout);
+    tab.vertical_split(PaneId::Terminal(3), None, 1).unwrap();
+    assert_eq!(tab.tiled_panes.panes.len(), 2, "Tab still has two panes");
+}
+
+#[test]
+pub fn cannot_split_panes_horizontally_when_active_pane_has_fixed_rows() {
+    let size = Size { cols: 50, rows: 20 };
+    let mut initial_layout = PaneLayout::default();
+    initial_layout.children_split_direction = SplitDirection::Horizontal;
+    let mut fixed_child = PaneLayout::default();
+    fixed_child.split_size = Some(SplitSize::Fixed(12));
+    initial_layout.children = vec![fixed_child, PaneLayout::default()];
+    let mut tab = create_new_tab_with_layout(size, initial_layout);
+    tab.horizontal_split(PaneId::Terminal(3), None, 1).unwrap();
+    assert_eq!(tab.tiled_panes.panes.len(), 2, "Tab still has two panes");
+}
+
+#[test]
 pub fn toggle_focused_pane_fullscreen() {
     let size = Size {
         cols: 121,