diff --git a/CHANGELOG.md b/CHANGELOG.md index ec786ea3..4e5543b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Regression in rendering performance with dense grids since 0.6.0 - Crash/Freezes with partially visible fullwidth characters due to alt screen resize -- Incorrect vi cursor position after invoking `ScrollPageHalfUp` action +- Incorrect vi cursor position after invoking `ScrollPage*` action - Slow PTY read performance with extremely dense grids - Crash when resizing during vi mode diff --git a/alacritty_terminal/src/term/mod.rs b/alacritty_terminal/src/term/mod.rs index f25bf1ac..ccec0166 100644 --- a/alacritty_terminal/src/term/mod.rs +++ b/alacritty_terminal/src/term/mod.rs @@ -1932,6 +1932,62 @@ mod tests { use crate::selection::{Selection, SelectionType}; use crate::term::cell::{Cell, Flags}; + #[test] + fn scroll_display_page_up() { + let size = SizeInfo::new(5., 10., 1.0, 1.0, 0.0, 0.0, false); + let mut term = Term::new(&MockConfig::default(), size, ()); + + // Create 11 lines of scrollback. + for _ in 0..20 { + term.newline(); + } + + // Scrollable amount to top is 11. + term.scroll_display(Scroll::PageUp); + assert_eq!(term.vi_mode_cursor.point, Point::new(Line(-1), Column(0))); + assert_eq!(term.grid.display_offset(), 10); + + // Scrollable amount to top is 1. + term.scroll_display(Scroll::PageUp); + assert_eq!(term.vi_mode_cursor.point, Point::new(Line(-2), Column(0))); + assert_eq!(term.grid.display_offset(), 11); + + // Scrollable amount to top is 0. + term.scroll_display(Scroll::PageUp); + assert_eq!(term.vi_mode_cursor.point, Point::new(Line(-2), Column(0))); + assert_eq!(term.grid.display_offset(), 11); + } + + #[test] + fn scroll_display_page_down() { + let size = SizeInfo::new(5., 10., 1.0, 1.0, 0.0, 0.0, false); + let mut term = Term::new(&MockConfig::default(), size, ()); + + // Create 11 lines of scrollback. + for _ in 0..20 { + term.newline(); + } + + // Change display_offset to topmost. + term.grid_mut().scroll_display(Scroll::Top); + term.vi_mode_cursor = ViModeCursor::new(Point::new(Line(-11), Column(0))); + + // Scrollable amount to bottom is 11. + term.scroll_display(Scroll::PageDown); + assert_eq!(term.vi_mode_cursor.point, Point::new(Line(-1), Column(0))); + assert_eq!(term.grid.display_offset(), 1); + + // Scrollable amount to bottom is 1. + term.scroll_display(Scroll::PageDown); + assert_eq!(term.vi_mode_cursor.point, Point::new(Line(0), Column(0))); + assert_eq!(term.grid.display_offset(), 0); + + // Scrollable amount to bottom is 0. + term.scroll_display(Scroll::PageDown); + assert_eq!(term.vi_mode_cursor.point, Point::new(Line(0), Column(0))); + assert_eq!(term.grid.display_offset(), 0); + } + #[test] fn semantic_selection_works() { let size = SizeInfo::new(5., 3., 1.0, 1.0, 0.0, 0.0, false); diff --git a/alacritty_terminal/src/vi_mode.rs b/alacritty_terminal/src/vi_mode.rs index 6f370642..5bf5eaed 100644 --- a/alacritty_terminal/src/vi_mode.rs +++ b/alacritty_terminal/src/vi_mode.rs @@ -1,4 +1,4 @@ -use std::cmp::{max, min}; +use std::cmp::min; use alacritty_config_derive::ConfigDeserialize; @@ -160,21 +160,11 @@ impl ViModeCursor { /// Get target cursor point for vim-like page movement. #[must_use = "this returns the result of the operation, without modifying the original"] pub fn scroll(mut self, term: &Term, lines: i32) -> Self { - // Check number of lines the cursor needs to be moved. - let overscroll = if lines > 0 { - let max_scroll = term.history_size() - term.grid().display_offset(); - max(0, lines - max_scroll as i32) - } else { - let max_scroll = term.grid().display_offset(); - min(0, lines + max_scroll as i32) - }; - // Clamp movement to within visible region. - let line = (self.point.line - overscroll).grid_clamp(term, Boundary::Grid); + let line = (self.point.line - lines).grid_clamp(term, Boundary::Grid); // Find the first occupied cell after scrolling has been performed. - let target_line = (self.point.line - lines).grid_clamp(term, Boundary::Grid); - let column = first_occupied_in_line(term, target_line).unwrap_or_default().column; + let column = first_occupied_in_line(term, line).unwrap_or_default().column; // Move cursor. self.point = Point::new(line, column); @@ -388,6 +378,7 @@ fn is_boundary(term: &Term, point: Point, direction: Direction) -> bool { mod tests { use super::*; + use crate::ansi::Handler; use crate::config::MockConfig; use crate::index::{Column, Line}; use crate::term::{SizeInfo, Term}; @@ -756,4 +747,73 @@ mod tests { cursor = cursor.motion(&mut term, ViMotion::WordLeft); assert_eq!(cursor.point, Point::new(Line(0), Column(0))); } + + #[test] + fn scroll_simple() { + let mut term = term(); + + // Create 1 line of scrollback. + for _ in 0..20 { + term.newline(); + } + + let mut cursor = ViModeCursor::new(Point::new(Line(0), Column(0))); + + cursor = cursor.scroll(&term, -1); + assert_eq!(cursor.point, Point::new(Line(1), Column(0))); + + cursor = cursor.scroll(&term, 1); + assert_eq!(cursor.point, Point::new(Line(0), Column(0))); + + cursor = cursor.scroll(&term, 1); + assert_eq!(cursor.point, Point::new(Line(-1), Column(0))); + } + + #[test] + fn scroll_over_top() { + let mut term = term(); + + // Create 40 lines of scrollback. + for _ in 0..59 { + term.newline(); + } + + let mut cursor = ViModeCursor::new(Point::new(Line(19), Column(0))); + + cursor = cursor.scroll(&term, 20); + assert_eq!(cursor.point, Point::new(Line(-1), Column(0))); + + cursor = cursor.scroll(&term, 20); + assert_eq!(cursor.point, Point::new(Line(-21), Column(0))); + + cursor = cursor.scroll(&term, 20); + assert_eq!(cursor.point, Point::new(Line(-40), Column(0))); + + cursor = cursor.scroll(&term, 20); + assert_eq!(cursor.point, Point::new(Line(-40), Column(0))); + } + + #[test] + fn scroll_over_bottom() { + let mut term = term(); + + // Create 40 lines of scrollback. + for _ in 0..59 { + term.newline(); + } + + let mut cursor = ViModeCursor::new(Point::new(Line(-40), Column(0))); + + cursor = cursor.scroll(&term, -20); + assert_eq!(cursor.point, Point::new(Line(-20), Column(0))); + + cursor = cursor.scroll(&term, -20); + assert_eq!(cursor.point, Point::new(Line(0), Column(0))); + + cursor = cursor.scroll(&term, -20); + assert_eq!(cursor.point, Point::new(Line(19), Column(0))); + + cursor = cursor.scroll(&term, -20); + assert_eq!(cursor.point, Point::new(Line(19), Column(0))); + } }