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 scrollback offset bugs #11

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/grid.rs
Expand Up @@ -122,7 +122,14 @@ impl Grid {
self.scrollback
.iter()
.skip(scrollback_len - self.scrollback_offset)
.chain(self.rows.iter().take(rows_len - self.scrollback_offset))
// when scrollback_offset > rows_len (e.g. rows = 3,
// scrollback_len = 10, offset = 9) the skip(10 - 9)
// will take 9 rows instead of 3. we need to set
// the upper bound to rows_len (e.g. 3)
.take(rows_len)
// same for rows_len - scrollback_offset (e.g. 3 - 9).
// it'll panic with overflow. we have to saturate the subtraction.
.chain(self.rows.iter().take(rows_len.saturating_sub(self.scrollback_offset)))
}

pub fn drawing_rows(&self) -> impl Iterator<Item = &crate::row::Row> {
Expand Down
24 changes: 24 additions & 0 deletions tests/scroll.rs
@@ -1,3 +1,5 @@
use std::ops::RangeInclusive;

mod helpers;

#[test]
Expand Down Expand Up @@ -110,3 +112,25 @@ fn edge_of_screen() {
b"\x1b[24;75H\x1b[31mfoobar\x1b[24;80H"
);
}

#[test]
fn scrollback_larger_than_rows() {
let mut parser = vt100::Parser::new(3, 20, 10);

parser.process(&gen_nums(1..=10, "\r\n").as_bytes());

// 1. Extra rows returned
parser.screen_mut().set_scrollback(4);
assert_eq!(parser.screen().contents(), gen_nums(4..=6, "\n"));

// 2. Subtraction overflow
parser.screen_mut().set_scrollback(10);
assert_eq!(parser.screen().contents(), gen_nums(1..=4, "\n"));
}

#[cfg(test)]
fn gen_nums(range: RangeInclusive<u8>, join: &str) -> String {
range.map(|num| num.to_string())
.collect::<Vec<String>>()
.join(join)
}