Fix selection with invisible start and end

This resolves an issue with the selection clamping, where no selection
would be rendered at all when the start was above the viewport while the
end was below it.

4 files changed, 53 insertions, 53 deletions

diff --git a/alacritty_terminal/src/grid/mod.rs b/alacritty_terminal/src/grid/mod.rs
index febdff69..34d989db 100644
--- a/alacritty_terminal/src/grid/mod.rs
+++ b/alacritty_terminal/src/grid/mod.rs
@@ -145,24 +145,21 @@ impl<T: GridCell + PartialEq + Copy> Grid<T> {
         Grid { raw, cols, lines, display_offset: 0, selection: None, max_scroll_limit: scrollback }
     }
 
-    pub fn buffer_to_visible(&self, point: impl Into<Point<usize>>) -> Option<Point<usize>> {
-        let mut point = point.into();
-
-        if point.line < self.display_offset || point.line >= self.display_offset + self.lines.0 {
-            return None;
+    /// Clamp a buffer point to the visible region.
+    pub fn clamp_buffer_to_visible(&self, point: Point<usize>) -> Point {
+        if point.line < self.display_offset {
+            Point::new(self.lines - 1, self.cols - 1)
+        } else if point.line >= self.display_offset + self.lines.0 {
+            Point::new(Line(0), Column(0))
+        } else {
+            // Since edgecases are handled, conversion is identical as visible to buffer
+            self.visible_to_buffer(point.into()).into()
         }
-
-        point.line = self.lines.0 + self.display_offset - point.line - 1;
-
-        Some(point)
     }
 
+    /// Convert viewport relative point to global buffer indexing.
     pub fn visible_to_buffer(&self, point: Point) -> Point<usize> {
-        Point { line: self.visible_line_to_buffer(point.line), col: point.col }
-    }
-
-    fn visible_line_to_buffer(&self, line: Line) -> usize {
-        self.line_to_offset(line) + self.display_offset
+        Point { line: self.lines.0 + self.display_offset - point.line.0 - 1, col: point.col }
     }
 
     /// Update the size of the scrollback history
@@ -453,17 +450,6 @@ impl<T: GridCell + PartialEq + Copy> Grid<T> {
         self.lines = target;
     }
 
-    /// Convert a Line index (active region) to a buffer offset
-    ///
-    /// # Panics
-    ///
-    /// This method will panic if `Line` is larger than the grid dimensions
-    pub fn line_to_offset(&self, line: Line) -> usize {
-        assert!(line < self.num_lines());
-
-        *(self.num_lines() - line - 1)
-    }
-
     #[inline]
     pub fn scroll_down(&mut self, region: &Range<Line>, positions: Line, template: &T) {
         let num_lines = self.num_lines().0;
diff --git a/alacritty_terminal/src/grid/tests.rs b/alacritty_terminal/src/grid/tests.rs
index e4fdad5c..e8f4fb8d 100644
--- a/alacritty_terminal/src/grid/tests.rs
+++ b/alacritty_terminal/src/grid/tests.rs
@@ -37,6 +37,40 @@ impl GridCell for usize {
     }
 }
 
+#[test]
+fn grid_clamp_buffer_point() {
+    let mut grid = Grid::new(Line(10), Column(10), 1_000, 0);
+    grid.display_offset = 5;
+
+    let point = grid.clamp_buffer_to_visible(Point::new(10, Column(3)));
+    assert_eq!(point, Point::new(Line(4), Column(3)));
+
+    let point = grid.clamp_buffer_to_visible(Point::new(15, Column(3)));
+    assert_eq!(point, Point::new(Line(0), Column(0)));
+
+    let point = grid.clamp_buffer_to_visible(Point::new(4, Column(3)));
+    assert_eq!(point, Point::new(Line(9), Column(9)));
+
+    grid.display_offset = 0;
+
+    let point = grid.clamp_buffer_to_visible(Point::new(4, Column(3)));
+    assert_eq!(point, Point::new(Line(5), Column(3)));
+}
+
+#[test]
+fn visible_to_buffer() {
+    let mut grid = Grid::new(Line(10), Column(10), 1_000, 0);
+    grid.display_offset = 5;
+
+    let point = grid.visible_to_buffer(Point::new(Line(4), Column(3)));
+    assert_eq!(point, Point::new(10, Column(3)));
+
+    grid.display_offset = 0;
+
+    let point = grid.visible_to_buffer(Point::new(Line(5), Column(3)));
+    assert_eq!(point, Point::new(4, Column(3)));
+}
+
 // Scroll up moves lines upwards
 #[test]
 fn scroll_up() {
diff --git a/alacritty_terminal/src/index.rs b/alacritty_terminal/src/index.rs
index 51566d8d..56d32003 100644
--- a/alacritty_terminal/src/index.rs
+++ b/alacritty_terminal/src/index.rs
@@ -73,12 +73,11 @@ impl<L> Point<L> {
 
 impl Ord for Point {
     fn cmp(&self, other: &Point) -> Ordering {
-        use std::cmp::Ordering::*;
         match (self.line.cmp(&other.line), self.col.cmp(&other.col)) {
-            (Equal, Equal) => Equal,
-            (Equal, ord) | (ord, Equal) => ord,
-            (Less, _) => Less,
-            (Greater, _) => Greater,
+            (Ordering::Equal, Ordering::Equal) => Ordering::Equal,
+            (Ordering::Equal, ord) | (ord, Ordering::Equal) => ord,
+            (Ordering::Less, _) => Ordering::Less,
+            (Ordering::Greater, _) => Ordering::Greater,
         }
     }
 }
diff --git a/alacritty_terminal/src/term/mod.rs b/alacritty_terminal/src/term/mod.rs
index d545d686..660aeb79 100644
--- a/alacritty_terminal/src/term/mod.rs
+++ b/alacritty_terminal/src/term/mod.rs
@@ -222,9 +222,8 @@ impl<'a, C> RenderableCellsIter<'a, C> {
     ) -> RenderableCellsIter<'b, C> {
         let grid = &term.grid;
         let num_cols = grid.num_cols();
-        let num_lines = grid.num_lines();
 
-        let cursor_offset = grid.line_to_offset(term.cursor.point.line);
+        let cursor_offset = grid.num_lines().0 - term.cursor.point.line.0 - 1;
         let inner = grid.display_iter();
 
         let selection_range = selection.and_then(|span| {
@@ -235,28 +234,14 @@ impl<'a, C> RenderableCellsIter<'a, C> {
             };
 
             // Get on-screen lines of the selection's locations
-            let start = term.buffer_to_visible(span.start);
-            let end = term.buffer_to_visible(span.end);
-
-            // Clamp visible selection to the viewport
-            let (mut start, mut end) = match (start, end) {
-                (Some(start), Some(end)) => (start, end),
-                (Some(start), None) => {
-                    let end = Point::new(num_lines.0 - 1, num_cols - 1);
-                    (start, end)
-                },
-                (None, Some(end)) => {
-                    let start = Point::new(0, Column(0));
-                    (start, end)
-                },
-                (None, None) => return None,
-            };
+            let mut start = grid.clamp_buffer_to_visible(span.start);
+            let mut end = grid.clamp_buffer_to_visible(span.end);
 
             // Trim start/end with partially visible block selection
             start.col = max(limit_start, start.col);
             end.col = min(limit_end, end.col);
 
-            Some(SelectionRange::new(start.into(), end.into(), span.is_block))
+            Some(SelectionRange::new(start, end, span.is_block))
         });
 
         // Load cursor glyph
@@ -1085,10 +1070,6 @@ impl<T> Term<T> {
         self.grid.visible_to_buffer(point)
     }
 
-    pub fn buffer_to_visible(&self, point: impl Into<Point<usize>>) -> Option<Point<usize>> {
-        self.grid.buffer_to_visible(point)
-    }
-
     /// Access to the raw grid data structure
     ///
     /// This is a bit of a hack; when the window is closed, the event processor