Fix selection of double-width characters

This changes the selection behavior to automatically select both cells
of double width characters in either selection direction.

This fixes #2322.

CHANGELOG.md

@@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Text Cursor disappearing
 - Incorrect positioning of zero-width characters over double-width characters
 - Mouse mode generating events when the cell has not changed
+- Selections not automatically expanding across double-width characters
 
 ## Version 0.3.2
 

alacritty_terminal/src/selection.rs

@@ -22,7 +22,8 @@ use std::cmp::{max, min};
 use std::ops::Range;
 
 use crate::index::{Column, Point, Side};
-use crate::term::Search;
+use crate::term::cell::Flags;
+use crate::term::{Search, Term};
 
 /// Describes a region of a 2-dimensional area
 ///
@@ -129,19 +130,35 @@ impl Selection {
         }
     }
 
-    pub fn to_span<G>(&self, grid: &G, alt_screen: bool) -> Option<Span>
+    pub fn to_span(&self, term: &Term, alt_screen: bool) -> Option<Span> {
-    where
+        let span = match *self {
-        G: Search + Dimensions,
+            Selection::Simple { ref region } => Selection::span_simple(term, region, alt_screen),
-    {
-        match *self {
-            Selection::Simple { ref region } => Selection::span_simple(grid, region, alt_screen),
             Selection::Semantic { ref region } => {
-                Selection::span_semantic(grid, region, alt_screen)
+                Selection::span_semantic(term, region, alt_screen)
             },
             Selection::Lines { ref region, initial_line } => {
-                Selection::span_lines(grid, region, initial_line, alt_screen)
+                Selection::span_lines(term, region, initial_line, alt_screen)
             },
-        }
+        };
+
+        // Expand selection across double-width cells
+        span.map(|mut span| {
+            let grid = term.grid();
+
+            if span.end.col < grid.num_cols()
+                && grid[span.end.line][span.end.col].flags.contains(Flags::WIDE_CHAR_SPACER)
+            {
+                span.end.col = Column(span.end.col.saturating_sub(1));
+            }
+
+            if span.start.col.0 < grid.num_cols().saturating_sub(1)
+                && grid[span.start.line][span.start.col].flags.contains(Flags::WIDE_CHAR)
+            {
+                span.start.col += 1;
+            }
+
+            span
+        })
     }
 
     pub fn is_empty(&self) -> bool {
@@ -161,27 +178,27 @@ impl Selection {
         let lines = grid.dimensions().line.0 as isize;
 
         // Normalize ordering of selected cells
-        let (mut front, mut tail) = if region.start < region.end {
+        let (mut start, mut end) = if region.start < region.end {
             (region.start, region.end)
         } else {
             (region.end, region.start)
         };
 
         if alt_screen {
-            Selection::alt_screen_clamp(&mut front, &mut tail, lines, cols)?;
+            Selection::alt_screen_clamp(&mut start, &mut end, lines, cols)?;
         }
 
-        let (mut start, mut end) = if front < tail && front.line == tail.line {
+        let (mut start, mut end) = if start < end && start.line == end.line {
-            (grid.semantic_search_left(front.into()), grid.semantic_search_right(tail.into()))
+            (grid.semantic_search_left(start.into()), grid.semantic_search_right(end.into()))
         } else {
-            (grid.semantic_search_right(front.into()), grid.semantic_search_left(tail.into()))
+            (grid.semantic_search_right(start.into()), grid.semantic_search_left(end.into()))
         };
 
         if start > end {
             ::std::mem::swap(&mut start, &mut end);
         }
 
-        Some(Span { cols, front: start, tail: end, ty: SpanType::Inclusive })
+        Some(Span { start, end })
     }
 
     fn span_lines<G>(
@@ -216,7 +233,7 @@ impl Selection {
             Selection::alt_screen_clamp(&mut start, &mut end, lines, cols)?;
         }
 
-        Some(Span { cols, front: start.into(), tail: end.into(), ty: SpanType::Inclusive })
+        Some(Span { start: start.into(), end: end.into() })
     }
 
     fn span_simple<G>(grid: &G, region: &Range<Anchor>, alt_screen: bool) -> Option<Span>
@@ -230,8 +247,8 @@ impl Selection {
         let cols = grid.dimensions().col;
         let lines = grid.dimensions().line.0 as isize;
 
-        // Make sure front is always the "bottom" and tail is always the "top"
+        // Make sure start is always the "bottom" and end is always the "top"
-        let (mut front, mut tail, front_side, tail_side) =
+        let (mut start, mut end, start_side, end_side) =
             if start.line > end.line || start.line == end.line && start.col <= end.col {
                 // Selected upward; start/end are swapped
                 (end, start, end_side, start_side)
@@ -241,139 +258,81 @@ impl Selection {
             };
 
         // No selection for single cell with identical sides or two cell with right+left sides
-        if (front == tail && front_side == tail_side)
+        if (start == end && start_side == end_side)
-            || (tail_side == Side::Right
+            || (end_side == Side::Right
-                && front_side == Side::Left
+                && start_side == Side::Left
-                && front.line == tail.line
+                && start.line == end.line
-                && front.col == tail.col + 1)
+                && start.col == end.col + 1)
         {
             return None;
         }
 
         // Remove last cell if selection ends to the left of a cell
-        if front_side == Side::Left && start != end {
+        if start_side == Side::Left && start != end {
             // Special case when selection starts to left of first cell
-            if front.col == Column(0) {
+            if start.col == Column(0) {
-                front.col = cols - 1;
+                start.col = cols - 1;
-                front.line += 1;
+                start.line += 1;
             } else {
-                front.col -= 1;
+                start.col -= 1;
             }
         }
 
         // Remove first cell if selection starts at the right of a cell
-        if tail_side == Side::Right && front != tail {
+        if end_side == Side::Right && start != end {
-            tail.col += 1;
+            end.col += 1;
         }
 
         if alt_screen {
-            Selection::alt_screen_clamp(&mut front, &mut tail, lines, cols)?;
+            Selection::alt_screen_clamp(&mut start, &mut end, lines, cols)?;
         }
 
         // Return the selection with all cells inclusive
-        Some(Span { cols, front: front.into(), tail: tail.into(), ty: SpanType::Inclusive })
+        Some(Span { start: start.into(), end: end.into() })
     }
 
     // Clamp selection in the alternate screen to the visible region
     fn alt_screen_clamp(
-        front: &mut Point<isize>,
+        start: &mut Point<isize>,
-        tail: &mut Point<isize>,
+        end: &mut Point<isize>,
         lines: isize,
         cols: Column,
     ) -> Option<()> {
-        if tail.line >= lines {
+        if end.line >= lines {
             // Don't show selection above visible region
-            if front.line >= lines {
+            if start.line >= lines {
                 return None;
             }
 
             // Clamp selection above viewport to visible region
-            tail.line = lines - 1;
+            end.line = lines - 1;
-            tail.col = Column(0);
+            end.col = Column(0);
         }
 
-        if front.line < 0 {
+        if start.line < 0 {
             // Don't show selection below visible region
-            if tail.line < 0 {
+            if end.line < 0 {
                 return None;
             }
 
             // Clamp selection below viewport to visible region
-            front.line = 0;
+            start.line = 0;
-            front.col = cols - 1;
+            start.col = cols - 1;
         }
 
         Some(())
     }
 }
 
-/// How to interpret the locations of a Span.
-#[derive(Debug, Eq, PartialEq)]
-pub enum SpanType {
-    /// Includes the beginning and end locations
-    Inclusive,
-
-    /// Exclude both beginning and end
-    Exclusive,
-
-    /// Excludes last cell of selection
-    ExcludeTail,
-
-    /// Excludes first cell of selection
-    ExcludeFront,
-}
-
 /// Represents a span of selected cells
 #[derive(Debug, Eq, PartialEq)]
 pub struct Span {
-    front: Point<usize>,
-    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) -> Locations {
-        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<usize>, cols: Column) -> Point<usize> {
-        if start.col == cols - 1 {
-            Point { line: start.line + 1, col: Column(0) }
-        } else {
-            start.col += 1;
-            start
-        }
-    }
-
-    fn wrap_end(end: Point<usize>, cols: Column) -> Point<usize> {
-        if end.col == Column(0) && end.line != 0 {
-            Point { line: end.line - 1, col: cols }
-        } else {
-            Point { line: end.line, col: end.col - 1 }
-        }
-    }
-}
-
 /// Tests for selection
 ///
 /// There are comments on all of the tests describing the selection. Pictograms
@@ -385,35 +344,27 @@ impl Span {
 /// look like [ B] and [E ].
 #[cfg(test)]
 mod test {
-    use super::{Selection, Span, SpanType};
+    use std::mem;
+
+    use super::{Selection, Span};
+    use crate::clipboard::Clipboard;
+    use crate::grid::Grid;
     use crate::index::{Column, Line, Point, Side};
-    use crate::url::Url;
+    use crate::message_bar::MessageBuffer;
+    use crate::term::cell::{Cell, Flags};
+    use crate::term::{SizeInfo, Term};
 
-    struct Dimensions(Point);
+    fn term(width: usize, height: usize) -> Term {
-    impl super::Dimensions for Dimensions {
+        let size = SizeInfo {
-        fn dimensions(&self) -> Point {
+            width: width as f32,
-            self.0
+            height: height as f32,
-        }
+            cell_width: 1.0,
-    }
+            cell_height: 1.0,
-
+            padding_x: 0.0,
-    impl Dimensions {
+            padding_y: 0.0,
-        pub fn new(line: usize, col: usize) -> Self {
+            dpr: 1.0,
-            Dimensions(Point { line: Line(line), col: Column(col) })
+        };
-        }
+        Term::new(&Default::default(), size, MessageBuffer::new(), Clipboard::new_nop())
-    }
-
-    impl super::Search for Dimensions {
-        fn semantic_search_left(&self, point: Point<usize>) -> Point<usize> {
-            point
-        }
-
-        fn semantic_search_right(&self, point: Point<usize>) -> Point<usize> {
-            point
-        }
-
-        fn url_search(&self, _: Point<usize>) -> Option<Url> {
-            None
-        }
     }
 
     /// Test case of single cell selection
@@ -427,11 +378,9 @@ mod test {
         let mut selection = Selection::simple(location, Side::Left);
         selection.update(location, Side::Right);
 
-        assert_eq!(selection.to_span(&Dimensions::new(1, 1), false).unwrap(), Span {
+        assert_eq!(selection.to_span(&term(1, 1), false).unwrap(), Span {
-            cols: Column(1),
+            start: location,
-            ty: SpanType::Inclusive,
+            end: location
-            front: location,
-            tail: location
         });
     }
 
@@ -446,11 +395,9 @@ mod test {
         let mut selection = Selection::simple(location, Side::Right);
         selection.update(location, Side::Left);
 
-        assert_eq!(selection.to_span(&Dimensions::new(1, 1), false).unwrap(), Span {
+        assert_eq!(selection.to_span(&term(1, 1), false).unwrap(), Span {
-            cols: Column(1),
+            start: location,
-            ty: SpanType::Inclusive,
+            end: location
-            front: location,
-            tail: location
         });
     }
 
@@ -464,7 +411,7 @@ mod test {
         let mut selection = Selection::simple(Point::new(0, Column(0)), Side::Right);
         selection.update(Point::new(0, Column(1)), Side::Left);
 
-        assert_eq!(selection.to_span(&Dimensions::new(1, 2), false), None);
+        assert_eq!(selection.to_span(&term(2, 1), false), None);
     }
 
     /// Test adjacent cell selection from right to left
@@ -477,7 +424,7 @@ mod test {
         let mut selection = Selection::simple(Point::new(0, Column(1)), Side::Left);
         selection.update(Point::new(0, Column(0)), Side::Right);
 
-        assert_eq!(selection.to_span(&Dimensions::new(1, 2), false), None);
+        assert_eq!(selection.to_span(&term(2, 1), false), None);
     }
 
     /// Test selection across adjacent lines
@@ -494,11 +441,9 @@ mod test {
         let mut selection = Selection::simple(Point::new(1, Column(1)), Side::Right);
         selection.update(Point::new(0, Column(1)), Side::Right);
 
-        assert_eq!(selection.to_span(&Dimensions::new(2, 5), false).unwrap(), Span {
+        assert_eq!(selection.to_span(&term(5, 2), false).unwrap(), Span {
-            cols: Column(5),
+            start: Point::new(0, Column(1)),
-            front: Point::new(0, Column(1)),
+            end: Point::new(1, Column(2)),
-            tail: Point::new(1, Column(2)),
-            ty: SpanType::Inclusive,
         });
     }
 
@@ -519,11 +464,9 @@ mod test {
         selection.update(Point::new(1, Column(1)), Side::Right);
         selection.update(Point::new(1, Column(0)), Side::Right);
 
-        assert_eq!(selection.to_span(&Dimensions::new(2, 5), false).unwrap(), Span {
+        assert_eq!(selection.to_span(&term(5, 2), false).unwrap(), Span {
-            cols: Column(5),
+            start: Point::new(0, Column(1)),
-            front: Point::new(0, Column(1)),
+            end: Point::new(1, Column(1)),
-            tail: Point::new(1, Column(1)),
-            ty: SpanType::Inclusive,
         });
     }
 
@@ -533,11 +476,9 @@ mod test {
         selection.update(Point::new(5, Column(3)), Side::Right);
         selection.rotate(-3);
 
-        assert_eq!(selection.to_span(&Dimensions::new(10, 5), true).unwrap(), Span {
+        assert_eq!(selection.to_span(&term(5, 10), true).unwrap(), Span {
-            cols: Column(5),
+            start: Point::new(0, Column(4)),
-            front: Point::new(0, Column(4)),
+            end: Point::new(2, Column(0)),
-            tail: Point::new(2, Column(0)),
-            ty: SpanType::Inclusive,
         });
     }
 
@@ -547,11 +488,9 @@ mod test {
         selection.update(Point::new(5, Column(3)), Side::Right);
         selection.rotate(-3);
 
-        assert_eq!(selection.to_span(&Dimensions::new(10, 5), true).unwrap(), Span {
+        assert_eq!(selection.to_span(&term(5, 10), true).unwrap(), Span {
-            cols: Column(5),
+            start: Point::new(0, Column(4)),
-            front: Point::new(0, Column(4)),
+            end: Point::new(2, Column(3)),
-            tail: Point::new(2, Column(3)),
-            ty: SpanType::Inclusive,
         });
     }
 
@@ -561,11 +500,28 @@ mod test {
         selection.update(Point::new(5, Column(3)), Side::Right);
         selection.rotate(-3);
 
-        assert_eq!(selection.to_span(&Dimensions::new(10, 5), true).unwrap(), Span {
+        assert_eq!(selection.to_span(&term(5, 10), true).unwrap(), Span {
-            cols: Column(5),
+            start: Point::new(0, Column(4)),
-            front: Point::new(0, Column(4)),
+            end: Point::new(2, Column(4)),
-            tail: Point::new(2, Column(4)),
+        });
-            ty: SpanType::Inclusive,
+    }
+
+    #[test]
+    fn double_width_expansion() {
+        let mut term = term(10, 1);
+        let mut grid = Grid::new(Line(1), Column(10), 0, Cell::default());
+        grid[Line(0)][Column(0)].flags.insert(Flags::WIDE_CHAR);
+        grid[Line(0)][Column(1)].flags.insert(Flags::WIDE_CHAR_SPACER);
+        grid[Line(0)][Column(8)].flags.insert(Flags::WIDE_CHAR);
+        grid[Line(0)][Column(9)].flags.insert(Flags::WIDE_CHAR_SPACER);
+        mem::swap(term.grid_mut(), &mut grid);
+
+        let mut selection = Selection::simple(Point::new(0, Column(1)), Side::Left);
+        selection.update(Point::new(0, Column(8)), Side::Right);
+
+        assert_eq!(selection.to_span(&term, false).unwrap(), Span {
+            start: Point::new(0, Column(9)),
+            end: Point::new(0, Column(0)),
         });
     }
 }

alacritty_terminal/src/term/mod.rs

@@ -35,7 +35,7 @@ use crate::grid::{
 use crate::index::{self, Column, Contains, IndexRange, Line, Linear, Point};
 use crate::input::FONT_SIZE_STEP;
 use crate::message_bar::MessageBuffer;
-use crate::selection::{self, Locations, Selection};
+use crate::selection::{self, Selection, Span};
 use crate::term::cell::{Cell, Flags, LineLength};
 use crate::term::color::Rgb;
 use crate::url::{Url, UrlParser};
@@ -174,7 +174,7 @@ impl<'a> RenderableCellsIter<'a> {
     fn new<'b>(
         term: &'b Term,
         config: &'b Config,
-        selection: Option<Locations>,
+        selection: Option<Span>,
         mut cursor_style: CursorStyle,
         metrics: font::Metrics,
     ) -> RenderableCellsIter<'b> {
@@ -183,22 +183,21 @@ impl<'a> RenderableCellsIter<'a> {
         let cursor_offset = grid.line_to_offset(term.cursor.point.line);
         let inner = grid.display_iter();
 
-        let mut selection_range = None;
+        let selection_range = selection.and_then(|span| {
-        if let Some(loc) = selection {
             // Get on-screen lines of the selection's locations
-            let start_line = grid.buffer_line_to_visible(loc.start.line);
+            let start_line = grid.buffer_line_to_visible(span.start.line);
-            let end_line = grid.buffer_line_to_visible(loc.end.line);
+            let end_line = grid.buffer_line_to_visible(span.end.line);
 
             // Get start/end locations based on what part of selection is on screen
             let locations = match (start_line, end_line) {
                 (ViewportPosition::Visible(start_line), ViewportPosition::Visible(end_line)) => {
-                    Some((start_line, loc.start.col, end_line, loc.end.col))
+                    Some((start_line, span.start.col, end_line, span.end.col))
                 },
                 (ViewportPosition::Visible(start_line), ViewportPosition::Above) => {
-                    Some((start_line, loc.start.col, Line(0), Column(0)))
+                    Some((start_line, span.start.col, Line(0), Column(0)))
                 },
                 (ViewportPosition::Below, ViewportPosition::Visible(end_line)) => {
-                    Some((grid.num_lines(), Column(0), end_line, loc.end.col))
+                    Some((grid.num_lines(), Column(0), end_line, span.end.col))
                 },
                 (ViewportPosition::Below, ViewportPosition::Above) => {
                     Some((grid.num_lines(), Column(0), Line(0), Column(0)))
@@ -206,7 +205,7 @@ impl<'a> RenderableCellsIter<'a> {
                 _ => None,
             };
 
-            if let Some((start_line, start_col, end_line, end_col)) = locations {
+            locations.map(|(start_line, start_col, end_line, end_col)| {
                 // start and end *lines* are swapped as we switch from buffer to
                 // Line coordinates.
                 let mut end = Point { line: start_line, col: start_col };
@@ -220,10 +219,9 @@ impl<'a> RenderableCellsIter<'a> {
                 let start = Linear::from_point(cols, start.into());
                 let end = Linear::from_point(cols, end.into());
 
-                // Update the selection
+                RangeInclusive::new(start, end)
-                selection_range = Some(RangeInclusive::new(start, end));
+            })
-            }
+        });
-        }
 
         // Load cursor glyph
         let cursor = &term.cursor.point;
@@ -1018,12 +1016,10 @@ impl Term {
 
         let alt_screen = self.mode.contains(TermMode::ALT_SCREEN);
         let selection = self.grid.selection.clone()?;
-        let span = selection.to_span(self, alt_screen)?;
+        let Span { mut start, mut end } = selection.to_span(self, alt_screen)?;
 
         let mut res = String::new();
 
-        let Locations { mut start, mut end } = span.to_locations();
-
         if start > end {
             ::std::mem::swap(&mut start, &mut end);
         }
@@ -1109,12 +1105,7 @@ impl Term {
         metrics: font::Metrics,
     ) -> RenderableCellsIter<'_> {
         let alt_screen = self.mode.contains(TermMode::ALT_SCREEN);
-        let selection = self
+        let selection = self.grid.selection.as_ref().and_then(|s| s.to_span(self, alt_screen));
-            .grid
-            .selection
-            .as_ref()
-            .and_then(|s| s.to_span(self, alt_screen))
-            .map(|span| span.to_locations());
 
         let cursor = if window_focused || !config.unfocused_hollow_cursor() {
             self.cursor_style.unwrap_or(self.default_cursor_style)