Added theme writing code, really bad race condition

Mikayla Maki created

Change summary

Cargo.lock                                  |  14 +
Cargo.toml                                  |   5 
constellation.txt                           |  10 +
crates/editor/Cargo.toml                    |   2 
crates/paths/Cargo.toml                     |  12 +
crates/paths/src/paths.rs                   |   0 
crates/settings/Cargo.toml                  |  10 
crates/settings/src/settings.rs             | 194 ++++++++++++++++++++++
crates/theme_selector/Cargo.toml            |   2 
crates/theme_selector/src/theme_selector.rs |  38 ++++
crates/zed/Cargo.toml                       |   2 
crates/zed/src/main.rs                      |  39 ++--
crates/zed/src/zed.rs                       |   1 
13 files changed, 297 insertions(+), 32 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -3738,6 +3738,14 @@ dependencies = [
  "rustc_version",
 ]
 
+[[package]]
+name = "paths"
+version = "0.1.0"
+dependencies = [
+ "dirs 3.0.2",
+ "lazy_static",
+]
+
 [[package]]
 name = "pbkdf2"
 version = "0.8.0"
@@ -5021,6 +5029,9 @@ dependencies = [
  "serde_path_to_error",
  "theme",
  "toml",
+ "tree-sitter",
+ "tree-sitter-json 0.19.0",
+ "unindent",
  "util",
 ]
 
