diff options
| author | Blaž Hrastnik | 2021-07-29 09:43:20 +0000 |
|---|---|---|
| committer | GitHub | 2021-07-29 09:43:20 +0000 |
| commit | 05d20e196f81c8b71c2aecaf46f5d443d6b6b582 (patch) | |
| tree | 0642d43c12f16ac3c68c19602c64fdea8108cc97 | |
| parent | 8a2fa692f26f5bff5861151f395304837f5d93ec (diff) | |
| parent | e4d41d06e3b52863d35ce3703f78cc8e0807c504 (diff) | |
Merge pull request #376 from cessen/great_line_ending_and_cursor_range_cleanup
The Great Line Ending & Cursor Range Cleanup
| -rw-r--r-- | helix-core/src/comment.rs | 6 | ||||
| -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/match_brackets.rs | 7 | ||||
| -rw-r--r-- | helix-core/src/movement.rs | 739 | ||||
| -rw-r--r-- | helix-core/src/object.rs | 2 | ||||
| -rw-r--r-- | helix-core/src/position.rs | 144 | ||||
| -rw-r--r-- | helix-core/src/search.rs | 38 | ||||
| -rw-r--r-- | helix-core/src/selection.rs | 662 | ||||
| -rw-r--r-- | helix-core/src/surround.rs | 75 | ||||
| -rw-r--r-- | helix-core/src/syntax.rs | 18 | ||||
| -rw-r--r-- | helix-core/src/textobject.rs | 261 | ||||
| -rw-r--r-- | helix-term/src/commands.rs | 621 | ||||
| -rw-r--r-- | helix-term/src/ui/completion.rs | 20 | ||||
| -rw-r--r-- | helix-term/src/ui/editor.rs | 103 | ||||
| -rw-r--r-- | helix-view/src/document.rs | 45 | ||||
| -rw-r--r-- | helix-view/src/editor.rs | 13 | ||||
| -rw-r--r-- | helix-view/src/view.rs | 14 |
18 files changed, 1743 insertions, 1055 deletions
diff --git a/helix-core/src/comment.rs b/helix-core/src/comment.rs index fadd88e0..6fc1234d 100644 --- a/helix-core/src/comment.rs +++ b/helix-core/src/comment.rs @@ -64,8 +64,10 @@ pub fn toggle_line_comments(doc: &Rope, selection: &Selection, token: Option<&st let mut min_next_line = 0; for selection in selection { - let start = text.char_to_line(selection.from()).max(min_next_line); - let end = text.char_to_line(selection.to()) + 1; + let (start, end) = selection.line_range(text); + let start = start.max(min_next_line).min(text.len_lines()); + let end = (end + 1).min(text.len_lines()); + lines.extend(start..end); min_next_line = end + 1; } 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/match_brackets.rs b/helix-core/src/match_brackets.rs index 2aa87620..f3d9e845 100644 --- a/helix-core/src/match_brackets.rs +++ b/helix-core/src/match_brackets.rs @@ -24,12 +24,13 @@ pub fn find(syntax: &Syntax, doc: &Rope, pos: usize) -> Option<usize> { return None; } - let start_byte = node.start_byte(); let len = doc.len_bytes(); - if start_byte >= len { + let start_byte = node.start_byte(); + let end_byte = node.end_byte() - 1; // it's end exclusive + if start_byte >= len || end_byte >= len { return None; } - let end_byte = node.end_byte() - 1; // it's end exclusive + let start_char = doc.byte_to_char(start_byte); let end_char = doc.byte_to_char(end_byte); diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs index f9e5deb4..74307636 100644 --- a/helix-core/src/movement.rs +++ b/helix-core/src/movement.rs @@ -1,12 +1,14 @@ -use std::iter::{self, from_fn}; +use std::iter; 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, + }, pos_at_coords, Position, Range, RopeSlice, }; @@ -29,25 +31,16 @@ 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) - } - Direction::Forward => { - let end_char_idx = line_end_char_index(&slice, line); - nth_next_grapheme_boundary(slice, pos, count).min(end_char_idx) - } - }; - let anchor = match behaviour { - Movement::Extend => range.anchor, - Movement::Move => pos, + let pos = range.cursor(slice); + + // Compute the new position. + let new_pos = match dir { + Direction::Forward => nth_next_grapheme_boundary(slice, pos, count), + Direction::Backward => nth_prev_grapheme_boundary(slice, pos, count), }; - Range::new(anchor, pos) + + // Compute the final new range. + range.put_cursor(slice, new_pos, behaviour == Movement::Extend) } pub fn move_vertically( @@ -57,36 +50,28 @@ pub fn move_vertically( count: usize, behaviour: Movement, ) -> Range { - let Position { row, col } = coords_at_pos(slice, range.head); + let pos = range.cursor(slice); + // 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 { + // Compute the new position. + let new_row = match dir { + Direction::Forward => (row + count).min(slice.len_lines().saturating_sub(1)), Direction::Backward => row.saturating_sub(count), - Direction::Forward => std::cmp::min( - row.saturating_add(count), - slice.len_lines().saturating_sub(2), - ), }; + let new_col = col.max(horiz as usize); + let new_pos = pos_at_coords(slice, Position::new(new_row, new_col), true); - // 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) - }; - - let new_col = std::cmp::min(horiz as usize, new_line_len); - - let pos = pos_at_coords(slice, Position::new(new_line, new_col)); - - let anchor = match behaviour { - Movement::Extend => range.anchor, - Movement::Move => pos, - }; + // Special-case to avoid moving to the end of the last non-empty line. + if behaviour == Movement::Extend && slice.line(new_row).len_chars() == 0 { + return range; + } - let mut range = Range::new(anchor, pos); - range.horiz = Some(horiz); - range + let mut new_range = range.put_cursor(slice, new_pos, behaviour == Movement::Extend); + new_range.horiz = Some(horiz); + new_range } pub fn move_next_word_start(slice: RopeSlice, range: Range, count: usize) -> Range { @@ -118,8 +103,41 @@ pub fn move_prev_word_end(slice: RopeSlice, range: Range, count: usize) -> Range } fn word_move(slice: RopeSlice, range: Range, count: usize, target: WordMotionTarget) -> Range { - (0..count).fold(range, |range, _| { - slice.chars_at(range.head).range_to_target(target, range) + let is_prev = matches!( + target, + WordMotionTarget::PrevWordStart + | WordMotionTarget::PrevLongWordStart + | WordMotionTarget::PrevWordEnd + ); + + // Special-case early-out. + if (is_prev && range.head == 0) || (!is_prev && range.head == slice.len_chars()) { + return range; + } + + // Prepare the range appropriately based on the target movement + // direction. This is addressing two things at once: + // + // 1. Block-cursor semantics. + // 2. The anchor position being irrelevant to the output result. + #[allow(clippy::collapsible_else_if)] // Makes the structure clearer in this case. + let start_range = if is_prev { + if range.anchor < range.head { + Range::new(range.head, prev_grapheme_boundary(slice, range.head)) + } else { + Range::new(next_grapheme_boundary(slice, range.head), range.head) + } + } else { + if range.anchor < range.head { + Range::new(prev_grapheme_boundary(slice, range.head), range.head) + } else { + Range::new(range.head, next_grapheme_boundary(slice, range.head)) + } + }; + + // Do the main work. + (0..count).fold(start_range, |r, _| { + slice.chars_at(r.head).range_to_target(target, r) }) } @@ -176,79 +194,75 @@ pub trait CharHelpers { fn range_to_target(&mut self, target: WordMotionTarget, origin: Range) -> Range; } -enum WordMotionPhase { - Start, - SkipNewlines, - ReachTarget, -} - impl CharHelpers for Chars<'_> { + /// Note: this only changes the anchor of the range if the head is effectively + /// starting on a boundary (either directly or after skipping newline characters). + /// Any other changes to the anchor should be handled by the calling code. fn range_to_target(&mut self, target: WordMotionTarget, origin: Range) -> Range { - // Characters are iterated forward or backwards depending on the motion direction. - let characters: Box<dyn Iterator<Item = char>> = match target { + let is_prev = matches!( + target, WordMotionTarget::PrevWordStart - | WordMotionTarget::PrevLongWordStart - | WordMotionTarget::PrevWordEnd => { + | WordMotionTarget::PrevLongWordStart + | WordMotionTarget::PrevWordEnd + ); + + // Reverse the iterator if needed for the motion direction. + if is_prev { + self.reverse(); + } + + // Function to advance index in the appropriate motion direction. + let advance: &dyn Fn(&mut usize) = if is_prev { + &|idx| *idx = idx.saturating_sub(1) + } else { + &|idx| *idx += 1 + }; + + // Initialize state variables. + let mut anchor = origin.anchor; + let mut head = origin.head; + let mut prev_ch = { + let ch = self.prev(); + if ch.is_some() { self.next(); - Box::new(from_fn(|| self.prev())) } - _ => Box::new(self), + ch }; - // Index advancement also depends on the direction. - let advance: &dyn Fn(&mut usize) = match target { - WordMotionTarget::PrevWordStart - | WordMotionTarget::PrevLongWordStart - | WordMotionTarget::PrevWordEnd => &|u| *u = u.saturating_sub(1), - _ => &|u| *u += 1, - }; + // Skip any initial newline characters. + while let Some(ch) = self.next() { + if char_is_line_ending(ch) { + prev_ch = Some(ch); + advance(&mut head); + } else { + self.prev(); + break; + } + } + if prev_ch.map(char_is_line_ending).unwrap_or(false) { + anchor = head; + } - let mut characters = characters.peekable(); - let mut phase = WordMotionPhase::Start; - let mut head = origin.head; - let mut anchor: Option<usize> = None; - let is_boundary = - |a: char, b: Option<char>| categorize_char(a) != categorize_char(b.unwrap_or(a)); - while let Some(peek) = characters.peek().copied() { - phase = match phase { - WordMotionPhase::Start => { - characters.next(); - if characters.peek().is_none() { - break; // We're at the end, so there's nothing to do. - } - // Anchor may remain here if the head wasn't at a boundary - if !is_boundary(peek, characters.peek().copied()) && !char_is_line_ending(peek) - { - anchor = Some(head); - } - // First character is always skipped by the head - advance(&mut head); - WordMotionPhase::SkipNewlines - } - WordMotionPhase::SkipNewlines => { - if char_is_line_ending(peek) { - characters.next(); - if characters.peek().is_some() { - advance(&mut head); - } - WordMotionPhase::SkipNewlines - } else { - WordMotionPhase::ReachTarget - } - } - WordMotionPhase::ReachTarget => { - characters.next(); - anchor = anchor.or(Some(head)); - if reached_target(target, peek, characters.peek()) { - break; - } else { - advance(&mut head); - } - WordMotionPhase::ReachTarget + // Find our target position(s). + let head_start = head; + while let Some(next_ch) = self.next() { + if prev_ch.is_none() || reached_target(target, prev_ch.unwrap(), next_ch) { + if head == head_start { + anchor = head; + } else { + break; } } + prev_ch = Some(next_ch); + advance(&mut head); + } + + // Un-reverse the iterator if needed. + if is_prev { + self.reverse(); } - Range::new(anchor.unwrap_or(origin.anchor), head) + + Range::new(anchor, head) } } @@ -265,28 +279,23 @@ fn is_long_word_boundary(a: char, b: char) -> bool { } } -fn reached_target(target: WordMotionTarget, peek: char, next_peek: Option<&char>) -> bool { - let next_peek = match next_peek { - Some(next_peek) => next_peek, - None => return true, - }; - +fn reached_target(target: WordMotionTarget, prev_ch: char, next_ch: char) -> bool { match target { WordMotionTarget::NextWordStart | WordMotionTarget::PrevWordEnd => { - is_word_boundary(peek, *next_peek) - && (char_is_line_ending(*next_peek) || !next_peek.is_whitespace()) + is_word_boundary(prev_ch, next_ch) + && (char_is_line_ending(next_ch) || !next_ch.is_whitespace()) } WordMotionTarget::NextWordEnd | WordMotionTarget::PrevWordStart => { - is_word_boundary(peek, *next_peek) - && (!peek.is_whitespace() || char_is_line_ending(*next_peek)) + is_word_boundary(prev_ch, next_ch) + && (!prev_ch.is_whitespace() || char_is_line_ending(next_ch)) } WordMotionTarget::NextLongWordStart => { - is_long_word_boundary(peek, *next_peek) - && (char_is_line_ending(*next_peek) || !next_peek.is_whitespace()) + is_long_word_boundary(prev_ch, next_ch) + && (char_is_line_ending(next_ch) || !next_ch.is_whitespace()) } WordMotionTarget::NextLongWordEnd | WordMotionTarget::PrevLongWordStart => { - is_long_word_boundary(peek, *next_peek) - && (!peek.is_whitespace() || char_is_line_ending(*next_peek)) + is_long_word_boundary(prev_ch, next_ch) + && (!prev_ch.is_whitespace() || char_is_line_ending(next_ch)) } } } @@ -317,7 +326,7 @@ mod test { fn test_vertical_move() { let text = Rope::from("abcd\nefg\nwrs"); let slice = text.slice(..); - let pos = pos_at_coords(slice, (0, 4).into()); + let pos = pos_at_coords(slice, (0, 4).into(), true); let range = Range::new(pos, pos); assert_eq!( @@ -330,10 +339,10 @@ 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()); + let position = pos_at_coords(slice, (0, 0).into(), true); let mut range = Range::point(position); @@ -353,23 +362,23 @@ 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()); + let position = pos_at_coords(slice, (0, 0).into(), true); 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 { @@ -383,7 +392,7 @@ mod test { fn selection_extending_moves_in_single_line_text() { let text = Rope::from(SINGLE_LINE_SAMPLE); let slice = text.slice(..); - let position = pos_at_coords(slice, (0, 0).into()); + let position = pos_at_coords(slice, (0, 0).into(), true); let mut range = Range::point(position); let original_anchor = range.anchor; @@ -403,18 +412,19 @@ mod test { #[test] fn vertical_moves_in_single_column() { let text = Rope::from(MULTILINE_SAMPLE); - let slice = dbg!(&text).slice(..); - let position = pos_at_coords(slice, (0, 0).into()); + let slice = text.slice(..); + let position = pos_at_coords(slice, (0, 0).into(), true); let mut range = Range::point(position); 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 { @@ -428,7 +438,7 @@ mod test { fn vertical_moves_jumping_column() { let text = Rope::from(MULTILINE_SAMPLE); let slice = text.slice(..); - let position = pos_at_coords(slice, (0, 0).into()); + let position = pos_at_coords(slice, (0, 0).into(), true); let mut range = Range::point(position); enum Axis { @@ -446,7 +456,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 { @@ -460,10 +471,10 @@ mod test { } #[test] - fn multibyte_character_column_jumps() { + fn multibyte_character_wide_column_jumps() { let text = Rope::from(MULTIBYTE_CHARACTER_SAMPLE); let slice = text.slice(..); - let position = pos_at_coords(slice, (0, 0).into()); + let position = pos_at_coords(slice, (0, 0).into(), true); let mut range = Range::point(position); // FIXME: The behaviour captured in this test diverges from both Kakoune and Vim. These @@ -474,10 +485,14 @@ mod test { V, } let moves_and_expected_coordinates = IntoIter::new([ - // Places cursor at the fourth kana + // Places cursor at the fourth kana. ((Axis::H, Direction::Forward, 4), (0, 4)), - // Descent places cursor at the fourth character. + // Descent places cursor at the 4th character. ((Axis::V, Direction::Forward, 1usize), (1, 4)), + // Moving back 1 character. + ((Axis::H, Direction::Backward, 1usize), (1, 3)), + // Jumping back up 1 line. + ((Axis::V, Direction::Backward, 1usize), (0, 3)), ]); for ((axis, direction, amount), coordinates) in moves_and_expected_coordinates { @@ -512,42 +527,42 @@ mod test { fn test_behaviour_when_moving_to_start_of_next_words() { let tests = array::IntoIter::new([ ("Basic forward motion stops at the first space", - vec![(1, Range::new(0, 0), Range::new(0, 5))]), + vec![(1, Range::new(0, 0), Range::new(0, 6))]), (" Starting from a boundary advances the anchor", - vec![(1, Range::new(0, 0), Range::new(1, 9))]), + vec![(1, Range::new(0, 0), Range::new(1, 10))]), ("Long whitespace gap is bridged by the head", - vec![(1, Range::new(0, 0), Range::new(0, 10))]), + vec![(1, Range::new(0, 0), Range::new(0, 11))]), ("Previous anchor is irrelevant for forward motions", - vec![(1, Range::new(12, 0), Range::new(0, 8))]), + vec![(1, Range::new(12, 0), Range::new(0, 9))]), (" Starting from whitespace moves to last space in sequence", - vec![(1, Range::new(0, 0), Range::new(0, 3))]), + vec![(1, Range::new(0, 0), Range::new(0, 4))]), ("Starting from mid-word leaves anchor at start position and moves head", - vec![(1, Range::new(3, 3), Range::new(3, 8))]), + vec![(1, Range::new(3, 3), Range::new(3, 9))]), ("Identifiers_with_underscores are considered a single word", - vec![(1, Range::new(0, 0), Range::new(0, 28))]), + vec![(1, Range::new(0, 0), Range::new(0, 29))]), ("Jumping\n into starting whitespace selects the spaces before 'into'", - vec![(1, Range::new(0, 6), Range::new(8, 11))]), + vec![(1, Range::new(0, 7), Range::new(8, 12))]), ("alphanumeric.!,and.?=punctuation are considered 'words' for the purposes of word motion", vec![ - (1, Range::new(0, 0), Range::new(0, 11)), - (1, Range::new(0, 11), Range::new(12, 14)), - (1, Range::new(12, 14), Range::new(15, 17)) + (1, Range::new(0, 0), Range::new(0, 12)), + (1, Range::new(0, 12), Range::new(12, 15)), + (1, Range::new(12, 15), Range::new(15, 18)) ]), ("... ... punctuation and spaces behave as expected", vec![ - (1, Range::new(0, 0), Range::new(0, 5)), - (1, Range::new(0, 5), Range::new(6, 9)), + (1, Range::new(0, 0), Range::new(0, 6)), + (1, Range::new(0, 6), Range::new(6, 10)), ]), (".._.._ punctuation is not joined by underscores into a single block", - vec![(1, Range::new(0, 0), Range::new(0, 1))]), + vec![(1, Range::new(0, 0), Range::new(0, 2))]), ("Newlines\n\nare bridged seamlessly.", vec![ - (1, Range::new(0, 0), Range::new(0, 7)), - (1, Range::new(0, 7), Range::new(10, 13)), + (1, Range::new(0, 0), Range::new(0, 8)), + (1, Range::new(0, 8), Range::new(10, 14)), ]), ("Jumping\n\n\n\n\n\n from newlines to whitespace selects whitespace.", vec![ - (1, Range::new(0, 8), Range::new(13, 15)), + (1, Range::new(0, 9), Range::new(13, 16)), ]), ("A failed motion does not modify the range", vec![ @@ -555,17 +570,17 @@ mod test { ]), ("oh oh oh two character words!", vec![ - (1, Range::new(0, 0), Range::new(0, 2)), - (1, Range::new(0, 2), Range::new(3, 5)), - (1, Range::new(0, 1), Range::new(2, 2)), + (1, Range::new(0, 0), Range::new(0, 3)), + (1, Range::new(0, 3), Range::new(3, 6)), + (1, Range::new(0, 2), Range::new(1, 3)), ]), ("Multiple motions at once resolve correctly", vec![ - (3, Range::new(0, 0), Range::new(17, 19)), + (3, Range::new(0, 0), Range::new(17, 20)), ]), ("Excessive motions are performed partially", vec![ - (999, Range::new(0, 0), Range::new(32, 40)), + (999, Range::new(0, 0), Range::new(32, 41)), ]), ("", // Edge case of moving forward in empty string vec![ @@ -573,16 +588,16 @@ mod test { ]), ("\n\n\n\n\n", // Edge case of moving forward in all newlines vec![ - (1, Range::new(0, 0), Range::new(0, 4)), + (1, Range::new(0, 0), Range::new(5, 5)), ]), ("\n \n \n Jumping through alternated space blocks and newlines selects the space blocks", vec![ - (1, Range::new(0, 0), Range::new(1, 3)), - (1, Range::new(1, 3), Range::new(5, 7)), + (1, Range::new(0, 0), Range::new(1, 4)), + (1, Range::new(1, 4), Range::new(5, 8)), ]), ("ヒーリクス multibyte characters behave as normal characters", vec![ - (1, Range::new(0, 0), Range::new(0, 5)), + (1, Range::new(0, 0), Range::new(0, 6)), ]), ]); @@ -598,40 +613,40 @@ mod test { fn test_behaviour_when_moving_to_start_of_next_long_words() { let tests = array::IntoIter::new([ ("Basic forward motion stops at the first space", - vec![(1, Range::new(0, 0), Range::new(0, 5))]), + vec![(1, Range::new(0, 0), Range::new(0, 6))]), (" Starting from a boundary advances the anchor", - vec![(1, Range::new(0, 0), Range::new(1, 9))]), + vec![(1, Range::new(0, 0), Range::new(1, 10))]), ("Long whitespace gap is bridged by the head", - vec![(1, Range::new(0, 0), Range::new(0, 10))]), + vec![(1, Range::new(0, 0), Range::new(0, 11))]), ("Previous anchor is irrelevant for forward motions", - vec![(1, Range::new(12, 0), Range::new(0, 8))]), + vec![(1, Range::new(12, 0), Range::new(0, 9))]), (" Starting from whitespace moves to last space in sequence", - vec![(1, Range::new(0, 0), Range::new(0, 3))]), + vec![(1, Range::new(0, 0), Range::new(0, 4))]), ("Starting from mid-word leaves anchor at start position and moves head", - vec![(1, Range::new(3, 3), Range::new(3, 8))]), + vec![(1, Range::new(3, 3), Range::new(3, 9))]), ("Identifiers_with_underscores are considered a single word", - vec![(1, Range::new(0, 0), Range::new(0, 28))]), + vec![(1, Range::new(0, 0), Range::new(0, 29))]), ("Jumping\n into starting whitespace selects the spaces before 'into'", - vec![(1, Range::new(0, 6), Range::new(8, 11))]), + vec![(1, Range::new(0, 7), Range::new(8, 12))]), ("alphanumeric.!,and.?=punctuation are not treated any differently than alphanumerics", vec![ - (1, Range::new(0, 0), Range::new(0, 32)), + (1, Range::new(0, 0), Range::new(0, 33)), ]), ("... ... punctuation and spaces behave as expected", vec![ - (1, Range::new(0, 0), Range::new(0, 5)), - (1, Range::new(0, 5), Range::new(6, 9)), + (1, Range::new(0, 0), Range::new(0, 6)), + (1, Range::new(0, 6), Range::new(6, 10)), ]), (".._.._ punctuation is joined by underscores into a single word, as it behaves like alphanumerics", - vec![(1, Range::new(0, 0), Range::new(0, 6))]), + vec![(1, Range::new(0, 0), Range::new(0, 7))]), ("Newlines\n\nare bridged seamlessly.", vec![ - (1, Range::new(0, 0), Range::new(0, 7)), - (1, Range::new(0, 7), Range::new(10, 13)), + (1, Range::new(0, 0), Range::new(0, 8)), + (1, Range::new(0, 8), Range::new(10, 14)), ]), ("Jumping\n\n\n\n\n\n from newlines to whitespace selects whitespace.", vec![ - (1, Range::new(0, 8), Range::new(13, 15)), + (1, Range::new(0, 9), Range::new(13, 16)), ]), ("A failed motion does not modify the range", vec![ @@ -639,17 +654,17 @@ mod test { ]), ("oh oh oh two character words!", vec![ - (1, Range::new(0, 0), Range::new(0, 2)), - (1, Range::new(0, 2), Range::new(3, 5)), - (1, Range::new(0, 1), Range::new(2, 2)), + (1, Range::new(0, 0), Range::new(0, 3)), + (1, Range::new(0, 3), Range::new(3, 6)), + (1, Range::new(0, 1), Range::new(0, 3)), ]), ("Multiple motions at once resolve correctly", vec![ - (3, Range::new(0, 0), Range::new(17, 19)), + (3, Range::new(0, 0), Range::new(17, 20)), ]), ("Excessive motions are performed partially", vec![ - (999, Range::new(0, 0), Range::new(32, 40)), + (999, Range::new(0, 0), Range::new(32, 41)), ]), ("", // Edge case of moving forward in empty string vec![ @@ -657,16 +672,16 @@ mod test { ]), ("\n\n\n\n\n", // Edge case of moving forward in all newlines vec![ - (1, Range::new(0, 0), Range::new(0, 4)), + (1, Range::new(0, 0), Range::new(5, 5)), ]), ("\n \n \n Jumping through alternated space blocks and newlines selects the space blocks", vec![ - (1, Range::new(0, 0), Range::new(1, 3)), - (1, Range::new(1, 3), Range::new(5, 7)), + (1, Range::new(0, 0), Range::new(1, 4)), + (1, Range::new(1, 4), Range::new(5, 8)), ]), ("ヒー..リクス multibyte characters behave as normal characters, including their interaction with punctuation", vec![ - (1, Range::new(0, 0), Range::new(0, 7)), + (1, Range::new(0, 0), Range::new(0, 8)), ]), ]); @@ -682,44 +697,47 @@ mod test { fn test_behaviour_when_moving_to_start_of_previous_words() { let tests = array::IntoIter::new([ ("Basic backward motion from the middle of a word", - vec![(1, Range::new(3, 3), Range::new(3, 0))]), - ("Starting from after boundary retreats the anchor", - vec![(1, Range::new(0, 8), Range::new(7, 0))]), + vec![(1, Range::new(3, 3), Range::new(4, 0))]), + + // // Why do we want this behavior? The current behavior fails this + // // test, but seems better and more consistent. + // ("Starting from after boundary retreats the anchor", + // vec![(1, Range::new(0, 9), Range::new(8, 0))]), + (" Jump to start of a word preceded by whitespace", - vec![(1, Range::new(5, 5), Range::new(5, 4))]), + vec![(1, Range::new(5, 5), Range::new(6, 4))]), (" Jump to start of line from start of word preceded by whitespace", - vec![(1, Range::new(4, 4), Range::new(3, 0))]), + vec![(1, Range::new(4, 4), Range::new(4, 0))]), ("Previous anchor is irrelevant for backward motions", - vec![(1, Range::new(12, 5), Range::new(5, 0))]), + vec![(1, Range::new(12, 5), Range::new(6, 0))]), (" Starting from whitespace moves to first space in sequence", - vec![(1, Range::new(0, 3), Range::new(3, 0))]), + vec![(1, Range::new(0, 4), Range::new(4, 0))]), ("Identifiers_with_underscores are considered a single word", vec![(1, Range::new(0, 20), Range::new(20, 0))]), ("Jumping\n \nback through a newline selects whitespace", - vec![(1, Range::new(0, 13), Range::new(11, 8))]), + vec![(1, Range::new(0, 13), Range::new(12, 8))]), ("Jumping to start of word from the end selects the word", - vec![(1, Range::new(6, 6), Range::new(6, 0))]), + vec![(1, Range::new(6, 7), Range::new(7, 0))]), ("alphanumeric.!,and.?=punctuation are considered 'words' for the purposes of word motion", vec![ - (1, Range::new(30, 30), Range::new(30, 21)), - (1, Range::new(30, 21), Range::new(20, 18)), - (1, Range::new(20, 18), Range::new(17, 15)) + (1, Range::new(29, 30), Range::new(30, 21)), + (1, Range::new(30, 21), Range::new(21, 18)), + (1, Range::new(21, 18), Range::new(18, 15)) ]), - ("... ... punctuation and spaces behave as expected", vec![ - (1, Range::new(0, 10), Range::new(9, 6)), - (1, Range::new(9, 6), Range::new(5, 0)), + (1, Range::new(0, 10), Range::new(10, 6)), + (1, Range::new(10, 6), Range::new(6, 0)), ]), (".._.._ punctuation is not joined by underscores into a single block", - vec![(1, Range::new(0, 5), Range::new(4, 3))]), + vec![(1, Range::new(0, 6), Range::new(5, 3))]), ("Newlines\n\nare bridged seamlessly.", vec![ - (1, Range::new(0, 10), Range::new(7, 0)), + (1, Range::new(0, 10), Range::new(8, 0)), ]), ("Jumping \n\n\n\n\nback from within a newline group selects previous block", vec![ - (1, Range::new(0, 13), Range::new(10, 0)), + (1, Range::new(0, 13), Range::new(11, 0)), ]), ("Failed motions do not modify the range", vec![ @@ -727,11 +745,11 @@ mod test { ]), ("Multiple motions at once resolve correctly", vec![ - (3, Range::new(18, 18), Range::new(8, 0)), + (3, Range::new(18, 18), Range::new(9, 0)), ]), ("Excessive motions are performed partially", vec![ - (999, Range::new(40, 40), Range::new(9, 0)), + (999, Range::new(40, 40), Range::new(10, 0)), ]), ("", // Edge case of moving backwards in empty string vec![ @@ -739,16 +757,16 @@ mod test { ]), ("\n\n\n\n\n", // Edge case of moving backwards in all newlines vec![ - (1, Range::new(0, 0), Range::new(0, 0)), + (1, Range::new(5, 5), Range::new(0, 0)), ]), (" \n \nJumping back through alternated space blocks and newlines selects the space blocks", vec![ - (1, Range::new(0, 7), Range::new(6, 4)), - (1, Range::new(6, 4), Range::new(2, 0)), + (1, Range::new(0, 8), Range::new(7, 4)), + (1, Range::new(7, 4), Range::new(3, 0)), ]), ("ヒーリクス multibyte characters behave as normal characters", vec![ - (1, Range::new(0, 5), Range::new(4, 0)), + (1, Range::new(0, 6), Range::new(6, 0)), ]), ]); @@ -763,72 +781,89 @@ mod test { #[test] fn test_behaviour_when_moving_to_start_of_previous_long_words() { let tests = array::IntoIter::new([ - ("Basic backward motion from the middle of a word", - vec![(1, Range::new(3, 3), Range::new(3, 0))]), - ("Starting from after boundary retreats the anchor", - vec![(1, Range::new(0, 8), Range::new(7, 0))]), - (" Jump to start of a word preceded by whitespace", - vec![(1, Range::new(5, 5), Range::new(5, 4))]), - (" Jump to start of line from start of word preceded by whitespace", - vec![(1, Range::new(4, 4), Range::new(3, 0))]), + ( + "Basic backward motion from the middle of a word", + vec![(1, Range::new(3, 3), Range::new(4, 0))], + ), + + // // Why do we want this behavior? The current behavior fails this + // // test, but seems better and more consistent. + // ("Starting from after boundary retreats the anchor", + // vec![(1, Range::new(0, 9), Range::new(8, 0))]), + + ( + " Jump to start of a word preceded by whitespace", + vec![(1, Range::new(5, 5), Range::new(6, 4))], + ), + ( + " Jump to start of line from start of word preceded by whitespace", + vec![(1, Range::new(3, 4), Range::new(4, 0))], + ), ("Previous anchor is irrelevant for backward motions", - vec![(1, Range::new(12, 5), Range::new(5, 0))]), - (" Starting from whitespace moves to first space in sequence", - vec![(1, Range::new(0, 3), Range::new(3, 0))]), + vec![(1, Range::new(12, 5), Range::new(6, 0))]), + ( + " Starting from whitespace moves to first space in sequence", + vec![(1, Range::new(0, 4), Range::new(4, 0))], + ), ("Identifiers_with_underscores are considered a single word", vec![(1, Range::new(0, 20), Range::new(20, 0))]), - ("Jumping\n \nback through a newline selects whitespace", - vec![(1, Range::new(0, 13), Range::new(11, 8))]), - ("Jumping to start of word from the end selects the word", - vec![(1, Range::new(6, 6), Range::new(6, 0))]), - ("alphanumeric.!,and.?=punctuation are treated exactly the same", - vec![ - (1, Range::new(30, 30), Range::new(30, 0)), - ]), - - ("... ... punctuation and spaces behave as expected", + ( + "Jumping\n \nback through a newline selects whitespace", + vec![(1, Range::new(0, 13), Range::new(12, 8))], + ), + ( + "Jumping to start of word from the end selects the word", + vec![(1, Range::new(6, 7), Range::new(7, 0))], + ), + ( + "alphanumeric.!,and.?=punctuation are treated exactly the same", + vec![(1, Range::new(29, 30), Range::new(30, 0))], + ), + ( + "... ... punctuation and spaces behave as expected", vec![ - (1, Range::new(0, 10), Range::new(9, 6)), - (1, Range::new(9, 6), Range::new(5, 0)), - ]), + (1, Range::new(0, 10), Range::new(10, 6)), + (1, Range::new(10, 6), Range::new(6, 0)), + ], + ), (".._.._ punctuation is joined by underscores into a single block", - vec![(1, Range::new(0, 5), Range::new(4, 0))]), - ("Newlines\n\nare bridged seamlessly.", - vec![ - (1, Range::new(0, 10), Range::new(7, 0)), - ]), - ("Jumping \n\n\n\n\nback from within a newline group selects previous block", - vec![ - (1, Range::new(0, 13), Range::new(10, 0)), - ]), - ("Failed motions do not modify the range", - vec![ - (0, Range::new(3, 0), Range::new(3, 0)), - ]), - ("Multiple motions at once resolve correctly", - vec![ - (3, Range::new(18, 18), Range::new(8, 0)), - ]), - ("Excessive motions are performed partially", - vec![ - (999, Range::new(40, 40), Range::new(9, 0)), - ]), - ("", // Edge case of moving backwards in empty string - vec![ - (1, Range::new(0, 0), Range::new(0, 0)), - ]), - ("\n\n\n\n\n", // Edge case of moving backwards in all newlines - vec![ - (1, Range::new(0, 0), Range::new(0, 0)), - ]), + vec![(1, Range::new(0, 6), Range::new(6, 0))]), + ( + "Newlines\n\nare bridged seamlessly.", + vec![(1, Range::new(0, 10), Range::new(8, 0))], + ), + ( + "Jumping \n\n\n\n\nback from within a newline group selects previous block", + vec![(1, Range::new(0, 13), Range::new(11, 0))], + ), + ( + "Failed motions do not modify the range", + vec![(0, Range::new(3, 0), Range::new(3, 0))], + ), + ( + "Multiple motions at once resolve correctly", + vec![(3, Range::new(19, 19), Range::new(9, 0))], + ), + ( + "Excessive motions are performed partially", + vec![(999, Range::new(40, 40), Range::new(10, 0))], + ), + ( + "", // Edge case of moving backwards in empty string + vec![(1, Range::new(0, 0), Range::new(0, 0))], + ), + ( + "\n\n\n\n\n", // Edge case of moving backwards in all newlines + vec![(1, Range::new(5, 5), Range::new(0, 0))], + ), (" \n \nJumping back through alternated space blocks and newlines selects the space blocks", vec![ - (1, Range::new(0, 7), Range::new(6, 4)), - (1, Range::new(6, 4), Range::new(2, 0)), + (1, Range::new(0, 8), Range::new(7, 4)), + (1, Range::new(7, 4), Range::new(3, 0)), ]), ("ヒーリ..クス multibyte characters behave as normal characters, including when interacting with punctuation", vec![ - (1, Range::new(0, 7), Range::new(6, 0)), + (1, Range::new(0, 8), Range::new(8, 0)), ]), ]); @@ -844,42 +879,46 @@ mod test { fn test_behaviour_when_moving_to_end_of_next_words() { let tests = array::IntoIter::new([ ("Basic forward motion from the start of a word to the end of it", - vec![(1, Range::new(0, 0), Range::new(0, 4))]), + vec![(1, Range::new(0, 0), Range::new(0, 5))]), ("Basic forward motion from the end of a word to the end of the next", - vec![(1, Range::new(0, 4), Range::new(5, 12))]), + vec![(1, Range::new(0, 5), Range::new(5, 13))]), ("Basic forward motion from the middle of a word to the end of it", - vec![(1, Range::new(2, 2), Range::new(2, 4))]), + vec![(1, Range::new(2, 2), Range::new(2, 5))]), (" Jumping to end of a word preceded by whitespace", - vec![(1, Range::new(0, 0), Range::new(0, 10))]), - (" Starting from a boundary advances the anchor", - vec![(1, Range::new(0, 0), Range::new(1, 8))]), + vec![(1, Range::new(0, 0), Range::new(0, 11))]), + + // // Why do we want this behavior? The current behavior fails this + // // test, but seems better and more consistent. + // (" Starting from a boundary advances the anchor", + // vec![(1, Range::new(0, 0), Range::new(1, 9))]), + ("Previous anchor is irrelevant for end of word motion", - vec![(1, Range::new(12, 2), Range::new(2, 7))]), + vec![(1, Range::new(12, 2), Range::new(2, 8))]), ("Identifiers_with_underscores are considered a single word", - vec![(1, Range::new(0, 0), Range::new(0, 27))]), + vec![(1, Range::new(0, 0), Range::new(0, 28))]), ("Jumping\n into starting whitespace selects up to the end of next word", - vec![(1, Range::new(0, 6), Range::new(8, 15))]), + vec![(1, Range::new(0, 7), Range::new(8, 16))]), ("alphanumeric.!,and.?=punctuation are considered 'words' for the purposes of word motion", vec![ - (1, Range::new(0, 0), Range::new(0, 11)), - (1, Range::new(0, 11), Range::new(12, 14)), - (1, Range::new(12, 14), Range::new(15, 17)) + (1, Range::new(0, 0), Range::new(0, 12)), + (1, Range::new(0, 12), Range::new(12, 15)), + (1, Range::new(12, 15), Range::new(15, 18)) ]), ("... ... punctuation and spaces behave as expected", vec![ - (1, Range::new(0, 0), Range::new(0, 2)), - (1, Range::new(0, 2), Range::new(3, 8)), + (1, Range::new(0, 0), Range::new(0, 3)), + (1, Range::new(0, 3), Range::new(3, 9)), ]), (".._.._ punctuation is not joined by underscores into a single block", - vec![(1, Range::new(0, 0), Range::new(0, 1))]), + vec![(1, Range::new(0, 0), Range::new(0, 2))]), ("Newlines\n\nare bridged seamlessly.", vec![ - (1, Range::new(0, 0), Range::new(0, 7)), - (1, Range::new(0, 7), Range::new(10, 12)), + (1, Range::new(0, 0), Range::new(0, 8)), + (1, Range::new(0, 8), Range::new(10, 13)), ]), ("Jumping\n\n\n\n\n\n from newlines to whitespace selects to end of next word.", vec![ - (1, Range::new(0, 8), Range::new(13, 19)), + (1, Range::new(0, 8), Range::new(13, 20)), ]), ("A failed motion does not modify the range", vec![ @@ -887,11 +926,11 @@ mod test { ]), ("Multiple motions at once resolve correctly", vec![ - (3, Range::new(0, 0), Range::new(16, 18)), + (3, Range::new(0, 0), Range::new(16, 19)), ]), ("Excessive motions are performed partially", vec![ - (999, Range::new(0, 0), Range::new(31, 40)), + (999, Range::new(0, 0), Range::new(31, 41)), ]), ("", // Edge case of moving forward in empty string vec![ @@ -899,16 +938,16 @@ mod test { ]), ("\n\n\n\n\n", // Edge case of moving forward in all newlines vec![ - (1, Range::new(0, 0), Range::new(0, 4)), + (1, Range::new(0, 0), Range::new(5, 5)), ]), ("\n \n \n Jumping through alternated space blocks and newlines selects the space blocks", vec![ - (1, Range::new(0, 0), Range::new(1, 3)), - (1, Range::new(1, 3), Range::new(5, 7)), + (1, Range::new(0, 0), Range::new(1, 4)), + (1, Range::new(1, 4), Range::new(5, 8)), ]), ("ヒーリクス multibyte characters behave as normal characters", vec![ - (1, Range::new(0, 0), Range::new(0, 4)), + (1, Range::new(0, 0), Range::new(0, 5)), ]), ]); @@ -924,44 +963,44 @@ mod test { fn test_behaviour_when_moving_to_end_of_previous_words() { let tests = array::IntoIter::new([ ("Basic backward motion from the middle of a word", - vec![(1, Range::new(9, 9), Range::new(9, 5))]), + vec![(1, Range::new(9, 9), Range::new(10, 5))]), ("Starting from after boundary retreats the anchor", - vec![(1, Range::new(0, 13), Range::new(12, 8))]), + vec![(1, Range::new(0, 14), Range::new(13, 8))]), ("Jump to end of a word succeeded by whitespace", - vec![(1, Range::new(10, 10), Range::new(10, 4))]), + vec![(1, Range::new(11, 11), Range::new(11, 4))]), (" Jump to start of line from end of word preceded by whitespace", - vec![(1, Range::new(7, 7), Range::new(7, 0))]), + vec![(1, Range::new(8, 8), Range::new(8, 0))]), ("Previous anchor is irrelevant for backward motions", - vec![(1, Range::new(26, 12), Range::new(12, 8))]), + vec![(1, Range::new(26, 12), Range::new(13, 8))]), (" Starting from whitespace moves to first space in sequence", - vec![(1, Range::new(0, 3), Range::new(3, 0))]), + vec![(1, Range::new(0, 4), Range::new(4, 0))]), ("Test identifiers_with_underscores are considered a single word", vec![(1, Range::new(0, 25), Range::new(25, 4))]), ("Jumping\n \nback through a newline selects whitespace", - vec![(1, Range::new(0, 13), Range::new(11, 8))]), + vec![(1, Range::new(0, 13), Range::new(12, 8))]), ("Jumping to start of word from the end selects the whole word", - vec![(1, Range::new(15, 15), Range::new(15, 10))]), + vec![(1, Range::new(16, 16), Range::new(16, 10))]), ("alphanumeric.!,and.?=punctuation are considered 'words' for the purposes of word motion", vec![ - (1, Range::new(30, 30), Range::new(30, 21)), - (1, Range::new(30, 21), Range::new(20, 18)), - (1, Range::new(20, 18), Range::new(17, 15)) + (1, Range::new(30, 30), Range::new(31, 21)), + (1, Range::new(31, 21), Range::new(21, 18)), + (1, Range::new(21, 18), Range::new(18, 15)) ]), ("... ... punctuation and spaces behave as expected", vec![ - (1, Range::new(0, 10), Range::new(9, 9)), - (1, Range::new(9, 6), Range::new(5, 3)), + (1, Range::new(0, 10), Range::new(9, 3)), + (1, Range::new(9, 3), Range::new(3, 0)), ]), (".._.._ punctuation is not joined by underscores into a single block", - vec![(1, Range::new(0, 5), Range::new(4, 3))]), + vec![(1, Range::new(0, 5), Range::new(5, 3))]), ("Newlines\n\nare bridged seamlessly.", vec![ - (1, Range::new(0, 10), Range::new(7, 0)), + (1, Range::new(0, 10), Range::new(8, 0)), ]), ("Jumping \n\n\n\n\nback from within a newline group selects previous block", vec![ - (1, Range::new(0, 13), Range::new(10, 7)), + (1, Range::new(0, 13), Range::new(11, 7)), ]), ("Failed motions do not modify the range", vec![ @@ -969,11 +1008,11 @@ mod test { ]), ("Multiple motions at once resolve correctly", vec![ - (3, Range::new(23, 23), Range::new(15, 8)), + (3, Range::new(24, 24), Range::new(16, 8)), ]), ("Excessive motions are performed partially", vec![ - (999, Range::new(40, 40), Range::new(8, 0)), + (999, Range::new(40, 40), Range::new(9, 0)), ]), ("", // Edge case of moving backwards in empty string vec![ @@ -981,16 +1020,16 @@ mod test { ]), ("\n\n\n\n\n", // Edge case of moving backwards in all newlines vec![ - (1, Range::new(0, 0), Range::new(0, 0)), + (1, Range::new(5, 5), Range::new(0, 0)), ]), (" \n \nJumping back through alternated space blocks and newlines selects the space blocks", vec![ - (1, Range::new(0, 7), Range::new(6, 4)), - (1, Range::new(6, 4), Range::new(2, 0)), + (1, Range::new(0, 8), Range::new(7, 4)), + (1, Range::new(7, 4), Range::new(3, 0)), ]), ("Test ヒーリクス multibyte characters behave as normal characters", vec![ - (1, Range::new(0, 9), Range::new(9, 4)), + (1, Range::new(0, 10), Range::new(10, 4)), ]), ]); @@ -1006,40 +1045,44 @@ mod test { fn test_behaviour_when_moving_to_end_of_next_long_words() { let tests = array::IntoIter::new([ ("Basic forward motion from the start of a word to the end of it", - vec![(1, Range::new(0, 0), Range::new(0, 4))]), + vec![(1, Range::new(0, 0), Range::new(0, 5))]), ("Basic forward motion from the end of a word to the end of the next", - vec![(1, Range::new(0, 4), Range::new(5, 12))]), + vec![(1, Range::new(0, 5), Range::new(5, 13))]), ("Basic forward motion from the middle of a word to the end of it", - vec![(1, Range::new(2, 2), Range::new(2, 4))]), + vec![(1, Range::new(2, 2), Range::new(2, 5))]), (" Jumping to end of a word preceded by whitespace", - vec![(1, Range::new(0, 0), Range::new(0, 10))]), - (" Starting from a boundary advances the anchor", - vec![(1, Range::new(0, 0), Range::new(1, 8))]), + vec![(1, Range::new(0, 0), Range::new(0, 11))]), + + // // Why do we want this behavior? The current behavior fails this + // // test, but seems better and more consistent. + // (" Starting from a boundary advances the anchor", + // vec![(1, Range::new(0, 0), Range::new(1, 9))]), + ("Previous anchor is irrelevant for end of word motion", - vec![(1, Range::new(12, 2), Range::new(2, 7))]), + vec![(1, Range::new(12, 2), Range::new(2, 8))]), ("Identifiers_with_underscores are considered a single word", - vec![(1, Range::new(0, 0), Range::new(0, 27))]), + vec![(1, Range::new(0, 0), Range::new(0, 28))]), ("Jumping\n into starting whitespace selects up to the end of next word", - vec![(1, Range::new(0, 6), Range::new(8, 15))]), + vec![(1, Range::new(0, 7), Range::new(8, 16))]), ("alphanumeric.!,and.?=punctuation are treated the same way", vec![ - (1, Range::new(0, 0), Range::new(0, 31)), + (1, Range::new(0, 0), Range::new(0, 32)), ]), ("... ... punctuation and spaces behave as expected", vec![ - (1, Range::new(0, 0), Range::new(0, 2)), - (1, Range::new(0, 2), Range::new(3, 8)), + (1, Range::new(0, 0), Range::new(0, 3)), + (1, Range::new(0, 3), Range::new(3, 9)), ]), (".._.._ punctuation is joined by underscores into a single block", - vec![(1, Range::new(0, 0), Range::new(0, 5))]), + vec![(1, Range::new(0, 0), Range::new(0, 6))]), ("Newlines\n\nare bridged seamlessly.", vec![ - (1, Range::new(0, 0), Range::new(0, 7)), - (1, Range::new(0, 7), Range::new(10, 12)), + (1, Range::new(0, 0), Range::new(0, 8)), + (1, Range::new(0, 8), Range::new(10, 13)), ]), ("Jumping\n\n\n\n\n\n from newlines to whitespace selects to end of next word.", vec![ - (1, Range::new(0, 8), Range::new(13, 19)), + (1, Range::new(0, 9), Range::new(13, 20)), ]), ("A failed motion does not modify the range", vec![ @@ -1047,11 +1090,11 @@ mod test { ]), ("Multiple motions at once resolve correctly", vec![ - (3, Range::new(0, 0), Range::new(16, 18)), + (3, Range::new(0, 0), Range::new(16, 19)), ]), ("Excessive motions are performed partially", vec![ - (999, Range::new(0, 0), Range::new(31, 40)), + (999, Range::new(0, 0), Range::new(31, 41)), ]), ("", // Edge case of moving forward in empty string vec![ @@ -1059,16 +1102,16 @@ mod test { ]), ("\n\n\n\n\n", // Edge case of moving forward in all newlines vec![ - (1, Range::new(0, 0), Range::new(0, 4)), + (1, Range::new(0, 0), Range::new(5, 5)), ]), ("\n \n \n Jumping through alternated space blocks and newlines selects the space blocks", vec![ - (1, Range::new(0, 0), Range::new(1, 3)), - (1, Range::new(1, 3), Range::new(5, 7)), + (1, Range::new(0, 0), Range::new(1, 4)), + (1, Range::new(1, 4), Range::new(5, 8)), ]), ("ヒーリ..クス multibyte characters behave as normal characters, including when they interact with punctuation", vec![ - (1, Range::new(0, 0), Range::new(0, 6)), + (1, Range::new(0, 0), Range::new(0, 7)), ]), ]); 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..611e6b76 100644 --- a/helix-core/src/position.rs +++ b/helix-core/src/position.rs @@ -1,6 +1,7 @@ use crate::{ chars::char_is_line_ending, - graphemes::{nth_next_grapheme_boundary, RopeGraphemes}, + graphemes::{ensure_grapheme_boundary_prev, RopeGraphemes}, + line_ending::line_end_char_index, RopeSlice, }; @@ -52,19 +53,50 @@ impl From<Position> for tree_sitter::Point { } } /// Convert a character index to (line, column) coordinates. +/// +/// TODO: this should be split into two methods: one for visual +/// row/column, and one for "objective" row/column (possibly with +/// the column specified in `char`s). The former would be used +/// for cursor movement, and the latter would be used for e.g. the +/// row:column display in the status line. pub fn coords_at_pos(text: RopeSlice, pos: usize) -> Position { let line = text.char_to_line(pos); + let line_start = text.line_to_char(line); + let pos = ensure_grapheme_boundary_prev(text, pos); let col = RopeGraphemes::new(text.slice(line_start..pos)).count(); + Position::new(line, col) } /// Convert (line, column) coordinates to a character index. -pub fn pos_at_coords(text: RopeSlice, coords: Position) -> usize { +/// +/// `is_1_width` specifies whether the position should be treated +/// as a block cursor or not. This effects how line-ends are handled. +/// `false` corresponds to properly round-tripping with `coords_at_pos()`, +/// whereas `true` will ensure that block cursors don't jump off the +/// end of the line. +/// +/// TODO: this should be changed to work in terms of visual row/column, not +/// graphemes. +pub fn pos_at_coords(text: RopeSlice, coords: Position, is_1_width: bool) -> usize { let Position { row, col } = coords; let line_start = text.line_to_char(row); - // line_start + col - nth_next_grapheme_boundary(text, line_start, col) + let line_end = if is_1_width { + line_end_char_index(&text, row) + } else { + text.line_to_char((row + 1).min(text.len_lines())) + }; + + let mut col_char_offset = 0; + for (i, g) in RopeGraphemes::new(text.slice(line_start..line_end)).enumerate() { + if i == col { + break; + } + col_char_offset += g.chars().count(); + } + + line_start + col_char_offset } #[cfg(test)] @@ -80,55 +112,109 @@ mod test { #[test] fn test_coords_at_pos() { - // let text = Rope::from("ḧëḷḷö\nẅöṛḷḋ"); - // let slice = text.slice(..); - // assert_eq!(coords_at_pos(slice, 0), (0, 0).into()); - // assert_eq!(coords_at_pos(slice, 5), (0, 5).into()); // position on \n - // assert_eq!(coords_at_pos(slice, 6), (1, 0).into()); // position on w - // assert_eq!(coords_at_pos(slice, 7), (1, 1).into()); // position on o - // assert_eq!(coords_at_pos(slice, 10), (1, 4).into()); // position on d - - // test with grapheme clusters + let text = Rope::from("ḧëḷḷö\nẅöṛḷḋ"); + let slice = text.slice(..); + assert_eq!(coords_at_pos(slice, 0), (0, 0).into()); + assert_eq!(coords_at_pos(slice, 5), (0, 5).into()); // position on \n + assert_eq!(coords_at_pos(slice, 6), (1, 0).into()); // position on w + assert_eq!(coords_at_pos(slice, 7), (1, 1).into()); // position on o + assert_eq!(coords_at_pos(slice, 10), (1, 4).into()); // position on d + + // Test with wide characters. + // TODO: account for character width. + let text = Rope::from("今日はいい\n"); + let slice = text.slice(..); + assert_eq!(coords_at_pos(slice, 0), (0, 0).into()); + assert_eq!(coords_at_pos(slice, 1), (0, 1).into()); + assert_eq!(coords_at_pos(slice, 2), (0, 2).into()); + assert_eq!(coords_at_pos(slice, 3), (0, 3).into()); + assert_eq!(coords_at_pos(slice, 4), (0, 4).into()); + assert_eq!(coords_at_pos(slice, 5), (0, 5).into()); + assert_eq!(coords_at_pos(slice, 6), (1, 0).into()); + + // Test with grapheme clusters. let text = Rope::from("a̐éö̲\r\n"); let slice = text.slice(..); assert_eq!(coords_at_pos(slice, 0), (0, 0).into()); assert_eq!(coords_at_pos(slice, 2), (0, 1).into()); assert_eq!(coords_at_pos(slice, 4), (0, 2).into()); assert_eq!(coords_at_pos(slice, 7), (0, 3).into()); + assert_eq!(coords_at_pos(slice, 9), (1, 0).into()); - let text = Rope::from("किमपि"); + // Test with wide-character grapheme clusters. + // TODO: account for character width. + let text = Rope::from("किमपि\n"); let slice = text.slice(..); assert_eq!(coords_at_pos(slice, 0), (0, 0).into()); assert_eq!(coords_at_pos(slice, 2), (0, 1).into()); assert_eq!(coords_at_pos(slice, 3), (0, 2).into()); assert_eq!(coords_at_pos(slice, 5), (0, 3).into()); + assert_eq!(coords_at_pos(slice, 6), (1, 0).into()); + + // Test with tabs. + // Todo: account for tab stops. + let text = Rope::from("\tHello\n"); + let slice = text.slice(..); + assert_eq!(coords_at_pos(slice, 0), (0, 0).into()); + assert_eq!(coords_at_pos(slice, 1), (0, 1).into()); + assert_eq!(coords_at_pos(slice, 2), (0, 2).into()); } #[test] fn test_pos_at_coords() { let text = Rope::from("ḧëḷḷö\nẅöṛḷḋ"); let slice = text.slice(..); - assert_eq!(pos_at_coords(slice, (0, 0).into()), 0); - assert_eq!(pos_at_coords(slice, (0, 5).into()), 5); // position on \n - assert_eq!(pos_at_coords(slice, (1, 0).into()), 6); // position on w - assert_eq!(pos_at_coords(slice, (1, 1).into()), 7); // position on o - assert_eq!(pos_at_coords(slice, (1, 4).into()), 10); // position on d + assert_eq!(pos_at_coords(slice, (0, 0).into(), false), 0); + assert_eq!(pos_at_coords(slice, (0, 5).into(), false), 5); // position on \n + assert_eq!(pos_at_coords(slice, (0, 6).into(), false), 6); // position after \n + assert_eq!(pos_at_coords(slice, (0, 6).into(), true), 5); // position after \n + assert_eq!(pos_at_coords(slice, (1, 0).into(), false), 6); // position on w + assert_eq!(pos_at_coords(slice, (1, 1).into(), false), 7); // position on o + assert_eq!(pos_at_coords(slice, (1, 4).into(), false), 10); // position on d - // test with grapheme clusters + // Test with wide characters. + // TODO: account for character width. + let text = Rope::from("今日はいい\n"); + let slice = text.slice(..); + assert_eq!(pos_at_coords(slice, (0, 0).into(), false), 0); + assert_eq!(pos_at_coords(slice, (0, 1).into(), false), 1); + assert_eq!(pos_at_coords(slice, (0, 2).into(), false), 2); + assert_eq!(pos_at_coords(slice, (0, 3).into(), false), 3); + assert_eq!(pos_at_coords(slice, (0, 4).into(), false), 4); + assert_eq!(pos_at_coords(slice, (0, 5).into(), false), 5); + assert_eq!(pos_at_coords(slice, (0, 6).into(), false), 6); + assert_eq!(pos_at_coords(slice, (0, 6).into(), true), 5); + assert_eq!(pos_at_coords(slice, (1, 0).into(), false), 6); + + // Test with grapheme clusters. let text = Rope::from("a̐éö̲\r\n"); let slice = text.slice(..); - assert_eq!(pos_at_coords(slice, (0, 0).into()), 0); - assert_eq!(pos_at_coords(slice, (0, 1).into()), 2); - assert_eq!(pos_at_coords(slice, (0, 2).into()), 4); - assert_eq!(pos_at_coords(slice, (0, 3).into()), 7); // \r\n is one char here - assert_eq!(pos_at_coords(slice, (0, 4).into()), 9); + assert_eq!(pos_at_coords(slice, (0, 0).into(), false), 0); + assert_eq!(pos_at_coords(slice, (0, 1).into(), false), 2); + assert_eq!(pos_at_coords(slice, (0, 2).into(), false), 4); + assert_eq!(pos_at_coords(slice, (0, 3).into(), false), 7); // \r\n is one char here + assert_eq!(pos_at_coords(slice, (0, 4).into(), false), 9); + assert_eq!(pos_at_coords(slice, (0, 4).into(), true), 7); + assert_eq!(pos_at_coords(slice, (1, 0).into(), false), 9); + + // Test with wide-character grapheme clusters. + // TODO: account for character width. let text = Rope::from("किमपि"); // 2 - 1 - 2 codepoints // TODO: delete handling as per https://news.ycombinator.com/item?id=20058454 let slice = text.slice(..); - assert_eq!(pos_at_coords(slice, (0, 0).into()), 0); - assert_eq!(pos_at_coords(slice, (0, 1).into()), 2); - assert_eq!(pos_at_coords(slice, (0, 2).into()), 3); - assert_eq!(pos_at_coords(slice, (0, 3).into()), 5); // eol + assert_eq!(pos_at_coords(slice, (0, 0).into(), false), 0); + assert_eq!(pos_at_coords(slice, (0, 1).into(), false), 2); + assert_eq!(pos_at_coords(slice, (0, 2).into(), false), 3); + assert_eq!(pos_at_coords(slice, (0, 3).into(), false), 5); + assert_eq!(pos_at_coords(slice, (0, 3).into(), true), 5); + + // Test with tabs. + // Todo: account for tab stops. + let text = Rope::from("\tHello\n"); + let slice = text.slice(..); + assert_eq!(pos_at_coords(slice, (0, 0).into(), false), 0); + assert_eq!(pos_at_coords(slice, (0, 1).into(), false), 1); + assert_eq!(pos_at_coords(slice, (0, 2).into(), false), 2); } } diff --git a/helix-core/src/search.rs b/helix-core/src/search.rs index 73be68c7..243ac227 100644 --- a/helix-core/src/search.rs +++ b/helix-core/src/search.rs @@ -1,18 +1,11 @@ use crate::RopeSlice; -pub fn find_nth_next( - text: RopeSlice, - ch: char, - mut pos: usize, - n: usize, - inclusive: bool, -) -> Option<usize> { - if pos >= text.len_chars() { +pub fn find_nth_next(text: RopeSlice, ch: char, mut pos: usize, n: usize) -> Option<usize> { + if pos >= text.len_chars() || n == 0 { return None; } - // start searching right after pos - let mut chars = text.chars_at(pos + 1); + let mut chars = text.chars_at(pos); for _ in 0..n { loop { @@ -26,28 +19,21 @@ pub fn find_nth_next( } } - if !inclusive { - pos -= 1; - } - - Some(pos) + Some(pos - 1) } -pub fn find_nth_prev( - text: RopeSlice, - ch: char, - mut pos: usize, - n: usize, - inclusive: bool, -) -> Option<usize> { - // start searching right before pos +pub fn find_nth_prev(text: RopeSlice, ch: char, mut pos: usize, n: usize) -> Option<usize> { + if pos == 0 || n == 0 { + return None; + } + let mut chars = text.chars_at(pos); for _ in 0..n { loop { let c = chars.prev()?; - pos = pos.saturating_sub(1); + pos -= 1; if c == ch { break; @@ -55,9 +41,5 @@ pub fn find_nth_prev( } } - if !inclusive { - pos += 1; - } - Some(pos) } diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index 63b9b557..14c54295 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -1,30 +1,59 @@ -//! 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 cursors are +//! defined as a 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, + prev_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. +/// +/// A range consists of an "anchor" and "head" position in +/// the text. The head is the part that the user moves when +/// directly extending a selection. The head and anchor +/// can be in any order, or even share the same position. +/// +/// The anchor and head positions use gap indexing, meaning +/// that their indices represent the the gaps *between* `char`s +/// rather than the `char`s themselves. For example, 1 +/// represents the position between the first and second `char`. +/// +/// 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 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. +/// +/// By convention, user-facing ranges are considered to have +/// a block cursor on the head-side of the range that spans a +/// single grapheme inward from the range's edge. There are a +/// variety of helper methods on `Range` for working in terms of +/// that block cursor, all of which have `cursor` in their name. #[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 { @@ -53,6 +82,20 @@ impl Range { std::cmp::max(self.anchor, self.head) } + /// The (inclusive) range of lines that the range overlaps. + #[inline] + #[must_use] + pub fn line_range(&self, text: RopeSlice) -> (usize, usize) { + let from = self.from(); + let to = if self.is_empty() { + self.to() + } else { + prev_grapheme_boundary(text, self.to()).max(from) + }; + + (text.char_to_line(from), text.char_to_line(to)) + } + /// `true` when head and anchor are at the same position. #[inline] pub fn is_empty(&self) -> bool { @@ -62,37 +105,39 @@ 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 /// after the changes are applied. pub fn map(self, changes: &ChangeSet) -> Self { - 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; - } + use std::cmp::Ordering; + let (anchor, head) = match self.anchor.cmp(&self.head) { + Ordering::Equal => ( + changes.map_pos(self.anchor, Assoc::After), + changes.map_pos(self.head, Assoc::After), + ), + Ordering::Less => ( + changes.map_pos(self.anchor, Assoc::After), + changes.map_pos(self.head, Assoc::Before), + ), + Ordering::Greater => ( + changes.map_pos(self.anchor, Assoc::Before), + changes.map_pos(self.head, Assoc::After), + ), + }; + + // 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 +148,41 @@ 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, + } } + } - Self { - anchor: self.anchor, - head: if abs_difference(from, self.anchor) > abs_difference(to, self.anchor) { - from - } else { - to - }, - 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, + } } } @@ -126,7 +190,120 @@ 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() + } + + //-------------------------------- + // Alignment methods. + + /// 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 { + None + }, + } + } + + /// 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 + } + } + + //-------------------------------- + // Block-cursor methods. + + /// Gets the left-side position of the block cursor. + #[must_use] + #[inline] + pub fn cursor(self, text: RopeSlice) -> usize { + if self.head > self.anchor { + prev_grapheme_boundary(text, self.head) + } else { + self.head + } + } + + /// Puts the left side of the block cursor at `char_idx`, optionally extending. + /// + /// This follows "1-width" semantics, and therefore does a combination of anchor + /// and head moves to behave as if both the front and back of the range are 1-width + /// blocks + /// + /// This method assumes that the range and `char_idx` are already properly + /// grapheme-aligned. + #[must_use] + #[inline] + pub fn put_cursor(self, text: RopeSlice, char_idx: usize, extend: bool) -> Range { + if extend { + let anchor = if self.head >= self.anchor && char_idx < self.anchor { + next_grapheme_boundary(text, self.anchor) + } else if self.head < self.anchor && char_idx >= self.anchor { + prev_grapheme_boundary(text, self.anchor) + } else { + self.anchor + }; + + if anchor <= char_idx { + Range::new(anchor, next_grapheme_boundary(text, char_idx)) + } else { + Range::new(anchor, char_idx) + } + } else { + Range::point(char_idx) + } + } + + /// The line number that the block-cursor is on. + #[inline] + #[must_use] + pub fn cursor_line(&self, text: RopeSlice) -> usize { + text.char_to_line(self.cursor(text)) } } @@ -157,11 +334,6 @@ impl Selection { self.ranges[self.primary_index] } - #[must_use] - pub fn cursor(&self) -> usize { - self.primary().head - } - /// Ensure selection containing only the primary selection. pub fn into_single(self) -> Self { if self.ranges.len() == 1 { @@ -174,13 +346,12 @@ impl Selection { } } + /// Adds a new range to the selection and makes it the primary range. pub fn push(mut self, range: Range) -> Self { - let index = self.ranges.len(); self.ranges.push(range); - - Self::normalize(self.ranges, index) + self.set_primary_index(self.ranges().len() - 1); + self.normalize() } - // replace_range /// Map selections over a set of changes. Useful for adjusting the selection position after /// applying changes to a document. @@ -206,6 +377,11 @@ impl Selection { self.primary_index } + pub fn set_primary_index(&mut self, idx: usize) { + assert!(idx < self.ranges.len()); + self.primary_index = idx; + } + #[must_use] /// Constructs a selection holding a single range. pub fn single(anchor: usize, head: usize) -> Self { @@ -224,80 +400,79 @@ 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]) { + self.ranges[prev_i] = self.ranges[prev_i].merge(self.ranges[i]); + } else { + prev_i += 1; + self.ranges[prev_i] = self.ranges[i]; + } + if i == self.primary_index { + self.primary_index = prev_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() + } + + // Ensures the selection adheres to the following invariants: + // 1. All ranges are grapheme aligned. + // 2. All ranges are at least 1 character wide, unless at the + // very end of the document. + // 3. Ranges are non-overlapping. + // 4. Ranges are sorted by their position in the text. + pub fn ensure_invariants(self, text: RopeSlice) -> Self { + self.transform(|r| r.min_width_1(text).grapheme_aligned(text)) + .normalize() + } + + /// Transforms the selection into all of the left-side head positions, + /// using block-cursor semantics. + pub fn cursors(self, text: RopeSlice) -> Self { + self.transform(|range| Range::point(range.cursor(text))) } pub fn fragments<'a>(&'a self, text: RopeSlice<'a>) -> impl Iterator<Item = Cow<str>> + 'a { @@ -363,7 +538,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 +559,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 +577,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 +664,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 +678,251 @@ 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_line_range() { + let r = Rope::from_str("\r\nHi\r\nthere!"); + let s = r.slice(..); + + // Zero-width ranges. + assert_eq!(Range::new(0, 0).line_range(s), (0, 0)); + assert_eq!(Range::new(1, 1).line_range(s), (0, 0)); + assert_eq!(Range::new(2, 2).line_range(s), (1, 1)); + assert_eq!(Range::new(3, 3).line_range(s), (1, 1)); + + // Forward ranges. + assert_eq!(Range::new(0, 1).line_range(s), (0, 0)); + assert_eq!(Range::new(0, 2).line_range(s), (0, 0)); + assert_eq!(Range::new(0, 3).line_range(s), (0, 1)); + assert_eq!(Range::new(1, 2).line_range(s), (0, 0)); + assert_eq!(Range::new(2, 3).line_range(s), (1, 1)); + assert_eq!(Range::new(3, 8).line_range(s), (1, 2)); + assert_eq!(Range::new(0, 12).line_range(s), (0, 2)); + + // Reverse ranges. + assert_eq!(Range::new(1, 0).line_range(s), (0, 0)); + assert_eq!(Range::new(2, 0).line_range(s), (0, 0)); + assert_eq!(Range::new(3, 0).line_range(s), (0, 1)); + assert_eq!(Range::new(2, 1).line_range(s), (0, 0)); + assert_eq!(Range::new(3, 2).line_range(s), (1, 1)); + assert_eq!(Range::new(8, 3).line_range(s), (1, 2)); + assert_eq!(Range::new(12, 0).line_range(s), (0, 2)); + } + + #[test] + fn test_cursor() { + let r = Rope::from_str("\r\nHi\r\nthere!"); + let s = r.slice(..); + + // Zero-width ranges. + assert_eq!(Range::new(0, 0).cursor(s), 0); + assert_eq!(Range::new(2, 2).cursor(s), 2); + assert_eq!(Range::new(3, 3).cursor(s), 3); + + // Forward ranges. + assert_eq!(Range::new(0, 2).cursor(s), 0); + assert_eq!(Range::new(0, 3).cursor(s), 2); + assert_eq!(Range::new(3, 6).cursor(s), 4); + + // Reverse ranges. + assert_eq!(Range::new(2, 0).cursor(s), 0); + assert_eq!(Range::new(6, 2).cursor(s), 2); + assert_eq!(Range::new(6, 3).cursor(s), 3); + } + + #[test] + fn test_put_cursor() { + let r = Rope::from_str("\r\nHi\r\nthere!"); + let s = r.slice(..); + + // Zero-width ranges. + assert_eq!(Range::new(0, 0).put_cursor(s, 0, true), Range::new(0, 2)); + assert_eq!(Range::new(0, 0).put_cursor(s, 2, true), Range::new(0, 3)); + assert_eq!(Range::new(2, 3).put_cursor(s, 4, true), Range::new(2, 6)); + assert_eq!(Range::new(2, 8).put_cursor(s, 4, true), Range::new(2, 6)); + assert_eq!(Range::new(8, 8).put_cursor(s, 4, true), Range::new(9, 4)); + + // Forward ranges. + assert_eq!(Range::new(3, 6).put_cursor(s, 0, true), Range::new(4, 0)); + assert_eq!(Range::new(3, 6).put_cursor(s, 2, true), Range::new(4, 2)); + assert_eq!(Range::new(3, 6).put_cursor(s, 3, true), Range::new(3, 4)); + assert_eq!(Range::new(3, 6).put_cursor(s, 4, true), Range::new(3, 6)); + assert_eq!(Range::new(3, 6).put_cursor(s, 6, true), Range::new(3, 7)); + assert_eq!(Range::new(3, 6).put_cursor(s, 8, true), Range::new(3, 9)); + + // Reverse ranges. + assert_eq!(Range::new(6, 3).put_cursor(s, 0, true), Range::new(6, 0)); + assert_eq!(Range::new(6, 3).put_cursor(s, 2, true), Range::new(6, 2)); + assert_eq!(Range::new(6, 3).put_cursor(s, 3, true), Range::new(6, 3)); + assert_eq!(Range::new(6, 3).put_cursor(s, 4, true), Range::new(6, 4)); + assert_eq!(Range::new(6, 3).put_cursor(s, 6, true), Range::new(4, 7)); + assert_eq!(Range::new(6, 3).put_cursor(s, 8, true), Range::new(4, 9)); } #[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/surround.rs b/helix-core/src/surround.rs index 52f60cab..4d3ed5f5 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -1,3 +1,4 @@ +use crate::graphemes::next_grapheme_boundary; use crate::{search, Selection}; use ropey::RopeSlice; @@ -40,23 +41,34 @@ pub fn find_nth_pairs_pos( ) -> Option<(usize, usize)> { let (open, close) = get_pair(ch); - let (open_pos, close_pos) = if open == close { - let prev = search::find_nth_prev(text, open, pos, n, true); - let next = search::find_nth_next(text, close, pos, n, true); - if text.char(pos) == open { - // cursor is *on* a pair - next.map(|n| (pos, n)).or_else(|| prev.map(|p| (p, pos)))? + if text.len_chars() < 2 || pos >= text.len_chars() { + return None; + } + + if open == close { + if Some(open) == text.get_char(pos) { + // Special case: cursor is directly on a matching char. + match pos { + 0 => Some((pos, search::find_nth_next(text, close, pos + 1, n)? + 1)), + _ if (pos + 1) == text.len_chars() => { + Some((search::find_nth_prev(text, open, pos, n)?, text.len_chars())) + } + // We return no match because there's no way to know which + // side of the char we should be searching on. + _ => None, + } } else { - (prev?, next?) + Some(( + search::find_nth_prev(text, open, pos, n)?, + search::find_nth_next(text, close, pos, n)? + 1, + )) } } else { - ( + Some(( find_nth_open_pair(text, open, close, pos, n)?, - find_nth_close_pair(text, open, close, pos, n)?, - ) - }; - - Some((open_pos, close_pos)) + next_grapheme_boundary(text, find_nth_close_pair(text, open, close, pos, n)?), + )) + } } fn find_nth_open_pair( @@ -173,12 +185,13 @@ mod test { let slice = doc.slice(..); // cursor on [t]ext - assert_eq!(find_nth_pairs_pos(slice, '(', 6, 1), Some((5, 10))); - assert_eq!(find_nth_pairs_pos(slice, ')', 6, 1), Some((5, 10))); + assert_eq!(find_nth_pairs_pos(slice, '(', 6, 1), Some((5, 11))); + assert_eq!(find_nth_pairs_pos(slice, ')', 6, 1), Some((5, 11))); // cursor on so[m]e assert_eq!(find_nth_pairs_pos(slice, '(', 2, 1), None); // cursor on bracket itself - assert_eq!(find_nth_pairs_pos(slice, '(', 5, 1), Some((5, 10))); + assert_eq!(find_nth_pairs_pos(slice, '(', 5, 1), Some((5, 11))); + assert_eq!(find_nth_pairs_pos(slice, '(', 10, 1), Some((5, 11))); } #[test] @@ -187,9 +200,9 @@ mod test { let slice = doc.slice(..); // cursor on go[o]d - assert_eq!(find_nth_pairs_pos(slice, '(', 13, 1), Some((10, 15))); - assert_eq!(find_nth_pairs_pos(slice, '(', 13, 2), Some((4, 21))); - assert_eq!(find_nth_pairs_pos(slice, '(', 13, 3), Some((0, 27))); + assert_eq!(find_nth_pairs_pos(slice, '(', 13, 1), Some((10, 16))); + assert_eq!(find_nth_pairs_pos(slice, '(', 13, 2), Some((4, 22))); + assert_eq!(find_nth_pairs_pos(slice, '(', 13, 3), Some((0, 28))); } #[test] @@ -198,14 +211,14 @@ mod test { let slice = doc.slice(..); // cursor on go[o]d - assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 1), Some((10, 15))); - assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 2), Some((4, 21))); - assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 3), Some((0, 27))); + assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 1), Some((10, 16))); + assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 2), Some((4, 22))); + assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 3), Some((0, 28))); // cursor on the quotes - assert_eq!(find_nth_pairs_pos(slice, '\'', 10, 1), Some((10, 15))); + assert_eq!(find_nth_pairs_pos(slice, '\'', 10, 1), None); // this is the best we can do since opening and closing pairs are same - assert_eq!(find_nth_pairs_pos(slice, '\'', 0, 1), Some((0, 4))); - assert_eq!(find_nth_pairs_pos(slice, '\'', 27, 1), Some((21, 27))); + assert_eq!(find_nth_pairs_pos(slice, '\'', 0, 1), Some((0, 5))); + assert_eq!(find_nth_pairs_pos(slice, '\'', 27, 1), Some((21, 28))); } #[test] @@ -214,8 +227,8 @@ mod test { let slice = doc.slice(..); // cursor on go[o]d - assert_eq!(find_nth_pairs_pos(slice, '(', 15, 1), Some((5, 24))); - assert_eq!(find_nth_pairs_pos(slice, '(', 15, 2), Some((0, 31))); + assert_eq!(find_nth_pairs_pos(slice, '(', 15, 1), Some((5, 25))); + assert_eq!(find_nth_pairs_pos(slice, '(', 15, 2), Some((0, 32))); } #[test] @@ -224,9 +237,9 @@ mod test { let slice = doc.slice(..); // cursor on go[o]d - assert_eq!(find_nth_pairs_pos(slice, '{', 13, 1), Some((10, 15))); - assert_eq!(find_nth_pairs_pos(slice, '[', 13, 1), Some((4, 21))); - assert_eq!(find_nth_pairs_pos(slice, '(', 13, 1), Some((0, 27))); + assert_eq!(find_nth_pairs_pos(slice, '{', 13, 1), Some((10, 16))); + assert_eq!(find_nth_pairs_pos(slice, '[', 13, 1), Some((4, 22))); + assert_eq!(find_nth_pairs_pos(slice, '(', 13, 1), Some((0, 28))); } #[test] @@ -243,7 +256,7 @@ mod test { get_surround_pos(slice, &selection, '(', 1) .unwrap() .as_slice(), - &[0, 5, 7, 13, 15, 23] + &[0, 6, 7, 14, 15, 24] ); } diff --git a/helix-core/src/syntax.rs b/helix-core/src/syntax.rs index 14f36a0a..c8cb0557 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -1760,10 +1760,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), diff --git a/helix-core/src/textobject.rs b/helix-core/src/textobject.rs index fbf66256..b06bca5d 100644 --- a/helix-core/src/textobject.rs +++ b/helix-core/src/textobject.rs @@ -1,21 +1,16 @@ use ropey::RopeSlice; -use crate::chars::{categorize_char, char_is_line_ending, char_is_whitespace, CharCategory}; -use crate::movement::{self, Direction}; +use crate::chars::{categorize_char, CharCategory}; +use crate::graphemes::{next_grapheme_boundary, prev_grapheme_boundary}; +use crate::movement::Direction; use crate::surround; use crate::Range; -fn this_word_end_pos(slice: RopeSlice, pos: usize) -> usize { - this_word_bound_pos(slice, pos, Direction::Forward) -} - -fn this_word_start_pos(slice: RopeSlice, pos: usize) -> usize { - this_word_bound_pos(slice, pos, Direction::Backward) -} +fn find_word_boundary(slice: RopeSlice, mut pos: usize, direction: Direction) -> usize { + use CharCategory::{Eol, Whitespace}; -fn this_word_bound_pos(slice: RopeSlice, mut pos: usize, direction: Direction) -> usize { let iter = match direction { - Direction::Forward => slice.chars_at(pos + 1), + Direction::Forward => slice.chars_at(pos), Direction::Backward => { let mut iter = slice.chars_at(pos); iter.reverse(); @@ -23,25 +18,31 @@ fn this_word_bound_pos(slice: RopeSlice, mut pos: usize, direction: Direction) - } }; - match categorize_char(slice.char(pos)) { - CharCategory::Eol | CharCategory::Whitespace => pos, - category => { - for peek in iter { - let curr_category = categorize_char(peek); - if curr_category != category - || curr_category == CharCategory::Eol - || curr_category == CharCategory::Whitespace - { + let mut prev_category = match direction { + Direction::Forward if pos == 0 => Whitespace, + Direction::Forward => categorize_char(slice.char(pos - 1)), + Direction::Backward if pos == slice.len_chars() => Whitespace, + Direction::Backward => categorize_char(slice.char(pos)), + }; + + for ch in iter { + match categorize_char(ch) { + Eol | Whitespace => return pos, + category => { + if category != prev_category && pos != 0 && pos != slice.len_chars() { return pos; - } - pos = match direction { - Direction::Forward => pos + 1, - Direction::Backward => pos.saturating_sub(1), + } else { + match direction { + Direction::Forward => pos += 1, + Direction::Backward => pos = pos.saturating_sub(1), + } + prev_category = category; } } - pos } } + + pos } #[derive(Copy, Clone, PartialEq, Eq, Debug)] @@ -55,46 +56,37 @@ pub fn textobject_word( slice: RopeSlice, range: Range, textobject: TextObject, - count: usize, + _count: usize, ) -> Range { - let this_word_start = this_word_start_pos(slice, range.head); - let this_word_end = this_word_end_pos(slice, range.head); + let pos = range.cursor(slice); + + let word_start = find_word_boundary(slice, pos, Direction::Backward); + let word_end = match slice.get_char(pos).map(categorize_char) { + None | Some(CharCategory::Whitespace | CharCategory::Eol) => pos, + _ => find_word_boundary(slice, pos + 1, Direction::Forward), + }; + + // Special case. + if word_start == word_end { + return Range::new(word_start, word_end); + } - let (anchor, head); match textobject { - TextObject::Inside => { - anchor = this_word_start; - head = this_word_end; - } - TextObject::Around => { - if slice - .get_char(this_word_end + 1) - .map_or(true, char_is_line_ending) + TextObject::Inside => Range::new(word_start, word_end), + TextObject::Around => Range::new( + match slice + .get_char(word_start.saturating_sub(1)) + .map(categorize_char) { - head = this_word_end; - if slice - .get_char(this_word_start.saturating_sub(1)) - .map_or(true, char_is_line_ending) - { - // single word on a line - anchor = this_word_start; - } else { - // last word on a line, select the whitespace before it too - anchor = movement::move_prev_word_end(slice, range, count).head; - } - } else if char_is_whitespace(slice.char(range.head)) { - // select whole whitespace and next word - head = movement::move_next_word_end(slice, range, count).head; - anchor = movement::backwards_skip_while(slice, range.head, |c| c.is_whitespace()) - .map(|p| p + 1) // p is first *non* whitespace char, so +1 to get whitespace pos - .unwrap_or(0); - } else { - head = movement::move_next_word_start(slice, range, count).head; - anchor = this_word_start; - } - } - }; - Range::new(anchor, head) + None | Some(CharCategory::Eol) => word_start, + _ => prev_grapheme_boundary(slice, word_start), + }, + match slice.get_char(word_end).map(categorize_char) { + None | Some(CharCategory::Eol) => word_end, + _ => next_grapheme_boundary(slice, word_end), + }, + ), + } } pub fn textobject_surround( @@ -106,7 +98,10 @@ pub fn textobject_surround( ) -> Range { surround::find_nth_pairs_pos(slice, ch, range.head, count) .map(|(anchor, head)| match textobject { - TextObject::Inside => Range::new(anchor + 1, head.saturating_sub(1)), + TextObject::Inside => Range::new( + next_grapheme_boundary(slice, anchor), + prev_grapheme_boundary(slice, head), + ), TextObject::Around => Range::new(anchor, head), }) .unwrap_or(range) @@ -126,70 +121,70 @@ mod test { let tests = &[ ( "cursor at beginning of doc", - vec![(0, Inside, (0, 5)), (0, Around, (0, 6))], + vec![(0, Inside, (0, 6)), (0, Around, (0, 7))], ), ( "cursor at middle of word", vec![ - (13, Inside, (10, 15)), - (10, Inside, (10, 15)), - (15, Inside, (10, 15)), - (13, Around, (10, 16)), - (10, Around, (10, 16)), - (15, Around, (10, 16)), + (13, Inside, (10, 16)), + (10, Inside, (10, 16)), + (15, Inside, (10, 16)), + (13, Around, (9, 17)), + (10, Around, (9, 17)), + (15, Around, (9, 17)), ], ), ( "cursor between word whitespace", - vec![(6, Inside, (6, 6)), (6, Around, (6, 13))], + vec![(6, Inside, (6, 6)), (6, Around, (6, 6))], ), ( "cursor on word before newline\n", vec![ - (22, Inside, (22, 28)), - (28, Inside, (22, 28)), - (25, Inside, (22, 28)), - (22, Around, (21, 28)), - (28, Around, (21, 28)), - (25, Around, (21, 28)), + (22, Inside, (22, 29)), + (28, Inside, (22, 29)), + (25, Inside, (22, 29)), + (22, Around, (21, 29)), + (28, Around, (21, 29)), + (25, Around, (21, 29)), ], ), ( "cursor on newline\nnext line", - vec![(17, Inside, (17, 17)), (17, Around, (17, 22))], + vec![(17, Inside, (17, 17)), (17, Around, (17, 17))], ), ( "cursor on word after newline\nnext line", vec![ - (29, Inside, (29, 32)), - (30, Inside, (29, 32)), - (32, Inside, (29, 32)), - (29, Around, (29, 33)), - (30, Around, (29, 33)), - (32, Around, (29, 33)), + (29, Inside, (29, 33)), + (30, Inside, (29, 33)), + (32, Inside, (29, 33)), + (29, Around, (29, 34)), + (30, Around, (29, 34)), + (32, Around, (29, 34)), ], ), ( "cursor on #$%:;* punctuation", vec![ - (13, Inside, (10, 15)), - (10, Inside, (10, 15)), - (15, Inside, (10, 15)), - (13, Around, (10, 16)), - (10, Around, (10, 16)), - (15, Around, (10, 16)), + (13, Inside, (10, 16)), + (10, Inside, (10, 16)), + (15, Inside, (10, 16)), + (13, Around, (9, 17)), + (10, Around, (9, 17)), + (15, Around, (9, 17)), ], ), ( "cursor on punc%^#$:;.tuation", vec![ - (14, Inside, (14, 20)), - (20, Inside, (14, 20)), - (17, Inside, (14, 20)), - (14, Around, (14, 20)), + (14, Inside, (14, 21)), + (20, Inside, (14, 21)), + (17, Inside, (14, 21)), + (14, Around, (13, 22)), // FIXME: edge case // (20, Around, (14, 20)), - (17, Around, (14, 20)), + (17, Around, (13, 22)), ], ), ( @@ -198,14 +193,14 @@ mod test { (9, Inside, (9, 9)), (10, Inside, (10, 10)), (11, Inside, (11, 11)), - (9, Around, (9, 16)), - (10, Around, (9, 16)), - (11, Around, (9, 16)), + (9, Around, (9, 9)), + (10, Around, (10, 10)), + (11, Around, (11, 11)), ], ), ( "cursor at end of doc", - vec![(19, Inside, (17, 19)), (19, Around, (16, 19))], + vec![(19, Inside, (17, 20)), (19, Around, (16, 20))], ), ]; @@ -234,67 +229,67 @@ mod test { "simple (single) surround pairs", vec![ (3, Inside, (3, 3), '(', 1), - (7, Inside, (8, 13), ')', 1), - (10, Inside, (8, 13), '(', 1), - (14, Inside, (8, 13), ')', 1), + (7, Inside, (8, 14), ')', 1), + (10, Inside, (8, 14), '(', 1), + (14, Inside, (8, 14), ')', 1), (3, Around, (3, 3), '(', 1), - (7, Around, (7, 14), ')', 1), - (10, Around, (7, 14), '(', 1), - (14, Around, (7, 14), ')', 1), + (7, Around, (7, 15), ')', 1), + (10, Around, (7, 15), '(', 1), + (14, Around, (7, 15), ')', 1), ], ), ( "samexx 'single' surround pairs", vec![ (3, Inside, (3, 3), '\'', 1), - (7, Inside, (8, 13), '\'', 1), - (10, Inside, (8, 13), '\'', 1), - (14, Inside, (8, 13), '\'', 1), + (7, Inside, (7, 7), '\'', 1), + (10, Inside, (8, 14), '\'', 1), + (14, Inside, (14, 14), '\'', 1), (3, Around, (3, 3), '\'', 1), - (7, Around, (7, 14), '\'', 1), - (10, Around, (7, 14), '\'', 1), - (14, Around, (7, 14), '\'', 1), + (7, Around, (7, 7), '\'', 1), + (10, Around, (7, 15), '\'', 1), + (14, Around, (14, 14), '\'', 1), ], ), ( "(nested (surround (pairs)) 3 levels)", vec![ - (0, Inside, (1, 34), '(', 1), - (6, Inside, (1, 34), ')', 1), - (8, Inside, (9, 24), '(', 1), - (8, Inside, (9, 34), ')', 2), - (20, Inside, (9, 24), '(', 2), - (20, Inside, (1, 34), ')', 3), - (0, Around, (0, 35), '(', 1), - (6, Around, (0, 35), ')', 1), - (8, Around, (8, 25), '(', 1), - (8, Around, (8, 35), ')', 2), - (20, Around, (8, 25), '(', 2), - (20, Around, (0, 35), ')', 3), + (0, Inside, (1, 35), '(', 1), + (6, Inside, (1, 35), ')', 1), + (8, Inside, (9, 25), '(', 1), + (8, Inside, (9, 35), ')', 2), + (20, Inside, (9, 25), '(', 2), + (20, Inside, (1, 35), ')', 3), + (0, Around, (0, 36), '(', 1), + (6, Around, (0, 36), ')', 1), + (8, Around, (8, 26), '(', 1), + (8, Around, (8, 36), ')', 2), + (20, Around, (8, 26), '(', 2), + (20, Around, (0, 36), ')', 3), ], ), ( "(mixed {surround [pair] same} line)", vec![ - (2, Inside, (1, 33), '(', 1), - (9, Inside, (8, 27), '{', 1), - (18, Inside, (18, 21), '[', 1), - (2, Around, (0, 34), '(', 1), - (9, Around, (7, 28), '{', 1), - (18, Around, (17, 22), '[', 1), + (2, Inside, (1, 34), '(', 1), + (9, Inside, (8, 28), '{', 1), + (18, Inside, (18, 22), '[', 1), + (2, Around, (0, 35), '(', 1), + (9, Around, (7, 29), '{', 1), + (18, Around, (17, 23), '[', 1), ], ), ( "(stepped (surround) pairs (should) skip)", - vec![(22, Inside, (1, 38), '(', 1), (22, Around, (0, 39), '(', 1)], + vec![(22, Inside, (1, 39), '(', 1), (22, Around, (0, 40), '(', 1)], ), ( "[surround pairs{\non different]\nlines}", vec![ - (7, Inside, (1, 28), '[', 1), - (15, Inside, (16, 35), '{', 1), - (7, Around, (0, 29), '[', 1), - (15, Around, (15, 36), '{', 1), + (7, Inside, (1, 29), '[', 1), + (15, Inside, (16, 36), '{', 1), + (7, Around, (0, 30), '[', 1), + (15, Around, (15, 37), '{', 1), ], ), ]; diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 32eb3f00..f6df26ba 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1,9 +1,6 @@ use helix_core::{ comment, coords_at_pos, find_first_non_whitespace_char, find_root, graphemes, indent, - line_ending::{ - get_line_ending_of_str, line_end_char_index, rope_end_without_line_ending, - str_is_line_ending, - }, + line_ending::{get_line_ending_of_str, line_end_char_index, str_is_line_ending}, match_brackets, movement::{self, Direction}, object, pos_at_coords, @@ -110,7 +107,10 @@ enum Align { } fn align_view(doc: &Document, view: &mut View, align: Align) { - let pos = doc.selection(view.id).cursor(); + let pos = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); let line = doc.text().char_to_line(pos); let relative = match align { @@ -340,7 +340,8 @@ fn move_char_left(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + + let selection = doc.selection(view.id).clone().transform(|range| { movement::move_horizontally(text, range, Direction::Backward, count, Movement::Move) }); doc.set_selection(view.id, selection); @@ -350,7 +351,8 @@ fn move_char_right(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + + let selection = doc.selection(view.id).clone().transform(|range| { movement::move_horizontally(text, range, Direction::Forward, count, Movement::Move) }); doc.set_selection(view.id, selection); @@ -360,7 +362,8 @@ fn move_line_up(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + + let selection = doc.selection(view.id).clone().transform(|range| { movement::move_vertically(text, range, Direction::Backward, count, Movement::Move) }); doc.set_selection(view.id, selection); @@ -370,7 +373,8 @@ fn move_line_down(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + + let selection = doc.selection(view.id).clone().transform(|range| { movement::move_vertically(text, range, Direction::Forward, count, Movement::Move) }); doc.set_selection(view.id, selection); @@ -378,84 +382,61 @@ fn move_line_down(cx: &mut Context) { fn goto_line_end(cx: &mut Context) { let (view, doc) = current!(cx.editor); + let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { - let text = doc.text(); - let line = text.char_to_line(range.head); + let selection = doc.selection(view.id).clone().transform(|range| { + let line = range.cursor_line(text); + let line_start = text.line_to_char(line); - let pos = line_end_char_index(&text.slice(..), line); - let pos = graphemes::nth_prev_grapheme_boundary(text.slice(..), pos, 1); - let pos = range.head.max(pos).max(text.line_to_char(line)); + let pos = graphemes::prev_grapheme_boundary(text, line_end_char_index(&text, line)) + .max(line_start); - Range::new( - match doc.mode { - Mode::Normal | Mode::Insert => pos, - Mode::Select => range.anchor, - }, - pos, - ) + range.put_cursor(text, pos, doc.mode == Mode::Select) }); - doc.set_selection(view.id, selection); } fn goto_line_end_newline(cx: &mut Context) { let (view, doc) = current!(cx.editor); + let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { - let text = doc.text(); - let line = text.char_to_line(range.head); + let selection = doc.selection(view.id).clone().transform(|range| { + let line = range.cursor_line(text); + let pos = line_end_char_index(&text, line); - let pos = line_end_char_index(&text.slice(..), line); - Range::new(pos, pos) + range.put_cursor(text, pos, doc.mode == Mode::Select) }); - doc.set_selection(view.id, selection); } fn goto_line_start(cx: &mut Context) { let (view, doc) = current!(cx.editor); + let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { - let text = doc.text(); - let line = text.char_to_line(range.head); + let selection = doc.selection(view.id).clone().transform(|range| { + let line = range.cursor_line(text); // adjust to start of the line let pos = text.line_to_char(line); - Range::new( - match doc.mode { - Mode::Normal => range.anchor, - Mode::Select | Mode::Insert => pos, - }, - pos, - ) + range.put_cursor(text, pos, doc.mode == Mode::Select) }); - doc.set_selection(view.id, selection); } fn goto_first_nonwhitespace(cx: &mut Context) { let (view, doc) = current!(cx.editor); + let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { - let text = doc.text(); - let line_idx = text.char_to_line(range.head); + let selection = doc.selection(view.id).clone().transform(|range| { + let line = range.cursor_line(text); - if let Some(pos) = find_first_non_whitespace_char(text.line(line_idx)) { - let pos = pos + text.line_to_char(line_idx); - Range::new( - match doc.mode { - Mode::Normal => pos, - Mode::Select => range.anchor, - Mode::Insert => unreachable!(), - }, - pos, - ) + if let Some(pos) = find_first_non_whitespace_char(text.line(line)) { + let pos = pos + text.line_to_char(line); + range.put_cursor(text, pos, doc.mode == Mode::Select) } else { range } }); - doc.set_selection(view.id, selection); } @@ -501,8 +482,8 @@ fn move_next_word_start(cx: &mut Context) { let selection = doc .selection(view.id) + .clone() .transform(|range| movement::move_next_word_start(text, range, count)); - doc.set_selection(view.id, selection); } @@ -513,8 +494,8 @@ fn move_prev_word_start(cx: &mut Context) { let selection = doc .selection(view.id) + .clone() .transform(|range| movement::move_prev_word_start(text, range, count)); - doc.set_selection(view.id, selection); } @@ -525,8 +506,8 @@ fn move_next_word_end(cx: &mut Context) { let selection = doc .selection(view.id) + .clone() .transform(|range| movement::move_next_word_end(text, range, count)); - doc.set_selection(view.id, selection); } @@ -537,8 +518,8 @@ fn move_next_long_word_start(cx: &mut Context) { let selection = doc .selection(view.id) + .clone() .transform(|range| movement::move_next_long_word_start(text, range, count)); - doc.set_selection(view.id, selection); } @@ -549,8 +530,8 @@ fn move_prev_long_word_start(cx: &mut Context) { let selection = doc .selection(view.id) + .clone() .transform(|range| movement::move_prev_long_word_start(text, range, count)); - doc.set_selection(view.id, selection); } @@ -561,8 +542,8 @@ fn move_next_long_word_end(cx: &mut Context) { let selection = doc .selection(view.id) + .clone() .transform(|range| movement::move_next_long_word_end(text, range, count)); - doc.set_selection(view.id, selection); } @@ -579,9 +560,7 @@ fn goto_file_start(cx: &mut Context) { fn goto_file_end(cx: &mut Context) { push_jump(cx.editor); let (view, doc) = current!(cx.editor); - let text = doc.text(); - let last_line = text.line_to_char(text.len_lines().saturating_sub(2)); - doc.set_selection(view.id, Selection::point(last_line)); + doc.set_selection(view.id, Selection::point(doc.text().len_chars())); } fn extend_next_word_start(cx: &mut Context) { @@ -589,12 +568,11 @@ fn extend_next_word_start(cx: &mut Context) { let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + let selection = doc.selection(view.id).clone().transform(|range| { let word = movement::move_next_word_start(text, range, count); - let pos = word.head; - Range::new(range.anchor, pos) + let pos = word.cursor(text); + range.put_cursor(text, pos, true) }); - doc.set_selection(view.id, selection); } @@ -603,10 +581,10 @@ fn extend_prev_word_start(cx: &mut Context) { let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + let selection = doc.selection(view.id).clone().transform(|range| { let word = movement::move_prev_word_start(text, range, count); - let pos = word.head; - Range::new(range.anchor, pos) + let pos = word.cursor(text); + range.put_cursor(text, pos, true) }); doc.set_selection(view.id, selection); } @@ -616,12 +594,11 @@ fn extend_next_word_end(cx: &mut Context) { let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + let selection = doc.selection(view.id).clone().transform(|range| { let word = movement::move_next_word_end(text, range, count); - let pos = word.head; - Range::new(range.anchor, pos) + let pos = word.cursor(text); + range.put_cursor(text, pos, true) }); - doc.set_selection(view.id, selection); } @@ -667,26 +644,56 @@ where let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { - search_fn(text, ch, range.head, count, inclusive).map_or(range, |pos| { - if extend { - Range::new(range.anchor, pos) - } else { - // select - Range::new(range.head, pos) - } - // or (pos, pos) to move to found val - }) - }); + let selection = doc.selection(view.id).clone().transform(|range| { + // TODO: use `Range::cursor()` here instead. However, that works in terms of + // graphemes, whereas this function doesn't yet. So we're doing the same logic + // here, but just in terms of chars instead. + let search_start_pos = if range.anchor < range.head { + range.head - 1 + } else { + range.head + }; + search_fn(text, ch, search_start_pos, count, inclusive) + .map_or(range, |pos| range.put_cursor(text, pos, extend)) + }); doc.set_selection(view.id, selection); }) } +fn find_next_char_impl( + text: RopeSlice, + ch: char, + pos: usize, + n: usize, + inclusive: bool, +) -> Option<usize> { + let pos = (pos + 1).min(text.len_chars()); + if inclusive { + search::find_nth_next(text, ch, pos, n) + } else { + search::find_nth_next(text, ch, pos, n).map(|n| n.saturating_sub(1)) + } +} + +fn find_prev_char_impl( + text: RopeSlice, + ch: char, + pos: usize, + n: usize, + inclusive: bool, +) -> Option<usize> { + if inclusive { + search::find_nth_prev(text, ch, pos, n) + } else { + search::find_nth_prev(text, ch, pos, n).map(|n| (n + 1).min(text.len_chars())) + } +} + fn find_till_char(cx: &mut Context) { find_char_impl( cx, - search::find_nth_next, + find_next_char_impl, false, /* inclusive */ false, /* extend */ ) @@ -695,7 +702,7 @@ fn find_till_char(cx: &mut Context) { fn find_next_char(cx: &mut Context) { find_char_impl( cx, - search::find_nth_next, + find_next_char_impl, true, /* inclusive */ false, /* extend */ ) @@ -704,7 +711,7 @@ fn find_next_char(cx: &mut Context) { fn extend_till_char(cx: &mut Context) { find_char_impl( cx, - search::find_nth_next, + find_next_char_impl, false, /* inclusive */ true, /* extend */ ) @@ -713,7 +720,7 @@ fn extend_till_char(cx: &mut Context) { fn extend_next_char(cx: &mut Context) { find_char_impl( cx, - search::find_nth_next, + find_next_char_impl, true, /* inclusive */ true, /* extend */ ) @@ -722,7 +729,7 @@ fn extend_next_char(cx: &mut Context) { fn till_prev_char(cx: &mut Context) { find_char_impl( cx, - search::find_nth_prev, + find_prev_char_impl, false, /* inclusive */ false, /* extend */ ) @@ -731,7 +738,7 @@ fn till_prev_char(cx: &mut Context) { fn find_prev_char(cx: &mut Context) { find_char_impl( cx, - search::find_nth_prev, + find_prev_char_impl, true, /* inclusive */ false, /* extend */ ) @@ -740,7 +747,7 @@ fn find_prev_char(cx: &mut Context) { fn extend_till_prev_char(cx: &mut Context) { find_char_impl( cx, - search::find_nth_prev, + find_prev_char_impl, false, /* inclusive */ true, /* extend */ ) @@ -749,7 +756,7 @@ fn extend_till_prev_char(cx: &mut Context) { fn extend_prev_char(cx: &mut Context) { find_char_impl( cx, - search::find_nth_prev, + find_prev_char_impl, true, /* inclusive */ true, /* extend */ ) @@ -773,24 +780,29 @@ fn replace(cx: &mut Context) { _ => None, }; - if let Some(ch) = ch { - let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { - let max_to = rope_end_without_line_ending(&doc.text().slice(..)); - let to = std::cmp::min(max_to, range.to() + 1); - let text: String = RopeGraphemes::new(doc.text().slice(range.from()..to)) - .map(|g| { - let cow: Cow<str> = g.into(); - if str_is_line_ending(&cow) { - cow - } else { - ch.into() - } - }) - .collect(); + let selection = doc.selection(view.id); - (range.from(), to, Some(text.into())) - }); + if let Some(ch) = ch { + let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { + if !range.is_empty() { + let text: String = + RopeGraphemes::new(doc.text().slice(range.from()..range.to())) + .map(|g| { + let cow: Cow<str> = g.into(); + if str_is_line_ending(&cow) { + cow + } else { + ch.into() + } + }) + .collect(); + + (range.from(), range.to(), Some(text.into())) + } else { + // No change. + (range.from(), range.to(), None) + } + }); doc.apply(&transaction, view.id); doc.append_changes_to_history(view.id); @@ -800,24 +812,24 @@ fn replace(cx: &mut Context) { fn switch_case(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { - let text: Tendril = range - .fragment(doc.text().slice(..)) - .chars() - .flat_map(|ch| { - if ch.is_lowercase() { - ch.to_uppercase().collect() - } else if ch.is_uppercase() { - ch.to_lowercase().collect() - } else { - vec![ch] - } - }) - .collect(); + let selection = doc.selection(view.id); + let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { + let text: Tendril = range + .fragment(doc.text().slice(..)) + .chars() + .flat_map(|ch| { + if ch.is_lowercase() { + ch.to_uppercase().collect() + } else if ch.is_uppercase() { + ch.to_lowercase().collect() + } else { + vec![ch] + } + }) + .collect(); - (range.from(), range.to() + 1, Some(text)) - }); + (range.from(), range.to(), Some(text)) + }); doc.apply(&transaction, view.id); doc.append_changes_to_history(view.id); @@ -825,12 +837,12 @@ fn switch_case(cx: &mut Context) { fn switch_to_uppercase(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { - let text: Tendril = range.fragment(doc.text().slice(..)).to_uppercase().into(); + let selection = doc.selection(view.id); + let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { + let text: Tendril = range.fragment(doc.text().slice(..)).to_uppercase().into(); - (range.from(), range.to() + 1, Some(text)) - }); + (range.from(), range.to(), Some(text)) + }); doc.apply(&transaction, view.id); doc.append_changes_to_history(view.id); @@ -838,12 +850,12 @@ fn switch_to_uppercase(cx: &mut Context) { fn switch_to_lowercase(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { - let text: Tendril = range.fragment(doc.text().slice(..)).to_lowercase().into(); + let selection = doc.selection(view.id); + let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { + let text: Tendril = range.fragment(doc.text().slice(..)).to_lowercase().into(); - (range.from(), range.to() + 1, Some(text)) - }); + (range.from(), range.to(), Some(text)) + }); doc.apply(&transaction, view.id); doc.append_changes_to_history(view.id); @@ -852,7 +864,12 @@ fn switch_to_lowercase(cx: &mut Context) { fn scroll(cx: &mut Context, offset: usize, direction: Direction) { use Direction::*; let (view, doc) = current!(cx.editor); - let cursor = coords_at_pos(doc.text().slice(..), doc.selection(view.id).cursor()); + let cursor = coords_at_pos( + doc.text().slice(..), + doc.selection(view.id) + .primary() + .cursor(doc.text().slice(..)), + ); let doc_last_line = doc.text().len_lines() - 1; let last_line = view.last_line(doc); @@ -881,7 +898,7 @@ fn scroll(cx: &mut Context, offset: usize, direction: Direction) { .min(last_line.saturating_sub(scrolloff)); let text = doc.text().slice(..); - let pos = pos_at_coords(text, Position::new(line, cursor.col)); // this func will properly truncate to line end + let pos = pos_at_coords(text, Position::new(line, cursor.col), true); // this func will properly truncate to line end // TODO: only manipulate main selection doc.set_selection(view.id, Selection::point(pos)); @@ -915,7 +932,8 @@ fn extend_char_left(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + + let selection = doc.selection(view.id).clone().transform(|range| { movement::move_horizontally(text, range, Direction::Backward, count, Movement::Extend) }); doc.set_selection(view.id, selection); @@ -925,7 +943,8 @@ fn extend_char_right(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + + let selection = doc.selection(view.id).clone().transform(|range| { movement::move_horizontally(text, range, Direction::Forward, count, Movement::Extend) }); doc.set_selection(view.id, selection); @@ -935,7 +954,8 @@ fn extend_line_up(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + + let selection = doc.selection(view.id).clone().transform(|range| { movement::move_vertically(text, range, Direction::Backward, count, Movement::Extend) }); doc.set_selection(view.id, selection); @@ -945,7 +965,8 @@ fn extend_line_down(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + + let selection = doc.selection(view.id).clone().transform(|range| { movement::move_vertically(text, range, Direction::Forward, count, Movement::Extend) }); doc.set_selection(view.id, selection); @@ -954,7 +975,7 @@ fn extend_line_down(cx: &mut Context) { fn select_all(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let end = rope_end_without_line_ending(&doc.text().slice(..)); + let end = doc.text().len_chars(); doc.set_selection(view.id, Selection::single(0, end)) } @@ -992,9 +1013,14 @@ fn split_selection_on_newline(cx: &mut Context) { } fn search_impl(doc: &mut Document, view: &mut View, contents: &str, regex: &Regex, extend: bool) { - let text = doc.text(); + let text = doc.text().slice(..); let selection = doc.selection(view.id); - let start = text.char_to_byte(selection.cursor()); + + // Get the right side of the primary block cursor. + let start = text.char_to_byte(graphemes::next_grapheme_boundary( + text, + selection.primary().cursor(text), + )); // use find_at to find the next match after the cursor, loop around the end // Careful, `Regex` uses `bytes` as offsets, not character indices! @@ -1011,12 +1037,10 @@ fn search_impl(doc: &mut Document, view: &mut View, contents: &str, regex: &Rege return; } - let head = end - 1; - let selection = if extend { - selection.clone().push(Range::new(start, head)) + selection.clone().push(Range::new(start, end)) } else { - Selection::single(start, head) + Selection::single(start, end) }; doc.set_selection(view.id, selection); @@ -1079,16 +1103,15 @@ fn extend_line(cx: &mut Context) { let count = cx.count(); let (view, doc) = current!(cx.editor); - let pos = doc.selection(view.id).primary(); let text = doc.text(); + let range = doc.selection(view.id).primary(); - let line_start = text.char_to_line(pos.anchor); - let start = text.line_to_char(line_start); - let line_end = text.char_to_line(pos.head); - let mut end = line_end_char_index(&text.slice(..), line_end + count.saturating_sub(1)); + let (start_line, end_line) = range.line_range(text.slice(..)); + let start = text.line_to_char(start_line); + let mut end = text.line_to_char((end_line + count).min(text.len_lines())); - if pos.anchor == start && pos.head == end && line_end < (text.len_lines() - 2) { - end = line_end_char_index(&text.slice(..), line_end + 1); + if range.from() == start && range.to() == end { + end = text.line_to_char((end_line + count + 1).min(text.len_lines())); } doc.set_selection(view.id, Selection::single(start, end)); @@ -1097,41 +1120,36 @@ fn extend_line(cx: &mut Context) { fn extend_to_line_bounds(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let text = doc.text(); - let selection = doc.selection(view.id).transform(|range| { - let start = text.line_to_char(text.char_to_line(range.from())); - let end = text - .line_to_char(text.char_to_line(range.to()) + 1) - .saturating_sub(1); - - if range.anchor < range.head { - Range::new(start, end) - } else { - Range::new(end, start) - } - }); + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + let text = doc.text(); - doc.set_selection(view.id, selection); + let (start_line, end_line) = range.line_range(text.slice(..)); + let start = text.line_to_char(start_line); + let end = text.line_to_char((end_line + 1).min(text.len_lines())); + + if range.anchor <= range.head { + Range::new(start, end) + } else { + Range::new(end, start) + } + }), + ); } fn delete_selection_impl(reg: &mut Register, doc: &mut Document, view_id: ViewId) { - // first yank the selection - let values: Vec<String> = doc - .selection(view_id) - .fragments(doc.text().slice(..)) - .map(Cow::into_owned) - .collect(); + let text = doc.text().slice(..); + let selection = doc.selection(view_id); + // first yank the selection + let values: Vec<String> = selection.fragments(text).map(Cow::into_owned).collect(); reg.write(values); // then delete - let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view_id), |range| { - let alltext = doc.text().slice(..); - let max_to = rope_end_without_line_ending(&alltext); - let to = std::cmp::min(max_to, range.to() + 1); - (range.from(), to, None) - }); + let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { + (range.from(), range.to(), None) + }); doc.apply(&transaction, view_id); } @@ -1159,19 +1177,22 @@ fn change_selection(cx: &mut Context) { fn collapse_selection(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let selection = doc - .selection(view.id) - .transform(|range| Range::new(range.head, range.head)); + let text = doc.text().slice(..); + let selection = doc.selection(view.id).clone().transform(|range| { + let pos = range.cursor(text); + Range::new(pos, pos) + }); doc.set_selection(view.id, selection); } fn flip_selections(cx: &mut Context) { let (view, doc) = current!(cx.editor); + let selection = doc .selection(view.id) + .clone() .transform(|range| Range::new(range.head, range.anchor)); - doc.set_selection(view.id, selection); } @@ -1186,6 +1207,7 @@ fn insert_mode(cx: &mut Context) { let selection = doc .selection(view.id) + .clone() .transform(|range| Range::new(range.to(), range.from())); doc.set_selection(view.id, selection); } @@ -1195,18 +1217,13 @@ fn append_mode(cx: &mut Context) { let (view, doc) = current!(cx.editor); enter_insert_mode(doc); doc.restore_cursor = true; - let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { - Range::new( - range.from(), - graphemes::next_grapheme_boundary(text, range.to()), // to() + next char - ) - }); + // Make sure there's room at the end of the document if the last + // selection butts up against it. let end = text.len_chars(); - - if selection.iter().any(|range| range.head == end) { + let last_range = doc.selection(view.id).iter().last().unwrap(); + if !last_range.is_empty() && last_range.head == end { let transaction = Transaction::change( doc.text(), std::array::IntoIter::new([(end, end, Some(doc.line_ending.as_str().into()))]), @@ -1214,6 +1231,12 @@ fn append_mode(cx: &mut Context) { doc.apply(&transaction, view.id); } + let selection = doc.selection(view.id).clone().transform(|range| { + Range::new( + range.from(), + graphemes::next_grapheme_boundary(doc.text().slice(..), range.to()), + ) + }); doc.set_selection(view.id, selection); } @@ -1636,11 +1659,10 @@ mod cmd { match cx.editor.clipboard_provider.get_contents() { Ok(contents) => { + let selection = doc.selection(view.id); let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { - let max_to = rope_end_without_line_ending(&doc.text().slice(..)); - let to = std::cmp::min(max_to, range.to() + 1); - (range.from(), to, Some(contents.as_str().into())) + Transaction::change_by_selection(doc.text(), &selection, |range| { + (range.from(), range.to(), Some(contents.as_str().into())) }); doc.apply(&transaction, view.id); @@ -2236,10 +2258,10 @@ fn append_to_line(cx: &mut Context) { let (view, doc) = current!(cx.editor); enter_insert_mode(doc); - let selection = doc.selection(view.id).transform(|range| { - let text = doc.text(); - let line = text.char_to_line(range.head); - let pos = line_end_char_index(&text.slice(..), line); + let selection = doc.selection(view.id).clone().transform(|range| { + let text = doc.text().slice(..); + let line = range.cursor_line(text); + let pos = line_end_char_index(&text, line); Range::new(pos, pos) }); doc.set_selection(view.id, selection); @@ -2299,7 +2321,7 @@ fn open(cx: &mut Context, open: Open) { let mut offs = 0; let mut transaction = Transaction::change_by_selection(contents, selection, |range| { - let line = text.char_to_line(range.head); + let line = range.cursor_line(text); let line = match open { // adjust position to the end of the line (next line - 1) @@ -2373,7 +2395,7 @@ fn normal_mode(cx: &mut Context) { // if leaving append mode, move cursor back by 1 if doc.restore_cursor { let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + let selection = doc.selection(view.id).clone().transform(|range| { Range::new( range.from(), graphemes::prev_grapheme_boundary(text, range.to()), @@ -2413,6 +2435,23 @@ fn goto_last_accessed_file(cx: &mut Context) { } fn select_mode(cx: &mut Context) { + let (view, doc) = current!(cx.editor); + let text = doc.text().slice(..); + + // Make sure end-of-document selections are also 1-width. + // (With the exception of being in an empty document, of course.) + let selection = doc.selection(view.id).clone().transform(|range| { + if range.is_empty() && range.head == text.len_chars() { + Range::new( + graphemes::prev_grapheme_boundary(text, range.anchor), + range.head, + ) + } else { + range + } + }); + doc.set_selection(view.id, selection); + doc_mut!(cx.editor).mode = Mode::Select; } @@ -2485,7 +2524,13 @@ fn goto_definition(cx: &mut Context) { let offset_encoding = language_server.offset_encoding(); - let pos = pos_to_lsp_pos(doc.text(), doc.selection(view.id).cursor(), offset_encoding); + let pos = pos_to_lsp_pos( + doc.text(), + doc.selection(view.id) + .primary() + .cursor(doc.text().slice(..)), + offset_encoding, + ); // TODO: handle fails let future = language_server.goto_definition(doc.identifier(), pos, None); @@ -2522,7 +2567,13 @@ fn goto_type_definition(cx: &mut Context) { let offset_encoding = language_server.offset_encoding(); - let pos = pos_to_lsp_pos(doc.text(), doc.selection(view.id).cursor(), offset_encoding); + let pos = pos_to_lsp_pos( + doc.text(), + doc.selection(view.id) + .primary() + .cursor(doc.text().slice(..)), + offset_encoding, + ); // TODO: handle fails let future = language_server.goto_type_definition(doc.identifier(), pos, None); @@ -2559,7 +2610,13 @@ fn goto_implementation(cx: &mut Context) { let offset_encoding = language_server.offset_encoding(); - let pos = pos_to_lsp_pos(doc.text(), doc.selection(view.id).cursor(), offset_encoding); + let pos = pos_to_lsp_pos( + doc.text(), + doc.selection(view.id) + .primary() + .cursor(doc.text().slice(..)), + offset_encoding, + ); // TODO: handle fails let future = language_server.goto_implementation(doc.identifier(), pos, None); @@ -2596,7 +2653,13 @@ fn goto_reference(cx: &mut Context) { let offset_encoding = language_server.offset_encoding(); - let pos = pos_to_lsp_pos(doc.text(), doc.selection(view.id).cursor(), offset_encoding); + let pos = pos_to_lsp_pos( + doc.text(), + doc.selection(view.id) + .primary() + .cursor(doc.text().slice(..)), + offset_encoding, + ); // TODO: handle fails let future = language_server.goto_reference(doc.identifier(), pos, None); @@ -2655,7 +2718,10 @@ fn goto_next_diag(cx: &mut Context) { let editor = &mut cx.editor; let (view, doc) = current!(editor); - let cursor_pos = doc.selection(view.id).cursor(); + let cursor_pos = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); let diag = if let Some(diag) = doc .diagnostics() .iter() @@ -2676,7 +2742,10 @@ fn goto_prev_diag(cx: &mut Context) { let editor = &mut cx.editor; let (view, doc) = current!(editor); - let cursor_pos = doc.selection(view.id).cursor(); + let cursor_pos = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); let diag = if let Some(diag) = doc .diagnostics() .iter() @@ -2704,7 +2773,9 @@ fn signature_help(cx: &mut Context) { let pos = pos_to_lsp_pos( doc.text(), - doc.selection(view.id).cursor(), + doc.selection(view.id) + .primary() + .cursor(doc.text().slice(..)), language_server.offset_encoding(), ); @@ -2827,11 +2898,11 @@ pub mod insert { let (view, doc) = current!(cx.editor); let text = doc.text(); - let selection = doc.selection(view.id); + let selection = doc.selection(view.id).clone().cursors(text.slice(..)); // run through insert hooks, stopping on the first one that returns Some(t) for hook in HOOKS { - if let Some(transaction) = hook(text, selection, c) { + if let Some(transaction) = hook(text, &selection, c) { doc.apply(&transaction, view.id); break; } @@ -2851,7 +2922,11 @@ pub mod insert { // indent by one to reach 4 spaces). let indent = Tendril::from(doc.indent_unit()); - let transaction = Transaction::insert(doc.text(), doc.selection(view.id), indent); + let transaction = Transaction::insert( + doc.text(), + &doc.selection(view.id).clone().cursors(doc.text().slice(..)), + indent, + ); doc.apply(&transaction, view.id); } @@ -2860,13 +2935,13 @@ pub mod insert { let text = doc.text().slice(..); let contents = doc.text(); - let selection = doc.selection(view.id); + let selection = doc.selection(view.id).clone().cursors(text); let mut ranges = SmallVec::with_capacity(selection.len()); // TODO: this is annoying, but we need to do it to properly calculate pos after edits let mut offs = 0; - let mut transaction = Transaction::change_by_selection(contents, selection, |range| { + let mut transaction = Transaction::change_by_selection(contents, &selection, |range| { let pos = range.head; let prev = if pos == 0 { @@ -2929,9 +3004,10 @@ pub mod insert { let text = doc.text().slice(..); let transaction = Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { + let pos = range.cursor(text); ( - graphemes::nth_prev_grapheme_boundary(text, range.head, count), - range.head, + graphemes::nth_prev_grapheme_boundary(text, pos, count), + pos, None, ) }); @@ -2944,9 +3020,10 @@ pub mod insert { let text = doc.text().slice(..); let transaction = Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { + let pos = range.cursor(text); ( - range.head, - graphemes::nth_next_grapheme_boundary(text, range.head, count), + pos, + graphemes::nth_next_grapheme_boundary(text, pos, count), None, ) }); @@ -2957,8 +3034,10 @@ pub mod insert { let count = cx.count(); let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); + let selection = doc .selection(view.id) + .clone() .transform(|range| movement::move_prev_word_start(text, range, count)); doc.set_selection(view.id, selection); delete_selection(cx) @@ -2986,9 +3065,11 @@ fn redo(cx: &mut Context) { fn yank(cx: &mut Context) { let (view, doc) = current!(cx.editor); + let text = doc.text().slice(..); + let values: Vec<String> = doc .selection(view.id) - .fragments(doc.text().slice(..)) + .fragments(text) .map(Cow::into_owned) .collect(); @@ -3007,10 +3088,11 @@ fn yank(cx: &mut Context) { fn yank_joined_to_clipboard_impl(editor: &mut Editor, separator: &str) -> anyhow::Result<()> { let (view, doc) = current!(editor); + let text = doc.text().slice(..); let values: Vec<String> = doc .selection(view.id) - .fragments(doc.text().slice(..)) + .fragments(text) .map(Cow::into_owned) .collect(); @@ -3038,11 +3120,9 @@ fn yank_joined_to_clipboard(cx: &mut Context) { fn yank_main_selection_to_clipboard_impl(editor: &mut Editor) -> anyhow::Result<()> { let (view, doc) = current!(editor); + let text = doc.text().slice(..); - let value = doc - .selection(view.id) - .primary() - .fragment(doc.text().slice(..)); + let value = doc.selection(view.id).primary().fragment(text); if let Err(e) = editor.clipboard_provider.set_contents(value.into_owned()) { bail!("Couldn't set system clipboard content: {:?}", e); @@ -3083,17 +3163,21 @@ fn paste_impl( let mut values = values.iter().cloned().map(Tendril::from).chain(repeat); let text = doc.text(); + let selection = doc.selection(view.id); - let transaction = Transaction::change_by_selection(text, doc.selection(view.id), |range| { + let transaction = Transaction::change_by_selection(text, selection, |range| { let pos = match (action, linewise) { // paste linewise before (Paste::Before, true) => text.line_to_char(text.char_to_line(range.from())), // paste linewise after - (Paste::After, true) => text.line_to_char(text.char_to_line(range.to()) + 1), + (Paste::After, true) => { + let line = range.line_range(text.slice(..)).1; + text.line_to_char((line + 1).min(text.len_lines())) + } // paste insert (Paste::Before, false) => range.from(), // paste append - (Paste::After, false) => range.to() + 1, + (Paste::After, false) => range.to(), }; (pos, pos, Some(values.next().unwrap())) }); @@ -3134,12 +3218,14 @@ fn replace_with_yanked(cx: &mut Context) { if let Some(values) = registers.read(reg_name) { if let Some(yank) = values.first() { - let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { - let max_to = rope_end_without_line_ending(&doc.text().slice(..)); - let to = std::cmp::min(max_to, range.to() + 1); - (range.from(), to, Some(yank.as_str().into())) - }); + let selection = doc.selection(view.id); + let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { + if !range.is_empty() { + (range.from(), range.to(), Some(yank.as_str().into())) + } else { + (range.from(), range.to(), None) + } + }); doc.apply(&transaction, view.id); doc.append_changes_to_history(view.id); @@ -3152,12 +3238,10 @@ fn replace_selections_with_clipboard_impl(editor: &mut Editor) -> anyhow::Result match editor.clipboard_provider.get_contents() { Ok(contents) => { - let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { - let max_to = rope_end_without_line_ending(&doc.text().slice(..)); - let to = std::cmp::min(max_to, range.to() + 1); - (range.from(), to, Some(contents.as_str().into())) - }); + let selection = doc.selection(view.id); + let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { + (range.from(), range.to(), Some(contents.as_str().into())) + }); doc.apply(&transaction, view.id); doc.append_changes_to_history(view.id); @@ -3204,8 +3288,7 @@ fn get_lines(doc: &Document, view_id: ViewId) -> Vec<usize> { // Get all line numbers for range in doc.selection(view_id) { - let start = doc.text().char_to_line(range.from()); - let end = doc.text().char_to_line(range.to()); + let (start, end) = range.line_range(doc.text().slice(..)); for line in start..=end { lines.push(line) @@ -3333,10 +3416,9 @@ fn join_selections(cx: &mut Context) { let fragment = Tendril::from(" "); for selection in doc.selection(view.id) { - let start = text.char_to_line(selection.from()); - let mut end = text.char_to_line(selection.to()); + let (start, mut end) = selection.line_range(slice); if start == end { - end += 1 + end = (end + 1).min(text.len_lines() - 1); } let lines = start..end; @@ -3433,13 +3515,17 @@ fn completion(cx: &mut Context) { }; let offset_encoding = language_server.offset_encoding(); + let cursor = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); - let pos = pos_to_lsp_pos(doc.text(), doc.selection(view.id).cursor(), offset_encoding); + let pos = pos_to_lsp_pos(doc.text(), cursor, offset_encoding); // TODO: handle fails let future = language_server.completion(doc.identifier(), pos, None); - let trigger_offset = doc.selection(view.id).cursor(); + let trigger_offset = cursor; cx.callback( future, @@ -3489,7 +3575,9 @@ fn hover(cx: &mut Context) { let pos = pos_to_lsp_pos( doc.text(), - doc.selection(view.id).cursor(), + doc.selection(view.id) + .primary() + .cursor(doc.text().slice(..)), language_server.offset_encoding(), ); @@ -3555,7 +3643,10 @@ fn match_brackets(cx: &mut Context) { let (view, doc) = current!(cx.editor); if let Some(syntax) = doc.syntax() { - let pos = doc.selection(view.id).cursor(); + let pos = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); if let Some(pos) = match_brackets::find(syntax, doc.text(), pos) { let selection = Selection::point(pos); doc.set_selection(view.id, selection); @@ -3655,7 +3746,10 @@ fn align_view_bottom(cx: &mut Context) { fn align_view_middle(cx: &mut Context) { let (view, doc) = current!(cx.editor); - let pos = doc.selection(view.id).cursor(); + let pos = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); let pos = coords_at_pos(doc.text().slice(..), pos); const OFFSET: usize = 7; // gutters @@ -3687,7 +3781,7 @@ fn select_textobject(cx: &mut Context, objtype: textobject::TextObject) { let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); - let selection = doc.selection(view.id).transform(|range| { + let selection = doc.selection(view.id).clone().transform(|range| { match ch { 'w' => textobject::textobject_word(text, range, objtype, count), // TODO: cancel new ranges if inconsistent surround matches across lines @@ -3697,7 +3791,6 @@ fn select_textobject(cx: &mut Context, objtype: textobject::TextObject) { _ => range, } }); - doc.set_selection(view.id, selection); } }) @@ -3707,18 +3800,13 @@ fn surround_add(cx: &mut Context) { cx.on_next_key(move |cx, event| { if let Some(ch) = event.char() { let (view, doc) = current!(cx.editor); - let text = doc.text().slice(..); let selection = doc.selection(view.id); let (open, close) = surround::get_pair(ch); let mut changes = Vec::new(); for range in selection.iter() { - let from = range.from(); - let max_to = rope_end_without_line_ending(&text); - let to = std::cmp::min(range.to() + 1, max_to); - - changes.push((from, from, Some(Tendril::from_char(open)))); - changes.push((to, to, Some(Tendril::from_char(close)))); + changes.push((range.from(), range.from(), Some(Tendril::from_char(open)))); + changes.push((range.to(), range.to(), Some(Tendril::from_char(close)))); } let transaction = Transaction::change(doc.text(), changes.into_iter()); @@ -3748,8 +3836,11 @@ fn surround_replace(cx: &mut Context) { let transaction = Transaction::change( doc.text(), change_pos.iter().enumerate().map(|(i, &pos)| { - let ch = if i % 2 == 0 { open } else { close }; - (pos, pos + 1, Some(Tendril::from_char(ch))) + if i % 2 == 0 { + (pos, pos + 1, Some(Tendril::from_char(open))) + } else { + (pos.saturating_sub(1), pos, Some(Tendril::from_char(close))) + } }), ); doc.apply(&transaction, view.id); diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs index be6db42c..2725d53d 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -86,7 +86,10 @@ impl Completion { let item = item.unwrap(); // if more text was entered, remove it - let cursor = doc.selection(view.id).cursor(); + let cursor = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); if trigger_offset < cursor { let remove = Transaction::change( doc.text(), @@ -109,7 +112,10 @@ impl Completion { ) } else { let text = item.insert_text.as_ref().unwrap_or(&item.label); - let cursor = doc.selection(view.id).cursor(); + let cursor = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); Transaction::change( doc.text(), vec![(cursor, cursor, Some(text.as_str().into()))].into_iter(), @@ -155,7 +161,10 @@ impl Completion { // TODO: hooks should get processed immediately so maybe do it after select!(), before // looping? - let cursor = doc.selection(view.id).cursor(); + let cursor = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); if self.trigger_offset <= cursor { let fragment = doc.text().slice(self.trigger_offset..cursor); let text = Cow::from(fragment); @@ -212,7 +221,10 @@ impl Component for Completion { .language() .and_then(|scope| scope.strip_prefix("source.")) .unwrap_or(""); - let cursor_pos = doc.selection(view.id).cursor(); + let cursor_pos = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); let cursor_pos = (helix_core::coords_at_pos(doc.text().slice(..), cursor_pos).row - view.first_line) as u16; diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 99b49309..d5c907b8 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -8,7 +8,7 @@ use crate::{ use helix_core::{ coords_at_pos, - graphemes::{ensure_grapheme_boundary, next_grapheme_boundary}, + graphemes::{ensure_grapheme_boundary_next, next_grapheme_boundary, prev_grapheme_boundary}, syntax::{self, HighlightEvent}, unicode::segmentation::UnicodeSegmentation, unicode::width::UnicodeWidthStr, @@ -166,8 +166,8 @@ impl EditorView { let highlights = highlights.into_iter().map(|event| match event.unwrap() { // convert byte offsets to char offset HighlightEvent::Source { start, end } => { - let start = ensure_grapheme_boundary(text, text.byte_to_char(start)); - let end = ensure_grapheme_boundary(text, text.byte_to_char(end)); + let start = ensure_grapheme_boundary_next(text, text.byte_to_char(start)); + let end = ensure_grapheme_boundary_next(text, text.byte_to_char(end)); HighlightEvent::Source { start, end } } event => event, @@ -191,21 +191,18 @@ impl EditorView { } .unwrap_or(base_cursor_scope); - let primary_selection_scope = theme - .find_scope_index("ui.selection.primary") - .unwrap_or(selection_scope); - let highlights: Box<dyn Iterator<Item = HighlightEvent>> = if is_focused { - // inject selections as highlight scopes - let mut spans: Vec<(usize, std::ops::Range<usize>)> = Vec::new(); - // TODO: primary + insert mode patching: // (ui.cursor.primary).patch(mode).unwrap_or(cursor) - let primary_cursor_scope = theme .find_scope_index("ui.cursor.primary") .unwrap_or(cursor_scope); + let primary_selection_scope = theme + .find_scope_index("ui.selection.primary") + .unwrap_or(selection_scope); + // inject selections as highlight scopes + let mut spans: Vec<(usize, std::ops::Range<usize>)> = Vec::new(); for (i, range) in selections.iter().enumerate() { let (cursor_scope, selection_scope) = if i == primary_idx { (primary_cursor_scope, primary_selection_scope) @@ -213,24 +210,23 @@ impl EditorView { (cursor_scope, selection_scope) }; - let cursor_end = next_grapheme_boundary(text, range.head); // Used in every case below. - - if range.head == range.anchor { - spans.push((cursor_scope, range.head..cursor_end)); + // Special-case: cursor at end of the rope. + if range.head == range.anchor && range.head == text.len_chars() { + spans.push((cursor_scope, range.head..range.head + 1)); continue; } - let reverse = range.head < range.anchor; - - if reverse { - spans.push((cursor_scope, range.head..cursor_end)); - spans.push(( - selection_scope, - cursor_end..next_grapheme_boundary(text, range.anchor), - )); + let range = range.min_width_1(text); + if range.head > range.anchor { + // Standard case. + let cursor_start = prev_grapheme_boundary(text, range.head); + spans.push((selection_scope, range.anchor..cursor_start)); + spans.push((cursor_scope, cursor_start..range.head)); } else { - spans.push((selection_scope, range.anchor..range.head)); + // Reverse case. + let cursor_end = next_grapheme_boundary(text, range.head); spans.push((cursor_scope, range.head..cursor_end)); + spans.push((selection_scope, cursor_end..range.anchor)); } } @@ -263,7 +259,10 @@ impl EditorView { spans.pop(); } HighlightEvent::Source { start, end } => { - let text = text.slice(start..end); + // `unwrap_or_else` part is for off-the-end indices of + // the rope, to allow cursor highlighting at the end + // of the rope. + let text = text.get_slice(start..end).unwrap_or_else(|| " ".into()); use helix_core::graphemes::{grapheme_width, RopeGraphemes}; @@ -332,7 +331,11 @@ impl EditorView { let info: Style = theme.get("info"); let hint: Style = theme.get("hint"); - for (i, line) in (view.first_line..last_line).enumerate() { + // Whether to draw the line number for the last line of the + // document or not. We only draw it if it's not an empty line. + let draw_last = text.line_to_byte(last_line) < text.len_bytes(); + + for (i, line) in (view.first_line..(last_line + 1)).enumerate() { use helix_core::diagnostic::Severity; if let Some(diagnostic) = doc.diagnostics().iter().find(|d| d.line == line) { surface.set_stringn( @@ -349,11 +352,17 @@ impl EditorView { ); } - // line numbers having selections are rendered differently + // Line numbers having selections are rendered + // differently, further below. + let line_number_text = if line == last_line && !draw_last { + " ~".into() + } else { + format!("{:>5}", line + 1) + }; surface.set_stringn( viewport.x + 1 - OFFSET, viewport.y + i as u16, - format!("{:>5}", line + 1), + line_number_text, 5, linenr, ); @@ -367,19 +376,34 @@ impl EditorView { if is_focused { let screen = { let start = text.line_to_char(view.first_line); - let end = text.line_to_char(last_line + 1); + let end = text.line_to_char(last_line + 1) + 1; // +1 for cursor at end of text. Range::new(start, end) }; let selection = doc.selection(view.id); for selection in selection.iter().filter(|range| range.overlaps(&screen)) { - let head = view.screen_coords_at_pos(doc, text, selection.head); + let head = view.screen_coords_at_pos( + doc, + text, + if selection.head > selection.anchor { + selection.head - 1 + } else { + selection.head + }, + ); if let Some(head) = head { + // Draw line number for selected lines. + let line_number = view.first_line + head.row; + let line_number_text = if line_number == last_line && !draw_last { + " ~".into() + } else { + format!("{:>5}", line_number + 1) + }; surface.set_stringn( viewport.x + 1 - OFFSET, viewport.y + head.row as u16, - format!("{:>5}", view.first_line + head.row + 1), + line_number_text, 5, linenr_select, ); @@ -387,7 +411,10 @@ impl EditorView { // TODO: set cursor position for IME if let Some(syntax) = doc.syntax() { use helix_core::match_brackets; - let pos = doc.selection(view.id).cursor(); + let pos = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); let pos = match_brackets::find(syntax, doc.text(), pos) .and_then(|pos| view.screen_coords_at_pos(doc, text, pos)); @@ -432,7 +459,10 @@ impl EditorView { widgets::{Paragraph, Widget}, }; - let cursor = doc.selection(view.id).cursor(); + let cursor = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); let diagnostics = doc.diagnostics().iter().filter(|diagnostic| { diagnostic.range.start <= cursor && diagnostic.range.end >= cursor @@ -544,7 +574,12 @@ impl EditorView { // _ => "indent:ERROR", // }; let position_info = { - let pos = coords_at_pos(doc.text().slice(..), doc.selection(view.id).cursor()); + let pos = coords_at_pos( + doc.text().slice(..), + doc.selection(view.id) + .primary() + .cursor(doc.text().slice(..)), + ); format!("{}:{}", pos.row + 1, pos.col + 1) // convert to 1-indexing }; diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index c751785d..c2078060 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -306,19 +306,6 @@ pub async fn to_writer<'a, W: tokio::io::AsyncWriteExt + Unpin + ?Sized>( Ok(()) } -/// Inserts the final line ending into `rope` if it's missing. [Why?](https://stackoverflow.com/questions/729692/why-should-text-files-end-with-a-newline) -pub fn with_line_ending(rope: &mut Rope) -> LineEnding { - // search for line endings - let line_ending = auto_detect_line_ending(rope).unwrap_or(DEFAULT_LINE_ENDING); - - // add missing newline at the end of file - if rope.len_bytes() == 0 || !char_is_line_ending(rope.char(rope.len_chars() - 1)) { - rope.insert(rope.len_chars(), line_ending.as_str()); - } - - line_ending -} - /// Like std::mem::replace() except it allows the replacement value to be mapped from the /// original value. fn take_with<T, F>(mut_ref: &mut T, closure: F) @@ -456,7 +443,7 @@ impl Document { theme: Option<&Theme>, config_loader: Option<&syntax::Loader>, ) -> Result<Self, Error> { - let (mut rope, encoding) = if path.exists() { + let (rope, encoding) = if path.exists() { let mut file = std::fs::File::open(&path).context(format!("unable to open {:?}", path))?; from_reader(&mut file, encoding)? @@ -465,7 +452,6 @@ impl Document { (Rope::from(DEFAULT_LINE_ENDING.as_str()), encoding) }; - let line_ending = with_line_ending(&mut rope); let mut doc = Self::from(rope, Some(encoding)); // set the path and try detecting the language @@ -474,9 +460,9 @@ impl Document { doc.detect_language(theme, loader); } - // Detect indentation style and set line ending. + // Detect indentation style and line ending. doc.detect_indent_style(); - doc.line_ending = line_ending; + doc.line_ending = auto_detect_line_ending(&doc.text).unwrap_or(DEFAULT_LINE_ENDING); Ok(doc) } @@ -605,17 +591,16 @@ impl Document { } let mut file = std::fs::File::open(path.unwrap())?; - let (mut rope, ..) = from_reader(&mut file, Some(encoding))?; - let line_ending = with_line_ending(&mut rope); + let (rope, ..) = from_reader(&mut file, Some(encoding))?; let transaction = helix_core::diff::compare_ropes(self.text(), &rope); self.apply(&transaction, view_id); self.append_changes_to_history(view_id); self.reset_modified(); - // Detect indentation style and set line ending. + // Detect indentation style and line ending. self.detect_indent_style(); - self.line_ending = line_ending; + self.line_ending = auto_detect_line_ending(&self.text).unwrap_or(DEFAULT_LINE_ENDING); Ok(()) } @@ -807,7 +792,8 @@ impl Document { pub fn set_selection(&mut self, view_id: ViewId, selection: Selection) { // TODO: use a transaction? - self.selections.insert(view_id, selection); + self.selections + .insert(view_id, selection.ensure_invariants(self.text().slice(..))); } fn apply_impl(&mut self, transaction: &Transaction, view_id: ViewId) -> bool { @@ -822,7 +808,12 @@ impl Document { .selection() .cloned() .unwrap_or_else(|| self.selection(view_id).clone().map(transaction.changes())); - self.set_selection(view_id, selection); + self.selections.insert(view_id, selection); + + // Ensure all selections accross all views still adhere to invariants. + for selection in self.selections.values_mut() { + *selection = selection.clone().ensure_invariants(self.text.slice(..)); + } } if !transaction.changes().is_empty() { @@ -1089,7 +1080,7 @@ impl Document { impl Default for Document { fn default() -> Self { - let text = Rope::from(DEFAULT_LINE_ENDING.as_str()); + let text = Rope::from(""); Self::from(text, None) } } @@ -1214,11 +1205,7 @@ mod test { #[test] fn test_line_ending() { - if cfg!(windows) { - assert_eq!(Document::default().text().to_string(), "\r\n"); - } else { - assert_eq!(Document::default().text().to_string(), "\n"); - } + assert_eq!(Document::default().text().to_string(), ""); } macro_rules! test_decode { diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 7ff689df..7e8548e7 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -138,12 +138,14 @@ impl Editor { let (view, doc) = current!(self); // initialize selection for view - let selection = doc - .selections + doc.selections .entry(view.id) .or_insert_with(|| Selection::point(0)); // TODO: reuse align_view - let pos = selection.cursor(); + let pos = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); let line = doc.text().char_to_line(pos); view.first_line = line.saturating_sub(view.area.height as usize / 2); @@ -293,7 +295,10 @@ impl Editor { const OFFSET: u16 = 7; // 1 diagnostic + 5 linenr + 1 gutter let view = view!(self); let doc = &self.documents[view.doc]; - let cursor = doc.selection(view.id).cursor(); + let cursor = doc + .selection(view.id) + .primary() + .cursor(doc.text().slice(..)); if let Some(mut pos) = view.screen_coords_at_pos(doc, doc.text().slice(..), cursor) { pos.col += view.area.x as usize + OFFSET as usize; pos.row += view.area.y as usize; diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index 24df7a4f..6b0c3c2a 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -84,18 +84,21 @@ impl View { } pub fn ensure_cursor_in_view(&mut self, doc: &Document) { - let cursor = doc.selection(self.id).cursor(); + let cursor = doc + .selection(self.id) + .primary() + .cursor(doc.text().slice(..)); let pos = coords_at_pos(doc.text().slice(..), cursor); let line = pos.row; let col = pos.col; let height = self.area.height.saturating_sub(1); // - 1 for statusline - let last_line = self.first_line + height as usize; + let last_line = (self.first_line + height as usize).saturating_sub(1); let scrolloff = PADDING.min(self.area.height as usize / 2); // TODO: user pref // TODO: not ideal const OFFSET: usize = 7; // 1 diagnostic + 5 linenr + 1 gutter - let last_col = self.first_col + (self.area.width as usize - OFFSET); + let last_col = (self.first_col + self.area.width as usize).saturating_sub(OFFSET + 1); if line > last_line.saturating_sub(scrolloff) { // scroll down @@ -119,8 +122,9 @@ impl View { pub fn last_line(&self, doc: &Document) -> usize { let height = self.area.height.saturating_sub(1); // - 1 for statusline std::cmp::min( - self.first_line + height as usize, - doc.text().len_lines() - 1, + // Saturating subs to make it inclusive zero indexing. + (self.first_line + height as usize).saturating_sub(1), + doc.text().len_lines().saturating_sub(1), ) } |