Skip to content

Commit

Permalink
fix(compatibility): full screen scroll region (#915)
Browse files Browse the repository at this point in the history
* fix(compatibility): properly handle scrolling when scroll region is full screen

* style(fmt): make clippy and rustfmt happy
  • Loading branch information
imsnif authored Dec 1, 2021
1 parent 4e5717e commit 464cb6b
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 6 deletions.
61 changes: 61 additions & 0 deletions src/tests/fixtures/scroll_region_full_screen
Original file line number Diff line number Diff line change
@@ -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
Expand Down
34 changes: 28 additions & 6 deletions zellij-server/src/panes/grid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,10 @@ pub struct Grid {
alternative_lines_above_viewport_and_cursor: Option<(VecDeque<Row>, Vec<Row>, Cursor)>,
cursor: Cursor,
saved_cursor_position: Option<Cursor>,
// 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<TerminalCharacter>,
Expand Down Expand Up @@ -936,13 +940,30 @@ 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);
} 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
return;
Expand Down Expand Up @@ -1226,6 +1247,7 @@ impl Grid {
pub fn set_scroll_region(&mut self, top_line_index: usize, bottom_line_index: Option<usize>) {
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;
Expand Down
20 changes: 20 additions & 0 deletions zellij-server/src/panes/unit/grid_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 464cb6b

Please sign in to comment.