From bf353059a0f10bbccec754deb0e182cec3f6efe0 Mon Sep 17 00:00:00 2001 From: Christian Duerr Date: Mon, 29 Apr 2019 14:33:25 +0000 Subject: [PATCH] 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 | 1 + alacritty_terminal/src/selection.rs | 286 ++++++++++++---------------- alacritty_terminal/src/term/mod.rs | 37 ++-- 3 files changed, 136 insertions(+), 188 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 458cbc9..f712139 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/alacritty_terminal/src/selection.rs b/alacritty_terminal/src/selection.rs index d200958..6aaedf2 100644 --- a/alacritty_terminal/src/selection.rs +++ b/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(&self, grid: &G, alt_screen: bool) -> Option - where - G: Search + Dimensions, - { - match *self { - Selection::Simple { ref region } => Selection::span_simple(grid, region, alt_screen), + pub fn to_span(&self, term: &Term, alt_screen: bool) -> Option { + let span = match *self { + Selection::Simple { ref region } => Selection::span_simple(term, 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 { - (grid.semantic_search_left(front.into()), grid.semantic_search_right(tail.into())) + let (mut start, mut end) = if start < end && start.line == end.line { + (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( @@ -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(grid: &G, region: &Range, alt_screen: bool) -> Option @@ -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" - let (mut front, mut tail, front_side, tail_side) = + // Make sure start is always the "bottom" and end is always the "top" + 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) - || (tail_side == Side::Right - && front_side == Side::Left - && front.line == tail.line - && front.col == tail.col + 1) + if (start == end && start_side == end_side) + || (end_side == Side::Right + && start_side == Side::Left + && start.line == end.line + && 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) { - front.col = cols - 1; - front.line += 1; + if start.col == Column(0) { + start.col = cols - 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 { - tail.col += 1; + if end_side == Side::Right && start != end { + 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, - tail: &mut Point, + start: &mut Point, + end: &mut Point, 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; - tail.col = Column(0); + end.line = lines - 1; + 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; - front.col = cols - 1; + start.line = 0; + 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, - tail: Point, - 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, /// End point towards top of buffer pub end: Point, } -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, cols: Column) -> Point { - if start.col == cols - 1 { - Point { line: start.line + 1, col: Column(0) } - } else { - start.col += 1; - start - } - } - - fn wrap_end(end: Point, cols: Column) -> Point { - 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); - impl super::Dimensions for Dimensions { - fn dimensions(&self) -> Point { - self.0 - } - } - - impl Dimensions { - pub fn new(line: usize, col: usize) -> Self { - Dimensions(Point { line: Line(line), col: Column(col) }) - } - } - - impl super::Search for Dimensions { - fn semantic_search_left(&self, point: Point) -> Point { - point - } - - fn semantic_search_right(&self, point: Point) -> Point { - point - } - - fn url_search(&self, _: Point) -> Option { - None - } + fn term(width: usize, height: usize) -> Term { + let size = SizeInfo { + width: width as f32, + height: height as f32, + cell_width: 1.0, + cell_height: 1.0, + padding_x: 0.0, + padding_y: 0.0, + dpr: 1.0, + }; + Term::new(&Default::default(), size, MessageBuffer::new(), Clipboard::new_nop()) } /// 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 { - cols: Column(1), - ty: SpanType::Inclusive, - front: location, - tail: location + assert_eq!(selection.to_span(&term(1, 1), false).unwrap(), Span { + start: location, + end: 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 { - cols: Column(1), - ty: SpanType::Inclusive, - front: location, - tail: location + assert_eq!(selection.to_span(&term(1, 1), false).unwrap(), Span { + start: location, + end: 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 { - cols: Column(5), - front: Point::new(0, Column(1)), - tail: Point::new(1, Column(2)), - ty: SpanType::Inclusive, + assert_eq!(selection.to_span(&term(5, 2), false).unwrap(), Span { + start: Point::new(0, Column(1)), + end: Point::new(1, Column(2)), }); } @@ -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 { - cols: Column(5), - front: Point::new(0, Column(1)), - tail: Point::new(1, Column(1)), - ty: SpanType::Inclusive, + assert_eq!(selection.to_span(&term(5, 2), false).unwrap(), Span { + start: Point::new(0, Column(1)), + end: Point::new(1, Column(1)), }); } @@ -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 { - cols: Column(5), - front: Point::new(0, Column(4)), - tail: Point::new(2, Column(0)), - ty: SpanType::Inclusive, + assert_eq!(selection.to_span(&term(5, 10), true).unwrap(), Span { + start: Point::new(0, Column(4)), + end: Point::new(2, Column(0)), }); } @@ -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 { - cols: Column(5), - front: Point::new(0, Column(4)), - tail: Point::new(2, Column(3)), - ty: SpanType::Inclusive, + assert_eq!(selection.to_span(&term(5, 10), true).unwrap(), Span { + start: Point::new(0, Column(4)), + end: Point::new(2, Column(3)), }); } @@ -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 { - cols: Column(5), - front: Point::new(0, Column(4)), - tail: Point::new(2, Column(4)), - ty: SpanType::Inclusive, + assert_eq!(selection.to_span(&term(5, 10), true).unwrap(), Span { + start: Point::new(0, Column(4)), + end: Point::new(2, Column(4)), + }); + } + + #[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)), }); } } diff --git a/alacritty_terminal/src/term/mod.rs b/alacritty_terminal/src/term/mod.rs index c89ac03..68a5fdc 100644 --- a/alacritty_terminal/src/term/mod.rs +++ b/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, + selection: Option, 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; - if let Some(loc) = selection { + let selection_range = selection.and_then(|span| { // Get on-screen lines of the selection's locations - let start_line = grid.buffer_line_to_visible(loc.start.line); - let end_line = grid.buffer_line_to_visible(loc.end.line); + let start_line = grid.buffer_line_to_visible(span.start.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 - selection_range = Some(RangeInclusive::new(start, end)); - } - } + 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 - .grid - .selection - .as_ref() - .and_then(|s| s.to_span(self, alt_screen)) - .map(|span| span.to_locations()); + let selection = self.grid.selection.as_ref().and_then(|s| s.to_span(self, alt_screen)); let cursor = if window_focused || !config.unfocused_hollow_cursor() { self.cursor_style.unwrap_or(self.default_cursor_style)