Added the ability to nested values to the settings file, while preserving user formatting

Mikayla Maki and max created

co-authored-by: max <max@zed.dev>

Change summary

crates/settings/src/settings.rs | 479 +++++++++++++++++++++-------------
1 file changed, 294 insertions(+), 185 deletions(-)

Detailed changes

crates/settings/src/settings.rs 🔗

@@ -18,7 +18,7 @@ use sqlez::{
     bindable::{Bind, Column, StaticColumnCount},
     statement::Statement,
 };
-use std::{collections::HashMap, fmt::Write as _, num::NonZeroU32, str, sync::Arc};
+use std::{collections::HashMap, num::NonZeroU32, str, sync::Arc};
 use theme::{Theme, ThemeRegistry};
 use tree_sitter::Query;
 use util::ResultExt as _;
@@ -686,13 +686,25 @@ pub fn settings_file_json_schema(
     serde_json::to_value(root_schema).unwrap()
 }
 
+fn merge<T: Copy>(target: &mut T, value: Option<T>) {
+    if let Some(value) = value {
+        *target = value;
+    }
+}
+
+pub fn parse_json_with_comments<T: DeserializeOwned>(content: &str) -> Result<T> {
+    Ok(serde_json::from_reader(
+        json_comments::CommentSettings::c_style().strip_comments(content.as_bytes()),
+    )?)
+}
+
 /// Expects the key to be unquoted, and the value to be valid JSON
 /// (e.g. values should be unquoted for numbers and bools, quoted for strings)