@@ -5626,6 +5637,7 @@ dependencies = [
  "gpui",
  "log",
  "parking_lot 0.11.2",
+ "paths",
  "picker",
  "postage",
  "settings",
@@ -7251,7 +7263,6 @@ dependencies = [
  "context_menu",
  "ctor",
  "diagnostics",
- "dirs 3.0.2",
  "easy-parallel",
  "editor",
  "env_logger",
@@ -7274,6 +7285,7 @@ dependencies = [
  "num_cpus",
  "outline",
  "parking_lot 0.11.2",
+ "paths",
  "plugin_runtime",
  "postage",
  "project",

Cargo.toml 🔗

@@ -3,6 +3,10 @@ members = ["crates/*"]
 default-members = ["crates/zed"]
 resolver = "2"
 
+[workspace.dependencies]
+serde = { version = "1.0", features = ["derive", "rc"] }
+serde_json = { version = "1.0", features = ["preserve_order", "raw_value"] }
+
 [patch.crates-io]
 tree-sitter = { git = "https://github.com/tree-sitter/tree-sitter", rev = "366210ae925d7ea0891bc7a0c738f60c77c04d7b" }
 async-task = { git = "https://github.com/zed-industries/async-task", rev = "341b57d6de98cdfd7b418567b8de2022ca993a6e" }
@@ -21,3 +25,4 @@ split-debuginfo = "unpacked"
 
 [profile.release]
 debug = true
+

constellation.txt 🔗

@@ -0,0 +1,10 @@
+Burn down pain points constellation
+
+---------------------
+|                   | 
+|             *     | Mikayla
+|     *             | Julia
+|                   |
+|              *    | Kay
+|                   |
+---------------------

crates/editor/Cargo.toml 🔗

@@ -48,7 +48,7 @@ ordered-float = "2.1.1"
 parking_lot = "0.11"
 postage = { version = "0.4", features = ["futures-traits"] }
 rand = { version = "0.8.3", optional = true }
-serde = { version = "1.0", features = ["derive", "rc"] }
+serde = { workspace = true }
 smallvec = { version = "1.6", features = ["union"] }
 smol = "1.2"
 tree-sitter-rust = { version = "*", optional = true }

crates/paths/Cargo.toml 🔗

@@ -0,0 +1,12 @@
+[package]
+name = "paths"
+version = "0.1.0"
+edition = "2021"
+
+[lib]
+path = "src/paths.rs"
+
+[dependencies]
+lazy_static = "1.4.0"
+dirs = "3.0"
+

crates/settings/Cargo.toml 🔗

@@ -19,7 +19,13 @@ util = { path = "../util" }
 anyhow = "1.0.38"
 json_comments = "0.2"
 schemars = "0.8"
-serde = { version = "1.0", features = ["derive", "rc"] }
-serde_json = { version = "1.0", features = ["preserve_order"] }
+serde = { workspace = true }
+serde_json = { workspace = true }
 serde_path_to_error = "0.1.4"
 toml = "0.5"
+tree-sitter = "*"
+tree-sitter-json = "*"
+
+[dev-dependencies]
+unindent = "0.1"
+gpui = { path = "../gpui", features = ["test-support"] }

crates/settings/src/settings.rs 🔗

@@ -12,8 +12,9 @@ use schemars::{
 };
 use serde::{de::DeserializeOwned, Deserialize};
 use serde_json::Value;
-use std::{collections::HashMap, num::NonZeroU32, str, sync::Arc};
+use std::{collections::HashMap, fmt::Write as _, num::NonZeroU32, str, sync::Arc};
 use theme::{Theme, ThemeRegistry};
+use tree_sitter::Query;
 use util::ResultExt as _;
 
 pub use keymap_file::{keymap_file_json_schema, KeymapFileContent};
@@ -501,6 +502,92 @@ pub fn settings_file_json_schema(
     serde_json::to_value(root_schema).unwrap()
 }
 
+pub fn write_theme(mut settings_content: String, new_val: &str) -> String {
+    let mut parser = tree_sitter::Parser::new();
+    parser.set_language(tree_sitter_json::language()).unwrap();
+    let tree = parser.parse(&settings_content, None).unwrap();
+
+    let mut cursor = tree_sitter::QueryCursor::new();
+
+    let query = Query::new(
+        tree_sitter_json::language(),
+        "
+        (document
+            (object
+                (pair
+                    key: (string) @key
+                    value: (_) @value)))
+    ",
+    )
+    .unwrap();
+
+    let mut first_key_start = None;
+    let mut existing_value_range = None;
+    let matches = cursor.matches(&query, tree.root_node(), settings_content.as_bytes());
+    for mat in matches {
+        if mat.captures.len() != 2 {
+            continue;
+        }
+
+        let key = mat.captures[0];
+        let value = mat.captures[1];
+
+        first_key_start.get_or_insert_with(|| key.node.start_byte());
+
+        if let Some(key_text) = settings_content.get(key.node.byte_range()) {
+            if key_text == "\"theme\"" {
+                existing_value_range = Some(value.node.byte_range());
+                break;
+            }
+        }
+    }
+
+    match (first_key_start, existing_value_range) {
+        (None, None) => {
+            // No document, create a new object and overwrite
+            settings_content.clear();
+            write!(settings_content, "{{\n    \"theme\": \"{new_val}\"\n}}\n").unwrap();
+        }
+
+        (_, Some(existing_value_range)) => {
+            // Existing theme key, overwrite
+            settings_content.replace_range(existing_value_range, &format!("\"{new_val}\""));
+        }
+
+        (Some(first_key_start), None) => {
+            // No existing theme key, but other settings. Prepend new theme settings and
+            // match style of first key
+            let mut row = 0;
+            let mut column = 0;
+            for (ix, char) in settings_content.char_indices() {
+                if ix == first_key_start {
+                    break;
+                }
+                if char == '\n' {
+                    row += 1;
+                    column = 0;
+                } else {
+                    column += char.len_utf8();
+                }
+            }
+
+            let content = format!(r#""theme": "{new_val}","#);
+            settings_content.insert_str(first_key_start, &content);
+
+            if row > 0 {
+                settings_content.insert_str(
+                    first_key_start + content.len(),
+                    &format!("\n{:width$}", ' ', width = column),
+                )
+            } else {
+                settings_content.insert_str(first_key_start + content.len(), " ")
+            }
+        }
+    }
+
+    settings_content
+}
+
 fn merge<T: Copy>(target: &mut T, value: Option<T>) {
     if let Some(value) = value {
         *target = value;
@@ -512,3 +599,108 @@ pub fn parse_json_with_comments<T: DeserializeOwned>(content: &str) -> Result<T>
         json_comments::CommentSettings::c_style().strip_comments(content.as_bytes()),
     )?)
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::write_theme;
+    use unindent::Unindent;
+
+    #[test]
+    fn test_write_theme_into_settings_with_theme() {
+        let settings = r#"
+            {
+                "theme": "one-dark"
+            }
+        "#
+        .unindent();
+
+        let new_settings = r#"
+            {
+                "theme": "summerfruit-light"
+            }
+        "#
+        .unindent();
+
+        let settings_after_theme = write_theme(settings, "summerfruit-light");
+
+        assert_eq!(settings_after_theme, new_settings)
+    }
+
+    #[test]
+    fn test_write_theme_into_empty_settings() {
+        let settings = r#"
+            {
+            }
+        "#
+        .unindent();
+
+        let new_settings = r#"
+            {
+                "theme": "summerfruit-light"
+            }
+        "#
+        .unindent();
+
+        let settings_after_theme = write_theme(settings, "summerfruit-light");
+
+        assert_eq!(settings_after_theme, new_settings)
+    }
+
+    #[test]
+    fn test_write_theme_into_no_settings() {
+        let settings = "".to_string();
+
+        let new_settings = r#"
+            {
+                "theme": "summerfruit-light"
+            }
+        "#
+        .unindent();
+
+        let settings_after_theme = write_theme(settings, "summerfruit-light");
+
+        assert_eq!(settings_after_theme, new_settings)
+    }
+
+    #[test]
+    fn test_write_theme_into_single_line_settings_without_theme() {
+        let settings = r#"{ "a": "", "ok": true }"#.to_string();
+        let new_settings = r#"{ "theme": "summerfruit-light", "a": "", "ok": true }"#;
+
+        let settings_after_theme = write_theme(settings, "summerfruit-light");
+
+        assert_eq!(settings_after_theme, new_settings)
+    }
+
+    #[test]
+    fn test_write_theme_pre_object_whitespace() {
+        let settings = r#"          { "a": "", "ok": true }"#.to_string();
+        let new_settings = r#"          { "theme": "summerfruit-light", "a": "", "ok": true }"#;
+
+        let settings_after_theme = write_theme(settings, "summerfruit-light");
+
+        assert_eq!(settings_after_theme, new_settings)
+    }
+
+    #[test]
+    fn test_write_theme_into_multi_line_settings_without_theme() {
+        let settings = r#"
+            {
+                "a": "b"
+            }
+        "#
+        .unindent();
+
+        let new_settings = r#"
+            {
+                "theme": "summerfruit-light",
+                "a": "b"
+            }
+        "#
+        .unindent();
+
+        let settings_after_theme = write_theme(settings, "summerfruit-light");
+
+        assert_eq!(settings_after_theme, new_settings)
+    }
+}

crates/theme_selector/Cargo.toml 🔗

@@ -14,8 +14,10 @@ gpui = { path = "../gpui" }
 picker = { path = "../picker" }
 theme = { path = "../theme" }
 settings = { path = "../settings" }
+paths = { path = "../paths" }
 workspace = { path = "../workspace" }
 log = { version = "0.4.16", features = ["kv_unstable_serde"] }
 parking_lot = "0.11.1"
 postage = { version = "0.4.1", features = ["futures-traits"] }
 smol = "1.2.5"
+

crates/theme_selector/src/theme_selector.rs 🔗

@@ -1,10 +1,12 @@
 use fuzzy::{match_strings, StringMatch, StringMatchCandidate};
 use gpui::{
-    actions, elements::*, AnyViewHandle, AppContext, Element, ElementBox, Entity, MouseState,
-    MutableAppContext, RenderContext, View, ViewContext, ViewHandle,
+    actions, anyhow::Result, elements::*, AnyViewHandle, AppContext, Element, ElementBox, Entity,
+    MouseState, MutableAppContext, RenderContext, View, ViewContext, ViewHandle,
 };
+use paths::SETTINGS;
 use picker::{Picker, PickerDelegate};
-use settings::Settings;
+use settings::{write_theme, Settings};
+use smol::{fs::read_to_string, io::AsyncWriteExt};
 use std::sync::Arc;
 use theme::{Theme, ThemeMeta, ThemeRegistry};
 use workspace::{AppState, Workspace};
@@ -107,7 +109,20 @@ impl ThemeSelector {
     fn show_selected_theme(&mut self, cx: &mut ViewContext<Self>) {
         if let Some(mat) = self.matches.get(self.selected_index) {
             match self.registry.get(&mat.string) {
-                Ok(theme) => Self::set_theme(theme, cx),
+                Ok(theme) => {
+                    Self::set_theme(theme, cx);
+
+                    let theme_name = mat.string.clone();
+
+                    cx.background()
+                        .spawn(async move {
+                            match write_theme_name(theme_name).await {
+                                Ok(_) => {}
+                                Err(_) => return, //TODO Pop toast
+                            }
+                        })
+                        .detach()
+                }
                 Err(error) => {
                     log::error!("error loading theme {}: {}", mat.string, error)
                 }
@@ -264,3 +279,18 @@ impl View for ThemeSelector {
         }
     }
 }
+
+async fn write_theme_name(theme_name: String) -> Result<()> {
+    let mut settings = read_to_string(SETTINGS.as_path()).await?;
+    settings = write_theme(settings, &theme_name);
+
+    let mut file = smol::fs::OpenOptions::new()
+        .truncate(true)
+        .write(true)
+        .open(SETTINGS.as_path())
+        .await?;
+
+    file.write_all(settings.as_bytes()).await?;
+
+    Ok(())
+}

crates/zed/Cargo.toml 🔗

@@ -40,6 +40,7 @@ journal = { path = "../journal" }
 language = { path = "../language" }
 lsp = { path = "../lsp" }
 outline = { path = "../outline" }
+paths = { path = "../paths" }
 plugin_runtime = { path = "../plugin_runtime" }
 project = { path = "../project" }
 project_panel = { path = "../project_panel" }
@@ -61,7 +62,6 @@ async-trait = "0.1"
 backtrace = "0.3"
 chrono = "0.4"
 ctor = "0.1.20"
-dirs = "3.0"
 easy-parallel = "3.1.0"
 env_logger = "0.9"
 futures = "0.3"

crates/zed/src/main.rs 🔗

@@ -53,7 +53,7 @@ fn main() {
         .map_or("dev".to_string(), |v| v.to_string());
     init_panic_hook(app_version, http.clone(), app.background());
     let db = app.background().spawn(async move {
-        project::Db::open(&*zed::paths::DB)
+        project::Db::open(&*paths::DB)
             .log_err()
             .unwrap_or_else(project::Db::null)
     });
@@ -90,7 +90,7 @@ fn main() {
     app.run(move |cx| {
         let client = client::Client::new(http.clone(), cx);
         let mut languages = LanguageRegistry::new(login_shell_env_loaded);
-        languages.set_language_server_download_dir(zed::paths::LANGUAGES_DIR.clone());
+        languages.set_language_server_download_dir(paths::LANGUAGES_DIR.clone());
         let languages = Arc::new(languages);
         let init_languages = cx
             .background()
@@ -200,23 +200,21 @@ fn main() {
 }
 
 fn init_paths() {
-    fs::create_dir_all(&*zed::paths::CONFIG_DIR).expect("could not create config path");
-    fs::create_dir_all(&*zed::paths::LANGUAGES_DIR).expect("could not create languages path");
-    fs::create_dir_all(&*zed::paths::DB_DIR).expect("could not create database path");
-    fs::create_dir_all(&*zed::paths::LOGS_DIR).expect("could not create logs path");
+    fs::create_dir_all(&*paths::CONFIG_DIR).expect("could not create config path");
+    fs::create_dir_all(&*paths::LANGUAGES_DIR).expect("could not create languages path");
+    fs::create_dir_all(&*paths::DB_DIR).expect("could not create database path");
+    fs::create_dir_all(&*paths::LOGS_DIR).expect("could not create logs path");
 
     // Copy setting files from legacy locations. TODO: remove this after a few releases.
     thread::spawn(|| {
-        if fs::metadata(&*zed::paths::legacy::SETTINGS).is_ok()
-            && fs::metadata(&*zed::paths::SETTINGS).is_err()
+        if fs::metadata(&*paths::legacy::SETTINGS).is_ok()
+            && fs::metadata(&*paths::SETTINGS).is_err()
         {
-            fs::copy(&*zed::paths::legacy::SETTINGS, &*zed::paths::SETTINGS).log_err();
+            fs::copy(&*paths::legacy::SETTINGS, &*paths::SETTINGS).log_err();
         }
 
-        if fs::metadata(&*zed::paths::legacy::KEYMAP).is_ok()
-            && fs::metadata(&*zed::paths::KEYMAP).is_err()
-        {
-            fs::copy(&*zed::paths::legacy::KEYMAP, &*zed::paths::KEYMAP).log_err();
+        if fs::metadata(&*paths::legacy::KEYMAP).is_ok() && fs::metadata(&*paths::KEYMAP).is_err() {
+            fs::copy(&*paths::legacy::KEYMAP, &*paths::KEYMAP).log_err();
         }
     });
 }
@@ -231,15 +229,14 @@ fn init_logger() {
         const KIB: u64 = 1024;
         const MIB: u64 = 1024 * KIB;
         const MAX_LOG_BYTES: u64 = MIB;
-        if fs::metadata(&*zed::paths::LOG).map_or(false, |metadata| metadata.len() > MAX_LOG_BYTES)
-        {
-            let _ = fs::rename(&*zed::paths::LOG, &*zed::paths::OLD_LOG);
+        if fs::metadata(&*paths::LOG).map_or(false, |metadata| metadata.len() > MAX_LOG_BYTES) {
+            let _ = fs::rename(&*paths::LOG, &*paths::OLD_LOG);
         }
 
         let log_file = OpenOptions::new()
             .create(true)
             .append(true)
-            .open(&*zed::paths::LOG)
+            .open(&*paths::LOG)
             .expect("could not open logfile");
         simplelog::WriteLogger::init(level, simplelog::Config::default(), log_file)
             .expect("could not initialize logger");
@@ -251,7 +248,7 @@ fn init_panic_hook(app_version: String, http: Arc<dyn HttpClient>, background: A
         .spawn({
             async move {
                 let panic_report_url = format!("{}/api/panic", &*client::ZED_SERVER_URL);
-                let mut children = smol::fs::read_dir(&*zed::paths::LOGS_DIR).await?;
+                let mut children = smol::fs::read_dir(&*paths::LOGS_DIR).await?;
                 while let Some(child) = children.next().await {
                     let child = child?;
                     let child_path = child.path();
@@ -339,7 +336,7 @@ fn init_panic_hook(app_version: String, http: Arc<dyn HttpClient>, background: A
 
         let panic_filename = chrono::Utc::now().format("%Y_%m_%d %H_%M_%S").to_string();
         fs::write(
-            zed::paths::LOGS_DIR.join(format!("zed-{}-{}.panic", app_version, panic_filename)),
+            paths::LOGS_DIR.join(format!("zed-{}-{}.panic", app_version, panic_filename)),
             &message,
         )
         .context("error writing panic to disk")
@@ -473,8 +470,8 @@ fn load_config_files(
         .clone()
         .spawn(async move {
             let settings_file =
-                WatchedJsonFile::new(fs.clone(), &executor, zed::paths::SETTINGS.clone()).await;
-            let keymap_file = WatchedJsonFile::new(fs, &executor, zed::paths::KEYMAP.clone()).await;
+                WatchedJsonFile::new(fs.clone(), &executor, paths::SETTINGS.clone()).await;
+            let keymap_file = WatchedJsonFile::new(fs, &executor, paths::KEYMAP.clone()).await;
             tx.send((settings_file, keymap_file)).ok()
         })
         .detach();

crates/zed/src/zed.rs 🔗

@@ -1,7 +1,6 @@
 mod feedback;
 pub mod languages;
 pub mod menus;
-pub mod paths;
 pub mod settings_file;
 #[cfg(any(test, feature = "test-support"))]
 pub mod test;