From fbb52f19e9073916db6e8a12e03bd78c82e9f9b7 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Sun, 16 Feb 2025 22:52:52 +0000 Subject: [PATCH 01/10] feat: implement basic functionality for finding the next / previous char pairs --- helix-core/src/search.rs | 60 +++++++++++++ helix-term/src/commands.rs | 146 +++++++++++++++++++++++++------ helix-term/src/keymap/default.rs | 3 + 3 files changed, 184 insertions(+), 25 deletions(-) diff --git a/helix-core/src/search.rs b/helix-core/src/search.rs index 81cb41293..fa1619765 100644 --- a/helix-core/src/search.rs +++ b/helix-core/src/search.rs @@ -44,6 +44,35 @@ pub fn find_nth_next( Some(pos - 1) } +pub fn find_nth_next_pair( + text: RopeSlice, + char_matcher: M, + char_matcher_2: M, + mut pos: usize, + n: usize, +) -> Option { + if pos >= text.len_chars() || n == 0 { + return None; + } + + let mut chars = text.chars_at(pos).peekable(); + + for _ in 0..n { + loop { + let c = chars.next()?; + let c2 = chars.peek()?; + + pos += 1; + + if char_matcher.char_match(c) && char_matcher_2.char_match(*c2) { + break; + } + } + } + + Some(pos - 1) +} + pub fn find_nth_prev(text: RopeSlice, ch: char, mut pos: usize, n: usize) -> Option { if pos == 0 || n == 0 { return None; @@ -65,3 +94,34 @@ pub fn find_nth_prev(text: RopeSlice, ch: char, mut pos: usize, n: usize) -> Opt Some(pos) } + + +pub fn find_nth_prev_pair( + text: RopeSlice, + char_matcher: M, + char_matcher_2: M, + mut pos: usize, + n: usize, +) -> Option { + if pos >= text.len_chars() || n == 0 { + return None; + } + + let mut chars = text.chars_at(pos).reversed().peekable(); + + for _ in 0..n { + loop { + let c = chars.next()?; + let c2 = chars.peek()?; + + pos -= 1; + + if char_matcher.char_match(c) && char_matcher_2.char_match(*c2) { + break; + } + } + } + + Some(pos - 1) +} + diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 19a22601e..05be02f2f 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -338,6 +338,8 @@ impl MappableCommand { extend_parent_node_start, "Extend to beginning of the parent node", find_till_char, "Move till next occurrence of char", find_next_char, "Move to next occurrence of char", + find_next_pair, "Move to next occurrence of 2 chars", + find_prev_pair, "Move to next occurrence of 2 chars", extend_till_char, "Extend till next occurrence of char", extend_next_char, "Extend to next occurrence of char", till_prev_char, "Move till previous occurrence of char", @@ -1510,11 +1512,73 @@ fn find_char_line_ending( doc.set_selection(view.id, selection); } -fn find_char(cx: &mut Context, direction: Direction, inclusive: bool, extend: bool) { +fn find_char(cx: &mut Context, direction: Direction, inclusive: bool, extend: bool, two_char: bool) { // TODO: count is reset to 1 before next key so we move it into the closure here. // Would be nice to carry over. let count = cx.count(); + if two_char { + + // need to wait for next key + // TODO: should this be done by grapheme rather than char? For example, + // we can't properly handle the line-ending CRLF case here in terms of char. + cx.on_next_key(move |cx, event| { + let ch = match event { + KeyEvent { + code: KeyCode::Enter, + .. + } => { + find_char_line_ending(cx, count, direction, inclusive, extend); + return; + } + + KeyEvent { + code: KeyCode::Tab, .. + } => '\t', + + KeyEvent { + code: KeyCode::Char(ch), + .. + } => ch, + _ => return, + }; + cx.on_next_key(move |cx, event| { + let ch2 = match event { + KeyEvent { + code: KeyCode::Enter, + .. + } => { + find_char_line_ending(cx, count, direction, inclusive, extend); + return; + } + + KeyEvent { + code: KeyCode::Tab, .. + } => '\t', + + KeyEvent { + code: KeyCode::Char(ch), + .. + } => ch, + _ => return, + }; + let motion = move |editor: &mut Editor| { + match direction { + Direction::Forward => { + find_char_impl(editor, &find_next_char_impl, inclusive, extend, ch, count, Some(ch2)) + } + Direction::Backward => { + find_char_impl(editor, &find_prev_char_impl, inclusive, extend, ch, count, Some(ch2)) + } + }; + }; + + cx.editor.apply_motion(motion); + }) + }) + + } else { + // need to wait for next key // TODO: should this be done by grapheme rather than char? For example, // we can't properly handle the line-ending CRLF case here in terms of char. @@ -1541,16 +1605,18 @@ fn find_char(cx: &mut Context, direction: Direction, inclusive: bool, extend: bo let motion = move |editor: &mut Editor| { match direction { Direction::Forward => { - find_char_impl(editor, &find_next_char_impl, inclusive, extend, ch, count) + find_char_impl(editor, &find_next_char_impl, inclusive, extend, ch, count, None) } Direction::Backward => { - find_char_impl(editor, &find_prev_char_impl, inclusive, extend, ch, count) + find_char_impl(editor, &find_prev_char_impl, inclusive, extend, ch, count, None) } }; }; cx.editor.apply_motion(motion); }) + + } } // @@ -1563,8 +1629,9 @@ fn find_char_impl( extend: bool, char_matcher: M, count: usize, + char_matcher_2: Option, ) where - F: Fn(RopeSlice, M, usize, usize, bool) -> Option + 'static, + F: Fn(RopeSlice, M, usize, usize, bool, Option) -> Option + 'static, { let (view, doc) = current!(editor); let text = doc.text().slice(..); @@ -1579,7 +1646,7 @@ fn find_char_impl( range.head }; - search_fn(text, char_matcher, search_start_pos, count, inclusive).map_or(range, |pos| { + search_fn(text, char_matcher, search_start_pos, count, inclusive, char_matcher_2).map_or(range, |pos| { if extend { range.put_cursor(text, pos, true) } else { @@ -1596,16 +1663,27 @@ fn find_next_char_impl( pos: usize, n: usize, inclusive: bool, + two_char: Option, ) -> Option { let pos = (pos + 1).min(text.len_chars()); - if inclusive { - search::find_nth_next(text, ch, pos, n) - } else { - let n = match text.get_char(pos) { - Some(next_ch) if next_ch == ch => n + 1, - _ => n, - }; - search::find_nth_next(text, ch, pos, n).map(|n| n.saturating_sub(1)) + match (inclusive, two_char) { + (true, Some(c)) => { + let n = match text.get_char(pos) { + Some(next_ch) if next_ch == ch => n + 1, + _ => n, + }; + search::find_nth_next_pair(text, ch, c, pos, n).map(|n| n.saturating_sub(1)) + }, + (false, Some(c)) => search::find_nth_next_pair(text, ch, c, pos, n), + (true, None) => search::find_nth_next(text, ch, pos, n), + (false, None) => { + let n = match text.get_char(pos) { + Some(next_ch) if next_ch == ch => n + 1, + _ => n, + + }; + search::find_nth_next(text, ch, pos, n).map(|n| n.saturating_sub(1)) + }, } } @@ -1615,48 +1693,66 @@ fn find_prev_char_impl( pos: usize, n: usize, inclusive: bool, + two_char: Option, ) -> Option { - if inclusive { - search::find_nth_prev(text, ch, pos, n) - } else { + match (inclusive, two_char) { + (true, Some(c)) => { let n = match text.get_char(pos.saturating_sub(1)) { Some(next_ch) if next_ch == ch => n + 1, _ => n, }; - search::find_nth_prev(text, ch, pos, n).map(|n| (n + 1).min(text.len_chars())) + search::find_nth_prev_pair(text, ch, c, pos, n).map(|n| n.saturating_sub(1)) + }, + (false, Some(c)) => search::find_nth_prev_pair(text, ch, c, pos, n), + (true, None) => search::find_nth_prev(text, ch, pos, n), + (false, None) => { + let n = match text.get_char(pos.saturating_sub(1)) { + Some(next_ch) if next_ch == ch => n + 1, + _ => n, + }; + search::find_nth_prev(text, ch, pos, n).map(|n| n.saturating_sub(1)) + }, } } +fn find_next_pair(cx: &mut Context) { + find_char(cx, Direction::Forward, true, false, true) +} + +fn find_prev_pair(cx: &mut Context) { + find_char(cx, Direction::Backward, true, false, true) +} + fn find_till_char(cx: &mut Context) { - find_char(cx, Direction::Forward, false, false); + find_char(cx, Direction::Forward, false, false, false); } fn find_next_char(cx: &mut Context) { - find_char(cx, Direction::Forward, true, false) + find_char(cx, Direction::Forward, true, false, false) } fn extend_till_char(cx: &mut Context) { - find_char(cx, Direction::Forward, false, true) + find_char(cx, Direction::Forward, false, true, false) } fn extend_next_char(cx: &mut Context) { - find_char(cx, Direction::Forward, true, true) + find_char(cx, Direction::Forward, true, true, false) } fn till_prev_char(cx: &mut Context) { - find_char(cx, Direction::Backward, false, false) + find_char(cx, Direction::Backward, false, false, false) } fn find_prev_char(cx: &mut Context) { - find_char(cx, Direction::Backward, true, false) + find_char(cx, Direction::Backward, true, false, false) } fn extend_till_prev_char(cx: &mut Context) { - find_char(cx, Direction::Backward, false, true) + find_char(cx, Direction::Backward, false, true, false) } fn extend_prev_char(cx: &mut Context) { - find_char(cx, Direction::Backward, true, true) + find_char(cx, Direction::Backward, true, true, false) } fn repeat_last_motion(cx: &mut Context) { diff --git a/helix-term/src/keymap/default.rs b/helix-term/src/keymap/default.rs index e160b2246..8071f688c 100644 --- a/helix-term/src/keymap/default.rs +++ b/helix-term/src/keymap/default.rs @@ -11,6 +11,9 @@ pub fn default() -> HashMap { "k" | "up" => move_visual_line_up, "l" | "right" => move_char_right, + "L" => find_next_pair, + "H" => find_prev_pair, + "t" => find_till_char, "f" => find_next_char, "T" => till_prev_char, From 2a7b165314d0b2bc049775b55ea8611273d97eba Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Mon, 17 Feb 2025 09:45:23 +0000 Subject: [PATCH 02/10] refactor: use if let else instead of match --- helix-term/src/commands.rs | 283 +++++++++++++++++++++---------------- 1 file changed, 159 insertions(+), 124 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 05be02f2f..6195d2cb9 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1512,110 +1512,136 @@ fn find_char_line_ending( doc.set_selection(view.id, selection); } -fn find_char(cx: &mut Context, direction: Direction, inclusive: bool, extend: bool, two_char: bool) { +fn find_char( + cx: &mut Context, + direction: Direction, + inclusive: bool, + extend: bool, + two_char: bool, +) { // TODO: count is reset to 1 before next key so we move it into the closure here. // Would be nice to carry over. let count = cx.count(); if two_char { - - // need to wait for next key - // TODO: should this be done by grapheme rather than char? For example, - // we can't properly handle the line-ending CRLF case here in terms of char. - cx.on_next_key(move |cx, event| { - let ch = match event { - KeyEvent { - code: KeyCode::Enter, - .. - } => { - find_char_line_ending(cx, count, direction, inclusive, extend); - return; - } - - KeyEvent { - code: KeyCode::Tab, .. - } => '\t', - - KeyEvent { - code: KeyCode::Char(ch), - .. - } => ch, - _ => return, - }; - cx.on_next_key(move |cx, event| { - let ch2 = match event { - KeyEvent { - code: KeyCode::Enter, - .. - } => { - find_char_line_ending(cx, count, direction, inclusive, extend); - return; - } - - KeyEvent { - code: KeyCode::Tab, .. - } => '\t', - - KeyEvent { - code: KeyCode::Char(ch), - .. - } => ch, - _ => return, - }; - let motion = move |editor: &mut Editor| { - match direction { - Direction::Forward => { - find_char_impl(editor, &find_next_char_impl, inclusive, extend, ch, count, Some(ch2)) - } - Direction::Backward => { - find_char_impl(editor, &find_prev_char_impl, inclusive, extend, ch, count, Some(ch2)) + // need to wait for next key + // TODO: should this be done by grapheme rather than char? For example, + // we can't properly handle the line-ending CRLF case here in terms of char. + cx.on_next_key(move |cx, event| { + let ch = match event { + KeyEvent { + code: KeyCode::Enter, + .. + } => { + find_char_line_ending(cx, count, direction, inclusive, extend); + return; } + + KeyEvent { + code: KeyCode::Tab, .. + } => '\t', + + KeyEvent { + code: KeyCode::Char(ch), + .. + } => ch, + _ => return, }; - }; + cx.on_next_key(move |cx, event| { + let ch2 = match event { + KeyEvent { + code: KeyCode::Enter, + .. + } => { + find_char_line_ending(cx, count, direction, inclusive, extend); + return; + } - cx.editor.apply_motion(motion); - }) - }) - + KeyEvent { + code: KeyCode::Tab, .. + } => '\t', + + KeyEvent { + code: KeyCode::Char(ch), + .. + } => ch, + _ => return, + }; + let motion = move |editor: &mut Editor| { + match direction { + Direction::Forward => find_char_impl( + editor, + &find_next_char_impl, + inclusive, + extend, + ch, + count, + Some(ch2), + ), + Direction::Backward => find_char_impl( + editor, + &find_prev_char_impl, + inclusive, + extend, + ch, + count, + Some(ch2), + ), + }; + }; + + cx.editor.apply_motion(motion); + }) + }) } else { - - // need to wait for next key - // TODO: should this be done by grapheme rather than char? For example, - // we can't properly handle the line-ending CRLF case here in terms of char. - cx.on_next_key(move |cx, event| { - let ch = match event { - KeyEvent { - code: KeyCode::Enter, - .. - } => { - find_char_line_ending(cx, count, direction, inclusive, extend); - return; - } - - KeyEvent { - code: KeyCode::Tab, .. - } => '\t', - - KeyEvent { - code: KeyCode::Char(ch), - .. - } => ch, - _ => return, - }; - let motion = move |editor: &mut Editor| { - match direction { - Direction::Forward => { - find_char_impl(editor, &find_next_char_impl, inclusive, extend, ch, count, None) - } - Direction::Backward => { - find_char_impl(editor, &find_prev_char_impl, inclusive, extend, ch, count, None) + // need to wait for next key + // TODO: should this be done by grapheme rather than char? For example, + // we can't properly handle the line-ending CRLF case here in terms of char. + cx.on_next_key(move |cx, event| { + let ch = match event { + KeyEvent { + code: KeyCode::Enter, + .. + } => { + find_char_line_ending(cx, count, direction, inclusive, extend); + return; } + + KeyEvent { + code: KeyCode::Tab, .. + } => '\t', + + KeyEvent { + code: KeyCode::Char(ch), + .. + } => ch, + _ => return, + }; + let motion = move |editor: &mut Editor| { + match direction { + Direction::Forward => find_char_impl( + editor, + &find_next_char_impl, + inclusive, + extend, + ch, + count, + None, + ), + Direction::Backward => find_char_impl( + editor, + &find_prev_char_impl, + inclusive, + extend, + ch, + count, + None, + ), + }; }; - }; - cx.editor.apply_motion(motion); - }) - + cx.editor.apply_motion(motion); + }) } } @@ -1646,7 +1672,15 @@ fn find_char_impl( range.head }; - search_fn(text, char_matcher, search_start_pos, count, inclusive, char_matcher_2).map_or(range, |pos| { + search_fn( + text, + char_matcher, + search_start_pos, + count, + inclusive, + char_matcher_2, + ) + .map_or(range, |pos| { if extend { range.put_cursor(text, pos, true) } else { @@ -1666,24 +1700,24 @@ fn find_next_char_impl( two_char: Option, ) -> Option { let pos = (pos + 1).min(text.len_chars()); - match (inclusive, two_char) { - (true, Some(c)) => { - let n = match text.get_char(pos) { - Some(next_ch) if next_ch == ch => n + 1, - _ => n, - }; - search::find_nth_next_pair(text, ch, c, pos, n).map(|n| n.saturating_sub(1)) - }, - (false, Some(c)) => search::find_nth_next_pair(text, ch, c, pos, n), - (true, None) => search::find_nth_next(text, ch, pos, n), - (false, None) => { - let n = match text.get_char(pos) { - Some(next_ch) if next_ch == ch => n + 1, - _ => n, - - }; - search::find_nth_next(text, ch, pos, n).map(|n| n.saturating_sub(1)) - }, + if let Some(c) = two_char { + if inclusive { + let n = match text.get_char(pos) { + Some(next_ch) if next_ch == ch => n + 1, + _ => n, + }; + search::find_nth_next_pair(text, ch, c, pos, n).map(|n| n.saturating_sub(1)) + } else { + search::find_nth_next_pair(text, ch, c, pos, n) + } + } else if inclusive { + search::find_nth_next(text, ch, pos, n) + } else { + let n = match text.get_char(pos) { + Some(next_ch) if next_ch == ch => n + 1, + _ => n, + }; + search::find_nth_next(text, ch, pos, n).map(|n| n.saturating_sub(1)) } } @@ -1695,23 +1729,24 @@ fn find_prev_char_impl( inclusive: bool, two_char: Option, ) -> Option { - match (inclusive, two_char) { - (true, Some(c)) => { + if let Some(c) = two_char { + if inclusive { + let n = match text.get_char(pos.saturating_sub(1)) { + Some(next_ch) if next_ch == ch => n + 1, + _ => n, + }; + search::find_nth_prev_pair(text, ch, c, pos, n).map(|n| n.saturating_sub(1)) + } else { + search::find_nth_prev_pair(text, ch, c, pos, n) + } + } else if inclusive { + search::find_nth_prev(text, ch, pos, n) + } else { let n = match text.get_char(pos.saturating_sub(1)) { Some(next_ch) if next_ch == ch => n + 1, _ => n, }; - search::find_nth_prev_pair(text, ch, c, pos, n).map(|n| n.saturating_sub(1)) - }, - (false, Some(c)) => search::find_nth_prev_pair(text, ch, c, pos, n), - (true, None) => search::find_nth_prev(text, ch, pos, n), - (false, None) => { - let n = match text.get_char(pos.saturating_sub(1)) { - Some(next_ch) if next_ch == ch => n + 1, - _ => n, - }; - search::find_nth_prev(text, ch, pos, n).map(|n| n.saturating_sub(1)) - }, + search::find_nth_prev(text, ch, pos, n).map(|n| n.saturating_sub(1)) } } From 69badf4629186a9ba298b04a84f623c570221d88 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Mon, 17 Feb 2025 10:02:54 +0000 Subject: [PATCH 03/10] refactor: `find_char` --- helix-term/src/commands.rs | 142 ++++++++++++------------------------- 1 file changed, 44 insertions(+), 98 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 6195d2cb9..d84fb208d 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1523,37 +1523,41 @@ fn find_char( // Would be nice to carry over. let count = cx.count(); - if two_char { - // need to wait for next key - // TODO: should this be done by grapheme rather than char? For example, - // we can't properly handle the line-ending CRLF case here in terms of char. - cx.on_next_key(move |cx, event| { - let ch = match event { - KeyEvent { - code: KeyCode::Enter, - .. - } => { - find_char_line_ending(cx, count, direction, inclusive, extend); - return; - } - - KeyEvent { - code: KeyCode::Tab, .. - } => '\t', - - KeyEvent { - code: KeyCode::Char(ch), - .. - } => ch, - _ => return, + // need to wait for next key + // TODO: should this be done by grapheme rather than char? For example, + // we can't properly handle the line-ending CRLF case here in terms of char. + cx.on_next_key(move |cx, event| { + let mkmotion = move |editor: &mut Editor, char_1: char, char_2: Option| { + match direction { + Direction::Forward => find_char_impl( + editor, + &find_next_char_impl, + inclusive, + extend, + char_1, + count, + char_2, + ), + Direction::Backward => find_char_impl( + editor, + &find_prev_char_impl, + inclusive, + extend, + char_1, + count, + char_2, + ), }; - cx.on_next_key(move |cx, event| { - let ch2 = match event { + }; + + macro_rules! resolve_char { + ($event:ident, $cx:ident) => { + match $event { KeyEvent { code: KeyCode::Enter, .. } => { - find_char_line_ending(cx, count, direction, inclusive, extend); + find_char_line_ending($cx, count, direction, inclusive, extend); return; } @@ -1566,83 +1570,25 @@ fn find_char( .. } => ch, _ => return, - }; - let motion = move |editor: &mut Editor| { - match direction { - Direction::Forward => find_char_impl( - editor, - &find_next_char_impl, - inclusive, - extend, - ch, - count, - Some(ch2), - ), - Direction::Backward => find_char_impl( - editor, - &find_prev_char_impl, - inclusive, - extend, - ch, - count, - Some(ch2), - ), - }; - }; + } + }; + } + + let ch = resolve_char!(event, cx); + + if two_char { + cx.on_next_key(move |cx, event| { + let ch2 = resolve_char!(event, cx); + let motion = move |editor: &mut Editor| mkmotion(editor, ch, Some(ch2)); cx.editor.apply_motion(motion); }) - }) - } else { - // need to wait for next key - // TODO: should this be done by grapheme rather than char? For example, - // we can't properly handle the line-ending CRLF case here in terms of char. - cx.on_next_key(move |cx, event| { - let ch = match event { - KeyEvent { - code: KeyCode::Enter, - .. - } => { - find_char_line_ending(cx, count, direction, inclusive, extend); - return; - } - - KeyEvent { - code: KeyCode::Tab, .. - } => '\t', - - KeyEvent { - code: KeyCode::Char(ch), - .. - } => ch, - _ => return, - }; - let motion = move |editor: &mut Editor| { - match direction { - Direction::Forward => find_char_impl( - editor, - &find_next_char_impl, - inclusive, - extend, - ch, - count, - None, - ), - Direction::Backward => find_char_impl( - editor, - &find_prev_char_impl, - inclusive, - extend, - ch, - count, - None, - ), - }; - }; + } else { + let motion = move |editor: &mut Editor| mkmotion(editor, ch, None); cx.editor.apply_motion(motion); - }) - } + } + }) } // From fd7a5623af2d36779d92b3de054ad422c377de0a Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Mon, 17 Feb 2025 10:38:27 +0000 Subject: [PATCH 04/10] feat: correct implementation of {find,extend}_next_pair --- helix-term/src/commands.rs | 271 ++++++++++++++++++------------- helix-term/src/keymap/default.rs | 5 +- 2 files changed, 164 insertions(+), 112 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index d84fb208d..3b15c0c96 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -340,6 +340,8 @@ impl MappableCommand { find_next_char, "Move to next occurrence of char", find_next_pair, "Move to next occurrence of 2 chars", find_prev_pair, "Move to next occurrence of 2 chars", + extend_next_pair, "Extend to next occurrence of 2 chars", + extend_prev_pair, "Extend to next occurrence of 2 chars", extend_till_char, "Extend till next occurrence of char", extend_next_char, "Extend to next occurrence of char", till_prev_char, "Move till previous occurrence of char", @@ -1512,13 +1514,7 @@ fn find_char_line_ending( doc.set_selection(view.id, selection); } -fn find_char( - cx: &mut Context, - direction: Direction, - inclusive: bool, - extend: bool, - two_char: bool, -) { +fn find_char(cx: &mut Context, direction: Direction, inclusive: bool, extend: bool) { // TODO: count is reset to 1 before next key so we move it into the closure here. // Would be nice to carry over. let count = cx.count(); @@ -1527,71 +1523,147 @@ fn find_char( // TODO: should this be done by grapheme rather than char? For example, // we can't properly handle the line-ending CRLF case here in terms of char. cx.on_next_key(move |cx, event| { - let mkmotion = move |editor: &mut Editor, char_1: char, char_2: Option| { + let ch = match event { + KeyEvent { + code: KeyCode::Enter, + .. + } => { + find_char_line_ending(cx, count, direction, inclusive, extend); + return; + } + + KeyEvent { + code: KeyCode::Tab, .. + } => '\t', + + KeyEvent { + code: KeyCode::Char(ch), + .. + } => ch, + _ => return, + }; + let motion = move |editor: &mut Editor| { match direction { - Direction::Forward => find_char_impl( - editor, - &find_next_char_impl, - inclusive, - extend, - char_1, - count, - char_2, - ), - Direction::Backward => find_char_impl( - editor, - &find_prev_char_impl, - inclusive, - extend, - char_1, - count, - char_2, - ), + Direction::Forward => { + find_char_impl(editor, &find_next_char_impl, inclusive, extend, ch, count) + } + Direction::Backward => { + find_char_impl(editor, &find_prev_char_impl, inclusive, extend, ch, count) + } }; }; - macro_rules! resolve_char { - ($event:ident, $cx:ident) => { - match $event { - KeyEvent { - code: KeyCode::Enter, - .. - } => { - find_char_line_ending($cx, count, direction, inclusive, extend); - return; - } - - KeyEvent { - code: KeyCode::Tab, .. - } => '\t', - - KeyEvent { - code: KeyCode::Char(ch), - .. - } => ch, - _ => return, - } - }; - } - - let ch = resolve_char!(event, cx); - - if two_char { - cx.on_next_key(move |cx, event| { - let ch2 = resolve_char!(event, cx); - let motion = move |editor: &mut Editor| mkmotion(editor, ch, Some(ch2)); - - cx.editor.apply_motion(motion); - }) - } else { - let motion = move |editor: &mut Editor| mkmotion(editor, ch, None); - - cx.editor.apply_motion(motion); - } + cx.editor.apply_motion(motion); }) } -// +fn find_char_pair(cx: &mut Context, direction: Direction, extend: bool) { + // TODO: count is reset to 1 before next key so we move it into the closure here. + // Would be nice to carry over. + let count = cx.count(); + + // need to wait for next key + // TODO: should this be done by grapheme rather than char? For example, + // we can't properly handle the line-ending CRLF case here in terms of char. + cx.on_next_key(move |cx, event| { + let ch = match event { + KeyEvent { + code: KeyCode::Enter, + .. + } => { + find_char_line_ending(cx, count, direction, true, false); + return; + } + + KeyEvent { + code: KeyCode::Tab, .. + } => '\t', + + KeyEvent { + code: KeyCode::Char(ch), + .. + } => ch, + _ => return, + }; + + cx.on_next_key(move |cx, event| { + let ch2 = match event { + KeyEvent { + code: KeyCode::Enter, + .. + } => { + find_char_line_ending(cx, count, direction, true, false); + return; + } + + KeyEvent { + code: KeyCode::Tab, .. + } => '\t', + + KeyEvent { + code: KeyCode::Char(ch), + .. + } => ch, + _ => return, + }; + let motion = move |editor: &mut Editor| { + match direction { + Direction::Forward => { + let (view, doc) = current!(editor); + let text = doc.text().slice(..); + 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::find_nth_next_pair(text, ch, ch2, search_start_pos, count) + .map_or(range, |pos| { + if extend { + Range::new(range.from(), pos + 2) + } else { + Range::new(pos, pos + 2) + } + }) + }); + doc.set_selection(view.id, selection); + } + Direction::Backward => { + let (view, doc) = current!(editor); + let text = doc.text().slice(..); + 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::find_nth_prev_pair(text, ch, ch2, search_start_pos, count) + .map_or(range, |pos| { + if extend { + Range::new(range.from(), pos + 1) + .with_direction(range.direction()) + } else { + Range::new(pos, pos + 1) + } + }) + }); + doc.set_selection(view.id, selection); + } + }; + }; + + cx.editor.apply_motion(motion); + }) + }) +} #[inline] fn find_char_impl( @@ -1601,9 +1673,8 @@ fn find_char_impl( extend: bool, char_matcher: M, count: usize, - char_matcher_2: Option, ) where - F: Fn(RopeSlice, M, usize, usize, bool, Option) -> Option + 'static, + F: Fn(RopeSlice, M, usize, usize, bool) -> Option + 'static, { let (view, doc) = current!(editor); let text = doc.text().slice(..); @@ -1618,15 +1689,7 @@ fn find_char_impl( range.head }; - search_fn( - text, - char_matcher, - search_start_pos, - count, - inclusive, - char_matcher_2, - ) - .map_or(range, |pos| { + search_fn(text, char_matcher, search_start_pos, count, inclusive).map_or(range, |pos| { if extend { range.put_cursor(text, pos, true) } else { @@ -1643,20 +1706,9 @@ fn find_next_char_impl( pos: usize, n: usize, inclusive: bool, - two_char: Option, ) -> Option { let pos = (pos + 1).min(text.len_chars()); - if let Some(c) = two_char { - if inclusive { - let n = match text.get_char(pos) { - Some(next_ch) if next_ch == ch => n + 1, - _ => n, - }; - search::find_nth_next_pair(text, ch, c, pos, n).map(|n| n.saturating_sub(1)) - } else { - search::find_nth_next_pair(text, ch, c, pos, n) - } - } else if inclusive { + if inclusive { search::find_nth_next(text, ch, pos, n) } else { let n = match text.get_char(pos) { @@ -1673,67 +1725,64 @@ fn find_prev_char_impl( pos: usize, n: usize, inclusive: bool, - two_char: Option, ) -> Option { - if let Some(c) = two_char { - if inclusive { - let n = match text.get_char(pos.saturating_sub(1)) { - Some(next_ch) if next_ch == ch => n + 1, - _ => n, - }; - search::find_nth_prev_pair(text, ch, c, pos, n).map(|n| n.saturating_sub(1)) - } else { - search::find_nth_prev_pair(text, ch, c, pos, n) - } - } else if inclusive { + if inclusive { search::find_nth_prev(text, ch, pos, n) } else { let n = match text.get_char(pos.saturating_sub(1)) { Some(next_ch) if next_ch == ch => n + 1, _ => n, }; - search::find_nth_prev(text, ch, pos, n).map(|n| n.saturating_sub(1)) + search::find_nth_prev(text, ch, pos, n).map(|n| (n + 1).min(text.len_chars())) } } fn find_next_pair(cx: &mut Context) { - find_char(cx, Direction::Forward, true, false, true) + find_char_pair(cx, Direction::Forward, false) } fn find_prev_pair(cx: &mut Context) { - find_char(cx, Direction::Backward, true, false, true) + find_char_pair(cx, Direction::Backward, false) +} + +fn extend_next_pair(cx: &mut Context) { + find_char_pair(cx, Direction::Forward, true) +} + +fn extend_prev_pair(cx: &mut Context) { + find_char_pair(cx, Direction::Backward, true) } fn find_till_char(cx: &mut Context) { - find_char(cx, Direction::Forward, false, false, false); + find_char(cx, Direction::Forward, false, false); } fn find_next_char(cx: &mut Context) { - find_char(cx, Direction::Forward, true, false, false) + find_char(cx, Direction::Forward, true, false) } fn extend_till_char(cx: &mut Context) { - find_char(cx, Direction::Forward, false, true, false) + find_char(cx, Direction::Forward, false, true) } fn extend_next_char(cx: &mut Context) { - find_char(cx, Direction::Forward, true, true, false) + find_char(cx, Direction::Forward, true, true) } fn till_prev_char(cx: &mut Context) { - find_char(cx, Direction::Backward, false, false, false) + find_char(cx, Direction::Backward, false, false) } fn find_prev_char(cx: &mut Context) { - find_char(cx, Direction::Backward, true, false, false) + find_char(cx, Direction::Backward, true, false) } fn extend_till_prev_char(cx: &mut Context) { - find_char(cx, Direction::Backward, false, true, false) + find_char(cx, Direction::Backward, false, true) } fn extend_prev_char(cx: &mut Context) { - find_char(cx, Direction::Backward, true, true, false) + find_char(cx, Direction::Backward, true, true) } fn repeat_last_motion(cx: &mut Context) { diff --git a/helix-term/src/keymap/default.rs b/helix-term/src/keymap/default.rs index 8071f688c..fe64eea21 100644 --- a/helix-term/src/keymap/default.rs +++ b/helix-term/src/keymap/default.rs @@ -13,7 +13,7 @@ pub fn default() -> HashMap { "L" => find_next_pair, "H" => find_prev_pair, - + "t" => find_till_char, "f" => find_next_char, "T" => till_prev_char, @@ -346,6 +346,9 @@ pub fn default() -> HashMap { "k" | "up" => extend_visual_line_up, "l" | "right" => extend_char_right, + "L" => extend_next_pair, + "H" => extend_prev_pair, + "w" => extend_next_word_start, "b" => extend_prev_word_start, "e" => extend_next_word_end, From 74cf0b5f5cedb184e48ea06e3c3be40fa911282f Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Mon, 17 Feb 2025 10:46:02 +0000 Subject: [PATCH 05/10] feat: correct implementation of `{find,extend}_{prev,next}_pair` --- helix-core/src/search.rs | 13 ++++++------- helix-term/src/commands.rs | 5 ++--- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/helix-core/src/search.rs b/helix-core/src/search.rs index fa1619765..5be05955f 100644 --- a/helix-core/src/search.rs +++ b/helix-core/src/search.rs @@ -95,11 +95,10 @@ pub fn find_nth_prev(text: RopeSlice, ch: char, mut pos: usize, n: usize) -> Opt Some(pos) } - pub fn find_nth_prev_pair( text: RopeSlice, - char_matcher: M, - char_matcher_2: M, + char_matcher_left: M, + char_matcher_right: M, mut pos: usize, n: usize, ) -> Option { @@ -111,17 +110,17 @@ pub fn find_nth_prev_pair( for _ in 0..n { loop { - let c = chars.next()?; - let c2 = chars.peek()?; + let c_right = chars.next()?; + let c_left = chars.peek()?; pos -= 1; - if char_matcher.char_match(c) && char_matcher_2.char_match(*c2) { + if char_matcher_left.char_match(*c_left) && char_matcher_right.char_match(c_right) { break; } } } + log::error!("{pos}"); Some(pos - 1) } - diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 3b15c0c96..b4fba543a 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1648,10 +1648,9 @@ fn find_char_pair(cx: &mut Context, direction: Direction, extend: bool) { search::find_nth_prev_pair(text, ch, ch2, search_start_pos, count) .map_or(range, |pos| { if extend { - Range::new(range.from(), pos + 1) - .with_direction(range.direction()) + Range::new(pos + 2, range.to()) } else { - Range::new(pos, pos + 1) + Range::new(pos + 2, pos) } }) }); From f8561763e3b5739881325baba14ef602a9509ba7 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Mon, 17 Feb 2025 10:50:53 +0000 Subject: [PATCH 06/10] docs: add info for {extend,find}_{next,prev}_pair --- book/src/generated/static-cmd.md | 4 ++ helix-core/src/search.rs | 65 ++++++++++++++++---------------- 2 files changed, 36 insertions(+), 33 deletions(-) diff --git a/book/src/generated/static-cmd.md b/book/src/generated/static-cmd.md index af7515b8e..532c032cb 100644 --- a/book/src/generated/static-cmd.md +++ b/book/src/generated/static-cmd.md @@ -45,6 +45,10 @@ | `extend_parent_node_start` | Extend to beginning of the parent node | select: `` `` | | `find_till_char` | Move till next occurrence of char | normal: `` t `` | | `find_next_char` | Move to next occurrence of char | normal: `` f `` | +| `find_next_pair` | Move to next occurrence of 2 chars | normal: `` L `` | +| `find_prev_pair` | Move to next occurrence of 2 chars | normal: `` H `` | +| `extend_next_pair` | Extend to next occurrence of 2 chars | select: `` L `` | +| `extend_prev_pair` | Extend to next occurrence of 2 chars | select: `` H `` | | `extend_till_char` | Extend till next occurrence of char | select: `` t `` | | `extend_next_char` | Extend to next occurrence of char | select: `` f `` | | `till_prev_char` | Move till previous occurrence of char | normal: `` T `` | diff --git a/helix-core/src/search.rs b/helix-core/src/search.rs index 5be05955f..0bdeb6274 100644 --- a/helix-core/src/search.rs +++ b/helix-core/src/search.rs @@ -44,35 +44,6 @@ pub fn find_nth_next( Some(pos - 1) } -pub fn find_nth_next_pair( - text: RopeSlice, - char_matcher: M, - char_matcher_2: M, - mut pos: usize, - n: usize, -) -> Option { - if pos >= text.len_chars() || n == 0 { - return None; - } - - let mut chars = text.chars_at(pos).peekable(); - - for _ in 0..n { - loop { - let c = chars.next()?; - let c2 = chars.peek()?; - - pos += 1; - - if char_matcher.char_match(c) && char_matcher_2.char_match(*c2) { - break; - } - } - } - - Some(pos - 1) -} - pub fn find_nth_prev(text: RopeSlice, ch: char, mut pos: usize, n: usize) -> Option { if pos == 0 || n == 0 { return None; @@ -95,6 +66,35 @@ pub fn find_nth_prev(text: RopeSlice, ch: char, mut pos: usize, n: usize) -> Opt Some(pos) } +pub fn find_nth_next_pair( + text: RopeSlice, + char_matcher_left: M, + char_matcher_right: M, + mut pos: usize, + n: usize, +) -> Option { + if pos >= text.len_chars() || n == 0 { + return None; + } + + let mut chars = text.chars_at(pos).peekable(); + + for _ in 0..n { + loop { + let ch_left = chars.next()?; + let ch_right = chars.peek()?; + + pos += 1; + + if char_matcher_left.char_match(ch_left) && char_matcher_right.char_match(*ch_right) { + break; + } + } + } + + Some(pos - 1) +} + pub fn find_nth_prev_pair( text: RopeSlice, char_matcher_left: M, @@ -110,17 +110,16 @@ pub fn find_nth_prev_pair( for _ in 0..n { loop { - let c_right = chars.next()?; - let c_left = chars.peek()?; + let ch_right = chars.next()?; + let ch_left = chars.peek()?; pos -= 1; - if char_matcher_left.char_match(*c_left) && char_matcher_right.char_match(c_right) { + if char_matcher_left.char_match(*ch_left) && char_matcher_right.char_match(ch_right) { break; } } } - log::error!("{pos}"); Some(pos - 1) } From 453430e49ba147e6f29fc189c7f8d0d4b5746852 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Mon, 17 Feb 2025 11:28:54 +0000 Subject: [PATCH 07/10] refactor: extract common logic --- helix-core/src/selection.rs | 12 +++++ helix-term/src/commands.rs | 88 ++++++++++++------------------------- 2 files changed, 40 insertions(+), 60 deletions(-) diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index 1db2d619e..8d16c7095 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -329,6 +329,18 @@ impl Range { //-------------------------------- // Block-cursor methods. + /// Gets the left-side position of the block cursor. + /// Not grapheme-aware. For the grapheme-aware version, use [`Range::cursor`] + #[must_use] + #[inline] + pub fn char_cursor(self) -> usize { + if self.head > self.anchor { + self.head - 1 + } else { + self.head + } + } + /// Gets the left-side position of the block cursor. #[must_use] #[inline] diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index b4fba543a..e653ca782 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1587,7 +1587,7 @@ fn find_char_pair(cx: &mut Context, direction: Direction, extend: bool) { }; cx.on_next_key(move |cx, event| { - let ch2 = match event { + let ch_2 = match event { KeyEvent { code: KeyCode::Enter, .. @@ -1607,56 +1607,32 @@ fn find_char_pair(cx: &mut Context, direction: Direction, extend: bool) { _ => return, }; let motion = move |editor: &mut Editor| { - match direction { - Direction::Forward => { - let (view, doc) = current!(editor); - let text = doc.text().slice(..); - 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::find_nth_next_pair(text, ch, ch2, search_start_pos, count) - .map_or(range, |pos| { - if extend { - Range::new(range.from(), pos + 2) - } else { - Range::new(pos, pos + 2) - } - }) - }); - doc.set_selection(view.id, selection); - } - Direction::Backward => { - let (view, doc) = current!(editor); - let text = doc.text().slice(..); - 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::find_nth_prev_pair(text, ch, ch2, search_start_pos, count) - .map_or(range, |pos| { - if extend { - Range::new(pos + 2, range.to()) - } else { - Range::new(pos + 2, pos) - } - }) - }); - doc.set_selection(view.id, selection); - } + let (view, doc) = current!(editor); + let text = doc.text().slice(..); + let selection = doc.selection(view.id).clone(); + let selection = match direction { + Direction::Forward => selection.transform(|range| { + search::find_nth_next_pair(text, ch, ch_2, range.char_cursor(), count) + .map_or(range, |pos| { + if extend { + Range::new(range.from(), pos + 2) + } else { + Range::new(pos, pos + 2) + } + }) + }), + Direction::Backward => selection.transform(|range| { + search::find_nth_prev_pair(text, ch, ch_2, range.char_cursor(), count) + .map_or(range, |pos| { + if extend { + Range::new(pos + 2, range.to()) + } else { + Range::new(pos + 2, pos) + } + }) + }), }; + doc.set_selection(view.id, selection); }; cx.editor.apply_motion(motion); @@ -1675,20 +1651,12 @@ fn find_char_impl( ) where F: Fn(RopeSlice, M, usize, usize, bool) -> Option + 'static, { + // TODO: make this grapheme-aware let (view, doc) = current!(editor); let text = doc.text().slice(..); 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, char_matcher, search_start_pos, count, inclusive).map_or(range, |pos| { + search_fn(text, char_matcher, range.char_cursor(), count, inclusive).map_or(range, |pos| { if extend { range.put_cursor(text, pos, true) } else { From 21282a9361a41eee3b67aace48f205aea77e10a4 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Mon, 17 Feb 2025 15:17:05 +0000 Subject: [PATCH 08/10] feat: consider line endings when finding the nth pair --- helix-core/src/search.rs | 206 +++++++++++++++++++++++++++++-------- helix-term/src/commands.rs | 23 ++--- 2 files changed, 171 insertions(+), 58 deletions(-) diff --git a/helix-core/src/search.rs b/helix-core/src/search.rs index 0bdeb6274..ddab8aed9 100644 --- a/helix-core/src/search.rs +++ b/helix-core/src/search.rs @@ -1,4 +1,4 @@ -use crate::RopeSlice; +use crate::{line_ending::line_end_char_index, movement::Direction, RopeSlice}; // TODO: switch to std::str::Pattern when it is stable. pub trait CharMatcher { @@ -66,60 +66,178 @@ pub fn find_nth_prev(text: RopeSlice, ch: char, mut pos: usize, n: usize) -> Opt Some(pos) } -pub fn find_nth_next_pair( +#[derive(Copy, Clone)] +pub enum PairMatcher<'a> { + Char(char), + LineEnding(&'a str), +} + +pub fn find_nth_pair( text: RopeSlice, - char_matcher_left: M, - char_matcher_right: M, - mut pos: usize, + pair_matcher_left: PairMatcher, + pair_matcher_right: PairMatcher, + pos: usize, n: usize, + direction: Direction, ) -> Option { if pos >= text.len_chars() || n == 0 { return None; } - let mut chars = text.chars_at(pos).peekable(); + let is_forward = direction == Direction::Forward; + let direction_multiplier = if is_forward { 1 } else { -1 }; - for _ in 0..n { - loop { - let ch_left = chars.next()?; - let ch_right = chars.peek()?; + match (pair_matcher_left, pair_matcher_right) { + (PairMatcher::Char(ch_left), PairMatcher::Char(ch_right)) => { + let chars = text.chars_at(pos); - pos += 1; + let mut chars = if is_forward { + chars.peekable() + } else { + chars.reversed().peekable() + }; - if char_matcher_left.char_match(ch_left) && char_matcher_right.char_match(*ch_right) { - break; + let mut offset = 0; + + for _ in 0..n { + loop { + let ch_next = chars.next()?; + let ch_peek = chars.peek()?; + + offset += 1; + + let matches_char = if is_forward { + ch_left == ch_next && ch_right == *ch_peek + } else { + ch_right == ch_next && ch_left == *ch_peek + }; + + if matches_char { + break; + } + } } + + let offs = offset * direction_multiplier; + let new_pos: usize = (pos as isize + offs) + .try_into() + .expect("Character offset cannot exceed character count"); + + Some(new_pos - 1) + } + (PairMatcher::Char(ch_left), PairMatcher::LineEnding(eol)) => { + let start_line = text.char_to_line(pos); + let start_line = if pos >= line_end_char_index(&text, start_line) { + // if our cursor is currently on a character just before the eol, or on the eol + // we start searching from the next line, instead of from the current line. + start_line + eol.len() + } else { + start_line + }; + + let mut lines = if is_forward { + text.lines_at(start_line).enumerate() + } else { + text.lines_at(start_line).reversed().enumerate() + }; + + if !is_forward { + // skip the line we are currently on when going backward + lines.next(); + } + + let mut matched_count = 0; + for (traversed_lines, _line) in lines { + let current_line = (start_line as isize + + (traversed_lines as isize * direction_multiplier)) + as usize; + + let ch_opposite_eol_i = if is_forward { + line_end_char_index(&text, current_line).saturating_sub(eol.len()) + } else { + text.line_to_char(current_line) + }; + + let ch_opposite_eol = text.char(ch_opposite_eol_i); + + if ch_opposite_eol == ch_left { + matched_count += 1; + if matched_count == n { + return Some(ch_opposite_eol_i - if is_forward { 0 } else { 1 }); + } + } + } + + None + } + (PairMatcher::LineEnding(eol), PairMatcher::Char(ch_right)) => { + // Search starting from the beginning of the next or previous line + let start_line = text.char_to_line(pos) + (is_forward as usize); + + let lines = if is_forward { + text.lines_at(start_line).enumerate() + } else { + text.lines_at(start_line).reversed().enumerate() + }; + + let mut matched_count = 0; + for (traversed_lines, _line) in lines { + let current_line = (start_line as isize + + (traversed_lines as isize * direction_multiplier)) + as usize; + + let ch_opposite_eol_i = if is_forward { + // eol, THEN character at the beginning of the current line + text.line_to_char(current_line) + } else { + // character at the end of the previous line, THEN eol + line_end_char_index(&text, current_line - 1) - eol.len() + }; + + let ch_opposite_eol = text.get_char(ch_opposite_eol_i)?; + + if ch_opposite_eol == ch_right { + matched_count += 1; + if matched_count == n { + return Some(ch_opposite_eol_i - (is_forward as usize)); + } + } + } + + None + } + (PairMatcher::LineEnding(eol), PairMatcher::LineEnding(_)) => { + // Search starting from the beginning of the + // line after the current one + let start_line = text.char_to_line(pos) + 1; + + let mut lines = if is_forward { + text.lines_at(start_line).enumerate() + } else { + text.lines_at(start_line).reversed().enumerate() + }; + + if !is_forward { + // skip the line we are currently on when going backward + lines.next(); + } + + let mut matched_count = 0; + for (traversed_lines, _line) in lines { + let current_line = (start_line as isize + + (traversed_lines as isize * direction_multiplier)) + as usize; + let current_line = text.line_to_char(current_line); + let current_line_end = current_line + eol.len(); + if text.slice(current_line..current_line_end).as_str()? == eol { + matched_count += 1; + if matched_count == n { + return Some(current_line - eol.len()); + } + } + } + + None } } - - Some(pos - 1) -} - -pub fn find_nth_prev_pair( - text: RopeSlice, - char_matcher_left: M, - char_matcher_right: M, - mut pos: usize, - n: usize, -) -> Option { - if pos >= text.len_chars() || n == 0 { - return None; - } - - let mut chars = text.chars_at(pos).reversed().peekable(); - - for _ in 0..n { - loop { - let ch_right = chars.next()?; - let ch_left = chars.peek()?; - - pos -= 1; - - if char_matcher_left.char_match(*ch_left) && char_matcher_right.char_match(ch_right) { - break; - } - } - } - - Some(pos - 1) } diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index e653ca782..8953172da 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1561,6 +1561,7 @@ fn find_char_pair(cx: &mut Context, direction: Direction, extend: bool) { // TODO: count is reset to 1 before next key so we move it into the closure here. // Would be nice to carry over. let count = cx.count(); + let eof = doc!(cx.editor).line_ending.as_str(); // need to wait for next key // TODO: should this be done by grapheme rather than char? For example, @@ -1570,19 +1571,16 @@ fn find_char_pair(cx: &mut Context, direction: Direction, extend: bool) { KeyEvent { code: KeyCode::Enter, .. - } => { - find_char_line_ending(cx, count, direction, true, false); - return; - } + } => search::PairMatcher::LineEnding(eof), KeyEvent { code: KeyCode::Tab, .. - } => '\t', + } => search::PairMatcher::Char('\t'), KeyEvent { code: KeyCode::Char(ch), .. - } => ch, + } => search::PairMatcher::Char(ch), _ => return, }; @@ -1591,19 +1589,16 @@ fn find_char_pair(cx: &mut Context, direction: Direction, extend: bool) { KeyEvent { code: KeyCode::Enter, .. - } => { - find_char_line_ending(cx, count, direction, true, false); - return; - } + } => search::PairMatcher::LineEnding(eof), KeyEvent { code: KeyCode::Tab, .. - } => '\t', + } => search::PairMatcher::Char('\t'), KeyEvent { code: KeyCode::Char(ch), .. - } => ch, + } => search::PairMatcher::Char(ch), _ => return, }; let motion = move |editor: &mut Editor| { @@ -1612,7 +1607,7 @@ fn find_char_pair(cx: &mut Context, direction: Direction, extend: bool) { let selection = doc.selection(view.id).clone(); let selection = match direction { Direction::Forward => selection.transform(|range| { - search::find_nth_next_pair(text, ch, ch_2, range.char_cursor(), count) + search::find_nth_pair(text, ch, ch_2, range.char_cursor(), count, direction) .map_or(range, |pos| { if extend { Range::new(range.from(), pos + 2) @@ -1622,7 +1617,7 @@ fn find_char_pair(cx: &mut Context, direction: Direction, extend: bool) { }) }), Direction::Backward => selection.transform(|range| { - search::find_nth_prev_pair(text, ch, ch_2, range.char_cursor(), count) + search::find_nth_pair(text, ch, ch_2, range.char_cursor(), count, direction) .map_or(range, |pos| { if extend { Range::new(pos + 2, range.to()) From 68e84530a4d240f06b87ec22c0c2966b65db022e Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Mon, 17 Feb 2025 15:36:29 +0000 Subject: [PATCH 09/10] test: add test for find next and previous pair --- helix-term/tests/test/commands.rs | 151 ++++++++++++++++++++++++++++++ 1 file changed, 151 insertions(+) diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index 2af1a054f..c09581526 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -6,6 +6,157 @@ mod insert; mod movement; mod write; +#[tokio::test(flavor = "multi_thread")] +async fn find_prev_pair() -> anyhow::Result<()> { + // finds prev pair of 2 letters + test(( + indoc! {"\ + hi + hi + #[hi|]# + hi"}, + "Hhi", + indoc! {"\ + hi + #[|hi]# + hi + hi"}, + )) + .await?; + // finds 3rd prev pair of 2 letters + test(( + indoc! {"\ + hi + hi + hi + #[hi|]#"}, + "3Hhi", + indoc! {"\ + #[|hi]# + hi + hi + hi"}, + )) + .await?; + // finds prev two newlines + test(( + indoc! {"\ + hi + hi + hi + + #[hi|]#"}, + "H", + indoc! {"\ + hi + hi + hi#[| + + ]#hi"}, + )) + .await?; + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn find_next_pair() -> anyhow::Result<()> { + // finds next pair of 2 letters (non-extend) + test(( + indoc! {"\ + #[hi|]# + hi + hi + hi"}, + "Lhi", + indoc! {"\ + hi + #[hi|]# + hi + hi"}, + )) + .await?; + // finds next pair of 2 letters (extend) + test(( + indoc! {"\ + hi + #[hi|]# + hi + hi"}, + "vLhi", + indoc! {"\ + hi + #[hi + hi|]# + hi"}, + )) + .await?; + // finds 3rd next pair of 2 letters (non-extend) + test(( + indoc! {"\ + #[hi|]# + hi + hi + hi"}, + "3Lhi", + indoc! {"\ + hi + hi + hi + #[hi|]#"}, + )) + .await?; + // finds 3rd next pair of 2 letters (extend) + test(( + indoc! {"\ + #[hi|]# + hi + hi + hi"}, + "v3Lhi", + indoc! {"\ + #[hi + hi + hi + hi|]#"}, + )) + .await?; + // finds next char followed by newline + test(( + indoc! {"\ + #[hi|]# + hi + hi + hi"}, + "Lh", + indoc! {"\ + hi#[ + h|]#i + hi + hi"}, + )) + .await?; + // finds next char followed by newline + test(( + indoc! {"\ + #[hi|]# + hi + hi + + hi"}, + "L", + indoc! {"\ + hi + hi + hi#[ + + |]#hi"}, + )) + .await?; + + Ok(()) +} + #[tokio::test(flavor = "multi_thread")] async fn search_selection_detect_word_boundaries_at_eof() -> anyhow::Result<()> { // From ce25dab101ece6c5cba10e00691bfdcb0d3a30fb Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Mon, 17 Feb 2025 15:38:43 +0000 Subject: [PATCH 10/10] docs: add information on new `H` and `L` mappings --- book/src/keymap.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/book/src/keymap.md b/book/src/keymap.md index 2797eaee2..7008d625e 100644 --- a/book/src/keymap.md +++ b/book/src/keymap.md @@ -49,8 +49,10 @@ Normal mode is the default mode when you launch helix. You can return to it from | `E` | Move next WORD end | `move_next_long_word_end` | | `t` | Find 'till next char | `find_till_char` | | `f` | Find next char | `find_next_char` | +| `L` | Find next 2 chars | `find_next_pair` | | `T` | Find 'till previous char | `till_prev_char` | | `F` | Find previous char | `find_prev_char` | +| `H` | Find prev 2 chars | `find_prev_pair` | | `G` | Go to line number `` | `goto_line` | | `Alt-.` | Repeat last motion (`f`, `t`, `m`, `[` or `]`) | `repeat_last_motion` | | `Home` | Move to the start of the line | `goto_line_start` |