From 22b68c06547d9b35ee079e0a846913b16a311c74 Mon Sep 17 00:00:00 2001 From: Aram Drevekenin Date: Tue, 26 Apr 2022 22:38:54 +0200 Subject: [PATCH] fix(compatibility): fix crash and don't allow cursor beyond row width (#1349) * fix(compatibility): fix crash and don't allow cursor beyond row width * style(fmt): rustfmt --- zellij-server/src/panes/grid.rs | 110 +++++++----------- zellij-server/src/panes/unit/grid_tests.rs | 17 +++ ...s__grid__grid_tests__ansi_csi_at_sign.snap | 7 ++ ...r__panes__grid__grid_tests__vttest8_1.snap | 4 +- ...r__panes__grid__grid_tests__vttest8_2.snap | 4 +- 5 files changed, 72 insertions(+), 70 deletions(-) create mode 100644 zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__ansi_csi_at_sign.snap diff --git a/zellij-server/src/panes/grid.rs b/zellij-server/src/panes/grid.rs index b74966f3..4a569bb1 100644 --- a/zellij-server/src/panes/grid.rs +++ b/zellij-server/src/panes/grid.rs @@ -382,7 +382,7 @@ impl Grid { .horizontal_tabstops .iter() .copied() - .find(|&tabstop| tabstop > self.cursor.x); + .find(|&tabstop| tabstop > self.cursor.x && tabstop < self.width); match next_tabstop { Some(tabstop) => { self.cursor.x = tabstop; @@ -935,37 +935,22 @@ impl Grid { pub fn move_cursor_to_beginning_of_line(&mut self) { self.cursor.x = 0; } - pub fn insert_character_at_cursor_position(&mut self, terminal_character: TerminalCharacter) { + pub fn add_character_at_cursor_position( + &mut self, + terminal_character: TerminalCharacter, + should_insert_character: bool, + ) { + // this function assumes the current line has enough room for terminal_character (that its + // width has been checked beforehand) match self.viewport.get_mut(self.cursor.y) { Some(row) => { - row.insert_character_at(terminal_character, self.cursor.x); - if row.width() > self.width { - row.truncate(self.width); - } - self.output_buffer.update_line(self.cursor.y); - } - None => { - // pad lines until cursor if they do not exist - for _ in self.viewport.len()..self.cursor.y { - self.viewport.push(Row::new(self.width).canonical()); - } - self.viewport.push( - Row::new(self.width) - .with_character(terminal_character) - .canonical(), - ); - self.output_buffer.update_all_lines(); - } - } - } - pub fn add_character_at_cursor_position(&mut self, terminal_character: TerminalCharacter) { - match self.viewport.get_mut(self.cursor.y) { - Some(row) => { - if self.insert_mode { + if self.insert_mode || should_insert_character { row.insert_character_at(terminal_character, self.cursor.x); + if row.width() > self.width { + row.truncate(self.width); + } } else { row.add_character_at(terminal_character, self.cursor.x); - // self.move_cursor_forward_until_edge(count_to_move_cursor); } self.output_buffer.update_line(self.cursor.y); } @@ -984,7 +969,6 @@ impl Grid { } } pub fn add_character(&mut self, terminal_character: TerminalCharacter) { - // TODO: try to separate adding characters from moving the cursors in this function let character_width = terminal_character.width; if character_width == 0 { return; @@ -993,28 +977,9 @@ impl Grid { if self.disable_linewrap { return; } - // line wrap - self.cursor.x = 0; - if self.cursor.y == self.height - 1 { - if self.alternate_lines_above_viewport_and_cursor.is_none() { - self.transfer_rows_to_lines_above(1); - } else { - self.viewport.remove(0); - } - let wrapped_row = Row::new(self.width); - self.viewport.push(wrapped_row); - self.selection.move_up(1); - self.output_buffer.update_all_lines(); - } else { - self.cursor.y += 1; - if self.viewport.len() <= self.cursor.y { - let line_wrapped_row = Row::new(self.width); - self.viewport.push(line_wrapped_row); - self.output_buffer.update_line(self.cursor.y); - } - } + self.line_wrap(); } - self.add_character_at_cursor_position(terminal_character); + self.add_character_at_cursor_position(terminal_character, false); self.move_cursor_forward_until_edge(character_width); } pub fn get_character_under_cursor(&self) -> Option { @@ -1075,6 +1040,27 @@ impl Grid { } self.output_buffer.update_all_lines(); } + fn line_wrap(&mut self) { + self.cursor.x = 0; + if self.cursor.y == self.height - 1 { + if self.alternate_lines_above_viewport_and_cursor.is_none() { + self.transfer_rows_to_lines_above(1); + } else { + self.viewport.remove(0); + } + let wrapped_row = Row::new(self.width); + self.viewport.push(wrapped_row); + self.selection.move_up(1); + self.output_buffer.update_all_lines(); + } else { + self.cursor.y += 1; + if self.viewport.len() <= self.cursor.y { + let line_wrapped_row = Row::new(self.width); + self.viewport.push(line_wrapped_row); + self.output_buffer.update_line(self.cursor.y); + } + } + } fn clear_lines_above(&mut self) { self.lines_above.clear(); self.scrollback_buffer_lines = self.recalculate_scrollback_buffer_count(); @@ -1546,8 +1532,8 @@ impl Perform for Grid { // Set color index. b"4" => { for chunk in params[1..].chunks(2) { - let index = parse_number(chunk[0]); - let color = xparse_color(chunk[1]); + let index = chunk.get(0).and_then(|index| parse_number(index)); + let color = chunk.get(1).and_then(|color| xparse_color(color)); if let (Some(i), Some(c)) = (index, color) { if self.changed_colors.is_none() { self.changed_colors = Some([None; 256]); @@ -1893,6 +1879,7 @@ impl Perform for Grid { self.add_empty_lines_in_scroll_region(line_count_to_add, pad_character); } else if c == 'G' || c == '`' { let column = next_param_or(1).saturating_sub(1); + let column = std::cmp::min(column, self.width.saturating_sub(1)); self.move_cursor_to_column(column); } else if c == 'g' { let clear_type = next_param_or(0); @@ -1933,8 +1920,9 @@ impl Perform for Grid { } else if c == '@' { let count = next_param_or(1); for _ in 0..count { - // TODO: should this be styled? - self.insert_character_at_cursor_position(EMPTY_TERMINAL_CHARACTER); + let mut pad_character = EMPTY_TERMINAL_CHARACTER; + pad_character.styles = self.cursor.pending_styles; + self.add_character_at_cursor_position(pad_character, true); } } else if c == 'b' { if let Some(c) = self.preceding_char { @@ -2327,9 +2315,10 @@ impl Row { self.columns.push_back(terminal_character); // this is much more performant than remove/insert let character = self.columns.swap_remove_back(absolute_x_index).unwrap(); - let excess_width = character.width.saturating_sub(1); + let excess_width = character.width.saturating_sub(terminal_character.width); for _ in 0..excess_width { - self.columns.insert(absolute_x_index, terminal_character); + self.columns + .insert(absolute_x_index, EMPTY_TERMINAL_CHARACTER); } } self.width = None; @@ -2417,17 +2406,6 @@ impl Row { self.width = None; self.columns = replace_with; } - pub fn replace_beginning_with(&mut self, mut line_part: VecDeque) { - // this assumes line_part has no wide characters - if line_part.len() > self.columns.len() { - self.columns.clear(); - } else { - drop(self.columns.drain(0..line_part.len())); - } - line_part.append(&mut self.columns); - self.width = None; - self.columns = line_part; - } pub fn len(&self) -> usize { self.columns.len() } diff --git a/zellij-server/src/panes/unit/grid_tests.rs b/zellij-server/src/panes/unit/grid_tests.rs index b0e00f9a..eac87284 100644 --- a/zellij-server/src/panes/unit/grid_tests.rs +++ b/zellij-server/src/panes/unit/grid_tests.rs @@ -1792,3 +1792,20 @@ fn terminal_pixel_size_reports_in_unsupported_terminals() { expected, ); } + +#[test] +pub fn ansi_csi_at_sign() { + let mut vte_parser = vte::Parser::new(); + let mut grid = Grid::new( + 51, + 112, + Palette::default(), + Rc::new(RefCell::new(LinkHandler::new())), + Rc::new(RefCell::new(None)), + ); + let content = "foo\u{1b}[2D\u{1b}[2@".as_bytes(); + for byte in content { + vte_parser.advance(&mut grid, *byte); + } + assert_snapshot!(format!("{:?}", grid)); +} diff --git a/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__ansi_csi_at_sign.snap b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__ansi_csi_at_sign.snap new file mode 100644 index 00000000..12ec5d49 --- /dev/null +++ b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__ansi_csi_at_sign.snap @@ -0,0 +1,7 @@ +--- +source: zellij-server/src/panes/./unit/grid_tests.rs +assertion_line: 1810 +expression: "format!(\"{:?}\", grid)" +--- +00 (C): f oo + diff --git a/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__vttest8_1.snap b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__vttest8_1.snap index 4381306b..3e4ded7a 100644 --- a/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__vttest8_1.snap +++ b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__vttest8_1.snap @@ -1,9 +1,9 @@ --- source: zellij-server/src/panes/./unit/grid_tests.rs +assertion_line: 447 expression: "format!(\"{:?}\", grid)" - --- -00 (C): A******************************************************************************BAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA +00 (C): A******************************************************************************BAAAAAAAAAAAAAAAAA 01 (C): 02 (C): 03 (C): Test of 'Insert Mode'. The top line should be 'A*** ... ***B'. Push diff --git a/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__vttest8_2.snap b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__vttest8_2.snap index 6d60398e..f319d10b 100644 --- a/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__vttest8_2.snap +++ b/zellij-server/src/panes/unit/snapshots/zellij_server__panes__grid__grid_tests__vttest8_2.snap @@ -1,9 +1,9 @@ --- source: zellij-server/src/panes/./unit/grid_tests.rs +assertion_line: 465 expression: "format!(\"{:?}\", grid)" - --- -00 (C): ABAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA +00 (C): ABAAAAAAAAAAAAAAAAA 01 (C): 02 (C): 03 (C): Test of 'Delete Character'. The top line should be 'AB'. Push