Fix some bugs in keymap handling (#3895)

- `base_keymap` setting was not respected, now it is
- without a `~/.config/zed/keymap.json` file, we would fail to load the
*default* keymap

Co-authored-by: Marshall <marshall@zed.dev>

crates/command_palette/src/command_palette.rs

@@ -370,6 +370,7 @@ mod tests {
     use gpui::TestAppContext;
     use language::Point;
     use project::Project;
+    use settings::KeymapFile;
     use workspace::{AppState, Workspace};
 
     #[test]
@@ -503,7 +504,20 @@ mod tests {
             workspace::init(app_state.clone(), cx);
             init(cx);
             Project::init_settings(cx);
-            settings::load_default_keymap(cx);
+            KeymapFile::parse(
+                r#"[
+                    {
+                        "bindings": {
+                            "cmd-n": "workspace::NewFile",
+                            "enter": "menu::Confirm",
+                            "cmd-shift-p": "command_palette::Toggle"
+                        }
+                    }
+                ]"#,
+            )
+            .unwrap()
+            .add_to_cx(cx)
+            .unwrap();
             app_state
         })
     }

crates/gpui/src/app.rs

@@ -327,6 +327,7 @@ impl AppContext {
     pub fn refresh(&mut self) {
         self.pending_effects.push_back(Effect::Refresh);
     }
+
     pub(crate) fn update<R>(&mut self, update: impl FnOnce(&mut Self) -> R) -> R {
         self.pending_updates += 1;
         let result = update(self);
@@ -840,10 +841,12 @@ impl AppContext {
     /// Update the global of the given type with a closure. Unlike `global_mut`, this method provides
     /// your closure with mutable access to the `AppContext` and the global simultaneously.
     pub fn update_global<G: 'static, R>(&mut self, f: impl FnOnce(&mut G, &mut Self) -> R) -> R {
-        let mut global = self.lease_global::<G>();
-        let result = f(&mut global, self);
-        self.end_global_lease(global);
+        self.update(|cx| {
+            let mut global = cx.lease_global::<G>();
+            let result = f(&mut global, cx);
+            cx.end_global_lease(global);
             result
+        })
     }
 
     /// Register a callback to be invoked when a global of the given type is updated.
@@ -941,6 +944,11 @@ impl AppContext {
         self.pending_effects.push_back(Effect::Refresh);
     }
 
+    pub fn clear_key_bindings(&mut self) {
+        self.keymap.lock().clear();
+        self.pending_effects.push_back(Effect::Refresh);
+    }
+
     /// Register a global listener for actions invoked via the keyboard.
     pub fn on_action<A: Action>(&mut self, listener: impl Fn(&A, &mut Self) + 'static) {
         self.global_action_listeners

crates/settings/src/settings_file.rs

@@ -1,4 +1,4 @@
-use crate::{settings_store::SettingsStore, KeymapFile, Settings};
+use crate::{settings_store::SettingsStore, Settings};
 use anyhow::Result;
 use fs::Fs;
 use futures::{channel::mpsc, StreamExt};
@@ -77,7 +77,6 @@ pub fn handle_settings_file_changes(
     });
     cx.spawn(move |mut cx| async move {
         while let Some(user_settings_content) = user_settings_file_rx.next().await {
-            eprintln!("settings file changed");
             let result = cx.update_global(|store: &mut SettingsStore, cx| {
                 store
                     .set_user_settings(&user_settings_content, cx)
@@ -121,14 +120,3 @@ pub fn update_settings_file<T: Settings>(
     })
     .detach_and_log_err(cx);
 }
-
-pub fn load_default_keymap(cx: &mut AppContext) {
-    for path in ["keymaps/default.json", "keymaps/vim.json"] {
-        KeymapFile::load_asset(path, cx).unwrap();
-    }
-
-    // todo!()
-    // if let Some(asset_path) = settings::get::<BaseKeymap>(cx).asset_path() {
-    //     KeymapFile::load_asset(asset_path, cx).unwrap();
-    // }
-}

crates/zed/src/zed.rs

@@ -18,11 +18,11 @@ pub use only_instance::*;
 pub use open_listener::*;
 
 use anyhow::{anyhow, Context as _};
-use futures::{channel::mpsc, StreamExt};
+use futures::{channel::mpsc, select_biased, StreamExt};
 use project_panel::ProjectPanel;
 use quick_action_bar::QuickActionBar;
 use search::project_search::ProjectSearchBar;
-use settings::{initial_local_settings_content, load_default_keymap, KeymapFile, Settings};
+use settings::{initial_local_settings_content, KeymapFile, Settings, SettingsStore};
 use std::{borrow::Cow, ops::Deref, sync::Arc};
 use terminal_view::terminal_panel::TerminalPanel;
 use util::{
@@ -32,6 +32,7 @@ use util::{
     ResultExt,
 };
 use uuid::Uuid;
+use welcome::BaseKeymap;
 use workspace::Pane;
 use workspace::{
     create_and_open_local_file, notifications::simple_message_notification::MessageNotification,
@@ -399,8 +400,7 @@ pub fn initialize_workspace(app_state: Arc<AppState>, cx: &mut AppContext) {
             });
 
         workspace.focus_handle(cx).focus(cx);
-        //todo!()
-        // load_default_keymap(cx);
+        load_default_keymap(cx);
     })
     .detach();
 }
@@ -558,38 +558,58 @@ pub fn handle_keymap_file_changes(
     mut user_keymap_file_rx: mpsc::UnboundedReceiver<String>,
     cx: &mut AppContext,
 ) {
-    cx.spawn(move |cx| async move {
-        //  let mut settings_subscription = None;
-        while let Some(user_keymap_content) = user_keymap_file_rx.next().await {
-            if let Some(keymap_content) = KeymapFile::parse(&user_keymap_content).log_err() {
-                cx.update(|cx| reload_keymaps(cx, &keymap_content)).ok();
+    BaseKeymap::register(cx);
 
-                // todo!()
-                // let mut old_base_keymap = cx.read(|cx| *settings::get::<BaseKeymap>(cx));
-                // drop(settings_subscription);
-                // settings_subscription = Some(cx.update(|cx| {
-                //     cx.observe_global::<SettingsStore, _>(move |cx| {
-                //         let new_base_keymap = *settings::get::<BaseKeymap>(cx);
-                //         if new_base_keymap != old_base_keymap {
-                //             old_base_keymap = new_base_keymap.clone();
-                //             reload_keymaps(cx, &keymap_content);
-                //         }
-                //     })
-                // }));
+    let (base_keymap_tx, mut base_keymap_rx) = mpsc::unbounded();
+    let mut old_base_keymap = *BaseKeymap::get_global(cx);
+    cx.observe_global::<SettingsStore>(move |cx| {
+        let new_base_keymap = *BaseKeymap::get_global(cx);
+        if new_base_keymap != old_base_keymap {
+            old_base_keymap = new_base_keymap.clone();
+            base_keymap_tx.unbounded_send(()).unwrap();
         }
+    })
+    .detach();
+
+    cx.spawn(move |cx| async move {
+        let mut user_keymap = KeymapFile::default();
+        loop {
+            select_biased! {
+                _ = base_keymap_rx.next() => {}
+                user_keymap_content = user_keymap_file_rx.next() => {
+                    if let Some(user_keymap_content) = user_keymap_content {
+                        if let Some(keymap_content) = KeymapFile::parse(&user_keymap_content).log_err() {
+                            user_keymap = keymap_content;
+                        } else {
+                            continue
+                        }
+                    }
+                }
+            }
+
+            cx.update(|cx| reload_keymaps(cx, &user_keymap)).ok();
         }
     })
     .detach();
 }
 
 fn reload_keymaps(cx: &mut AppContext, keymap_content: &KeymapFile) {
-    // todo!()
-    // cx.clear_bindings();
+    cx.clear_key_bindings();
     load_default_keymap(cx);
     keymap_content.clone().add_to_cx(cx).log_err();
     cx.set_menus(app_menus());
 }
 
+pub fn load_default_keymap(cx: &mut AppContext) {
+    for path in ["keymaps/default.json", "keymaps/vim.json"] {
+        KeymapFile::load_asset(path, cx).unwrap();
+    }
+
+    if let Some(asset_path) = BaseKeymap::get_global(cx).asset_path() {
+        KeymapFile::load_asset(asset_path, cx).unwrap();
+    }
+}
+
 fn open_local_settings_file(
     workspace: &mut Workspace,
     _: &OpenLocalSettings,