From 7ac1ec5e2ff317f918d423c5de0878faf0858103 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Tue, 24 Mar 2020 01:29:07 +0000 Subject: [PATCH] Fix cursor position after alt screen resize This fixes a regression introduced in 4cc6421, which ignored the main grid's cursor when increasing the number of lines available, causing incorrect cursor position after restoring to the primary screen. Additionally another similar bug has been fixed where the grid was not scrolled correctly when shrinking while in the alternate screen. When the grid is resized multiple lines at once, there was also an issue with Alacritty either pulling all lines from history or none at all, instead of mixing both approaches and pulling just what is required. This lead to incorrect cursor positions when the resize could partially make use of history. Fixes #3499. --- CHANGELOG.md | 1 + alacritty_terminal/src/grid/mod.rs | 27 ++++-- alacritty_terminal/src/term/mod.rs | 150 ++++++++++++++++++++++++----- 3 files changed, 147 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e15f8612..96fceadc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Decorations visible when in fullscreen on Wayland - Window size not persisted correctly after fullscreening on macOS - Crash on startup with some locales on X11 +- Shrinking terminal height in alt screen deleting primary screen content ### Removed diff --git a/alacritty_terminal/src/grid/mod.rs b/alacritty_terminal/src/grid/mod.rs index 34d989db..5984950a 100644 --- a/alacritty_terminal/src/grid/mod.rs +++ b/alacritty_terminal/src/grid/mod.rs @@ -205,8 +205,8 @@ impl Grid { } match self.lines.cmp(&lines) { - Ordering::Less => self.grow_lines(lines, template), - Ordering::Greater => self.shrink_lines(lines), + Ordering::Less => self.grow_lines(lines, cursor_pos, template), + Ordering::Greater => self.shrink_lines(lines, cursor_pos, template), Ordering::Equal => (), } @@ -236,17 +236,22 @@ impl Grid { /// Alacritty keeps the cursor at the bottom of the terminal as long as there /// is scrollback available. Once scrollback is exhausted, new lines are /// simply added to the bottom of the screen. - fn grow_lines(&mut self, new_line_count: Line, template: &T) { + fn grow_lines(&mut self, new_line_count: Line, cursor_pos: &mut Point, template: &T) { let lines_added = new_line_count - self.lines; // Need to "resize" before updating buffer self.raw.grow_visible_lines(new_line_count, Row::new(self.cols, template)); self.lines = new_line_count; - // Move existing lines up if there is no scrollback to fill new lines let history_size = self.history_size(); - if lines_added.0 > history_size { - self.scroll_up(&(Line(0)..new_line_count), lines_added - history_size, template); + let from_history = min(history_size, lines_added.0); + + // Move cursor down for all lines pulled from history + cursor_pos.line += from_history; + + if from_history != lines_added.0 { + // Move existing lines up for every line that couldn't be pulled from history + self.scroll_up(&(Line(0)..new_line_count), lines_added - from_history, template); } self.decrease_scroll_limit(*lines_added); @@ -441,11 +446,15 @@ impl Grid { /// of the terminal window. /// /// Alacritty takes the same approach. - fn shrink_lines(&mut self, target: Line) { - let prev = self.lines; + fn shrink_lines(&mut self, target: Line, cursor_pos: &mut Point, template: &T) { + // Scroll up to keep cursor inside the window + let required_scrolling = (cursor_pos.line + 1).saturating_sub(target.0); + if required_scrolling > 0 { + self.scroll_up(&(Line(0)..self.lines), Line(required_scrolling), template); + } self.selection = None; - self.raw.rotate(*prev as isize - *target as isize); + self.raw.rotate((self.lines - target).0 as isize); self.raw.shrink_visible_lines(target); self.lines = target; } diff --git a/alacritty_terminal/src/term/mod.rs b/alacritty_terminal/src/term/mod.rs index 7fbd8dc8..590c7d63 100644 --- a/alacritty_terminal/src/term/mod.rs +++ b/alacritty_terminal/src/term/mod.rs @@ -1146,32 +1146,13 @@ impl Term { num_lines = Line(2); } - // Scroll up to keep cursor in terminal - if self.cursor.point.line >= num_lines { - let lines = self.cursor.point.line - num_lines + 1; - let template = Cell { bg: self.cursor.template.bg, ..Cell::default() }; - self.grid.scroll_up(&(Line(0)..old_lines), lines, &template); - } - - // Scroll up alt grid as well - if self.cursor_save_alt.point.line >= num_lines { - let lines = self.cursor_save_alt.point.line - num_lines + 1; - let template = Cell { bg: self.cursor_save_alt.template.bg, ..Cell::default() }; - self.alt_grid.scroll_up(&(Line(0)..old_lines), lines, &template); - } - - // Move prompt down when growing if scrollback lines are available - if num_lines > old_lines && !self.mode.contains(TermMode::ALT_SCREEN) { - let growage = min(num_lines - old_lines, Line(self.grid.history_size())); - self.cursor.point.line += growage; - } - debug!("New num_cols is {} and num_lines is {}", num_cols, num_lines); - // Resize grids to new size let is_alt = self.mode.contains(TermMode::ALT_SCREEN); let alt_cursor_point = if is_alt { &mut self.cursor_save.point } else { &mut self.cursor_save_alt.point }; + + // Resize grids to new size self.grid.resize(!is_alt, num_lines, num_cols, &mut self.cursor.point, &Cell::default()); self.alt_grid.resize(is_alt, num_lines, num_cols, alt_cursor_point, &Cell::default()); @@ -2183,6 +2164,8 @@ impl IndexMut for TabStops { #[cfg(test)] mod tests { + use super::*; + use std::mem; use crate::ansi::{self, CharsetIndex, Handler, StandardCharset}; @@ -2193,7 +2176,6 @@ mod tests { use crate::index::{Column, Line, Point, Side}; use crate::selection::Selection; use crate::term::cell::{Cell, Flags}; - use crate::term::{SizeInfo, Term}; struct Mock; impl EventListener for Mock { @@ -2362,6 +2344,130 @@ mod tests { assert_eq!(term.grid, scrolled_grid); } + #[test] + fn grow_lines_updates_active_cursor_pos() { + let mut size = SizeInfo { + width: 100.0, + height: 10.0, + cell_width: 1.0, + cell_height: 1.0, + padding_x: 0.0, + padding_y: 0.0, + dpr: 1.0, + }; + let mut term = Term::new(&MockConfig::default(), &size, Clipboard::new_nop(), Mock); + + // Create 10 lines of scrollback + for _ in 0..19 { + term.newline(); + } + assert_eq!(term.grid.history_size(), 10); + assert_eq!(term.cursor.point, Point::new(Line(9), Column(0))); + + // Increase visible lines + size.height = 30.; + term.resize(&size); + + assert_eq!(term.grid.history_size(), 0); + assert_eq!(term.cursor.point, Point::new(Line(19), Column(0))); + } + + #[test] + fn grow_lines_updates_inactive_cursor_pos() { + let mut size = SizeInfo { + width: 100.0, + height: 10.0, + cell_width: 1.0, + cell_height: 1.0, + padding_x: 0.0, + padding_y: 0.0, + dpr: 1.0, + }; + let mut term = Term::new(&MockConfig::default(), &size, Clipboard::new_nop(), Mock); + + // Create 10 lines of scrollback + for _ in 0..19 { + term.newline(); + } + assert_eq!(term.grid.history_size(), 10); + assert_eq!(term.cursor.point, Point::new(Line(9), Column(0))); + + // Enter alt screen + term.set_mode(ansi::Mode::SwapScreenAndSetRestoreCursor); + + // Increase visible lines + size.height = 30.; + term.resize(&size); + + // Leave alt screen + term.unset_mode(ansi::Mode::SwapScreenAndSetRestoreCursor); + + assert_eq!(term.grid().history_size(), 0); + assert_eq!(term.cursor.point, Point::new(Line(19), Column(0))); + } + + #[test] + fn shrink_lines_updates_active_cursor_pos() { + let mut size = SizeInfo { + width: 100.0, + height: 10.0, + cell_width: 1.0, + cell_height: 1.0, + padding_x: 0.0, + padding_y: 0.0, + dpr: 1.0, + }; + let mut term = Term::new(&MockConfig::default(), &size, Clipboard::new_nop(), Mock); + + // Create 10 lines of scrollback + for _ in 0..19 { + term.newline(); + } + assert_eq!(term.grid.history_size(), 10); + assert_eq!(term.cursor.point, Point::new(Line(9), Column(0))); + + // Increase visible lines + size.height = 5.; + term.resize(&size); + + assert_eq!(term.grid().history_size(), 15); + assert_eq!(term.cursor.point, Point::new(Line(4), Column(0))); + } + + #[test] + fn shrink_lines_updates_inactive_cursor_pos() { + let mut size = SizeInfo { + width: 100.0, + height: 10.0, + cell_width: 1.0, + cell_height: 1.0, + padding_x: 0.0, + padding_y: 0.0, + dpr: 1.0, + }; + let mut term = Term::new(&MockConfig::default(), &size, Clipboard::new_nop(), Mock); + + // Create 10 lines of scrollback + for _ in 0..19 { + term.newline(); + } + assert_eq!(term.grid.history_size(), 10); + assert_eq!(term.cursor.point, Point::new(Line(9), Column(0))); + + // Enter alt screen + term.set_mode(ansi::Mode::SwapScreenAndSetRestoreCursor); + + // Increase visible lines + size.height = 5.; + term.resize(&size); + + // Leave alt screen + term.unset_mode(ansi::Mode::SwapScreenAndSetRestoreCursor); + + assert_eq!(term.grid().history_size(), 15); + assert_eq!(term.cursor.point, Point::new(Line(4), Column(0))); + } + #[test] fn window_title() { let size = SizeInfo {