diff options
author | Aram Drevekenin <aram@poor.dev> | 2022-10-13 13:55:16 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-10-13 13:55:16 +0200 |
commit | bece86b95c63d645368d7c82e56b5ce553f1ea1e (patch) | |
tree | 2c50fbf31f2837228f7a71dc02aec19a64253988 /zellij-utils | |
parent | 8d56def4fc3441b7231eda8c2ca944445af43920 (diff) |
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
Diffstat (limited to 'zellij-utils')
9 files changed, 564 insertions, 78 deletions
diff --git a/zellij-utils/src/input/layout.rs b/zellij-utils/src/input/layout.rs index eea80859f..f823ed401 100644 --- a/zellij-utils/src/input/layout.rs +++ b/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)> { - split_space(space, self, space) + pub fn position_panes_in_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![]; diff --git a/zellij-utils/src/input/unit/layout_test.rs b/zellij-utils/src/input/unit/layout_test.rs index ffd35f207..ce077a42e 100644 --- a/zellij-utils/src/input/unit/layout_test.rs +++ b/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"); +} diff --git a/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__args_added_to_args_in_template.snap b/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__args_added_to_args_in_template.snap new file mode 100644 index 000000000..6ff62582d --- /dev/null +++ b/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, + }, + ), +} diff --git a/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__args_override_args_in_template.snap b/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__args_override_args_in_template.snap new file mode 100644 index 000000000..4a449d35a --- /dev/null +++ b/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, + }, + ), +} diff --git a/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__children_not_as_first_child_of_pane_template.snap b/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__children_not_as_first_child_of_pane_template.snap index f82879d2b..f7f904f62 100644 --- a/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__children_not_as_first_child_of_pane_template.snap +++ b/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, diff --git a/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__cwd_added_to_cwd_in_template.snap b/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__cwd_added_to_cwd_in_template.snap new file mode 100644 index 000000000..3c26fbdd6 --- /dev/null +++ b/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, + }, + ), +} diff --git a/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__cwd_override_cwd_in_template.snap b/zellij-utils/src/input/unit/snapshots/zellij_utils__input__layout__layout_test__cwd_override_cwd_in_template.snap new file mode 100644 index 000000000..fd1bc15a1 --- /dev/null +++ b/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, + }, + ), +} diff --git a/zellij-utils/src/kdl/kdl_layout_parser.rs b/zellij-utils/src/kdl/kdl_layout_parser.rs index 7261fecbe..6bf2d3724 100644 --- a/zellij-utils/src/kdl/kdl_layout_parser.rs +++ b/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> { - 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 = match kdl_get_child!(pane_node, "args") { + fn parse_args(&self, pane_node: &KdlNode) -> Result<Option<Vec<String>>, ConfigError> { + 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, - }; - match (command, cwd, args) { - (None, Some(_cwd), _) => Err(ConfigError::new_kdl_error( + None => Ok(None), + } + } + 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() { |