Migrate keymap and settings + edit predictions rename (#23834)

- [x] snake case keymap properties
- [x] flatten actions
- [x] keymap migration + notfication
- [x] settings migration + notification
- [x] inline completions -> edit predictions 

### future: 
- keymap notification doesn't show up on start up, only on keymap save.
this is existing bug in zed, will be addressed in seperate PR.

Release Notes:

- Added a notification for deprecated settings and keymaps, allowing you
to migrate them with a single click. A backup of your existing keymap
and settings will be created in your home directory.
- Modified some keymap actions and settings for consistency.

---------

Co-authored-by: Piotr Osiewicz <piotr@zed.dev>
Co-authored-by: Max Brunsfeld <maxbrunsfeld@gmail.com>

crates/settings/src/keymap_file.rs

@@ -1,22 +1,24 @@
-use std::rc::Rc;
-
-use crate::{settings_store::parse_json_with_comments, SettingsAssets};
-use anyhow::anyhow;
+use anyhow::{anyhow, Context as _, Result};
 use collections::{HashMap, IndexMap};
+use fs::Fs;
 use gpui::{
     Action, ActionBuildError, App, InvalidKeystrokeError, KeyBinding, KeyBindingContextPredicate,
     NoAction, SharedString, KEYSTROKE_PARSE_EXPECTED_MESSAGE,
 };
+use migrator::migrate_keymap;
 use schemars::{
     gen::{SchemaGenerator, SchemaSettings},
     schema::{ArrayValidation, InstanceType, Schema, SchemaObject, SubschemaValidation},
     JsonSchema,
 };
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
 use serde_json::Value;
-use std::fmt::Write;
+use std::rc::Rc;
+use std::{fmt::Write, sync::Arc};
 use util::{asset_str, markdown::MarkdownString};
 
+use crate::{settings_store::parse_json_with_comments, SettingsAssets};
+
 // Note that the doc comments on these are shown by json-language-server when editing the keymap, so
 // they should be considered user-facing documentation. Documentation is not handled well with
 // schemars-0.8 - when there are newlines, it is rendered as plaintext (see
@@ -28,12 +30,12 @@ use util::{asset_str, markdown::MarkdownString};
 
 /// Keymap configuration consisting of sections. Each section may have a context predicate which
 /// determines whether its bindings are used.
-#[derive(Debug, Deserialize, Default, Clone, JsonSchema)]
+#[derive(Debug, Deserialize, Default, Clone, JsonSchema, Serialize)]
 #[serde(transparent)]
 pub struct KeymapFile(Vec<KeymapSection>);
 
 /// Keymap section which binds keystrokes to actions.
-#[derive(Debug, Deserialize, Default, Clone, JsonSchema)]
+#[derive(Debug, Deserialize, Default, Clone, JsonSchema, Serialize)]
 pub struct KeymapSection {
     /// Determines when these bindings are active. When just a name is provided, like `Editor` or
     /// `Workspace`, the bindings will be active in that context. Boolean expressions like `X && Y`,
@@ -78,9 +80,9 @@ impl KeymapSection {
 /// Unlike the other json types involved in keymaps (including actions), this doc-comment will not
 /// be included in the generated JSON schema, as it manually defines its `JsonSchema` impl. The
 /// actual schema used for it is automatically generated in `KeymapFile::generate_json_schema`.
-#[derive(Debug, Deserialize, Default, Clone)]
+#[derive(Debug, Deserialize, Default, Clone, Serialize)]
 #[serde(transparent)]
-pub struct KeymapAction(Value);
+pub struct KeymapAction(pub(crate) Value);
 
 impl std::fmt::Display for KeymapAction {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -114,9 +116,11 @@ impl JsonSchema for KeymapAction {
 pub enum KeymapFileLoadResult {
     Success {
         key_bindings: Vec<KeyBinding>,
+        keymap_file: KeymapFile,
     },
     SomeFailedToLoad {
         key_bindings: Vec<KeyBinding>,
+        keymap_file: KeymapFile,
         error_message: MarkdownString,
     },
     JsonParseFailure {
@@ -150,6 +154,7 @@ impl KeymapFile {
             KeymapFileLoadResult::SomeFailedToLoad {
                 key_bindings,
                 error_message,
+                ..
             } if key_bindings.is_empty() => Err(anyhow!(
                 "Error loading built-in keymap \"{asset_path}\": {error_message}"
             )),
@@ -164,7 +169,7 @@ impl KeymapFile {
     #[cfg(feature = "test-support")]
     pub fn load_panic_on_failure(content: &str, cx: &App) -> Vec<KeyBinding> {
         match Self::load(content, cx) {
-            KeymapFileLoadResult::Success { key_bindings } => key_bindings,
+            KeymapFileLoadResult::Success { key_bindings, .. } => key_bindings,
             KeymapFileLoadResult::SomeFailedToLoad { error_message, .. } => {
                 panic!("{error_message}");
             }
@@ -180,6 +185,7 @@ impl KeymapFile {
         if content.is_empty() {
             return KeymapFileLoadResult::Success {
                 key_bindings: Vec::new(),
+                keymap_file: KeymapFile(Vec::new()),
             };
         }
         let keymap_file = match parse_json_with_comments::<Self>(content) {
@@ -266,7 +272,10 @@ impl KeymapFile {
         }
 
         if errors.is_empty() {
-            KeymapFileLoadResult::Success { key_bindings }
+            KeymapFileLoadResult::Success {
+                key_bindings,
+                keymap_file,
+            }
         } else {
             let mut error_message = "Errors in user keymap file.\n".to_owned();
             for (context, section_errors) in errors {
@@ -284,6 +293,7 @@ impl KeymapFile {
             }
             KeymapFileLoadResult::SomeFailedToLoad {
                 key_bindings,
+                keymap_file,
                 error_message: MarkdownString(error_message),
             }
         }
@@ -551,6 +561,55 @@ impl KeymapFile {
     pub fn sections(&self) -> impl DoubleEndedIterator<Item = &KeymapSection> {
         self.0.iter()
     }
+
+    async fn load_keymap_file(fs: &Arc<dyn Fs>) -> Result<String> {
+        match fs.load(paths::keymap_file()).await {
+            result @ Ok(_) => result,
+            Err(err) => {
+                if let Some(e) = err.downcast_ref::<std::io::Error>() {
+                    if e.kind() == std::io::ErrorKind::NotFound {
+                        return Ok(crate::initial_keymap_content().to_string());
+                    }
+                }
+                Err(err)
+            }
+        }
+    }
+
+    pub fn should_migrate_keymap(keymap_file: Self) -> bool {
+        let Ok(old_text) = serde_json::to_string(&keymap_file) else {
+            return false;
+        };
+        migrate_keymap(&old_text).is_some()
+    }
+
+    pub async fn migrate_keymap(fs: Arc<dyn Fs>) -> Result<()> {
+        let old_text = Self::load_keymap_file(&fs).await?;
+        let Some(new_text) = migrate_keymap(&old_text) else {
+            return Ok(());
+        };
+        let initial_path = paths::keymap_file().as_path();
+        if fs.is_file(initial_path).await {
+            let backup_path = paths::home_dir().join(".zed_keymap_backup");
+            fs.atomic_write(backup_path, old_text)
+                .await
+                .with_context(|| {
+                    "Failed to create settings backup in home directory".to_string()
+                })?;
+            let resolved_path = fs.canonicalize(initial_path).await.with_context(|| {
+                format!("Failed to canonicalize keymap path {:?}", initial_path)
+            })?;
+            fs.atomic_write(resolved_path.clone(), new_text)
+                .await
+                .with_context(|| format!("Failed to write keymap to file {:?}", resolved_path))?;
+        } else {
+            fs.atomic_write(initial_path.to_path_buf(), new_text)
+                .await
+                .with_context(|| format!("Failed to write keymap to file {:?}", initial_path))?;
+        }
+
+        Ok(())
+    }
 }
 
 // Double quotes a string and wraps it in backticks for markdown inline code..
@@ -560,7 +619,7 @@ fn inline_code_string(text: &str) -> MarkdownString {
 
 #[cfg(test)]
 mod tests {
-    use crate::KeymapFile;
+    use super::KeymapFile;
 
     #[test]
     fn can_deserialize_keymap_with_trailing_comma() {

crates/settings/src/settings_file.rs

@@ -81,7 +81,7 @@ pub fn watch_config_file(
 pub fn handle_settings_file_changes(
     mut user_settings_file_rx: mpsc::UnboundedReceiver<String>,
     cx: &mut App,
-    settings_changed: impl Fn(Option<anyhow::Error>, &mut App) + 'static,
+    settings_changed: impl Fn(Result<serde_json::Value, anyhow::Error>, &mut App) + 'static,
 ) {
     let user_settings_content = cx
         .background_executor()
@@ -92,7 +92,7 @@ pub fn handle_settings_file_changes(
         if let Err(err) = &result {
             log::error!("Failed to load user settings: {err}");
         }
-        settings_changed(result.err(), cx);
+        settings_changed(result, cx);
     });
     cx.spawn(move |cx| async move {
         while let Some(user_settings_content) = user_settings_file_rx.next().await {
@@ -101,7 +101,7 @@ pub fn handle_settings_file_changes(
                 if let Err(err) = &result {
                     log::error!("Failed to load user settings: {err}");
                 }
-                settings_changed(result.err(), cx);
+                settings_changed(result, cx);
                 cx.refresh_windows();
             });
             if result.is_err() {

crates/settings/src/settings_store.rs

@@ -4,6 +4,7 @@ use ec4rs::{ConfigParser, PropertiesSource, Section};
 use fs::Fs;
 use futures::{channel::mpsc, future::LocalBoxFuture, FutureExt, StreamExt};
 use gpui::{App, AsyncApp, BorrowAppContext, Global, Task, UpdateGlobal};
+use migrator::migrate_settings;
 use paths::{local_settings_file_relative_path, EDITORCONFIG_NAME};
 use schemars::{gen::SchemaGenerator, schema::RootSchema, JsonSchema};
 use serde::{de::DeserializeOwned, Deserialize, Serialize};
@@ -17,7 +18,9 @@ use std::{
     sync::{Arc, LazyLock},
 };
 use tree_sitter::Query;
-use util::{merge_non_null_json_value_into, RangeExt, ResultExt as _};
+use util::RangeExt;
+
+use util::{merge_non_null_json_value_into, ResultExt as _};
 
 pub type EditorconfigProperties = ec4rs::Properties;
 
@@ -544,7 +547,11 @@ impl SettingsStore {
     }
 
     /// Sets the user settings via a JSON string.
-    pub fn set_user_settings(&mut self, user_settings_content: &str, cx: &mut App) -> Result<()> {
+    pub fn set_user_settings(
+        &mut self,
+        user_settings_content: &str,
+        cx: &mut App,
+    ) -> Result<serde_json::Value> {
         let settings: serde_json::Value = if user_settings_content.is_empty() {
             parse_json_with_comments("{}")?
         } else {
@@ -552,9 +559,9 @@ impl SettingsStore {
         };
 
         anyhow::ensure!(settings.is_object(), "settings must be an object");
-        self.raw_user_settings = settings;
+        self.raw_user_settings = settings.clone();
         self.recompute_values(None, cx)?;
-        Ok(())
+        Ok(settings)
     }
 
     pub fn set_server_settings(
@@ -988,6 +995,52 @@ impl SettingsStore {
         properties.use_fallbacks();
         Some(properties)
     }
+
+    pub fn should_migrate_settings(settings: &serde_json::Value) -> bool {
+        let Ok(old_text) = serde_json::to_string(settings) else {
+            return false;
+        };
+        migrate_settings(&old_text).is_some()
+    }
+
+    pub fn migrate_settings(&self, fs: Arc<dyn Fs>) {
+        self.setting_file_updates_tx
+            .unbounded_send(Box::new(move |_: AsyncApp| {
+                async move {
+                    let old_text = Self::load_settings(&fs).await?;
+                    let Some(new_text) = migrate_settings(&old_text) else {
+                        return anyhow::Ok(());
+                    };
+                    let initial_path = paths::settings_file().as_path();
+                    if fs.is_file(initial_path).await {
+                        let backup_path = paths::home_dir().join(".zed_settings_backup");
+                        fs.atomic_write(backup_path, old_text)
+                            .await
+                            .with_context(|| {
+                                "Failed to create settings backup in home directory".to_string()
+                            })?;
+                        let resolved_path =
+                            fs.canonicalize(initial_path).await.with_context(|| {
+                                format!("Failed to canonicalize settings path {:?}", initial_path)
+                            })?;
+                        fs.atomic_write(resolved_path.clone(), new_text)
+                            .await
+                            .with_context(|| {
+                                format!("Failed to write settings to file {:?}", resolved_path)
+                            })?;
+                    } else {
+                        fs.atomic_write(initial_path.to_path_buf(), new_text)
+                            .await
+                            .with_context(|| {
+                                format!("Failed to write settings to file {:?}", initial_path)
+                            })?;
+                    }
+                    anyhow::Ok(())
+                }
+                .boxed_local()
+            }))
+            .ok();
+    }
 }
 
 #[derive(Debug, Clone, PartialEq)]
@@ -1235,7 +1288,9 @@ fn replace_value_in_json_text(
 
         let found_key = text
             .get(key_range.clone())
-            .map(|key_text| key_text == format!("\"{}\"", key_path[depth]))
+            .map(|key_text| {
+                depth < key_path.len() && key_text == format!("\"{}\"", key_path[depth])
+            })
             .unwrap_or(false);
 
         if found_key {