Fix selection expansion across full-width glyphs

Instead of trying to expand the start and end of a selection across
full-width glyphs, the selection should now only go from its origin to
the end without any kind of expansion.

Instead, the expansion is now done where the cells are actually checked
for their selection status, expanding across the entire full-width glyph
whenever any part of it is selected.

Fixes #3106.

5 files changed, 89 insertions, 119 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 202f0779..781c85e2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -45,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Inconsistent fontconfig fallback
 - Backspace deleting characters while IME is open on macOS
 - Handling of OpenType variable fonts
+- Expansion of block-selection on partially selected full-width glyphs
 
 ### Removed
 
diff --git a/alacritty/src/url.rs b/alacritty/src/url.rs
index 1840c22c..e538331d 100644
--- a/alacritty/src/url.rs
+++ b/alacritty/src/url.rs
@@ -42,7 +42,7 @@ impl Url {
     }
 
     pub fn end(&self) -> Point {
-        self.lines[self.lines.len() - 1].end.sub(self.num_cols, self.end_offset as usize, false)
+        self.lines[self.lines.len() - 1].end.sub(self.num_cols, self.end_offset as usize)
     }
 }
 
@@ -83,7 +83,7 @@ impl Urls {
         let end = point;
 
         // Reset URL when empty cells have been skipped
-        if point != Point::default() && Some(point.sub(num_cols, 1, false)) != self.last_point {
+        if point != Point::default() && Some(point.sub(num_cols, 1)) != self.last_point {
             self.reset();
         }
 
diff --git a/alacritty_terminal/src/index.rs b/alacritty_terminal/src/index.rs
index fb21baa0..51566d8d 100644
--- a/alacritty_terminal/src/index.rs
+++ b/alacritty_terminal/src/index.rs
@@ -44,33 +44,29 @@ impl<L> Point<L> {
 
     #[inline]
     #[must_use = "this returns the result of the operation, without modifying the original"]
-    pub fn sub(mut self, num_cols: usize, length: usize, absolute_indexing: bool) -> Point<L>
+    pub fn sub(mut self, num_cols: usize, rhs: usize) -> Point<L>
     where
-        L: Copy + Add<usize, Output = L> + Sub<usize, Output = L>,
+        L: Copy + Default + Into<Line> + Add<usize, Output = L> + Sub<usize, Output = L>,
     {
-        let line_changes = f32::ceil(length.saturating_sub(self.col.0) as f32 / num_cols as f32);
-        if absolute_indexing {
-            self.line = self.line + line_changes as usize;
+        let line_changes =
+            f32::ceil(rhs.saturating_sub(self.col.0) as f32 / num_cols as f32) as usize;
+        if self.line.into() > Line(line_changes) {
+            self.line = self.line - line_changes;
         } else {
-            self.line = self.line - line_changes as usize;
+            self.line = Default::default();
         }
-        self.col = Column((num_cols + self.col.0 - length % num_cols) % num_cols);
+        self.col = Column((num_cols + self.col.0 - rhs % num_cols) % num_cols);
         self
     }
 
     #[inline]
     #[must_use = "this returns the result of the operation, without modifying the original"]
-    pub fn add(mut self, num_cols: usize, length: usize, absolute_indexing: bool) -> Point<L>
+    pub fn add(mut self, num_cols: usize, rhs: usize) -> Point<L>
     where
-        L: Copy + Add<usize, Output = L> + Sub<usize, Output = L>,
+        L: Add<usize, Output = L> + Sub<usize, Output = L>,
     {
-        let line_changes = (length + self.col.0) / num_cols;
-        if absolute_indexing {
-            self.line = self.line - line_changes;
-        } else {
-            self.line = self.line + line_changes;
-        }
-        self.col = Column((self.col.0 + length) % num_cols);
+        self.line = self.line + (rhs + self.col.0) / num_cols;
+        self.col = Column((self.col.0 + rhs) % num_cols);
         self
     }
 }
diff --git a/alacritty_terminal/src/selection.rs b/alacritty_terminal/src/selection.rs
index f2c98a55..333c31fb 100644
--- a/alacritty_terminal/src/selection.rs
+++ b/alacritty_terminal/src/selection.rs
@@ -23,7 +23,6 @@ use std::mem;
 use std::ops::Range;
 
 use crate::index::{Column, Line, Point, Side};
-use crate::term::cell::Flags;
 use crate::term::{Search, Term};
 
 /// A Point and side within that point.
@@ -249,67 +248,12 @@ impl Selection {
         let is_block = self.ty == SelectionType::Block;
         let (start, end) = Self::grid_clamp(start, end, is_block, grid.len()).ok()?;
 
-        let range = match self.ty {
+        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
-        range.map(|range| Self::range_expand_fullwidth(term, range))
-    }
-
-    /// Expand the start/end of the selection range to account for fullwidth glyphs.
-    fn range_expand_fullwidth<T>(term: &Term<T>, mut range: SelectionRange) -> SelectionRange {
-        let grid = term.grid();
-        let num_cols = grid.num_cols();
-
-        // Helper for checking if cell at `point` contains `flag`
-        let flag_at = |point: Point<usize>, flag: Flags| -> bool {
-            grid[point.line][point.col].flags.contains(flag)
-        };
-
-        // Include all double-width cells and placeholders at top left of selection
-        if range.start.col < num_cols {
-            // Expand from wide char spacer to wide char
-            if range.start.line + 1 != grid.len() || range.start.col.0 != 0 {
-                let prev = range.start.sub(num_cols.0, 1, true);
-                if flag_at(range.start, Flags::WIDE_CHAR_SPACER) && flag_at(prev, Flags::WIDE_CHAR)
-                {
-                    range.start = prev;
-                }
-            }
-
-            // Expand from wide char to wide char spacer for linewrapping
-            if range.start.line + 1 != grid.len() || range.start.col.0 != 0 {
-                let prev = range.start.sub(num_cols.0, 1, true);
-                if (prev.line + 1 != grid.len() || prev.col.0 != 0)
-                    && flag_at(prev, Flags::WIDE_CHAR_SPACER)
-                    && !flag_at(prev.sub(num_cols.0, 1, true), Flags::WIDE_CHAR)
-                {
-                    range.start = prev;
-                }
-            }
-        }
-
-        // Include all double-width cells and placeholders at bottom right of selection
-        if range.end.line != 0 || range.end.col < num_cols {
-            // Expand from wide char spacer for linewrapping to wide char
-            if (range.end.line + 1 != grid.len() || range.end.col.0 != 0)
-                && flag_at(range.end, Flags::WIDE_CHAR_SPACER)
-                && !flag_at(range.end.sub(num_cols.0, 1, true), Flags::WIDE_CHAR)
-            {
-                range.end = range.end.add(num_cols.0, 1, true);
-            }
-
-            // Expand from wide char to wide char spacer
-            if flag_at(range.end, Flags::WIDE_CHAR) {
-                range.end = range.end.add(num_cols.0, 1, true);
-            }
         }
-
-        range
     }
 
     // Bring start and end points in the correct order
@@ -443,15 +387,11 @@ impl Selection {
 /// look like [ B] and [E ].
 #[cfg(test)]
 mod tests {
-    use std::mem;
-
     use super::{Selection, SelectionRange};
     use crate::clipboard::Clipboard;
     use crate::config::MockConfig;
     use crate::event::{Event, EventListener};
-    use crate::grid::Grid;
     use crate::index::{Column, Line, Point, Side};
-    use crate::term::cell::{Cell, Flags};
     use crate::term::{SizeInfo, Term};
 
     struct Mock;
@@ -638,26 +578,6 @@ mod tests {
     }
 
     #[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_range(&term).unwrap(), SelectionRange {
-            start: Point::new(0, Column(0)),
-            end: Point::new(0, Column(9)),
-            is_block: false,
-        });
-    }
-
-    #[test]
     fn simple_is_empty() {
         let mut selection = Selection::simple(Point::new(0, Column(0)), Side::Right);
         assert!(selection.is_empty());
diff --git a/alacritty_terminal/src/term/mod.rs b/alacritty_terminal/src/term/mod.rs
index ddba07ef..57733bf5 100644
--- a/alacritty_terminal/src/term/mod.rs
+++ b/alacritty_terminal/src/term/mod.rs
@@ -281,6 +281,49 @@ impl<'a, C> RenderableCellsIter<'a, C> {
             cursor_style,
         }
     }
+
+    /// Check selection state of a cell.
+    fn is_selected(&self, point: Point) -> bool {
+        let selection = match self.selection {
+            Some(selection) => selection,
+            None => return false,
+        };
+
+        // Point itself is selected
+        if selection.contains(point.col, point.line) {
+            return true;
+        }
+
+        let num_cols = self.grid.num_cols().0;
+        let cell = self.grid[&point];
+
+        // Check if wide char's spacers are selected
+        if cell.flags.contains(Flags::WIDE_CHAR) {
+            let prevprev = point.sub(num_cols, 2);
+            let prev = point.sub(num_cols, 1);
+            let next = point.add(num_cols, 1);
+
+            // Check trailing spacer
+            selection.contains(next.col, next.line)
+                // Check line-wrapping, leading spacer
+                || (self.grid[&prev].flags.contains(Flags::WIDE_CHAR_SPACER)
+                    && !self.grid[&prevprev].flags.contains(Flags::WIDE_CHAR)
+                    && selection.contains(prev.col, prev.line))
+        } else if cell.flags.contains(Flags::WIDE_CHAR_SPACER) {
+            // Check if spacer's wide char is selected
+            let prev = point.sub(num_cols, 1);
+
+            if self.grid[&prev].flags.contains(Flags::WIDE_CHAR) {
+                // Check previous cell for trailing spacer
+                self.is_selected(prev)
+            } else {
+                // Check next cell for line-wrapping, leading spacer
+                self.is_selected(point.add(num_cols, 1))
+            }
+        } else {
+            false
+        }
+    }
 }
 
 #[derive(Copy, Clone, Debug)]
@@ -414,11 +457,7 @@ impl<'a, C> Iterator for RenderableCellsIter<'a, C> {
     fn next(&mut self) -> Option<Self::Item> {
         loop {
             if self.cursor_offset == self.inner.offset() && self.inner.column() == self.cursor.col {
-                let selected = self
-                    .selection
-                    .as_ref()
-                    .map(|range| range.contains(self.cursor.col, self.cursor.line))
-                    .unwrap_or(false);
+                let selected = self.is_selected(Point::new(self.cursor.line, self.cursor.col));
 
                 // Handle cursor
                 if let Some(cursor_key) = self.cursor_key.take() {
@@ -458,11 +497,7 @@ impl<'a, C> Iterator for RenderableCellsIter<'a, C> {
             } else {
                 let cell = self.inner.next()?;
 
-                let selected = self
-                    .selection
-                    .as_ref()
-                    .map(|range| range.contains(cell.column, cell.line))
-                    .unwrap_or(false);
+                let selected = self.is_selected(Point::new(cell.line, cell.column));
 
                 if !cell.is_empty() || selected {
                     return Some(RenderableCell::new(self.config, self.colors, cell, selected));
@@ -949,9 +984,9 @@ impl<T> Term<T> {
 
         if is_block {
             for line in (end.line + 1..=start.line).rev() {
-                res += &(self.line_to_string(line, start.col..end.col) + "\n");
+                res += &(self.line_to_string(line, start.col..end.col, start.col.0 != 0) + "\n");
             }
-            res += &self.line_to_string(end.line, start.col..end.col);
+            res += &self.line_to_string(end.line, start.col..end.col, true);
         } else {
             res = self.bounds_to_string(start, end);
         }
@@ -967,23 +1002,31 @@ impl<T> Term<T> {
             let start_col = if line == start.line { start.col } else { Column(0) };
             let end_col = if line == end.line { end.col } else { self.cols() - 1 };
 
-            res += &self.line_to_string(line, start_col..end_col);
+            res += &self.line_to_string(line, start_col..end_col, line == end.line);
         }
 
         res
     }
 
     /// Convert a single line in the grid to a String.
-    fn line_to_string(&self, line: usize, cols: Range<Column>) -> String {
+    fn line_to_string(
+        &self,
+        line: usize,
+        mut cols: Range<Column>,
+        include_wrapped_wide: bool,
+    ) -> String {
         let mut text = String::new();
 
         let grid_line = &self.grid[line];
-        let line_length = grid_line.line_length();
-        let line_end = min(line_length, cols.end + 1);
+        let line_length = min(grid_line.line_length(), cols.end + 1);
 
-        let mut tab_mode = false;
+        // Include wide char when trailing spacer is selected
+        if grid_line[cols.start].flags.contains(Flags::WIDE_CHAR_SPACER) {
+            cols.start -= 1;
+        }
 
-        for col in IndexRange::from(cols.start..line_end) {
+        let mut tab_mode = false;
+        for col in IndexRange::from(cols.start..line_length) {
             let cell = grid_line[col];
 
             // Skip over cells until next tab-stop once a tab was found
@@ -1011,12 +1054,22 @@ impl<T> Term<T> {
         }
 
         if cols.end >= self.cols() - 1
-            && (line_end == Column(0)
-                || !self.grid[line][line_end - 1].flags.contains(Flags::WRAPLINE))
+            && (line_length.0 == 0
+                || !self.grid[line][line_length - 1].flags.contains(Flags::WRAPLINE))
         {
             text.push('\n');
         }
 
+        // If wide char is not part of the selection, but leading spacer is, include it
+        if line_length == self.grid.num_cols()
+            && line_length.0 >= 2
+            && grid_line[line_length - 1].flags.contains(Flags::WIDE_CHAR_SPACER)
+            && !grid_line[line_length - 2].flags.contains(Flags::WIDE_CHAR)
+            && include_wrapped_wide
+        {
+            text.push(self.grid[line - 1][Column(0)].c);
+        }
+
         text
     }