fix(layouts): various kdl layout issues and features (#1797)

* fix(layouts): error on non-bare children node

* refactor(layout): consolidate split direction parsing

* refactor(layout): remove unused import

* fix(layouts): log error when there is no room for layout

* fix(layout): error on size 0

* feat(layouts): allow pane templates to override template command attributes

* style(fmt): rustfmt

zellij-server/src/tab/mod.rs

@@ -492,109 +492,124 @@ impl Tab {
         free_space.cols.set_inner(viewport_cols);
         free_space.rows.set_inner(viewport_rows);
 
-        let positions_in_layout = layout.position_panes_in_space(&free_space);
+        match layout.position_panes_in_space(&free_space) {
+            Ok(positions_in_layout) => {
+                let positions_and_size = positions_in_layout.iter();
+                let mut new_ids = new_ids.iter();
 
-        let positions_and_size = positions_in_layout.iter();
+                let mut focus_pane_id: Option<PaneId> = None;
-        let mut new_ids = new_ids.iter();
+                let mut set_focus_pane_id = |layout: &PaneLayout, pane_id: PaneId| {
+                    if layout.focus.unwrap_or(false) && focus_pane_id.is_none() {
+                        focus_pane_id = Some(pane_id);
+                    }
+                };
 
-        let mut focus_pane_id: Option<PaneId> = None;
+                for (layout, position_and_size) in positions_and_size {
-        let mut set_focus_pane_id = |layout: &PaneLayout, pane_id: PaneId| {
+                    // A plugin pane
-            if layout.focus.unwrap_or(false) && focus_pane_id.is_none() {
+                    if let Some(Run::Plugin(run)) = layout.run.clone() {
-                focus_pane_id = Some(pane_id);
+                        let (pid_tx, pid_rx) = channel();
-            }
+                        let pane_title = run.location.to_string();
-        };
+                        self.senders
-
+                            .send_to_plugin(PluginInstruction::Load(
-        for (layout, position_and_size) in positions_and_size {
+                                pid_tx, run, tab_index, client_id,
-            // A plugin pane
+                            ))
-            if let Some(Run::Plugin(run)) = layout.run.clone() {
+                            .with_context(err_context)?;
-                let (pid_tx, pid_rx) = channel();
+                        let pid = pid_rx.recv().with_context(err_context)?;
-                let pane_title = run.location.to_string();
+                        let mut new_plugin = PluginPane::new(
-                self.senders
+                            pid,
-                    .send_to_plugin(PluginInstruction::Load(pid_tx, run, tab_index, client_id))
+                            *position_and_size,
-                    .with_context(err_context)?;
+                            self.senders
-                let pid = pid_rx.recv().with_context(err_context)?;
+                                .to_plugin
-                let mut new_plugin = PluginPane::new(
+                                .as_ref()
-                    pid,
+                                .with_context(err_context)?
-                    *position_and_size,
+                                .clone(),
-                    self.senders
+                            pane_title,
-                        .to_plugin
+                            layout.name.clone().unwrap_or_default(),
-                        .as_ref()
+                        );
-                        .with_context(err_context)?
+                        new_plugin.set_borderless(layout.borderless);
-                        .clone(),
+                        self.tiled_panes
-                    pane_title,
+                            .add_pane_with_existing_geom(PaneId::Plugin(pid), Box::new(new_plugin));
-                    layout.name.clone().unwrap_or_default(),
+                        set_focus_pane_id(layout, PaneId::Plugin(pid));
-                );
+                    } else {
-                new_plugin.set_borderless(layout.borderless);
+                        // there are still panes left to fill, use the pids we received in this method
-                self.tiled_panes
+                        if let Some(pid) = new_ids.next() {
-                    .add_pane_with_existing_geom(PaneId::Plugin(pid), Box::new(new_plugin));
+                            let next_terminal_position = self.get_next_terminal_position();
-                set_focus_pane_id(layout, PaneId::Plugin(pid));
+                            let initial_title = match &layout.run {
-            } else {
+                                Some(Run::Command(run_command)) => Some(run_command.to_string()),
-                // there are still panes left to fill, use the pids we received in this method
+                                _ => None,
-                if let Some(pid) = new_ids.next() {
+                            };
-                    let next_terminal_position = self.get_next_terminal_position();
+                            let mut new_pane = TerminalPane::new(
-                    let initial_title = match &layout.run {
+                                *pid,
-                        Some(Run::Command(run_command)) => Some(run_command.to_string()),
+                                *position_and_size,
-                        _ => None,
+                                self.style,
-                    };
+                                next_terminal_position,
-                    let mut new_pane = TerminalPane::new(
+                                layout.name.clone().unwrap_or_default(),
-                        *pid,
+                                self.link_handler.clone(),
-                        *position_and_size,
+                                self.character_cell_size.clone(),
-                        self.style,
+                                self.sixel_image_store.clone(),
-                        next_terminal_position,
+                                self.terminal_emulator_colors.clone(),
-                        layout.name.clone().unwrap_or_default(),
+                                self.terminal_emulator_color_codes.clone(),
-                        self.link_handler.clone(),
+                                initial_title,
-                        self.character_cell_size.clone(),
+                            );
-                        self.sixel_image_store.clone(),
+                            new_pane.set_borderless(layout.borderless);
-                        self.terminal_emulator_colors.clone(),
+                            self.tiled_panes.add_pane_with_existing_geom(
-                        self.terminal_emulator_color_codes.clone(),
+                                PaneId::Terminal(*pid),
-                        initial_title,
+                                Box::new(new_pane),
-                    );
+                            );
-                    new_pane.set_borderless(layout.borderless);
+                            set_focus_pane_id(layout, PaneId::Terminal(*pid));
-                    self.tiled_panes
+                        }
-                        .add_pane_with_existing_geom(PaneId::Terminal(*pid), Box::new(new_pane));
+                    }
-                    set_focus_pane_id(layout, PaneId::Terminal(*pid));
                 }
-            }
+                for unused_pid in new_ids {
-        }
+                    // this is a bit of a hack and happens because we don't have any central location that
-        for unused_pid in new_ids {
+                    // can query the screen as to how many panes it needs to create a layout
-            // this is a bit of a hack and happens because we don't have any central location that
+                    // fixing this will require a bit of an architecture change
-            // can query the screen as to how many panes it needs to create a layout
+                    self.senders
-            // fixing this will require a bit of an architecture change
+                        .send_to_pty(PtyInstruction::ClosePane(PaneId::Terminal(*unused_pid)))
-            self.senders
+                        .with_context(err_context)?;
-                .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!
-        // FIXME: This is another hack to crop the viewport to fixed-size panes. Once you can have
+                let display_area = {
-        // non-fixed panes that are part of the viewport, get rid of this!
+                    let display_area = self.display_area.borrow();
-        let display_area = {
+                    *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();
-        self.resize_whole_tab(display_area);
+                for geom in boundary_geoms {
-        let boundary_geoms = self.tiled_panes.fixed_pane_geoms();
+                    self.offset_viewport(&geom)
-        for geom in boundary_geoms {
+                }
-            self.offset_viewport(&geom)
+                self.tiled_panes.set_pane_frames(self.draw_pane_frames);
-        }
+                self.should_clear_display_before_rendering = true;
-        self.tiled_panes.set_pane_frames(self.draw_pane_frames);
-        self.should_clear_display_before_rendering = true;
 
-        if let Some(pane_id) = focus_pane_id {
+                if let Some(pane_id) = focus_pane_id {
-            self.focus_pane_id = Some(pane_id);
+                    self.focus_pane_id = Some(pane_id);
-            self.tiled_panes.focus_pane(pane_id, client_id);
+                    self.tiled_panes.focus_pane(pane_id, client_id);
-        } else {
+                } else {
-            // This is the end of the nasty viewport hack...
+                    // This is the end of the nasty viewport hack...
-            let next_selectable_pane_id = self.tiled_panes.first_selectable_pane_id();
+                    let next_selectable_pane_id = self.tiled_panes.first_selectable_pane_id();
-            match next_selectable_pane_id {
+                    match next_selectable_pane_id {
-                Some(active_pane_id) => {
+                        Some(active_pane_id) => {
-                    self.tiled_panes.focus_pane(active_pane_id, client_id);
+                            self.tiled_panes.focus_pane(active_pane_id, client_id);
-                },
+                        },
-                None => {
+                        None => {
-                    // this is very likely a configuration error (layout with no selectable panes)
+                            // this is very likely a configuration error (layout with no selectable panes)
-                    self.tiled_panes.clear_active_panes();
+                            self.tiled_panes.clear_active_panes();
-                },
+                        },
-            }
+                    }
+                }
+                Ok(())
+            },
+            Err(e) => {
+                for unused_pid in new_ids {
+                    self.senders
+                        .send_to_pty(PtyInstruction::ClosePane(PaneId::Terminal(unused_pid)))
+                        .with_context(err_context)?;
+                }
+                log::error!("{}", e); // TODO: propagate this to the user
+                Ok(())
+            },
         }
-        Ok(())
     }
     pub fn update_input_modes(&mut self) -> Result<()> {
         // this updates all plugins with the client's input mode

zellij-utils/src/input/layout.rs

@@ -156,8 +156,17 @@ impl PaneLayout {
         }
         count
     }
-    pub fn position_panes_in_space(&self, space: &PaneGeom) -> Vec<(PaneLayout, PaneGeom)> {
+    pub fn position_panes_in_space(
-        split_space(space, self, space)
+        &self,
+        space: &PaneGeom,
+    ) -> Result<Vec<(PaneLayout, PaneGeom)>, &'static str> {
+        let layouts = split_space(space, self, space);
+        for (_pane_layout, pane_geom) in layouts.iter() {
+            if !pane_geom.is_at_least_minimum_size() {
+                return Err("No room on screen for this layout!");
+            }
+        }
+        Ok(layouts)
     }
     pub fn extract_run_instructions(&self) -> Vec<Option<Run>> {
         let mut run_instructions = vec![];

zellij-utils/src/input/unit/layout_test.rs

@@ -700,7 +700,7 @@ fn children_not_as_first_child_of_pane_template() {
                 }
                 pane
             }
-            horizontal-with-vertical-top name="my tab" {
+            horizontal-with-vertical-top name="my pane" {
                 pane
                 pane
             }
@@ -1014,3 +1014,140 @@ fn error_on_more_than_one_focused_tab() {
     let layout_error = Layout::from_kdl(kdl_layout, "layout_file_name".into()).unwrap_err();
     assert_snapshot!(format!("{:?}", layout_error));
 }
+
+#[test]
+fn args_override_args_in_template() {
+    let kdl_layout = r#"
+        layout {
+            pane_template name="tail" {
+                command "tail"
+                args "-f" "/tmp/foo"
+            }
+            tail
+            tail {
+                args "-f" "/tmp/bar"
+            }
+        }
+    "#;
+    let layout = Layout::from_kdl(kdl_layout, "layout_file_name".into()).unwrap();
+    assert_snapshot!(format!("{:#?}", layout));
+}
+
+#[test]
+fn args_added_to_args_in_template() {
+    let kdl_layout = r#"
+        layout {
+            pane_template name="tail" {
+                command "tail"
+            }
+            tail
+            tail {
+                args "-f" "/tmp/bar"
+            }
+        }
+    "#;
+    let layout = Layout::from_kdl(kdl_layout, "layout_file_name".into()).unwrap();
+    assert_snapshot!(format!("{:#?}", layout));
+}
+
+#[test]
+fn cwd_override_cwd_in_template() {
+    let kdl_layout = r#"
+        layout {
+            pane_template name="tail" {
+                command "tail"
+                cwd "/tmp"
+            }
+            tail
+            tail {
+                cwd "/"
+            }
+        }
+    "#;
+    let layout = Layout::from_kdl(kdl_layout, "layout_file_name".into()).unwrap();
+    assert_snapshot!(format!("{:#?}", layout));
+}
+
+#[test]
+fn cwd_added_to_cwd_in_template() {
+    let kdl_layout = r#"
+        layout {
+            pane_template name="tail" {
+                command "tail"
+            }
+            tail
+            tail {
+                cwd "/home"
+            }
+        }
+    "#;
+    let layout = Layout::from_kdl(kdl_layout, "layout_file_name".into()).unwrap();
+    assert_snapshot!(format!("{:#?}", layout));
+}
+
+#[test]
+fn error_on_mixed_command_and_child_panes() {
+    let kdl_layout = r#"
+        layout {
+            pane command="tail" {
+                pane
+            }
+        }
+    "#;
+    let layout = Layout::from_kdl(kdl_layout, "layout_file_name".into());
+    assert!(layout.is_err(), "error provided");
+}
+
+#[test]
+fn error_on_bare_args_without_command() {
+    let kdl_layout = r#"
+        layout {
+            pane {
+                args "-f"
+            }
+        }
+    "#;
+    let layout = Layout::from_kdl(kdl_layout, "layout_file_name".into());
+    assert!(layout.is_err(), "error provided");
+}
+
+#[test]
+fn error_on_bare_cwd_without_command() {
+    let kdl_layout = r#"
+        layout {
+            pane {
+                cwd "/"
+            }
+        }
+    "#;
+    let layout = Layout::from_kdl(kdl_layout, "layout_file_name".into());
+    assert!(layout.is_err(), "error provided");
+}
+
+#[test]
+fn error_on_bare_cwd_in_template_without_command() {
+    let kdl_layout = r#"
+        layout {
+            pane_template name="my_template"
+            my_template {
+                cwd "/"
+            }
+        }
+    "#;
+    let layout = Layout::from_kdl(kdl_layout, "layout_file_name".into());
+    assert!(layout.is_err(), "error provided");
+}
+
+#[test]
+fn error_on_bare_args_in_template_without_command() {
+    let kdl_layout = r#"
+        layout {
+            pane_template name="my_template"
+            my_template {
+                args "--help"
+            }
+        }
+    "#;
+    let layout = Layout::from_kdl(kdl_layout, "layout_file_name".into());
+    assert!(layout.is_err(), "error provided");
+}

zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__args_added_to_args_in_template.snap

@@ -0,0 +1,63 @@
+---
+source: zellij-utils/src/input/./unit/layout_test.rs
+assertion_line: 1050
+expression: "format!(\"{:#?}\", layout)"
+---
+Layout {
+    tabs: [],
+    focused_tab_index: None,
+    template: Some(
+        PaneLayout {
+            children_split_direction: Horizontal,
+            name: None,
+            children: [
+                PaneLayout {
+                    children_split_direction: Horizontal,
+                    name: None,
+                    children: [],
+                    split_size: None,
+                    run: Some(
+                        Command(
+                            RunCommand {
+                                command: "tail",
+                                args: [],
+                                cwd: None,
+                                hold_on_close: true,
+                            },
+                        ),
+                    ),
+                    borderless: false,
+                    focus: None,
+                    external_children_index: None,
+                },
+                PaneLayout {
+                    children_split_direction: Horizontal,
+                    name: None,
+                    children: [],
+                    split_size: None,
+                    run: Some(
+                        Command(
+                            RunCommand {
+                                command: "tail",
+                                args: [
+                                    "-f",
+                                    "/tmp/bar",
+                                ],
+                                cwd: None,
+                                hold_on_close: true,
+                            },
+                        ),
+                    ),
+                    borderless: false,
+                    focus: None,
+                    external_children_index: None,
+                },
+            ],
+            split_size: None,
+            run: None,
+            borderless: false,
+            focus: None,
+            external_children_index: None,
+        },
+    ),
+}

zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__args_override_args_in_template.snap

@@ -0,0 +1,66 @@
+---
+source: zellij-utils/src/input/./unit/layout_test.rs
+assertion_line: 1033
+expression: "format!(\"{:#?}\", layout)"
+---
+Layout {
+    tabs: [],
+    focused_tab_index: None,
+    template: Some(
+        PaneLayout {
+            children_split_direction: Horizontal,
+            name: None,
+            children: [
+                PaneLayout {
+                    children_split_direction: Horizontal,
+                    name: None,
+                    children: [],
+                    split_size: None,
+                    run: Some(
+                        Command(
+                            RunCommand {
+                                command: "tail",
+                                args: [
+                                    "-f",
+                                    "/tmp/foo",
+                                ],
+                                cwd: None,
+                                hold_on_close: true,
+                            },
+                        ),
+                    ),
+                    borderless: false,
+                    focus: None,
+                    external_children_index: None,
+                },
+                PaneLayout {
+                    children_split_direction: Horizontal,
+                    name: None,
+                    children: [],
+                    split_size: None,
+                    run: Some(
+                        Command(
+                            RunCommand {
+                                command: "tail",
+                                args: [
+                                    "-f",
+                                    "/tmp/bar",
+                                ],
+                                cwd: None,
+                                hold_on_close: true,
+                            },
+                        ),
+                    ),
+                    borderless: false,
+                    focus: None,
+                    external_children_index: None,
+                },
+            ],
+            split_size: None,
+            run: None,
+            borderless: false,
+            focus: None,
+            external_children_index: None,
+        },
+    ),
+}

zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__children_not_as_first_child_of_pane_template.snap

@@ -1,6 +1,6 @@
 ---
 source: zellij-utils/src/input/./unit/layout_test.rs
-assertion_line: 848
+assertion_line: 711
 expression: "format!(\"{:#?}\", layout)"
 ---
 Layout {
@@ -13,7 +13,9 @@ Layout {
             children: [
                 PaneLayout {
                     children_split_direction: Horizontal,
-                    name: None,
+                    name: Some(
+                        "my pane",
+                    ),
                     children: [
                         PaneLayout {
                             children_split_direction: Vertical,

zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__cwd_added_to_cwd_in_template.snap

@@ -0,0 +1,62 @@
+---
+source: zellij-utils/src/input/./unit/layout_test.rs
+assertion_line: 1085
+expression: "format!(\"{:#?}\", layout)"
+---
+Layout {
+    tabs: [],
+    focused_tab_index: None,
+    template: Some(
+        PaneLayout {
+            children_split_direction: Horizontal,
+            name: None,
+            children: [
+                PaneLayout {
+                    children_split_direction: Horizontal,
+                    name: None,
+                    children: [],
+                    split_size: None,
+                    run: Some(
+                        Command(
+                            RunCommand {
+                                command: "tail",
+                                args: [],
+                                cwd: None,
+                                hold_on_close: true,
+                            },
+                        ),
+                    ),
+                    borderless: false,
+                    focus: None,
+                    external_children_index: None,
+                },
+                PaneLayout {
+                    children_split_direction: Horizontal,
+                    name: None,
+                    children: [],
+                    split_size: None,
+                    run: Some(
+                        Command(
+                            RunCommand {
+                                command: "tail",
+                                args: [],
+                                cwd: Some(
+                                    "/home",
+                                ),
+                                hold_on_close: true,
+                            },
+                        ),
+                    ),
+                    borderless: false,
+                    focus: None,
+                    external_children_index: None,
+                },
+            ],
+            split_size: None,
+            run: None,
+            borderless: false,
+            focus: None,
+            external_children_index: None,
+        },
+    ),
+}

zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__cwd_override_cwd_in_template.snap

@@ -0,0 +1,64 @@
+---
+source: zellij-utils/src/input/./unit/layout_test.rs
+assertion_line: 1068
+expression: "format!(\"{:#?}\", layout)"
+---
+Layout {
+    tabs: [],
+    focused_tab_index: None,
+    template: Some(
+        PaneLayout {
+            children_split_direction: Horizontal,
+            name: None,
+            children: [
+                PaneLayout {
+                    children_split_direction: Horizontal,
+                    name: None,
+                    children: [],
+                    split_size: None,
+                    run: Some(
+                        Command(
+                            RunCommand {
+                                command: "tail",
+                                args: [],
+                                cwd: Some(
+                                    "/tmp",
+                                ),
+                                hold_on_close: true,
+                            },
+                        ),
+                    ),
+                    borderless: false,
+                    focus: None,
+                    external_children_index: None,
+                },
+                PaneLayout {
+                    children_split_direction: Horizontal,
+                    name: None,
+                    children: [],
+                    split_size: None,
+                    run: Some(
+                        Command(
+                            RunCommand {
+                                command: "tail",
+                                args: [],
+                                cwd: Some(
+                                    "/",
+                                ),
+                                hold_on_close: true,
+                            },
+                        ),
+                    ),
+                    borderless: false,
+                    focus: None,
+                    external_children_index: None,
+                },
+            ],
+            split_size: None,
+            run: None,
+            borderless: false,
+            focus: None,
+            external_children_index: None,
+        },
+    ),
+}

zellij-utils/src/kdl/kdl_layout_parser.rs

@@ -12,7 +12,7 @@ use std::str::FromStr;
 use crate::{
     kdl_child_with_name, kdl_children_nodes, kdl_get_bool_property_or_child_value,
     kdl_get_bool_property_or_child_value_with_error, kdl_get_child,
-    kdl_get_int_property_or_child_value, kdl_get_property_or_child, kdl_get_string_entry,
+    kdl_get_int_property_or_child_value, kdl_get_property_or_child,
     kdl_get_string_property_or_child_value, kdl_get_string_property_or_child_value_with_error,
     kdl_name, kdl_parsing_error, kdl_property_names, kdl_property_or_child_value_node,
     kdl_string_arguments,
@@ -92,8 +92,22 @@ impl<'a> KdlLayoutParser<'a> {
     }
     fn parse_split_size(&self, kdl_node: &KdlNode) -> Result<Option<SplitSize>, ConfigError> {
         if let Some(size) = kdl_get_string_property_or_child_value!(kdl_node, "size") {
-            Ok(Some(SplitSize::from_str(size)?))
+            match SplitSize::from_str(size) {
+                Ok(size) => Ok(Some(size)),
+                Err(_e) => Err(kdl_parsing_error!(
+                    format!(
+                        "size should be a fixed number (eg. 1) or a quoted percent (eg. \"50%\")"
+                    ),
+                    kdl_node
+                )),
+            }
         } else if let Some(size) = kdl_get_int_property_or_child_value!(kdl_node, "size") {
+            if size == 0 {
+                return Err(kdl_parsing_error!(
+                    format!("size should be greater than 0"),
+                    kdl_node
+                ));
+            }
             Ok(Some(SplitSize::Fixed(size as usize)))
         } else if let Some(node) = kdl_property_or_child_value_node!(kdl_node, "size") {
             Err(kdl_parsing_error!(
@@ -143,37 +157,44 @@ impl<'a> KdlLayoutParser<'a> {
             location,
         })))
     }
-    fn parse_pane_command(&self, pane_node: &KdlNode) -> Result<Option<Run>, ConfigError> {
+    fn parse_args(&self, pane_node: &KdlNode) -> Result<Option<Vec<String>>, ConfigError> {
-        let command = kdl_get_string_property_or_child_value_with_error!(pane_node, "command")
+        match kdl_get_child!(pane_node, "args") {
-            .map(|c| PathBuf::from(c));
-        let cwd = kdl_get_string_property_or_child_value_with_error!(pane_node, "cwd")
-            .map(|c| PathBuf::from(c));
-        let args = match kdl_get_child!(pane_node, "args") {
             Some(kdl_args) => {
                 if kdl_args.entries().is_empty() {
                     return Err(kdl_parsing_error!(format!("args cannot be empty and should contain one or more command arguments (eg. args \"-h\" \"-v\")"), kdl_args));
                 }
-                Some(
+                Ok(Some(
                     kdl_string_arguments!(kdl_args)
                         .iter()
                         .map(|s| String::from(*s))
                         .collect(),
-                )
+                ))
             },
-            None => None,
+            None => Ok(None),
-        };
+        }
-        match (command, cwd, args) {
+    }
-            (None, Some(_cwd), _) => Err(ConfigError::new_kdl_error(
+    fn parse_pane_command(
+        &self,
+        pane_node: &KdlNode,
+        is_template: bool,
+    ) -> Result<Option<Run>, ConfigError> {
+        let command = kdl_get_string_property_or_child_value_with_error!(pane_node, "command")
+            .map(|c| PathBuf::from(c));
+        let cwd = kdl_get_string_property_or_child_value_with_error!(pane_node, "cwd")
+            .map(|c| PathBuf::from(c));
+        let args = self.parse_args(pane_node)?;
+        match (command, cwd, args, is_template) {
+            (None, Some(_cwd), _, false) => Err(ConfigError::new_kdl_error(
                 "cwd can only be set if a command was specified".into(),
                 pane_node.span().offset(),
                 pane_node.span().len(),
             )),
-            (None, _, Some(_args)) => Err(ConfigError::new_kdl_error(
+            (None, _, Some(_args), false) => Err(ConfigError::new_kdl_error(
                 "args can only be set if a command was specified".into(),
                 pane_node.span().offset(),
                 pane_node.span().len(),
             )),
-            (Some(command), cwd, args) => Ok(Some(Run::Command(RunCommand {
+            (Some(command), cwd, args, _) => Ok(Some(Run::Command(RunCommand {
                 command,
                 args: args.unwrap_or_else(|| vec![]),
                 cwd,
@@ -186,7 +207,24 @@ impl<'a> KdlLayoutParser<'a> {
         &self,
         kdl_node: &KdlNode,
     ) -> Result<Option<Run>, ConfigError> {
-        let mut run = self.parse_pane_command(kdl_node)?;
+        let mut run = self.parse_pane_command(kdl_node, false)?;
+        if let Some(plugin_block) = kdl_get_child!(kdl_node, "plugin") {
+            if run.is_some() {
+                return Err(ConfigError::new_kdl_error(
+                    "Cannot have both a command and a plugin block for a single pane".into(),
+                    plugin_block.span().offset(),
+                    plugin_block.span().len(),
+                ));
+            }
+            run = self.parse_plugin_block(plugin_block)?;
+        }
+        Ok(run)
+    }
+    fn parse_command_or_plugin_block_for_template(
+        &self,
+        kdl_node: &KdlNode,
+    ) -> Result<Option<Run>, ConfigError> {
+        let mut run = self.parse_pane_command(kdl_node, true)?;
         if let Some(plugin_block) = kdl_get_child!(kdl_node, "plugin") {
             if run.is_some() {
                 return Err(ConfigError::new_kdl_error(
@@ -235,56 +273,90 @@ impl<'a> KdlLayoutParser<'a> {
         let focus = kdl_get_bool_property_or_child_value_with_error!(kdl_node, "focus");
         let name = kdl_get_string_property_or_child_value_with_error!(kdl_node, "name")
             .map(|name| name.to_string());
+        let args = self.parse_args(kdl_node)?;
+        let cwd = kdl_get_string_property_or_child_value_with_error!(kdl_node, "cwd")
+            .map(|c| PathBuf::from(c));
         let split_size = self.parse_split_size(kdl_node)?;
-        let run = self.parse_command_or_plugin_block(kdl_node)?;
+        let run = self.parse_command_or_plugin_block_for_template(kdl_node)?;
+        if let (None, None, true, true) | (None, None, false, true) | (None, None, true, false) =
+            (&run, &pane_layout.run, args.is_some(), cwd.is_some())
+        {
+            let mut offending_nodes = vec![];
+            if args.is_some() {
+                offending_nodes.push("args");
+            }
+            if cwd.is_some() {
+                offending_nodes.push("cwd");
+            }
+            return Err(kdl_parsing_error!(
+                format!("{} can only be specified if a command was specified either in the pane_template or in the pane", offending_nodes.join(" and ")),
+                kdl_node
+            ));
+        }
 
         let children_split_direction = self.parse_split_direction(kdl_node)?;
-        match kdl_children_nodes!(kdl_node) {
+
-            Some(children) => {
+        let (external_children_index, pane_parts) = match kdl_children_nodes!(kdl_node) {
-                let child_panes = self.parse_child_pane_nodes_for_tab(children)?;
+            Some(children) => self.parse_child_pane_nodes_for_pane(&children)?,
-                let child_panes_layout = PaneLayout {
+            None => (None, vec![]),
-                    children_split_direction,
+        };
-                    children: child_panes,
+        if pane_parts.len() > 0 {
-                    ..Default::default()
+            let child_panes_layout = PaneLayout {
-                };
+                children_split_direction,
-                self.assert_one_children_block(&pane_layout, pane_layout_kdl_node)?;
+                children: pane_parts,
-                self.insert_layout_children_or_error(
+                external_children_index,
-                    &mut pane_layout,
+                ..Default::default()
-                    child_panes_layout,
+            };
-                    pane_layout_kdl_node,
+            self.assert_one_children_block(&pane_layout, pane_layout_kdl_node)?;
-                )?;
+            self.insert_layout_children_or_error(
-            },
+                &mut pane_layout,
-            None => {
+                child_panes_layout,
-                if let Some(borderless) = borderless {
+                pane_layout_kdl_node,
-                    pane_layout.borderless = borderless;
+            )?;
-                }
+        }
-                if let Some(focus) = focus {
+        if let Some(borderless) = borderless {
-                    pane_layout.focus = Some(focus);
+            pane_layout.borderless = borderless;
-                }
+        }
-                if let Some(name) = name {
+        if let Some(focus) = focus {
-                    pane_layout.name = Some(name);
+            pane_layout.focus = Some(focus);
-                }
+        }
-                if let Some(split_size) = split_size {
+        if let Some(name) = name {
-                    pane_layout.split_size = Some(split_size);
+            pane_layout.name = Some(name);
-                }
+        }
-                if let Some(run) = run {
+        if let Some(split_size) = split_size {
-                    pane_layout.run = Some(run);
+            pane_layout.split_size = Some(split_size);
-                }
+        }
-                if let Some(index_of_children) = pane_layout.external_children_index {
+        if let Some(run) = run {
-                    pane_layout
+            pane_layout.run = Some(run);
-                        .children
+        }
-                        .insert(index_of_children, PaneLayout::default());
+        if let (Some(args), Some(Run::Command(run_command))) = (args, pane_layout.run.as_mut()) {
-                }
+            run_command.args = args.clone();
-            },
+        }
+        if let (Some(cwd), Some(Run::Command(run_command))) = (cwd, pane_layout.run.as_mut()) {
+            run_command.cwd = Some(cwd.clone());
+        }
+        if let Some(index_of_children) = pane_layout.external_children_index {
+            pane_layout
+                .children
+                .insert(index_of_children, PaneLayout::default());
         }
         pane_layout.external_children_index = None;
         Ok(pane_layout)
     }
     fn parse_split_direction(&self, kdl_node: &KdlNode) -> Result<SplitDirection, ConfigError> {
-        Ok(match kdl_get_string_entry!(kdl_node, "split_direction") {
+        match kdl_get_string_property_or_child_value_with_error!(kdl_node, "split_direction") {
-            Some(direction) => SplitDirection::from_str(direction)?,
+            Some(direction) => match SplitDirection::from_str(direction) {
-            None => SplitDirection::default(),
+                Ok(split_direction) => Ok(split_direction),
-        })
+                Err(_e) => Err(kdl_parsing_error!(
+                    format!(
+                        "split_direction should be either \"horizontal\" or \"vertical\" found: {}",
+                        direction
+                    ),
+                    kdl_node
+                )),
+            },
+            None => Ok(SplitDirection::default()),
+        }
     }
     fn parse_pane_template_node(&mut self, kdl_node: &KdlNode) -> Result<(), ConfigError> {
         self.assert_valid_pane_properties(kdl_node)?;
@@ -333,10 +405,7 @@ impl<'a> KdlLayoutParser<'a> {
         let tab_name =
             kdl_get_string_property_or_child_value!(kdl_node, "name").map(|s| s.to_string());
         let is_focused = kdl_get_bool_property_or_child_value!(kdl_node, "focus").unwrap_or(false);
-        let children_split_direction = match kdl_get_string_entry!(kdl_node, "split_direction") {
+        let children_split_direction = self.parse_split_direction(kdl_node)?;
-            Some(direction) => SplitDirection::from_str(direction)?,
-            None => SplitDirection::default(),
-        };
         let children = match kdl_children_nodes!(kdl_node) {
             Some(children) => self.parse_child_pane_nodes_for_tab(children)?,
             None => vec![],
@@ -397,6 +466,15 @@ impl<'a> KdlLayoutParser<'a> {
             if kdl_name!(child) == "pane" {
                 nodes.push(self.parse_pane_node(child)?);
             } else if kdl_name!(child) == "children" {
+                let node_has_child_nodes = child.children().map(|c| !c.is_empty()).unwrap_or(false);
+                let node_has_entries = !child.entries().is_empty();
+                if node_has_child_nodes || node_has_entries {
+                    return Err(ConfigError::new_kdl_error(
+                        format!("The `children` node must be bare. All properties should be places on the node consuming this template."),
+                        child.span().offset(),
+                        child.span().len(),
+                    ));
+                }
                 external_children_index = Some(i);
             } else if let Some((pane_template, pane_template_kdl_node)) =
                 self.pane_templates.get(kdl_name!(child)).cloned()
@@ -496,7 +574,6 @@ impl<'a> KdlLayoutParser<'a> {
             kdl_get_bool_property_or_child_value_with_error!(kdl_node, "focus").is_some();
         let has_run_prop = self.parse_command_or_plugin_block(kdl_node)?.is_some();
         let has_nested_nodes_or_children_block = self.has_child_panes_tabs_or_templates(kdl_node);
-
         if has_nested_nodes_or_children_block
             && (has_borderless_prop || has_focus_prop || has_run_prop)
         {
@@ -549,10 +626,7 @@ impl<'a> KdlLayoutParser<'a> {
         let tab_name =
             kdl_get_string_property_or_child_value!(kdl_node, "name").map(|s| s.to_string());
         let is_focused = kdl_get_bool_property_or_child_value!(kdl_node, "focus").unwrap_or(false);
-        let children_split_direction = match kdl_get_string_entry!(kdl_node, "split_direction") {
+        let children_split_direction = self.parse_split_direction(kdl_node)?;
-            Some(direction) => SplitDirection::from_str(direction)?,
-            None => SplitDirection::default(),
-        };
         match kdl_children_nodes!(kdl_node) {
             Some(children) => {
                 let child_panes = self.parse_child_pane_nodes_for_tab(children)?;
@@ -618,11 +692,7 @@ impl<'a> KdlLayoutParser<'a> {
     }
     fn parse_tab_template_node(&self, kdl_node: &KdlNode) -> Result<PaneLayout, ConfigError> {
         self.assert_valid_tab_properties(kdl_node)?;
-        let children_split_direction =
+        let children_split_direction = self.parse_split_direction(kdl_node)?;
-            match kdl_get_string_property_or_child_value_with_error!(kdl_node, "split_direction") {
-                Some(direction) => SplitDirection::from_str(direction)?,
-                None => SplitDirection::default(),
-            };
         let mut tab_children = vec![];
         let mut external_children_index = None;
         if let Some(children) = kdl_children_nodes!(kdl_node) {
@@ -630,6 +700,16 @@ impl<'a> KdlLayoutParser<'a> {
                 if kdl_name!(child) == "pane" {
                     tab_children.push(self.parse_pane_node(child)?);
                 } else if kdl_name!(child) == "children" {
+                    let node_has_child_nodes =
+                        child.children().map(|c| !c.is_empty()).unwrap_or(false);
+                    let node_has_entries = !child.entries().is_empty();
+                    if node_has_child_nodes || node_has_entries {
+                        return Err(ConfigError::new_kdl_error(
+                            format!("The `children` node must be bare. All properties should be places on the node consuming this template."),
+                            child.span().offset(),
+                            child.span().len(),
+                        ));
+                    }
                     external_children_index = Some(i);
                 } else if let Some((pane_template, pane_template_kdl_node)) =
                     self.pane_templates.get(kdl_name!(child)).cloned()

zellij-utils/src/pane_size.rs

@@ -130,6 +130,9 @@ impl PaneGeom {
             && self.y <= row
             && row < self.y + self.rows.as_usize()
     }
+    pub fn is_at_least_minimum_size(&self) -> bool {
+        self.rows.as_usize() > 0 && self.cols.as_usize() > 0
+    }
 }
 
 impl Offset {