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.
This commit is contained in:
Christian Duerr 2020-02-07 06:50:18 +00:00 committed by GitHub
parent cbec5e415b
commit 6832b86aa7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 89 additions and 119 deletions

View File

@ -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

View File

@ -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();
}

View File

@ -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
}
}

View File

@ -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;
@ -637,26 +577,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);

View File

@ -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);
// Include wide char when trailing spacer is selected
if grid_line[cols.start].flags.contains(Flags::WIDE_CHAR_SPACER) {
cols.start -= 1;
}
let mut tab_mode = false;
for col in IndexRange::from(cols.start..line_end) {
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
}