vim: Fix escape key switching back to default mode instead of normal mode (#31843)

Sanjeev Shrestha and Conrad Irwin created

Closes #31728

This PR introduced new setting `"helix_mode"`. Enabling which will
enable the `vim_mode` along with `helix` behavior.

This solves issue where `vim`'s `default_mode` was being used to switch
between mode instead of opening in `default_mode`.

When `helix_mode` is enabled switcing to `Normal mode` will now switch
to `HelixNormal`


Release Notes:

- Fixed - escape key not switching to normal mode when default_mode is
insert
- Added - `helix_mode` setting to enable/disable helix key bindings

---------

Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>

Change summary

Cargo.toml                                      |  2 +
assets/settings/default.json                    |  2 +
crates/vim/src/insert.rs                        |  8 ++++
crates/vim/src/normal/paste.rs                  |  8 ++++
crates/vim/src/vim.rs                           | 20 +++++++----
crates/vim_mode_setting/src/vim_mode_setting.rs | 31 +++++++++++++++++-
crates/zed/Cargo.toml                           |  1 
crates/zed/src/zed.rs                           | 14 ++++++-
8 files changed, 72 insertions(+), 14 deletions(-)

Detailed changes

Cargo.toml 🔗

@@ -65,6 +65,7 @@ members = [
     "crates/gpui",
     "crates/gpui_macros",
     "crates/gpui_tokio",
+
     "crates/html_to_markdown",
     "crates/http_client",
     "crates/http_client_tls",
@@ -374,6 +375,7 @@ util = { path = "crates/util" }
 util_macros = { path = "crates/util_macros" }
 vim = { path = "crates/vim" }
 vim_mode_setting = { path = "crates/vim_mode_setting" }
+
 watch = { path = "crates/watch" }
 web_search = { path = "crates/web_search" }
 web_search_providers = { path = "crates/web_search_providers" }

assets/settings/default.json 🔗

@@ -110,6 +110,8 @@
   "multi_cursor_modifier": "alt",
   // Whether to enable vim modes and key bindings.
   "vim_mode": false,
+  // Whether to enable helix mode and key bindings.
+  "helix_mode": false,
   // Whether to show the informational hover box when moving the mouse
   // over symbols in the editor.
   "hover_popover_enabled": true,

crates/vim/src/insert.rs 🔗

@@ -2,6 +2,8 @@ use crate::{Vim, state::Mode};
 use editor::{Bias, Editor, scroll::Autoscroll};
 use gpui::{Action, Context, Window, actions};
 use language::SelectionGoal;
+use settings::Settings;
+use vim_mode_setting::HelixModeSetting;
 
 actions!(vim, [NormalBefore, TemporaryNormal]);
 
@@ -36,7 +38,11 @@ impl Vim {
                     });
                 });
             });
-            self.switch_mode(self.default_mode(cx), false, window, cx);
+            if HelixModeSetting::get_global(cx).0 {
+                self.switch_mode(Mode::HelixNormal, false, window, cx);
+            } else {
+                self.switch_mode(Mode::Normal, false, window, cx);
+            }
             return;
         }
 

crates/vim/src/normal/paste.rs 🔗

@@ -3,7 +3,9 @@ use gpui::{Context, Window, impl_actions};
 use language::{Bias, SelectionGoal};
 use schemars::JsonSchema;
 use serde::Deserialize;
+use settings::Settings;
 use std::cmp;
+use vim_mode_setting::HelixModeSetting;
 
 use crate::{
     Vim,
@@ -218,7 +220,11 @@ impl Vim {
             });
         });
 
