From bdd28f4766a39ec679cc7422d5cff5c6e586ae43 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Fri, 24 Jan 2020 23:57:22 +0100 Subject: [PATCH] Fix selection rotating outside of scrolling region Fixes #2983. --- CHANGELOG.md | 1 + alacritty_terminal/src/grid/mod.rs | 37 +++- alacritty_terminal/src/selection.rs | 317 +++++++++++++++++----------- 3 files changed, 219 insertions(+), 136 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e9019f..75c49bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Backwards bracket selection - Stack overflow when printing shader creation error - Underline position for bitmap fonts +- Selection rotating outside of scrolling region ### Removed diff --git a/alacritty_terminal/src/grid/mod.rs b/alacritty_terminal/src/grid/mod.rs index 9c26fac..1db5b74 100644 --- a/alacritty_terminal/src/grid/mod.rs +++ b/alacritty_terminal/src/grid/mod.rs @@ -492,6 +492,9 @@ impl Grid { #[inline] pub fn scroll_down(&mut self, region: &Range, positions: Line, template: &T) { + let num_lines = self.num_lines().0; + let num_cols = self.num_cols().0; + // Whether or not there is a scrolling region active, as long as it // starts at the top, we can do a full rotation which just involves // changing the start index. @@ -501,9 +504,10 @@ impl Grid { // Rotate the entire line buffer. If there's a scrolling region // active, the bottom lines are restored in the next step. self.raw.rotate_up(*positions); - if let Some(ref mut selection) = self.selection { - selection.rotate(-(*positions as isize)); - } + self.selection = self + .selection + .take() + .and_then(|s| s.rotate(num_lines, num_cols, region, -(*positions as isize))); self.decrease_scroll_limit(*positions); @@ -518,6 +522,12 @@ impl Grid { self.raw[i].reset(&template); } } else { + // Rotate selection to track content + self.selection = self + .selection + .take() + .and_then(|s| s.rotate(num_lines, num_cols, region, -(*positions as isize))); + // Subregion rotation for line in IndexRange((region.start + positions)..region.end).rev() { self.raw.swap_lines(line, line - positions); @@ -533,11 +543,13 @@ impl Grid { /// /// This is the performance-sensitive part of scrolling. pub fn scroll_up(&mut self, region: &Range, positions: Line, template: &T) { + let num_lines = self.num_lines().0; + let num_cols = self.num_cols().0; + if region.start == Line(0) { // Update display offset when not pinned to active area if self.display_offset != 0 { - self.display_offset = - min(self.display_offset + *positions, self.len() - self.num_lines().0); + self.display_offset = min(self.display_offset + *positions, self.len() - num_lines); } self.increase_scroll_limit(*positions, template); @@ -545,15 +557,16 @@ impl Grid { // Rotate the entire line buffer. If there's a scrolling region // active, the bottom lines are restored in the next step. self.raw.rotate(-(*positions as isize)); - if let Some(ref mut selection) = self.selection { - selection.rotate(*positions as isize); - } + self.selection = self + .selection + .take() + .and_then(|s| s.rotate(num_lines, num_cols, region, *positions as isize)); // This next loop swaps "fixed" lines outside of a scroll region // back into place after the rotation. The work is done in buffer- // space rather than terminal-space to avoid redundant // transformations. - let fixed_lines = *self.num_lines() - *region.end; + let fixed_lines = num_lines - *region.end; for i in 0..fixed_lines { self.raw.swap(i, i + *positions); @@ -566,6 +579,12 @@ impl Grid { self.raw[i + fixed_lines].reset(&template); } } else { + // Rotate selection to track content + self.selection = self + .selection + .take() + .and_then(|s| s.rotate(num_lines, num_cols, region, *positions as isize)); + // Subregion rotation for line in IndexRange(region.start..(region.end - positions)) { self.raw.swap_lines(line, line + positions); diff --git a/alacritty_terminal/src/selection.rs b/alacritty_terminal/src/selection.rs index f23e646..92335e1 100644 --- a/alacritty_terminal/src/selection.rs +++ b/alacritty_terminal/src/selection.rs @@ -22,12 +22,12 @@ use std::convert::TryFrom; use std::mem; use std::ops::Range; -use crate::index::{Column, Point, Side}; +use crate::index::{Column, Line, Point, Side}; use crate::term::cell::Flags; use crate::term::{Search, Term}; /// A Point and side within that point. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Copy, Clone, PartialEq)] pub struct Anchor { point: Point, side: Side, @@ -66,87 +66,147 @@ impl SelectionRange { } } +/// Different kinds of selection. +#[derive(Debug, Copy, Clone, PartialEq)] +enum SelectionType { + Simple, + Block, + Semantic, + Lines, +} + /// Describes a region of a 2-dimensional area. /// -/// Used to track a text selection. There are three supported modes, each with its own constructor: -/// [`simple`], [`semantic`], and [`lines`]. The [`simple`] mode precisely tracks which cells are -/// selected without any expansion. [`semantic`] mode expands the initial selection to the nearest -/// semantic escape char in either direction. [`lines`] will always select entire lines. +/// Used to track a text selection. There are four supported modes, each with its own constructor: +/// [`simple`], [`block`], [`semantic`], and [`lines`]. The [`simple`] mode precisely tracks which +/// cells are selected without any expansion. [`block`] will select rectangular regions. +/// [`semantic`] mode expands the initial selection to the nearest semantic escape char in either +/// direction. [`lines`] will always select entire lines. /// -/// Calls to [`update`] operate different based on the selection kind. The [`simple`] mode does -/// nothing special, simply tracks points and sides. [`semantic`] will continue to expand out to -/// semantic boundaries as the selection point changes. Similarly, [`lines`] will always expand the -/// new point to encompass entire lines. +/// Calls to [`update`] operate different based on the selection kind. The [`simple`] and [`block`] +/// mode do nothing special, simply track points and sides. [`semantic`] will continue to expand +/// out to semantic boundaries as the selection point changes. Similarly, [`lines`] will always +/// expand the new point to encompass entire lines. /// /// [`simple`]: enum.Selection.html#method.simple +/// [`block`]: enum.Selection.html#method.block /// [`semantic`]: enum.Selection.html#method.semantic /// [`lines`]: enum.Selection.html#method.lines /// [`update`]: enum.Selection.html#method.update #[derive(Debug, Clone, PartialEq)] -pub enum Selection { - Simple { - /// The region representing start and end of cursor movement. - region: Range, - }, - Block { - /// The region representing start and end of cursor movement. - region: Range, - }, - Semantic { - /// The region representing start and end of cursor movement. - region: Range>, - }, - Lines { - /// The region representing start and end of cursor movement. - region: Range>, - }, +pub struct Selection { + region: Range, + ty: SelectionType, } impl Selection { pub fn simple(location: Point, side: Side) -> Selection { - Selection::Simple { + Self { region: Range { start: Anchor::new(location, side), end: Anchor::new(location, side) }, + ty: SelectionType::Simple, } } pub fn block(location: Point, side: Side) -> Selection { - Selection::Block { + Self { region: Range { start: Anchor::new(location, side), end: Anchor::new(location, side) }, + ty: SelectionType::Block, } } - pub fn semantic(point: Point) -> Selection { - Selection::Semantic { region: Range { start: point, end: point } } + pub fn semantic(location: Point) -> Selection { + Self { + region: Range { + start: Anchor::new(location, Side::Left), + end: Anchor::new(location, Side::Right), + }, + ty: SelectionType::Semantic, + } } - pub fn lines(point: Point) -> Selection { - Selection::Lines { region: Range { start: point, end: point } } + pub fn lines(location: Point) -> Selection { + Self { + region: Range { + start: Anchor::new(location, Side::Left), + end: Anchor::new(location, Side::Right), + }, + ty: SelectionType::Lines, + } } pub fn update(&mut self, location: Point, side: Side) { - let (_, end) = self.points_mut(); - *end = location; - - if let Some((_, end_side)) = self.sides_mut() { - *end_side = side; - } + self.region.end.point = location; + self.region.end.side = side; } - pub fn rotate(&mut self, offset: isize) { - let (start, end) = self.points_mut(); - start.line = usize::try_from(start.line as isize + offset).unwrap_or(0); - end.line = usize::try_from(end.line as isize + offset).unwrap_or(0); + pub fn rotate( + mut self, + num_lines: usize, + num_cols: usize, + scrolling_region: &Range, + offset: isize, + ) -> Option { + // Convert scrolling region from viewport to buffer coordinates + let region_start = num_lines - scrolling_region.start.0; + let region_end = num_lines - scrolling_region.end.0; + + let (mut start, mut end) = (&mut self.region.start, &mut self.region.end); + if Self::points_need_swap(start.point, end.point) { + mem::swap(&mut start, &mut end); + } + + // Rotate start of selection + if (start.point.line < region_start || region_start == num_lines) + && start.point.line >= region_end + { + start.point.line = usize::try_from(start.point.line as isize + offset).unwrap_or(0); + + // If end is within the same region, delete selection once start rotates out + if start.point.line < region_end && end.point.line >= region_end { + return None; + } + + // Clamp selection to start of region + if start.point.line >= region_start && region_start != num_lines { + if self.ty != SelectionType::Block { + start.point.col = Column(0); + start.side = Side::Left; + } + start.point.line = region_start - 1; + } + } + + // Rotate end of selection + if (end.point.line < region_start || region_start == num_lines) + && end.point.line >= region_end + { + end.point.line = usize::try_from(end.point.line as isize + offset).unwrap_or(0); + + // Delete selection if end has overtaken the start + if end.point.line > start.point.line { + return None; + } + + // Clamp selection to end of region + if end.point.line < region_end { + if self.ty != SelectionType::Block { + end.point.col = Column(num_cols - 1); + end.side = Side::Right; + } + end.point.line = region_end; + } + } + + Some(self) } pub fn is_empty(&self) -> bool { - match self { - Selection::Simple { ref region } => { - let (start, end) = - if Selection::points_need_swap(region.start.point, region.end.point) { - (®ion.end, ®ion.start) - } else { - (®ion.start, ®ion.end) - }; + match self.ty { + SelectionType::Simple => { + let (mut start, mut end) = (self.region.start, self.region.end); + if Selection::points_need_swap(start.point, end.point) { + mem::swap(&mut start, &mut end); + } // Simple selection is empty when the points are identical // or two adjacent cells have the sides right -> left @@ -156,7 +216,9 @@ impl Selection { && (start.point.line == end.point.line) && start.point.col + 1 == end.point.col) }, - Selection::Block { region: Range { ref start, ref end } } => { + SelectionType::Block => { + let (start, end) = (self.region.start, self.region.end); + // Block selection is empty when the points' columns and sides are identical // or two cells with adjacent columns have the sides right -> left, // regardless of their lines @@ -168,7 +230,7 @@ impl Selection { && start.side == Side::Left && end.side == Side::Right) }, - Selection::Semantic { .. } | Selection::Lines { .. } => false, + SelectionType::Semantic | SelectionType::Lines => false, } } @@ -177,29 +239,21 @@ impl Selection { let grid = term.grid(); let num_cols = grid.num_cols(); - // Get selection boundaries - let points = self.points(); - let (start, end) = (*points.0, *points.1); - - // Get selection sides, falling back to `Side::Left` if it will not be used - let sides = self.sides().unwrap_or((&Side::Left, &Side::Left)); - let (start_side, end_side) = (*sides.0, *sides.1); - // Order start above the end - let (start, end) = if Self::points_need_swap(start, end) { - (Anchor { point: end, side: end_side }, Anchor { point: start, side: start_side }) - } else { - (Anchor { point: start, side: start_side }, Anchor { point: end, side: end_side }) - }; + let (mut start, mut end) = (self.region.start, self.region.end); + if Self::points_need_swap(start.point, end.point) { + mem::swap(&mut start, &mut end); + } // Clamp to inside the grid buffer - let (start, end) = Self::grid_clamp(start, end, self.is_block(), grid.len()).ok()?; + let is_block = self.ty == SelectionType::Block; + let (start, end) = Self::grid_clamp(start, end, is_block, grid.len()).ok()?; - let range = match self { - Self::Simple { .. } => self.range_simple(start, end, num_cols), - Self::Block { .. } => self.range_block(start, end), - Self::Semantic { .. } => Self::range_semantic(term, start.point, end.point), - Self::Lines { .. } => Self::range_lines(term, start.point, end.point), + let range = match self.ty { + SelectionType::Simple => self.range_simple(start, end, num_cols), + SelectionType::Block => self.range_block(start, end), + SelectionType::Semantic => Self::range_semantic(term, start.point, end.point), + SelectionType::Lines => Self::range_lines(term, start.point, end.point), }; // Expand selection across fullwidth cells @@ -376,53 +430,6 @@ impl Selection { Some(SelectionRange { start: start.point, end: end.point, is_block: true }) } - - fn points(&self) -> (&Point, &Point) { - match self { - Self::Simple { ref region } | Self::Block { ref region } => { - (®ion.start.point, ®ion.end.point) - }, - Self::Semantic { ref region } | Self::Lines { ref region } => { - (®ion.start, ®ion.end) - }, - } - } - - fn points_mut(&mut self) -> (&mut Point, &mut Point) { - match self { - Self::Simple { ref mut region } | Self::Block { ref mut region } => { - (&mut region.start.point, &mut region.end.point) - }, - Self::Semantic { ref mut region } | Self::Lines { ref mut region } => { - (&mut region.start, &mut region.end) - }, - } - } - - fn sides(&self) -> Option<(&Side, &Side)> { - match self { - Self::Simple { ref region } | Self::Block { ref region } => { - Some((®ion.start.side, ®ion.end.side)) - }, - Self::Semantic { .. } | Self::Lines { .. } => None, - } - } - - fn sides_mut(&mut self) -> Option<(&mut Side, &mut Side)> { - match self { - Self::Simple { ref mut region } | Self::Block { ref mut region } => { - Some((&mut region.start.side, &mut region.end.side)) - }, - Self::Semantic { .. } | Self::Lines { .. } => None, - } - } - - fn is_block(&self) -> bool { - match self { - Self::Block { .. } => true, - _ => false, - } - } } /// Tests for selection. @@ -572,11 +579,13 @@ mod test { #[test] fn line_selection() { + let num_lines = 10; + let num_cols = 5; let mut selection = Selection::lines(Point::new(0, Column(1))); selection.update(Point::new(5, Column(1)), Side::Right); - selection.rotate(7); + selection = selection.rotate(num_lines, num_cols, &(Line(0)..Line(num_lines)), 7).unwrap(); - assert_eq!(selection.to_range(&term(5, 10)).unwrap(), SelectionRange { + assert_eq!(selection.to_range(&term(num_cols, num_lines)).unwrap(), SelectionRange { start: Point::new(9, Column(0)), end: Point::new(7, Column(4)), is_block: false, @@ -585,11 +594,13 @@ mod test { #[test] fn semantic_selection() { + let num_lines = 10; + let num_cols = 5; let mut selection = Selection::semantic(Point::new(0, Column(3))); selection.update(Point::new(5, Column(1)), Side::Right); - selection.rotate(7); + selection = selection.rotate(num_lines, num_cols, &(Line(0)..Line(num_lines)), 7).unwrap(); - assert_eq!(selection.to_range(&term(5, 10)).unwrap(), SelectionRange { + assert_eq!(selection.to_range(&term(num_cols, num_lines)).unwrap(), SelectionRange { start: Point::new(9, Column(0)), end: Point::new(7, Column(3)), is_block: false, @@ -598,11 +609,13 @@ mod test { #[test] fn simple_selection() { + let num_lines = 10; + let num_cols = 5; let mut selection = Selection::simple(Point::new(0, Column(3)), Side::Right); selection.update(Point::new(5, Column(1)), Side::Right); - selection.rotate(7); + selection = selection.rotate(num_lines, num_cols, &(Line(0)..Line(num_lines)), 7).unwrap(); - assert_eq!(selection.to_range(&term(5, 10)).unwrap(), SelectionRange { + assert_eq!(selection.to_range(&term(num_cols, num_lines)).unwrap(), SelectionRange { start: Point::new(9, Column(0)), end: Point::new(7, Column(3)), is_block: false, @@ -611,11 +624,13 @@ mod test { #[test] fn block_selection() { + let num_lines = 10; + let num_cols = 5; let mut selection = Selection::block(Point::new(0, Column(3)), Side::Right); selection.update(Point::new(5, Column(1)), Side::Right); - selection.rotate(7); + selection = selection.rotate(num_lines, num_cols, &(Line(0)..Line(num_lines)), 7).unwrap(); - assert_eq!(selection.to_range(&term(5, 10)).unwrap(), SelectionRange { + assert_eq!(selection.to_range(&term(num_cols, num_lines)).unwrap(), SelectionRange { start: Point::new(9, Column(2)), end: Point::new(7, Column(3)), is_block: true @@ -667,4 +682,52 @@ mod test { selection.update(Point::new(1, Column(1)), Side::Right); assert!(!selection.is_empty()); } + + #[test] + fn rotate_in_region_up() { + let num_lines = 10; + let num_cols = 5; + let mut selection = Selection::simple(Point::new(2, Column(3)), Side::Right); + selection.update(Point::new(5, Column(1)), Side::Right); + selection = + selection.rotate(num_lines, num_cols, &(Line(1)..Line(num_lines - 1)), 4).unwrap(); + + assert_eq!(selection.to_range(&term(num_cols, num_lines)).unwrap(), SelectionRange { + start: Point::new(8, Column(0)), + end: Point::new(6, Column(3)), + is_block: false, + }); + } + + #[test] + fn rotate_in_region_down() { + let num_lines = 10; + let num_cols = 5; + let mut selection = Selection::simple(Point::new(5, Column(3)), Side::Right); + selection.update(Point::new(8, Column(1)), Side::Left); + selection = + selection.rotate(num_lines, num_cols, &(Line(1)..Line(num_lines - 1)), -5).unwrap(); + + assert_eq!(selection.to_range(&term(num_cols, num_lines)).unwrap(), SelectionRange { + start: Point::new(3, Column(1)), + end: Point::new(1, Column(num_cols - 1)), + is_block: false, + }); + } + + #[test] + fn rotate_in_region_up_block() { + let num_lines = 10; + let num_cols = 5; + let mut selection = Selection::block(Point::new(2, Column(3)), Side::Right); + selection.update(Point::new(5, Column(1)), Side::Right); + selection = + selection.rotate(num_lines, num_cols, &(Line(1)..Line(num_lines - 1)), 4).unwrap(); + + assert_eq!(selection.to_range(&term(num_cols, num_lines)).unwrap(), SelectionRange { + start: Point::new(8, Column(2)), + end: Point::new(6, Column(3)), + is_block: true, + }); + } }