Support selections with scrolling buffer

Selections now *mostly* work. They move as the buffer scrolls, copying
works as it should, and it looks like the different selection modes
behave properly as well.

The new Selection implementation uses buffer coordinates instead of
screen coordinates. This leads to doing a transform from mouse input to
update the selection, and back to screen coordinates when displaying the
selection. Scrolling the selection is fast because the grid is already
operating in buffer coordinates.

There are several bugs to address:

* A _partially_ visible selection will lead to a crash since the drawing
  routine converts selection coordinates to screen coordinates. The
  solution will be to clip the coordinates at draw time.
* A selection scrolling off the buffer in either direction leads to
  indexing out-of-bounds. The solution again is to clip, but this needs
  to be done within Selection::rotate by passing a max limit. It may
  also need a return type to indicate that the selection is no longer
  visible and should be discarded.
* A selection scrolling out of a logical scrolling region is not
  clipped. A temporary and robust workaround is to simply discard the
  selection in the case of scrolling in a region.

wip selections

fix issue with line selection

selection mostly working

need to support selection not being on the screen at draw time

Fix selection_to_string

Uncomment tests

src/event.rs

@@ -81,34 +81,33 @@ impl<'a, N: Notify + 'a> input::ActionContext for ActionContext<'a, N> {
     }
 
     fn update_selection(&mut self, point: Point, side: Side) {
+        self.terminal.dirty = true;
+        let point = self.terminal.visible_to_buffer(point);
 
         // Update selection if one exists
-        let mut had_selection = false; // borrowck
         if let Some(ref mut selection) = *self.terminal.selection_mut() {
             selection.update(point, side);
-            had_selection = true;
-        }
-
-        if had_selection { // borrowck
-            self.terminal.dirty = true;
             return;
         }
 
         // Otherwise, start a regular selection
-        self.simple_selection(point, side);
+        *self.terminal.selection_mut() = Some(Selection::simple(point, side));
     }
 
     fn simple_selection(&mut self, point: Point, side: Side) {
+        let point = self.terminal.visible_to_buffer(point);
         *self.terminal.selection_mut() = Some(Selection::simple(point, side));
         self.terminal.dirty = true;
     }
 
     fn semantic_selection(&mut self, point: Point) {
+        let point = self.terminal.visible_to_buffer(point);
         *self.terminal.selection_mut() = Some(Selection::semantic(point, &*self.terminal));
         self.terminal.dirty = true;
     }
 
     fn line_selection(&mut self, point: Point) {
+        let point = self.terminal.visible_to_buffer(point);
         *self.terminal.selection_mut() = Some(Selection::lines(point));
         self.terminal.dirty = true;
     }

src/grid/mod.rs

@@ -17,7 +17,7 @@
 use std::cmp::{min, max, Ordering};
 use std::ops::{Deref, Range, Index, IndexMut, RangeTo, RangeFrom, RangeFull};
 
-use index::{self, Point, Line, Column, IndexRange, RangeInclusive};
+use index::{self, Point, Line, Column, IndexRange};
 use selection::Selection;
 
 mod row;
@@ -29,11 +29,6 @@ mod tests;
 mod storage;
 use self::storage::Storage;
 