-        self.switch_mode(self.default_mode(cx), true, window, cx);
+        if HelixModeSetting::get_global(cx).0 {
+            self.switch_mode(Mode::HelixNormal, true, window, cx);
+        } else {
+            self.switch_mode(Mode::Normal, true, window, cx);
+        }
     }
 
     pub fn replace_with_register_object(

crates/vim/src/vim.rs 🔗

@@ -44,6 +44,7 @@ use std::{mem, ops::Range, sync::Arc};
 use surrounds::SurroundsType;
 use theme::ThemeSettings;
 use ui::{IntoElement, SharedString, px};
+use vim_mode_setting::HelixModeSetting;
 use vim_mode_setting::VimModeSetting;
 use workspace::{self, Pane, Workspace};
 
@@ -359,8 +360,13 @@ impl Vim {
     pub fn new(window: &mut Window, cx: &mut Context<Editor>) -> Entity<Self> {
         let editor = cx.entity().clone();
 
+        let mut initial_mode = VimSettings::get_global(cx).default_mode;
+        if initial_mode == Mode::Normal && HelixModeSetting::get_global(cx).0 {
+            initial_mode = Mode::HelixNormal;
+        }
+
         cx.new(|cx| Vim {
-            mode: VimSettings::get_global(cx).default_mode,
+            mode: initial_mode,
             last_mode: Mode::Normal,
             temp_mode: false,
             exit_temporary_mode: false,
@@ -445,7 +451,11 @@ impl Vim {
 
         vim.update(cx, |_, cx| {
             Vim::action(editor, cx, |vim, _: &SwitchToNormalMode, window, cx| {
-                vim.switch_mode(vim.default_mode(cx), false, window, cx)
+                if HelixModeSetting::get_global(cx).0 {
+                    vim.switch_mode(Mode::HelixNormal, false, window, cx)
+                } else {
+                    vim.switch_mode(Mode::Normal, false, window, cx)
+                }
             });
 
             Vim::action(editor, cx, |vim, _: &SwitchToInsertMode, window, cx| {
@@ -748,10 +758,6 @@ impl Vim {
         cx.on_release(|_, _| drop(subscription)).detach();
     }
 
-    pub fn default_mode(&self, cx: &App) -> Mode {
-        VimSettings::get_global(cx).default_mode
-    }
-
     pub fn editor(&self) -> Option<Entity<Editor>> {
         self.editor.upgrade()
     }
@@ -766,7 +772,7 @@ impl Vim {
     }
 
     pub fn enabled(cx: &mut App) -> bool {
-        VimModeSetting::get_global(cx).0
+        VimModeSetting::get_global(cx).0 || HelixModeSetting::get_global(cx).0
     }
 
     /// Called whenever an keystroke is typed so vim can observe all actions

crates/vim_mode_setting/src/vim_mode_setting.rs 🔗

@@ -1,7 +1,7 @@
-//! Contains the [`VimModeSetting`] used to enable/disable Vim mode.
+//! Contains the [`VimModeSetting`] and [`HelixModeSetting`] used to enable/disable Vim and Helix modes.
 //!
 //! This is in its own crate as we want other crates to be able to enable or
-//! disable Vim mode without having to depend on the `vim` crate in its
+//! disable Vim/Helix modes without having to depend on the `vim` crate in its
 //! entirety.
 
 use anyhow::Result;
@@ -11,6 +11,7 @@ use settings::{Settings, SettingsSources};
 /// Initializes the `vim_mode_setting` crate.
 pub fn init(cx: &mut App) {
     VimModeSetting::register(cx);
+    HelixModeSetting::register(cx);
 }
 
 /// Whether or not to enable Vim mode.
@@ -38,3 +39,29 @@ impl Settings for VimModeSetting {
         // TODO: could possibly check if any of the `vim.<foo>` keys are set?
     }
 }
+
+/// Whether or not to enable Helix mode.
+///
+/// Default: false
+pub struct HelixModeSetting(pub bool);
+
+impl Settings for HelixModeSetting {
+    const KEY: Option<&'static str> = Some("helix_mode");
+
+    type FileContent = Option<bool>;
+
+    fn load(sources: SettingsSources<Self::FileContent>, _: &mut App) -> Result<Self> {
+        Ok(Self(
+            sources
+                .user
+                .or(sources.server)
+                .copied()
+                .flatten()
+                .unwrap_or(sources.default.ok_or_else(Self::missing_default)?),
+        ))
+    }
+
+    fn import_from_vscode(_vscode: &settings::VsCodeSettings, _current: &mut Self::FileContent) {
+        // TODO: could possibly check if any of the `helix.<foo>` keys are set?
+    }
+}

crates/zed/Cargo.toml 🔗

@@ -65,6 +65,7 @@ git_ui.workspace = true
 go_to_line.workspace = true
 gpui = { workspace = true, features = ["wayland", "x11", "font-kit"] }
 gpui_tokio.workspace = true
+
 http_client.workspace = true
 image_viewer.workspace = true
 indoc.workspace = true

crates/zed/src/zed.rs 🔗

@@ -1210,19 +1210,27 @@ pub fn handle_keymap_file_changes(
     cx: &mut App,
 ) {
     BaseKeymap::register(cx);
-    VimModeSetting::register(cx);
+    vim_mode_setting::init(cx);
 
     let (base_keymap_tx, mut base_keymap_rx) = mpsc::unbounded();
     let (keyboard_layout_tx, mut keyboard_layout_rx) = mpsc::unbounded();
     let mut old_base_keymap = *BaseKeymap::get_global(cx);
     let mut old_vim_enabled = VimModeSetting::get_global(cx).0;
+    let mut old_helix_enabled = vim_mode_setting::HelixModeSetting::get_global(cx).0;
+
     cx.observe_global::<SettingsStore>(move |cx| {
         let new_base_keymap = *BaseKeymap::get_global(cx);
         let new_vim_enabled = VimModeSetting::get_global(cx).0;
+        let new_helix_enabled = vim_mode_setting::HelixModeSetting::get_global(cx).0;
 
-        if new_base_keymap != old_base_keymap || new_vim_enabled != old_vim_enabled {
+        if new_base_keymap != old_base_keymap
+            || new_vim_enabled != old_vim_enabled
+            || new_helix_enabled != old_helix_enabled
+        {
             old_base_keymap = new_base_keymap;
             old_vim_enabled = new_vim_enabled;
+            old_helix_enabled = new_helix_enabled;
+
             base_keymap_tx.unbounded_send(()).unwrap();
         }
     })
@@ -1410,7 +1418,7 @@ pub fn load_default_keymap(cx: &mut App) {
         cx.bind_keys(KeymapFile::load_asset(asset_path, cx).unwrap());
     }
 
-    if VimModeSetting::get_global(cx).0 {
+    if VimModeSetting::get_global(cx).0 || vim_mode_setting::HelixModeSetting::get_global(cx).0 {
         cx.bind_keys(KeymapFile::load_asset(VIM_KEYMAP_PATH, cx).unwrap());
     }
 }