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.
This commit is contained in:
Christian Duerr 2020-03-24 01:29:07 +00:00 committed by GitHub
parent c9c5fbbe2b
commit c35dbc9657
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 147 additions and 31 deletions

View File

@ -94,6 +94,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

View File

@ -205,8 +205,8 @@ impl<T: GridCell + PartialEq + Copy> Grid<T> {
}
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<T: GridCell + PartialEq + Copy> Grid<T> {
/// 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);
@ -437,11 +442,15 @@ impl<T: GridCell + PartialEq + Copy> Grid<T> {
/// 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;
}

View File

@ -1153,32 +1153,13 @@ impl<T> Term<T> {
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());
@ -2299,6 +2280,8 @@ impl IndexMut<Column> for TabStops {
#[cfg(test)]
mod tests {
use super::*;
use std::mem;
use crate::ansi::{self, CharsetIndex, Handler, StandardCharset};
@ -2309,7 +2292,6 @@ mod tests {
use crate::index::{Column, Line, Point, Side};
use crate::selection::{Selection, SelectionType};
use crate::term::cell::{Cell, Flags};
use crate::term::{SizeInfo, Term};
struct Mock;
impl EventListener for Mock {
@ -2495,6 +2477,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 {