From 8a603feef3c70f63ad1b7658b1776536df948a48 Mon Sep 17 00:00:00 2001 From: Aram Drevekenin Date: Wed, 1 Dec 2021 10:01:17 +0100 Subject: [PATCH 1/2] fix(compatibility): properly handle scrolling when scroll region is full screen --- zellij-server/src/panes/grid.rs | 35 ++++++++--- zellij-server/src/panes/unit/grid_tests.rs | 20 +++++++ ...ll_screen_scroll_region_and_scroll_up.snap | 60 +++++++++++++++++++ 3 files changed, 108 insertions(+), 7 deletions(-) create mode 100644 zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__full_screen_scroll_region_and_scroll_up.snap diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 706e0e5499..5330d57b8a 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -374,6 +374,10 @@ pub struct Grid { alternative_lines_above_viewport_and_cursor: Option<(VecDeque, Vec, Cursor)>, cursor: Cursor, saved_cursor_position: Option, + // FIXME: change scroll_region to be (usize, usize) - where the top line is always the first + // line of the viewport and the bottom line the last unless it's changed with CSI r and friends + // Having it as an Option causes lots of complexity and issues, and according to DECSTBM, this + // should be the behaviour anyway scroll_region: Option<(usize, usize)>, active_charset: CharsetIndex, preceding_char: Option, @@ -936,15 +940,31 @@ impl Grid { // the state is corrupted return; } - self.viewport.remove(scroll_region_top); - let columns = VecDeque::from(vec![EMPTY_TERMINAL_CHARACTER; self.width]); - if self.viewport.len() >= scroll_region_bottom { - self.viewport - .insert(scroll_region_bottom, Row::from_columns(columns).canonical()); - } else { + if scroll_region_bottom == self.height -1 && scroll_region_top == 0 { + let row_count_to_transfer = 1; + let transferred_rows_count = transfer_rows_from_viewport_to_lines_above( + &mut self.viewport, + &mut self.lines_above, + row_count_to_transfer, + self.width, + ); + self.scrollback_buffer_lines = + subtract_isize_from_usize(self.scrollback_buffer_lines, transferred_rows_count); + let columns = VecDeque::from(vec![EMPTY_TERMINAL_CHARACTER; self.width]); self.viewport.push(Row::from_columns(columns).canonical()); + self.selection.move_up(1); + self.output_buffer.update_all_lines(); + } else { + self.viewport.remove(scroll_region_top); + let columns = VecDeque::from(vec![EMPTY_TERMINAL_CHARACTER; self.width]); + if self.viewport.len() >= scroll_region_bottom { + self.viewport + .insert(scroll_region_bottom, Row::from_columns(columns).canonical()); + } else { + self.viewport.push(Row::from_columns(columns).canonical()); + } + self.output_buffer.update_all_lines(); // TODO: only update scroll region lines } - self.output_buffer.update_all_lines(); // TODO: only update scroll region lines return; } } @@ -1226,6 +1246,7 @@ impl Grid { pub fn set_scroll_region(&mut self, top_line_index: usize, bottom_line_index: Option) { let bottom_line_index = bottom_line_index.unwrap_or(self.height); self.scroll_region = Some((top_line_index, bottom_line_index)); + self.move_cursor_to(0, 0, EMPTY_TERMINAL_CHARACTER); // DECSTBM moves the cursor to column 1 line 1 of the page } pub fn clear_scroll_region(&mut self) { self.scroll_region = None; diff --git a/zellij-server/src/panes/unit/grid_tests.rs b/zellij-server/src/panes/unit/grid_tests.rs index 57ebf405e6..d210060499 100644 --- a/zellij-server/src/panes/unit/grid_tests.rs +++ b/zellij-server/src/panes/unit/grid_tests.rs @@ -1022,3 +1022,23 @@ pub fn insert_wide_characters_in_existing_line() { } assert_snapshot!(format!("{:?}", grid)); } + +#[test] +pub fn full_screen_scroll_region_and_scroll_up() { + // this test is a regression test for a bug + // where the scroll region would be set to the + // full viewport and then scrolling up would cause + // lines to get deleted from the viewport rather + // than moving to "lines_above" + let mut vte_parser = vte::Parser::new(); + let mut grid = Grid::new(54, 80, Palette::default()); + let fixture_name = "scroll_region_full_screen"; + let content = read_fixture(fixture_name); + for byte in content { + vte_parser.advance(&mut grid, byte); + } + grid.scroll_up_one_line(); + grid.scroll_up_one_line(); + grid.scroll_up_one_line(); + assert_snapshot!(format!("{:?}", grid)); +} diff --git a/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__full_screen_scroll_region_and_scroll_up.snap b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__full_screen_scroll_region_and_scroll_up.snap new file mode 100644 index 0000000000..2c15090923 --- /dev/null +++ b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__full_screen_scroll_region_and_scroll_up.snap @@ -0,0 +1,60 @@ +--- +source: zellij-server/src/panes/./unit/grid_tests.rs +expression: "format!(\"{:?}\", grid)" + +--- +00 (C): line5 +01 (C): line6 +02 (C): line7 +03 (C): line8 +04 (C): line9 +05 (C): line10 +06 (C): line11 +07 (C): line12 +08 (C): line13 +09 (C): line14 +10 (C): line15 +11 (C): line16 +12 (C): line17 +13 (C): line18 +14 (C): line19 +15 (C): line20 +16 (C): line21 +17 (C): line22 +18 (C): line23 +19 (C): line24 +20 (C): line25 +21 (C): line26 +22 (C): line27 +23 (C): line28 +24 (C): line29 +25 (C): line30 +26 (C): line31 +27 (C): line32 +28 (C): line33 +29 (C): line34 +30 (C): line35 +31 (C): line36 +32 (C): line37 +33 (C): line38 +34 (C): line39 +35 (C): line40 +36 (C): line41 +37 (C): line42 +38 (C): line43 +39 (C): line44 +40 (C): line45 +41 (C): line46 +42 (C): line47 +43 (C): line48 +44 (C): line49 +45 (C): line50 +46 (C): line51 +47 (C): line52 +48 (C): line53 +49 (C): line54 +50 (C): line55 +51 (C): line56 +52 (C): line57 +53 (C): line58 + From 9291ae3ebd0f5930a297326b972e0f12778bd353 Mon Sep 17 00:00:00 2001 From: Aram Drevekenin Date: Wed, 1 Dec 2021 10:02:46 +0100 Subject: [PATCH 2/2] style(fmt): make clippy and rustfmt happy --- src/tests/fixtures/scroll_region_full_screen | 61 ++++++++++++++++++++ zellij-server/src/panes/grid.rs | 11 ++-- 2 files changed, 67 insertions(+), 5 deletions(-) create mode 100755 src/tests/fixtures/scroll_region_full_screen diff --git a/src/tests/fixtures/scroll_region_full_screen b/src/tests/fixtures/scroll_region_full_screen new file mode 100755 index 0000000000..c4ee3bf270 --- /dev/null +++ b/src/tests/fixtures/scroll_region_full_screen @@ -0,0 +1,61 @@ + + line1 + line2 + line3 + line4 + line5 + line6 + line7 + line8 + line9 + line10 + line11 + line12 + line13 + line14 + line15 + line16 + line17 + line18 + line19 + line20 + line21 + line22 + line23 + line24 + line25 + line26 + line27 + line28 + line29 + line30 + line31 + line32 + line33 + line34 + line35 + line36 + line37 + line38 + line39 + line40 + line41 + line42 + line43 + line44 + line45 + line46 + line47 + line48 + line49 + line50 + line51 + line52 + line53 + line54 + line55 + line56 + line57 + line58 + line59 + line60 diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 5330d57b8a..c4796e8a36 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -940,7 +940,7 @@ impl Grid { // the state is corrupted return; } - if scroll_region_bottom == self.height -1 && scroll_region_top == 0 { + if scroll_region_bottom == self.height - 1 && scroll_region_top == 0 { let row_count_to_transfer = 1; let transferred_rows_count = transfer_rows_from_viewport_to_lines_above( &mut self.viewport, @@ -948,12 +948,13 @@ impl Grid { row_count_to_transfer, self.width, ); - self.scrollback_buffer_lines = - subtract_isize_from_usize(self.scrollback_buffer_lines, transferred_rows_count); + self.scrollback_buffer_lines = subtract_isize_from_usize( + self.scrollback_buffer_lines, + transferred_rows_count, + ); let columns = VecDeque::from(vec![EMPTY_TERMINAL_CHARACTER; self.width]); self.viewport.push(Row::from_columns(columns).canonical()); self.selection.move_up(1); - self.output_buffer.update_all_lines(); } else { self.viewport.remove(scroll_region_top); let columns = VecDeque::from(vec![EMPTY_TERMINAL_CHARACTER; self.width]); @@ -963,8 +964,8 @@ impl Grid { } else { self.viewport.push(Row::from_columns(columns).canonical()); } - self.output_buffer.update_all_lines(); // TODO: only update scroll region lines } + self.output_buffer.update_all_lines(); // TODO: only update scroll region lines return; } }