-/// Convert a type to a linear index range.
-pub trait ToRange {
-    fn to_range(&self) -> RangeInclusive<index::Linear>;
-}
-
 /// Bidirection iterator
 pub trait BidirectionalIterator: Iterator {
     fn prev(&mut self) -> Option<Self::Item>;
@@ -105,8 +100,17 @@ pub struct Grid<T> {
 }
 
 pub struct GridIterator<'a, T: 'a> {
+    /// Immutable grid reference
     grid: &'a Grid<T>,
-    pub cur: Point,
+
+    /// Current position of the iterator within the grid.
+    pub cur: Point<usize>,
+
+    /// Bottom of screen (buffer)
+    bot: usize,
+
+    /// Top of screen (buffer)
+    top: usize,
 }
 
 impl<T: Copy + Clone> Grid<T> {
@@ -135,6 +139,28 @@ impl<T: Copy + Clone> Grid<T> {
         }
     }
 
+    pub fn visible_to_buffer(&self, point: Point) -> Point<usize> {
+        Point {
+            line: self.visible_line_to_buffer(point.line),
+            col: point.col
+        }
+    }
+
+    pub fn buffer_to_visible(&self, point: Point<usize>) -> Point {
+        Point {
+            line: self.buffer_line_to_visible(point.line),
+            col: point.col
+        }
+    }
+
+    pub fn buffer_line_to_visible(&self, line: usize) -> Line {
+        self.offset_to_line(line - self.display_offset)
+    }
+
+    pub fn visible_line_to_buffer(&self, line: Line) -> usize {
+        self.line_to_offset(line) + self.display_offset
+    }
+
     pub fn scroll_display(&mut self, count: isize) {
         self.display_offset = min(
                 max((self.display_offset as isize) + count, 0isize) as usize,
@@ -224,6 +250,7 @@ impl<T: Copy + Clone> Grid<T> {
 
         let prev = self.lines;
 
+        self.selection = None;
         self.raw.rotate(*prev as isize - *target as isize);
         self.raw.set_visible_lines(target);
         self.lines = target;
@@ -240,6 +267,12 @@ impl<T: Copy + Clone> Grid<T> {
         *(self.num_lines() - line - 1)
     }
 
+    pub fn offset_to_line(&self, offset: usize) -> Line {
+        assert!(offset < *self.num_lines());
+
+        self.lines - offset - 1
+    }
+
     #[inline]
     pub fn scroll_down(&mut self, region: &Range<index::Line>, positions: index::Line) {
         // Whether or not there is a scrolling region active, as long as it
@@ -251,6 +284,9 @@ impl<T: Copy + Clone> Grid<T> {
             // 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.decrease_scroll_limit(*positions);
 
@@ -292,6 +328,9 @@ impl<T: Copy + Clone> Grid<T> {
             // 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);
+            }
 
             // Now, restore any lines outside the scroll region
             for idx in (*region.end .. *self.num_lines()).rev() {
@@ -334,10 +373,12 @@ impl<T> Grid<T> {
         self.cols
     }
 
-    pub fn iter_from(&self, point: Point) -> GridIterator<T> {
+    pub fn iter_from(&self, point: Point<usize>) -> GridIterator<T> {
         GridIterator {
             grid: self,
             cur: point,
+            bot: self.display_offset,
+            top: self.display_offset + *self.num_lines() - 1,
         }
     }
 
@@ -369,15 +410,12 @@ impl<'a, T> Iterator for GridIterator<'a, T> {
     type Item = &'a T;
 
     fn next(&mut self) -> Option<Self::Item> {
-        let last_line = self.grid.num_lines() - Line(1);
         let last_col = self.grid.num_cols() - Column(1);
         match self.cur {
-            Point { line, col } if
+            Point { line, col } if (line == self.bot) && (col == last_col) => None,
-                (line == last_line) &&
-                (col == last_col) => None,
             Point { col, .. } if
                 (col == last_col) => {
-                self.cur.line += Line(1);
+                self.cur.line -= 1;
                 self.cur.col = Column(0);
                 Some(&self.grid[self.cur.line][self.cur.col])
             },
@@ -394,9 +432,9 @@ impl<'a, T> BidirectionalIterator for GridIterator<'a, T> {
         let num_cols = self.grid.num_cols();
 
         match self.cur {
-            Point { line: Line(0), col: Column(0) } => None,
+            Point { line, col: Column(0) } if line == self.top => None,
             Point { col: Column(0), .. } => {
-                self.cur.line -= Line(1);
+                self.cur.line += 1;
                 self.cur.col = num_cols - Column(1);
                 Some(&self.grid[self.cur.line][self.cur.col])
             },

src/index.rs

@@ -28,13 +28,13 @@ pub enum Side {
 
 /// Index in the grid using row, column notation
 #[derive(Debug, Clone, Copy, Default, Eq, PartialEq, Serialize, Deserialize, PartialOrd)]
-pub struct Point {
+pub struct Point<L=Line> {
-    pub line: Line,
+    pub line: L,
     pub col: Column,
 }
 
-impl Point {
+impl<L> Point<L> {
-    pub fn new(line: Line, col: Column) -> Point {
+    pub fn new(line: L, col: Column) -> Point<L> {
         Point { line, col }
     }
 }

src/selection.rs

@@ -21,8 +21,7 @@
 use std::cmp::{min, max};
 use std::ops::Range;
 
-use index::{Point, Column, RangeInclusive, Side, Linear, Line};
+use index::{Point, Column, Side};
-use grid::ToRange;
 
 /// Describes a region of a 2-dimensional area
 ///
@@ -47,32 +46,32 @@ pub enum Selection {
     },
     Semantic {
         /// The region representing start and end of cursor movement
-        region: Range<Point>,
+        region: Range<Point<usize>>,
 
         /// When beginning a semantic selection, the grid is searched around the
         /// initial point to find semantic escapes, and this initial expansion
         /// marks those points.
-        initial_expansion: Range<Point>
+        initial_expansion: Range<Point<usize>>
     },
     Lines {
         /// The region representing start and end of cursor movement
-        region: Range<Point>,
+        region: Range<Point<usize>>,
 
         /// The line under the initial point. This is always selected regardless
         /// of which way the cursor is moved.
-        initial_line: Line
+        initial_line: usize
     }
 }
 
 /// A Point and side within that point.
 #[derive(Debug, Clone)]
 pub struct Anchor {
-    point: Point,
+    point: Point<usize>,
     side: Side,
 }
 
 impl Anchor {
-    fn new(point: Point, side: Side) -> Anchor {
+    fn new(point: Point<usize>, side: Side) -> Anchor {
         Anchor { point, side }
     }
 }
@@ -83,9 +82,9 @@ impl Anchor {
 /// points are two dimensional indices.
 pub trait SemanticSearch {
     /// Find the nearest semantic boundary _to the left_ of provided point.
-    fn semantic_search_left(&self, _: Point) -> Point;
+    fn semantic_search_left(&self, _: Point<usize>) -> Point<usize>;
     /// Find the nearest semantic boundary _to the point_ of provided point.
-    fn semantic_search_right(&self, _: Point) -> Point;
+    fn semantic_search_right(&self, _: Point<usize>) -> Point<usize>;
 }
 
 /// A type that has 2-dimensional boundaries
@@ -95,7 +94,7 @@ pub trait Dimensions {
 }
 
 impl Selection {
-    pub fn simple(location: Point, side: Side) -> Selection {
+    pub fn simple(location: Point<usize>, side: Side) -> Selection {
         Selection::Simple {
             region: Range {
                 start: Anchor::new(location, side),
@@ -104,7 +103,27 @@ impl Selection {
         }
     }
 
-    pub fn semantic<G: SemanticSearch>(point: Point, grid: &G) -> Selection {
+    pub fn rotate(&mut self, offset: isize) {
+        match *self {
+            Selection::Simple { ref mut region } => {
+                region.start.point.line = (region.start.point.line as isize + offset) as usize;
+                region.end.point.line = (region.end.point.line as isize + offset) as usize;
+            },
+            Selection::Semantic { ref mut region, ref mut initial_expansion } => {
+                region.start.line = (region.start.line as isize + offset) as usize;
+                region.end.line = (region.end.line as isize + offset) as usize;
+                initial_expansion.start.line = (initial_expansion.start.line as isize + offset) as usize;
+                initial_expansion.end.line = (initial_expansion.end.line as isize + offset) as usize;
+            },
+            Selection::Lines { ref mut region, ref mut initial_line } => {
+                region.start.line = (region.start.line as isize + offset) as usize;
+                region.end.line = (region.end.line as isize + offset) as usize;
+                *initial_line = (*initial_line as isize + offset) as usize;
+            }
+        }
+    }
+
+    pub fn semantic<G: SemanticSearch>(point: Point<usize>, grid: &G) -> Selection {
         let (start, end) = (grid.semantic_search_left(point), grid.semantic_search_right(point));
         Selection::Semantic {
             region: Range {
@@ -118,7 +137,7 @@ impl Selection {
         }
     }
 
-    pub fn lines(point: Point) -> Selection {
+    pub fn lines(point: Point<usize>) -> Selection {
         Selection::Lines {
             region: Range {
                 start: point,
@@ -128,7 +147,7 @@ impl Selection {
         }
     }
 
-    pub fn update(&mut self, location: Point, side: Side) {
+    pub fn update(&mut self, location: Point<usize>, side: Side) {
         // Always update the `end`; can normalize later during span generation.
         match *self {
             Selection::Simple { ref mut region } => {
@@ -151,14 +170,14 @@ impl Selection {
                 Selection::span_semantic(grid, region, initial_expansion)
             },
             Selection::Lines { ref region, ref initial_line } => {
-                Selection::span_lines(grid, region, initial_line)
+                Selection::span_lines(grid, region, *initial_line)
             }
         }
     }
     fn span_semantic<G>(
         grid: &G,
-        region: &Range<Point>,
+        region: &Range<Point<usize>>,
-        initial_expansion: &Range<Point>
+        initial_expansion: &Range<Point<usize>>
     ) -> Option<Span>
         where G: SemanticSearch + Dimensions
     {
@@ -172,14 +191,20 @@ impl Selection {
             (region.end, region.start)
         };
 
-        // Update start of selection *if* front has moved beyond initial start
+        println!("BEFORE front={:?}, start={:?}, tail={:?}, end={:?}", front, start, tail, end);
-        if front < start {
+
+        if front < tail && front.line == tail.line {
             start = grid.semantic_search_left(front);
+            end = grid.semantic_search_right(tail);
+        } else {
+            start = grid.semantic_search_right(front);
+            end = grid.semantic_search_left(tail);
         }
 
-        // Update end of selection *if* tail has moved beyond initial end.
+        println!("AFTER  front={:?}, start={:?}, tail={:?}, end={:?}", front, start, tail, end);
-        if tail > end {
+
-            end = grid.semantic_search_right(tail);
+        if start > end {
+            ::std::mem::swap(&mut start, &mut end);
         }
 
         Some(Span {
@@ -190,27 +215,27 @@ impl Selection {
         })
     }
 
-    fn span_lines<G>(grid: &G, region: &Range<Point>, initial_line: &Line) -> Option<Span>
+    fn span_lines<G>(grid: &G, region: &Range<Point<usize>>, initial_line: usize) -> Option<Span>
         where G: Dimensions
     {
         // First, create start and end points based on initial line and the grid
         // dimensions.
         let mut start = Point {
-            col: Column(0),
+            col: grid.dimensions().col - 1,
-            line: *initial_line
+            line: initial_line
         };
         let mut end = Point {
-            col: grid.dimensions().col - 1,
+            col: Column(0),
-            line: *initial_line
+            line: initial_line
         };
 
         // Now, expand lines based on where cursor started and ended.
         if region.start.line < region.end.line {
-            // Start is above end
+            // Start is below end
             start.line = min(start.line, region.start.line);
             end.line = max(end.line, region.end.line);
         } else {
-            // Start is below end
+            // Start is above end
             start.line = min(start.line, region.end.line);
             end.line = max(end.line, region.start.line);
         }
@@ -313,27 +338,37 @@ pub enum SpanType {
 /// Represents a span of selected cells
 #[derive(Debug, Eq, PartialEq)]
 pub struct Span {
-    front: Point,
+    front: Point<usize>,
-    tail: Point,
+    tail: Point<usize>,
     cols: Column,
 
     /// The type says whether ends are included or not.
     ty: SpanType,
 }
 
+#[derive(Debug)]
+pub struct Locations {
+    /// Start point from bottom of buffer
+    pub start: Point<usize>,
+    /// End point towards top of buffer
+    pub end: Point<usize>,
+}
+
 impl Span {
-    pub fn to_locations(&self) -> (Point, Point) {
+    pub fn to_locations(&self) -> Locations {
-        match self.ty {
+        let (start, end) = match self.ty {
             SpanType::Inclusive => (self.front, self.tail),
             SpanType::Exclusive => {
                 (Span::wrap_start(self.front, self.cols), Span::wrap_end(self.tail, self.cols))
             },
             SpanType::ExcludeFront => (Span::wrap_start(self.front, self.cols), self.tail),
             SpanType::ExcludeTail => (self.front, Span::wrap_end(self.tail, self.cols))
-        }
+        };
+
+        Locations { start, end }
     }
 
-    fn wrap_start(mut start: Point, cols: Column) -> Point {
+    fn wrap_start(mut start: Point<usize>, cols: Column) -> Point<usize> {
         if start.col == cols - 1 {
             Point {
                 line: start.line + 1,
@@ -345,8 +380,8 @@ impl Span {
         }
     }
 
-    fn wrap_end(end: Point, cols: Column) -> Point {
+    fn wrap_end(end: Point<usize>, cols: Column) -> Point<usize> {
-        if end.col == Column(0) && end.line != Line(0) {
+        if end.col == Column(0) && end.line != 0 {
             Point {
                 line: end.line - 1,
                 col: cols
@@ -358,37 +393,6 @@ impl Span {
             }
         }
     }
-
-    #[inline]
-    fn exclude_start(start: Linear) -> Linear {
-        start + 1
-    }
-
-    #[inline]
-    fn exclude_end(end: Linear) -> Linear {
-        if end > Linear(0) {
-            end - 1
-        } else {
-            end
-        }
-    }
-}
-
-impl ToRange for Span {
-    fn to_range(&self) -> RangeInclusive<Linear> {
-        let cols = self.cols;
-        let start = Linear(self.front.line.0 * cols.0 + self.front.col.0);
-        let end = Linear(self.tail.line.0 * cols.0 + self.tail.col.0);
-
-        let (start, end) = match self.ty {
-            SpanType::Inclusive => (start, end),
-            SpanType::Exclusive => (Span::exclude_start(start), Span::exclude_end(end)),
-            SpanType::ExcludeFront => (Span::exclude_start(start), end),
-            SpanType::ExcludeTail => (start, Span::exclude_end(end))
-        };
-
-        RangeInclusive::new(start, end)
-    }
 }
 
 /// Tests for selection
@@ -422,8 +426,8 @@ mod test {
     }
 
     impl super::SemanticSearch for Dimensions {
-        fn semantic_search_left(&self, _: Point) -> Point { unimplemented!(); }
+        fn semantic_search_left(&self, _: Point<usize>) -> Point<usize> { unimplemented!(); }
-        fn semantic_search_right(&self, _: Point) -> Point { unimplemented!(); }
+        fn semantic_search_right(&self, _: Point<usize>) -> Point<usize> { unimplemented!(); }
     }
 
     /// Test case of single cell selection
@@ -433,7 +437,7 @@ mod test {
     /// 3. [BE]
     #[test]
     fn single_cell_left_to_right() {
-        let location = Point { line: Line(0), col: Column(0) };
+        let location = Point { line: 0, col: Column(0) };
         let mut selection = Selection::simple(location, Side::Left);
         selection.update(location, Side::Right);
 
@@ -452,7 +456,7 @@ mod test {
     /// 3. [EB]
     #[test]
     fn single_cell_right_to_left() {
-        let location = Point { line: Line(0), col: Column(0) };
+        let location = Point { line: 0, col: Column(0) };
         let mut selection = Selection::simple(location, Side::Right);
         selection.update(location, Side::Left);
 
@@ -471,8 +475,8 @@ mod test {
     /// 3. [ B][E ]
     #[test]
     fn between_adjacent_cells_left_to_right() {
-        let mut selection = Selection::simple(Point::new(Line(0), Column(0)), Side::Right);
+        let mut selection = Selection::simple(Point::new(0, Column(0)), Side::Right);
-        selection.update(Point::new(Line(0), Column(1)), Side::Left);
+        selection.update(Point::new(0, Column(1)), Side::Left);
 
         assert_eq!(selection.to_span(&Dimensions::new(1, 2)), None);
     }
@@ -484,8 +488,8 @@ mod test {
     /// 3. [ E][B ]
     #[test]
     fn between_adjacent_cells_right_to_left() {
-        let mut selection = Selection::simple(Point::new(Line(0), Column(1)), Side::Left);
+        let mut selection = Selection::simple(Point::new(0, Column(1)), Side::Left);
-        selection.update(Point::new(Line(0), Column(0)), Side::Right);
+        selection.update(Point::new(0, Column(0)), Side::Right);
 
         assert_eq!(selection.to_span(&Dimensions::new(1, 2)), None);
     }
@@ -501,13 +505,13 @@ mod test {
     ///     [XX][XB][  ][  ][  ]
     #[test]
     fn across_adjacent_lines_upward_final_cell_exclusive() {
-        let mut selection = Selection::simple(Point::new(Line(1), Column(1)), Side::Right);
+        let mut selection = Selection::simple(Point::new(1, Column(1)), Side::Right);
-        selection.update(Point::new(Line(0), Column(1)), Side::Right);
+        selection.update(Point::new(0, Column(1)), Side::Right);
 
         assert_eq!(selection.to_span(&Dimensions::new(2, 5)).unwrap(), Span {
             cols: Column(5),
-            front: Point::new(Line(0), Column(1)),
+            front: Point::new(0, Column(1)),
-            tail: Point::new(Line(1), Column(1)),
+            tail: Point::new(1, Column(1)),
             ty: SpanType::ExcludeFront
         });
     }
@@ -525,14 +529,14 @@ mod test {
     ///     [XE][  ][  ][  ][  ]
     #[test]
     fn selection_bigger_then_smaller() {
-        let mut selection = Selection::simple(Point::new(Line(0), Column(1)), Side::Right);
+        let mut selection = Selection::simple(Point::new(0, Column(1)), Side::Right);
-        selection.update(Point::new(Line(1), Column(1)), Side::Right);
+        selection.update(Point::new(1, Column(1)), Side::Right);
-        selection.update(Point::new(Line(1), Column(0)), Side::Right);
+        selection.update(Point::new(1, Column(0)), Side::Right);
 
         assert_eq!(selection.to_span(&Dimensions::new(2, 5)).unwrap(), Span {
             cols: Column(5),
-            front: Point::new(Line(0), Column(1)),
+            front: Point::new(0, Column(1)),
-            tail: Point::new(Line(1), Column(0)),
+            tail: Point::new(1, Column(0)),
             ty: SpanType::ExcludeFront
         });
     }

src/term/mod.rs

@@ -24,9 +24,9 @@ use unicode_width::UnicodeWidthChar;
 
 use font::{self, Size};
 use ansi::{self, Color, NamedColor, Attr, Handler, CharsetIndex, StandardCharset, CursorStyle};
-use grid::{BidirectionalIterator, Grid, ToRange, Indexed, IndexRegion, DisplayIter};
+use grid::{BidirectionalIterator, Grid, Indexed, IndexRegion, DisplayIter};
-use index::{self, Point, Column, Line, IndexRange, Contains, RangeInclusive};
+use index::{self, Point, Column, Line, IndexRange, Contains, RangeInclusive, Linear};
-use selection::{self, Span, Selection};
+use selection::{self, Selection, Locations};
 use config::{Config, VisualBellAnimation};
 use {MouseCursor, Rgb};
 use copypasta::{Clipboard, Load, Store};
@@ -37,7 +37,7 @@ pub use self::cell::Cell;
 use self::cell::LineLength;
 
 impl selection::SemanticSearch for Term {
-    fn semantic_search_left(&self, mut point: Point) -> Point {
+    fn semantic_search_left(&self, mut point: Point<usize>) -> Point<usize> {
         let mut iter = self.grid.iter_from(point);
         let last_col = self.grid.num_cols() - Column(1);
 
@@ -56,7 +56,7 @@ impl selection::SemanticSearch for Term {
         point
     }
 
-    fn semantic_search_right(&self, mut point: Point) -> Point {
+    fn semantic_search_right(&self, mut point: Point<usize>) -> Point<usize> {
         let mut iter = self.grid.iter_from(point);
         let last_col = self.grid.num_cols() - Column(1);
 
@@ -116,12 +116,37 @@ impl<'a> RenderableCellsIter<'a> {
         colors: &'b color::List,
         mode: TermMode,
         config: &'b Config,
-        selection: Option<RangeInclusive<index::Linear>>,
+        selection: Option<Locations>,
         cursor_style: CursorStyle,
     ) -> RenderableCellsIter<'b> {
         let cursor_offset = grid.line_to_offset(cursor.line);
         let inner = grid.display_iter();
 
+        let selection = selection.map(|loc| {
+            // start and end *lines* are swapped as we switch from buffer to
+            // Line coordinates.
+            let mut end = Point {
+                line: grid.buffer_line_to_visible(loc.start.line),
+                col: loc.start.col
+            };
+            let mut start = Point {
+                line: grid.buffer_line_to_visible(loc.end.line),
+                col: loc.end.col
+            };
+
+            if start > end {
+                ::std::mem::swap(&mut start, &mut end);
+            }
+
+            println!("start={:?}, end={:?}", start, end);
+
+            let cols = grid.num_cols();
+            let start = Linear(start.line.0 * cols.0 + start.col.0);
+            let end = Linear(end.line.0 * cols.0 + end.col.0);
+
+            RangeInclusive::new(start, end)
+        });
+
         RenderableCellsIter {
             cursor: cursor,
             cursor_offset: cursor_offset,
@@ -350,12 +375,13 @@ impl<'a> Iterator for RenderableCellsIter<'a> {
             } else {
                 let cell = self.inner.next()?;
 
+                let index = Linear(cell.line.0 * self.grid.num_cols().0 + cell.column.0);
+
                 // XXX (jwilm) selection temp disabled
                 //
-                // let selected = self.selection.as_ref()
+                let selected = self.selection.as_ref()
-                //     .map(|range| range.contains_(index))
+                    .map(|range| range.contains_(index))
-                //     .unwrap_or(false);
+                    .unwrap_or(false);
-                let selected = false;
 
                 // Skip empty cells
                 if cell.is_empty() && !selected {
@@ -877,7 +903,7 @@ impl Term {
         /// Need a generic push() for the Append trait
         trait PushChar {
             fn push_char(&mut self, c: char);
-            fn maybe_newline(&mut self, grid: &Grid<Cell>, line: Line, ending: Column) {
+            fn maybe_newline(&mut self, grid: &Grid<Cell>, line: usize, ending: Column) {
                 if ending != Column(0) && !grid[line][ending - 1].flags.contains(cell::Flags::WRAPLINE) {
                     self.push_char('\n');
                 }
@@ -894,14 +920,14 @@ impl Term {
         use std::ops::Range;
 
         trait Append : PushChar {
-            fn append(&mut self, grid: &Grid<Cell>, line: Line, cols: Range<Column>) -> Option<Range<Column>>;
+            fn append(&mut self, grid: &Grid<Cell>, line: usize, cols: Range<Column>) -> Option<Range<Column>>;
         }
 
         impl Append for String {
             fn append(
                 &mut self,
                 grid: &Grid<Cell>,
-                line: Line,
+                line: usize,
                 cols: Range<Column>
             ) -> Option<Range<Column>> {
                 let grid_line = &grid[line];
@@ -934,43 +960,54 @@ impl Term {
 
         let mut res = String::new();
 
-        let (start, end) = span.to_locations();
+        let Locations { mut start, mut end } = span.to_locations();
+
+        if start > end {
+            ::std::mem::swap(&mut start, &mut end);
+        }
+
         let line_count = end.line - start.line;
         let max_col = Column(usize::max_value() - 1);
 
         match line_count {
             // Selection within single line
-            Line(0) => {
+            0 => {
                 res.append(&self.grid, start.line, start.col..end.col);
             },
 
             // Selection ends on line following start
-            Line(1) => {
+            1 => {
-                // Starting line
-                res.append(&self.grid, start.line, start.col..max_col);
-
                 // Ending line
-                res.append(&self.grid, end.line, Column(0)..end.col);
+                res.append(&self.grid, end.line, end.col..max_col);
+
+                // Starting line
+                res.append(&self.grid, start.line, Column(0)..start.col);
+
             },
 
             // Multi line selection
             _ => {
-                // Starting line
+                // Ending line
-                res.append(&self.grid, start.line, start.col..max_col);
+                res.append(&self.grid, end.line, end.col..max_col);
 
-                let middle_range = IndexRange::from((start.line + 1)..(end.line));
+                let middle_range = (start.line + 1)..(end.line);
                 for line in middle_range {
                     res.append(&self.grid, line, Column(0)..max_col);
                 }
 
-                // Ending line
+                // Starting line
-                res.append(&self.grid, end.line, Column(0)..end.col);
+                res.append(&self.grid, start.line, Column(0)..(start.col + 1));
+
             }
         }
 
         Some(res)
     }
 
+    pub(crate) fn visible_to_buffer(&self, point: Point) -> Point<usize> {
+        self.grid.visible_to_buffer(point)
+    }
+
     /// Convert the given pixel values to a grid coordinate
     ///
     /// The mouse coordinates are expected to be relative to the top left. The
@@ -999,8 +1036,12 @@ impl Term {
         config: &'b Config,
         window_focused: bool,
     ) -> RenderableCellsIter {
-        let selection = self.grid.selection.as_ref().and_then(|s| s.to_span(self))
+        let selection = self.grid.selection.as_ref()
-            .map(|span| span.to_range());
+            .and_then(|s| s.to_span(self))
+            .map(|span| {
+                // println!("span={:?}, locations={:?}", span, span.to_locations());
+                span.to_locations()
+            });
         let cursor = if window_focused {
             self.cursor_style.unwrap_or(self.default_cursor_style)
         } else {