From b4c59b444cc4963f95a95fe10f166e58ef857288 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Thu, 8 Jul 2021 16:45:19 -0700 Subject: Update surround commands to work with gap indexing. --- helix-core/src/surround.rs | 76 +++++++++++++++++++++++++++------------------- 1 file changed, 45 insertions(+), 31 deletions(-) (limited to 'helix-core/src/surround.rs') diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index 52f60cab..af357c96 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,35 @@ 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, true)?)), + _ if (pos + 1) == text.len_chars() => Some(( + search::find_nth_prev(text, open, pos, n, true)?, + 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, true)?, + search::find_nth_next(text, close, pos, n, true)?, + )) } } 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 +186,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 +201,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 +212,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 +228,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 +238,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 +257,7 @@ mod test { get_surround_pos(slice, &selection, '(', 1) .unwrap() .as_slice(), - &[0, 5, 7, 13, 15, 23] + &[0, 6, 7, 14, 15, 24] ); } -- cgit v1.2.3-70-g09d2 From f96b8b769b3c7457935b5c02db870af97036f7b6 Mon Sep 17 00:00:00 2001 From: Nathan Vegdahl Date: Sat, 24 Jul 2021 07:44:11 -0700 Subject: Switch to a cleaner range-head moving abstraction. Also fix a bunch of bugs related to it. --- helix-core/src/movement.rs | 35 ++++++++------- helix-core/src/search.rs | 26 ++---------- helix-core/src/selection.rs | 20 +++++---- helix-core/src/surround.rs | 13 +++--- helix-term/src/commands.rs | 101 ++++++++++++++++++++++++++++++++------------ 5 files changed, 111 insertions(+), 84 deletions(-) (limited to 'helix-core/src/surround.rs') diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs index 7f3a5ef4..80d19501 100644 --- a/helix-core/src/movement.rs +++ b/helix-core/src/movement.rs @@ -42,20 +42,18 @@ pub fn move_horizontally( }; // Compute the new position. - let mut new_pos = if dir == Direction::Backward { + let new_pos = if dir == Direction::Backward { nth_prev_grapheme_boundary(slice, pos, count) } else { nth_next_grapheme_boundary(slice, pos, count) }; - // Shift forward one grapheme if needed, for the - // visual 1-width cursor. - if behaviour == Extend && new_pos >= range.anchor { - new_pos = next_grapheme_boundary(slice, new_pos); - }; - // Compute the final new range. - range.put(slice, new_pos, behaviour == Extend) + if behaviour == Extend { + range.move_head(slice, new_pos, true) + } else { + Range::point(new_pos) + } } pub fn move_vertically( @@ -78,14 +76,17 @@ pub fn move_vertically( let horiz = range.horiz.unwrap_or(col as u32); // Compute the new position. - let new_pos = { + let (new_pos, new_row) = { let new_row = if dir == Direction::Backward { row.saturating_sub(count) } else { (row + count).min(slice.len_lines().saturating_sub(1)) }; let new_col = col.max(horiz as usize); - pos_at_coords(slice, Position::new(new_row, new_col), true) + ( + pos_at_coords(slice, Position::new(new_row, new_col), true), + new_row, + ) }; // Compute the new range according to the type of movement. @@ -97,15 +98,13 @@ pub fn move_vertically( }, Movement::Extend => { - let new_head = if new_pos >= range.anchor { - next_grapheme_boundary(slice, new_pos) + if slice.line(new_row).len_chars() > 0 { + let mut new_range = range.move_head(slice, new_pos, true); + new_range.horiz = Some(horiz); + new_range } else { - new_pos - }; - - let mut new_range = range.put(slice, new_head, true); - new_range.horiz = Some(horiz); - new_range + range + } } } } diff --git a/helix-core/src/search.rs b/helix-core/src/search.rs index d4eb11a9..243ac227 100644 --- a/helix-core/src/search.rs +++ b/helix-core/src/search.rs @@ -1,12 +1,6 @@ use crate::RopeSlice; -pub fn find_nth_next( - text: RopeSlice, - ch: char, - mut pos: usize, - n: usize, - inclusive: bool, -) -> Option { +pub fn find_nth_next(text: RopeSlice, ch: char, mut pos: usize, n: usize) -> Option { if pos >= text.len_chars() || n == 0 { return None; } @@ -25,20 +19,10 @@ 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 { +pub fn find_nth_prev(text: RopeSlice, ch: char, mut pos: usize, n: usize) -> Option { if pos == 0 || n == 0 { return None; } @@ -57,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 e40fceb1..cef68212 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -248,18 +248,18 @@ impl Range { } } - /// Moves the `Range` to `char_idx`. If `extend == true`, then only the head - /// is moved to `char_idx`, and the anchor is adjusted only as needed to - /// preserve 1-width range semantics. + /// Moves the head of the `Range` to `char_idx`, adjusting the anchor + /// as needed to preserve 1-width range semantics. + /// + /// `block_cursor` specifies whether it should treat `char_idx` as a block + /// cursor position or as a range-end position. /// /// This method assumes that the range and `char_idx` are already properly /// grapheme-aligned. #[must_use] #[inline] - pub fn put(self, text: RopeSlice, char_idx: usize, extend: bool) -> Range { - let anchor = if !extend { - char_idx - } else if self.head >= self.anchor && char_idx < self.anchor { + pub fn move_head(self, text: RopeSlice, char_idx: usize, block_cursor: bool) -> Range { + 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) @@ -267,7 +267,11 @@ impl Range { self.anchor }; - Range::new(anchor, char_idx) + if block_cursor && anchor <= char_idx { + Range::new(anchor, next_grapheme_boundary(text, char_idx)) + } else { + Range::new(anchor, char_idx) + } } // groupAt diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index af357c96..4d3ed5f5 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -49,19 +49,18 @@ pub fn find_nth_pairs_pos( 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, true)?)), - _ if (pos + 1) == text.len_chars() => Some(( - search::find_nth_prev(text, open, pos, n, true)?, - text.len_chars(), - )), + 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 { Some(( - search::find_nth_prev(text, open, pos, n, true)?, - search::find_nth_next(text, close, pos, n, true)?, + search::find_nth_prev(text, open, pos, n)?, + search::find_nth_next(text, close, pos, n)? + 1, )) } } else { diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 93867ee1..173d0b7b 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -373,15 +373,16 @@ fn goto_line_end(cx: &mut Context) { let selection = doc.selection(view.id).clone().transform(|range| { let line = range.head_line(text); + let line_start = text.line_to_char(line); - let mut pos = line_end_char_index(&text, line); - if doc.mode != Mode::Select { - pos = graphemes::prev_grapheme_boundary(text, pos); - } - - 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.put(text, pos, doc.mode == Mode::Select) + if doc.mode == Mode::Select { + range.move_head(text, pos, true) + } else { + Range::point(pos) + } }); doc.set_selection(view.id, selection); } @@ -392,12 +393,13 @@ fn goto_line_end_newline(cx: &mut Context) { let selection = doc.selection(view.id).clone().transform(|range| { let line = range.head_line(text); + let pos = line_end_char_index(&text, line); - let mut pos = text.line_to_char((line + 1).min(text.len_lines())); - if doc.mode != Mode::Select { - pos = graphemes::prev_grapheme_boundary(text, pos); + if doc.mode == Mode::Select { + range.move_head(text, pos, true) + } else { + Range::point(pos) } - range.put(text, pos, doc.mode == Mode::Select) }); doc.set_selection(view.id, selection); } @@ -411,7 +413,11 @@ fn goto_line_start(cx: &mut Context) { // adjust to start of the line let pos = text.line_to_char(line); - range.put(text, pos, doc.mode == Mode::Select) + if doc.mode == Mode::Select { + range.move_head(text, pos, true) + } else { + Range::point(pos) + } }); doc.set_selection(view.id, selection); } @@ -425,7 +431,11 @@ fn goto_first_nonwhitespace(cx: &mut Context) { if let Some(pos) = find_first_non_whitespace_char(text.line(line)) { let pos = pos + text.line_to_char(line); - range.put(text, pos, doc.mode == Mode::Select) + if doc.mode == Mode::Select { + range.move_head(text, pos, true) + } else { + Range::point(pos) + } } else { range } @@ -569,8 +579,8 @@ fn extend_next_word_start(cx: &mut Context) { .min_width_1(text) .transform(|range| { let word = movement::move_next_word_start(text, range, count); - let pos = word.head; - range.put(text, pos, true) + let pos = graphemes::prev_grapheme_boundary(text, word.head); + range.move_head(text, pos, true) }); doc.set_selection(view.id, selection); } @@ -587,7 +597,7 @@ fn extend_prev_word_start(cx: &mut Context) { .transform(|range| { let word = movement::move_prev_word_start(text, range, count); let pos = word.head; - range.put(text, pos, true) + range.move_head(text, pos, true) }); doc.set_selection(view.id, selection); } @@ -603,8 +613,8 @@ fn extend_next_word_end(cx: &mut Context) { .min_width_1(text) .transform(|range| { let word = movement::move_next_word_end(text, range, count); - let pos = word.head; - range.put(text, pos, true) + let pos = graphemes::prev_grapheme_boundary(text, word.head); + range.move_head(text, pos, true) }); doc.set_selection(view.id, selection); } @@ -652,11 +662,17 @@ where let text = doc.text().slice(..); let selection = doc.selection(view.id).clone().transform(|range| { + let range = if range.anchor < range.head { + // For 1-width cursor semantics. + Range::new(range.anchor, range.head - 1) + } else { + range + }; search_fn(text, ch, range.head, count, inclusive).map_or(range, |pos| { if extend { - range.put(text, pos, true) + range.move_head(text, pos, true) } else { - range.put(text, pos.saturating_sub(1), false) + Range::point(pos) } }) }); @@ -664,10 +680,39 @@ where }) } +fn find_next_char_impl( + text: RopeSlice, + ch: char, + pos: usize, + n: usize, + inclusive: bool, +) -> Option { + 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 { + 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 */ ) @@ -676,7 +721,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 */ ) @@ -685,7 +730,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 */ ) @@ -694,7 +739,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 */ ) @@ -703,7 +748,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 */ ) @@ -712,7 +757,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 */ ) @@ -721,7 +766,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 */ ) @@ -730,7 +775,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 */ ) -- cgit v1.2.3-70-g09d2