From 2ed84e385cff2732a75282f3607e5fd297ac5dff Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Sun, 17 Dec 2023 21:46:20 +0000 Subject: [PATCH 01/18] refactor: Simplify transfer_rows_from_viewport_to_lines_above next_lines is always consolidated to a single Row, which immediately gets removed - we can remove some dead code as a result --- zellij-server/src/panes/grid.rs | 37 +++++++++++++-------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index b7ed874f70..17956a83ab 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -166,37 +166,28 @@ fn transfer_rows_from_viewport_to_lines_above( count: usize, max_viewport_width: usize, ) -> isize { - let mut next_lines: Vec = vec![]; let mut transferred_rows_count: isize = 0; for _ in 0..count { - if next_lines.is_empty() { - if !viewport.is_empty() { - let next_line = viewport.remove(0); - transferred_rows_count += - calculate_row_display_height(next_line.width(), max_viewport_width) as isize; - if !next_line.is_canonical { - let mut bottom_canonical_row_and_wraps_in_dst = - get_lines_above_bottom_canonical_row_and_wraps(lines_above); - next_lines.append(&mut bottom_canonical_row_and_wraps_in_dst); - } - next_lines.push(next_line); - next_lines = vec![Row::from_rows(next_lines)]; - } else { - break; // no more rows - } + let mut next_lines: Vec = vec![]; + if !viewport.is_empty() { + let next_line = viewport.remove(0); + transferred_rows_count += + calculate_row_display_height(next_line.width(), max_viewport_width) as isize; + if !next_line.is_canonical { + let mut bottom_canonical_row_and_wraps_in_dst = + get_lines_above_bottom_canonical_row_and_wraps(lines_above); + next_lines.append(&mut bottom_canonical_row_and_wraps_in_dst); + } + next_lines.push(next_line); + } else { + break; // no more rows } - let dropped_line_width = bounded_push(lines_above, sixel_grid, next_lines.remove(0)); + let dropped_line_width = bounded_push(lines_above, sixel_grid, Row::from_rows(next_lines)); if let Some(width) = dropped_line_width { transferred_rows_count -= calculate_row_display_height(width, max_viewport_width) as isize; } } - if !next_lines.is_empty() { - let excess_rows = Row::from_rows(next_lines).split_to_rows_of_length(max_viewport_width); - for row in excess_rows { - viewport.insert(0, row); - } - } transferred_rows_count } From c1d8b5d89595fe68ebfe51361262694d3791884d Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Sun, 17 Dec 2023 21:57:57 +0000 Subject: [PATCH 02/18] perf: Batch remove rows from the viewport for performance Given a 1MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~9s to ~3s --- zellij-server/src/panes/grid.rs | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 17956a83ab..941b5ebe6f 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -167,21 +167,17 @@ fn transfer_rows_from_viewport_to_lines_above( max_viewport_width: usize, ) -> isize { let mut transferred_rows_count: isize = 0; - for _ in 0..count { + let drained_lines = std::cmp::min(count, viewport.len()); + for next_line in viewport.drain(..drained_lines) { let mut next_lines: Vec = vec![]; - if !viewport.is_empty() { - let next_line = viewport.remove(0); - transferred_rows_count += - calculate_row_display_height(next_line.width(), max_viewport_width) as isize; - if !next_line.is_canonical { - let mut bottom_canonical_row_and_wraps_in_dst = - get_lines_above_bottom_canonical_row_and_wraps(lines_above); - next_lines.append(&mut bottom_canonical_row_and_wraps_in_dst); - } - next_lines.push(next_line); - } else { - break; // no more rows - } + transferred_rows_count += + calculate_row_display_height(next_line.width(), max_viewport_width) as isize; + if !next_line.is_canonical { + let mut bottom_canonical_row_and_wraps_in_dst = + get_lines_above_bottom_canonical_row_and_wraps(lines_above); + next_lines.append(&mut bottom_canonical_row_and_wraps_in_dst); + } + next_lines.push(next_line); let dropped_line_width = bounded_push(lines_above, sixel_grid, Row::from_rows(next_lines)); if let Some(width) = dropped_line_width { transferred_rows_count -= From 78fafb12ffc795468c554a86a802125ca67287fc Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Mon, 18 Dec 2023 00:42:12 +0000 Subject: [PATCH 03/18] perf: Optimize Row::drain_until by splitting chars in one step Given a 10MB line catted into the terminal, a toggle-fullscreen + toggle-fullscreen + close-pane + `run true` goes from ~23s to ~20s --- zellij-server/src/panes/grid.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 941b5ebe6f..4b950f9288 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -3366,19 +3366,20 @@ impl Row { self.width = None; } pub fn drain_until(&mut self, x: usize) -> VecDeque { - let mut drained_part: VecDeque = VecDeque::new(); let mut drained_part_len = 0; - while let Some(next_character) = self.columns.remove(0) { + let mut split_pos = 0; + for next_character in self.columns.iter() { // drained_part_len == 0 here is so that if the grid is resized // to a size of 1, we won't drop wide characters if drained_part_len + next_character.width <= x || drained_part_len == 0 { - drained_part.push_back(next_character); drained_part_len += next_character.width; + split_pos += 1 } else { - self.columns.push_front(next_character); // put it back break; } } + // Can't use split_off because it doesn't reduce capacity, causing OOM with long lines + let drained_part = self.columns.drain(..split_pos).collect(); self.width = None; drained_part } From cca9656df14ae40d41c76011a13fd53c598ec063 Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Wed, 20 Dec 2023 11:00:52 +0000 Subject: [PATCH 04/18] refactor: Simplify `if let` into a `.map` --- zellij-server/src/panes/grid.rs | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 4b950f9288..f8d9e1def1 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -850,25 +850,17 @@ impl Grid { self.viewport = new_viewport_rows; let mut new_cursor_y = self.canonical_line_y_coordinates(cursor_canonical_line_index); - let mut saved_cursor_y_coordinates = - if let Some(saved_cursor) = self.saved_cursor_position.as_ref() { - Some(self.canonical_line_y_coordinates(saved_cursor.y)) - } else { - None - }; + let mut saved_cursor_y_coordinates = self.saved_cursor_position.as_ref() + .map(|saved_cursor| self.canonical_line_y_coordinates(saved_cursor.y)); let new_cursor_x = (cursor_index_in_canonical_line / new_columns) + (cursor_index_in_canonical_line % new_columns); - let saved_cursor_x_coordinates = if let Some(saved_cursor_index_in_canonical_line) = - saved_cursor_index_in_canonical_line.as_ref() - { - Some( - (*saved_cursor_index_in_canonical_line / new_columns) - + (*saved_cursor_index_in_canonical_line % new_columns), - ) - } else { - None - }; + let saved_cursor_x_coordinates = saved_cursor_index_in_canonical_line.as_ref() + .map(|saved_cursor_index_in_canonical_line| + (*saved_cursor_index_in_canonical_line / new_columns) + + (*saved_cursor_index_in_canonical_line % new_columns) + ); + let current_viewport_row_count = self.viewport.len(); match current_viewport_row_count.cmp(&self.height) { Ordering::Less => { From 8ce6ce10b9cb282efbe9ffcbbb1643064b31f684 Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Sun, 24 Dec 2023 20:46:09 +0000 Subject: [PATCH 05/18] refactor: There are only new saved coordinates when there were old ones --- zellij-server/src/panes/grid.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index f8d9e1def1..c199169720 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -911,10 +911,9 @@ impl Grid { saved_cursor_position.x = saved_cursor_x_coordinates; saved_cursor_position.y = saved_cursor_y_coordinates; }, - _ => { - saved_cursor_position.x = new_cursor_x; - saved_cursor_position.y = new_cursor_y; - }, + _ => unreachable!("saved cursor {:?} {:?}", + saved_cursor_x_coordinates, + saved_cursor_y_coordinates), } }; } From 110bf46c2acd20c668fa3589da26b33cc05b365c Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Sun, 24 Dec 2023 20:50:32 +0000 Subject: [PATCH 06/18] refactor: Unify viewport transfer: use common variable names --- zellij-server/src/panes/grid.rs | 42 ++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index c199169720..8070bba225 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -918,6 +918,14 @@ impl Grid { }; } if new_rows != self.height { + let mut new_cursor_y = self.cursor.y; + let mut saved_cursor_y_coordinates = + self.saved_cursor_position.as_ref().map(|saved_cursor| saved_cursor.y); + + let new_cursor_x = self.cursor.x; + let mut saved_cursor_x_coordinates = + self.saved_cursor_position.as_ref().map(|saved_cursor| saved_cursor.x); + let current_viewport_row_count = self.viewport.len(); match current_viewport_row_count.cmp(&new_rows) { Ordering::Less => { @@ -930,23 +938,22 @@ impl Grid { new_columns, ); let rows_pulled = self.viewport.len() - current_viewport_row_count; - self.cursor.y += rows_pulled; - if let Some(saved_cursor_position) = self.saved_cursor_position.as_mut() { - saved_cursor_position.y += rows_pulled + new_cursor_y += rows_pulled; + if let Some(saved_cursor_y_coordinates) = saved_cursor_y_coordinates.as_mut() { + *saved_cursor_y_coordinates += rows_pulled; }; }, Ordering::Greater => { let row_count_to_transfer = current_viewport_row_count - new_rows; - if row_count_to_transfer > self.cursor.y { - self.cursor.y = 0; - if let Some(saved_cursor_position) = self.saved_cursor_position.as_mut() { - saved_cursor_position.y = 0 + if row_count_to_transfer > new_cursor_y { + new_cursor_y = 0; + if let Some(saved_cursor_y_coordinates) = saved_cursor_y_coordinates.as_mut() { + *saved_cursor_y_coordinates = 0 }; } else { - self.cursor.y -= row_count_to_transfer; - if let Some(saved_cursor_position) = self.saved_cursor_position.as_mut() { - saved_cursor_position.y = saved_cursor_position - .y + new_cursor_y -= row_count_to_transfer; + if let Some(saved_cursor_y_coordinates) = saved_cursor_y_coordinates.as_mut() { + *saved_cursor_y_coordinates = saved_cursor_y_coordinates .saturating_sub(row_count_to_transfer); }; } @@ -960,6 +967,19 @@ impl Grid { }, Ordering::Equal => {}, } + self.cursor.y = new_cursor_y; + self.cursor.x = new_cursor_x; + if let Some(saved_cursor_position) = self.saved_cursor_position.as_mut() { + match (saved_cursor_x_coordinates, saved_cursor_y_coordinates) { + (Some(saved_cursor_x_coordinates), Some(saved_cursor_y_coordinates)) => { + saved_cursor_position.x = saved_cursor_x_coordinates; + saved_cursor_position.y = saved_cursor_y_coordinates; + }, + _ => unreachable!("saved cursor {:?} {:?}", + saved_cursor_x_coordinates, + saved_cursor_y_coordinates), + } + }; } self.height = new_rows; self.width = new_columns; From 75143e6531b6ca2352f80667260a10d62feb5dc6 Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Mon, 25 Dec 2023 15:23:58 +0000 Subject: [PATCH 07/18] fix: Use same saved cursor logic in height resize as width See #2182 for original introduction that only added it in one branch, this fixes an issue where the saved cursor was incorrectly reset when the real cursor was --- zellij-server/src/panes/grid.rs | 16 +++---- zellij-server/src/panes/unit/grid_tests.rs | 42 +++++++++++++++++++ ...rid_tests__saved_cursor_across_resize.snap | 9 ++++ 3 files changed, 59 insertions(+), 8 deletions(-) create mode 100644 zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize.snap diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 8070bba225..82d08231a3 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -923,7 +923,7 @@ impl Grid { self.saved_cursor_position.as_ref().map(|saved_cursor| saved_cursor.y); let new_cursor_x = self.cursor.x; - let mut saved_cursor_x_coordinates = + let saved_cursor_x_coordinates = self.saved_cursor_position.as_ref().map(|saved_cursor| saved_cursor.x); let current_viewport_row_count = self.viewport.len(); @@ -947,15 +947,15 @@ impl Grid { let row_count_to_transfer = current_viewport_row_count - new_rows; if row_count_to_transfer > new_cursor_y { new_cursor_y = 0; - if let Some(saved_cursor_y_coordinates) = saved_cursor_y_coordinates.as_mut() { - *saved_cursor_y_coordinates = 0 - }; } else { new_cursor_y -= row_count_to_transfer; - if let Some(saved_cursor_y_coordinates) = saved_cursor_y_coordinates.as_mut() { - *saved_cursor_y_coordinates = saved_cursor_y_coordinates - .saturating_sub(row_count_to_transfer); - }; + } + if let Some(saved_cursor_y_coordinates) = saved_cursor_y_coordinates.as_mut() { + if row_count_to_transfer > *saved_cursor_y_coordinates { + *saved_cursor_y_coordinates = 0; + } else { + *saved_cursor_y_coordinates -= row_count_to_transfer; + } } transfer_rows_from_viewport_to_lines_above( &mut self.viewport, diff --git a/zellij-server/src/panes/unit/grid_tests.rs b/zellij-server/src/panes/unit/grid_tests.rs index 781904c365..01d6cc9402 100644 --- a/zellij-server/src/panes/unit/grid_tests.rs +++ b/zellij-server/src/panes/unit/grid_tests.rs @@ -2400,6 +2400,48 @@ pub fn scroll_up_increase_width_and_scroll_down() { assert_snapshot!(format!("{:?}", grid)); } +#[test] +fn saved_cursor_across_resize() { + let mut vte_parser = vte::Parser::new(); + let sixel_image_store = Rc::new(RefCell::new(SixelImageStore::default())); + let terminal_emulator_color_codes = Rc::new(RefCell::new(HashMap::new())); + let debug = false; + let arrow_fonts = true; + let styled_underlines = true; + let mut grid = Grid::new( + 4, + 20, + Rc::new(RefCell::new(Palette::default())), + terminal_emulator_color_codes, + Rc::new(RefCell::new(LinkHandler::new())), + Rc::new(RefCell::new(None)), + sixel_image_store, + Style::default(), + debug, + arrow_fonts, + styled_underlines, + ); + let mut parse = |s, grid: &mut Grid| + for b in Vec::from(s) { vte_parser.advance(&mut *grid, b) }; + let content = "\n +\rLine 1 >fill to 20_< +\rLine 2 >fill to 20_< +\rLine 3 >fill to 20_< +\rL\u{1b}[sine 4 >fill to 20_<"; + parse(content, &mut grid); + // Move real cursor position up three lines + let content = "\u{1b}[3A"; + parse(content, &mut grid); + // Truncate top of terminal, resetting cursor (but not saved cursor) + grid.change_size(3, 20); + // Wrap, resetting cursor again (but not saved cursor) + grid.change_size(3, 10); + // Restore saved cursor position and write ZZZ + let content = "\u{1b}[uZZZ"; + parse(content, &mut grid); + assert_snapshot!(format!("{:?}", grid)); +} + #[test] pub fn move_cursor_below_scroll_region() { let mut vte_parser = vte::Parser::new(); diff --git a/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize.snap b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize.snap new file mode 100644 index 0000000000..dcb20d3880 --- /dev/null +++ b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize.snap @@ -0,0 +1,9 @@ +--- +source: zellij-server/src/panes/./unit/grid_tests.rs +assertion_line: 2443 +expression: "format!(\"{:?}\", grid)" +--- +00 (W): ll to 20_< +01 (C): LZZZ 4 >fi +02 (W): ll to 20_< + From 37d0dab0b0778999dc7557fa7843a6f429e4bf65 Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Mon, 25 Dec 2023 18:17:59 +0000 Subject: [PATCH 08/18] fix: Correct saved+real cursor calculations when reflowing long lines --- zellij-server/src/panes/grid.rs | 14 ++++---- zellij-server/src/panes/unit/grid_tests.rs | 35 ++++++++++++++++++- ...__saved_cursor_across_resize_longline.snap | 10 ++++++ 3 files changed, 52 insertions(+), 7 deletions(-) create mode 100644 zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_longline.snap diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 82d08231a3..30896b7847 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -849,16 +849,18 @@ impl Grid { self.viewport = new_viewport_rows; - let mut new_cursor_y = self.canonical_line_y_coordinates(cursor_canonical_line_index); + let mut new_cursor_y = self.canonical_line_y_coordinates(cursor_canonical_line_index) + + (cursor_index_in_canonical_line / new_columns); let mut saved_cursor_y_coordinates = self.saved_cursor_position.as_ref() - .map(|saved_cursor| self.canonical_line_y_coordinates(saved_cursor.y)); + .map(|saved_cursor| + self.canonical_line_y_coordinates(saved_cursor.y) + + saved_cursor_index_in_canonical_line.as_ref().unwrap() / new_columns + ); - let new_cursor_x = (cursor_index_in_canonical_line / new_columns) - + (cursor_index_in_canonical_line % new_columns); + let new_cursor_x = cursor_index_in_canonical_line % new_columns; let saved_cursor_x_coordinates = saved_cursor_index_in_canonical_line.as_ref() .map(|saved_cursor_index_in_canonical_line| - (*saved_cursor_index_in_canonical_line / new_columns) - + (*saved_cursor_index_in_canonical_line % new_columns) + *saved_cursor_index_in_canonical_line % new_columns ); let current_viewport_row_count = self.viewport.len(); diff --git a/zellij-server/src/panes/unit/grid_tests.rs b/zellij-server/src/panes/unit/grid_tests.rs index 01d6cc9402..a17ac2bc1a 100644 --- a/zellij-server/src/panes/unit/grid_tests.rs +++ b/zellij-server/src/panes/unit/grid_tests.rs @@ -2423,7 +2423,7 @@ fn saved_cursor_across_resize() { ); let mut parse = |s, grid: &mut Grid| for b in Vec::from(s) { vte_parser.advance(&mut *grid, b) }; - let content = "\n + let content = " \rLine 1 >fill to 20_< \rLine 2 >fill to 20_< \rLine 3 >fill to 20_< @@ -2442,6 +2442,39 @@ fn saved_cursor_across_resize() { assert_snapshot!(format!("{:?}", grid)); } +#[test] +fn saved_cursor_across_resize_longline() { + let mut vte_parser = vte::Parser::new(); + let sixel_image_store = Rc::new(RefCell::new(SixelImageStore::default())); + let terminal_emulator_color_codes = Rc::new(RefCell::new(HashMap::new())); + let debug = false; + let arrow_fonts = true; + let styled_underlines = true; + let mut grid = Grid::new( + 4, + 20, + Rc::new(RefCell::new(Palette::default())), + terminal_emulator_color_codes, + Rc::new(RefCell::new(LinkHandler::new())), + Rc::new(RefCell::new(None)), + sixel_image_store, + Style::default(), + debug, + arrow_fonts, + styled_underlines, + ); + let mut parse = |s, grid: &mut Grid| + for b in Vec::from(s) { vte_parser.advance(&mut *grid, b) }; + let content = " +\rLine 1 >fill \u{1b}[sto 20_<"; + parse(content, &mut grid); + grid.change_size(4, 10); + // Write 'YY' at the end, restore to the saved cursor and overwrite 'to' with 'ZZ' + let content = "YY\u{1b}[uZZ\u{1b}[s"; + parse(content, &mut grid); + assert_snapshot!(format!("{:?}", grid)); +} + #[test] pub fn move_cursor_below_scroll_region() { let mut vte_parser = vte::Parser::new(); diff --git a/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_longline.snap b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_longline.snap new file mode 100644 index 0000000000..a183095da0 --- /dev/null +++ b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_longline.snap @@ -0,0 +1,10 @@ +--- +source: zellij-server/src/panes/./unit/grid_tests.rs +assertion_line: 2475 +expression: "format!(\"{:?}\", grid)" +--- +00 (C): +01 (C): Line 1 >fi +02 (W): ll ZZ 20_< +03 (C): YY + From 918b64296e03f56d558b88f86a1206ceeb339458 Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Mon, 25 Dec 2023 20:14:30 +0000 Subject: [PATCH 09/18] fix: Don't create canonical lines if cursor ends on EOL after resize Previously if a 20 character line were split into two 10 character lines, the cursor would be placed on the line after the two lines. New characters would then be treated as a new canonical line. This commit fixes this by biasing cursors to the end of the previous line. --- zellij-server/src/panes/grid.rs | 22 +++++++++++++++---- zellij-server/src/panes/unit/grid_tests.rs | 3 ++- ...__saved_cursor_across_resize_longline.snap | 2 +- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 30896b7847..2e9b9db677 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -857,11 +857,25 @@ impl Grid { + saved_cursor_index_in_canonical_line.as_ref().unwrap() / new_columns ); - let new_cursor_x = cursor_index_in_canonical_line % new_columns; + // A cursor at EOL has two equivalent positions - end of this line or beginning of + // next. If not already at the beginning of line, bias to EOL so add character logic + // doesn't create spurious canonical lines + let mut new_cursor_x = cursor_index_in_canonical_line % new_columns; + if self.cursor.x != 0 && new_cursor_x == 0 { + new_cursor_y -= 1; + new_cursor_x = new_columns + } let saved_cursor_x_coordinates = saved_cursor_index_in_canonical_line.as_ref() - .map(|saved_cursor_index_in_canonical_line| - *saved_cursor_index_in_canonical_line % new_columns - ); + .map(|saved_cursor_index_in_canonical_line| { + let x = self.saved_cursor_position.as_ref().unwrap().x; + let mut new_x = *saved_cursor_index_in_canonical_line % new_columns; + let new_y = saved_cursor_y_coordinates.as_mut().unwrap(); + if x != 0 && new_x == 0 { + *new_y -= 1; + new_x = new_columns + } + new_x + }); let current_viewport_row_count = self.viewport.len(); match current_viewport_row_count.cmp(&self.height) { diff --git a/zellij-server/src/panes/unit/grid_tests.rs b/zellij-server/src/panes/unit/grid_tests.rs index a17ac2bc1a..f940ddeaa9 100644 --- a/zellij-server/src/panes/unit/grid_tests.rs +++ b/zellij-server/src/panes/unit/grid_tests.rs @@ -2468,9 +2468,10 @@ fn saved_cursor_across_resize_longline() { let content = " \rLine 1 >fill \u{1b}[sto 20_<"; parse(content, &mut grid); + // Wrap each line halfway grid.change_size(4, 10); // Write 'YY' at the end, restore to the saved cursor and overwrite 'to' with 'ZZ' - let content = "YY\u{1b}[uZZ\u{1b}[s"; + let content = "YY\u{1b}[uZZ"; parse(content, &mut grid); assert_snapshot!(format!("{:?}", grid)); } diff --git a/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_longline.snap b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_longline.snap index a183095da0..1aedfaa9e7 100644 --- a/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_longline.snap +++ b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_longline.snap @@ -6,5 +6,5 @@ expression: "format!(\"{:?}\", grid)" 00 (C): 01 (C): Line 1 >fi 02 (W): ll ZZ 20_< -03 (C): YY +03 (W): YY From e2c9d6fdf7408865c5e2c78a68430aafb6d28e9c Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Mon, 25 Dec 2023 22:24:09 +0000 Subject: [PATCH 10/18] fix: for cursor index calculation in lines that are already wrapped --- zellij-server/src/panes/grid.rs | 9 ++--- zellij-server/src/panes/unit/grid_tests.rs | 40 ++++++++++++++++++- ...ts__saved_cursor_across_resize_rewrap.snap | 10 +++++ 3 files changed, 52 insertions(+), 7 deletions(-) create mode 100644 zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_rewrap.snap diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 2e9b9db677..13dd0affd7 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -623,8 +623,8 @@ impl Grid { cursor_canonical_line_index = i; } if i == self.cursor.y { - let line_wrap_position_in_line = self.cursor.y - cursor_canonical_line_index; - cursor_index_in_canonical_line = line_wrap_position_in_line + self.cursor.x; + let line_wraps = self.cursor.y - cursor_canonical_line_index; + cursor_index_in_canonical_line = (line_wraps * self.width) + self.cursor.x; break; } } @@ -639,10 +639,9 @@ impl Grid { cursor_canonical_line_index = i; } if i == saved_cursor_position.y { - let line_wrap_position_in_line = - saved_cursor_position.y - cursor_canonical_line_index; + let line_wraps = saved_cursor_position.y - cursor_canonical_line_index; cursor_index_in_canonical_line = - line_wrap_position_in_line + saved_cursor_position.x; + (line_wraps * self.width) + saved_cursor_position.x; break; } } diff --git a/zellij-server/src/panes/unit/grid_tests.rs b/zellij-server/src/panes/unit/grid_tests.rs index f940ddeaa9..7888e79be5 100644 --- a/zellij-server/src/panes/unit/grid_tests.rs +++ b/zellij-server/src/panes/unit/grid_tests.rs @@ -2468,14 +2468,50 @@ fn saved_cursor_across_resize_longline() { let content = " \rLine 1 >fill \u{1b}[sto 20_<"; parse(content, &mut grid); - // Wrap each line halfway + // Wrap each line precisely halfway grid.change_size(4, 10); - // Write 'YY' at the end, restore to the saved cursor and overwrite 'to' with 'ZZ' + // Write 'YY' at the end (ends up on a new wrapped line), restore to the saved cursor + // and overwrite 'to' with 'ZZ' let content = "YY\u{1b}[uZZ"; parse(content, &mut grid); assert_snapshot!(format!("{:?}", grid)); } +#[test] +fn saved_cursor_across_resize_rewrap() { + let mut vte_parser = vte::Parser::new(); + let sixel_image_store = Rc::new(RefCell::new(SixelImageStore::default())); + let terminal_emulator_color_codes = Rc::new(RefCell::new(HashMap::new())); + let debug = false; + let arrow_fonts = true; + let styled_underlines = true; + let mut grid = Grid::new( + 4, + 4*8, + Rc::new(RefCell::new(Palette::default())), + terminal_emulator_color_codes, + Rc::new(RefCell::new(LinkHandler::new())), + Rc::new(RefCell::new(None)), + sixel_image_store, + Style::default(), + debug, + arrow_fonts, + styled_underlines, + ); + let mut parse = |s, grid: &mut Grid| + for b in Vec::from(s) { vte_parser.advance(&mut *grid, b) }; + let content = " +\r12345678123456781234567\u{1b}[s812345678"; // 4*8 chars + parse(content, &mut grid); + // Wrap each line precisely halfway, then rewrap to halve them again + grid.change_size(4, 16); + grid.change_size(4, 8); + // Write 'Z' at the end of line 3 + let content = "\u{1b}[uZ"; + parse(content, &mut grid); + assert_snapshot!(format!("{:?}", grid)); +} + #[test] pub fn move_cursor_below_scroll_region() { let mut vte_parser = vte::Parser::new(); diff --git a/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_rewrap.snap b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_rewrap.snap new file mode 100644 index 0000000000..f250f8658f --- /dev/null +++ b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__saved_cursor_across_resize_rewrap.snap @@ -0,0 +1,10 @@ +--- +source: zellij-server/src/panes/./unit/grid_tests.rs +assertion_line: 2512 +expression: "format!(\"{:?}\", grid)" +--- +00 (C): 12345678 +01 (W): 12345678 +02 (W): 1234567Z +03 (W): 12345678 + From fba2574e8c2019061f3e736aac69eef518cc3e68 Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Tue, 26 Dec 2023 15:34:32 +0000 Subject: [PATCH 11/18] chore: test for real/saved cursor position being handled separately --- zellij-server/src/tab/unit/tab_integration_tests.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/zellij-server/src/tab/unit/tab_integration_tests.rs b/zellij-server/src/tab/unit/tab_integration_tests.rs index f1820b725c..1ee1a72701 100644 --- a/zellij-server/src/tab/unit/tab_integration_tests.rs +++ b/zellij-server/src/tab/unit/tab_integration_tests.rs @@ -2039,10 +2039,20 @@ fn save_cursor_position_across_resizes() { 1, Vec::from("\n\n\rI am some text\n\rI am another line of text\n\rLet's save the cursor position here \u{1b}[sI should be ovewritten".as_bytes()), ).unwrap(); - tab.resize_whole_tab(Size { cols: 100, rows: 3 }).unwrap(); + + // We check cursor and saved cursor are handled separately by: + // 1. moving real cursor up two lines + tab.handle_pty_bytes(1, Vec::from("\u{1b}[2A".as_bytes())); + // 2. resizing so real cursor gets lost above the viewport, which resets it to row 0 + // The saved cursor ends up on row 1, allowing detection if it (incorrectly) gets reset too + tab.resize_whole_tab(Size { cols: 35, rows: 4 }).unwrap(); + + // Now overwrite tab.handle_pty_bytes(1, Vec::from("\u{1b}[uthis overwrote me!".as_bytes())) .unwrap(); + tab.resize_whole_tab(Size { cols: 100, rows: 3 }).unwrap(); + tab.render(&mut output).unwrap(); let snapshot = take_snapshot( output.serialize().unwrap().get(&client_id).unwrap(), From 528ccbfe7b425972368e89759f2b6715036aa109 Mon Sep 17 00:00:00 2001 From: Aidan Hobson Sayers Date: Wed, 27 Dec 2023 13:28:27 +0000 Subject: [PATCH 12/18] chore: Apply cargo format --- zellij-server/src/panes/grid.rs | 39 +++++++++++++--------- zellij-server/src/panes/unit/grid_tests.rs | 23 +++++++++---- 2 files changed, 39 insertions(+), 23 deletions(-) diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index 13dd0affd7..a600db59e9 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -850,11 +850,11 @@ impl Grid { let mut new_cursor_y = self.canonical_line_y_coordinates(cursor_canonical_line_index) + (cursor_index_in_canonical_line / new_columns); - let mut saved_cursor_y_coordinates = self.saved_cursor_position.as_ref() - .map(|saved_cursor| + let mut saved_cursor_y_coordinates = + self.saved_cursor_position.as_ref().map(|saved_cursor| { self.canonical_line_y_coordinates(saved_cursor.y) + saved_cursor_index_in_canonical_line.as_ref().unwrap() / new_columns - ); + }); // A cursor at EOL has two equivalent positions - end of this line or beginning of // next. If not already at the beginning of line, bias to EOL so add character logic @@ -864,8 +864,8 @@ impl Grid { new_cursor_y -= 1; new_cursor_x = new_columns } - let saved_cursor_x_coordinates = saved_cursor_index_in_canonical_line.as_ref() - .map(|saved_cursor_index_in_canonical_line| { + let saved_cursor_x_coordinates = saved_cursor_index_in_canonical_line.as_ref().map( + |saved_cursor_index_in_canonical_line| { let x = self.saved_cursor_position.as_ref().unwrap().x; let mut new_x = *saved_cursor_index_in_canonical_line % new_columns; let new_y = saved_cursor_y_coordinates.as_mut().unwrap(); @@ -874,7 +874,8 @@ impl Grid { new_x = new_columns } new_x - }); + }, + ); let current_viewport_row_count = self.viewport.len(); match current_viewport_row_count.cmp(&self.height) { @@ -926,20 +927,25 @@ impl Grid { saved_cursor_position.x = saved_cursor_x_coordinates; saved_cursor_position.y = saved_cursor_y_coordinates; }, - _ => unreachable!("saved cursor {:?} {:?}", - saved_cursor_x_coordinates, - saved_cursor_y_coordinates), + _ => unreachable!( + "saved cursor {:?} {:?}", + saved_cursor_x_coordinates, saved_cursor_y_coordinates + ), } }; } if new_rows != self.height { let mut new_cursor_y = self.cursor.y; - let mut saved_cursor_y_coordinates = - self.saved_cursor_position.as_ref().map(|saved_cursor| saved_cursor.y); + let mut saved_cursor_y_coordinates = self + .saved_cursor_position + .as_ref() + .map(|saved_cursor| saved_cursor.y); let new_cursor_x = self.cursor.x; - let saved_cursor_x_coordinates = - self.saved_cursor_position.as_ref().map(|saved_cursor| saved_cursor.x); + let saved_cursor_x_coordinates = self + .saved_cursor_position + .as_ref() + .map(|saved_cursor| saved_cursor.x); let current_viewport_row_count = self.viewport.len(); match current_viewport_row_count.cmp(&new_rows) { @@ -990,9 +996,10 @@ impl Grid { saved_cursor_position.x = saved_cursor_x_coordinates; saved_cursor_position.y = saved_cursor_y_coordinates; }, - _ => unreachable!("saved cursor {:?} {:?}", - saved_cursor_x_coordinates, - saved_cursor_y_coordinates), + _ => unreachable!( + "saved cursor {:?} {:?}", + saved_cursor_x_coordinates, saved_cursor_y_coordinates + ), } }; } diff --git a/zellij-server/src/panes/unit/grid_tests.rs b/zellij-server/src/panes/unit/grid_tests.rs index 7888e79be5..2459796b80 100644 --- a/zellij-server/src/panes/unit/grid_tests.rs +++ b/zellij-server/src/panes/unit/grid_tests.rs @@ -2421,8 +2421,11 @@ fn saved_cursor_across_resize() { arrow_fonts, styled_underlines, ); - let mut parse = |s, grid: &mut Grid| - for b in Vec::from(s) { vte_parser.advance(&mut *grid, b) }; + let mut parse = |s, grid: &mut Grid| { + for b in Vec::from(s) { + vte_parser.advance(&mut *grid, b) + } + }; let content = " \rLine 1 >fill to 20_< \rLine 2 >fill to 20_< @@ -2463,8 +2466,11 @@ fn saved_cursor_across_resize_longline() { arrow_fonts, styled_underlines, ); - let mut parse = |s, grid: &mut Grid| - for b in Vec::from(s) { vte_parser.advance(&mut *grid, b) }; + let mut parse = |s, grid: &mut Grid| { + for b in Vec::from(s) { + vte_parser.advance(&mut *grid, b) + } + }; let content = " \rLine 1 >fill \u{1b}[sto 20_<"; parse(content, &mut grid); @@ -2487,7 +2493,7 @@ fn saved_cursor_across_resize_rewrap() { let styled_underlines = true; let mut grid = Grid::new( 4, - 4*8, + 4 * 8, Rc::new(RefCell::new(Palette::default())), terminal_emulator_color_codes, Rc::new(RefCell::new(LinkHandler::new())), @@ -2498,8 +2504,11 @@ fn saved_cursor_across_resize_rewrap() { arrow_fonts, styled_underlines, ); - let mut parse = |s, grid: &mut Grid| - for b in Vec::from(s) { vte_parser.advance(&mut *grid, b) }; + let mut parse = |s, grid: &mut Grid| { + for b in Vec::from(s) { + vte_parser.advance(&mut *grid, b) + } + }; let content = " \r12345678123456781234567\u{1b}[s812345678"; // 4*8 chars parse(content, &mut grid); From 11b59da0e712aba7fd7858b1a781a134e0ea3c19 Mon Sep 17 00:00:00 2001 From: Aram Drevekenin Date: Thu, 28 Dec 2023 16:46:14 +0100 Subject: [PATCH 13/18] chore(repo): update issue templates --- .github/ISSUE_TEMPLATE/blank_issue.md | 10 ---------- .github/ISSUE_TEMPLATE/bug_report.md | 4 ++-- 2 files changed, 2 insertions(+), 12 deletions(-) delete mode 100644 .github/ISSUE_TEMPLATE/blank_issue.md diff --git a/.github/ISSUE_TEMPLATE/blank_issue.md b/.github/ISSUE_TEMPLATE/blank_issue.md deleted file mode 100644 index a08ad07cbf..0000000000 --- a/.github/ISSUE_TEMPLATE/blank_issue.md +++ /dev/null @@ -1,10 +0,0 @@ ---- -name: Blank Issue -about: Create a blank issue. -title: '' -labels: '' -assignees: '' - ---- - - diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 8ae1af7049..e84ba13cca 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -10,7 +10,7 @@ assignees: '' @@ -36,7 +36,7 @@ Please attach the files that were created in `/tmp/zellij-1000/zellij-log/` to t ## Further information Reproduction steps, noticeable behavior, related issues, etc -# 2. Issues with the Zellij UI / behavior +# 2. Issues with the Zellij UI / behavior / crash