Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(compatibility): fix crash and don't allow cursor beyond row width #1349

Merged
merged 2 commits into from
Apr 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 44 additions & 66 deletions zellij-server/src/panes/grid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
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) {
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) => {
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);
}
Expand All @@ -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;
Expand All @@ -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<TerminalCharacter> {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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]);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -2417,17 +2406,6 @@ impl Row {
self.width = None;
self.columns = replace_with;
}
pub fn replace_beginning_with(&mut self, mut line_part: VecDeque<TerminalCharacter>) {
// 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()
}
Expand Down
17 changes: 17 additions & 0 deletions zellij-server/src/panes/unit/grid_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
source: zellij-server/src/panes/./unit/grid_tests.rs
assertion_line: 1810
expression: "format!(\"{:?}\", grid)"
---
00 (C): f oo

Original file line number Diff line number Diff line change
@@ -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 <RETURN>
Expand Down
Original file line number Diff line number Diff line change
@@ -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 <RETURN>
Expand Down