Fix some real/saved cursor bugs during resize

src/tests/e2e/snapshots/zellij__tests__e2e__cases__start_without_pane_frames.snap

@@ -1,11 +1,11 @@
 ---
 source: src/tests/e2e/cases.rs
-assertion_line: 1194
+assertion_line: 1326
 expression: last_snapshot
 ---
  Zellij (e2e-test)  Tab #1 
 $                                                          │$ █                                                         
-                                                           │                                                            
+$                                                          │                                                            
                                                            │                                                            
                                                            │                                                            
                                                            │                                                            

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.saturating_sub(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;
                 }
             }
@@ -849,26 +848,44 @@ 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 =
-                if let Some(saved_cursor) = self.saved_cursor_position.as_ref() {
-                    Some(self.canonical_line_y_coordinates(saved_cursor.y))
-                } else {
-                    None
-                };
-
-            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
+                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
+            // 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 = new_cursor_y.saturating_sub(1);
+                new_cursor_x = new_columns
+            }
+            let saved_cursor_x_coordinates = match (
+                saved_cursor_index_in_canonical_line.as_ref(),
+                self.saved_cursor_position.as_mut(),
+                saved_cursor_y_coordinates.as_mut(),
+            ) {
+                (
+                    Some(saved_cursor_index_in_canonical_line),
+                    Some(saved_cursor_position),
+                    Some(saved_cursor_y_coordinates),
+                ) => {
+                    let x = saved_cursor_position.x;
+                    let mut new_x = *saved_cursor_index_in_canonical_line % new_columns;
+                    let new_y = saved_cursor_y_coordinates;
+                    if x != 0 && new_x == 0 {
+                        *new_y = new_y.saturating_sub(1);
+                        new_x = new_columns
+                    }
+                    Some(new_x)
+                },
+                _ => None,
             };
+
             let current_viewport_row_count = self.viewport.len();
             match current_viewport_row_count.cmp(&self.height) {
                 Ordering::Less => {
@@ -919,14 +936,27 @@ 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;
-                    },
+                    _ => log::error!(
+                        "invalid state - cannot set saved cursor to {:?} {:?}",
+                        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 new_cursor_x = self.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) {
                 Ordering::Less => {
@@ -939,25 +969,24 @@ 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;
                     } 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
-                                .saturating_sub(row_count_to_transfer);
-                        };
+                        new_cursor_y -= 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,
@@ -969,6 +998,21 @@ 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;
+                    },
+                    _ => log::error!(
+                        "invalid state - cannot set saved cursor to {:?} {:?}",
+                        saved_cursor_x_coordinates,
+                        saved_cursor_y_coordinates
+                    ),
+                }
+            };
         }
         self.height = new_rows;
         self.width = new_columns;

zellij-server/src/panes/unit/grid_tests.rs

@@ -2400,6 +2400,127 @@ 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 = "
+\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]
+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);
+    // Wrap each line precisely halfway
+    grid.change_size(4, 10);
+    // 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();

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_<
+

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 (W): YY
+

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
+

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(),