bendns' repos / helix
Unnamed repository; edit this file 'description' to name the repository.
Fix find char with line ending (#14437)
Browse Source
Denys Rybalka 4 months ago
parent 60231f0 · commit b645c8e
Diffstat
-rw-r--r--helix-core/src/search.rs76
-rw-r--r--helix-core/src/surround.rs4
-rw-r--r--helix-term/src/commands.rs198
-rw-r--r--helix-term/tests/test/movement.rs116
4 files changed, 238 insertions, 156 deletions
diff --git a/helix-core/src/search.rs b/helix-core/src/search.rs
index 81cb4129..7cbc58d6 100644
--- a/helix-core/src/search.rs
+++ b/helix-core/src/search.rs
@@ -1,3 +1,4 @@
+use crate::movement::Direction;
use crate::RopeSlice;
// TODO: switch to std::str::Pattern when it is stable.
@@ -17,51 +18,70 @@ impl<F: Fn(&char) -> bool> CharMatcher for F {
}
}
-pub fn find_nth_next<M: CharMatcher>(
+// Finds the positions of the nth matching character in given direction
+// starting from the pos gap-index (see Range struct for explanation)
+pub fn find_nth_char<M: CharMatcher>(
+ mut n: usize,
text: RopeSlice,
char_matcher: M,
mut pos: usize,
- n: usize,
+ direction: Direction,
) -> Option<usize> {
- if pos >= text.len_chars() || n == 0 {
+ if n == 0 {
return None;
}
- let mut chars = text.chars_at(pos);
+ let mut chars = text.get_chars_at(pos)?;
- for _ in 0..n {
- loop {
+ match direction {
+ Direction::Forward => loop {
let c = chars.next()?;
-
+ if char_matcher.char_match(c) {
+ n -= 1;
+ if n == 0 {
+ return Some(pos);
+ }
+ }
pos += 1;
-
+ },
+ Direction::Backward => loop {
+ let c = chars.prev()?;
+ pos -= 1;
if char_matcher.char_match(c) {
- break;
+ n -= 1;
+ if n == 0 {
+ return Some(pos);
+ }
}
- }
- }
-
- Some(pos - 1)
+ },
+ };
}
-pub fn find_nth_prev(text: RopeSlice, ch: char, mut pos: usize, n: usize) -> Option<usize> {
- if pos == 0 || n == 0 {
- return None;
- }
+#[cfg(test)]
+mod test {
+ use super::*;
+ use crate::movement::Direction;
- let mut chars = text.chars_at(pos);
+ #[test]
+ fn test_find_nth_char() {
+ let text = RopeSlice::from("aa ⌚aa \r\n aa");
- for _ in 0..n {
- loop {
- let c = chars.prev()?;
+ // Forward direction
+ assert_eq!(find_nth_char(1, text, 'a', 5, Direction::Forward), Some(5));
+ assert_eq!(find_nth_char(2, text, 'a', 5, Direction::Forward), Some(10));
+ assert_eq!(find_nth_char(3, text, 'a', 5, Direction::Forward), Some(11));
+ assert_eq!(find_nth_char(4, text, 'a', 5, Direction::Forward), None);
- pos -= 1;
+ // Backward direction
+ assert_eq!(find_nth_char(1, text, 'a', 5, Direction::Backward), Some(4));
+ assert_eq!(find_nth_char(2, text, 'a', 5, Direction::Backward), Some(1));
+ assert_eq!(find_nth_char(3, text, 'a', 5, Direction::Backward), Some(0));
+ assert_eq!(find_nth_char(4, text, 'a', 5, Direction::Backward), None);
- if c == ch {
- break;
- }
- }
+ // Edge cases
+ assert_eq!(find_nth_char(0, text, 'a', 5, Direction::Forward), None); // n = 0
+ assert_eq!(find_nth_char(1, text, 'x', 5, Direction::Forward), None); // Not found
+ assert_eq!(find_nth_char(1, text, 'a', 20, Direction::Forward), None); // Beyond text
+ assert_eq!(find_nth_char(1, text, 'a', 0, Direction::Backward), None); // At start going backward
}
-
- Some(pos)
}
diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs
index dbd197eb..4dec41f1 100644
--- a/helix-core/src/surround.rs
+++ b/helix-core/src/surround.rs
@@ -196,8 +196,8 @@ pub fn find_nth_pairs_pos(
.ok_or(Error::CursorOnAmbiguousPair)?
} else {
(
- search::find_nth_prev(text, open, pos, n),
- search::find_nth_next(text, close, pos, n),
+ search::find_nth_char(n, text, open, pos, Direction::Backward),
+ search::find_nth_char(n, text, close, pos, Direction::Forward),
)
}
} else {
diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs
index df2eade8..24546908 100644
--- a/helix-term/src/commands.rs
+++ b/helix-term/src/commands.rs
@@ -35,7 +35,7 @@ use helix_core::{
movement::{self, move_vertically_visual, Direction},
object, pos_at_coords,
regex::{self, Regex},
- search::{self, CharMatcher},
+ search::{self},
selection, surround,
syntax::config::{BlockCommentToken, LanguageServerFeature},
text_annotations::{Overlay, TextAnnotations},
@@ -46,7 +46,7 @@ use helix_core::{
};
use helix_view::{
document::{FormatterError, Mode, SCRATCH_BUFFER_NAME},
- editor::Action,
+ editor::{Action, Motion},
expansion,
info::Info,
input::KeyEvent,
@@ -1492,51 +1492,53 @@ fn extend_next_sub_word_end(cx: &mut Context) {
// This is necessary because the one document can have different line endings inside. And we
// cannot predict what character to find when <ret> is pressed. On the current line it can be `lf`
// but on the next line it can be `crlf`. That's why [`find_char_impl`] cannot be applied here.
-fn find_char_line_ending(
- cx: &mut Context,
+fn find_char_line_ending_motion(
+ editor: &mut Editor,
count: usize,
direction: Direction,
inclusive: bool,
extend: bool,
) {
- let (view, doc) = current!(cx.editor);
+ let (view, doc) = current!(editor);
let text = doc.text().slice(..);
let selection = doc.selection(view.id).clone().transform(|range| {
- let cursor = range.cursor(text);
+ let cursor_anchor = range.cursor(text);
+ let cursor_head = next_grapheme_boundary(text, cursor_anchor);
let cursor_line = range.cursor_line(text);
- // Finding the line where we're going to find <ret>. Depends mostly on
- // `count`, but also takes into account edge cases where we're already at the end
- // of a line or the beginning of a line
- let find_on_line = match direction {
+ let pos = match direction {
Direction::Forward => {
- let on_edge = line_end_char_index(&text, cursor_line) == cursor;
- let line = cursor_line + count - 1 + (on_edge as usize);
+ let line_end = line_end_char_index(&text, cursor_line);
+ let on_edge = if inclusive {
+ line_end == cursor_anchor
+ } else {
+ line_end == cursor_head || line_end == cursor_anchor
+ };
+ let line = cursor_line + count - 1 + on_edge as usize;
if line >= text.len_lines() - 1 {
return range;
- } else {
- line
}
+ line_end_char_index(&text, line) - !inclusive as usize
}
Direction::Backward => {
- let on_edge = text.line_to_char(cursor_line) == cursor && !inclusive;
- let line = cursor_line as isize - (count as isize - 1 + on_edge as isize);
- if line <= 0 {
- return range;
+ if inclusive {
+ let line = cursor_line as isize - count as isize;
+ if line < 0 {
+ return range;
+ }
+ line_end_char_index(&text, line as usize)
} else {
- line as usize
+ let on_edge = text.line_to_char(cursor_line) == cursor_anchor;
+ let line = cursor_line as isize - count as isize + 1 - on_edge as isize;
+ if line <= 0 {
+ return range;
+ }
+ text.line_to_char(line as usize)
}
}
};
- let pos = match (direction, inclusive) {
- (Direction::Forward, true) => line_end_char_index(&text, find_on_line),
- (Direction::Forward, false) => line_end_char_index(&text, find_on_line) - 1,
- (Direction::Backward, true) => line_end_char_index(&text, find_on_line - 1),
- (Direction::Backward, false) => text.line_to_char(find_on_line),
- };
-
if extend {
range.put_cursor(text, pos, true)
} else {
@@ -1555,112 +1557,56 @@ fn find_char(cx: &mut Context, direction: Direction, inclusive: bool, extend: bo
// 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)
- }
- Direction::Backward => {
- find_char_impl(editor, &find_prev_char_impl, inclusive, extend, ch, count)
- }
- };
- };
-
- cx.editor.apply_motion(motion);
- })
-}
+ let motion: Motion = if event.code == KeyCode::Enter {
+ Box::new(move |editor: &mut Editor| {
+ find_char_line_ending_motion(editor, count, direction, inclusive, extend);
+ })
+ } else if let Some(ch) = match event.code {
+ KeyCode::Tab => Some('\t'),
+ KeyCode::Char(ch) => Some(ch),
+ _ => None,
+ } {
+ Box::new(move |editor: &mut Editor| {
+ let (view, doc) = current!(editor);
+ let text = doc.text().slice(..);
-//
+ let selection = doc.selection(view.id).clone().transform(|range| {
+ let cursor_anchor = range.cursor(text);
+ let cursor_head = next_grapheme_boundary(text, cursor_anchor);
+
+ // Exclusive search skips the next char after cursor to enable repeated application
+ let search_start_pos = match (inclusive, direction) {
+ (true, Direction::Forward) => cursor_head,
+ (true, Direction::Backward) => cursor_anchor,
+ (false, Direction::Forward) => cursor_head + 1,
+ (false, Direction::Backward) => cursor_anchor - 1,
+ };
-#[inline]
-fn find_char_impl<F, M: CharMatcher + Clone + Copy>(
- editor: &mut Editor,
- search_fn: &F,
- inclusive: bool,
- extend: bool,
- char_matcher: M,
- count: usize,
-) where
- F: Fn(RopeSlice, M, usize, usize, bool) -> Option<usize> + 'static,
-{
- let (view, doc) = current!(editor);
- let text = doc.text().slice(..);
+ search::find_nth_char(count, text, ch, search_start_pos, direction)
+ // Exclusive search should stop on previous character
+ .map(|pos| match (inclusive, direction) {
+ (true, Direction::Forward) => pos,
+ (true, Direction::Backward) => pos,
+ (false, Direction::Forward) => pos - 1,
+ (false, Direction::Backward) => pos + 1,
+ })
+ .map_or(range, |pos| {
+ if extend {
+ range.put_cursor(text, pos, true)
+ } else {
+ Range::point(range.cursor(text)).put_cursor(text, pos, true)
+ }
+ })
+ });
- 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
+ doc.set_selection(view.id, selection);
+ })
} else {
- range.head
- };
-
- search_fn(text, char_matcher, search_start_pos, count, inclusive).map_or(range, |pos| {
- if extend {
- range.put_cursor(text, pos, true)
- } else {
- Range::point(range.cursor(text)).put_cursor(text, pos, true)
- }
- })
- });
- 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 {
- let n = match text.get_char(pos) {
- Some(next_ch) if next_ch == ch => n + 1,
- _ => n,
+ return;
};
- 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 {
- 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()))
- }
+ cx.editor.apply_motion(motion);
+ })
}
fn find_till_char(cx: &mut Context) {
diff --git a/helix-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs
index 062d3796..0a17c929 100644
--- a/helix-term/tests/test/movement.rs
+++ b/helix-term/tests/test/movement.rs
@@ -549,7 +549,69 @@ async fn select_mode_tree_sitter_prev_function_goes_backwards_to_object() -> any
}
#[tokio::test(flavor = "multi_thread")]
+async fn find_char() -> anyhow::Result<()> {
+ test(("he#[l|]#lo\nhello", "fl", "he#[ll|]#o\nhello")).await?;
+ test(("hel#[l|]#o\nhello", "fl", "hel#[lo\nhel|]#lo")).await?;
+ test(("hel#[l|]#o\nhello", "fx", "hel#[l|]#o\nhello")).await?;
+ test(("he#[l|]#lo\nhello", "2fl", "he#[llo\nhel|]#lo")).await?;
+ test(("#[h|]#ello\nhello", "9fl", "#[h|]#ello\nhello")).await?;
+
+ test(("h#[e|]#llo\nhello", "tl", "h#[el|]#lo\nhello")).await?;
+ test(("he#[l|]#lo\nhello", "tl", "he#[llo\nhe|]#llo")).await?;
+ test(("hel#[l|]#o\nhello", "tl", "hel#[lo\nhe|]#llo")).await?;
+ test(("hel#[l|]#o\nhello", "tx", "hel#[l|]#o\nhello")).await?;
+ test(("he#[l|]#lo\nhello", "2tl", "he#[llo\nhel|]#lo")).await?;
+ test(("#[h|]#ello\nhello", "9tl", "#[h|]#ello\nhello")).await?;
+
+ test(("hello\nhel#[l|]#o", "Fl", "hello\nhe#[|ll]#o")).await?;
+ test(("hello\nhe#[l|]#lo", "Fl", "hel#[|lo\nhel]#lo")).await?;
+ test(("hello\n#[h|]#ello", "Fx", "hello\n#[h|]#ello")).await?;
+ test(("hello\nhel#[l|]#o", "2Fl", "hel#[|lo\nhell]#o")).await?;
+ test(("hello\nhell#[o|]#", "9Fl", "hello\nhell#[o|]#")).await?;
+
+ test(("hello\nhell#[o|]#", "Tl", "hello\nhel#[|lo]#")).await?;
+ test(("hello\nhel#[l|]#o", "Tl", "hell#[|o\nhell]#o")).await?;
+ test(("hello\nhe#[l|]#lo", "Tl", "hell#[|o\nhel]#lo")).await?;
+ test(("hello\n#[h|]#ello", "Tx", "hello\n#[h|]#ello")).await?;
+ test(("hello\nhel#[l|]#o", "2Tl", "hel#[|lo\nhell]#o")).await?;
+ test(("hello\nhell#[o|]#", "9Tl", "hello\nhell#[o|]#")).await?;
+
+ Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread")]
async fn find_char_line_ending() -> anyhow::Result<()> {
+ test(("on#[e|]#\ntwo\n", "f<ret>", "on#[e\n|]#two\n")).await?;
+ test(("one#[\n|]#two\n", "f<ret>", "one#[\ntwo\n|]#")).await?;
+ test(("one\n#[t|]#wo\n", "f<ret>", "one\n#[two\n|]#")).await?;
+ test(("one#[\n|]#", "f<ret>", "one#[\n|]#")).await?;
+ test(("#[o|]#ne\ntwo\n", "2f<ret>", "#[one\ntwo\n|]#")).await?;
+ test(("#[o|]#ne\ntwo\n", "9f<ret>", "#[o|]#ne\ntwo\n")).await?;
+
+ test(("o#[n|]#e\ntwo\n", "t<ret>", "o#[ne|]#\ntwo\n")).await?;
+ test(("on#[e|]#\ntwo\n", "t<ret>", "on#[e\ntwo|]#\n")).await?;
+ test(("one#[\n|]#two\n", "t<ret>", "one#[\ntwo|]#\n")).await?;
+ test(("one#[\n|]#", "t<ret>", "one#[\n|]#")).await?;
+ test(("on#[e|]#\n", "t<ret>", "on#[e|]#\n")).await?;
+ test(("#[o|]#ne\ntwo\n", "2t<ret>", "#[one\ntwo|]#\n")).await?;
+ test(("#[o|]#ne\ntwo\n", "9t<ret>", "#[o|]#ne\ntwo\n")).await?;
+
+ test(("one\ntwo\n#[t|]#hree\n", "F<ret>", "one\ntwo#[|\nt]#hree\n")).await?;
+ test(("one\ntwo#[\n|]#three\n", "F<ret>", "one#[|\ntwo\n]#three\n")).await?;
+ test(("one\ntw#[o|]#\nthree\n", "F<ret>", "one#[|\ntwo]#\nthree\n")).await?;
+ test(("o#[n|]#e\n", "F<ret>", "o#[n|]#e\n")).await?;
+ test(("#[o|]#ne\n", "F<ret>", "#[o|]#ne\n")).await?;
+ test(("one\ntwo\nth#[r|]#ee\n", "2F\n", "one#[|\ntwo\nthr]#ee\n")).await?;
+ test(("one\ntwo\nth#[r|]#ee\n", "9F\n", "one\ntwo\nth#[r|]#ee\n")).await?;
+
+ test(("one\ntwo\nth#[r|]#ee\n", "T<ret>", "one\ntwo\n#[|thr]#ee\n")).await?;
+ test(("one\ntwo\n#[t|]#hree\n", "T<ret>", "one\n#[|two\nt]#hree\n")).await?;
+ test(("one\ntwo#[\n|]#three\n", "T<ret>", "one\n#[|two\n]#three\n")).await?;
+ test(("o#[n|]#e\n", "T<ret>", "o#[n|]#e\n")).await?;
+ test(("#[o|]#ne\n", "T<ret>", "#[o|]#ne\n")).await?;
+ test(("one\ntwo\nth#[r|]#ee\n", "2T\n", "one\n#[|two\nthr]#ee\n")).await?;
+ test(("one\ntwo\nth#[r|]#ee\n", "9T\n", "one\ntwo\nth#[r|]#ee\n")).await?;
+
test((
indoc! {
"\
@@ -588,6 +650,60 @@ async fn find_char_line_ending() -> anyhow::Result<()> {
}
#[tokio::test(flavor = "multi_thread")]
+async fn repeat_find_char() -> anyhow::Result<()> {
+ test((
+ indoc! {
+ "\
+ #[o|]#ne two
+ one two"
+ },
+ "ft<A-.>",
+ indoc! {
+ "\
+ one #[two
+ one t|]#wo"
+ },
+ ))
+ .await?;
+
+ test((
+ indoc! {
+ "\
+ #[o|]#ne two
+ one two
+ "
+ },
+ "f<ret><A-.>",
+ indoc! {
+ "\
+ one two#[
+ one two
+ |]#"
+ },
+ ))
+ .await?;
+
+ test((
+ indoc! {
+ "\
+ #[o|]#ne two
+ one two
+ "
+ },
+ "ftf<ret><A-.>",
+ indoc! {
+ "\
+ one two#[
+ one two
+ |]#"
+ },
+ ))
+ .await?;
+
+ Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread")]
async fn test_surround_replace() -> anyhow::Result<()> {
test((
indoc! {"\