diff options
Diffstat (limited to 'helix-core')
| -rw-r--r-- | helix-core/src/graphemes.rs | 23 | ||||
| -rw-r--r-- | helix-core/src/line_ending.rs | 7 | ||||
| -rw-r--r-- | helix-core/src/movement.rs | 181 | ||||
| -rw-r--r-- | helix-core/src/object.rs | 2 | ||||
| -rw-r--r-- | helix-core/src/position.rs | 4 | ||||
| -rw-r--r-- | helix-core/src/selection.rs | 455 | ||||
| -rw-r--r-- | helix-core/src/syntax.rs | 18 |
7 files changed, 504 insertions, 186 deletions
diff --git a/helix-core/src/graphemes.rs b/helix-core/src/graphemes.rs index f71b6d5f..0465fe51 100644 --- a/helix-core/src/graphemes.rs +++ b/helix-core/src/graphemes.rs @@ -71,6 +71,8 @@ pub fn nth_prev_grapheme_boundary(slice: RopeSlice, char_idx: usize, n: usize) - } /// Finds the previous grapheme boundary before the given char position. +#[must_use] +#[inline(always)] pub fn prev_grapheme_boundary(slice: RopeSlice, char_idx: usize) -> usize { nth_prev_grapheme_boundary(slice, char_idx, 1) } @@ -117,21 +119,38 @@ pub fn nth_next_grapheme_boundary(slice: RopeSlice, char_idx: usize, n: usize) - } /// Finds the next grapheme boundary after the given char position. +#[must_use] +#[inline(always)] pub fn next_grapheme_boundary(slice: RopeSlice, char_idx: usize) -> usize { nth_next_grapheme_boundary(slice, char_idx, 1) } /// Returns the passed char index if it's already a grapheme boundary, /// or the next grapheme boundary char index if not. -pub fn ensure_grapheme_boundary(slice: RopeSlice, char_idx: usize) -> usize { +#[must_use] +#[inline] +pub fn ensure_grapheme_boundary_next(slice: RopeSlice, char_idx: usize) -> usize { if char_idx == 0 { - 0 + char_idx } else { next_grapheme_boundary(slice, char_idx - 1) } } +/// Returns the passed char index if it's already a grapheme boundary, +/// or the prev grapheme boundary char index if not. +#[must_use] +#[inline] +pub fn ensure_grapheme_boundary_prev(slice: RopeSlice, char_idx: usize) -> usize { + if char_idx == slice.len_chars() { + char_idx + } else { + prev_grapheme_boundary(slice, char_idx + 1) + } +} + /// Returns whether the given char position is a grapheme boundary. +#[must_use] pub fn is_grapheme_boundary(slice: RopeSlice, char_idx: usize) -> bool { // Bounds check debug_assert!(char_idx <= slice.len_chars()); diff --git a/helix-core/src/line_ending.rs b/helix-core/src/line_ending.rs index e3ff6478..18ea5f9f 100644 --- a/helix-core/src/line_ending.rs +++ b/helix-core/src/line_ending.rs @@ -159,6 +159,13 @@ pub fn line_end_char_index(slice: &RopeSlice, line: usize) -> usize { .unwrap_or(0) } +/// Fetches line `line_idx` from the passed rope slice, sans any line ending. +pub fn line_without_line_ending<'a>(slice: &'a RopeSlice, line_idx: usize) -> RopeSlice<'a> { + let start = slice.line_to_char(line_idx); + let end = line_end_char_index(slice, line_idx); + slice.slice(start..end) +} + /// Returns the char index of the end of the given RopeSlice, not including /// any final line ending. pub fn rope_end_without_line_ending(slice: &RopeSlice) -> usize { diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs index f9e5deb4..bc56f9a4 100644 --- a/helix-core/src/movement.rs +++ b/helix-core/src/movement.rs @@ -5,8 +5,11 @@ use ropey::iter::Chars; use crate::{ chars::{categorize_char, char_is_line_ending, CharCategory}, coords_at_pos, - graphemes::{nth_next_grapheme_boundary, nth_prev_grapheme_boundary}, - line_ending::{get_line_ending, line_end_char_index}, + graphemes::{ + next_grapheme_boundary, nth_next_grapheme_boundary, nth_prev_grapheme_boundary, + prev_grapheme_boundary, RopeGraphemes, + }, + line_ending::line_without_line_ending, pos_at_coords, Position, Range, RopeSlice, }; @@ -29,25 +32,60 @@ pub fn move_horizontally( count: usize, behaviour: Movement, ) -> Range { - let pos = range.head; - let line = slice.char_to_line(pos); - // TODO: we can optimize clamping by passing in RopeSlice limited to current line. that way - // we stop calculating past start/end of line. - let pos = match dir { - Direction::Backward => { - let start = slice.line_to_char(line); - nth_prev_grapheme_boundary(slice, pos, count).max(start) + match (behaviour, dir) { + (Movement::Move, Direction::Backward) => { + let count = if range.anchor < range.head { + count + 1 + } else { + count + }; + let pos = nth_prev_grapheme_boundary(slice, range.head, count); + Range::new(pos, pos) } - Direction::Forward => { - let end_char_idx = line_end_char_index(&slice, line); - nth_next_grapheme_boundary(slice, pos, count).min(end_char_idx) + (Movement::Move, Direction::Forward) => { + let count = if range.anchor < range.head { + count - 1 + } else { + count + }; + let pos = nth_next_grapheme_boundary(slice, range.head, count); + Range::new(pos, pos) } - }; - let anchor = match behaviour { - Movement::Extend => range.anchor, - Movement::Move => pos, - }; - Range::new(anchor, pos) + (Movement::Extend, Direction::Backward) => { + // Ensure a valid initial selection state. + let range = range.min_width_1(slice); + + // Do the main movement. + let mut head = nth_prev_grapheme_boundary(slice, range.head, count); + let mut anchor = range.anchor; + + // If the head and anchor crossed over each other, we need to + // fiddle around to make it behave like a 1-wide cursor. + if head <= anchor && range.head > range.anchor { + anchor = next_grapheme_boundary(slice, anchor); + head = prev_grapheme_boundary(slice, head); + } + + Range::new(anchor, head) + } + (Movement::Extend, Direction::Forward) => { + // Ensure a valid initial selection state. + let range = range.min_width_1(slice); + + // Do the main movement. + let mut head = nth_next_grapheme_boundary(slice, range.head, count); + let mut anchor = range.anchor; + + // If the head and anchor crossed over each other, we need to + // fiddle around to make it behave like a 1-wide cursor. + if head >= anchor && range.head < range.anchor { + anchor = prev_grapheme_boundary(slice, anchor); + head = next_grapheme_boundary(slice, head); + } + + Range::new(anchor, head) + } + } } pub fn move_vertically( @@ -57,36 +95,61 @@ pub fn move_vertically( count: usize, behaviour: Movement, ) -> Range { - let Position { row, col } = coords_at_pos(slice, range.head); + // Shift back one grapheme if needed, to account for + // the cursor being visually 1-width. + let pos = if range.head > range.anchor { + prev_grapheme_boundary(slice, range.head) + } else { + range.head + }; + // Compute the current position's 2d coordinates. + let Position { row, col } = coords_at_pos(slice, pos); let horiz = range.horiz.unwrap_or(col as u32); - let new_line = match dir { - Direction::Backward => row.saturating_sub(count), - Direction::Forward => std::cmp::min( - row.saturating_add(count), - slice.len_lines().saturating_sub(2), - ), - }; + // Compute the new position. + let new_pos = { + let new_row = if dir == Direction::Backward { + row.saturating_sub(count) + } else { + (row + count).min(slice.len_lines().saturating_sub(1)) + }; + let max_col = RopeGraphemes::new(line_without_line_ending(&slice, new_row)).count(); + let new_col = col.max(horiz as usize).min(max_col); - // Length of the line sans line-ending. - let new_line_len = { - let line = slice.line(new_line); - line.len_chars() - get_line_ending(&line).map(|le| le.len_chars()).unwrap_or(0) + pos_at_coords(slice, Position::new(new_row, new_col)) }; - let new_col = std::cmp::min(horiz as usize, new_line_len); - - let pos = pos_at_coords(slice, Position::new(new_line, new_col)); + // Compute the new range according to the type of movement. + match behaviour { + Movement::Move => Range { + anchor: new_pos, + head: new_pos, + horiz: Some(horiz), + }, + + Movement::Extend => { + let new_head = if new_pos >= range.anchor { + next_grapheme_boundary(slice, new_pos) + } else { + new_pos + }; - let anchor = match behaviour { - Movement::Extend => range.anchor, - Movement::Move => pos, - }; + let new_anchor = if range.anchor <= range.head && range.anchor > new_head { + next_grapheme_boundary(slice, range.anchor) + } else if range.anchor > range.head && range.anchor < new_head { + prev_grapheme_boundary(slice, range.anchor) + } else { + range.anchor + }; - let mut range = Range::new(anchor, pos); - range.horiz = Some(horiz); - range + Range { + anchor: new_anchor, + head: new_head, + horiz: Some(horiz), + } + } + } } pub fn move_next_word_start(slice: RopeSlice, range: Range, count: usize) -> Range { @@ -330,7 +393,7 @@ mod test { } #[test] - fn horizontal_moves_through_single_line_in_single_line_text() { + fn horizontal_moves_through_single_line_text() { let text = Rope::from(SINGLE_LINE_SAMPLE); let slice = text.slice(..); let position = pos_at_coords(slice, (0, 0).into()); @@ -353,7 +416,7 @@ mod test { } #[test] - fn horizontal_moves_through_single_line_in_multiline_text() { + fn horizontal_moves_through_multiline_text() { let text = Rope::from(MULTILINE_SAMPLE); let slice = text.slice(..); let position = pos_at_coords(slice, (0, 0).into()); @@ -361,15 +424,15 @@ mod test { let mut range = Range::point(position); let moves_and_expected_coordinates = IntoIter::new([ - ((Direction::Forward, 1usize), (0, 1)), // M|ultiline\n - ((Direction::Forward, 2usize), (0, 3)), // Mul|tiline\n - ((Direction::Backward, 6usize), (0, 0)), // |Multiline\n - ((Direction::Backward, 999usize), (0, 0)), // |Multiline\n - ((Direction::Forward, 3usize), (0, 3)), // Mul|tiline\n - ((Direction::Forward, 0usize), (0, 3)), // Mul|tiline\n - ((Direction::Backward, 0usize), (0, 3)), // Mul|tiline\n - ((Direction::Forward, 999usize), (0, 9)), // Multiline|\n - ((Direction::Forward, 999usize), (0, 9)), // Multiline|\n + ((Direction::Forward, 11usize), (1, 1)), // Multiline\nt|ext sample\n... + ((Direction::Backward, 1usize), (1, 0)), // Multiline\n|text sample\n... + ((Direction::Backward, 5usize), (0, 5)), // Multi|line\ntext sample\n... + ((Direction::Backward, 999usize), (0, 0)), // |Multiline\ntext sample\n... + ((Direction::Forward, 3usize), (0, 3)), // Mul|tiline\ntext sample\n... + ((Direction::Forward, 0usize), (0, 3)), // Mul|tiline\ntext sample\n... + ((Direction::Backward, 0usize), (0, 3)), // Mul|tiline\ntext sample\n... + ((Direction::Forward, 999usize), (5, 0)), // ...and whitespaced\n| + ((Direction::Forward, 999usize), (5, 0)), // ...and whitespaced\n| ]); for ((direction, amount), coordinates) in moves_and_expected_coordinates { @@ -409,12 +472,13 @@ mod test { let moves_and_expected_coordinates = IntoIter::new([ ((Direction::Forward, 1usize), (1, 0)), ((Direction::Forward, 2usize), (3, 0)), + ((Direction::Forward, 1usize), (4, 0)), ((Direction::Backward, 999usize), (0, 0)), - ((Direction::Forward, 3usize), (3, 0)), - ((Direction::Forward, 0usize), (3, 0)), - ((Direction::Backward, 0usize), (3, 0)), - ((Direction::Forward, 5), (4, 0)), - ((Direction::Forward, 999usize), (4, 0)), + ((Direction::Forward, 4usize), (4, 0)), + ((Direction::Forward, 0usize), (4, 0)), + ((Direction::Backward, 0usize), (4, 0)), + ((Direction::Forward, 5), (5, 0)), + ((Direction::Forward, 999usize), (5, 0)), ]); for ((direction, amount), coordinates) in moves_and_expected_coordinates { @@ -446,7 +510,8 @@ mod test { ((Axis::V, Direction::Forward, 1usize), (3, 8)), // Behaviour is preserved even through long jumps ((Axis::V, Direction::Backward, 999usize), (0, 8)), - ((Axis::V, Direction::Forward, 999usize), (4, 8)), + ((Axis::V, Direction::Forward, 4usize), (4, 8)), + ((Axis::V, Direction::Forward, 999usize), (5, 0)), ]); for ((axis, direction, amount), coordinates) in moves_and_expected_coordinates { diff --git a/helix-core/src/object.rs b/helix-core/src/object.rs index 33d90971..d9558dd8 100644 --- a/helix-core/src/object.rs +++ b/helix-core/src/object.rs @@ -5,7 +5,7 @@ use crate::{Range, RopeSlice, Selection, Syntax}; pub fn expand_selection(syntax: &Syntax, text: RopeSlice, selection: &Selection) -> Selection { let tree = syntax.tree(); - selection.transform(|range| { + selection.clone().transform(|range| { let from = text.char_to_byte(range.from()); let to = text.char_to_byte(range.to()); diff --git a/helix-core/src/position.rs b/helix-core/src/position.rs index 3d114b52..c4e8c9d6 100644 --- a/helix-core/src/position.rs +++ b/helix-core/src/position.rs @@ -53,6 +53,8 @@ impl From<Position> for tree_sitter::Point { } /// Convert a character index to (line, column) coordinates. pub fn coords_at_pos(text: RopeSlice, pos: usize) -> Position { + // TODO: this isn't correct. This needs to work in terms of + // visual horizontal position, not graphemes. let line = text.char_to_line(pos); let line_start = text.line_to_char(line); let col = RopeGraphemes::new(text.slice(line_start..pos)).count(); @@ -61,6 +63,8 @@ pub fn coords_at_pos(text: RopeSlice, pos: usize) -> Position { /// Convert (line, column) coordinates to a character index. pub fn pos_at_coords(text: RopeSlice, coords: Position) -> usize { + // TODO: this isn't correct. This needs to work in terms of + // visual horizontal position, not graphemes. let Position { row, col } = coords; let line_start = text.line_to_char(row); // line_start + col diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index 63b9b557..64ff51d8 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -1,30 +1,49 @@ -//! Selections are the primary editing construct. Even a single cursor is defined as an empty -//! single selection range. +//! Selections are the primary editing construct. Even a single cursor is +//! defined as a single empty or 1-wide selection range. //! //! All positioning is done via `char` offsets into the buffer. -use crate::{Assoc, ChangeSet, RopeSlice}; +use crate::{ + graphemes::{ + ensure_grapheme_boundary_next, ensure_grapheme_boundary_prev, next_grapheme_boundary, + }, + Assoc, ChangeSet, RopeSlice, +}; use smallvec::{smallvec, SmallVec}; use std::borrow::Cow; -#[inline] -fn abs_difference(x: usize, y: usize) -> usize { - if x < y { - y - x - } else { - x - y - } -} - -/// A single selection range. Anchor-inclusive, head-exclusive. +/// A single selection range. +/// +/// The range consists of an "anchor" and "head" position in +/// the text. The head is the part that the user moves when +/// directly extending the selection. The head and anchor +/// can be in any order: either can precede or follow the +/// other in the text, and they can share the same position +/// for a zero-width range. +/// +/// Below are some example `Range` configurations to better +/// illustrate. The anchor and head indices are show as +/// "(anchor, head)", followed by example text with "[" and "]" +/// inserted to visually represent the anchor and head positions: +/// +/// - (0, 3): [Som]e text. +/// - (3, 0): ]Som[e text. +/// - (2, 7): So[me te]xt. +/// - (1, 1): S[]ome text. +/// +/// Ranges are considered to be inclusive on the left and +/// exclusive on the right, regardless of anchor-head ordering. +/// This means, for example, that non-zero-width ranges that +/// are directly adjecent, sharing an edge, do not overlap. +/// However, a zero-width range will overlap with the shared +/// left-edge of another range. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct Range { - // TODO: optimize into u32 /// The anchor of the range: the side that doesn't move when extending. pub anchor: usize, /// The head of the range, moved when extending. pub head: usize, pub horiz: Option<u32>, -} // TODO: might be cheaper to store normalized as from/to and an inverted flag +} impl Range { pub fn new(anchor: usize, head: usize) -> Self { @@ -62,25 +81,14 @@ impl Range { /// Check two ranges for overlap. #[must_use] pub fn overlaps(&self, other: &Self) -> bool { - // cursor overlap is checked differently - if self.is_empty() { - let pos = self.head; - pos >= other.from() && other.to() >= pos - } else { - self.to() > other.from() && other.to() > self.from() - } + // To my eye, it's non-obvious why this works, but I arrived + // at it after transforming the slower version that explicitly + // enumerated more cases. The unit tests are thorough. + self.from() == other.from() || (self.to() > other.from() && other.to() > self.from()) } pub fn contains(&self, pos: usize) -> bool { - if self.is_empty() { - return false; - } - - if self.anchor < self.head { - self.anchor <= pos && pos < self.head - } else { - self.head < pos && pos <= self.anchor - } + self.from() <= pos && pos < self.to() } /// Map a range through a set of changes. Returns a new range representing the same position @@ -89,10 +97,10 @@ impl Range { let anchor = changes.map_pos(self.anchor, Assoc::After); let head = changes.map_pos(self.head, Assoc::After); - // TODO: possibly unnecessary - if self.anchor == anchor && self.head == head { - return self; - } + // We want to return a new `Range` with `horiz == None` every time, + // even if the anchor and head haven't changed, because we don't + // know if the *visual* position hasn't changed due to + // character-width or grapheme changes earlier in the text. Self { anchor, head, @@ -103,22 +111,100 @@ impl Range { /// Extend the range to cover at least `from` `to`. #[must_use] pub fn extend(&self, from: usize, to: usize) -> Self { - if from <= self.anchor && to >= self.anchor { - return Self { - anchor: from, - head: to, + debug_assert!(from <= to); + + if self.anchor <= self.head { + Self { + anchor: self.anchor.min(from), + head: self.head.max(to), + horiz: None, + } + } else { + Self { + anchor: self.anchor.max(to), + head: self.head.min(from), + horiz: None, + } + } + } + + /// Returns a range that encompasses both input ranges. + /// + /// This is like `extend()`, but tries to negotiate the + /// anchor/head ordering between the two input ranges. + #[must_use] + pub fn merge(&self, other: Self) -> Self { + if self.anchor > self.head && other.anchor > other.head { + Range { + anchor: self.anchor.max(other.anchor), + head: self.head.min(other.head), + horiz: None, + } + } else { + Range { + anchor: self.from().min(other.from()), + head: self.to().max(other.to()), horiz: None, - }; + } } + } - Self { - anchor: self.anchor, - head: if abs_difference(from, self.anchor) > abs_difference(to, self.anchor) { - from + /// Compute a possibly new range from this range, attempting to ensure + /// a minimum range width of 1 char by shifting the head in the forward + /// direction as needed. + /// + /// This method will never shift the anchor, and will only shift the + /// head in the forward direction. Therefore, this method can fail + /// at ensuring the minimum width if and only if the passed range is + /// both zero-width and at the end of the `RopeSlice`. + /// + /// If the input range is grapheme-boundary aligned, the returned range + /// will also be. Specifically, if the head needs to shift to achieve + /// the minimum width, it will shift to the next grapheme boundary. + #[must_use] + #[inline] + pub fn min_width_1(&self, slice: RopeSlice) -> Self { + if self.anchor == self.head { + Range { + anchor: self.anchor, + head: next_grapheme_boundary(slice, self.head), + horiz: self.horiz, + } + } else { + *self + } + } + + /// Compute a possibly new range from this range, with its ends + /// shifted as needed to align with grapheme boundaries. + /// + /// Zero-width ranges will always stay zero-width, and non-zero-width + /// ranges will never collapse to zero-width. + #[must_use] + pub fn grapheme_aligned(&self, slice: RopeSlice) -> Self { + use std::cmp::Ordering; + let (new_anchor, new_head) = match self.anchor.cmp(&self.head) { + Ordering::Equal => { + let pos = ensure_grapheme_boundary_prev(slice, self.anchor); + (pos, pos) + } + Ordering::Less => ( + ensure_grapheme_boundary_prev(slice, self.anchor), + ensure_grapheme_boundary_next(slice, self.head), + ), + Ordering::Greater => ( + ensure_grapheme_boundary_next(slice, self.anchor), + ensure_grapheme_boundary_prev(slice, self.head), + ), + }; + Range { + anchor: new_anchor, + head: new_head, + horiz: if new_anchor == self.anchor { + self.horiz } else { - to + None }, - horiz: None, } } @@ -126,7 +212,7 @@ impl Range { #[inline] pub fn fragment<'a, 'b: 'a>(&'a self, text: RopeSlice<'b>) -> Cow<'b, str> { - Cow::from(text.slice(self.from()..self.to() + 1)) + text.slice(self.from()..self.to()).into() } } @@ -175,10 +261,8 @@ impl Selection { } pub fn push(mut self, range: Range) -> Self { - let index = self.ranges.len(); self.ranges.push(range); - - Self::normalize(self.ranges, index) + self.normalize() } // replace_range @@ -224,80 +308,68 @@ impl Selection { Self::single(pos, pos) } - fn normalize(mut ranges: SmallVec<[Range; 1]>, mut primary_index: usize) -> Self { - let primary = ranges[primary_index]; - ranges.sort_unstable_by_key(Range::from); - primary_index = ranges.iter().position(|&range| range == primary).unwrap(); - - let mut result = SmallVec::with_capacity(ranges.len()); // approx - - // TODO: we could do with one vec by removing elements as we mutate - - let mut i = 0; - - for range in ranges.into_iter() { - // if previous value exists - if let Some(prev) = result.last_mut() { - // and we overlap it - - // TODO: we used to simply check range.from() <(=) prev.to() - // avoiding two comparisons - if range.overlaps(prev) { - let from = prev.from(); - let to = std::cmp::max(range.to(), prev.to()); - - if i <= primary_index { - primary_index -= 1 - } - - // merge into previous - if range.anchor > range.head { - prev.anchor = to; - prev.head = from; - } else { - prev.anchor = from; - prev.head = to; - } - continue; + /// Normalizes a `Selection`. + fn normalize(mut self) -> Self { + let primary = self.ranges[self.primary_index]; + self.ranges.sort_unstable_by_key(Range::from); + self.primary_index = self + .ranges + .iter() + .position(|&range| range == primary) + .unwrap(); + + let mut prev_i = 0; + for i in 1..self.ranges.len() { + if self.ranges[prev_i].overlaps(&self.ranges[i]) { + if i == self.primary_index { + self.primary_index = prev_i; } + self.ranges[prev_i] = self.ranges[prev_i].merge(self.ranges[i]); + } else { + prev_i += 1; + self.ranges[prev_i] = self.ranges[i]; } - - result.push(range); - i += 1 } - Self { - ranges: result, - primary_index, - } + self.ranges.truncate(prev_i + 1); + + self } // TODO: consume an iterator or a vec to reduce allocations? #[must_use] pub fn new(ranges: SmallVec<[Range; 1]>, primary_index: usize) -> Self { assert!(!ranges.is_empty()); + debug_assert!(primary_index < ranges.len()); - // fast path for a single selection (cursor) - if ranges.len() == 1 { - return Self { - ranges, - primary_index: 0, - }; + let mut selection = Self { + ranges, + primary_index, + }; + + if selection.ranges.len() > 1 { + // TODO: only normalize if needed (any ranges out of order) + selection = selection.normalize(); } - // TODO: only normalize if needed (any ranges out of order) - Self::normalize(ranges, primary_index) + selection } - /// Takes a closure and maps each selection over the closure. - pub fn transform<F>(&self, f: F) -> Self + /// Takes a closure and maps each `Range` over the closure. + pub fn transform<F>(mut self, f: F) -> Self where F: Fn(Range) -> Range, { - Self::new( - self.ranges.iter().copied().map(f).collect(), - self.primary_index, - ) + for range in self.ranges.iter_mut() { + *range = f(*range) + } + + self.normalize() + } + + /// A convenience short-cut for `transform(|r| r.min_width_1(text))`. + pub fn min_width_1(self, text: RopeSlice) -> Self { + self.transform(|r| r.min_width_1(text)) } pub fn fragments<'a>(&'a self, text: RopeSlice<'a>) -> impl Iterator<Item = Cow<str>> + 'a { @@ -363,7 +435,7 @@ pub fn select_on_matches( let start = text.byte_to_char(start_byte + mat.start()); let end = text.byte_to_char(start_byte + mat.end()); - result.push(Range::new(start, end.saturating_sub(1))); + result.push(Range::new(start, end)); } } @@ -384,6 +456,12 @@ pub fn split_on_matches( let mut result = SmallVec::with_capacity(selection.len()); for sel in selection { + // Special case: zero-width selection. + if sel.from() == sel.to() { + result.push(*sel); + continue; + } + // TODO: can't avoid occasional allocations since Regex can't operate on chunks yet let fragment = sel.fragment(text); @@ -396,13 +474,12 @@ pub fn split_on_matches( for mat in regex.find_iter(&fragment) { // TODO: retain range direction - let end = text.byte_to_char(start_byte + mat.start()); - result.push(Range::new(start, end.saturating_sub(1))); + result.push(Range::new(start, end)); start = text.byte_to_char(start_byte + mat.end()); } - if start <= sel_end { + if start < sel_end { result.push(Range::new(start, sel_end)); } } @@ -484,7 +561,7 @@ mod test { .collect::<Vec<String>>() .join(","); - assert_eq!(res, "8/10,10/12"); + assert_eq!(res, "8/10,10/12,12/12"); } #[test] @@ -498,35 +575,171 @@ mod test { assert_eq!(range.contains(13), false); let range = Range::new(9, 6); - assert_eq!(range.contains(9), true); + assert_eq!(range.contains(9), false); assert_eq!(range.contains(7), true); - assert_eq!(range.contains(6), false); + assert_eq!(range.contains(6), true); + } + + #[test] + fn test_overlaps() { + fn overlaps(a: (usize, usize), b: (usize, usize)) -> bool { + Range::new(a.0, a.1).overlaps(&Range::new(b.0, b.1)) + } + + // Two non-zero-width ranges, no overlap. + assert!(!overlaps((0, 3), (3, 6))); + assert!(!overlaps((0, 3), (6, 3))); + assert!(!overlaps((3, 0), (3, 6))); + assert!(!overlaps((3, 0), (6, 3))); + assert!(!overlaps((3, 6), (0, 3))); + assert!(!overlaps((3, 6), (3, 0))); + assert!(!overlaps((6, 3), (0, 3))); + assert!(!overlaps((6, 3), (3, 0))); + + // Two non-zero-width ranges, overlap. + assert!(overlaps((0, 4), (3, 6))); + assert!(overlaps((0, 4), (6, 3))); + assert!(overlaps((4, 0), (3, 6))); + assert!(overlaps((4, 0), (6, 3))); + assert!(overlaps((3, 6), (0, 4))); + assert!(overlaps((3, 6), (4, 0))); + assert!(overlaps((6, 3), (0, 4))); + assert!(overlaps((6, 3), (4, 0))); + + // Zero-width and non-zero-width range, no overlap. + assert!(!overlaps((0, 3), (3, 3))); + assert!(!overlaps((3, 0), (3, 3))); + assert!(!overlaps((3, 3), (0, 3))); + assert!(!overlaps((3, 3), (3, 0))); + + // Zero-width and non-zero-width range, overlap. + assert!(overlaps((1, 4), (1, 1))); + assert!(overlaps((4, 1), (1, 1))); + assert!(overlaps((1, 1), (1, 4))); + assert!(overlaps((1, 1), (4, 1))); + + assert!(overlaps((1, 4), (3, 3))); + assert!(overlaps((4, 1), (3, 3))); + assert!(overlaps((3, 3), (1, 4))); + assert!(overlaps((3, 3), (4, 1))); + + // Two zero-width ranges, no overlap. + assert!(!overlaps((0, 0), (1, 1))); + assert!(!overlaps((1, 1), (0, 0))); + + // Two zero-width ranges, overlap. + assert!(overlaps((1, 1), (1, 1))); + } + + #[test] + fn test_graphem_aligned() { + let r = Rope::from_str("\r\nHi\r\n"); + let s = r.slice(..); + + // Zero-width. + assert_eq!(Range::new(0, 0).grapheme_aligned(s), Range::new(0, 0)); + assert_eq!(Range::new(1, 1).grapheme_aligned(s), Range::new(0, 0)); + assert_eq!(Range::new(2, 2).grapheme_aligned(s), Range::new(2, 2)); + assert_eq!(Range::new(3, 3).grapheme_aligned(s), Range::new(3, 3)); + assert_eq!(Range::new(4, 4).grapheme_aligned(s), Range::new(4, 4)); + assert_eq!(Range::new(5, 5).grapheme_aligned(s), Range::new(4, 4)); + assert_eq!(Range::new(6, 6).grapheme_aligned(s), Range::new(6, 6)); + + // Forward. + assert_eq!(Range::new(0, 1).grapheme_aligned(s), Range::new(0, 2)); + assert_eq!(Range::new(1, 2).grapheme_aligned(s), Range::new(0, 2)); + assert_eq!(Range::new(2, 3).grapheme_aligned(s), Range::new(2, 3)); + assert_eq!(Range::new(3, 4).grapheme_aligned(s), Range::new(3, 4)); + assert_eq!(Range::new(4, 5).grapheme_aligned(s), Range::new(4, 6)); + assert_eq!(Range::new(5, 6).grapheme_aligned(s), Range::new(4, 6)); + + assert_eq!(Range::new(0, 2).grapheme_aligned(s), Range::new(0, 2)); + assert_eq!(Range::new(1, 3).grapheme_aligned(s), Range::new(0, 3)); + assert_eq!(Range::new(2, 4).grapheme_aligned(s), Range::new(2, 4)); + assert_eq!(Range::new(3, 5).grapheme_aligned(s), Range::new(3, 6)); + assert_eq!(Range::new(4, 6).grapheme_aligned(s), Range::new(4, 6)); + + // Reverse. + assert_eq!(Range::new(1, 0).grapheme_aligned(s), Range::new(2, 0)); + assert_eq!(Range::new(2, 1).grapheme_aligned(s), Range::new(2, 0)); + assert_eq!(Range::new(3, 2).grapheme_aligned(s), Range::new(3, 2)); + assert_eq!(Range::new(4, 3).grapheme_aligned(s), Range::new(4, 3)); + assert_eq!(Range::new(5, 4).grapheme_aligned(s), Range::new(6, 4)); + assert_eq!(Range::new(6, 5).grapheme_aligned(s), Range::new(6, 4)); + + assert_eq!(Range::new(2, 0).grapheme_aligned(s), Range::new(2, 0)); + assert_eq!(Range::new(3, 1).grapheme_aligned(s), Range::new(3, 0)); + assert_eq!(Range::new(4, 2).grapheme_aligned(s), Range::new(4, 2)); + assert_eq!(Range::new(5, 3).grapheme_aligned(s), Range::new(6, 3)); + assert_eq!(Range::new(6, 4).grapheme_aligned(s), Range::new(6, 4)); + } + + #[test] + fn test_min_width_1() { + let r = Rope::from_str("\r\nHi\r\n"); + let s = r.slice(..); + + // Zero-width. + assert_eq!(Range::new(0, 0).min_width_1(s), Range::new(0, 2)); + assert_eq!(Range::new(1, 1).min_width_1(s), Range::new(1, 2)); + assert_eq!(Range::new(2, 2).min_width_1(s), Range::new(2, 3)); + assert_eq!(Range::new(3, 3).min_width_1(s), Range::new(3, 4)); + assert_eq!(Range::new(4, 4).min_width_1(s), Range::new(4, 6)); + assert_eq!(Range::new(5, 5).min_width_1(s), Range::new(5, 6)); + assert_eq!(Range::new(6, 6).min_width_1(s), Range::new(6, 6)); + + // Forward. + assert_eq!(Range::new(0, 1).min_width_1(s), Range::new(0, 1)); + assert_eq!(Range::new(1, 2).min_width_1(s), Range::new(1, 2)); + assert_eq!(Range::new(2, 3).min_width_1(s), Range::new(2, 3)); + assert_eq!(Range::new(3, 4).min_width_1(s), Range::new(3, 4)); + assert_eq!(Range::new(4, 5).min_width_1(s), Range::new(4, 5)); + assert_eq!(Range::new(5, 6).min_width_1(s), Range::new(5, 6)); + + // Reverse. + assert_eq!(Range::new(1, 0).min_width_1(s), Range::new(1, 0)); + assert_eq!(Range::new(2, 1).min_width_1(s), Range::new(2, 1)); + assert_eq!(Range::new(3, 2).min_width_1(s), Range::new(3, 2)); + assert_eq!(Range::new(4, 3).min_width_1(s), Range::new(4, 3)); + assert_eq!(Range::new(5, 4).min_width_1(s), Range::new(5, 4)); + assert_eq!(Range::new(6, 5).min_width_1(s), Range::new(6, 5)); } #[test] fn test_split_on_matches() { use crate::regex::Regex; - let text = Rope::from("abcd efg wrs xyz 123 456"); + let text = Rope::from(" abcd efg wrs xyz 123 456"); - let selection = Selection::new(smallvec![Range::new(0, 8), Range::new(10, 19),], 0); + let selection = Selection::new(smallvec![Range::new(0, 9), Range::new(11, 20),], 0); let result = split_on_matches(text.slice(..), &selection, &Regex::new(r"\s+").unwrap()); assert_eq!( result.ranges(), &[ - Range::new(0, 3), - Range::new(5, 7), - Range::new(10, 11), - Range::new(15, 17), - Range::new(19, 19), + // TODO: rather than this behavior, maybe we want it + // to be based on which side is the anchor? + // + // We get a leading zero-width range when there's + // a leading match because ranges are inclusive on + // the left. Imagine, for example, if the entire + // selection range were matched: you'd still want + // at least one range to remain after the split. + Range::new(0, 0), + Range::new(1, 5), + Range::new(6, 9), + Range::new(11, 13), + Range::new(16, 19), + // In contrast to the comment above, there is no + // _trailing_ zero-width range despite the trailing + // match, because ranges are exclusive on the right. ] ); assert_eq!( result.fragments(text.slice(..)).collect::<Vec<_>>(), - &["abcd", "efg", "rs", "xyz", "1"] + &["", "abcd", "efg", "rs", "xyz"] ); } } diff --git a/helix-core/src/syntax.rs b/helix-core/src/syntax.rs index 5b45a88f..84a5f9bd 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -1759,10 +1759,20 @@ impl<I: Iterator<Item = HighlightEvent>> Iterator for Merge<I> { self.next_event = self.iter.next(); Some(event) } - // can happen if deleting and cursor at EOF, and diagnostic reaches past the end - (None, Some((_, _))) => { - self.next_span = None; - None + // Can happen if cursor at EOF and/or diagnostic reaches past the end. + // We need to actually emit events for the cursor-at-EOF situation, + // even though the range is past the end of the text. This needs to be + // handled appropriately by the drawing code by not assuming that + // all `Source` events point to valid indices in the rope. + (None, Some((span, range))) => { + let event = HighlightStart(Highlight(*span)); + self.queue.push(HighlightEnd); + self.queue.push(Source { + start: range.start, + end: range.end, + }); + self.next_span = self.spans.next(); + Some(event) } (None, None) => None, e => unreachable!("{:?}", e), |