-pub fn write_settings_key(
-    mut settings_content: String,
-    top_level_key: &str,
-    new_val: &str,
-) -> String {
+pub fn write_settings_key<T: ?Sized + Serialize + Clone>(
+    settings_content: &mut String,
+    key_path: &[&str],
+    new_value: &T,
+) {
     let mut parser = tree_sitter::Parser::new();
     parser.set_language(tree_sitter_json::language()).unwrap();
     let tree = parser.parse(&settings_content, None).unwrap();
@@ -702,56 +714,64 @@ pub fn write_settings_key(
     let query = Query::new(
         tree_sitter_json::language(),
         "
-        (document
-            (object
-                (pair
-                    key: (string) @key
-                    value: (_) @value)))
-    ",
+            (pair
+                key: (string) @key
+                value: (_) @value)
+        ",
     )
     .unwrap();
 
+    let mut depth = 0;
     let mut first_key_start = None;
-    let mut existing_value_range = None;
+    let mut existing_value_range = 0..settings_content.len();
     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];
+        let key_range = mat.captures[0].node.byte_range();
+        let value_range = mat.captures[1].node.byte_range();
+
+        if key_range.start > existing_value_range.end {
+            break;
+        }
+
+        first_key_start.get_or_insert_with(|| key_range.start);
+
+        let found_key = settings_content
+            .get(key_range.clone())
+            .map(|key_text| key_text == format!("\"{}\"", key_path[depth]))
+            .unwrap_or(false);
 
-        first_key_start.get_or_insert_with(|| key.node.start_byte());
+        if found_key {
+            existing_value_range = value_range;
+            depth += 1;
 
-        if let Some(key_text) = settings_content.get(key.node.byte_range()) {
-            if key_text == format!("\"{top_level_key}\"") {
-                existing_value_range = Some(value.node.byte_range());
+            if depth == key_path.len() {
                 break;
+            } else {
+                first_key_start = None;
             }
         }
     }
 
-    match (first_key_start, existing_value_range) {
-        (None, None) => {
-            // No document, create a new object and overwrite
-            settings_content.clear();
-            write!(
-                settings_content,
-                "{{\n    \"{}\": {new_val}\n}}\n",
-                top_level_key
-            )
-            .unwrap();
-        }
-
-        (_, Some(existing_value_range)) => {
-            // Existing theme key, overwrite
-            settings_content.replace_range(existing_value_range, &new_val);
+    // We found the exact key we want, insert the new value
+    if depth == key_path.len() {
+        let new_val = serde_json::to_string_pretty(new_value)
+            .expect("Could not serialize new json field to string");
+        settings_content.replace_range(existing_value_range, &new_val);
+    } else {
+        // We have key paths, construct the sub objects
+        let new_key = key_path[depth];
+
+        // We don't have the key, construct the nested objects
+        let mut new_value = serde_json::to_value(new_value).unwrap();
+        for key in key_path[(depth + 1)..].iter().rev() {
+            new_value = serde_json::json!({ key.to_string(): new_value });
         }
 
-        (Some(first_key_start), None) => {
-            // No existing theme key, but other settings. Prepend new theme settings and
-            // match style of first key
+        if let Some(first_key_start) = first_key_start {
             let mut row = 0;
             let mut column = 0;
             for (ix, char) in settings_content.char_indices() {
@@ -766,70 +786,118 @@ pub fn write_settings_key(
                 }
             }
 
-            let content = format!(r#""{top_level_key}": {new_val},"#);
-            settings_content.insert_str(first_key_start, &content);
-
             if row > 0 {
+                let new_val = to_pretty_json(&new_value, column, column);
+                let content = format!(r#""{new_key}": {new_val},"#);
+                settings_content.insert_str(first_key_start, &content);
+
                 settings_content.insert_str(
                     first_key_start + content.len(),
                     &format!("\n{:width$}", ' ', width = column),
                 )
             } else {
-                settings_content.insert_str(first_key_start + content.len(), " ")
+                let new_val = serde_json::to_string(&new_value).unwrap();
+                let mut content = format!(r#""{new_key}": {new_val},"#);
+                content.push(' ');
+                settings_content.insert_str(first_key_start, &content);
+            }
+        } else {
+            new_value = serde_json::json!({ new_key.to_string(): new_value });
+            let indent_prefix_len = 4 * depth;
+            let new_val = to_pretty_json(&new_value, 4, indent_prefix_len);
+
+            settings_content.replace_range(existing_value_range, &new_val);
+            if depth == 0 {
+                settings_content.push('\n');
             }
         }
     }
-
-    settings_content
 }
 
-fn merge<T: Copy>(target: &mut T, value: Option<T>) {
-    if let Some(value) = value {
-        *target = value;
-    }
-}
+fn to_pretty_json(
+    value: &serde_json::Value,
+    indent_size: usize,
+    indent_prefix_len: usize,
+) -> String {
+    const SPACES: [u8; 32] = [b' '; 32];
 
-pub fn parse_json_with_comments<T: DeserializeOwned>(content: &str) -> Result<T> {
-    Ok(serde_json::from_reader(
-        json_comments::CommentSettings::c_style().strip_comments(content.as_bytes()),
-    )?)
+    debug_assert!(indent_size <= SPACES.len());
+    debug_assert!(indent_prefix_len <= SPACES.len());
+
+    let mut output = Vec::new();
+    let mut ser = serde_json::Serializer::with_formatter(
+        &mut output,
+        serde_json::ser::PrettyFormatter::with_indent(&SPACES[0..indent_size.min(SPACES.len())]),
+    );
+
+    value.serialize(&mut ser).unwrap();
+    let text = String::from_utf8(output).unwrap();
+
+    let mut adjusted_text = String::new();
+    for (i, line) in text.split('\n').enumerate() {
+        if i > 0 {
+            adjusted_text.push_str(str::from_utf8(&SPACES[0..indent_prefix_len]).unwrap());
+        }
+        adjusted_text.push_str(line);
+        adjusted_text.push('\n');
+    }
+    adjusted_text.pop();
+    adjusted_text
 }
 
 pub fn update_settings_file(
-    old_text: String,
+    mut text: String,
     old_file_content: SettingsFileContent,
     update: impl FnOnce(&mut SettingsFileContent),
 ) -> String {
     let mut new_file_content = old_file_content.clone();
-    update(&mut new_file_content);
 
-    let old_json = to_json_object(old_file_content);
-    let new_json = to_json_object(new_file_content);
-
-    // Find changed fields
-    let mut diffs = vec![];
-    for (key, old_value) in old_json.iter() {
-        let new_value = new_json.get(key).unwrap();
-        if old_value != new_value {
-            if matches!(
-                new_value,
-                &Value::Null | &Value::Object(_) | &Value::Array(_)
-            ) {
-                unimplemented!("We only support updating basic values at the top level");
-            }
+    update(&mut new_file_content);
 
-            let new_json = serde_json::to_string_pretty(new_value)
-                .expect("Could not serialize new json field to string");
+    let old_object = to_json_object(old_file_content);
+    let new_object = to_json_object(new_file_content);
 
-            diffs.push((key, new_json));
+    fn apply_changes_to_json_text(
+        old_object: &serde_json::Map<String, Value>,
+        new_object: &serde_json::Map<String, Value>,
+        current_key_path: Vec<&str>,
+        json_text: &mut String,
+    ) {
+        for (key, old_value) in old_object.iter() {
+            // We know that these two are from the same shape of object, so we can just unwrap
+            let new_value = new_object.get(key).unwrap();
+            if old_value != new_value {
+                match new_value {
+                    Value::Bool(_) | Value::Number(_) | Value::String(_) => {
+                        let mut key_path = current_key_path.clone();
+                        key_path.push(key);
+                        write_settings_key(json_text, &key_path, &new_value);
+                    }
+                    Value::Object(new_sub_object) => {
+                        let mut key_path = current_key_path.clone();
+                        key_path.push(key);
+                        if let Value::Object(old_sub_object) = old_value {
+                            apply_changes_to_json_text(
+                                old_sub_object,
+                                new_sub_object,
+                                key_path,
+                                json_text,
+                            );
+                        } else {
+                            unimplemented!("This function doesn't support changing values from simple values to objects yet");
+                        }
+                    }
+                    Value::Null | Value::Array(_) => {
+                        unimplemented!("We only support objects and simple values");
+                    }
+                }
+            }
         }
     }
 
-    let mut new_text = old_text;
-    for (key, new_value) in diffs {
-        new_text = write_settings_key(new_text, key, &new_value)
-    }
-    new_text
+    apply_changes_to_json_text(&old_object, &new_object, vec![], &mut text);
+
+    text
 }
 
 fn to_json_object(settings_file: SettingsFileContent) -> serde_json::Map<String, Value> {
@@ -851,7 +919,7 @@ mod tests {
         expected_new_json: S2,
     ) {
         let old_json = old_json.into();
-        let old_content: SettingsFileContent = serde_json::from_str(&old_json).unwrap();
+        let old_content: SettingsFileContent = serde_json::from_str(&old_json).unwrap_or_default();
         let new_json = update_settings_file(old_json, old_content, update);
         assert_eq!(new_json, expected_new_json.into());
     }
@@ -859,23 +927,27 @@ mod tests {
     #[test]
     fn test_update_telemetry_setting_multiple_fields() {
         assert_new_settings(
-            r#"{
-                "telemetry": {
-                    "metrics": false,
-                    "diagnostics": false
+            r#"
+                {
+                    "telemetry": {
+                        "metrics": false,
+                        "diagnostics": false
+                    }
                 }
-            }"#
+            "#
             .unindent(),
             |settings| {
                 settings.telemetry.set_diagnostics(true);
                 settings.telemetry.set_metrics(true);
             },
-            r#"{
-                "telemetry": {
-                    "metrics": true,
-                    "diagnostics": true
+            r#"
+                {
+                    "telemetry": {
+                        "metrics": true,
+                        "diagnostics": true
+                    }
                 }
-            }"#
+            "#
             .unindent(),
         );
     }
@@ -898,20 +970,45 @@ mod tests {
     #[test]
     fn test_update_telemetry_setting_other_fields() {
         assert_new_settings(
-            r#"{
-                "telemetry": {
-                    "metrics": false,
-                    "diagnostics": true
+            r#"
+                {
+                    "telemetry": {
+                        "metrics": false,
+                        "diagnostics": true
+                    }
                 }
-            }"#
+            "#
             .unindent(),
             |settings| settings.telemetry.set_diagnostics(false),
-            r#"{
-                "telemetry": {
-                    "metrics": false,
-                    "diagnostics": false
+            r#"
+                {
+                    "telemetry": {
+                        "metrics": false,
+                        "diagnostics": false
+                    }
                 }
-            }"#
+            "#
+            .unindent(),
+        );
+    }
+
+    #[test]
+    fn test_update_telemetry_setting_empty_telemetry() {
+        assert_new_settings(
+            r#"
+                {
+                    "telemetry": {}
+                }
+            "#
+            .unindent(),
+            |settings| settings.telemetry.set_diagnostics(false),
+            r#"
+                {
+                    "telemetry": {
+                        "diagnostics": false
+                    }
+                }
+            "#
             .unindent(),
         );
     }
@@ -919,18 +1016,22 @@ mod tests {
     #[test]
     fn test_update_telemetry_setting_pre_existing() {
         assert_new_settings(
-            r#"{
-                "telemetry": {
-                    "diagnostics": true
+            r#"
+                {
+                    "telemetry": {
+                        "diagnostics": true
+                    }
                 }
-            }"#
+            "#
             .unindent(),
             |settings| settings.telemetry.set_diagnostics(false),
-            r#"{
-                "telemetry": {
-                    "diagnostics": false
+            r#"
+                {
+                    "telemetry": {
+                        "diagnostics": false
+                    }
                 }
-            }"#
+            "#
             .unindent(),
         );
     }
@@ -940,111 +1041,119 @@ mod tests {
         assert_new_settings(
             "{}",
             |settings| settings.telemetry.set_diagnostics(true),
-            r#"{
-                "telemetry": {
-                    "diagnostics": true
+            r#"
+                {
+                    "telemetry": {
+                        "diagnostics": true
+                    }
                 }
-            }"#
+            "#
             .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_settings_key(settings, "theme", "\"summerfruit-light\"");
+    fn test_update_object_empty_doc() {
+        assert_new_settings(
+            "",
+            |settings| settings.telemetry.set_diagnostics(true),
+            r#"
+                {
+                    "telemetry": {
+                        "diagnostics": true
+                    }
+                }
+            "#
+            .unindent(),
+        );
+    }
 
-        assert_eq!(settings_after_theme, new_settings)
+    #[test]
+    fn test_write_theme_into_settings_with_theme() {
+        assert_new_settings(
+            r#"
+                {
+                    "theme": "One Dark"
+                }
+            "#
+            .unindent(),
+            |settings| settings.theme = Some("summerfruit-light".to_string()),
+            r#"
+                {
+                    "theme": "summerfruit-light"
+                }
+            "#
+            .unindent(),
+        );
     }
 
     #[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_settings_key(settings, "theme", "\"summerfruit-light\"");
-
-        assert_eq!(settings_after_theme, new_settings)
+        assert_new_settings(
+            r#"
+                {
+                }
+            "#
+            .unindent(),
+            |settings| settings.theme = Some("summerfruit-light".to_string()),
+            r#"
+                {
+                    "theme": "summerfruit-light"
+                }
+            "#
+            .unindent(),
+        );
     }
 
     #[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_settings_key(settings, "theme", "\"summerfruit-light\"");
-
-        assert_eq!(settings_after_theme, new_settings)
+    fn write_key_no_document() {
+        assert_new_settings(
+            "",
+            |settings| settings.theme = Some("summerfruit-light".to_string()),
+            r#"
+                {
+                    "theme": "summerfruit-light"
+                }
+            "#
+            .unindent(),
+        );
     }
 
     #[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_settings_key(settings, "theme", "\"summerfruit-light\"");
-
-        assert_eq!(settings_after_theme, new_settings)
+        assert_new_settings(
+            r#"{ "a": "", "ok": true }"#,
+            |settings| settings.theme = Some("summerfruit-light".to_string()),
+            r#"{ "theme": "summerfruit-light", "a": "", "ok": true }"#,
+        );
     }
 
     #[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_settings_key(settings, "theme", "\"summerfruit-light\"");
-
-        assert_eq!(settings_after_theme, new_settings)
+        assert_new_settings(
+            r#"          { "a": "", "ok": true }"#,
+            |settings| settings.theme = Some("summerfruit-light".to_string()),
+            r#"          { "theme": "summerfruit-light", "a": "", "ok": true }"#.unindent(),
+        );
     }
 
     #[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_settings_key(settings, "theme", "\"summerfruit-light\"");
-
-        assert_eq!(settings_after_theme, new_settings)
+        assert_new_settings(
+            r#"
+                {
+                    "a": "b"
+                }
+            "#
+            .unindent(),
+            |settings| settings.theme = Some("summerfruit-light".to_string()),
+            r#"
+                {
+                    "theme": "summerfruit-light",
+                    "a": "b"
+                }
+            "#
+            .unindent(),
+        );
     }
 }