From 05bc741eaf060792a355c37ed5bfb3b664c73e86 Mon Sep 17 00:00:00 2001 From: fantacell Date: Sun, 6 Jul 2025 21:30:15 +0200 Subject: [PATCH 01/28] Add a basic helix match operator --- assets/keymaps/vim.json | 10 +++++++--- crates/vim/src/normal.rs | 2 ++ crates/vim/src/normal/select.rs | 29 +++++++++++++++++++++++++++++ crates/vim/src/object.rs | 4 ++-- crates/vim/src/state.rs | 8 ++++++-- crates/vim/src/vim.rs | 5 +++++ 6 files changed, 51 insertions(+), 7 deletions(-) create mode 100644 crates/vim/src/normal/select.rs diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index ba3012cc54..6507f313e9 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -383,6 +383,7 @@ "shift-a": "vim::InsertEndOfLine", "o": "vim::InsertLineBelow", "shift-o": "vim::InsertLineAbove", + "m": "vim::PushHelixMatch", "~": "vim::ChangeCase", "ctrl-a": "vim::Increment", "ctrl-x": "vim::Decrement", @@ -460,9 +461,6 @@ "space c": "editor::ToggleComments", "space y": "editor::Copy", "space p": "editor::Paste", - // Match mode - "m m": "vim::Matching", - "m i w": ["workspace::SendKeystrokes", "v i w"], "shift-u": "editor::Redo", "ctrl-c": "editor::ToggleComments", "d": "vim::HelixDelete", @@ -568,6 +566,12 @@ "e": "vim::EntireFile" } }, + { + "context": "vim_operator == ℳ", + "bindings": { + "m": "vim::Matching" + } + }, { "context": "vim_operator == c", "bindings": { diff --git a/crates/vim/src/normal.rs b/crates/vim/src/normal.rs index f772c446fe..1e03f964fa 100644 --- a/crates/vim/src/normal.rs +++ b/crates/vim/src/normal.rs @@ -7,6 +7,7 @@ mod paste; pub(crate) mod repeat; mod scroll; pub(crate) mod search; +mod select; pub mod substitute; mod toggle_comments; pub(crate) mod yank; @@ -362,6 +363,7 @@ impl Vim { self.replace_with_register_object(object, around, window, cx) } Some(Operator::Exchange) => self.exchange_object(object, around, window, cx), + Some(Operator::HelixMatch) => self.select_object(object, around, window, cx), _ => { // Can't do anything for namespace operators. Ignoring } diff --git a/crates/vim/src/normal/select.rs b/crates/vim/src/normal/select.rs new file mode 100644 index 0000000000..9b20e59faf --- /dev/null +++ b/crates/vim/src/normal/select.rs @@ -0,0 +1,29 @@ +use text::SelectionGoal; +use ui::{Context, Window}; + +use crate::{Vim, object::Object}; + +impl Vim { + /// Selects the text object each cursor is over. + pub fn select_object( + &mut self, + object: Object, + around: bool, + window: &mut Window, + cx: &mut Context, + ) { + self.stop_recording(cx); + self.update_editor(window, cx, |_, editor, window, cx| { + editor.change_selections(Default::default(), window, cx, |s| { + s.move_with(|map, selection| { + let Some(range) = object.range(map, selection.clone(), around, None) else { + return; + }; + + selection.set_head(range.end, SelectionGoal::None); + selection.start = range.start; + }); + }); + }); + } +} diff --git a/crates/vim/src/object.rs b/crates/vim/src/object.rs index 63139d7e94..93a19b9158 100644 --- a/crates/vim/src/object.rs +++ b/crates/vim/src/object.rs @@ -399,11 +399,11 @@ impl Vim { let count = Self::take_count(cx); match self.mode { - Mode::Normal => self.normal_object(object, count, window, cx), + Mode::Normal | Mode::HelixNormal => self.normal_object(object, count, window, cx), Mode::Visual | Mode::VisualLine | Mode::VisualBlock => { self.visual_object(object, count, window, cx) } - Mode::Insert | Mode::Replace | Mode::HelixNormal => { + Mode::Insert | Mode::Replace => { // Shouldn't execute a text object in insert mode. Ignoring } } diff --git a/crates/vim/src/state.rs b/crates/vim/src/state.rs index c4be034871..4bbf429fe7 100644 --- a/crates/vim/src/state.rs +++ b/crates/vim/src/state.rs @@ -132,6 +132,7 @@ pub enum Operator { ToggleComments, ReplaceWithRegister, Exchange, + HelixMatch, } #[derive(Default, Clone, Debug)] @@ -1024,6 +1025,7 @@ impl Operator { Operator::RecordRegister => "q", Operator::ReplayRegister => "@", Operator::ToggleComments => "gc", + Operator::HelixMatch => "ℳ", } } @@ -1075,7 +1077,8 @@ impl Operator { | Operator::Object { .. } | Operator::ChangeSurrounds { target: None } | Operator::OppositeCase - | Operator::ToggleComments => false, + | Operator::ToggleComments + | Operator::HelixMatch => false, } } @@ -1114,7 +1117,8 @@ impl Operator { | Operator::Jump { .. } | Operator::Register | Operator::RecordRegister - | Operator::ReplayRegister => false, + | Operator::ReplayRegister + | Operator::HelixMatch => false, } } } diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index 9229f145d9..4ae2931b02 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -222,6 +222,8 @@ actions!( PushReplaceWithRegister, /// Toggles comments. PushToggleComments, + /// Starts a match operation. + PushHelixMatch, ] ); @@ -761,6 +763,9 @@ impl Vim { Vim::action(editor, cx, |vim, _: &Enter, window, cx| { vim.input_ignored("\n".into(), window, cx) }); + Vim::action(editor, cx, |vim, _: &PushHelixMatch, window, cx| { + vim.push_operator(Operator::HelixMatch, window, cx) + }); normal::register(editor, cx); insert::register(editor, cx); From 309831ea9a2023d9c8381dc0d63a39120a6af56e Mon Sep 17 00:00:00 2001 From: fantacell Date: Tue, 8 Jul 2025 11:18:47 +0200 Subject: [PATCH 02/28] Solve an inconsistency between vim and helix --- crates/vim/src/normal/select.rs | 2 +- crates/vim/src/object.rs | 69 +++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/crates/vim/src/normal/select.rs b/crates/vim/src/normal/select.rs index 9b20e59faf..56f1cfdaaf 100644 --- a/crates/vim/src/normal/select.rs +++ b/crates/vim/src/normal/select.rs @@ -16,7 +16,7 @@ impl Vim { self.update_editor(window, cx, |_, editor, window, cx| { editor.change_selections(Default::default(), window, cx, |s| { s.move_with(|map, selection| { - let Some(range) = object.range(map, selection.clone(), around, None) else { + let Some(range) = object.helix_range(map, selection.clone(), around) else { return; }; diff --git a/crates/vim/src/object.rs b/crates/vim/src/object.rs index 93a19b9158..fdc4357ab2 100644 --- a/crates/vim/src/object.rs +++ b/crates/vim/src/object.rs @@ -714,6 +714,27 @@ impl Object { } } + /// Returns the range the object spans if the cursor is over it. + /// Follows helix convention. + pub fn helix_range( + self, + map: &DisplaySnapshot, + selection: Selection, + around: bool, + ) -> Option> { + let relative_to = selection.head(); + match self { + Object::Word { ignore_punctuation } => { + if around { + helix_around_word(map, relative_to, ignore_punctuation) + } else { + helix_in_word(map, relative_to, ignore_punctuation) + } + } + _ => self.range(map, selection, around, None), + } + } + pub fn expand_selection( self, map: &DisplaySnapshot, @@ -759,6 +780,42 @@ fn in_word( Some(start..end) } +/// Returns a range that surrounds the word `relative_to` is in. +/// +/// If `relative_to` is between words, return `None`. +fn helix_in_word( + map: &DisplaySnapshot, + relative_to: DisplayPoint, + ignore_punctuation: bool, +) -> Option> { + // Use motion::right so that we consider the character under the cursor when looking for the start + let classifier = map + .buffer_snapshot + .char_classifier_at(relative_to.to_point(map)) + .ignore_punctuation(ignore_punctuation); + let char = map + .buffer_chars_at(relative_to.to_offset(map, Bias::Left)) + .next()? + .0; + + if classifier.kind(char) == CharKind::Whitespace { + return None; + } + + let start = movement::find_preceding_boundary_display_point( + map, + right(map, relative_to, 1), + movement::FindRange::SingleLine, + |left, right| classifier.kind(left) != classifier.kind(right), + ); + + let end = movement::find_boundary(map, relative_to, FindRange::SingleLine, |left, right| { + classifier.kind(left) != classifier.kind(right) + }); + + Some(start..end) +} + fn in_subword( map: &DisplaySnapshot, relative_to: DisplayPoint, @@ -927,6 +984,18 @@ fn around_word( } } +/// Returns the range of the word the cursor is over and all the whitespace on one side. +/// If there is whitespace after that is included, otherwise it's whitespace before the word if any. +fn helix_around_word( + map: &DisplaySnapshot, + relative_to: DisplayPoint, + ignore_punctuation: bool, +) -> Option> { + let word_range = helix_in_word(map, relative_to, ignore_punctuation)?; + + Some(expand_to_include_whitespace(map, word_range, true)) +} + fn around_subword( map: &DisplaySnapshot, relative_to: DisplayPoint, From 459f7e3403d53068aceabfe20b292eef1a914d0c Mon Sep 17 00:00:00 2001 From: fantacell Date: Tue, 8 Jul 2025 15:02:09 +0200 Subject: [PATCH 03/28] Add a test --- crates/vim/src/object.rs | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/crates/vim/src/object.rs b/crates/vim/src/object.rs index fdc4357ab2..44f314a427 100644 --- a/crates/vim/src/object.rs +++ b/crates/vim/src/object.rs @@ -1822,6 +1822,45 @@ mod test { .assert_matches(); } + #[gpui::test] + async fn test_select_word_object(cx: &mut gpui::TestAppContext) { + let mut cx = VimTestContext::new(cx, true).await; + let start = indoc! {" + The quick brˇowˇnˇ + fox «ˇjumps» ov«er + the laˇ»zy dogˇ + " + }; + + cx.set_state(start, Mode::HelixNormal); + + cx.simulate_keystrokes("m i w"); + + cx.assert_state( + indoc! {" + The quick «brownˇ» + fox «jumpsˇ» over + the «lazyˇ» dogˇ + " + }, + Mode::HelixNormal, + ); + + cx.set_state(start, Mode::HelixNormal); + + cx.simulate_keystrokes("m a w"); + + cx.assert_state( + indoc! {" + The quick« brownˇ» + fox «jumps ˇ»over + the «lazy ˇ»dogˇ + " + }, + Mode::HelixNormal, + ); + } + #[gpui::test] async fn test_visual_word_object(cx: &mut gpui::TestAppContext) { let mut cx = NeovimBackedTestContext::new(cx).await; From b93869c564e8e21a971377d1d857e1d0c215daa0 Mon Sep 17 00:00:00 2001 From: fantacell Date: Wed, 9 Jul 2025 12:35:24 +0200 Subject: [PATCH 04/28] Start a helix directory --- crates/vim/src/helix.rs | 2 + crates/vim/src/helix/object.rs | 129 +++++++++++++++++++++++++++++++++ crates/vim/src/object.rs | 110 +--------------------------- 3 files changed, 132 insertions(+), 109 deletions(-) create mode 100644 crates/vim/src/helix/object.rs diff --git a/crates/vim/src/helix.rs b/crates/vim/src/helix.rs index e271c06a5e..93edf757b2 100644 --- a/crates/vim/src/helix.rs +++ b/crates/vim/src/helix.rs @@ -1,3 +1,5 @@ +mod object; + use editor::{DisplayPoint, Editor, movement}; use gpui::{Action, actions}; use gpui::{Context, Window}; diff --git a/crates/vim/src/helix/object.rs b/crates/vim/src/helix/object.rs new file mode 100644 index 0000000000..d08e188d1e --- /dev/null +++ b/crates/vim/src/helix/object.rs @@ -0,0 +1,129 @@ +use std::ops::Range; + +use editor::{ + DisplayPoint, + display_map::DisplaySnapshot, + movement::{self, FindRange}, +}; +use language::CharKind; +use text::{Bias, Selection}; + +use crate::{ + motion::right, + object::{Object, expand_to_include_whitespace}, +}; + +impl Object { + /// Returns + /// Follows helix convention. + pub fn helix_range( + self, + map: &DisplaySnapshot, + selection: Selection, + around: bool, + ) -> Option> { + let relative_to = selection.head(); + match self { + Object::Word { ignore_punctuation } => { + if around { + helix_around_word(map, relative_to, ignore_punctuation) + } else { + helix_in_word(map, relative_to, ignore_punctuation) + } + } + _ => self.range(map, selection, around, None), + } + } +} + +/// Returns a range that surrounds the word `relative_to` is in. +/// +/// If `relative_to` is between words, return `None`. +fn helix_in_word( + map: &DisplaySnapshot, + relative_to: DisplayPoint, + ignore_punctuation: bool, +) -> Option> { + // Use motion::right so that we consider the character under the cursor when looking for the start + let classifier = map + .buffer_snapshot + .char_classifier_at(relative_to.to_point(map)) + .ignore_punctuation(ignore_punctuation); + let char = map + .buffer_chars_at(relative_to.to_offset(map, Bias::Left)) + .next()? + .0; + + if classifier.kind(char) == CharKind::Whitespace { + return None; + } + + let start = movement::find_preceding_boundary_display_point( + map, + right(map, relative_to, 1), + movement::FindRange::SingleLine, + |left, right| classifier.kind(left) != classifier.kind(right), + ); + + let end = movement::find_boundary(map, relative_to, FindRange::SingleLine, |left, right| { + classifier.kind(left) != classifier.kind(right) + }); + + Some(start..end) +} + +/// Returns the range of the word the cursor is over and all the whitespace on one side. +/// If there is whitespace after that is included, otherwise it's whitespace before the word if any. +fn helix_around_word( + map: &DisplaySnapshot, + relative_to: DisplayPoint, + ignore_punctuation: bool, +) -> Option> { + let word_range = helix_in_word(map, relative_to, ignore_punctuation)?; + + Some(expand_to_include_whitespace(map, word_range, true)) +} + +#[cfg(test)] +mod test { + use crate::{state::Mode, test::VimTestContext}; + + #[gpui::test] + async fn test_select_word_object(cx: &mut gpui::TestAppContext) { + let mut cx = VimTestContext::new(cx, true).await; + let start = indoc! {" + The quick brˇowˇnˇ + fox «ˇjumps» ov«er + the laˇ»zy dogˇ + " + }; + + cx.set_state(start, Mode::HelixNormal); + + cx.simulate_keystrokes("m i w"); + + cx.assert_state( + indoc! {" + The quick «brownˇ» + fox «jumpsˇ» over + the «lazyˇ» dogˇ + " + }, + Mode::HelixNormal, + ); + + cx.set_state(start, Mode::HelixNormal); + + cx.simulate_keystrokes("m a w"); + + cx.assert_state( + indoc! {" + The quick« brownˇ» + fox «jumps ˇ»over + the «lazy ˇ»dogˇ + " + }, + Mode::HelixNormal, + ); + } +} diff --git a/crates/vim/src/object.rs b/crates/vim/src/object.rs index 44f314a427..14e7fc7243 100644 --- a/crates/vim/src/object.rs +++ b/crates/vim/src/object.rs @@ -714,27 +714,6 @@ impl Object { } } - /// Returns the range the object spans if the cursor is over it. - /// Follows helix convention. - pub fn helix_range( - self, - map: &DisplaySnapshot, - selection: Selection, - around: bool, - ) -> Option> { - let relative_to = selection.head(); - match self { - Object::Word { ignore_punctuation } => { - if around { - helix_around_word(map, relative_to, ignore_punctuation) - } else { - helix_in_word(map, relative_to, ignore_punctuation) - } - } - _ => self.range(map, selection, around, None), - } - } - pub fn expand_selection( self, map: &DisplaySnapshot, @@ -780,42 +759,6 @@ fn in_word( Some(start..end) } -/// Returns a range that surrounds the word `relative_to` is in. -/// -/// If `relative_to` is between words, return `None`. -fn helix_in_word( - map: &DisplaySnapshot, - relative_to: DisplayPoint, - ignore_punctuation: bool, -) -> Option> { - // Use motion::right so that we consider the character under the cursor when looking for the start - let classifier = map - .buffer_snapshot - .char_classifier_at(relative_to.to_point(map)) - .ignore_punctuation(ignore_punctuation); - let char = map - .buffer_chars_at(relative_to.to_offset(map, Bias::Left)) - .next()? - .0; - - if classifier.kind(char) == CharKind::Whitespace { - return None; - } - - let start = movement::find_preceding_boundary_display_point( - map, - right(map, relative_to, 1), - movement::FindRange::SingleLine, - |left, right| classifier.kind(left) != classifier.kind(right), - ); - - let end = movement::find_boundary(map, relative_to, FindRange::SingleLine, |left, right| { - classifier.kind(left) != classifier.kind(right) - }); - - Some(start..end) -} - fn in_subword( map: &DisplaySnapshot, relative_to: DisplayPoint, @@ -984,18 +927,6 @@ fn around_word( } } -/// Returns the range of the word the cursor is over and all the whitespace on one side. -/// If there is whitespace after that is included, otherwise it's whitespace before the word if any. -fn helix_around_word( - map: &DisplaySnapshot, - relative_to: DisplayPoint, - ignore_punctuation: bool, -) -> Option> { - let word_range = helix_in_word(map, relative_to, ignore_punctuation)?; - - Some(expand_to_include_whitespace(map, word_range, true)) -} - fn around_subword( map: &DisplaySnapshot, relative_to: DisplayPoint, @@ -1435,7 +1366,7 @@ fn is_sentence_end(map: &DisplaySnapshot, offset: usize) -> bool { /// Expands the passed range to include whitespace on one side or the other in a line. Attempts to add the /// whitespace to the end first and falls back to the start if there was none. -fn expand_to_include_whitespace( +pub fn expand_to_include_whitespace( map: &DisplaySnapshot, range: Range, stop_at_newline: bool, @@ -1822,45 +1753,6 @@ mod test { .assert_matches(); } - #[gpui::test] - async fn test_select_word_object(cx: &mut gpui::TestAppContext) { - let mut cx = VimTestContext::new(cx, true).await; - let start = indoc! {" - The quick brˇowˇnˇ - fox «ˇjumps» ov«er - the laˇ»zy dogˇ - " - }; - - cx.set_state(start, Mode::HelixNormal); - - cx.simulate_keystrokes("m i w"); - - cx.assert_state( - indoc! {" - The quick «brownˇ» - fox «jumpsˇ» over - the «lazyˇ» dogˇ - " - }, - Mode::HelixNormal, - ); - - cx.set_state(start, Mode::HelixNormal); - - cx.simulate_keystrokes("m a w"); - - cx.assert_state( - indoc! {" - The quick« brownˇ» - fox «jumps ˇ»over - the «lazy ˇ»dogˇ - " - }, - Mode::HelixNormal, - ); - } - #[gpui::test] async fn test_visual_word_object(cx: &mut gpui::TestAppContext) { let mut cx = NeovimBackedTestContext::new(cx).await; From 63771f1f1bad309acdce7f86905b11aee9260357 Mon Sep 17 00:00:00 2001 From: fantacell Date: Wed, 9 Jul 2025 12:39:04 +0200 Subject: [PATCH 05/28] Change how the match operator is displayed --- assets/keymaps/vim.json | 2 +- crates/vim/src/helix/object.rs | 2 ++ crates/vim/src/state.rs | 3 ++- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 6507f313e9..106d85187b 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -567,7 +567,7 @@ } }, { - "context": "vim_operator == ℳ", + "context": "vim_operator == helix_m", "bindings": { "m": "vim::Matching" } diff --git a/crates/vim/src/helix/object.rs b/crates/vim/src/helix/object.rs index d08e188d1e..ab9bb345f7 100644 --- a/crates/vim/src/helix/object.rs +++ b/crates/vim/src/helix/object.rs @@ -86,6 +86,8 @@ fn helix_around_word( #[cfg(test)] mod test { + use db::indoc; + use crate::{state::Mode, test::VimTestContext}; #[gpui::test] diff --git a/crates/vim/src/state.rs b/crates/vim/src/state.rs index 4bbf429fe7..3eb07e4c64 100644 --- a/crates/vim/src/state.rs +++ b/crates/vim/src/state.rs @@ -1025,7 +1025,7 @@ impl Operator { Operator::RecordRegister => "q", Operator::ReplayRegister => "@", Operator::ToggleComments => "gc", - Operator::HelixMatch => "ℳ", + Operator::HelixMatch => "helix_m", } } @@ -1039,6 +1039,7 @@ impl Operator { } => format!("^V{prefix}"), Operator::AutoIndent => "=".to_string(), Operator::ShellCommand => "=".to_string(), + Operator::HelixMatch => "m".to_string(), _ => self.id().to_string(), } } From 5d4cd65c6bf28eef551ab45ead06e7e3bcfc1c48 Mon Sep 17 00:00:00 2001 From: fantacell Date: Wed, 9 Jul 2025 12:45:32 +0200 Subject: [PATCH 06/28] Add select.rs to the new helix directory --- crates/vim/src/helix.rs | 1 + crates/vim/src/normal.rs | 1 - crates/vim/src/normal/select.rs | 29 ----------------------------- 3 files changed, 1 insertion(+), 30 deletions(-) delete mode 100644 crates/vim/src/normal/select.rs diff --git a/crates/vim/src/helix.rs b/crates/vim/src/helix.rs index 93edf757b2..fbc4b0641a 100644 --- a/crates/vim/src/helix.rs +++ b/crates/vim/src/helix.rs @@ -1,4 +1,5 @@ mod object; +mod select; use editor::{DisplayPoint, Editor, movement}; use gpui::{Action, actions}; diff --git a/crates/vim/src/normal.rs b/crates/vim/src/normal.rs index 1e03f964fa..c5ddfc0fac 100644 --- a/crates/vim/src/normal.rs +++ b/crates/vim/src/normal.rs @@ -7,7 +7,6 @@ mod paste; pub(crate) mod repeat; mod scroll; pub(crate) mod search; -mod select; pub mod substitute; mod toggle_comments; pub(crate) mod yank; diff --git a/crates/vim/src/normal/select.rs b/crates/vim/src/normal/select.rs deleted file mode 100644 index 56f1cfdaaf..0000000000 --- a/crates/vim/src/normal/select.rs +++ /dev/null @@ -1,29 +0,0 @@ -use text::SelectionGoal; -use ui::{Context, Window}; - -use crate::{Vim, object::Object}; - -impl Vim { - /// Selects the text object each cursor is over. - pub fn select_object( - &mut self, - object: Object, - around: bool, - window: &mut Window, - cx: &mut Context, - ) { - self.stop_recording(cx); - self.update_editor(window, cx, |_, editor, window, cx| { - editor.change_selections(Default::default(), window, cx, |s| { - s.move_with(|map, selection| { - let Some(range) = object.helix_range(map, selection.clone(), around) else { - return; - }; - - selection.set_head(range.end, SelectionGoal::None); - selection.start = range.start; - }); - }); - }); - } -} From bb55357a62d90434e60b411a93c8b63d169c69cb Mon Sep 17 00:00:00 2001 From: fantacell Date: Wed, 9 Jul 2025 12:59:17 +0200 Subject: [PATCH 07/28] Add select.rs to git tracking --- crates/vim/src/helix/select.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 crates/vim/src/helix/select.rs diff --git a/crates/vim/src/helix/select.rs b/crates/vim/src/helix/select.rs new file mode 100644 index 0000000000..56f1cfdaaf --- /dev/null +++ b/crates/vim/src/helix/select.rs @@ -0,0 +1,29 @@ +use text::SelectionGoal; +use ui::{Context, Window}; + +use crate::{Vim, object::Object}; + +impl Vim { + /// Selects the text object each cursor is over. + pub fn select_object( + &mut self, + object: Object, + around: bool, + window: &mut Window, + cx: &mut Context, + ) { + self.stop_recording(cx); + self.update_editor(window, cx, |_, editor, window, cx| { + editor.change_selections(Default::default(), window, cx, |s| { + s.move_with(|map, selection| { + let Some(range) = object.helix_range(map, selection.clone(), around) else { + return; + }; + + selection.set_head(range.end, SelectionGoal::None); + selection.start = range.start; + }); + }); + }); + } +} From 69eaf04dad3dabd85a03b1bf5cce27f072d57d48 Mon Sep 17 00:00:00 2001 From: fantacell Date: Wed, 9 Jul 2025 19:08:31 +0200 Subject: [PATCH 08/28] Fix marks in vim normal mode --- assets/keymaps/vim.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index b28086bb6e..e5f68db32e 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -358,7 +358,6 @@ "shift-a": "vim::InsertEndOfLine", "o": "vim::InsertLineBelow", "shift-o": "vim::InsertLineAbove", - "m": "vim::PushHelixMatch", "~": "vim::ChangeCase", "ctrl-a": "vim::Increment", "ctrl-x": "vim::Decrement", @@ -377,6 +376,7 @@ { "context": "vim_mode == helix_normal && !menu", "bindings": { + "m": "vim::PushHelixMatch", "ctrl-[": "editor::Cancel", ":": "command_palette::Toggle", "left": "vim::WrappingLeft", From 9b44bb67064d2ca3f46937c5aea2753b1b89ce8b Mon Sep 17 00:00:00 2001 From: fantacell Date: Sat, 19 Jul 2025 15:31:58 +0200 Subject: [PATCH 09/28] Add the ] and [ operators to helix --- assets/keymaps/vim.json | 16 +- crates/text/src/selection.rs | 13 ++ crates/vim/src/helix.rs | 1 + crates/vim/src/helix/boundary.rs | 184 +++++++++++++++++++++++ crates/vim/src/helix/object.rs | 249 ++++++++++++++++++++++++------- crates/vim/src/helix/select.rs | 56 ++++++- crates/vim/src/normal.rs | 8 +- crates/vim/src/state.rs | 12 +- crates/vim/src/vim.rs | 36 +++++ 9 files changed, 504 insertions(+), 71 deletions(-) create mode 100644 crates/vim/src/helix/boundary.rs diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index e5f68db32e..746a05afd7 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -376,9 +376,11 @@ { "context": "vim_mode == helix_normal && !menu", "bindings": { - "m": "vim::PushHelixMatch", "ctrl-[": "editor::Cancel", ":": "command_palette::Toggle", + "m": "vim::PushHelixMatch", + "]": ["vim::PushHelixNext", { "around": true }], + "[": ["vim::PushHelixPrevious", { "around": true }], "left": "vim::WrappingLeft", "right": "vim::WrappingRight", "h": "vim::WrappingLeft", @@ -456,6 +458,12 @@ "alt-shift-c": "editor::AddSelectionAbove" } }, + { + "context": "vim_operator == helix_m", + "bindings": { + "m": "vim::Matching" + } + }, { "context": "vim_mode == insert && !(showing_code_actions || showing_completions)", "bindings": { @@ -553,12 +561,6 @@ "e": "vim::EntireFile" } }, - { - "context": "vim_operator == helix_m", - "bindings": { - "m": "vim::Matching" - } - }, { "context": "vim_operator == c", "bindings": { diff --git a/crates/text/src/selection.rs b/crates/text/src/selection.rs index 18b82dbb6a..05e46b754d 100644 --- a/crates/text/src/selection.rs +++ b/crates/text/src/selection.rs @@ -104,6 +104,19 @@ impl Selection { self.goal = new_goal; } + pub fn set_tail_head(&mut self, tail: T, head: T, new_goal: SelectionGoal) { + if tail > head { + self.start = head; + self.end = tail; + self.reversed = true; + } else { + self.start = tail; + self.end = head; + self.reversed = false; + } + self.goal = new_goal; + } + pub fn swap_head_tail(&mut self) { if self.reversed { self.reversed = false; diff --git a/crates/vim/src/helix.rs b/crates/vim/src/helix.rs index d3f3ef2b45..bae68d1cde 100644 --- a/crates/vim/src/helix.rs +++ b/crates/vim/src/helix.rs @@ -1,3 +1,4 @@ +mod boundary; mod object; mod select; diff --git a/crates/vim/src/helix/boundary.rs b/crates/vim/src/helix/boundary.rs new file mode 100644 index 0000000000..290ae19374 --- /dev/null +++ b/crates/vim/src/helix/boundary.rs @@ -0,0 +1,184 @@ +use std::{error::Error, fmt::Display}; + +use editor::{ + DisplayPoint, + display_map::{DisplaySnapshot, ToDisplayPoint}, +}; +use language::{CharClassifier, CharKind}; +use text::Bias; + +use crate::object::Object; + +#[derive(Debug)] +pub struct UnboundedErr; +impl Display for UnboundedErr { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "object can't be found with simple boundary checking") + } +} +impl Error for UnboundedErr {} + +impl Object { + /// Returns the beginning of the inside of the closest object after the cursor if it can easily be found. Follows helix convention; + pub fn helix_next_start( + self, + map: &DisplaySnapshot, + relative_to: DisplayPoint, + ) -> Result, UnboundedErr> { + try_find_boundary(map, relative_to, |left, right| { + let classifier = map + .buffer_snapshot + .char_classifier_at(relative_to.to_point(map)); + self.helix_is_start(right, left, classifier) + }) + } + /// Returns the end of the inside of the closest object after the cursor if it can easily be found. Follows helix convention; + pub fn helix_next_end( + self, + map: &DisplaySnapshot, + relative_to: DisplayPoint, + ) -> Result, UnboundedErr> { + try_find_boundary(map, relative_to, |left, right| { + let classifier = map + .buffer_snapshot + .char_classifier_at(relative_to.to_point(map)); + self.helix_is_end(right, left, classifier) + }) + } + /// Returns the beginning of the inside of the closest object before the cursor if it can easily be found. Follows helix convention; + pub fn helix_previous_start( + self, + map: &DisplaySnapshot, + relative_to: DisplayPoint, + ) -> Result, UnboundedErr> { + try_find_preceding_boundary(map, relative_to, |left, right| { + let classifier = map + .buffer_snapshot + .char_classifier_at(relative_to.to_point(map)); + self.helix_is_start(right, left, classifier) + }) + } + /// Returns the end of the inside of the closest object before the cursor if it can easily be found. Follows helix convention; + pub fn helix_previous_end( + self, + map: &DisplaySnapshot, + relative_to: DisplayPoint, + ) -> Result, UnboundedErr> { + try_find_preceding_boundary(map, relative_to, |left, right| { + let classifier = map + .buffer_snapshot + .char_classifier_at(relative_to.to_point(map)); + self.helix_is_end(right, left, classifier) + }) + } + + fn helix_is_start( + self, + right: char, + left: char, + classifier: CharClassifier, + ) -> Result { + match self { + Self::Word { ignore_punctuation } => { + let classifier = classifier.ignore_punctuation(ignore_punctuation); + Ok(is_word_start(left, right, classifier)) + } + Self::Subword { ignore_punctuation } => { + todo!() + } + Self::AngleBrackets => Ok(left == '<'), + Self::BackQuotes => Ok(left == '`'), + Self::CurlyBrackets => Ok(left == '{'), + Self::DoubleQuotes => Ok(left == '"'), + Self::Parentheses => Ok(left == '('), + Self::SquareBrackets => Ok(left == '['), + Self::VerticalBars => Ok(left == '|'), + _ => Err(UnboundedErr), + } + } + + fn helix_is_end( + self, + right: char, + left: char, + classifier: CharClassifier, + ) -> Result { + match self { + Self::Word { ignore_punctuation } => { + let classifier = classifier.ignore_punctuation(ignore_punctuation); + Ok(is_word_end(left, right, classifier)) + } + Self::Subword { ignore_punctuation } => { + todo!() + } + Self::AngleBrackets => Ok(right == '>'), + Self::BackQuotes => Ok(right == '`'), + Self::CurlyBrackets => Ok(right == '}'), + Self::DoubleQuotes => Ok(right == '"'), + Self::Parentheses => Ok(right == ')'), + Self::SquareBrackets => Ok(right == ']'), + Self::VerticalBars => Ok(right == '|'), + Self::Sentence => Ok(left == '.'), + _ => Err(UnboundedErr), + } + } +} + +fn try_find_boundary( + map: &DisplaySnapshot, + from: DisplayPoint, + mut is_boundary: impl FnMut(char, char) -> Result, +) -> Result, UnboundedErr> { + let mut offset = from.to_offset(map, Bias::Right); + let mut prev_ch = map + .buffer_snapshot + .reversed_chars_at(offset) + .next() + .unwrap_or('\0'); + + for ch in map.buffer_snapshot.chars_at(offset) { + if is_boundary(prev_ch, ch)? { + return Ok(Some( + map.clip_point(offset.to_display_point(map), Bias::Right), + )); + } + offset += ch.len_utf8(); + prev_ch = ch; + } + + Ok(None) +} + +fn try_find_preceding_boundary( + map: &DisplaySnapshot, + from: DisplayPoint, + mut is_boundary: impl FnMut(char, char) -> Result, +) -> Result, UnboundedErr> { + let mut offset = from.to_offset(map, Bias::Right); + let mut prev_ch = map.buffer_snapshot.chars_at(offset).next().unwrap_or('\0'); + + for ch in map.buffer_snapshot.reversed_chars_at(offset) { + if is_boundary(ch, prev_ch)? { + return Ok(Some( + map.clip_point(offset.to_display_point(map), Bias::Right), + )); + } + offset -= ch.len_utf8(); + prev_ch = ch; + } + + Ok(None) +} + +fn is_buffer_start(left: char) -> bool { + left == '\0' +} + +fn is_word_start(left: char, right: char, classifier: CharClassifier) -> bool { + classifier.kind(left) != classifier.kind(right) + && classifier.kind(right) != CharKind::Whitespace +} + +fn is_word_end(left: char, right: char, classifier: CharClassifier) -> bool { + classifier.kind(left) != classifier.kind(right) && classifier.kind(left) != CharKind::Whitespace +} diff --git a/crates/vim/src/helix/object.rs b/crates/vim/src/helix/object.rs index ab9bb345f7..29df3d6640 100644 --- a/crates/vim/src/helix/object.rs +++ b/crates/vim/src/helix/object.rs @@ -1,20 +1,16 @@ -use std::ops::Range; +use std::{cmp::Ordering, ops::Range}; use editor::{ DisplayPoint, display_map::DisplaySnapshot, - movement::{self, FindRange}, + movement::{self}, }; -use language::CharKind; -use text::{Bias, Selection}; +use text::Selection; -use crate::{ - motion::right, - object::{Object, expand_to_include_whitespace}, -}; +use crate::{helix::boundary::UnboundedErr, object::Object}; impl Object { - /// Returns + /// Returns the range of the object the cursor is over. /// Follows helix convention. pub fn helix_range( self, @@ -23,65 +19,204 @@ impl Object { around: bool, ) -> Option> { let relative_to = selection.head(); - match self { - Object::Word { ignore_punctuation } => { - if around { - helix_around_word(map, relative_to, ignore_punctuation) - } else { - helix_in_word(map, relative_to, ignore_punctuation) - } + if let Ok(selection) = self.current_bounded_object(map, relative_to) { + if around { + selection.map(|s| self.surround(map, s).unwrap()) + } else { + selection + } + } else { + let head = selection.head(); + let range = self.range(map, selection, around, None)?; + + if range.start > head { + None + } else { + Some(range) } - _ => self.range(map, selection, around, None), } } -} -/// Returns a range that surrounds the word `relative_to` is in. -/// -/// If `relative_to` is between words, return `None`. -fn helix_in_word( - map: &DisplaySnapshot, - relative_to: DisplayPoint, - ignore_punctuation: bool, -) -> Option> { - // Use motion::right so that we consider the character under the cursor when looking for the start - let classifier = map - .buffer_snapshot - .char_classifier_at(relative_to.to_point(map)) - .ignore_punctuation(ignore_punctuation); - let char = map - .buffer_chars_at(relative_to.to_offset(map, Bias::Left)) - .next()? - .0; + /// Returns the range of the next object the cursor is not over. + /// Follows helix convention. + pub fn helix_next_range( + self, + map: &DisplaySnapshot, + selection: Selection, + around: bool, + ) -> Option> { + let relative_to = selection.head(); + if let Ok(selection) = self.next_bounded_object(map, relative_to) { + if around { + selection.map(|s| self.surround(map, s).unwrap()) + } else { + selection + } + } else { + let head = selection.head(); + let range = self.range(map, selection, around, None)?; - if classifier.kind(char) == CharKind::Whitespace { - return None; + if range.start > head { + Some(range) + } else { + None + } + } } - let start = movement::find_preceding_boundary_display_point( - map, - right(map, relative_to, 1), - movement::FindRange::SingleLine, - |left, right| classifier.kind(left) != classifier.kind(right), - ); + /// Returns the range of the previous object the cursor is not over. + /// Follows helix convention. + pub fn helix_previous_range( + self, + map: &DisplaySnapshot, + selection: Selection, + around: bool, + ) -> Option> { + let relative_to = selection.head(); + if let Ok(selection) = self.previous_bounded_object(map, relative_to) { + if around { + selection.map(|s| self.surround(map, s).unwrap()) + } else { + selection + } + } else { + None + } + } - let end = movement::find_boundary(map, relative_to, FindRange::SingleLine, |left, right| { - classifier.kind(left) != classifier.kind(right) - }); + /// Returns the range of the object the cursor is over if it can be found with simple boundary checking. Potentially none. Follows helix convention. + fn current_bounded_object( + self, + map: &DisplaySnapshot, + relative_to: DisplayPoint, + ) -> Result>, UnboundedErr> { + let maybe_prev_end = self.helix_previous_end(map, relative_to)?; + let Some(prev_start) = self.helix_previous_start(map, relative_to)? else { + return Ok(None); + }; + let Some(next_end) = self.helix_next_end(map, movement::right(map, relative_to))? else { + return Ok(None); + }; + let maybe_next_start = self.helix_next_start(map, movement::right(map, relative_to))?; - Some(start..end) -} + if let Some(next_start) = maybe_next_start { + match next_start.cmp(&next_end) { + Ordering::Less => return Ok(None), + Ordering::Equal if self.can_be_zero_width() => return Ok(None), + _ => (), + } + } + if let Some(prev_end) = maybe_prev_end { + if prev_start == prev_end && self.can_be_zero_width() { + return Ok(None); + } + debug_assert!(prev_end <= prev_start) + } -/// Returns the range of the word the cursor is over and all the whitespace on one side. -/// If there is whitespace after that is included, otherwise it's whitespace before the word if any. -fn helix_around_word( - map: &DisplaySnapshot, - relative_to: DisplayPoint, - ignore_punctuation: bool, -) -> Option> { - let word_range = helix_in_word(map, relative_to, ignore_punctuation)?; + Ok(Some(prev_start..next_end)) + } - Some(expand_to_include_whitespace(map, word_range, true)) + /// Returns the range of the next object the cursor is not over if it can be found with simple boundary checking. Potentially none. Follows helix convention. + fn next_bounded_object( + self, + map: &DisplaySnapshot, + relative_to: DisplayPoint, + ) -> Result>, UnboundedErr> { + let Some(next_start) = self.helix_next_start(map, movement::right(map, relative_to))? + else { + return Ok(None); + }; + let search_start = if self.can_be_zero_width() { + next_start + } else { + movement::right(map, next_start) + }; + let Some(end) = self.helix_next_end(map, search_start)? else { + return Ok(None); + }; + + Ok(Some(next_start..end)) + } + + /// Returns the previous range of the object the cursor not is over if it can be found with simple boundary checking. Potentially none. Follows helix convention. + fn previous_bounded_object( + self, + map: &DisplaySnapshot, + relative_to: DisplayPoint, + ) -> Result>, UnboundedErr> { + let Some(prev_end) = self.helix_previous_end(map, relative_to)? else { + return Ok(None); + }; + let search_start = if self.can_be_zero_width() { + prev_end + } else { + movement::left(map, prev_end) + }; + let Some(start) = self.helix_previous_start(map, search_start)? else { + return Ok(None); + }; + + Ok(Some(start..prev_end)) + } + + /// Switches from an 'mi' range to an 'ma' range. Follows helix convention. + fn surround( + self, + map: &DisplaySnapshot, + selection: Range, + ) -> Result, UnboundedErr> { + match self { + Self::Word { .. } | Self::Subword { .. } => { + let row = selection.end.row(); + let line_start = DisplayPoint::new(row, 0); + let line_end = DisplayPoint::new(row, map.line_len(row)); + let next_start = self + .helix_next_start(map, selection.end) + .unwrap() + .unwrap() + .min(line_end); + let prev_end = self + .helix_previous_end(map, selection.start) + .unwrap() + .unwrap() + .max(line_start); + if next_start > selection.end { + Ok(selection.start..next_start) + } else { + Ok(prev_end..selection.end) + } + } + Self::AngleBrackets + | Self::BackQuotes + | Self::CurlyBrackets + | Self::DoubleQuotes + | Self::Parentheses + | Self::SquareBrackets + | Self::VerticalBars => { + Ok(movement::left(map, selection.start)..movement::right(map, selection.end)) + } + _ => Err(UnboundedErr), + } + } + + const fn can_be_zero_width(&self) -> bool { + match self { + Self::AngleBrackets + | Self::AnyBrackets + | Self::AnyQuotes + | Self::BackQuotes + | Self::CurlyBrackets + | Self::DoubleQuotes + | Self::EntireFile + | Self::MiniBrackets + | Self::MiniQuotes + | Self::Parentheses + | Self::Quotes + | Self::SquareBrackets + | Self::VerticalBars => true, + _ => false, + } + } } #[cfg(test)] diff --git a/crates/vim/src/helix/select.rs b/crates/vim/src/helix/select.rs index 56f1cfdaaf..f266a40140 100644 --- a/crates/vim/src/helix/select.rs +++ b/crates/vim/src/helix/select.rs @@ -4,8 +4,9 @@ use ui::{Context, Window}; use crate::{Vim, object::Object}; impl Vim { - /// Selects the text object each cursor is over. - pub fn select_object( + /// Selects the object each cursor is over. + /// Follows helix convention. + pub fn select_current_object( &mut self, object: Object, around: bool, @@ -20,8 +21,55 @@ impl Vim { return; }; - selection.set_head(range.end, SelectionGoal::None); - selection.start = range.start; + selection.set_tail_head(range.start, range.end, SelectionGoal::None); + }); + }); + }); + } + + /// Selects the next object from each cursor which the cursor is not over. + /// Follows helix convention. + pub fn select_next_object( + &mut self, + object: Object, + around: bool, + window: &mut Window, + cx: &mut Context, + ) { + self.stop_recording(cx); + self.update_editor(window, cx, |_, editor, window, cx| { + editor.change_selections(Default::default(), window, cx, |s| { + s.move_with(|map, selection| { + let Some(range) = object.helix_next_range(map, selection.clone(), around) + else { + return; + }; + + selection.set_tail_head(range.start, range.end, SelectionGoal::None); + }); + }); + }); + } + + /// Selects the previous object from each cursor which the cursor is not over. + /// Follows helix convention. + pub fn select_previous_object( + &mut self, + object: Object, + around: bool, + window: &mut Window, + cx: &mut Context, + ) { + self.stop_recording(cx); + self.update_editor(window, cx, |_, editor, window, cx| { + editor.change_selections(Default::default(), window, cx, |s| { + s.move_with(|map, selection| { + let Some(range) = object.helix_previous_range(map, selection.clone(), around) + else { + return; + }; + + selection.set_tail_head(range.start, range.end, SelectionGoal::None); }); }); }); diff --git a/crates/vim/src/normal.rs b/crates/vim/src/normal.rs index 22e9f6094a..d099834bb2 100644 --- a/crates/vim/src/normal.rs +++ b/crates/vim/src/normal.rs @@ -479,7 +479,13 @@ impl Vim { self.replace_with_register_object(object, around, window, cx) } Some(Operator::Exchange) => self.exchange_object(object, around, window, cx), - Some(Operator::HelixMatch) => self.select_object(object, around, window, cx), + Some(Operator::HelixMatch) => { + self.select_current_object(object, around, window, cx) + } + Some(Operator::SelectNext) => self.select_next_object(object, around, window, cx), + Some(Operator::SelectPrevious) => { + self.select_previous_object(object, around, window, cx) + } _ => { // Can't do anything for namespace operators. Ignoring } diff --git a/crates/vim/src/state.rs b/crates/vim/src/state.rs index 3eb07e4c64..ef5e16c663 100644 --- a/crates/vim/src/state.rs +++ b/crates/vim/src/state.rs @@ -133,6 +133,8 @@ pub enum Operator { ReplaceWithRegister, Exchange, HelixMatch, + SelectNext, + SelectPrevious, } #[derive(Default, Clone, Debug)] @@ -1026,6 +1028,8 @@ impl Operator { Operator::ReplayRegister => "@", Operator::ToggleComments => "gc", Operator::HelixMatch => "helix_m", + Operator::SelectNext { .. } => "helix_]", + Operator::SelectPrevious { .. } => "helix_[", } } @@ -1079,7 +1083,9 @@ impl Operator { | Operator::ChangeSurrounds { target: None } | Operator::OppositeCase | Operator::ToggleComments - | Operator::HelixMatch => false, + | Operator::HelixMatch + | Operator::SelectNext { .. } + | Operator::SelectPrevious { .. } => false, } } @@ -1103,7 +1109,9 @@ impl Operator { | Operator::AddSurrounds { target: None } | Operator::ChangeSurrounds { target: None } | Operator::DeleteSurrounds - | Operator::Exchange => true, + | Operator::Exchange + | Operator::SelectNext { .. } + | Operator::SelectPrevious { .. } => true, Operator::Yank | Operator::Object { .. } | Operator::FindForward { .. } diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index 4efd68bf40..3f226b3f41 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -84,6 +84,22 @@ struct PushFindBackward { multiline: bool, } +#[derive(Clone, Deserialize, JsonSchema, PartialEq, Action)] +#[action(namespace = vim)] +#[serde(deny_unknown_fields)] +/// Selects the next object. +struct PushHelixNext { + around: bool, +} + +#[derive(Clone, Deserialize, JsonSchema, PartialEq, Action)] +#[action(namespace = vim)] +#[serde(deny_unknown_fields)] +/// Selects the previous object. +struct PushHelixPrevious { + around: bool, +} + #[derive(Clone, Deserialize, JsonSchema, PartialEq, Action)] #[action(namespace = vim)] #[serde(deny_unknown_fields)] @@ -768,6 +784,26 @@ impl Vim { Vim::action(editor, cx, |vim, _: &PushHelixMatch, window, cx| { vim.push_operator(Operator::HelixMatch, window, cx) }); + Vim::action(editor, cx, |vim, action: &PushHelixNext, window, cx| { + vim.push_operator(Operator::SelectNext, window, cx); + vim.push_operator( + Operator::Object { + around: action.around, + }, + window, + cx, + ) + }); + Vim::action(editor, cx, |vim, action: &PushHelixPrevious, window, cx| { + vim.push_operator(Operator::SelectPrevious, window, cx); + vim.push_operator( + Operator::Object { + around: action.around, + }, + window, + cx, + ); + }); normal::register(editor, cx); insert::register(editor, cx); From bd3b5c68296d36f2a63b07b70f6361491a1f06b1 Mon Sep 17 00:00:00 2001 From: fantacell Date: Mon, 21 Jul 2025 21:57:19 +0200 Subject: [PATCH 10/28] Fix issues: - fix the operators' display in the status bar - fill the subword object placeholder - fix some spelling mistakes - fix a bug where the first and last words in a buffer couldn't be selected --- crates/editor/src/movement.rs | 33 +++++++++++++++++++------------- crates/vim/src/helix/boundary.rs | 33 +++++++++++++++++++++----------- crates/vim/src/helix/object.rs | 9 ++++++--- crates/vim/src/state.rs | 2 ++ 4 files changed, 50 insertions(+), 27 deletions(-) diff --git a/crates/editor/src/movement.rs b/crates/editor/src/movement.rs index b9b7cb2e58..87b731b182 100644 --- a/crates/editor/src/movement.rs +++ b/crates/editor/src/movement.rs @@ -4,7 +4,7 @@ use super::{Bias, DisplayPoint, DisplaySnapshot, SelectionGoal, ToDisplayPoint}; use crate::{DisplayRow, EditorStyle, ToOffset, ToPoint, scroll::ScrollAnchor}; use gpui::{Pixels, WindowTextSystem}; -use language::Point; +use language::{CharClassifier, Point}; use multi_buffer::{MultiBufferRow, MultiBufferSnapshot}; use serde::Deserialize; use workspace::searchable::Direction; @@ -303,15 +303,18 @@ pub fn previous_subword_start(map: &DisplaySnapshot, point: DisplayPoint) -> Dis let classifier = map.buffer_snapshot.char_classifier_at(raw_point); find_preceding_boundary_display_point(map, point, FindRange::MultiLine, |left, right| { - let is_word_start = - classifier.kind(left) != classifier.kind(right) && !right.is_whitespace(); - let is_subword_start = classifier.is_word('-') && left == '-' && right != '-' - || left == '_' && right != '_' - || left.is_lowercase() && right.is_uppercase(); - is_word_start || is_subword_start || left == '\n' + is_subword_start(left, right, &classifier) || left == '\n' }) } +pub fn is_subword_start(left: char, right: char, classifier: &CharClassifier) -> bool { + let is_word_start = classifier.kind(left) != classifier.kind(right) && !right.is_whitespace(); + let is_subword_start = classifier.is_word('-') && left == '-' && right != '-' + || left == '_' && right != '_' + || left.is_lowercase() && right.is_uppercase(); + is_word_start || is_subword_start +} + /// Returns a position of the next word boundary, where a word character is defined as either /// uppercase letter, lowercase letter, '_' character or language-specific word character (like '-' in CSS). pub fn next_word_end(map: &DisplaySnapshot, point: DisplayPoint) -> DisplayPoint { @@ -361,15 +364,19 @@ pub fn next_subword_end(map: &DisplaySnapshot, point: DisplayPoint) -> DisplayPo let classifier = map.buffer_snapshot.char_classifier_at(raw_point); find_boundary(map, point, FindRange::MultiLine, |left, right| { - let is_word_end = - (classifier.kind(left) != classifier.kind(right)) && !classifier.is_whitespace(left); - let is_subword_end = classifier.is_word('-') && left != '-' && right == '-' - || left != '_' && right == '_' - || left.is_lowercase() && right.is_uppercase(); - is_word_end || is_subword_end || right == '\n' + is_subword_end(left, right, &classifier) || right == '\n' }) } +pub fn is_subword_end(left: char, right: char, classifier: &CharClassifier) -> bool { + let is_word_end = + (classifier.kind(left) != classifier.kind(right)) && !classifier.is_whitespace(left); + let is_subword_end = classifier.is_word('-') && left != '-' && right == '-' + || left != '_' && right == '_' + || left.is_lowercase() && right.is_uppercase(); + is_word_end || is_subword_end +} + /// Returns a position of the start of the current paragraph, where a paragraph /// is defined as a run of non-blank lines. pub fn start_of_paragraph( diff --git a/crates/vim/src/helix/boundary.rs b/crates/vim/src/helix/boundary.rs index 290ae19374..40cd00bb46 100644 --- a/crates/vim/src/helix/boundary.rs +++ b/crates/vim/src/helix/boundary.rs @@ -3,6 +3,7 @@ use std::{error::Error, fmt::Display}; use editor::{ DisplayPoint, display_map::{DisplaySnapshot, ToDisplayPoint}, + movement, }; use language::{CharClassifier, CharKind}; use text::Bias; @@ -19,7 +20,8 @@ impl Display for UnboundedErr { impl Error for UnboundedErr {} impl Object { - /// Returns the beginning of the inside of the closest object after the cursor if it can easily be found. Follows helix convention; + /// Returns the beginning of the inside of the closest object after the cursor if it can easily be found. + /// Follows helix convention. pub fn helix_next_start( self, map: &DisplaySnapshot, @@ -32,7 +34,8 @@ impl Object { self.helix_is_start(right, left, classifier) }) } - /// Returns the end of the inside of the closest object after the cursor if it can easily be found. Follows helix convention; + /// Returns the end of the inside of the closest object after the cursor if it can easily be found. + /// Follows helix convention. pub fn helix_next_end( self, map: &DisplaySnapshot, @@ -45,7 +48,8 @@ impl Object { self.helix_is_end(right, left, classifier) }) } - /// Returns the beginning of the inside of the closest object before the cursor if it can easily be found. Follows helix convention; + /// Returns the beginning of the inside of the closest object before the cursor if it can easily be found. + /// Follows helix convention. pub fn helix_previous_start( self, map: &DisplaySnapshot, @@ -58,7 +62,8 @@ impl Object { self.helix_is_start(right, left, classifier) }) } - /// Returns the end of the inside of the closest object before the cursor if it can easily be found. Follows helix convention; + /// Returns the end of the inside of the closest object before the cursor if it can easily be found. + /// Follows helix convention. pub fn helix_previous_end( self, map: &DisplaySnapshot, @@ -81,10 +86,11 @@ impl Object { match self { Self::Word { ignore_punctuation } => { let classifier = classifier.ignore_punctuation(ignore_punctuation); - Ok(is_word_start(left, right, classifier)) + Ok(is_word_start(left, right, classifier) || is_buffer_start(left)) } Self::Subword { ignore_punctuation } => { - todo!() + let classifier = classifier.ignore_punctuation(ignore_punctuation); + Ok(movement::is_subword_start(left, right, &classifier) || is_buffer_start(left)) } Self::AngleBrackets => Ok(left == '<'), Self::BackQuotes => Ok(left == '`'), @@ -106,10 +112,11 @@ impl Object { match self { Self::Word { ignore_punctuation } => { let classifier = classifier.ignore_punctuation(ignore_punctuation); - Ok(is_word_end(left, right, classifier)) + Ok(is_word_end(left, right, classifier) || is_buffer_end(right)) } Self::Subword { ignore_punctuation } => { - todo!() + let classifier = classifier.ignore_punctuation(ignore_punctuation); + Ok(movement::is_subword_end(left, right, &classifier) || is_buffer_end(right)) } Self::AngleBrackets => Ok(right == '>'), Self::BackQuotes => Ok(right == '`'), @@ -136,7 +143,7 @@ fn try_find_boundary( .next() .unwrap_or('\0'); - for ch in map.buffer_snapshot.chars_at(offset) { + for ch in map.buffer_snapshot.chars_at(offset).chain(['\0']) { if is_boundary(prev_ch, ch)? { return Ok(Some( map.clip_point(offset.to_display_point(map), Bias::Right), @@ -157,13 +164,13 @@ fn try_find_preceding_boundary( let mut offset = from.to_offset(map, Bias::Right); let mut prev_ch = map.buffer_snapshot.chars_at(offset).next().unwrap_or('\0'); - for ch in map.buffer_snapshot.reversed_chars_at(offset) { + for ch in map.buffer_snapshot.reversed_chars_at(offset).chain(['\0']) { if is_boundary(ch, prev_ch)? { return Ok(Some( map.clip_point(offset.to_display_point(map), Bias::Right), )); } - offset -= ch.len_utf8(); + offset = offset.saturating_sub(ch.len_utf8()); prev_ch = ch; } @@ -174,6 +181,10 @@ fn is_buffer_start(left: char) -> bool { left == '\0' } +fn is_buffer_end(right: char) -> bool { + right == '\0' +} + fn is_word_start(left: char, right: char, classifier: CharClassifier) -> bool { classifier.kind(left) != classifier.kind(right) && classifier.kind(right) != CharKind::Whitespace diff --git a/crates/vim/src/helix/object.rs b/crates/vim/src/helix/object.rs index 29df3d6640..3f222efd72 100644 --- a/crates/vim/src/helix/object.rs +++ b/crates/vim/src/helix/object.rs @@ -84,7 +84,8 @@ impl Object { } } - /// Returns the range of the object the cursor is over if it can be found with simple boundary checking. Potentially none. Follows helix convention. + /// Returns the range of the object the cursor is over if it can be found with simple boundary checking. + /// Potentially none. Follows helix convention. fn current_bounded_object( self, map: &DisplaySnapshot, @@ -116,7 +117,8 @@ impl Object { Ok(Some(prev_start..next_end)) } - /// Returns the range of the next object the cursor is not over if it can be found with simple boundary checking. Potentially none. Follows helix convention. + /// Returns the range of the next object the cursor is not over if it can be found with simple boundary checking. + /// Potentially none. Follows helix convention. fn next_bounded_object( self, map: &DisplaySnapshot, @@ -138,7 +140,8 @@ impl Object { Ok(Some(next_start..end)) } - /// Returns the previous range of the object the cursor not is over if it can be found with simple boundary checking. Potentially none. Follows helix convention. + /// Returns the previous range of the object the cursor not is over if it can be found with simple boundary checking. + /// Potentially none. Follows helix convention. fn previous_bounded_object( self, map: &DisplaySnapshot, diff --git a/crates/vim/src/state.rs b/crates/vim/src/state.rs index ef5e16c663..fc5234ff80 100644 --- a/crates/vim/src/state.rs +++ b/crates/vim/src/state.rs @@ -1044,6 +1044,8 @@ impl Operator { Operator::AutoIndent => "=".to_string(), Operator::ShellCommand => "=".to_string(), Operator::HelixMatch => "m".to_string(), + Operator::SelectNext => "]".to_string(), + Operator::SelectPrevious => "[".to_string(), _ => self.id().to_string(), } } From 11cfb88b36fdd47d6640e3a2a344226acc274ccd Mon Sep 17 00:00:00 2001 From: fantacell Date: Fri, 25 Jul 2025 11:33:43 +0200 Subject: [PATCH 11/28] Fix nested brackets and the failing tests --- crates/vim/src/helix/boundary.rs | 16 ++-- crates/vim/src/helix/object.rs | 153 +++++++++++++++++++++++-------- crates/zed/src/zed.rs | 2 + 3 files changed, 129 insertions(+), 42 deletions(-) diff --git a/crates/vim/src/helix/boundary.rs b/crates/vim/src/helix/boundary.rs index 40cd00bb46..55de056c09 100644 --- a/crates/vim/src/helix/boundary.rs +++ b/crates/vim/src/helix/boundary.rs @@ -86,11 +86,13 @@ impl Object { match self { Self::Word { ignore_punctuation } => { let classifier = classifier.ignore_punctuation(ignore_punctuation); - Ok(is_word_start(left, right, classifier) || is_buffer_start(left)) + Ok(is_word_start(left, right, &classifier) + || (is_buffer_start(left) && classifier.kind(right) != CharKind::Whitespace)) } Self::Subword { ignore_punctuation } => { let classifier = classifier.ignore_punctuation(ignore_punctuation); - Ok(movement::is_subword_start(left, right, &classifier) || is_buffer_start(left)) + Ok(movement::is_subword_start(left, right, &classifier) + || (is_buffer_start(left) && classifier.kind(right) != CharKind::Whitespace)) } Self::AngleBrackets => Ok(left == '<'), Self::BackQuotes => Ok(left == '`'), @@ -112,11 +114,13 @@ impl Object { match self { Self::Word { ignore_punctuation } => { let classifier = classifier.ignore_punctuation(ignore_punctuation); - Ok(is_word_end(left, right, classifier) || is_buffer_end(right)) + Ok(is_word_end(left, right, &classifier) + || (is_buffer_end(right) && classifier.kind(left) != CharKind::Whitespace)) } Self::Subword { ignore_punctuation } => { let classifier = classifier.ignore_punctuation(ignore_punctuation); - Ok(movement::is_subword_end(left, right, &classifier) || is_buffer_end(right)) + Ok(movement::is_subword_end(left, right, &classifier) + || (is_buffer_end(right) && classifier.kind(right) != CharKind::Whitespace)) } Self::AngleBrackets => Ok(right == '>'), Self::BackQuotes => Ok(right == '`'), @@ -185,11 +189,11 @@ fn is_buffer_end(right: char) -> bool { right == '\0' } -fn is_word_start(left: char, right: char, classifier: CharClassifier) -> bool { +fn is_word_start(left: char, right: char, classifier: &CharClassifier) -> bool { classifier.kind(left) != classifier.kind(right) && classifier.kind(right) != CharKind::Whitespace } -fn is_word_end(left: char, right: char, classifier: CharClassifier) -> bool { +fn is_word_end(left: char, right: char, classifier: &CharClassifier) -> bool { classifier.kind(left) != classifier.kind(right) && classifier.kind(left) != CharKind::Whitespace } diff --git a/crates/vim/src/helix/object.rs b/crates/vim/src/helix/object.rs index 3f222efd72..ce5a25057f 100644 --- a/crates/vim/src/helix/object.rs +++ b/crates/vim/src/helix/object.rs @@ -18,7 +18,7 @@ impl Object { selection: Selection, around: bool, ) -> Option> { - let relative_to = selection.head(); + let relative_to = cursor_start(&selection, map); if let Ok(selection) = self.current_bounded_object(map, relative_to) { if around { selection.map(|s| self.surround(map, s).unwrap()) @@ -26,10 +26,9 @@ impl Object { selection } } else { - let head = selection.head(); let range = self.range(map, selection, around, None)?; - if range.start > head { + if range.start > relative_to { None } else { Some(range) @@ -45,7 +44,7 @@ impl Object { selection: Selection, around: bool, ) -> Option> { - let relative_to = selection.head(); + let relative_to = cursor_start(&selection, map); if let Ok(selection) = self.next_bounded_object(map, relative_to) { if around { selection.map(|s| self.surround(map, s).unwrap()) @@ -53,10 +52,9 @@ impl Object { selection } } else { - let head = selection.head(); let range = self.range(map, selection, around, None)?; - if range.start > head { + if range.start > relative_to { Some(range) } else { None @@ -72,7 +70,7 @@ impl Object { selection: Selection, around: bool, ) -> Option> { - let relative_to = selection.head(); + let relative_to = cursor_start(&selection, map); if let Ok(selection) = self.previous_bounded_object(map, relative_to) { if around { selection.map(|s| self.surround(map, s).unwrap()) @@ -91,30 +89,29 @@ impl Object { map: &DisplaySnapshot, relative_to: DisplayPoint, ) -> Result>, UnboundedErr> { - let maybe_prev_end = self.helix_previous_end(map, relative_to)?; - let Some(prev_start) = self.helix_previous_start(map, relative_to)? else { + let Some(start) = self.helix_previous_start(map, relative_to)? else { return Ok(None); }; - let Some(next_end) = self.helix_next_end(map, movement::right(map, relative_to))? else { + let Some(end) = self.close_at_end(start, map)? else { return Ok(None); }; - let maybe_next_start = self.helix_next_start(map, movement::right(map, relative_to))?; - if let Some(next_start) = maybe_next_start { - match next_start.cmp(&next_end) { - Ordering::Less => return Ok(None), - Ordering::Equal if self.can_be_zero_width() => return Ok(None), - _ => (), - } - } - if let Some(prev_end) = maybe_prev_end { - if prev_start == prev_end && self.can_be_zero_width() { - return Ok(None); - } - debug_assert!(prev_end <= prev_start) + if end > relative_to { + return Ok(Some(start..end)); } - Ok(Some(prev_start..next_end)) + let Some(end) = self.helix_next_end(map, movement::right(map, relative_to))? else { + return Ok(None); + }; + let Some(start) = self.close_at_start(end, map)? else { + return Ok(None); + }; + + if start <= relative_to { + return Ok(Some(start..end)); + } + + Ok(None) } /// Returns the range of the next object the cursor is not over if it can be found with simple boundary checking. @@ -128,12 +125,7 @@ impl Object { else { return Ok(None); }; - let search_start = if self.can_be_zero_width() { - next_start - } else { - movement::right(map, next_start) - }; - let Some(end) = self.helix_next_end(map, search_start)? else { + let Some(end) = self.close_at_end(next_start, map)? else { return Ok(None); }; @@ -150,12 +142,7 @@ impl Object { let Some(prev_end) = self.helix_previous_end(map, relative_to)? else { return Ok(None); }; - let search_start = if self.can_be_zero_width() { - prev_end - } else { - movement::left(map, prev_end) - }; - let Some(start) = self.helix_previous_start(map, search_start)? else { + let Some(start) = self.close_at_start(prev_end, map)? else { return Ok(None); }; @@ -202,6 +189,71 @@ impl Object { } } + fn close_at_end( + self, + start: DisplayPoint, + map: &DisplaySnapshot, + ) -> Result, UnboundedErr> { + let mut last_start = movement::right(map, start); + let mut opened = 1; + while let Some(next_end) = self.helix_next_end(map, last_start)? { + if !self.can_be_nested() { + return Ok(Some(next_end)); + } + if let Some(next_start) = self.helix_next_start(map, last_start)? { + match next_start.cmp(&next_end) { + Ordering::Less => { + opened += 1; + last_start = movement::right(map, next_start); + continue; + } + Ordering::Equal if self.can_be_zero_width() => { + last_start = movement::right(map, next_start); + continue; + } + _ => (), + } + } + // When this is reached one opened object can be closed. + opened -= 1; + if opened == 0 { + return Ok(Some(next_end)); + } + last_start = movement::right(map, next_end); + } + Ok(None) + } + + fn close_at_start( + self, + end: DisplayPoint, + map: &DisplaySnapshot, + ) -> Result, UnboundedErr> { + let mut last_end = movement::left(map, end); + let mut opened = 1; + while let Some(previous_start) = self.helix_previous_start(map, last_end)? { + if !self.can_be_nested() { + return Ok(Some(previous_start)); + } + if let Some(previous_end) = self.helix_previous_end(map, last_end)? { + if previous_end > previous_start + || previous_end == previous_start && self.can_be_zero_width() + { + opened += 1; + last_end = movement::left(map, previous_end); + continue; + } + } + // When this is reached one opened object can be closed. + opened -= 1; + if opened == 0 { + return Ok(Some(previous_start)); + } + last_end = movement::left(map, previous_start); + } + Ok(None) + } + const fn can_be_zero_width(&self) -> bool { match self { Self::AngleBrackets @@ -220,6 +272,32 @@ impl Object { _ => false, } } + + const fn can_be_nested(&self) -> bool { + match self { + Self::AngleBrackets + | Self::AnyBrackets + | Self::CurlyBrackets + | Self::MiniBrackets + | Self::Parentheses + | Self::SquareBrackets + | Self::AnyQuotes + | Self::Class + | Self::Method + | Self::Tag + | Self::Argument => true, + _ => false, + } + } +} + +/// Returns the start of the cursor of a selection, whether that is collapsed or not. +fn cursor_start(selection: &Selection, map: &DisplaySnapshot) -> DisplayPoint { + if selection.is_empty() | selection.reversed { + selection.head() + } else { + movement::left(map, selection.head()) + } } #[cfg(test)] @@ -235,6 +313,7 @@ mod test { The quick brˇowˇnˇ fox «ˇjumps» ov«er the laˇ»zy dogˇ + " }; @@ -247,6 +326,7 @@ mod test { The quick «brownˇ» fox «jumpsˇ» over the «lazyˇ» dogˇ + " }, Mode::HelixNormal, @@ -261,6 +341,7 @@ mod test { The quick« brownˇ» fox «jumps ˇ»over the «lazy ˇ»dogˇ + " }, Mode::HelixNormal, diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 57534c8cd5..ba9d619348 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -4214,6 +4214,8 @@ mod tests { | "vim::PushJump" | "vim::PushDigraph" | "vim::PushLiteral" + | "vim::PushHelixNext" + | "vim::PushHelixPrevious" | "vim::Number" | "vim::SelectRegister" | "git::StageAndNext" From ec621c60e0c199ff58a9277a58d172c0b3c94c0f Mon Sep 17 00:00:00 2001 From: fantacell Date: Fri, 25 Jul 2025 16:41:55 +0200 Subject: [PATCH 12/28] Create a different object operator rather than pushing two operators on one input --- assets/keymaps/vim.json | 2 +- crates/vim/src/normal.rs | 10 ++++++---- crates/vim/src/state.rs | 24 ++++++++++++++---------- crates/vim/src/vim.rs | 8 +++----- 4 files changed, 24 insertions(+), 20 deletions(-) diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 1875f44f78..88a4855cf0 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -528,7 +528,7 @@ } }, { - "context": "vim_operator == a || vim_operator == i || vim_operator == cs", + "context": "vim_operator == a || vim_operator == i || vim_operator == cs || vim_operator == helix_next || vim_operator == helix_previous", "bindings": { "w": "vim::Word", "shift-w": ["vim::Word", { "ignore_punctuation": true }], diff --git a/crates/vim/src/normal.rs b/crates/vim/src/normal.rs index e2c022dce1..2f3f40da22 100644 --- a/crates/vim/src/normal.rs +++ b/crates/vim/src/normal.rs @@ -498,14 +498,16 @@ impl Vim { Some(Operator::HelixMatch) => { self.select_current_object(object, around, window, cx) } - Some(Operator::SelectNext) => self.select_next_object(object, around, window, cx), - Some(Operator::SelectPrevious) => { - self.select_previous_object(object, around, window, cx) - } _ => { // Can't do anything for namespace operators. Ignoring } }, + Some(Operator::HelixNext { around }) => { + self.select_next_object(object, around, window, cx); + } + Some(Operator::HelixPrevious { around }) => { + self.select_previous_object(object, around, window, cx); + } Some(Operator::DeleteSurrounds) => { waiting_operator = Some(Operator::DeleteSurrounds); } diff --git a/crates/vim/src/state.rs b/crates/vim/src/state.rs index fc5234ff80..556113e20a 100644 --- a/crates/vim/src/state.rs +++ b/crates/vim/src/state.rs @@ -133,8 +133,12 @@ pub enum Operator { ReplaceWithRegister, Exchange, HelixMatch, - SelectNext, - SelectPrevious, + HelixNext { + around: bool, + }, + HelixPrevious { + around: bool, + }, } #[derive(Default, Clone, Debug)] @@ -1028,8 +1032,8 @@ impl Operator { Operator::ReplayRegister => "@", Operator::ToggleComments => "gc", Operator::HelixMatch => "helix_m", - Operator::SelectNext { .. } => "helix_]", - Operator::SelectPrevious { .. } => "helix_[", + Operator::HelixNext { .. } => "helix_next", + Operator::HelixPrevious { .. } => "helix_previous", } } @@ -1044,8 +1048,8 @@ impl Operator { Operator::AutoIndent => "=".to_string(), Operator::ShellCommand => "=".to_string(), Operator::HelixMatch => "m".to_string(), - Operator::SelectNext => "]".to_string(), - Operator::SelectPrevious => "[".to_string(), + Operator::HelixNext { .. } => "]".to_string(), + Operator::HelixPrevious { .. } => "[".to_string(), _ => self.id().to_string(), } } @@ -1086,8 +1090,8 @@ impl Operator { | Operator::OppositeCase | Operator::ToggleComments | Operator::HelixMatch - | Operator::SelectNext { .. } - | Operator::SelectPrevious { .. } => false, + | Operator::HelixNext { .. } + | Operator::HelixPrevious { .. } => false, } } @@ -1112,8 +1116,8 @@ impl Operator { | Operator::ChangeSurrounds { target: None } | Operator::DeleteSurrounds | Operator::Exchange - | Operator::SelectNext { .. } - | Operator::SelectPrevious { .. } => true, + | Operator::HelixNext { .. } + | Operator::HelixPrevious { .. } => true, Operator::Yank | Operator::Object { .. } | Operator::FindForward { .. } diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index 525d291389..a8164b5b96 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -785,19 +785,17 @@ impl Vim { vim.push_operator(Operator::HelixMatch, window, cx) }); Vim::action(editor, cx, |vim, action: &PushHelixNext, window, cx| { - vim.push_operator(Operator::SelectNext, window, cx); vim.push_operator( - Operator::Object { + Operator::HelixNext { around: action.around, }, window, cx, - ) + ); }); Vim::action(editor, cx, |vim, action: &PushHelixPrevious, window, cx| { - vim.push_operator(Operator::SelectPrevious, window, cx); vim.push_operator( - Operator::Object { + Operator::HelixPrevious { around: action.around, }, window, From a67a5e0d35863730ec2895a7a755e8be159f47c0 Mon Sep 17 00:00:00 2001 From: fantacell Date: Sat, 9 Aug 2025 12:55:28 +0200 Subject: [PATCH 13/28] Rewrite the logic to use multiple traits instead of a lot of methods on one object. This makes return types shorter and makes it easy to add an extra helix object entirely. Also add logic for the sentence and paragraph text objects. The way this is implemented would make it easy to for example ignore brackets after a backslash. --- crates/vim/src/helix/boundary.rs | 629 ++++++++++++++++++++++++------- crates/vim/src/helix/object.rs | 315 ++++------------ 2 files changed, 560 insertions(+), 384 deletions(-) diff --git a/crates/vim/src/helix/boundary.rs b/crates/vim/src/helix/boundary.rs index 55de056c09..1cfa784a86 100644 --- a/crates/vim/src/helix/boundary.rs +++ b/crates/vim/src/helix/boundary.rs @@ -1,4 +1,4 @@ -use std::{error::Error, fmt::Display}; +use std::ops::Range; use editor::{ DisplayPoint, @@ -8,138 +8,474 @@ use editor::{ use language::{CharClassifier, CharKind}; use text::Bias; -use crate::object::Object; +use crate::helix::object::HelixTextObject; -#[derive(Debug)] -pub struct UnboundedErr; -impl Display for UnboundedErr { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "object can't be found with simple boundary checking") - } -} -impl Error for UnboundedErr {} - -impl Object { - /// Returns the beginning of the inside of the closest object after the cursor if it can easily be found. - /// Follows helix convention. - pub fn helix_next_start( - self, +/// Text objects (after helix definition) that can easily be +/// found by reading a buffer and comparing two neighboring chars +/// until a start / end is found +trait BoundedObject { + /// The next start since `from` (inclusive). + fn next_start(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option; + /// The next end since `from` (inclusive). + fn next_end(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option; + /// The previous start since `from` (inclusive). + fn previous_start(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option; + /// The previous end since `from` (inclusive). + fn previous_end(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option; + /// Switches from an 'mi' range to an 'ma' range. Follows helix convention. + fn surround( + &self, map: &DisplaySnapshot, - relative_to: DisplayPoint, - ) -> Result, UnboundedErr> { - try_find_boundary(map, relative_to, |left, right| { - let classifier = map - .buffer_snapshot - .char_classifier_at(relative_to.to_point(map)); - self.helix_is_start(right, left, classifier) - }) - } - /// Returns the end of the inside of the closest object after the cursor if it can easily be found. - /// Follows helix convention. - pub fn helix_next_end( - self, - map: &DisplaySnapshot, - relative_to: DisplayPoint, - ) -> Result, UnboundedErr> { - try_find_boundary(map, relative_to, |left, right| { - let classifier = map - .buffer_snapshot - .char_classifier_at(relative_to.to_point(map)); - self.helix_is_end(right, left, classifier) - }) - } - /// Returns the beginning of the inside of the closest object before the cursor if it can easily be found. - /// Follows helix convention. - pub fn helix_previous_start( - self, - map: &DisplaySnapshot, - relative_to: DisplayPoint, - ) -> Result, UnboundedErr> { - try_find_preceding_boundary(map, relative_to, |left, right| { - let classifier = map - .buffer_snapshot - .char_classifier_at(relative_to.to_point(map)); - self.helix_is_start(right, left, classifier) - }) - } - /// Returns the end of the inside of the closest object before the cursor if it can easily be found. - /// Follows helix convention. - pub fn helix_previous_end( - self, - map: &DisplaySnapshot, - relative_to: DisplayPoint, - ) -> Result, UnboundedErr> { - try_find_preceding_boundary(map, relative_to, |left, right| { - let classifier = map - .buffer_snapshot - .char_classifier_at(relative_to.to_point(map)); - self.helix_is_end(right, left, classifier) - }) - } - - fn helix_is_start( - self, - right: char, - left: char, - classifier: CharClassifier, - ) -> Result { - match self { - Self::Word { ignore_punctuation } => { - let classifier = classifier.ignore_punctuation(ignore_punctuation); - Ok(is_word_start(left, right, &classifier) - || (is_buffer_start(left) && classifier.kind(right) != CharKind::Whitespace)) + inner_range: Range, + ) -> Range; + /// Whether these objects can be inside ones of the same kind. + /// If so, the trait assumes they can have zero width. + fn can_be_nested(&self) -> bool; + /// The next end since `start` (inclusive) on the same nesting level. + fn close_at_end(&self, start: DisplayPoint, map: &DisplaySnapshot) -> Option { + if !self.can_be_nested() { + return self.next_end(map, movement::right(map, start)); + } + let mut end_search_start = start; + let mut start_search_start = movement::right(map, start); + loop { + let next_end = self.next_end(map, end_search_start)?; + let maybe_next_start = self.next_start(map, start_search_start); + if let Some(next_start) = maybe_next_start + && next_start <= next_end + { + let closing = self.close_at_end(next_start, map)?; + end_search_start = movement::right(map, closing); + start_search_start = movement::right(map, closing); + continue; + } else { + return Some(next_end); } - Self::Subword { ignore_punctuation } => { - let classifier = classifier.ignore_punctuation(ignore_punctuation); - Ok(movement::is_subword_start(left, right, &classifier) - || (is_buffer_start(left) && classifier.kind(right) != CharKind::Whitespace)) - } - Self::AngleBrackets => Ok(left == '<'), - Self::BackQuotes => Ok(left == '`'), - Self::CurlyBrackets => Ok(left == '{'), - Self::DoubleQuotes => Ok(left == '"'), - Self::Parentheses => Ok(left == '('), - Self::SquareBrackets => Ok(left == '['), - Self::VerticalBars => Ok(left == '|'), - _ => Err(UnboundedErr), } } - - fn helix_is_end( - self, - right: char, - left: char, - classifier: CharClassifier, - ) -> Result { - match self { - Self::Word { ignore_punctuation } => { - let classifier = classifier.ignore_punctuation(ignore_punctuation); - Ok(is_word_end(left, right, &classifier) - || (is_buffer_end(right) && classifier.kind(left) != CharKind::Whitespace)) + /// The previous start since `end` (inclusive) on the same nesting level. + fn close_at_start(&self, end: DisplayPoint, map: &DisplaySnapshot) -> Option { + if !self.can_be_nested() { + return self.previous_start(map, end); + } + let mut start_search_start = end; + let mut end_search_start = movement::left(map, end); + loop { + let prev_start = self.previous_start(map, start_search_start)?; + let maybe_prev_end = self.previous_end(map, end_search_start); + if let Some(prev_end) = maybe_prev_end + && prev_end >= prev_start + { + let closing = self.close_at_start(prev_end, map)?; + end_search_start = movement::left(map, closing); + start_search_start = movement::left(map, closing); + continue; + } else { + return Some(prev_start); } - Self::Subword { ignore_punctuation } => { - let classifier = classifier.ignore_punctuation(ignore_punctuation); - Ok(movement::is_subword_end(left, right, &classifier) - || (is_buffer_end(right) && classifier.kind(right) != CharKind::Whitespace)) - } - Self::AngleBrackets => Ok(right == '>'), - Self::BackQuotes => Ok(right == '`'), - Self::CurlyBrackets => Ok(right == '}'), - Self::DoubleQuotes => Ok(right == '"'), - Self::Parentheses => Ok(right == ')'), - Self::SquareBrackets => Ok(right == ']'), - Self::VerticalBars => Ok(right == '|'), - Self::Sentence => Ok(left == '.'), - _ => Err(UnboundedErr), } } } +impl HelixTextObject for B { + fn range( + &self, + map: &DisplaySnapshot, + relative_to: Range, + around: bool, + ) -> Option> { + let start = self.close_at_start(relative_to.start, map)?; + let end = self.close_at_end(start, map)?; + if end < relative_to.end { + return None; + } + + if around { + Some(self.surround(map, start..end)) + } else { + Some(start..end) + } + } + + fn next_range( + &self, + map: &DisplaySnapshot, + relative_to: Range, + around: bool, + ) -> Option> { + let start = self.next_start(map, relative_to.end)?; + let end = self.close_at_end(start, map)?; + let range = if around { + self.surround(map, start..end) + } else { + start..end + }; + + Some(range) + } + + fn previous_range( + &self, + map: &DisplaySnapshot, + relative_to: Range, + around: bool, + ) -> Option> { + let end = self.previous_end(map, relative_to.start)?; + let start = self.close_at_start(end, map)?; + let range = if around { + self.surround(map, start..end) + } else { + start..end + }; + + Some(range) + } +} + +/// A textobject whose boundaries can easily be found between two chars +pub enum ImmediateBoundary { + Word { ignore_punctuation: bool }, + Subword { ignore_punctuation: bool }, + AngleBrackets, + BackQuotes, + CurlyBrackets, + DoubleQuotes, + Parentheses, + SingleQuotes, + SquareBrackets, + VerticalBars, +} + +/// A textobject whose start and end can be found from an easy-to-find +/// boundary between two chars by following a simple path from there +pub enum FuzzyBoundary { + Sentence, + Paragraph, +} + +impl ImmediateBoundary { + fn is_start(&self, left: char, right: char, classifier: CharClassifier) -> bool { + match self { + Self::Word { ignore_punctuation } => { + let classifier = classifier.ignore_punctuation(*ignore_punctuation); + is_word_start(left, right, &classifier) + || (is_buffer_start(left) && classifier.kind(right) != CharKind::Whitespace) + } + Self::Subword { ignore_punctuation } => { + let classifier = classifier.ignore_punctuation(*ignore_punctuation); + movement::is_subword_start(left, right, &classifier) + || (is_buffer_start(left) && classifier.kind(right) != CharKind::Whitespace) + } + Self::AngleBrackets => left == '<', + Self::BackQuotes => left == '`', + Self::CurlyBrackets => left == '{', + Self::DoubleQuotes => left == '"', + Self::Parentheses => left == '(', + Self::SingleQuotes => left == '\'', + Self::SquareBrackets => left == '[', + Self::VerticalBars => left == '|', + } + } + + fn is_end(&self, left: char, right: char, classifier: CharClassifier) -> bool { + match self { + Self::Word { ignore_punctuation } => { + let classifier = classifier.ignore_punctuation(*ignore_punctuation); + is_word_end(left, right, &classifier) + || (is_buffer_end(right) && classifier.kind(left) != CharKind::Whitespace) + } + Self::Subword { ignore_punctuation } => { + let classifier = classifier.ignore_punctuation(*ignore_punctuation); + movement::is_subword_start(left, right, &classifier) + || (is_buffer_end(right) && classifier.kind(left) != CharKind::Whitespace) + } + Self::AngleBrackets => right == '>', + Self::BackQuotes => right == '`', + Self::CurlyBrackets => right == '}', + Self::DoubleQuotes => right == '"', + Self::Parentheses => right == ')', + Self::SingleQuotes => right == '\'', + Self::SquareBrackets => right == ']', + Self::VerticalBars => right == '|', + } + } +} + +impl BoundedObject for ImmediateBoundary { + fn next_start(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option { + try_find_boundary(map, from, |left, right| { + let classifier = map + .buffer_snapshot + .char_classifier_at(from.to_offset(map, Bias::Left)); + self.is_start(left, right, classifier) + }) + } + fn next_end(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option { + try_find_boundary(map, from, |left, right| { + let classifier = map + .buffer_snapshot + .char_classifier_at(from.to_offset(map, Bias::Left)); + self.is_end(left, right, classifier) + }) + } + fn previous_start(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option { + try_find_preceding_boundary(map, from, |left, right| { + let classifier = map + .buffer_snapshot + .char_classifier_at(from.to_offset(map, Bias::Left)); + self.is_start(left, right, classifier) + }) + } + fn previous_end(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option { + try_find_preceding_boundary(map, from, |left, right| { + let classifier = map + .buffer_snapshot + .char_classifier_at(from.to_offset(map, Bias::Left)); + self.is_end(left, right, classifier) + }) + } + fn surround( + &self, + map: &DisplaySnapshot, + inner_range: Range, + ) -> Range { + match self { + Self::AngleBrackets + | Self::BackQuotes + | Self::CurlyBrackets + | Self::DoubleQuotes + | Self::Parentheses + | Self::SingleQuotes + | Self::SquareBrackets + | Self::VerticalBars => { + movement::left(map, inner_range.start)..movement::right(map, inner_range.end) + } + Self::Subword { .. } | Self::Word { .. } => { + let row = inner_range.end.row(); + let line_start = DisplayPoint::new(row, 0); + let line_end = DisplayPoint::new(row, map.line_len(row)); + let next_start = self.next_start(map, inner_range.end).unwrap().min(line_end); + let prev_end = self + .previous_end(map, inner_range.start) + .unwrap() + .max(line_start); + if next_start > inner_range.end { + inner_range.start..next_start + } else { + prev_end..inner_range.end + } + } + } + } + fn can_be_nested(&self) -> bool { + match self { + Self::Subword { .. } | Self::Word { .. } => false, + _ => true, + } + } +} + +impl FuzzyBoundary { + /// When between two chars that form an easy-to-find identifier boundary, + /// what's the way to get to the actual start of the object, if any + fn is_near_potential_start<'a>( + &self, + left: char, + right: char, + classifier: CharClassifier, + ) -> Option Option>> { + if is_buffer_start(left) { + return Some(Box::new(|identifier, _| Some(identifier))); + } + match self { + Self::Paragraph => { + if left != '\n' || right != '\n' { + return None; + } + Some(Box::new(|identifier, map| { + try_find_boundary(map, identifier, |left, right| left == '\n' && right != '\n') + })) + } + Self::Sentence => { + if !is_sentence_end(left, right, &classifier) { + return None; + } + Some(Box::new(|identifier, map| { + let word = ImmediateBoundary::Word { + ignore_punctuation: false, + }; + word.next_start(map, identifier) + })) + } + } + } + /// When between two chars that form an easy-to-find identifier boundary, + /// what's the way to get to the actual end of the object, if any + fn is_near_potential_end<'a>( + &self, + left: char, + right: char, + classifier: CharClassifier, + ) -> Option Option>> { + if is_buffer_end(right) { + return Some(Box::new(|identifier, _| Some(identifier))); + } + match self { + Self::Paragraph => { + if left != '\n' || right != '\n' { + return None; + } + Some(Box::new(|identifier, map| { + try_find_preceding_boundary(map, identifier, |left, right| { + left != '\n' && right == '\n' + }) + })) + } + Self::Sentence => { + if !is_sentence_end(left, right, &classifier) { + return None; + } + Some(Box::new(|identifier, _| Some(identifier))) + } + } + } +} + +impl BoundedObject for FuzzyBoundary { + fn next_start(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option { + let mut previous_search_start = from; + while let Some((identifier, reach_start)) = + try_find_boundary_data(map, previous_search_start, |left, right, point| { + let classifier = map + .buffer_snapshot + .char_classifier_at(point.to_offset(map, Bias::Left)); + self.is_near_potential_start(left, right, classifier) + .map(|reach_start| (point, reach_start)) + }) + { + let Some(start) = reach_start(identifier, map) else { + continue; + }; + if start < from { + previous_search_start = movement::right(map, identifier); + } else { + return Some(start); + } + } + None + } + fn next_end(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option { + let mut previous_search_start = from; + while let Some((identifier, reach_end)) = + try_find_boundary_data(map, previous_search_start, |left, right, point| { + let classifier = map + .buffer_snapshot + .char_classifier_at(point.to_offset(map, Bias::Left)); + self.is_near_potential_end(left, right, classifier) + .map(|reach_end| (point, reach_end)) + }) + { + let Some(end) = reach_end(identifier, map) else { + continue; + }; + if end < from { + previous_search_start = movement::right(map, identifier); + } else { + return Some(end); + } + } + None + } + fn previous_start(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option { + let mut previous_search_start = from; + while let Some((identifier, reach_start)) = + try_find_preceding_boundary_data(map, previous_search_start, |left, right, point| { + let classifier = map + .buffer_snapshot + .char_classifier_at(point.to_offset(map, Bias::Left)); + self.is_near_potential_start(left, right, classifier) + .map(|reach_start| (point, reach_start)) + }) + { + let Some(start) = reach_start(identifier, map) else { + continue; + }; + if start > from { + previous_search_start = movement::left(map, identifier); + } else { + return Some(start); + } + } + None + } + fn previous_end(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option { + let mut previous_search_start = from; + while let Some((identifier, reach_end)) = + try_find_preceding_boundary_data(map, previous_search_start, |left, right, point| { + let classifier = map + .buffer_snapshot + .char_classifier_at(point.to_offset(map, Bias::Left)); + self.is_near_potential_end(left, right, classifier) + .map(|reach_end| (point, reach_end)) + }) + { + let Some(end) = reach_end(identifier, map) else { + continue; + }; + if end > from { + previous_search_start = movement::left(map, identifier); + } else { + return Some(end); + } + } + None + } + fn surround( + &self, + map: &DisplaySnapshot, + inner_range: Range, + ) -> Range { + let next_start = self + .next_start(map, inner_range.end) + .unwrap_or(map.max_point()); + if next_start > inner_range.end { + return inner_range.start..next_start; + } + let previous_end = self + .previous_end(map, inner_range.end) + .unwrap_or(DisplayPoint::zero()); + previous_end..inner_range.end + } + fn can_be_nested(&self) -> bool { + false + } +} + +/// Returns the first boundary after or at `from` in text direction. +/// The start and end of the file are the chars `'\0'`. fn try_find_boundary( map: &DisplaySnapshot, from: DisplayPoint, - mut is_boundary: impl FnMut(char, char) -> Result, -) -> Result, UnboundedErr> { + is_boundary: impl Fn(char, char) -> bool, +) -> Option { + let boundary = try_find_boundary_data(map, from, |left, right, point| { + if is_boundary(left, right) { + Some(point) + } else { + None + } + })?; + Some(boundary) +} + +/// Returns some information about it (of type `T`) as soon as +/// there is a boundary after or at `from` in text direction +/// The start and end of the file are the chars `'\0'`. +fn try_find_boundary_data( + map: &DisplaySnapshot, + from: DisplayPoint, + boundary_information: impl Fn(char, char, DisplayPoint) -> Option, +) -> Option { let mut offset = from.to_offset(map, Bias::Right); let mut prev_ch = map .buffer_snapshot @@ -148,37 +484,55 @@ fn try_find_boundary( .unwrap_or('\0'); for ch in map.buffer_snapshot.chars_at(offset).chain(['\0']) { - if is_boundary(prev_ch, ch)? { - return Ok(Some( - map.clip_point(offset.to_display_point(map), Bias::Right), - )); + let display_point = offset.to_display_point(map); + if let Some(boundary_information) = boundary_information(prev_ch, ch, display_point) { + return Some(boundary_information); } offset += ch.len_utf8(); prev_ch = ch; } - Ok(None) + None } +/// Returns the first boundary after or at `from` in text direction. +/// The start and end of the file are the chars `'\0'`. fn try_find_preceding_boundary( map: &DisplaySnapshot, from: DisplayPoint, - mut is_boundary: impl FnMut(char, char) -> Result, -) -> Result, UnboundedErr> { - let mut offset = from.to_offset(map, Bias::Right); + is_boundary: impl Fn(char, char) -> bool, +) -> Option { + let boundary = try_find_preceding_boundary_data(map, from, |left, right, point| { + if is_boundary(left, right) { + Some(point) + } else { + None + } + })?; + Some(boundary) +} + +/// Returns some information about it (of type `T`) as soon as +/// there is a boundary before or at `from` in opposite text direction +/// The start and end of the file are the chars `'\0'`. +fn try_find_preceding_boundary_data( + map: &DisplaySnapshot, + from: DisplayPoint, + is_boundary: impl Fn(char, char, DisplayPoint) -> Option, +) -> Option { + let mut offset = from.to_offset(map, Bias::Left); let mut prev_ch = map.buffer_snapshot.chars_at(offset).next().unwrap_or('\0'); for ch in map.buffer_snapshot.reversed_chars_at(offset).chain(['\0']) { - if is_boundary(ch, prev_ch)? { - return Ok(Some( - map.clip_point(offset.to_display_point(map), Bias::Right), - )); + let display_point = offset.to_display_point(map); + if let Some(boundary_information) = is_boundary(ch, prev_ch, display_point) { + return Some(boundary_information); } offset = offset.saturating_sub(ch.len_utf8()); prev_ch = ch; } - Ok(None) + None } fn is_buffer_start(left: char) -> bool { @@ -197,3 +551,12 @@ fn is_word_start(left: char, right: char, classifier: &CharClassifier) -> bool { fn is_word_end(left: char, right: char, classifier: &CharClassifier) -> bool { classifier.kind(left) != classifier.kind(right) && classifier.kind(left) != CharKind::Whitespace } + +fn is_sentence_end(left: char, right: char, classifier: &CharClassifier) -> bool { + const ENDS: [char; 1] = ['.']; + + if classifier.kind(right) != CharKind::Whitespace { + return false; + } + ENDS.into_iter().any(|end| left == end) +} diff --git a/crates/vim/src/helix/object.rs b/crates/vim/src/helix/object.rs index ce5a25057f..08ccdcdc8a 100644 --- a/crates/vim/src/helix/object.rs +++ b/crates/vim/src/helix/object.rs @@ -1,15 +1,38 @@ -use std::{cmp::Ordering, ops::Range}; +use std::ops::Range; -use editor::{ - DisplayPoint, - display_map::DisplaySnapshot, - movement::{self}, -}; +use editor::{DisplayPoint, display_map::DisplaySnapshot, movement}; use text::Selection; -use crate::{helix::boundary::UnboundedErr, object::Object}; +use crate::{ + helix::boundary::{FuzzyBoundary, ImmediateBoundary}, + object::Object as VimObject, +}; -impl Object { +/// A text object from helix or an extra one +pub trait HelixTextObject { + fn range( + &self, + map: &DisplaySnapshot, + relative_to: Range, + around: bool, + ) -> Option>; + + fn next_range( + &self, + map: &DisplaySnapshot, + relative_to: Range, + around: bool, + ) -> Option>; + + fn previous_range( + &self, + map: &DisplaySnapshot, + relative_to: Range, + around: bool, + ) -> Option>; +} + +impl VimObject { /// Returns the range of the object the cursor is over. /// Follows helix convention. pub fn helix_range( @@ -18,24 +41,19 @@ impl Object { selection: Selection, around: bool, ) -> Option> { - let relative_to = cursor_start(&selection, map); - if let Ok(selection) = self.current_bounded_object(map, relative_to) { - if around { - selection.map(|s| self.surround(map, s).unwrap()) - } else { - selection - } + let cursor = cursor_range(&selection, map); + if let Some(helix_object) = self.to_helix_object() { + helix_object.range(map, cursor, around) } else { let range = self.range(map, selection, around, None)?; - if range.start > relative_to { + if range.start > cursor.start { None } else { Some(range) } } } - /// Returns the range of the next object the cursor is not over. /// Follows helix convention. pub fn helix_next_range( @@ -44,24 +62,10 @@ impl Object { selection: Selection, around: bool, ) -> Option> { - let relative_to = cursor_start(&selection, map); - if let Ok(selection) = self.next_bounded_object(map, relative_to) { - if around { - selection.map(|s| self.surround(map, s).unwrap()) - } else { - selection - } - } else { - let range = self.range(map, selection, around, None)?; - - if range.start > relative_to { - Some(range) - } else { - None - } - } + let cursor = cursor_range(&selection, map); + let helix_object = self.to_helix_object()?; + helix_object.next_range(map, cursor, around) } - /// Returns the range of the previous object the cursor is not over. /// Follows helix convention. pub fn helix_previous_range( @@ -70,233 +74,42 @@ impl Object { selection: Selection, around: bool, ) -> Option> { - let relative_to = cursor_start(&selection, map); - if let Ok(selection) = self.previous_bounded_object(map, relative_to) { - if around { - selection.map(|s| self.surround(map, s).unwrap()) - } else { - selection - } - } else { - None - } + let cursor = cursor_range(&selection, map); + let helix_object = self.to_helix_object()?; + helix_object.previous_range(map, cursor, around) } +} - /// Returns the range of the object the cursor is over if it can be found with simple boundary checking. - /// Potentially none. Follows helix convention. - fn current_bounded_object( - self, - map: &DisplaySnapshot, - relative_to: DisplayPoint, - ) -> Result>, UnboundedErr> { - let Some(start) = self.helix_previous_start(map, relative_to)? else { - return Ok(None); - }; - let Some(end) = self.close_at_end(start, map)? else { - return Ok(None); - }; - - if end > relative_to { - return Ok(Some(start..end)); - } - - let Some(end) = self.helix_next_end(map, movement::right(map, relative_to))? else { - return Ok(None); - }; - let Some(start) = self.close_at_start(end, map)? else { - return Ok(None); - }; - - if start <= relative_to { - return Ok(Some(start..end)); - } - - Ok(None) - } - - /// Returns the range of the next object the cursor is not over if it can be found with simple boundary checking. - /// Potentially none. Follows helix convention. - fn next_bounded_object( - self, - map: &DisplaySnapshot, - relative_to: DisplayPoint, - ) -> Result>, UnboundedErr> { - let Some(next_start) = self.helix_next_start(map, movement::right(map, relative_to))? - else { - return Ok(None); - }; - let Some(end) = self.close_at_end(next_start, map)? else { - return Ok(None); - }; - - Ok(Some(next_start..end)) - } - - /// Returns the previous range of the object the cursor not is over if it can be found with simple boundary checking. - /// Potentially none. Follows helix convention. - fn previous_bounded_object( - self, - map: &DisplaySnapshot, - relative_to: DisplayPoint, - ) -> Result>, UnboundedErr> { - let Some(prev_end) = self.helix_previous_end(map, relative_to)? else { - return Ok(None); - }; - let Some(start) = self.close_at_start(prev_end, map)? else { - return Ok(None); - }; - - Ok(Some(start..prev_end)) - } - - /// Switches from an 'mi' range to an 'ma' range. Follows helix convention. - fn surround( - self, - map: &DisplaySnapshot, - selection: Range, - ) -> Result, UnboundedErr> { - match self { - Self::Word { .. } | Self::Subword { .. } => { - let row = selection.end.row(); - let line_start = DisplayPoint::new(row, 0); - let line_end = DisplayPoint::new(row, map.line_len(row)); - let next_start = self - .helix_next_start(map, selection.end) - .unwrap() - .unwrap() - .min(line_end); - let prev_end = self - .helix_previous_end(map, selection.start) - .unwrap() - .unwrap() - .max(line_start); - if next_start > selection.end { - Ok(selection.start..next_start) - } else { - Ok(prev_end..selection.end) - } +impl VimObject { + fn to_helix_object(self) -> Option> { + Some(match self { + Self::AngleBrackets => Box::new(ImmediateBoundary::AngleBrackets), + Self::BackQuotes => Box::new(ImmediateBoundary::BackQuotes), + Self::CurlyBrackets => Box::new(ImmediateBoundary::CurlyBrackets), + Self::DoubleQuotes => Box::new(ImmediateBoundary::DoubleQuotes), + Self::Paragraph => Box::new(FuzzyBoundary::Paragraph), + Self::Parentheses => Box::new(ImmediateBoundary::Parentheses), + Self::Quotes => Box::new(ImmediateBoundary::SingleQuotes), + Self::Sentence => Box::new(FuzzyBoundary::Sentence), + Self::SquareBrackets => Box::new(ImmediateBoundary::SquareBrackets), + Self::Subword { ignore_punctuation } => { + Box::new(ImmediateBoundary::Subword { ignore_punctuation }) } - Self::AngleBrackets - | Self::BackQuotes - | Self::CurlyBrackets - | Self::DoubleQuotes - | Self::Parentheses - | Self::SquareBrackets - | Self::VerticalBars => { - Ok(movement::left(map, selection.start)..movement::right(map, selection.end)) + Self::VerticalBars => Box::new(ImmediateBoundary::VerticalBars), + Self::Word { ignore_punctuation } => { + Box::new(ImmediateBoundary::Word { ignore_punctuation }) } - _ => Err(UnboundedErr), - } - } - - fn close_at_end( - self, - start: DisplayPoint, - map: &DisplaySnapshot, - ) -> Result, UnboundedErr> { - let mut last_start = movement::right(map, start); - let mut opened = 1; - while let Some(next_end) = self.helix_next_end(map, last_start)? { - if !self.can_be_nested() { - return Ok(Some(next_end)); - } - if let Some(next_start) = self.helix_next_start(map, last_start)? { - match next_start.cmp(&next_end) { - Ordering::Less => { - opened += 1; - last_start = movement::right(map, next_start); - continue; - } - Ordering::Equal if self.can_be_zero_width() => { - last_start = movement::right(map, next_start); - continue; - } - _ => (), - } - } - // When this is reached one opened object can be closed. - opened -= 1; - if opened == 0 { - return Ok(Some(next_end)); - } - last_start = movement::right(map, next_end); - } - Ok(None) - } - - fn close_at_start( - self, - end: DisplayPoint, - map: &DisplaySnapshot, - ) -> Result, UnboundedErr> { - let mut last_end = movement::left(map, end); - let mut opened = 1; - while let Some(previous_start) = self.helix_previous_start(map, last_end)? { - if !self.can_be_nested() { - return Ok(Some(previous_start)); - } - if let Some(previous_end) = self.helix_previous_end(map, last_end)? { - if previous_end > previous_start - || previous_end == previous_start && self.can_be_zero_width() - { - opened += 1; - last_end = movement::left(map, previous_end); - continue; - } - } - // When this is reached one opened object can be closed. - opened -= 1; - if opened == 0 { - return Ok(Some(previous_start)); - } - last_end = movement::left(map, previous_start); - } - Ok(None) - } - - const fn can_be_zero_width(&self) -> bool { - match self { - Self::AngleBrackets - | Self::AnyBrackets - | Self::AnyQuotes - | Self::BackQuotes - | Self::CurlyBrackets - | Self::DoubleQuotes - | Self::EntireFile - | Self::MiniBrackets - | Self::MiniQuotes - | Self::Parentheses - | Self::Quotes - | Self::SquareBrackets - | Self::VerticalBars => true, - _ => false, - } - } - - const fn can_be_nested(&self) -> bool { - match self { - Self::AngleBrackets - | Self::AnyBrackets - | Self::CurlyBrackets - | Self::MiniBrackets - | Self::Parentheses - | Self::SquareBrackets - | Self::AnyQuotes - | Self::Class - | Self::Method - | Self::Tag - | Self::Argument => true, - _ => false, - } + _ => return None, + }) } } /// Returns the start of the cursor of a selection, whether that is collapsed or not. -fn cursor_start(selection: &Selection, map: &DisplaySnapshot) -> DisplayPoint { +fn cursor_range(selection: &Selection, map: &DisplaySnapshot) -> Range { if selection.is_empty() | selection.reversed { - selection.head() + selection.head()..movement::right(map, selection.head()) } else { - movement::left(map, selection.head()) + movement::left(map, selection.head())..selection.head() } } From bbeb1ab02e650473845438db16f422d1027f3323 Mon Sep 17 00:00:00 2001 From: fantacell Date: Tue, 12 Aug 2025 23:29:41 +0200 Subject: [PATCH 14/28] Adjust to upstream changes --- crates/vim/src/helix/select.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/vim/src/helix/select.rs b/crates/vim/src/helix/select.rs index f266a40140..f6e9e44b43 100644 --- a/crates/vim/src/helix/select.rs +++ b/crates/vim/src/helix/select.rs @@ -14,7 +14,7 @@ impl Vim { cx: &mut Context, ) { self.stop_recording(cx); - self.update_editor(window, cx, |_, editor, window, cx| { + self.update_editor(cx, |_, editor, cx| { editor.change_selections(Default::default(), window, cx, |s| { s.move_with(|map, selection| { let Some(range) = object.helix_range(map, selection.clone(), around) else { @@ -37,7 +37,7 @@ impl Vim { cx: &mut Context, ) { self.stop_recording(cx); - self.update_editor(window, cx, |_, editor, window, cx| { + self.update_editor(cx, |_, editor, cx| { editor.change_selections(Default::default(), window, cx, |s| { s.move_with(|map, selection| { let Some(range) = object.helix_next_range(map, selection.clone(), around) @@ -61,7 +61,7 @@ impl Vim { cx: &mut Context, ) { self.stop_recording(cx); - self.update_editor(window, cx, |_, editor, window, cx| { + self.update_editor(cx, |_, editor, cx| { editor.change_selections(Default::default(), window, cx, |s| { s.move_with(|map, selection| { let Some(range) = object.helix_previous_range(map, selection.clone(), around) From 52103bc99a1f27f1475cf9ea64e816f05f1f9fa1 Mon Sep 17 00:00:00 2001 From: fantacell Date: Sat, 16 Aug 2025 15:44:39 +0200 Subject: [PATCH 15/28] Don't let sentences span across paragraphs --- crates/vim/src/helix/boundary.rs | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/crates/vim/src/helix/boundary.rs b/crates/vim/src/helix/boundary.rs index 1cfa784a86..86cb050874 100644 --- a/crates/vim/src/helix/boundary.rs +++ b/crates/vim/src/helix/boundary.rs @@ -282,7 +282,7 @@ impl FuzzyBoundary { &self, left: char, right: char, - classifier: CharClassifier, + classifier: &CharClassifier, ) -> Option Option>> { if is_buffer_start(left) { return Some(Box::new(|identifier, _| Some(identifier))); @@ -297,7 +297,11 @@ impl FuzzyBoundary { })) } Self::Sentence => { - if !is_sentence_end(left, right, &classifier) { + if let Some(find_paragraph_start) = + Self::Paragraph.is_near_potential_start(left, right, classifier) + { + return Some(find_paragraph_start); + } else if !is_sentence_end(left, right, classifier) { return None; } Some(Box::new(|identifier, map| { @@ -315,7 +319,7 @@ impl FuzzyBoundary { &self, left: char, right: char, - classifier: CharClassifier, + classifier: &CharClassifier, ) -> Option Option>> { if is_buffer_end(right) { return Some(Box::new(|identifier, _| Some(identifier))); @@ -332,7 +336,11 @@ impl FuzzyBoundary { })) } Self::Sentence => { - if !is_sentence_end(left, right, &classifier) { + if let Some(find_paragraph_end) = + Self::Paragraph.is_near_potential_end(left, right, classifier) + { + return Some(find_paragraph_end); + } else if !is_sentence_end(left, right, &classifier) { return None; } Some(Box::new(|identifier, _| Some(identifier))) @@ -349,7 +357,7 @@ impl BoundedObject for FuzzyBoundary { let classifier = map .buffer_snapshot .char_classifier_at(point.to_offset(map, Bias::Left)); - self.is_near_potential_start(left, right, classifier) + self.is_near_potential_start(left, right, &classifier) .map(|reach_start| (point, reach_start)) }) { @@ -371,7 +379,7 @@ impl BoundedObject for FuzzyBoundary { let classifier = map .buffer_snapshot .char_classifier_at(point.to_offset(map, Bias::Left)); - self.is_near_potential_end(left, right, classifier) + self.is_near_potential_end(left, right, &classifier) .map(|reach_end| (point, reach_end)) }) { @@ -393,7 +401,7 @@ impl BoundedObject for FuzzyBoundary { let classifier = map .buffer_snapshot .char_classifier_at(point.to_offset(map, Bias::Left)); - self.is_near_potential_start(left, right, classifier) + self.is_near_potential_start(left, right, &classifier) .map(|reach_start| (point, reach_start)) }) { @@ -415,7 +423,7 @@ impl BoundedObject for FuzzyBoundary { let classifier = map .buffer_snapshot .char_classifier_at(point.to_offset(map, Bias::Left)); - self.is_near_potential_end(left, right, classifier) + self.is_near_potential_end(left, right, &classifier) .map(|reach_end| (point, reach_end)) }) { From e45dafb2537482f13c31fe7cb03ec37e308ecdd6 Mon Sep 17 00:00:00 2001 From: fantacell Date: Tue, 19 Aug 2025 17:40:20 +0200 Subject: [PATCH 16/28] Fix clippy in boundary.rs --- crates/vim/src/helix/boundary.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/vim/src/helix/boundary.rs b/crates/vim/src/helix/boundary.rs index 86cb050874..4f5c6ea855 100644 --- a/crates/vim/src/helix/boundary.rs +++ b/crates/vim/src/helix/boundary.rs @@ -340,7 +340,7 @@ impl FuzzyBoundary { Self::Paragraph.is_near_potential_end(left, right, classifier) { return Some(find_paragraph_end); - } else if !is_sentence_end(left, right, &classifier) { + } else if !is_sentence_end(left, right, classifier) { return None; } Some(Box::new(|identifier, _| Some(identifier))) From e075100e976b7cad8b9a2c3da90220518f44c9e7 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 19 Aug 2025 11:28:10 -0600 Subject: [PATCH 17/28] Fix helix bindings --- assets/keymaps/vim.json | 111 ++++++++++++++++++++++++++-------------- 1 file changed, 73 insertions(+), 38 deletions(-) diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 6016175566..671cb58e89 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -32,34 +32,6 @@ "(": "vim::SentenceBackward", ")": "vim::SentenceForward", "|": "vim::GoToColumn", - "] ]": "vim::NextSectionStart", - "] [": "vim::NextSectionEnd", - "[ [": "vim::PreviousSectionStart", - "[ ]": "vim::PreviousSectionEnd", - "] m": "vim::NextMethodStart", - "] shift-m": "vim::NextMethodEnd", - "[ m": "vim::PreviousMethodStart", - "[ shift-m": "vim::PreviousMethodEnd", - "[ *": "vim::PreviousComment", - "[ /": "vim::PreviousComment", - "] *": "vim::NextComment", - "] /": "vim::NextComment", - "[ -": "vim::PreviousLesserIndent", - "[ +": "vim::PreviousGreaterIndent", - "[ =": "vim::PreviousSameIndent", - "] -": "vim::NextLesserIndent", - "] +": "vim::NextGreaterIndent", - "] =": "vim::NextSameIndent", - "] b": "pane::ActivateNextItem", - "[ b": "pane::ActivatePreviousItem", - "] shift-b": "pane::ActivateLastItem", - "[ shift-b": ["pane::ActivateItem", 0], - "] space": "vim::InsertEmptyLineBelow", - "[ space": "vim::InsertEmptyLineAbove", - "[ e": "editor::MoveLineUp", - "] e": "editor::MoveLineDown", - "[ f": "workspace::FollowNextCollaborator", - "] f": "workspace::FollowNextCollaborator", // Word motions "w": "vim::NextWordStart", @@ -83,10 +55,6 @@ "n": "vim::MoveToNextMatch", "shift-n": "vim::MoveToPreviousMatch", "%": "vim::Matching", - "] }": ["vim::UnmatchedForward", { "char": "}" }], - "[ {": ["vim::UnmatchedBackward", { "char": "{" }], - "] )": ["vim::UnmatchedForward", { "char": ")" }], - "[ (": ["vim::UnmatchedBackward", { "char": "(" }], "f": ["vim::PushFindForward", { "before": false, "multiline": false }], "t": ["vim::PushFindForward", { "before": true, "multiline": false }], "shift-f": ["vim::PushFindBackward", { "after": false, "multiline": false }], @@ -219,6 +187,43 @@ ".": "vim::Repeat" } }, + { + "context": "vim_mode == normal || vim_mode == visual", + "bindings": { + "] ]": "vim::NextSectionStart", + "] [": "vim::NextSectionEnd", + "[ [": "vim::PreviousSectionStart", + "[ ]": "vim::PreviousSectionEnd", + "] m": "vim::NextMethodStart", + "] shift-m": "vim::NextMethodEnd", + "[ m": "vim::PreviousMethodStart", + "[ shift-m": "vim::PreviousMethodEnd", + "[ *": "vim::PreviousComment", + "[ /": "vim::PreviousComment", + "] *": "vim::NextComment", + "] /": "vim::NextComment", + "[ -": "vim::PreviousLesserIndent", + "[ +": "vim::PreviousGreaterIndent", + "[ =": "vim::PreviousSameIndent", + "] -": "vim::NextLesserIndent", + "] +": "vim::NextGreaterIndent", + "] =": "vim::NextSameIndent", + "] b": "pane::ActivateNextItem", + "[ b": "pane::ActivatePreviousItem", + "] shift-b": "pane::ActivateLastItem", + "[ shift-b": ["pane::ActivateItem", 0], + "] space": "vim::InsertEmptyLineBelow", + "[ space": "vim::InsertEmptyLineAbove", + "[ e": "editor::MoveLineUp", + "] e": "editor::MoveLineDown", + "[ f": "workspace::FollowNextCollaborator", + "] f": "workspace::FollowNextCollaborator", + "] }": ["vim::UnmatchedForward", { "char": "}" }], + "[ {": ["vim::UnmatchedBackward", { "char": "{" }], + "] )": ["vim::UnmatchedForward", { "char": ")" }], + "[ (": ["vim::UnmatchedBackward", { "char": "(" }] + } + }, { "context": "vim_mode == normal", "bindings": { @@ -469,12 +474,6 @@ "alt-shift-c": "editor::AddSelectionAbove" } }, - { - "context": "vim_operator == helix_m", - "bindings": { - "m": "vim::Matching" - } - }, { "context": "vim_mode == insert && !(showing_code_actions || showing_completions)", "bindings": { @@ -572,6 +571,42 @@ "e": "vim::EntireFile" } }, + { + "context": "vim_operator == helix_m", + "bindings": { + "m": "vim::Matching" + } + }, + { + "context": "vim_operator == helix_next", + "bindings": { + "z": "vim::NextSectionStart", + "shift-z": "vim::NextSectionEnd", + "*": "vim::NextComment", + "/": "vim::NextComment", + "-": "vim::NextLesserIndent", + "+": "vim::NextGreaterIndent", + "=": "vim::NextSameIndent", + "b": "pane::ActivateNextItem", + "shift-b": "pane::ActivateLastItem", + "space": "vim::InsertEmptyLineBelow" + } + }, + { + "context": "vim_operator == helix_previous", + "bindings": { + "z": "vim::PreviousSectionStart", + "shift-z": "vim::PreviousSectionEnd", + "*": "vim::PreviousComment", + "/": "vim::PreviousComment", + "-": "vim::PreviousLesserIndent", + "+": "vim::PreviousGreaterIndent", + "=": "vim::PreviousSameIndent", + "b": "pane::ActivatePreviousItem", + "shift-b": ["pane::ActivateItem", 0], + "space": "vim::InsertEmptyLineAbove" + } + }, { "context": "vim_operator == c", "bindings": { From 320b715dcb60b5fde94cf50c1542d43a4b750c0b Mon Sep 17 00:00:00 2001 From: fantacell Date: Wed, 20 Aug 2025 12:02:20 +0200 Subject: [PATCH 18/28] Move a few more keybindings around --- assets/keymaps/vim.json | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 671cb58e89..6a0b8d58f5 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -221,7 +221,10 @@ "] }": ["vim::UnmatchedForward", { "char": "}" }], "[ {": ["vim::UnmatchedBackward", { "char": "{" }], "] )": ["vim::UnmatchedForward", { "char": ")" }], - "[ (": ["vim::UnmatchedBackward", { "char": "(" }] + "[ (": ["vim::UnmatchedBackward", { "char": "(" }], + // tree-sitter related commands + "[ x": "vim::SelectLargerSyntaxNode", + "] x": "vim::SelectSmallerSyntaxNode" } }, { @@ -254,9 +257,6 @@ "g w": "vim::PushRewrap", "g q": "vim::PushRewrap", "insert": "vim::InsertBefore", - // tree-sitter related commands - "[ x": "vim::SelectLargerSyntaxNode", - "] x": "vim::SelectSmallerSyntaxNode", "] d": "editor::GoToDiagnostic", "[ d": "editor::GoToPreviousDiagnostic", "] c": "editor::GoToHunk", @@ -322,10 +322,7 @@ "g w": "vim::Rewrap", "g ?": "vim::ConvertToRot13", // "g ?": "vim::ConvertToRot47", - "\"": "vim::PushRegister", - // tree-sitter related commands - "[ x": "editor::SelectLargerSyntaxNode", - "] x": "editor::SelectSmallerSyntaxNode" + "\"": "vim::PushRegister" } }, { @@ -418,13 +415,6 @@ "insert": "vim::InsertBefore", "alt-.": "vim::RepeatFind", "alt-s": ["editor::SplitSelectionIntoLines", { "keep_selections": true }], - // tree-sitter related commands - "[ x": "editor::SelectLargerSyntaxNode", - "] x": "editor::SelectSmallerSyntaxNode", - "] d": "editor::GoToDiagnostic", - "[ d": "editor::GoToPreviousDiagnostic", - "] c": "editor::GoToHunk", - "[ c": "editor::GoToPreviousHunk", // Goto mode "g n": "pane::ActivateNextItem", "g p": "pane::ActivatePreviousItem", @@ -589,6 +579,9 @@ "=": "vim::NextSameIndent", "b": "pane::ActivateNextItem", "shift-b": "pane::ActivateLastItem", + "x": "editor::SelectSmallerSyntaxNode", + "d": "editor::GoToDiagnostic", + "c": "editor::GoToHunk", "space": "vim::InsertEmptyLineBelow" } }, @@ -604,6 +597,9 @@ "=": "vim::PreviousSameIndent", "b": "pane::ActivatePreviousItem", "shift-b": ["pane::ActivateItem", 0], + "x": "editor::SelectLargerSyntaxNode", + "d": "editor::GoToPreviousDiagnostic", + "c": "editor::GoToPreviousHunk", "space": "vim::InsertEmptyLineAbove" } }, From c602172edb3bf83571a86df6c489758dd9132f39 Mon Sep 17 00:00:00 2001 From: fantacell Date: Thu, 21 Aug 2025 13:23:09 +0200 Subject: [PATCH 19/28] Make matching vim objects in helix mode fallible --- crates/vim/src/helix/object.rs | 53 +++++++++++++++++++++++----------- crates/vim/src/helix/select.rs | 15 +++++++--- 2 files changed, 47 insertions(+), 21 deletions(-) diff --git a/crates/vim/src/helix/object.rs b/crates/vim/src/helix/object.rs index 08ccdcdc8a..798cd7162e 100644 --- a/crates/vim/src/helix/object.rs +++ b/crates/vim/src/helix/object.rs @@ -1,4 +1,8 @@ -use std::ops::Range; +use std::{ + error::Error, + fmt::{self, Display}, + ops::Range, +}; use editor::{DisplayPoint, display_map::DisplaySnapshot, movement}; use text::Selection; @@ -40,18 +44,12 @@ impl VimObject { map: &DisplaySnapshot, selection: Selection, around: bool, - ) -> Option> { + ) -> Result>, VimToHelixError> { let cursor = cursor_range(&selection, map); if let Some(helix_object) = self.to_helix_object() { - helix_object.range(map, cursor, around) + Ok(helix_object.range(map, cursor, around)) } else { - let range = self.range(map, selection, around, None)?; - - if range.start > cursor.start { - None - } else { - Some(range) - } + Err(VimToHelixError) } } /// Returns the range of the next object the cursor is not over. @@ -61,10 +59,13 @@ impl VimObject { map: &DisplaySnapshot, selection: Selection, around: bool, - ) -> Option> { + ) -> Result>, VimToHelixError> { let cursor = cursor_range(&selection, map); - let helix_object = self.to_helix_object()?; - helix_object.next_range(map, cursor, around) + if let Some(helix_object) = self.to_helix_object() { + Ok(helix_object.next_range(map, cursor, around)) + } else { + Err(VimToHelixError) + } } /// Returns the range of the previous object the cursor is not over. /// Follows helix convention. @@ -73,13 +74,28 @@ impl VimObject { map: &DisplaySnapshot, selection: Selection, around: bool, - ) -> Option> { + ) -> Result>, VimToHelixError> { let cursor = cursor_range(&selection, map); - let helix_object = self.to_helix_object()?; - helix_object.previous_range(map, cursor, around) + if let Some(helix_object) = self.to_helix_object() { + Ok(helix_object.previous_range(map, cursor, around)) + } else { + Err(VimToHelixError) + } } } +#[derive(Debug)] +pub struct VimToHelixError; +impl Display for VimToHelixError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "Not all vim text objects have an implemented helix equivalent" + ) + } +} +impl Error for VimToHelixError {} + impl VimObject { fn to_helix_object(self) -> Option> { Some(match self { @@ -105,7 +121,10 @@ impl VimObject { } /// Returns the start of the cursor of a selection, whether that is collapsed or not. -fn cursor_range(selection: &Selection, map: &DisplaySnapshot) -> Range { +pub(crate) fn cursor_range( + selection: &Selection, + map: &DisplaySnapshot, +) -> Range { if selection.is_empty() | selection.reversed { selection.head()..movement::right(map, selection.head()) } else { diff --git a/crates/vim/src/helix/select.rs b/crates/vim/src/helix/select.rs index 7c3f14aa98..ece6512ef5 100644 --- a/crates/vim/src/helix/select.rs +++ b/crates/vim/src/helix/select.rs @@ -1,7 +1,7 @@ use text::SelectionGoal; use ui::{Context, Window}; -use crate::{Vim, object::Object}; +use crate::{Vim, helix::object::cursor_range, object::Object}; impl Vim { /// Selects the object each cursor is over. @@ -17,7 +17,13 @@ impl Vim { self.update_editor(cx, |_, editor, cx| { editor.change_selections(Default::default(), window, cx, |s| { s.move_with(|map, selection| { - let Some(range) = object.helix_range(map, selection.clone(), around) else { + let Some(range) = object + .helix_range(map, selection.clone(), around) + .unwrap_or({ + let vim_range = object.range(map, selection.clone(), around, None); + vim_range.filter(|r| r.start <= cursor_range(selection, map).start) + }) + else { return; }; @@ -40,7 +46,7 @@ impl Vim { self.update_editor(cx, |_, editor, cx| { editor.change_selections(Default::default(), window, cx, |s| { s.move_with(|map, selection| { - let Some(range) = object.helix_next_range(map, selection.clone(), around) + let Ok(Some(range)) = object.helix_next_range(map, selection.clone(), around) else { return; }; @@ -64,7 +70,8 @@ impl Vim { self.update_editor(cx, |_, editor, cx| { editor.change_selections(Default::default(), window, cx, |s| { s.move_with(|map, selection| { - let Some(range) = object.helix_previous_range(map, selection.clone(), around) + let Ok(Some(range)) = + object.helix_previous_range(map, selection.clone(), around) else { return; }; From 62fc82b86f267e4f658340a0d185f5a419b9c8a3 Mon Sep 17 00:00:00 2001 From: fantacell Date: Thu, 21 Aug 2025 14:09:04 +0200 Subject: [PATCH 20/28] Switch the cursor side of the selection after a [ motion --- crates/vim/src/helix/select.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/vim/src/helix/select.rs b/crates/vim/src/helix/select.rs index ece6512ef5..d782e8b450 100644 --- a/crates/vim/src/helix/select.rs +++ b/crates/vim/src/helix/select.rs @@ -76,7 +76,7 @@ impl Vim { return; }; - selection.set_head_tail(range.end, range.start, SelectionGoal::None); + selection.set_head_tail(range.start, range.end, SelectionGoal::None); }); }); }); From 94a045caf527735b511a964c9656ca0481bd297e Mon Sep 17 00:00:00 2001 From: fantacell Date: Fri, 22 Aug 2025 15:28:22 +0200 Subject: [PATCH 21/28] Count the "around range" of an object as part of the object, unless it is only whitespace --- crates/vim/src/helix/boundary.rs | 516 ++++++++++++++++++++++--------- 1 file changed, 367 insertions(+), 149 deletions(-) diff --git a/crates/vim/src/helix/boundary.rs b/crates/vim/src/helix/boundary.rs index 4f5c6ea855..ee592b53eb 100644 --- a/crates/vim/src/helix/boundary.rs +++ b/crates/vim/src/helix/boundary.rs @@ -15,38 +15,123 @@ use crate::helix::object::HelixTextObject; /// until a start / end is found trait BoundedObject { /// The next start since `from` (inclusive). - fn next_start(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option; + /// If outer is true it is the start of "a" object (m a) rather than "inner" object (m i). + fn next_start( + &self, + map: &DisplaySnapshot, + from: DisplayPoint, + outer: bool, + ) -> Option; /// The next end since `from` (inclusive). - fn next_end(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option; + /// If outer is true it is the end of "a" object (m a) rather than "inner" object (m i). + fn next_end( + &self, + map: &DisplaySnapshot, + from: DisplayPoint, + outer: bool, + ) -> Option; /// The previous start since `from` (inclusive). - fn previous_start(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option; + /// If outer is true it is the start of "a" object (m a) rather than "inner" object (m i). + fn previous_start( + &self, + map: &DisplaySnapshot, + from: DisplayPoint, + outer: bool, + ) -> Option; /// The previous end since `from` (inclusive). - fn previous_end(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option; - /// Switches from an 'mi' range to an 'ma' range. Follows helix convention. - fn surround( + /// If outer is true it is the end of "a" object (m a) rather than "inner" object (m i). + fn previous_end( + &self, + map: &DisplaySnapshot, + from: DisplayPoint, + outer: bool, + ) -> Option; + + /// Whether the range inside or outside the object can have be zero characters wide. + /// If so, the trait assumes that these ranges can't be directly adjacent to each other. + fn can_be_zero_width(&self, around: bool) -> bool; + /// Whether the "ma" can exceed the "mi" range on both sides at the same time + fn surround_on_both_sides(&self) -> bool; + + /// Switches from an "mi" range to an "ma" one. + /// Assumes the inner range is valid. + fn around( &self, map: &DisplaySnapshot, inner_range: Range, - ) -> Range; - /// Whether these objects can be inside ones of the same kind. - /// If so, the trait assumes they can have zero width. - fn can_be_nested(&self) -> bool; - /// The next end since `start` (inclusive) on the same nesting level. - fn close_at_end(&self, start: DisplayPoint, map: &DisplaySnapshot) -> Option { - if !self.can_be_nested() { - return self.next_end(map, movement::right(map, start)); + ) -> Range { + if self.surround_on_both_sides() { + let start = self + .previous_start(map, inner_range.start, true) + .unwrap_or(inner_range.start); + let end = self + .next_end(map, inner_range.end, true) + .unwrap_or(inner_range.end); + + return start..end; } - let mut end_search_start = start; + + let mut start = inner_range.start; + let end = self + .next_end(map, inner_range.end, true) + .unwrap_or(inner_range.end); + if end == inner_range.end { + start = self + .previous_start(map, inner_range.start, true) + .unwrap_or(inner_range.start) + } + + start..end + } + /// Switches from an "ma" range to an "mi" one. + /// Assumes the inner range is valid. + fn inside( + &self, + map: &DisplaySnapshot, + outer_range: Range, + ) -> Range { + let inner_start = self + .next_start(map, outer_range.start, false) + .unwrap_or_else(|| { + todo!(); + outer_range.start + }); + let inner_end = self + .previous_end(map, outer_range.end, false) + .unwrap_or_else(|| { + todo!(); + outer_range.end + }); + inner_start..inner_end + } + + /// The next end since `start` (inclusive) on the same nesting level. + fn close_at_end( + &self, + start: DisplayPoint, + map: &DisplaySnapshot, + outer: bool, + ) -> Option { + let mut end_search_start = if self.can_be_zero_width(outer) { + start + } else { + movement::right(map, start) + }; let mut start_search_start = movement::right(map, start); loop { - let next_end = self.next_end(map, end_search_start)?; - let maybe_next_start = self.next_start(map, start_search_start); + let next_end = self.next_end(map, end_search_start, outer)?; + let maybe_next_start = self.next_start(map, start_search_start, outer); if let Some(next_start) = maybe_next_start - && next_start <= next_end + && ((next_start < next_end) + || (next_start == next_end && self.can_be_zero_width(outer))) { - let closing = self.close_at_end(next_start, map)?; + let closing = self.close_at_end(next_start, map, outer)?; end_search_start = movement::right(map, closing); - start_search_start = movement::right(map, closing); + start_search_start = if self.can_be_zero_width(outer) { + movement::right(map, closing) + } else { + closing + }; continue; } else { return Some(next_end); @@ -54,20 +139,31 @@ trait BoundedObject { } } /// The previous start since `end` (inclusive) on the same nesting level. - fn close_at_start(&self, end: DisplayPoint, map: &DisplaySnapshot) -> Option { - if !self.can_be_nested() { - return self.previous_start(map, end); - } - let mut start_search_start = end; + fn close_at_start( + &self, + end: DisplayPoint, + map: &DisplaySnapshot, + outer: bool, + ) -> Option { + let mut start_search_start = if self.can_be_zero_width(outer) { + end + } else { + movement::left(map, end) + }; let mut end_search_start = movement::left(map, end); loop { - let prev_start = self.previous_start(map, start_search_start)?; - let maybe_prev_end = self.previous_end(map, end_search_start); + let prev_start = self.previous_start(map, start_search_start, outer)?; + let maybe_prev_end = self.previous_end(map, end_search_start, outer); if let Some(prev_end) = maybe_prev_end - && prev_end >= prev_start + && ((prev_end > prev_start) + || (prev_end == prev_start && self.can_be_zero_width(outer))) { - let closing = self.close_at_start(prev_end, map)?; - end_search_start = movement::left(map, closing); + let closing = self.close_at_start(prev_end, map, outer)?; + end_search_start = if self.can_be_zero_width(outer) { + movement::left(map, closing) + } else { + closing + }; start_search_start = movement::left(map, closing); continue; } else { @@ -84,16 +180,29 @@ impl HelixTextObject for B { relative_to: Range, around: bool, ) -> Option> { - let start = self.close_at_start(relative_to.start, map)?; - let end = self.close_at_end(start, map)?; - if end < relative_to.end { + let search_start = if self.can_be_zero_width(true) { + relative_to.start + } else { + // If the objects can be directly next to each other an object start at the + // cursor (relative_to) start would not count for close_at_start, so the search + // needs to start one character to the left. + movement::right(map, relative_to.start) + }; + let min_start = self.close_at_start(search_start, map, self.surround_on_both_sides())?; + let max_end = self.close_at_end(min_start, map, self.surround_on_both_sides())?; + + if max_end < relative_to.end { return None; } - if around { - Some(self.surround(map, start..end)) + if around && !self.surround_on_both_sides() { + // max_end is not yet the outer end + Some(self.around(map, min_start..max_end)) + } else if !around && self.surround_on_both_sides() { + // max_end is the outer end, but the final result should have the inner end + Some(self.inside(map, min_start..max_end)) } else { - Some(start..end) + Some(min_start..max_end) } } @@ -103,15 +212,18 @@ impl HelixTextObject for B { relative_to: Range, around: bool, ) -> Option> { - let start = self.next_start(map, relative_to.end)?; - let end = self.close_at_end(start, map)?; - let range = if around { - self.surround(map, start..end) - } else { - start..end - }; + let min_start = self.next_start(map, relative_to.end, self.surround_on_both_sides())?; + let max_end = self.close_at_end(min_start, map, self.surround_on_both_sides())?; - Some(range) + if around && !self.surround_on_both_sides() { + // max_end is not yet the outer end + Some(self.around(map, min_start..max_end)) + } else if !around && self.surround_on_both_sides() { + // max_end is the outer end, but the final result should have the inner end + Some(self.inside(map, min_start..max_end)) + } else { + Some(min_start..max_end) + } } fn previous_range( @@ -120,15 +232,18 @@ impl HelixTextObject for B { relative_to: Range, around: bool, ) -> Option> { - let end = self.previous_end(map, relative_to.start)?; - let start = self.close_at_start(end, map)?; - let range = if around { - self.surround(map, start..end) - } else { - start..end - }; + let max_end = self.previous_end(map, relative_to.start, self.surround_on_both_sides())?; + let min_start = self.close_at_start(max_end, map, self.surround_on_both_sides())?; - Some(range) + if around && !self.surround_on_both_sides() { + // max_end is not yet the outer end + Some(self.around(map, min_start..max_end)) + } else if !around && self.surround_on_both_sides() { + // max_end is the outer end, but the final result should have the inner end + Some(self.inside(map, min_start..max_end)) + } else { + Some(min_start..max_end) + } } } @@ -154,7 +269,7 @@ pub enum FuzzyBoundary { } impl ImmediateBoundary { - fn is_start(&self, left: char, right: char, classifier: CharClassifier) -> bool { + fn is_inner_start(&self, left: char, right: char, classifier: CharClassifier) -> bool { match self { Self::Word { ignore_punctuation } => { let classifier = classifier.ignore_punctuation(*ignore_punctuation); @@ -176,8 +291,7 @@ impl ImmediateBoundary { Self::VerticalBars => left == '|', } } - - fn is_end(&self, left: char, right: char, classifier: CharClassifier) -> bool { + fn is_inner_end(&self, left: char, right: char, classifier: CharClassifier) -> bool { match self { Self::Word { ignore_punctuation } => { let classifier = classifier.ignore_punctuation(*ignore_punctuation); @@ -199,75 +313,118 @@ impl ImmediateBoundary { Self::VerticalBars => right == '|', } } + fn is_outer_start(&self, left: char, right: char, classifier: CharClassifier) -> bool { + match self { + word @ Self::Word { .. } => word.is_inner_end(left, right, classifier) || left == '\n', + subword @ Self::Subword { .. } => { + subword.is_inner_end(left, right, classifier) || left == '\n' + } + Self::AngleBrackets => right == '<', + Self::BackQuotes => right == '`', + Self::CurlyBrackets => right == '{', + Self::DoubleQuotes => right == '"', + Self::Parentheses => right == '(', + Self::SingleQuotes => right == '\'', + Self::SquareBrackets => right == '[', + Self::VerticalBars => right == '|', + } + } + fn is_outer_end(&self, left: char, right: char, classifier: CharClassifier) -> bool { + match self { + word @ Self::Word { .. } => { + word.is_inner_start(left, right, classifier) || right == '\n' + } + subword @ Self::Subword { .. } => { + subword.is_inner_start(left, right, classifier) || right == '\n' + } + Self::AngleBrackets => left == '>', + Self::BackQuotes => left == '`', + Self::CurlyBrackets => left == '}', + Self::DoubleQuotes => left == '"', + Self::Parentheses => left == ')', + Self::SingleQuotes => left == '\'', + Self::SquareBrackets => left == ']', + Self::VerticalBars => left == '|', + } + } } impl BoundedObject for ImmediateBoundary { - fn next_start(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option { - try_find_boundary(map, from, |left, right| { - let classifier = map - .buffer_snapshot - .char_classifier_at(from.to_offset(map, Bias::Left)); - self.is_start(left, right, classifier) - }) - } - fn next_end(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option { - try_find_boundary(map, from, |left, right| { - let classifier = map - .buffer_snapshot - .char_classifier_at(from.to_offset(map, Bias::Left)); - self.is_end(left, right, classifier) - }) - } - fn previous_start(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option { - try_find_preceding_boundary(map, from, |left, right| { - let classifier = map - .buffer_snapshot - .char_classifier_at(from.to_offset(map, Bias::Left)); - self.is_start(left, right, classifier) - }) - } - fn previous_end(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option { - try_find_preceding_boundary(map, from, |left, right| { - let classifier = map - .buffer_snapshot - .char_classifier_at(from.to_offset(map, Bias::Left)); - self.is_end(left, right, classifier) - }) - } - fn surround( + fn next_start( &self, map: &DisplaySnapshot, - inner_range: Range, - ) -> Range { + from: DisplayPoint, + outer: bool, + ) -> Option { + try_find_boundary(map, from, |left, right| { + let classifier = map + .buffer_snapshot + .char_classifier_at(from.to_offset(map, Bias::Left)); + if outer { + self.is_outer_start(left, right, classifier) + } else { + self.is_inner_start(left, right, classifier) + } + }) + } + fn next_end( + &self, + map: &DisplaySnapshot, + from: DisplayPoint, + outer: bool, + ) -> Option { + try_find_boundary(map, from, |left, right| { + let classifier = map + .buffer_snapshot + .char_classifier_at(from.to_offset(map, Bias::Left)); + if outer { + self.is_outer_end(left, right, classifier) + } else { + self.is_inner_end(left, right, classifier) + } + }) + } + fn previous_start( + &self, + map: &DisplaySnapshot, + from: DisplayPoint, + outer: bool, + ) -> Option { + try_find_preceding_boundary(map, from, |left, right| { + let classifier = map + .buffer_snapshot + .char_classifier_at(from.to_offset(map, Bias::Left)); + if outer { + self.is_outer_start(left, right, classifier) + } else { + self.is_inner_start(left, right, classifier) + } + }) + } + fn previous_end( + &self, + map: &DisplaySnapshot, + from: DisplayPoint, + outer: bool, + ) -> Option { + try_find_preceding_boundary(map, from, |left, right| { + let classifier = map + .buffer_snapshot + .char_classifier_at(from.to_offset(map, Bias::Left)); + if outer { + self.is_outer_end(left, right, classifier) + } else { + self.is_inner_end(left, right, classifier) + } + }) + } + fn can_be_zero_width(&self, around: bool) -> bool { match self { - Self::AngleBrackets - | Self::BackQuotes - | Self::CurlyBrackets - | Self::DoubleQuotes - | Self::Parentheses - | Self::SingleQuotes - | Self::SquareBrackets - | Self::VerticalBars => { - movement::left(map, inner_range.start)..movement::right(map, inner_range.end) - } - Self::Subword { .. } | Self::Word { .. } => { - let row = inner_range.end.row(); - let line_start = DisplayPoint::new(row, 0); - let line_end = DisplayPoint::new(row, map.line_len(row)); - let next_start = self.next_start(map, inner_range.end).unwrap().min(line_end); - let prev_end = self - .previous_end(map, inner_range.start) - .unwrap() - .max(line_start); - if next_start > inner_range.end { - inner_range.start..next_start - } else { - prev_end..inner_range.end - } - } + Self::Subword { .. } | Self::Word { .. } => false, + _ => !around, } } - fn can_be_nested(&self) -> bool { + fn surround_on_both_sides(&self) -> bool { match self { Self::Subword { .. } | Self::Word { .. } => false, _ => true, @@ -278,7 +435,7 @@ impl BoundedObject for ImmediateBoundary { impl FuzzyBoundary { /// When between two chars that form an easy-to-find identifier boundary, /// what's the way to get to the actual start of the object, if any - fn is_near_potential_start<'a>( + fn is_near_potential_inner_start<'a>( &self, left: char, right: char, @@ -298,7 +455,7 @@ impl FuzzyBoundary { } Self::Sentence => { if let Some(find_paragraph_start) = - Self::Paragraph.is_near_potential_start(left, right, classifier) + Self::Paragraph.is_near_potential_inner_start(left, right, classifier) { return Some(find_paragraph_start); } else if !is_sentence_end(left, right, classifier) { @@ -308,14 +465,14 @@ impl FuzzyBoundary { let word = ImmediateBoundary::Word { ignore_punctuation: false, }; - word.next_start(map, identifier) + word.next_start(map, identifier, false) })) } } } /// When between two chars that form an easy-to-find identifier boundary, /// what's the way to get to the actual end of the object, if any - fn is_near_potential_end<'a>( + fn is_near_potential_inner_end<'a>( &self, left: char, right: char, @@ -337,7 +494,7 @@ impl FuzzyBoundary { } Self::Sentence => { if let Some(find_paragraph_end) = - Self::Paragraph.is_near_potential_end(left, right, classifier) + Self::Paragraph.is_near_potential_inner_end(left, right, classifier) { return Some(find_paragraph_end); } else if !is_sentence_end(left, right, classifier) { @@ -347,18 +504,62 @@ impl FuzzyBoundary { } } } + /// When between two chars that form an easy-to-find identifier boundary, + /// what's the way to get to the actual end of the object, if any + fn is_near_potential_outer_start<'a>( + &self, + left: char, + right: char, + classifier: &CharClassifier, + ) -> Option Option>> { + match self { + paragraph @ Self::Paragraph => { + paragraph.is_near_potential_inner_end(left, right, classifier) + } + sentence @ Self::Sentence => { + sentence.is_near_potential_inner_end(left, right, classifier) + } + } + } + /// When between two chars that form an easy-to-find identifier boundary, + /// what's the way to get to the actual end of the object, if any + fn is_near_potential_outer_end<'a>( + &self, + left: char, + right: char, + classifier: &CharClassifier, + ) -> Option Option>> { + match self { + paragraph @ Self::Paragraph => { + paragraph.is_near_potential_inner_start(left, right, classifier) + } + sentence @ Self::Sentence => { + sentence.is_near_potential_inner_start(left, right, classifier) + } + } + } } impl BoundedObject for FuzzyBoundary { - fn next_start(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option { + fn next_start( + &self, + map: &DisplaySnapshot, + from: DisplayPoint, + outer: bool, + ) -> Option { let mut previous_search_start = from; while let Some((identifier, reach_start)) = try_find_boundary_data(map, previous_search_start, |left, right, point| { let classifier = map .buffer_snapshot .char_classifier_at(point.to_offset(map, Bias::Left)); - self.is_near_potential_start(left, right, &classifier) - .map(|reach_start| (point, reach_start)) + if outer { + self.is_near_potential_outer_start(left, right, &classifier) + .map(|reach_start| (point, reach_start)) + } else { + self.is_near_potential_inner_start(left, right, &classifier) + .map(|reach_start| (point, reach_start)) + } }) { let Some(start) = reach_start(identifier, map) else { @@ -372,15 +573,25 @@ impl BoundedObject for FuzzyBoundary { } None } - fn next_end(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option { + fn next_end( + &self, + map: &DisplaySnapshot, + from: DisplayPoint, + outer: bool, + ) -> Option { let mut previous_search_start = from; while let Some((identifier, reach_end)) = try_find_boundary_data(map, previous_search_start, |left, right, point| { let classifier = map .buffer_snapshot .char_classifier_at(point.to_offset(map, Bias::Left)); - self.is_near_potential_end(left, right, &classifier) - .map(|reach_end| (point, reach_end)) + if outer { + self.is_near_potential_outer_end(left, right, &classifier) + .map(|reach_end| (point, reach_end)) + } else { + self.is_near_potential_inner_end(left, right, &classifier) + .map(|reach_end| (point, reach_end)) + } }) { let Some(end) = reach_end(identifier, map) else { @@ -394,15 +605,25 @@ impl BoundedObject for FuzzyBoundary { } None } - fn previous_start(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option { + fn previous_start( + &self, + map: &DisplaySnapshot, + from: DisplayPoint, + outer: bool, + ) -> Option { let mut previous_search_start = from; while let Some((identifier, reach_start)) = try_find_preceding_boundary_data(map, previous_search_start, |left, right, point| { let classifier = map .buffer_snapshot .char_classifier_at(point.to_offset(map, Bias::Left)); - self.is_near_potential_start(left, right, &classifier) - .map(|reach_start| (point, reach_start)) + if outer { + self.is_near_potential_outer_start(left, right, &classifier) + .map(|reach_start| (point, reach_start)) + } else { + self.is_near_potential_inner_start(left, right, &classifier) + .map(|reach_start| (point, reach_start)) + } }) { let Some(start) = reach_start(identifier, map) else { @@ -416,15 +637,25 @@ impl BoundedObject for FuzzyBoundary { } None } - fn previous_end(&self, map: &DisplaySnapshot, from: DisplayPoint) -> Option { + fn previous_end( + &self, + map: &DisplaySnapshot, + from: DisplayPoint, + outer: bool, + ) -> Option { let mut previous_search_start = from; while let Some((identifier, reach_end)) = try_find_preceding_boundary_data(map, previous_search_start, |left, right, point| { let classifier = map .buffer_snapshot .char_classifier_at(point.to_offset(map, Bias::Left)); - self.is_near_potential_end(left, right, &classifier) - .map(|reach_end| (point, reach_end)) + if outer { + self.is_near_potential_outer_end(left, right, &classifier) + .map(|reach_end| (point, reach_end)) + } else { + self.is_near_potential_inner_end(left, right, &classifier) + .map(|reach_end| (point, reach_end)) + } }) { let Some(end) = reach_end(identifier, map) else { @@ -438,23 +669,10 @@ impl BoundedObject for FuzzyBoundary { } None } - fn surround( - &self, - map: &DisplaySnapshot, - inner_range: Range, - ) -> Range { - let next_start = self - .next_start(map, inner_range.end) - .unwrap_or(map.max_point()); - if next_start > inner_range.end { - return inner_range.start..next_start; - } - let previous_end = self - .previous_end(map, inner_range.end) - .unwrap_or(DisplayPoint::zero()); - previous_end..inner_range.end + fn can_be_zero_width(&self, _: bool) -> bool { + false } - fn can_be_nested(&self) -> bool { + fn surround_on_both_sides(&self) -> bool { false } } From 5133db60abfec811bef039b209c2b47ff4a19a9e Mon Sep 17 00:00:00 2001 From: fantacell Date: Sun, 24 Aug 2025 22:25:12 +0200 Subject: [PATCH 22/28] Fix a bug where the next fuzzy boundary is not actually the closest one --- crates/vim/src/helix/boundary.rs | 183 ++++++++++++++----------------- 1 file changed, 82 insertions(+), 101 deletions(-) diff --git a/crates/vim/src/helix/boundary.rs b/crates/vim/src/helix/boundary.rs index ee592b53eb..ccaefc072b 100644 --- a/crates/vim/src/helix/boundary.rs +++ b/crates/vim/src/helix/boundary.rs @@ -1,10 +1,11 @@ -use std::ops::Range; +use std::{cmp::Ordering, ops::Range}; use editor::{ DisplayPoint, display_map::{DisplaySnapshot, ToDisplayPoint}, movement, }; +use itertools::Itertools; use language::{CharClassifier, CharKind}; use text::Bias; @@ -538,6 +539,82 @@ impl FuzzyBoundary { } } } + + // The boundary can be on the other side of `from` than the identifier, so the search needs to go both ways. + // Also, the distance (and direction) between identifier and boundary could vary, so a few ones need to be + // compared, even if one boundary was already found on the right side of `from`. + fn to_boundary( + &self, + map: &DisplaySnapshot, + from: DisplayPoint, + outer: bool, + backward: bool, + boundary_kind: Boundary, + ) -> Option { + let generate_boundary_data = |left, right, point: DisplayPoint| { + let classifier = map + .buffer_snapshot + .char_classifier_at(point.to_offset(map, Bias::Left)); + let reach_boundary = if outer && boundary_kind == Boundary::Start { + self.is_near_potential_outer_start(left, right, &classifier) + } else if !outer && boundary_kind == Boundary::Start { + self.is_near_potential_inner_start(left, right, &classifier) + } else if outer && boundary_kind == Boundary::End { + self.is_near_potential_outer_end(left, right, &classifier) + } else { + self.is_near_potential_inner_end(left, right, &classifier) + }; + + reach_boundary.map(|reach_start| (point, reach_start)) + }; + + let forwards = std::iter::successors( + try_find_boundary_data(map, from, generate_boundary_data), + |(previous_identifier, _)| { + if *previous_identifier == map.max_point() { + return None; + } + try_find_boundary_data( + map, + movement::right(map, *previous_identifier), + generate_boundary_data, + ) + }, + ); + let backwards = std::iter::successors( + try_find_preceding_boundary_data(map, from, generate_boundary_data), + |(previous_identifier, _)| { + if *previous_identifier == DisplayPoint::zero() { + return None; + } + try_find_preceding_boundary_data( + map, + movement::left(map, *previous_identifier), + generate_boundary_data, + ) + }, + ); + let boundaries = forwards + .interleave(backwards) + .take(4) + .filter_map(|(identifier, reach_boundary)| reach_boundary(identifier, map)) + .filter(|boundary| match boundary.cmp(&from) { + Ordering::Equal => true, + Ordering::Less => backward, + Ordering::Greater => !backward, + }); + if backward { + boundaries.max() + } else { + boundaries.min() + } + } +} + +#[derive(PartialEq)] +enum Boundary { + Start, + End, } impl BoundedObject for FuzzyBoundary { @@ -547,31 +624,7 @@ impl BoundedObject for FuzzyBoundary { from: DisplayPoint, outer: bool, ) -> Option { - let mut previous_search_start = from; - while let Some((identifier, reach_start)) = - try_find_boundary_data(map, previous_search_start, |left, right, point| { - let classifier = map - .buffer_snapshot - .char_classifier_at(point.to_offset(map, Bias::Left)); - if outer { - self.is_near_potential_outer_start(left, right, &classifier) - .map(|reach_start| (point, reach_start)) - } else { - self.is_near_potential_inner_start(left, right, &classifier) - .map(|reach_start| (point, reach_start)) - } - }) - { - let Some(start) = reach_start(identifier, map) else { - continue; - }; - if start < from { - previous_search_start = movement::right(map, identifier); - } else { - return Some(start); - } - } - None + self.to_boundary(map, from, outer, false, Boundary::Start) } fn next_end( &self, @@ -579,31 +632,7 @@ impl BoundedObject for FuzzyBoundary { from: DisplayPoint, outer: bool, ) -> Option { - let mut previous_search_start = from; - while let Some((identifier, reach_end)) = - try_find_boundary_data(map, previous_search_start, |left, right, point| { - let classifier = map - .buffer_snapshot - .char_classifier_at(point.to_offset(map, Bias::Left)); - if outer { - self.is_near_potential_outer_end(left, right, &classifier) - .map(|reach_end| (point, reach_end)) - } else { - self.is_near_potential_inner_end(left, right, &classifier) - .map(|reach_end| (point, reach_end)) - } - }) - { - let Some(end) = reach_end(identifier, map) else { - continue; - }; - if end < from { - previous_search_start = movement::right(map, identifier); - } else { - return Some(end); - } - } - None + self.to_boundary(map, from, outer, false, Boundary::End) } fn previous_start( &self, @@ -611,31 +640,7 @@ impl BoundedObject for FuzzyBoundary { from: DisplayPoint, outer: bool, ) -> Option { - let mut previous_search_start = from; - while let Some((identifier, reach_start)) = - try_find_preceding_boundary_data(map, previous_search_start, |left, right, point| { - let classifier = map - .buffer_snapshot - .char_classifier_at(point.to_offset(map, Bias::Left)); - if outer { - self.is_near_potential_outer_start(left, right, &classifier) - .map(|reach_start| (point, reach_start)) - } else { - self.is_near_potential_inner_start(left, right, &classifier) - .map(|reach_start| (point, reach_start)) - } - }) - { - let Some(start) = reach_start(identifier, map) else { - continue; - }; - if start > from { - previous_search_start = movement::left(map, identifier); - } else { - return Some(start); - } - } - None + self.to_boundary(map, from, outer, true, Boundary::Start) } fn previous_end( &self, @@ -643,31 +648,7 @@ impl BoundedObject for FuzzyBoundary { from: DisplayPoint, outer: bool, ) -> Option { - let mut previous_search_start = from; - while let Some((identifier, reach_end)) = - try_find_preceding_boundary_data(map, previous_search_start, |left, right, point| { - let classifier = map - .buffer_snapshot - .char_classifier_at(point.to_offset(map, Bias::Left)); - if outer { - self.is_near_potential_outer_end(left, right, &classifier) - .map(|reach_end| (point, reach_end)) - } else { - self.is_near_potential_inner_end(left, right, &classifier) - .map(|reach_end| (point, reach_end)) - } - }) - { - let Some(end) = reach_end(identifier, map) else { - continue; - }; - if end > from { - previous_search_start = movement::left(map, identifier); - } else { - return Some(end); - } - } - None + self.to_boundary(map, from, outer, true, Boundary::End) } fn can_be_zero_width(&self, _: bool) -> bool { false From 95f164b5ec7db4a90a48fa23b0234420e7c5b55f Mon Sep 17 00:00:00 2001 From: fantacell Date: Sun, 24 Aug 2025 01:06:51 +0200 Subject: [PATCH 23/28] Mention the differences to helix's way of doing this in the docs --- docs/src/helix.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/src/helix.md b/docs/src/helix.md index ddf997d3f0..37eaf516c9 100644 --- a/docs/src/helix.md +++ b/docs/src/helix.md @@ -9,3 +9,7 @@ For a guide on Vim-related features that are also available in Helix mode, pleas To check the current status of Helix mode, or to request a missing Helix feature, checkout out the ["Are we Helix yet?" discussion](https://github.com/zed-industries/zed/discussions/33580). For a detailed list of Helix's default keybindings, please visit the [official Helix documentation](https://docs.helix-editor.com/keymap.html). + +## Core differences + +Text object motions like `mi|` work, even if the cursor is on the `|` itself. Also, any text object that work with `mi` or `ma` also works with `]` and `[`, so for example `](` selects the next pair of parentheses after the cursor. From ea14bbbfa83cec76c156e29f81e7fddd76b67933 Mon Sep 17 00:00:00 2001 From: fantacell Date: Mon, 25 Aug 2025 22:29:23 +0200 Subject: [PATCH 24/28] Throw a warning instead of crashing --- crates/vim/src/helix/boundary.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/vim/src/helix/boundary.rs b/crates/vim/src/helix/boundary.rs index ccaefc072b..e07d2cffe5 100644 --- a/crates/vim/src/helix/boundary.rs +++ b/crates/vim/src/helix/boundary.rs @@ -94,13 +94,13 @@ trait BoundedObject { let inner_start = self .next_start(map, outer_range.start, false) .unwrap_or_else(|| { - todo!(); + log::warn!("The motion might not have found the text object correctly"); outer_range.start }); let inner_end = self .previous_end(map, outer_range.end, false) .unwrap_or_else(|| { - todo!(); + log::warn!("The motion might not have found the text object correctly"); outer_range.end }); inner_start..inner_end From fff15e983e8128030df8e77ad87603fb6163b6fe Mon Sep 17 00:00:00 2001 From: fantacell Date: Tue, 26 Aug 2025 16:37:32 +0200 Subject: [PATCH 25/28] Make it way faster by removing redundant checks --- crates/vim/src/helix/boundary.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/vim/src/helix/boundary.rs b/crates/vim/src/helix/boundary.rs index e07d2cffe5..f6f769c0cd 100644 --- a/crates/vim/src/helix/boundary.rs +++ b/crates/vim/src/helix/boundary.rs @@ -596,7 +596,7 @@ impl FuzzyBoundary { ); let boundaries = forwards .interleave(backwards) - .take(4) + .take(2) .filter_map(|(identifier, reach_boundary)| reach_boundary(identifier, map)) .filter(|boundary| match boundary.cmp(&from) { Ordering::Equal => true, From eaa9d37a04fca250eef4d98d70b2f9709ce37dc9 Mon Sep 17 00:00:00 2001 From: fantacell Date: Tue, 26 Aug 2025 16:44:24 +0200 Subject: [PATCH 26/28] Now it can be written more nicely --- crates/vim/src/helix/boundary.rs | 34 +++++--------------------------- 1 file changed, 5 insertions(+), 29 deletions(-) diff --git a/crates/vim/src/helix/boundary.rs b/crates/vim/src/helix/boundary.rs index f6f769c0cd..45f6eef0a1 100644 --- a/crates/vim/src/helix/boundary.rs +++ b/crates/vim/src/helix/boundary.rs @@ -568,35 +568,11 @@ impl FuzzyBoundary { reach_boundary.map(|reach_start| (point, reach_start)) }; - let forwards = std::iter::successors( - try_find_boundary_data(map, from, generate_boundary_data), - |(previous_identifier, _)| { - if *previous_identifier == map.max_point() { - return None; - } - try_find_boundary_data( - map, - movement::right(map, *previous_identifier), - generate_boundary_data, - ) - }, - ); - let backwards = std::iter::successors( - try_find_preceding_boundary_data(map, from, generate_boundary_data), - |(previous_identifier, _)| { - if *previous_identifier == DisplayPoint::zero() { - return None; - } - try_find_preceding_boundary_data( - map, - movement::left(map, *previous_identifier), - generate_boundary_data, - ) - }, - ); - let boundaries = forwards - .interleave(backwards) - .take(2) + let forwards = try_find_boundary_data(map, from, generate_boundary_data); + let backwards = try_find_preceding_boundary_data(map, from, generate_boundary_data); + let boundaries = [forwards, backwards] + .into_iter() + .filter_map(|data| data) .filter_map(|(identifier, reach_boundary)| reach_boundary(identifier, map)) .filter(|boundary| match boundary.cmp(&from) { Ordering::Equal => true, From 48a9580cbe38bee30d5d3ce3899eb13801789463 Mon Sep 17 00:00:00 2001 From: fantacell Date: Tue, 26 Aug 2025 16:49:12 +0200 Subject: [PATCH 27/28] Fix clippy --- crates/vim/src/helix/boundary.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/vim/src/helix/boundary.rs b/crates/vim/src/helix/boundary.rs index 45f6eef0a1..df4590ca04 100644 --- a/crates/vim/src/helix/boundary.rs +++ b/crates/vim/src/helix/boundary.rs @@ -5,7 +5,6 @@ use editor::{ display_map::{DisplaySnapshot, ToDisplayPoint}, movement, }; -use itertools::Itertools; use language::{CharClassifier, CharKind}; use text::Bias; From 2f9a6d9b376e1876376f92ecfaa851a5cb16061a Mon Sep 17 00:00:00 2001 From: fantacell Date: Tue, 26 Aug 2025 18:33:08 +0200 Subject: [PATCH 28/28] Actually fix clippy --- crates/vim/src/helix/boundary.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/vim/src/helix/boundary.rs b/crates/vim/src/helix/boundary.rs index df4590ca04..ea00663772 100644 --- a/crates/vim/src/helix/boundary.rs +++ b/crates/vim/src/helix/boundary.rs @@ -571,7 +571,7 @@ impl FuzzyBoundary { let backwards = try_find_preceding_boundary_data(map, from, generate_boundary_data); let boundaries = [forwards, backwards] .into_iter() - .filter_map(|data| data) + .flatten() .filter_map(|(identifier, reach_boundary)| reach_boundary(identifier, map)) .filter(|boundary| match boundary.cmp(&from) { Ordering::Equal => true,