vim: `f` and `t` multiline option (#8448)

Rom Grk created

I'm not sure how compliant you're aiming to be with vim, but the `f`
behavior is more useful when it can search on multiple lines instead of
a single one, so I'd like to propose this change.

This change is quite frequent in vim/neovim as a plugin (e.g.
[clever-f](https://github.com/VSCodeVim/Vim),
[improved-ft](https://github.com/backdround/improved-ft.nvim), etc), and
in other vim emulations (e.g.
[vscode-vim](https://github.com/VSCodeVim/Vim)).

Change summary

assets/settings/default.json                 |  3 
crates/editor/src/movement.rs                |  2 
crates/vim/src/motion.rs                     | 96 ++++++++++++++-------
crates/vim/src/normal.rs                     | 88 +++++++++++++++++++
crates/vim/src/vim.rs                        | 17 +++
docs/src/configuring_zed__configuring_vim.md | 17 +++
6 files changed, 186 insertions(+), 37 deletions(-)

Detailed changes

assets/settings/default.json 🔗

@@ -571,7 +571,8 @@
   },
   // Vim settings
   "vim": {
-    "use_system_clipboard": "always"
+    "use_system_clipboard": "always",
+    "use_multiline_find": false
   },
   // The server to connect to. If the environment variable
   // ZED_SERVER_URL is set, it will override this setting.

crates/editor/src/movement.rs 🔗

@@ -12,7 +12,7 @@ use std::{ops::Range, sync::Arc};
 /// Defines search strategy for items in `movement` module.
 /// `FindRange::SingeLine` only looks for a match on a single line at a time, whereas
 /// `FindRange::MultiLine` keeps going until the end of a string.
-#[derive(Debug, PartialEq)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum FindRange {
     SingleLine,
     MultiLine,

crates/vim/src/motion.rs 🔗

@@ -22,27 +22,57 @@ use crate::{
 pub enum Motion {
     Left,
     Backspace,
-    Down { display_lines: bool },
-    Up { display_lines: bool },
+    Down {
+        display_lines: bool,
+    },
+    Up {
+        display_lines: bool,
+    },
     Right,
     Space,
-    NextWordStart { ignore_punctuation: bool },
-    NextWordEnd { ignore_punctuation: bool },
-    PreviousWordStart { ignore_punctuation: bool },
-    PreviousWordEnd { ignore_punctuation: bool },
-    FirstNonWhitespace { display_lines: bool },
+    NextWordStart {
+        ignore_punctuation: bool,
+    },
+    NextWordEnd {
+        ignore_punctuation: bool,
+    },
+    PreviousWordStart {
+        ignore_punctuation: bool,
+    },
+    PreviousWordEnd {
+        ignore_punctuation: bool,
+    },
+    FirstNonWhitespace {
+        display_lines: bool,
+    },
     CurrentLine,
-    StartOfLine { display_lines: bool },
-    EndOfLine { display_lines: bool },
+    StartOfLine {
+        display_lines: bool,
+    },
+    EndOfLine {
+        display_lines: bool,
+    },
     StartOfParagraph,
     EndOfParagraph,
     StartOfDocument,
     EndOfDocument,
     Matching,
-    FindForward { before: bool, char: char },
-    FindBackward { after: bool, char: char },
-    RepeatFind { last_find: Box<Motion> },
-    RepeatFindReversed { last_find: Box<Motion> },
+    FindForward {
+        before: bool,
+        char: char,
+        mode: FindRange,
+    },
+    FindBackward {
+        after: bool,
+        char: char,
+        mode: FindRange,
+    },
+    RepeatFind {
+        last_find: Box<Motion>,
+    },
+    RepeatFindReversed {
+        last_find: Box<Motion>,
+    },
     NextLineStart,
     StartOfLineDownward,
     EndOfLineDownward,
@@ -481,30 +511,30 @@ impl Motion {
             ),
             Matching => (matching(map, point), SelectionGoal::None),
             // t f
-            FindForward { before, char } => {
-                return find_forward(map, point, *before, *char, times)
+            FindForward { before, char, mode } => {
+                return find_forward(map, point, *before, *char, times, *mode)
                     .map(|new_point| (new_point, SelectionGoal::None))
             }
             // T F
-            FindBackward { after, char } => (
-                find_backward(map, point, *after, *char, times),
+            FindBackward { after, char, mode } => (
+                find_backward(map, point, *after, *char, times, *mode),
                 SelectionGoal::None,
             ),
             // ; -- repeat the last find done with t, f, T, F
             RepeatFind { last_find } => match **last_find {
-                Motion::FindForward { before, char } => {
-                    let mut new_point = find_forward(map, point, before, char, times);
+                Motion::FindForward { before, char, mode } => {
+                    let mut new_point = find_forward(map, point, before, char, times, mode);
                     if new_point == Some(point) {
-                        new_point = find_forward(map, point, before, char, times + 1);
+                        new_point = find_forward(map, point, before, char, times + 1, mode);
                     }
 
                     return new_point.map(|new_point| (new_point, SelectionGoal::None));
                 }
 
-                Motion::FindBackward { after, char } => {
-                    let mut new_point = find_backward(map, point, after, char, times);
+                Motion::FindBackward { after, char, mode } => {
+                    let mut new_point = find_backward(map, point, after, char, times, mode);
                     if new_point == point {
-                        new_point = find_backward(map, point, after, char, times + 1);
+                        new_point = find_backward(map, point, after, char, times + 1, mode);
                     }
 
                     (new_point, SelectionGoal::None)
@@ -513,19 +543,19 @@ impl Motion {
             },
             // , -- repeat the last find done with t, f, T, F, in opposite direction
             RepeatFindReversed { last_find } => match **last_find {
-                Motion::FindForward { before, char } => {
-                    let mut new_point = find_backward(map, point, before, char, times);
+                Motion::FindForward { before, char, mode } => {
+                    let mut new_point = find_backward(map, point, before, char, times, mode);
                     if new_point == point {
-                        new_point = find_backward(map, point, before, char, times + 1);
+                        new_point = find_backward(map, point, before, char, times + 1, mode);
                     }
 
                     (new_point, SelectionGoal::None)
                 }
 
-                Motion::FindBackward { after, char } => {
-                    let mut new_point = find_forward(map, point, after, char, times);
+                Motion::FindBackward { after, char, mode } => {
+                    let mut new_point = find_forward(map, point, after, char, times, mode);
                     if new_point == Some(point) {
-                        new_point = find_forward(map, point, after, char, times + 1);
+                        new_point = find_forward(map, point, after, char, times + 1, mode);
                     }
 
                     return new_point.map(|new_point| (new_point, SelectionGoal::None));
@@ -1011,13 +1041,14 @@ fn find_forward(
     before: bool,
     target: char,
     times: usize,
+    mode: FindRange,
 ) -> Option<DisplayPoint> {
     let mut to = from;
     let mut found = false;
 
     for _ in 0..times {
         found = false;
-        let new_to = find_boundary(map, to, FindRange::SingleLine, |_, right| {
+        let new_to = find_boundary(map, to, mode, |_, right| {
             found = right == target;
             found
         });
@@ -1045,14 +1076,13 @@ fn find_backward(
     after: bool,
     target: char,
     times: usize,
+    mode: FindRange,
 ) -> DisplayPoint {
     let mut to = from;
 
     for _ in 0..times {
         let new_to =
-            find_preceding_boundary_display_point(map, to, FindRange::SingleLine, |_, right| {
-                right == target
-            });
+            find_preceding_boundary_display_point(map, to, mode, |_, right| right == target);
         if to == new_to {
             break;
         }

crates/vim/src/normal.rs 🔗

@@ -379,10 +379,12 @@ pub(crate) fn normal_replace(text: Arc<str>, cx: &mut WindowContext) {
 mod test {
     use gpui::TestAppContext;
     use indoc::indoc;
+    use settings::SettingsStore;
 
     use crate::{
         state::Mode::{self},
-        test::NeovimBackedTestContext,
+        test::{NeovimBackedTestContext, VimTestContext},
+        VimSettings,
     };
 
     #[gpui::test]
@@ -903,6 +905,90 @@ mod test {
         }
     }
 
+    #[gpui::test]
+    async fn test_f_and_t_multiline(cx: &mut gpui::TestAppContext) {
+        let mut cx = VimTestContext::new(cx, true).await;
+        cx.update_global(|store: &mut SettingsStore, cx| {
+            store.update_user_settings::<VimSettings>(cx, |s| {
+                s.use_multiline_find = Some(true);
+            });
+        });
+
+        cx.assert_binding(
+            ["f", "l"],
+            indoc! {"
+            ˇfunction print() {
+                console.log('ok')
+            }
+            "},
+            Mode::Normal,
+            indoc! {"
+            function print() {
+                consoˇle.log('ok')
+            }
+            "},
+            Mode::Normal,
+        );
+
+        cx.assert_binding(
+            ["t", "l"],
+            indoc! {"
+            ˇfunction print() {
+                console.log('ok')
+            }
+            "},
+            Mode::Normal,
+            indoc! {"
+            function print() {
+                consˇole.log('ok')
+            }
+            "},
+            Mode::Normal,
+        );
+    }
+
+    #[gpui::test]
+    async fn test_capital_f_and_capital_t_multiline(cx: &mut gpui::TestAppContext) {
+        let mut cx = VimTestContext::new(cx, true).await;
+        cx.update_global(|store: &mut SettingsStore, cx| {
+            store.update_user_settings::<VimSettings>(cx, |s| {
+                s.use_multiline_find = Some(true);
+            });
+        });
+
+        cx.assert_binding(
+            ["shift-f", "p"],
+            indoc! {"
+            function print() {
+                console.ˇlog('ok')
+            }
+            "},
+            Mode::Normal,
+            indoc! {"
+            function ˇprint() {
+                console.log('ok')
+            }
+            "},
+            Mode::Normal,
+        );
+
+        cx.assert_binding(
+            ["shift-t", "p"],
+            indoc! {"
+            function print() {
+                console.ˇlog('ok')
+            }
+            "},
+            Mode::Normal,
+            indoc! {"
+            function pˇrint() {
+                console.log('ok')
+            }
+            "},
+            Mode::Normal,
+        );
+    }
+
     #[gpui::test]
     async fn test_percent(cx: &mut TestAppContext) {
         let mut cx = NeovimBackedTestContext::new(cx).await.binding(["%"]);

crates/vim/src/vim.rs 🔗

@@ -17,7 +17,10 @@ mod visual;
 use anyhow::Result;
 use collections::HashMap;
 use command_palette_hooks::{CommandPaletteFilter, CommandPaletteInterceptor};
-use editor::{movement, Editor, EditorEvent, EditorMode};
+use editor::{
+    movement::{self, FindRange},
+    Editor, EditorEvent, EditorMode,
+};
 use gpui::{
     actions, impl_actions, Action, AppContext, EntityId, Global, Subscription, View, ViewContext,
     WeakView, WindowContext,
@@ -482,6 +485,11 @@ impl Vim {
                 let find = Motion::FindForward {
                     before,
                     char: text.chars().next().unwrap(),
+                    mode: if VimSettings::get_global(cx).use_multiline_find {
+                        FindRange::MultiLine
+                    } else {
+                        FindRange::SingleLine
+                    },
                 };
                 Vim::update(cx, |vim, _| {
                     vim.workspace_state.last_find = Some(find.clone())
@@ -492,6 +500,11 @@ impl Vim {
                 let find = Motion::FindBackward {
                     after,
                     char: text.chars().next().unwrap(),
+                    mode: if VimSettings::get_global(cx).use_multiline_find {
+                        FindRange::MultiLine
+                    } else {
+                        FindRange::SingleLine
+                    },
                 };
                 Vim::update(cx, |vim, _| {
                     vim.workspace_state.last_find = Some(find.clone())
@@ -628,11 +641,13 @@ struct VimSettings {
     // vim always uses system cliupbaord
     // some magic where yy is system and dd is not.
     pub use_system_clipboard: UseSystemClipboard,
+    pub use_multiline_find: bool,
 }
 
 #[derive(Clone, Default, Serialize, Deserialize, JsonSchema)]
 struct VimSettingsContent {
     pub use_system_clipboard: Option<UseSystemClipboard>,
+    pub use_multiline_find: Option<bool>,
 }
 
 impl Settings for VimSettings {

docs/src/configuring_zed__configuring_vim.md 🔗

@@ -144,6 +144,23 @@ Currently supported vim-specific commands (as of Zed 0.106):
     to sort the current selection (with i, case-insensitively)
 ```
 
+## Vim settings
+
+Some vim settings are available to modify the default vim behavior:
+
+```json
+{
+  "vim": {
+    // "always": use system clipboard
+    // "never": don't use system clipboard
+    // "on_yank": use system clipboard for yank operations
+    "use_system_clipboard": "always",
+    // Enable multi-line find for `f` and `t` motions
+    "use_multiline_find": false
+  }
+}
+```
+
 ## Related settings
 
 There are a few Zed settings that you may also enjoy if you use vim mode: