From bb3e6998e641f25f3f73d0805522b451c18ac633 Mon Sep 17 00:00:00 2001 From: woojiq <122799969+woojiq@users.noreply.github.com> Date: Mon, 4 Sep 2023 00:12:38 +0300 Subject: [PATCH] Fix find commands for buffers with non-LF line-endings (#8111) --- helix-term/src/commands.rs | 69 +++++++++++++++++++++++++++---- helix-term/tests/test/movement.rs | 39 +++++++++++++++++ 2 files changed, 101 insertions(+), 7 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 31ea7581..6e3fb939 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1251,6 +1251,65 @@ fn extend_next_long_word_end(cx: &mut Context) { extend_word_impl(cx, movement::move_next_long_word_end) } +/// Separate branch to find_char designed only for char. +// +// This is necessary because the one document can have different line endings inside. And we +// cannot predict what character to find when 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, + count: usize, + direction: Direction, + inclusive: bool, + extend: bool, +) { + let (view, doc) = current!(cx.editor); + let text = doc.text().slice(..); + + let selection = doc.selection(view.id).clone().transform(|range| { + let cursor = range.cursor(text); + let cursor_line = range.cursor_line(text); + + // Finding the line where we're going to find . 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 { + Direction::Forward => { + let on_edge = line_end_char_index(&text, cursor_line) == cursor; + let line = cursor_line + count - 1 + (on_edge as usize); + if line >= text.len_lines() - 1 { + return range; + } else { + line + } + } + 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; + } else { + 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 { + Range::point(range.cursor(text)).put_cursor(text, pos, true) + } + }); + doc.set_selection(view.id, selection); +} + 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. @@ -1264,13 +1323,9 @@ fn find_char(cx: &mut Context, direction: Direction, inclusive: bool, extend: bo KeyEvent { code: KeyCode::Enter, .. - } => - // TODO: this isn't quite correct when CRLF is involved. - // This hack will work in most cases, since documents don't - // usually mix line endings. But we should fix it eventually - // anyway. - { - doc!(cx.editor).line_ending.as_str().chars().next().unwrap() + } => { + find_char_line_ending(cx, count, direction, inclusive, extend); + return; } KeyEvent { diff --git a/helix-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs index 9a48cdbc..e3c2668d 100644 --- a/helix-term/tests/test/movement.rs +++ b/helix-term/tests/test/movement.rs @@ -513,3 +513,42 @@ async fn select_mode_tree_sitter_prev_function_goes_backwards_to_object() -> any Ok(()) } + +#[tokio::test(flavor = "multi_thread")] +async fn find_char_line_ending() -> anyhow::Result<()> { + test(( + helpers::platform_line(indoc! { + "\ + one + #[|t]#wo + three" + }), + "Tgll2f", + helpers::platform_line(indoc! { + "\ + one + two#[ + |]#three" + }), + )) + .await?; + + test(( + helpers::platform_line(indoc! { + "\ + #[|o]#ne + two + three" + }), + "f2tghTF", + helpers::platform_line(indoc! { + "\ + one#[| + t]#wo + three" + }), + )) + .await?; + + Ok(()) +}