diff options
author | Aram Drevekenin <aram@poor.dev> | 2023-02-25 11:16:11 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-02-25 11:16:11 +0100 |
commit | 5bcc1bb382659da60f29e4fc79336d3f6a727b57 (patch) | |
tree | e54fed009064e6d10d630c2c80070873f6a1fce6 /zellij-utils | |
parent | d65e8220b67ed7324bc4d79f265f6519c516e198 (diff) |
fix(layout): various parser and ui fixes (#2191)
* fix(layout): error on nodes outside layout node
* fix(layout): move stacked property to pane
* fix(layout): various stack exceptions
* fix(ui): non-flexible stacked pane titles now take up their full length
* fix(ui): stack titles with no-pane-frames take up their proper length
* style(fmt): rustfmt
Diffstat (limited to 'zellij-utils')
7 files changed, 354 insertions, 58 deletions
diff --git a/zellij-utils/assets/layouts/compact.swap.kdl b/zellij-utils/assets/layouts/compact.swap.kdl index e38fa2101..da9fd593c 100644 --- a/zellij-utils/assets/layouts/compact.swap.kdl +++ b/zellij-utils/assets/layouts/compact.swap.kdl @@ -51,7 +51,7 @@ swap_tiled_layout name="stacked" { ui min_panes=4 { pane split_direction="vertical" { pane - pane { children stacked=true; } + pane stacked=true { children; } } } } diff --git a/zellij-utils/assets/layouts/default.swap.kdl b/zellij-utils/assets/layouts/default.swap.kdl index 51e8afed0..014ce43fb 100644 --- a/zellij-utils/assets/layouts/default.swap.kdl +++ b/zellij-utils/assets/layouts/default.swap.kdl @@ -54,7 +54,7 @@ swap_tiled_layout name="stacked" { ui min_panes=5 { pane split_direction="vertical" { pane - pane { children stacked=true; } + pane stacked=true { children; } } } } diff --git a/zellij-utils/assets/layouts/strider.swap.kdl b/zellij-utils/assets/layouts/strider.swap.kdl index c6df15755..95d219b23 100644 --- a/zellij-utils/assets/layouts/strider.swap.kdl +++ b/zellij-utils/assets/layouts/strider.swap.kdl @@ -62,7 +62,7 @@ swap_tiled_layout name="stacked" { ui min_panes=6 { pane split_direction="vertical" { pane focus=true - pane { children stacked=true; } + pane stacked=true { children; } } } } diff --git a/zellij-utils/src/input/unit/layout_test.rs b/zellij-utils/src/input/unit/layout_test.rs index f15fd709d..670273b0f 100644 --- a/zellij-utils/src/input/unit/layout_test.rs +++ b/zellij-utils/src/input/unit/layout_test.rs @@ -1849,3 +1849,96 @@ fn can_load_swap_layouts_from_a_different_file() { .unwrap(); assert_snapshot!(format!("{:#?}", layout)); } + +#[test] +fn can_define_stacked_children_for_pane_node() { + let kdl_layout = r#" + layout { + pane stacked=true { + pane + pane + } + } + "#; + let layout = Layout::from_kdl(kdl_layout, "layout_file_name".into(), None, None).unwrap(); + assert_snapshot!(format!("{:#?}", layout)); +} + +#[test] +fn can_define_stacked_children_for_pane_template() { + let kdl_layout = r#" + layout { + pane_template name="stack" stacked=true { + children + } + stack { + pane + pane + } + } + "#; + let layout = Layout::from_kdl(kdl_layout, "layout_file_name".into(), None, None).unwrap(); + assert_snapshot!(format!("{:#?}", layout)); +} + +#[test] +fn cannot_define_stacked_panes_for_bare_node() { + let kdl_layout = r#" + layout { + pane stacked=true + } + "#; + let layout = Layout::from_kdl(kdl_layout, "layout_file_name".into(), None, None); + assert!(layout.is_err(), "error provided for tab name with space"); +} + +#[test] +fn cannot_define_stacked_panes_with_vertical_split_direction() { + let kdl_layout = r#" + layout { + pane stacked=true split_direction="vertical" { + pane + pane + } + } + "#; + let layout = Layout::from_kdl(kdl_layout, "layout_file_name".into(), None, None); + assert!(layout.is_err(), "error provided for tab name with space"); +} + +#[test] +fn cannot_define_stacked_panes_with_grandchildren() { + let kdl_layout = r#" + layout { + pane stacked=true { + pane { + pane + pane + } + pane + } + } + "#; + let layout = Layout::from_kdl(kdl_layout, "layout_file_name".into(), None, None); + assert!(layout.is_err(), "error provided for tab name with space"); +} + +#[test] +fn cannot_define_stacked_panes_with_grandchildren_in_pane_template() { + let kdl_layout = r#" + layout { + pane_template name="stack" stacked=true { + children + } + stack { + pane + pane { + pane + pane + } + } + } + "#; + let layout = Layout::from_kdl(kdl_layout, "layout_file_name".into(), None, None); + assert!(layout.is_err(), "error provided for tab name with space"); +} diff --git a/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__can_define_stacked_children_for_pane_node.snap b/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__can_define_stacked_children_for_pane_node.snap new file mode 100644 index 000000000..74a872bcf --- /dev/null +++ b/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__can_define_stacked_children_for_pane_node.snap @@ -0,0 +1,63 @@ +--- +source: zellij-utils/src/input/./unit/layout_test.rs +assertion_line: 1870 +expression: "format!(\"{:#?}\", layout)" +--- +Layout { + tabs: [], + focused_tab_index: None, + template: Some( + ( + TiledPaneLayout { + children_split_direction: Horizontal, + name: None, + children: [ + TiledPaneLayout { + children_split_direction: Horizontal, + name: None, + children: [ + TiledPaneLayout { + children_split_direction: Horizontal, + name: None, + children: [], + split_size: None, + run: None, + borderless: false, + focus: None, + external_children_index: None, + children_are_stacked: false, + }, + TiledPaneLayout { + children_split_direction: Horizontal, + name: None, + children: [], + split_size: None, + run: None, + borderless: false, + focus: None, + external_children_index: None, + children_are_stacked: false, + }, + ], + split_size: None, + run: None, + borderless: false, + focus: None, + external_children_index: None, + children_are_stacked: true, + }, + ], + split_size: None, + run: None, + borderless: false, + focus: None, + external_children_index: None, + children_are_stacked: false, + }, + [], + ), + ), + swap_layouts: [], + swap_tiled_layouts: [], + swap_floating_layouts: [], +} diff --git a/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__can_define_stacked_children_for_pane_template.snap b/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__can_define_stacked_children_for_pane_template.snap new file mode 100644 index 000000000..9653eda53 --- /dev/null +++ b/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__can_define_stacked_children_for_pane_template.snap @@ -0,0 +1,75 @@ +--- +source: zellij-utils/src/input/./unit/layout_test.rs +assertion_line: 1893 +expression: "format!(\"{:#?}\", layout)" +--- +Layout { + tabs: [], + focused_tab_index: None, + template: Some( + ( + TiledPaneLayout { + children_split_direction: Horizontal, + name: None, + children: [ + TiledPaneLayout { + children_split_direction: Horizontal, + name: None, + children: [ + TiledPaneLayout { + children_split_direction: Horizontal, + name: None, + children: [ + TiledPaneLayout { + children_split_direction: Horizontal, + name: None, + children: [], + split_size: None, + run: None, + borderless: false, + focus: None, + external_children_index: None, + children_are_stacked: false, + }, + TiledPaneLayout { + children_split_direction: Horizontal, + name: None, + children: [], + split_size: None, + run: None, + borderless: false, + focus: None, + external_children_index: None, + children_are_stacked: false, + }, + ], + split_size: None, + run: None, + borderless: false, + focus: None, + external_children_index: None, + children_are_stacked: true, + }, + ], + split_size: None, + run: None, + borderless: false, + focus: None, + external_children_index: None, + children_are_stacked: true, + }, + ], + split_size: None, + run: None, + borderless: false, + focus: None, + external_children_index: None, + children_are_stacked: false, + }, + [], + ), + ), + swap_layouts: [], + swap_tiled_layouts: [], + swap_floating_layouts: [], +} diff --git a/zellij-utils/src/kdl/kdl_layout_parser.rs b/zellij-utils/src/kdl/kdl_layout_parser.rs index 7125d1fe8..5d89fab1b 100644 --- a/zellij-utils/src/kdl/kdl_layout_parser.rs +++ b/zellij-utils/src/kdl/kdl_layout_parser.rs @@ -53,6 +53,8 @@ impl<'a> KdlLayoutParser<'a> { } } fn is_a_reserved_word(&self, word: &str) -> bool { + // note that it's important that none of these words happens to also be a config property, + // otherwise they might collide word == "pane" || word == "layout" || word == "pane_template" @@ -90,6 +92,7 @@ impl<'a> KdlLayoutParser<'a> { || property_name == "split_direction" || property_name == "pane" || property_name == "children" + || property_name == "stacked" } fn is_a_valid_floating_pane_property(&self, property_name: &str) -> bool { property_name == "borderless" @@ -166,6 +169,25 @@ impl<'a> KdlLayoutParser<'a> { Ok(()) } } + fn assert_no_grandchildren_in_stack( + &self, + children: &[KdlNode], + is_part_of_stack: bool, + ) -> Result<(), ConfigError> { + if is_part_of_stack { + for child in children { + if kdl_name!(child) == "pane" || self.pane_templates.get(kdl_name!(child)).is_some() + { + return Err(ConfigError::new_layout_kdl_error( + format!("Stacked panes cannot have children"), + child.span().offset(), + child.span().len(), + )); + } + } + } + Ok(()) + } 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") { match SplitSize::from_str(size) { @@ -408,8 +430,14 @@ impl<'a> KdlLayoutParser<'a> { } Ok(run) } - fn parse_pane_node(&self, kdl_node: &KdlNode) -> Result<TiledPaneLayout, ConfigError> { + fn parse_pane_node( + &self, + kdl_node: &KdlNode, + is_part_of_stack: bool, + ) -> Result<TiledPaneLayout, ConfigError> { self.assert_valid_pane_properties(kdl_node)?; + let children_are_stacked = + kdl_get_bool_property_or_child_value_with_error!(kdl_node, "stacked").unwrap_or(false); let borderless = kdl_get_bool_property_or_child_value_with_error!(kdl_node, "borderless"); 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") @@ -417,11 +445,26 @@ impl<'a> KdlLayoutParser<'a> { let split_size = self.parse_split_size(kdl_node)?; let run = self.parse_command_plugin_or_edit_block(kdl_node)?; let children_split_direction = self.parse_split_direction(kdl_node)?; - let (external_children_index, children_are_stacked, children) = - match kdl_children_nodes!(kdl_node) { - Some(children) => self.parse_child_pane_nodes_for_pane(&children)?, - None => (None, false, vec![]), - }; + let (external_children_index, children) = match kdl_children_nodes!(kdl_node) { + Some(children) => { + self.assert_no_grandchildren_in_stack(&children, is_part_of_stack)?; + self.parse_child_pane_nodes_for_pane(&children, children_are_stacked)? + }, + None => (None, vec![]), + }; + if children_are_stacked && external_children_index.is_none() && children.is_empty() { + return Err(ConfigError::new_layout_kdl_error( + format!("A stacked pane must have children nodes or possibly a \"children\" node if in a swap_layout"), + kdl_node.span().offset(), + kdl_node.span().len(), + )); + } else if children_are_stacked && children_split_direction == SplitDirection::Vertical { + return Err(ConfigError::new_layout_kdl_error( + format!("Stacked panes cannot be vertical"), + kdl_node.span().offset(), + kdl_node.span().len(), + )); + } self.assert_no_mixed_children_and_properties(kdl_node)?; Ok(TiledPaneLayout { borderless: borderless.unwrap_or_default(), @@ -467,12 +510,16 @@ impl<'a> KdlLayoutParser<'a> { pane_template: &mut TiledPaneLayout, pane_template_kdl_node: &KdlNode, ) -> Result<(), ConfigError> { + let children_are_stacked = + kdl_get_bool_property_or_child_value_with_error!(kdl_node, "stacked") + .unwrap_or(pane_template.children_are_stacked); let children_split_direction = self.parse_split_direction(kdl_node)?; - let (external_children_index, children_are_stacked, pane_parts) = - match kdl_children_nodes!(kdl_node) { - Some(children) => self.parse_child_pane_nodes_for_pane(&children)?, - None => (None, false, vec![]), - }; + let (external_children_index, pane_parts) = match kdl_children_nodes!(kdl_node) { + Some(children) => { + self.parse_child_pane_nodes_for_pane(&children, children_are_stacked)? + }, + None => (None, vec![]), + }; if pane_parts.len() > 0 { let child_panes_layout = TiledPaneLayout { children_split_direction, @@ -493,20 +540,14 @@ impl<'a> KdlLayoutParser<'a> { fn populate_external_children_index( &self, kdl_node: &KdlNode, - ) -> Result<Option<(usize, bool)>, ConfigError> { - // Option<(external_children_index, is_stacked)> + ) -> Result<Option<usize>, ConfigError> { + // Option<external_children_index> if let Some(pane_child_nodes) = kdl_children_nodes!(kdl_node) { for (i, child) in pane_child_nodes.iter().enumerate() { if kdl_name!(child) == "children" { - let stacked = - kdl_get_bool_property_or_child_value_with_error!(kdl_node, "stacked") - .unwrap_or(false); if let Some(grand_children) = kdl_children_nodes!(child) { - let grand_children: Vec<&str> = grand_children - .iter() - .map(|g| kdl_name!(g)) - .filter(|g| g != &"stacked") - .collect(); + let grand_children: Vec<&str> = + grand_children.iter().map(|g| kdl_name!(g)).collect(); if !grand_children.is_empty() { return Err(ConfigError::new_layout_kdl_error( format!( @@ -518,7 +559,7 @@ impl<'a> KdlLayoutParser<'a> { )); } } - return Ok(Some((i, stacked))); + return Ok(Some(i)); } } } @@ -539,6 +580,8 @@ 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 children_are_stacked = + kdl_get_bool_property_or_child_value_with_error!(kdl_node, "stacked"); let args = self.parse_args(kdl_node)?; let close_on_exit = kdl_get_bool_property_or_child_value_with_error!(kdl_node, "close_on_exit"); @@ -547,10 +590,7 @@ impl<'a> KdlLayoutParser<'a> { let split_size = self.parse_split_size(kdl_node)?; let run = self.parse_command_plugin_or_edit_block_for_template(kdl_node)?; - // TODO: change should_insert_children to should_keep_pane_external_children_index - // or smth - let external_children_index_and_is_stacked = if should_mark_external_children_index - { + let external_children_index = if should_mark_external_children_index { self.populate_external_children_index(kdl_node)? } else { None @@ -589,15 +629,18 @@ impl<'a> KdlLayoutParser<'a> { pane_template.split_size = Some(split_size); } if let Some(index_of_children) = pane_template.external_children_index { - pane_template - .children - .insert(index_of_children, TiledPaneLayout::default()); + pane_template.children.insert( + index_of_children, + TiledPaneLayout { + children_are_stacked: children_are_stacked.unwrap_or_default(), + ..Default::default() + }, + ); } - pane_template.external_children_index = - external_children_index_and_is_stacked.map(|(index, _is_stacked)| index); - pane_template.children_are_stacked = external_children_index_and_is_stacked - .map(|(_index, is_stacked)| is_stacked) - .unwrap_or(false); + if let Some(children_are_stacked) = children_are_stacked { + pane_template.children_are_stacked = children_are_stacked; + } + pane_template.external_children_index = external_children_index; Ok(pane_template) }, PaneOrFloatingPane::FloatingPane(_) => { @@ -758,6 +801,8 @@ impl<'a> KdlLayoutParser<'a> { ) -> Result<bool, ConfigError> { // pane properties let borderless = kdl_get_bool_property_or_child_value_with_error!(kdl_node, "borderless"); + let children_are_stacked = + kdl_get_bool_property_or_child_value_with_error!(kdl_node, "stacked"); let split_size = self.parse_split_size(kdl_node)?; let split_direction = kdl_get_string_property_or_child_value_with_error!(kdl_node, "split_direction"); @@ -772,6 +817,7 @@ impl<'a> KdlLayoutParser<'a> { let has_pane_properties = borderless.is_some() || split_size.is_some() || split_direction.is_some() + || children_are_stacked.is_some() || has_children_nodes; let has_floating_pane_properties = height.is_some() || width.is_some() || x.is_some() || y.is_some(); @@ -789,6 +835,8 @@ impl<'a> KdlLayoutParser<'a> { // pane properties let borderless = kdl_get_bool_property_or_child_value_with_error!(kdl_node, "borderless"); + let children_are_stacked = + kdl_get_bool_property_or_child_value_with_error!(kdl_node, "stacked"); let split_size = self.parse_split_size(kdl_node)?; let split_direction = kdl_get_string_property_or_child_value_with_error!(kdl_node, "split_direction"); @@ -803,6 +851,7 @@ impl<'a> KdlLayoutParser<'a> { let has_pane_properties = borderless.is_some() || split_size.is_some() || split_direction.is_some() + || children_are_stacked.is_some() || has_children_nodes; let has_floating_pane_properties = height.is_some() || width.is_some() || x.is_some() || y.is_some(); @@ -812,6 +861,9 @@ impl<'a> KdlLayoutParser<'a> { if borderless.is_some() { pane_properties.push("borderless"); } + if children_are_stacked.is_some() { + pane_properties.push("stacked"); + } if split_size.is_some() { pane_properties.push("split_size"); } @@ -905,13 +957,18 @@ impl<'a> KdlLayoutParser<'a> { // pane properties let borderless = kdl_get_bool_property_or_child_value_with_error!(kdl_node, "borderless"); + let children_are_stacked = + kdl_get_bool_property_or_child_value_with_error!(kdl_node, "stacked") + .unwrap_or(false); let split_size = self.parse_split_size(kdl_node)?; let children_split_direction = self.parse_split_direction(kdl_node)?; - let (external_children_index, children_are_stacked, pane_parts) = - match kdl_children_nodes!(kdl_node) { - Some(children) => self.parse_child_pane_nodes_for_pane(&children)?, - None => (None, false, vec![]), - }; + let is_part_of_stack = false; + let (external_children_index, pane_parts) = match kdl_children_nodes!(kdl_node) { + Some(children) => { + self.parse_child_pane_nodes_for_pane(&children, children_are_stacked)? + }, + None => (None, vec![]), + }; self.assert_no_mixed_children_and_properties(kdl_node)?; self.pane_templates.insert( template_name, @@ -983,9 +1040,10 @@ impl<'a> KdlLayoutParser<'a> { child_floating_panes: &mut Vec<FloatingPaneLayout>, ) -> Result<Vec<TiledPaneLayout>, ConfigError> { let mut nodes = vec![]; + let is_part_of_stack = false; for child in children { if kdl_name!(child) == "pane" { - nodes.push(self.parse_pane_node(child)?); + nodes.push(self.parse_pane_node(child, is_part_of_stack)?); } else if let Some((pane_template, pane_template_kdl_node)) = self.pane_templates.get(kdl_name!(child)).cloned() { @@ -1019,23 +1077,18 @@ impl<'a> KdlLayoutParser<'a> { fn parse_child_pane_nodes_for_pane( &self, children: &[KdlNode], - ) -> Result<(Option<usize>, bool, Vec<TiledPaneLayout>), ConfigError> { - // usize is external_children_index, bool is "children_are_stacked" + is_part_of_stack: bool, + ) -> Result<(Option<usize>, Vec<TiledPaneLayout>), ConfigError> { + // usize is external_children_index let mut external_children_index = None; - let mut children_are_stacked = false; let mut nodes = vec![]; for (i, child) in children.iter().enumerate() { if kdl_name!(child) == "pane" { - nodes.push(self.parse_pane_node(child)?); + nodes.push(self.parse_pane_node(child, is_part_of_stack)?); } else if kdl_name!(child) == "children" { - let stacked = kdl_get_bool_property_or_child_value_with_error!(child, "stacked") - .unwrap_or(false); if let Some(grand_children) = kdl_children_nodes!(child) { |