From ea2d9ced6207acd2f401f6d95ac66565cc06d4a9 Mon Sep 17 00:00:00 2001 From: Aram Drevekenin Date: Tue, 28 Feb 2023 22:08:01 +0100 Subject: [PATCH] feat(layouts): exact panes constraint (#2206) * style(fmt): remove warnings * fix(swap-layouts): introduce exact panes constraint * fix(swap-layouts): improve floating pane swap layout ux * style(fmt): rustfmt --- zellij-server/src/pty.rs | 2 +- zellij-server/src/tab/swap_layouts.rs | 14 +- ...t_the_focus_goes_to_last_focused_pane.snap | 42 +++--- .../src/tab/unit/tab_integration_tests.rs | 6 +- zellij-utils/assets/layouts/compact.swap.kdl | 24 +-- zellij-utils/assets/layouts/default.swap.kdl | 24 +-- zellij-utils/assets/layouts/strider.swap.kdl | 26 ++-- zellij-utils/src/input/layout.rs | 1 + zellij-utils/src/kdl/kdl_layout_parser.rs | 49 +++++- ...efault_config_with_no_cli_arguments-2.snap | 142 +++++++++++++++--- 10 files changed, 250 insertions(+), 80 deletions(-) diff --git a/zellij-server/src/pty.rs b/zellij-server/src/pty.rs index 83255e61c..5daccd17f 100644 --- a/zellij-server/src/pty.rs +++ b/zellij-server/src/pty.rs @@ -467,7 +467,7 @@ impl Pty { TerminalAction::RunCommand(ref mut command) => { command.cwd = Some(cwd); }, - TerminalAction::OpenFile(ref file, line_number, ref mut edit_cwd) => { + TerminalAction::OpenFile(ref _file, _line_number, ref mut edit_cwd) => { match edit_cwd.as_mut() { Some(edit_cwd) => { *edit_cwd = cwd.join(&edit_cwd); diff --git a/zellij-server/src/tab/swap_layouts.rs b/zellij-server/src/tab/swap_layouts.rs index 57085779f..942a65408 100644 --- a/zellij-server/src/tab/swap_layouts.rs +++ b/zellij-server/src/tab/swap_layouts.rs @@ -40,12 +40,14 @@ impl SwapLayouts { let mut base_swap_floating_layout = BTreeMap::new(); let tiled_panes_count = layout.0.pane_count(); let floating_panes_count = layout.1.len(); - // we set MaxPanes to the current panes in the layout, because the base layout is not + // we set ExactPanes to the current panes in the layout, because the base layout is not // intended to be progressive - i.e. to have additional panes added to it + // we also don't want it to be applied for less than the expected amount of panes, because + // then unintended things can happen // we still want to keep it around in case we'd like to swap layouts without adding panes - base_swap_tiled_layout.insert(LayoutConstraint::MaxPanes(tiled_panes_count), layout.0); + base_swap_tiled_layout.insert(LayoutConstraint::ExactPanes(tiled_panes_count), layout.0); base_swap_floating_layout - .insert(LayoutConstraint::MaxPanes(floating_panes_count), layout.1); + .insert(LayoutConstraint::ExactPanes(floating_panes_count), layout.1); self.swap_tiled_layouts .insert(0, (base_swap_tiled_layout, Some("BASE".into()))); self.swap_floating_layouts @@ -174,6 +176,9 @@ impl SwapLayouts { LayoutConstraint::MinPanes(min_panes) => { tiled_panes.visible_panes_count() >= *min_panes }, + LayoutConstraint::ExactPanes(pane_count) => { + tiled_panes.visible_panes_count() == *pane_count + }, LayoutConstraint::NoConstraint => true, } } @@ -189,6 +194,9 @@ impl SwapLayouts { LayoutConstraint::MinPanes(min_panes) => { floating_panes.visible_panes_count() >= *min_panes }, + LayoutConstraint::ExactPanes(pane_count) => { + floating_panes.visible_panes_count() == *pane_count + }, LayoutConstraint::NoConstraint => true, } } diff --git a/zellij-server/src/tab/unit/snapshots/zellij_server__tab__tab_integration_tests__when_closing_a_pane_in_auto_layout_the_focus_goes_to_last_focused_pane.snap b/zellij-server/src/tab/unit/snapshots/zellij_server__tab__tab_integration_tests__when_closing_a_pane_in_auto_layout_the_focus_goes_to_last_focused_pane.snap index 8f548b1dc..48fb7d0cd 100644 --- a/zellij-server/src/tab/unit/snapshots/zellij_server__tab__tab_integration_tests__when_closing_a_pane_in_auto_layout_the_focus_goes_to_last_focused_pane.snap +++ b/zellij-server/src/tab/unit/snapshots/zellij_server__tab__tab_integration_tests__when_closing_a_pane_in_auto_layout_the_focus_goes_to_last_focused_pane.snap @@ -1,26 +1,26 @@ --- source: zellij-server/src/tab/./unit/tab_integration_tests.rs -assertion_line: 5333 +assertion_line: 5700 expression: snapshot --- -00 (C): ┌ Pane #1 ──────────────────────────────────────────────────────────────────────────────────────────────────────────────┐ -01 (C): │ │ -02 (C): │ │ -03 (C): │ │ -04 (C): │ │ -05 (C): │ │ -06 (C): │ │ -07 (C): │ │ -08 (C): │ │ -09 (C): └───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘ -10 (C): ┌ Pane #2 ──────────────────────────────────────────────────────────────────────────────────────────────────────────────┐ -11 (C): │ │ -12 (C): │ │ -13 (C): │ │ -14 (C): │ │ -15 (C): │ │ -16 (C): │ │ -17 (C): │ │ -18 (C): │ │ -19 (C): └───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘ +00 (C): ┌ Pane #1 ──────────────────────────────────────────────────┐┌ Pane #2 ─────────────────────────────────────────────────┐ +01 (C): │ ││ │ +02 (C): │ ││ │ +03 (C): │ ││ │ +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): └───────────────────────────────────────────────────────────┘└──────────────────────────────────────────────────────────┘ diff --git a/zellij-server/src/tab/unit/tab_integration_tests.rs b/zellij-server/src/tab/unit/tab_integration_tests.rs index 45b3defdf..11de1c298 100644 --- a/zellij-server/src/tab/unit/tab_integration_tests.rs +++ b/zellij-server/src/tab/unit/tab_integration_tests.rs @@ -5681,8 +5681,8 @@ fn when_closing_a_pane_in_auto_layout_the_focus_goes_to_last_focused_pane() { )), true, ); - tab.move_focus_down(client_id); - tab.move_focus_down(client_id); + let _ = tab.move_focus_down(client_id); + let _ = tab.move_focus_down(client_id); tab.close_pane(PaneId::Terminal(3), false, Some(client_id)); tab.render(&mut output, None).unwrap(); @@ -5694,7 +5694,7 @@ fn when_closing_a_pane_in_auto_layout_the_focus_goes_to_last_focused_pane() { ); assert_eq!( cursor_coordinates, - Some((1, 11)), + Some((62, 1)), "cursor coordinates moved to the new pane", ); assert_snapshot!(snapshot); diff --git a/zellij-utils/assets/layouts/compact.swap.kdl b/zellij-utils/assets/layouts/compact.swap.kdl index da9fd593c..923a117c1 100644 --- a/zellij-utils/assets/layouts/compact.swap.kdl +++ b/zellij-utils/assets/layouts/compact.swap.kdl @@ -62,15 +62,15 @@ swap_floating_layout name="staggered" { swap_floating_layout name="enlarged" { floating_panes max_panes=10 { - pane { x 1; y 1; width "90%"; height "90%"; } - pane { x 2; y 2; width "90%"; height "90%"; } - pane { x 3; y 3; width "90%"; height "90%"; } - pane { x 4; y 4; width "90%"; height "90%"; } - pane { x 5; y 5; width "90%"; height "90%"; } - pane { x 6; y 6; width "90%"; height "90%"; } - pane { x 7; y 7; width "90%"; height "90%"; } - pane { x 8; y 8; width "90%"; height "90%"; } - pane { x 9; y 9; width "90%"; height "90%"; } + pane { x "5%"; y 1; width "90%"; height "90%"; } + pane { x "5%"; y 2; width "90%"; height "90%"; } + pane { x "5%"; y 3; width "90%"; height "90%"; } + pane { x "5%"; y 4; width "90%"; height "90%"; } + pane { x "5%"; y 5; width "90%"; height "90%"; } + pane { x "5%"; y 6; width "90%"; height "90%"; } + pane { x "5%"; y 7; width "90%"; height "90%"; } + pane { x "5%"; y 8; width "90%"; height "90%"; } + pane { x "5%"; y 9; width "90%"; height "90%"; } pane focus=true { x 10; y 10; width "90%"; height "90%"; } } } @@ -88,4 +88,10 @@ swap_floating_layout name="spread" { pane { x "1%"; y "1%"; width "45%"; } pane { x "50%"; y "1%"; width "45%"; } } + floating_panes max_panes=4 { + pane { x "1%"; y "55%"; width "45%"; height "45%"; } + pane focus=true { x "50%"; y "55%"; width "45%"; height "45%"; } + pane { x "1%"; y "1%"; width "45%"; height "45%"; } + pane { x "50%"; y "1%"; width "45%"; height "45%"; } + } } diff --git a/zellij-utils/assets/layouts/default.swap.kdl b/zellij-utils/assets/layouts/default.swap.kdl index 014ce43fb..6d5cdb552 100644 --- a/zellij-utils/assets/layouts/default.swap.kdl +++ b/zellij-utils/assets/layouts/default.swap.kdl @@ -65,15 +65,15 @@ swap_floating_layout name="staggered" { swap_floating_layout name="enlarged" { floating_panes max_panes=10 { - pane { x 1; y 1; width "90%"; height "90%"; } - pane { x 2; y 2; width "90%"; height "90%"; } - pane { x 3; y 3; width "90%"; height "90%"; } - pane { x 4; y 4; width "90%"; height "90%"; } - pane { x 5; y 5; width "90%"; height "90%"; } - pane { x 6; y 6; width "90%"; height "90%"; } - pane { x 7; y 7; width "90%"; height "90%"; } - pane { x 8; y 8; width "90%"; height "90%"; } - pane { x 9; y 9; width "90%"; height "90%"; } + pane { x "5%"; y 1; width "90%"; height "90%"; } + pane { x "5%"; y 2; width "90%"; height "90%"; } + pane { x "5%"; y 3; width "90%"; height "90%"; } + pane { x "5%"; y 4; width "90%"; height "90%"; } + pane { x "5%"; y 5; width "90%"; height "90%"; } + pane { x "5%"; y 6; width "90%"; height "90%"; } + pane { x "5%"; y 7; width "90%"; height "90%"; } + pane { x "5%"; y 8; width "90%"; height "90%"; } + pane { x "5%"; y 9; width "90%"; height "90%"; } pane focus=true { x 10; y 10; width "90%"; height "90%"; } } } @@ -91,4 +91,10 @@ swap_floating_layout name="spread" { pane { x "1%"; y "1%"; width "45%"; } pane { x "50%"; y "1%"; width "45%"; } } + floating_panes max_panes=4 { + pane { x "1%"; y "55%"; width "45%"; height "45%"; } + pane focus=true { x "50%"; y "55%"; width "45%"; height "45%"; } + pane { x "1%"; y "1%"; width "45%"; height "45%"; } + pane { x "50%"; y "1%"; width "45%"; height "45%"; } + } } diff --git a/zellij-utils/assets/layouts/strider.swap.kdl b/zellij-utils/assets/layouts/strider.swap.kdl index 95d219b23..579fcc8e9 100644 --- a/zellij-utils/assets/layouts/strider.swap.kdl +++ b/zellij-utils/assets/layouts/strider.swap.kdl @@ -61,7 +61,7 @@ swap_tiled_layout name="horizontal" { swap_tiled_layout name="stacked" { ui min_panes=6 { pane split_direction="vertical" { - pane focus=true + pane pane stacked=true { children; } } } @@ -73,15 +73,15 @@ swap_floating_layout name="staggered" { swap_floating_layout name="enlarged" { floating_panes max_panes=10 { - pane { x 1; y 1; width "90%"; height "90%"; } - pane { x 2; y 2; width "90%"; height "90%"; } - pane { x 3; y 3; width "90%"; height "90%"; } - pane { x 4; y 4; width "90%"; height "90%"; } - pane { x 5; y 5; width "90%"; height "90%"; } - pane { x 6; y 6; width "90%"; height "90%"; } - pane { x 7; y 7; width "90%"; height "90%"; } - pane { x 8; y 8; width "90%"; height "90%"; } - pane { x 9; y 9; width "90%"; height "90%"; } + pane { x "5%"; y 1; width "90%"; height "90%"; } + pane { x "5%"; y 2; width "90%"; height "90%"; } + pane { x "5%"; y 3; width "90%"; height "90%"; } + pane { x "5%"; y 4; width "90%"; height "90%"; } + pane { x "5%"; y 5; width "90%"; height "90%"; } + pane { x "5%"; y 6; width "90%"; height "90%"; } + pane { x "5%"; y 7; width "90%"; height "90%"; } + pane { x "5%"; y 8; width "90%"; height "90%"; } + pane { x "5%"; y 9; width "90%"; height "90%"; } pane focus=true { x 10; y 10; width "90%"; height "90%"; } } } @@ -99,4 +99,10 @@ swap_floating_layout name="spread" { pane { x "1%"; y "1%"; width "45%"; } pane { x "50%"; y "1%"; width "45%"; } } + floating_panes max_panes=4 { + pane { x "1%"; y "55%"; width "45%"; height "45%"; } + pane focus=true { x "50%"; y "55%"; width "45%"; height "45%"; } + pane { x "1%"; y "1%"; width "45%"; height "45%"; } + pane { x "50%"; y "1%"; width "45%"; height "45%"; } + } } diff --git a/zellij-utils/src/input/layout.rs b/zellij-utils/src/input/layout.rs index d6f5a2f7d..915c348a8 100644 --- a/zellij-utils/src/input/layout.rs +++ b/zellij-utils/src/input/layout.rs @@ -252,6 +252,7 @@ impl fmt::Display for RunPluginLocation { pub enum LayoutConstraint { MaxPanes(usize), MinPanes(usize), + ExactPanes(usize), NoConstraint, } diff --git a/zellij-utils/src/kdl/kdl_layout_parser.rs b/zellij-utils/src/kdl/kdl_layout_parser.rs index cf6afaf70..ee6c00399 100644 --- a/zellij-utils/src/kdl/kdl_layout_parser.rs +++ b/zellij-utils/src/kdl/kdl_layout_parser.rs @@ -119,6 +119,7 @@ impl<'a> KdlLayoutParser<'a> { || property_name == "children" || property_name == "max_panes" || property_name == "min_panes" + || property_name == "exact_panes" } fn assert_legal_node_name(&self, name: &str, kdl_node: &KdlNode) -> Result<(), ConfigError> { if name.contains(char::is_whitespace) { @@ -1747,6 +1748,12 @@ impl<'a> KdlLayoutParser<'a> { tab_template_kdl_node, )?; swap_tiled_layout.insert(layout_constraint, layout); + } else { + return Err(ConfigError::new_layout_kdl_error( + format!("Unknown layout node: '{}'", layout_node_name), + layout.span().offset(), + layout.span().len(), + )); } } swap_tiled_layouts.push((swap_tiled_layout, swap_layout_name)); @@ -1787,6 +1794,12 @@ impl<'a> KdlLayoutParser<'a> { tab_template_kdl_node, )?; swap_floating_layout.insert(layout_constraint, layout); + } else { + return Err(ConfigError::new_layout_kdl_error( + format!("Unknown layout node: '{}'", layout_node_name), + layout.span().offset(), + layout.span().len(), + )); } } swap_floating_layouts.push((swap_floating_layout, swap_layout_name)); @@ -1814,17 +1827,41 @@ impl<'a> KdlLayoutParser<'a> { layout_node )); }; + if let Some(exact_panes) = + kdl_get_string_property_or_child_value!(layout_node, "exact_panes") + { + return Err(kdl_parsing_error!( + format!( + "exact_panes should be a fixed number (eg. 1) and not a quoted string (\"{}\")", + exact_panes, + ), + layout_node + )); + }; let max_panes = kdl_get_int_property_or_child_value!(layout_node, "max_panes"); let min_panes = kdl_get_int_property_or_child_value!(layout_node, "min_panes"); - match (min_panes, max_panes) { - (Some(_min_panes), Some(_max_panes)) => Err(kdl_parsing_error!( + let exact_panes = kdl_get_int_property_or_child_value!(layout_node, "exact_panes"); + let mut constraint_count = 0; + let mut constraint = None; + if let Some(max_panes) = max_panes { + constraint_count += 1; + constraint = Some(LayoutConstraint::MaxPanes(max_panes as usize)); + } + if let Some(min_panes) = min_panes { + constraint_count += 1; + constraint = Some(LayoutConstraint::MinPanes(min_panes as usize)); + } + if let Some(exact_panes) = exact_panes { + constraint_count += 1; + constraint = Some(LayoutConstraint::ExactPanes(exact_panes as usize)); + } + if constraint_count > 1 { + return Err(kdl_parsing_error!( format!("cannot have more than one constraint (eg. max_panes + min_panes)'"), layout_node - )), - (Some(min_panes), None) => Ok(LayoutConstraint::MinPanes(min_panes as usize)), - (None, Some(max_panes)) => Ok(LayoutConstraint::MaxPanes(max_panes as usize)), - _ => Ok(LayoutConstraint::NoConstraint), + )); } + Ok(constraint.unwrap_or(LayoutConstraint::NoConstraint)) } fn populate_one_swap_tiled_layout( &self, diff --git a/zellij-utils/src/snapshots/zellij_utils__setup__setup_test__default_config_with_no_cli_arguments-2.snap b/zellij-utils/src/snapshots/zellij_utils__setup__setup_test__default_config_with_no_cli_arguments-2.snap index b87b9344c..f53ecc5ef 100644 --- a/zellij-utils/src/snapshots/zellij_utils__setup__setup_test__default_config_with_no_cli_arguments-2.snap +++ b/zellij-utils/src/snapshots/zellij_utils__setup__setup_test__default_config_with_no_cli_arguments-2.snap @@ -1,6 +1,6 @@ --- source: zellij-utils/src/setup.rs -assertion_line: 601 +assertion_line: 622 expression: "format!(\"{:#?}\", layout)" --- Layout { @@ -1213,8 +1213,8 @@ Layout { ), ), x: Some( - Fixed( - 1, + Percent( + 5, ), ), y: Some( @@ -1238,8 +1238,8 @@ Layout { ), ), x: Some( - Fixed( - 2, + Percent( + 5, ), ), y: Some( @@ -1263,8 +1263,8 @@ Layout { ), ), x: Some( - Fixed( - 3, + Percent( + 5, ), ), y: Some( @@ -1288,8 +1288,8 @@ Layout { ), ), x: Some( - Fixed( - 4, + Percent( + 5, ), ), y: Some( @@ -1313,7 +1313,7 @@ Layout { ), ), x: Some( - Fixed( + Percent( 5, ), ), @@ -1338,8 +1338,8 @@ Layout { ), ), x: Some( - Fixed( - 6, + Percent( + 5, ), ), y: Some( @@ -1363,8 +1363,8 @@ Layout { ), ), x: Some( - Fixed( - 7, + Percent( + 5, ), ), y: Some( @@ -1388,8 +1388,8 @@ Layout { ), ), x: Some( - Fixed( - 8, + Percent( + 5, ), ), y: Some( @@ -1413,8 +1413,8 @@ Layout { ), ), x: Some( - Fixed( - 9, + Percent( + 5, ), ), y: Some( @@ -1596,6 +1596,112 @@ Layout { focus: None, }, ], + MaxPanes( + 4, + ): [ + FloatingPaneLayout { + name: None, + height: Some( + Percent( + 45, + ), + ), + width: Some( + Percent( + 45, + ), + ), + x: Some( + Percent( + 1, + ), + ), + y: Some( + Percent( + 55, + ), + ), + run: None, + focus: None, + }, + FloatingPaneLayout { + name: None, + height: Some( + Percent( + 45, + ), + ), + width: Some( + Percent( + 45, + ), + ), + x: Some( + Percent( + 50, + ), + ), + y: Some( + Percent( + 55, + ), + ), + run: None, + focus: Some( + true, + ), + }, + FloatingPaneLayout { + name: None, + height: Some( + Percent( + 45, + ), + ), + width: Some( + Percent( + 45, + ), + ), + x: Some( + Percent( + 1, + ), + ), + y: Some( + Percent( + 1, + ), + ), + run: None, + focus: None, + }, + FloatingPaneLayout { + name: None, + height: Some( + Percent( + 45, + ), + ), + width: Some( + Percent( + 45, + ), + ), + x: Some( + Percent( + 50, + ), + ), + y: Some( + Percent( + 1, + ), + ), + run: None, + focus: None, + }, + ], }, Some( "spread",