From 28d2d6880462509f0d3131eb2eb928bb8859e058 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Fri, 2 Jul 2021 09:51:29 -0700 Subject: [PATCH] Make horizontal selection movement work properly. --- helix-core/src/movement.rs | 98 ++++++++++++++++++++++++++------------ helix-term/src/commands.rs | 20 +++++++- 2 files changed, 87 insertions(+), 31 deletions(-) diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs index b810876c..e01786eb 100644 --- a/helix-core/src/movement.rs +++ b/helix-core/src/movement.rs @@ -5,8 +5,11 @@ use ropey::iter::Chars; use crate::{ chars::{categorize_char, char_is_line_ending, CharCategory}, coords_at_pos, - graphemes::{nth_next_grapheme_boundary, nth_prev_grapheme_boundary}, - line_ending::{get_line_ending, line_end_char_index}, + graphemes::{ + next_grapheme_boundary, nth_next_grapheme_boundary, nth_prev_grapheme_boundary, + prev_grapheme_boundary, + }, + line_ending::get_line_ending, pos_at_coords, Position, Range, RopeSlice, }; @@ -29,25 +32,60 @@ pub fn move_horizontally( count: usize, behaviour: Movement, ) -> Range { - let pos = range.head; - let line = slice.char_to_line(pos); - // TODO: we can optimize clamping by passing in RopeSlice limited to current line. that way - // we stop calculating past start/end of line. - let pos = match dir { - Direction::Backward => { - let start = slice.line_to_char(line); - nth_prev_grapheme_boundary(slice, pos, count).max(start) + match (behaviour, dir) { + (Movement::Move, Direction::Backward) => { + let count = if range.anchor < range.head { + count + 1 + } else { + count + }; + let pos = nth_prev_grapheme_boundary(slice, range.head, count); + Range::new(pos, pos) } - Direction::Forward => { - let end_char_idx = line_end_char_index(&slice, line); - nth_next_grapheme_boundary(slice, pos, count).min(end_char_idx) + (Movement::Move, Direction::Forward) => { + let count = if range.anchor < range.head { + count - 1 + } else { + count + }; + let pos = nth_next_grapheme_boundary(slice, range.head, count); + Range::new(pos, pos) } - }; - let anchor = match behaviour { - Movement::Extend => range.anchor, - Movement::Move => pos, - }; - Range::new(anchor, pos) + (Movement::Extend, Direction::Backward) => { + // Ensure a valid initial selection state. + let range = range.min_width_1(slice); + + // Do the main movement. + let mut head = nth_prev_grapheme_boundary(slice, range.head, count); + let mut anchor = range.anchor; + + // If the head and anchor crossed over each other, we need to + // fiddle around to make it behave like a 1-wide cursor. + if head <= anchor && range.head > range.anchor { + anchor = next_grapheme_boundary(slice, anchor); + head = prev_grapheme_boundary(slice, head); + } + + Range::new(anchor, head) + } + (Movement::Extend, Direction::Forward) => { + // Ensure a valid initial selection state. + let range = range.min_width_1(slice); + + // Do the main movement. + let mut head = nth_next_grapheme_boundary(slice, range.head, count); + let mut anchor = range.anchor; + + // If the head and anchor crossed over each other, we need to + // fiddle around to make it behave like a 1-wide cursor. + if head >= anchor && range.head < range.anchor { + anchor = prev_grapheme_boundary(slice, anchor); + head = next_grapheme_boundary(slice, head); + } + + Range::new(anchor, head) + } + } } pub fn move_vertically( @@ -323,7 +361,7 @@ mod test { } #[test] - fn horizontal_moves_through_single_line_in_single_line_text() { + fn horizontal_moves_through_single_line_text() { let text = Rope::from(SINGLE_LINE_SAMPLE); let slice = text.slice(..); let position = pos_at_coords(slice, (0, 0).into()); @@ -346,7 +384,7 @@ mod test { } #[test] - fn horizontal_moves_through_single_line_in_multiline_text() { + fn horizontal_moves_through_multiline_text() { let text = Rope::from(MULTILINE_SAMPLE); let slice = text.slice(..); let position = pos_at_coords(slice, (0, 0).into()); @@ -354,15 +392,15 @@ mod test { let mut range = Range::point(position); let moves_and_expected_coordinates = IntoIter::new([ - ((Direction::Forward, 1usize), (0, 1)), // M|ultiline\n - ((Direction::Forward, 2usize), (0, 3)), // Mul|tiline\n - ((Direction::Backward, 6usize), (0, 0)), // |Multiline\n - ((Direction::Backward, 999usize), (0, 0)), // |Multiline\n - ((Direction::Forward, 3usize), (0, 3)), // Mul|tiline\n - ((Direction::Forward, 0usize), (0, 3)), // Mul|tiline\n - ((Direction::Backward, 0usize), (0, 3)), // Mul|tiline\n - ((Direction::Forward, 999usize), (0, 9)), // Multiline|\n - ((Direction::Forward, 999usize), (0, 9)), // Multiline|\n + ((Direction::Forward, 11usize), (1, 1)), // Multiline\nt|ext sample\n... + ((Direction::Backward, 1usize), (1, 0)), // Multiline\n|text sample\n... + ((Direction::Backward, 5usize), (0, 5)), // Multi|line\ntext sample\n... + ((Direction::Backward, 999usize), (0, 0)), // |Multiline\ntext sample\n... + ((Direction::Forward, 3usize), (0, 3)), // Mul|tiline\ntext sample\n... + ((Direction::Forward, 0usize), (0, 3)), // Mul|tiline\ntext sample\n... + ((Direction::Backward, 0usize), (0, 3)), // Mul|tiline\ntext sample\n... + ((Direction::Forward, 999usize), (5, 0)), // ...and whitespaced\n| + ((Direction::Forward, 999usize), (5, 0)), // ...and whitespaced\n| ]); for ((direction, amount), coordinates) in moves_and_expected_coordinates { diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index e76962f0..5ab0926a 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2193,7 +2193,25 @@ fn goto_mode(cx: &mut Context) { } fn select_mode(cx: &mut Context) { - doc_mut!(cx.editor).mode = Mode::Select; + let (view, doc) = current!(cx.editor); + + // Make sure all selections are at least 1-wide. + // (With the exception of being in an empty document, of course.) + doc.set_selection( + view.id, + doc.selection(view.id).clone().transform(|range| { + if range.is_empty() && range.head == doc.text().len_chars() { + Range::new( + graphemes::prev_grapheme_boundary(doc.text().slice(..), range.anchor), + range.head, + ) + } else { + range.min_width_1(doc.text().slice(..)) + } + }), + ); + + doc.mode = Mode::Select; } fn exit_select_mode(cx: &mut Context) {