Merge pull request #1232 from zed-industries/language-defaults

Antonio Scandurra created

Don't override top-level settings with language defaults

Change summary

crates/editor/src/display_map.rs |   7 -
crates/editor/src/editor.rs      |  16 ++--
crates/project/src/project.rs    |   6 
crates/settings/src/settings.rs  | 119 ++++++++++++++++-----------------
crates/zed/src/main.rs           |  36 +++++-----
crates/zed/src/settings_file.rs  |  76 ++++++++++++++++++---
6 files changed, 152 insertions(+), 108 deletions(-)

Detailed changes

crates/editor/src/display_map.rs 🔗

@@ -965,10 +965,9 @@ pub mod tests {
         );
         language.set_theme(&theme);
         cx.update(|cx| {
-            cx.set_global(Settings {
-                tab_size: 2,
-                ..Settings::test(cx)
-            })
+            let mut settings = Settings::test(cx);
+            settings.language_settings.tab_size = Some(2);
+            cx.set_global(settings);
         });
 
         let buffer = cx.add_model(|cx| Buffer::new(0, text, cx).with_language(language, cx));

crates/editor/src/editor.rs 🔗

@@ -6162,7 +6162,7 @@ mod tests {
     use language::{FakeLspAdapter, LanguageConfig};
     use lsp::FakeLanguageServer;
     use project::FakeFs;
-    use settings::LanguageOverride;
+    use settings::LanguageSettings;
     use std::{cell::RefCell, rc::Rc, time::Instant};
     use text::Point;
     use unindent::Unindent;
@@ -7499,7 +7499,7 @@ mod tests {
         let mut cx = EditorTestContext::new(cx).await;
         cx.update(|cx| {
             cx.update_global::<Settings, _, _>(|settings, _| {
-                settings.hard_tabs = true;
+                settings.language_settings.hard_tabs = Some(true);
             });
         });
 
@@ -7580,16 +7580,16 @@ mod tests {
     fn test_indent_outdent_with_excerpts(cx: &mut gpui::MutableAppContext) {
         cx.set_global(
             Settings::test(cx)
-                .with_overrides(
+                .with_language_defaults(
                     "TOML",
-                    LanguageOverride {
+                    LanguageSettings {
                         tab_size: Some(2),
                         ..Default::default()
                     },
                 )
-                .with_overrides(
+                .with_language_defaults(
                     "Rust",
-                    LanguageOverride {
+                    LanguageSettings {
                         tab_size: Some(4),
                         ..Default::default()
                     },
@@ -9162,7 +9162,7 @@ mod tests {
             cx.update_global::<Settings, _, _>(|settings, _| {
                 settings.language_overrides.insert(
                     "Rust".into(),
-                    LanguageOverride {
+                    LanguageSettings {
                         tab_size: Some(8),
                         ..Default::default()
                     },
@@ -9276,7 +9276,7 @@ mod tests {
             cx.update_global::<Settings, _, _>(|settings, _| {
                 settings.language_overrides.insert(
                     "Rust".into(),
-                    LanguageOverride {
+                    LanguageSettings {
                         tab_size: Some(8),
                         ..Default::default()
                     },

crates/project/src/project.rs 🔗

@@ -6673,7 +6673,7 @@ mod tests {
             cx.update_global(|settings: &mut Settings, _| {
                 settings.language_overrides.insert(
                     Arc::from("Rust"),
-                    settings::LanguageOverride {
+                    settings::LanguageSettings {
                         enable_language_server: Some(false),
                         ..Default::default()
                     },
@@ -6690,14 +6690,14 @@ mod tests {
             cx.update_global(|settings: &mut Settings, _| {
                 settings.language_overrides.insert(
                     Arc::from("Rust"),
-                    settings::LanguageOverride {
+                    settings::LanguageSettings {
                         enable_language_server: Some(true),
                         ..Default::default()
                     },
                 );
                 settings.language_overrides.insert(
                     Arc::from("JavaScript"),
-                    settings::LanguageOverride {
+                    settings::LanguageSettings {
                         enable_language_server: Some(false),
                         ..Default::default()
                     },

crates/settings/src/settings.rs 🔗

@@ -24,18 +24,14 @@ pub struct Settings {
     pub buffer_font_size: f32,
     pub default_buffer_font_size: f32,
     pub vim_mode: bool,
-    pub tab_size: u32,
-    pub hard_tabs: bool,
-    pub soft_wrap: SoftWrap,
-    pub preferred_line_length: u32,
-    pub format_on_save: bool,
-    pub enable_language_server: bool,
-    pub language_overrides: HashMap<Arc<str>, LanguageOverride>,
+    pub language_settings: LanguageSettings,
+    pub language_defaults: HashMap<Arc<str>, LanguageSettings>,
+    pub language_overrides: HashMap<Arc<str>, LanguageSettings>,
     pub theme: Arc<Theme>,
 }
 
 #[derive(Clone, Debug, Default, Deserialize, JsonSchema)]
-pub struct LanguageOverride {
+pub struct LanguageSettings {
     pub tab_size: Option<u32>,
     pub hard_tabs: Option<bool>,
     pub soft_wrap: Option<SoftWrap>,
@@ -67,9 +63,9 @@ pub struct SettingsFileContent {
     #[serde(default)]
     pub enable_language_server: Option<bool>,
     #[serde(flatten)]
-    pub editor: LanguageOverride,
+    pub editor: LanguageSettings,
     #[serde(default)]
-    pub language_overrides: HashMap<Arc<str>, LanguageOverride>,
+    pub language_overrides: HashMap<Arc<str>, LanguageSettings>,
     #[serde(default)]
     pub theme: Option<String>,
 }
@@ -85,68 +81,68 @@ impl Settings {
             buffer_font_size: 15.,
             default_buffer_font_size: 15.,
             vim_mode: false,
-            tab_size: 4,
-            hard_tabs: false,
-            soft_wrap: SoftWrap::None,
-            preferred_line_length: 80,
+            language_settings: Default::default(),
+            language_defaults: Default::default(),
             language_overrides: Default::default(),
-            format_on_save: true,
-            enable_language_server: true,
             projects_online_by_default: true,
             theme,
         })
     }
 
-    pub fn with_overrides(
+    pub fn with_language_defaults(
         mut self,
         language_name: impl Into<Arc<str>>,
-        overrides: LanguageOverride,
+        overrides: LanguageSettings,
     ) -> Self {
-        self.language_overrides
+        self.language_defaults
             .insert(language_name.into(), overrides);
         self
     }
 
     pub fn tab_size(&self, language: Option<&str>) -> u32 {
-        language
-            .and_then(|language| self.language_overrides.get(language))
-            .and_then(|settings| settings.tab_size)
-            .unwrap_or(self.tab_size)
+        self.language_setting(language, |settings| settings.tab_size)
+            .unwrap_or(4)
     }
 
     pub fn hard_tabs(&self, language: Option<&str>) -> bool {
-        language
-            .and_then(|language| self.language_overrides.get(language))
-            .and_then(|settings| settings.hard_tabs)
-            .unwrap_or(self.hard_tabs)
+        self.language_setting(language, |settings| settings.hard_tabs)
+            .unwrap_or(false)
     }
 
     pub fn soft_wrap(&self, language: Option<&str>) -> SoftWrap {
-        language
-            .and_then(|language| self.language_overrides.get(language))
-            .and_then(|settings| settings.soft_wrap)
-            .unwrap_or(self.soft_wrap)
+        self.language_setting(language, |settings| settings.soft_wrap)
+            .unwrap_or(SoftWrap::None)
     }
 
     pub fn preferred_line_length(&self, language: Option<&str>) -> u32 {
-        language
-            .and_then(|language| self.language_overrides.get(language))
-            .and_then(|settings| settings.preferred_line_length)
-            .unwrap_or(self.preferred_line_length)
+        self.language_setting(language, |settings| settings.preferred_line_length)
+            .unwrap_or(80)
     }
 
     pub fn format_on_save(&self, language: Option<&str>) -> bool {
-        language
-            .and_then(|language| self.language_overrides.get(language))
-            .and_then(|settings| settings.format_on_save)
-            .unwrap_or(self.format_on_save)
+        self.language_setting(language, |settings| settings.format_on_save)
+            .unwrap_or(true)
     }
 
     pub fn enable_language_server(&self, language: Option<&str>) -> bool {
-        language
-            .and_then(|language| self.language_overrides.get(language))
-            .and_then(|settings| settings.enable_language_server)
-            .unwrap_or(self.enable_language_server)
+        self.language_setting(language, |settings| settings.enable_language_server)
+            .unwrap_or(true)
+    }
+
+    fn language_setting<F, R>(&self, language: Option<&str>, f: F) -> Option<R>
+    where
+        F: Fn(&LanguageSettings) -> Option<R>,
+    {
+        let mut language_override = None;
+        let mut language_default = None;
+        if let Some(language) = language {
+            language_override = self.language_overrides.get(language).and_then(&f);
+            language_default = self.language_defaults.get(language).and_then(&f);
+        }
+
+        language_override
+            .or_else(|| f(&self.language_settings))
+            .or(language_default)
     }
 
     #[cfg(any(test, feature = "test-support"))]
@@ -156,12 +152,8 @@ impl Settings {
             buffer_font_size: 14.,
             default_buffer_font_size: 14.,
             vim_mode: false,
-            tab_size: 4,
-            hard_tabs: false,
-            soft_wrap: SoftWrap::None,
-            preferred_line_length: 80,
-            format_on_save: true,
-            enable_language_server: true,
+            language_settings: Default::default(),
+            language_defaults: Default::default(),
             language_overrides: Default::default(),
             projects_online_by_default: true,
             theme: gpui::fonts::with_font_cache(cx.font_cache().clone(), || Default::default()),
@@ -200,15 +192,18 @@ impl Settings {
         merge(&mut self.buffer_font_size, data.buffer_font_size);
         merge(&mut self.default_buffer_font_size, data.buffer_font_size);
         merge(&mut self.vim_mode, data.vim_mode);
-        merge(&mut self.format_on_save, data.format_on_save);
-        merge(
-            &mut self.enable_language_server,
+        merge_option(
+            &mut self.language_settings.format_on_save,
+            data.format_on_save,
+        );
+        merge_option(
+            &mut self.language_settings.enable_language_server,
             data.enable_language_server,
         );
-        merge(&mut self.soft_wrap, data.editor.soft_wrap);
-        merge(&mut self.tab_size, data.editor.tab_size);
-        merge(
-            &mut self.preferred_line_length,
+        merge_option(&mut self.language_settings.soft_wrap, data.editor.soft_wrap);
+        merge_option(&mut self.language_settings.tab_size, data.editor.tab_size);
+        merge_option(
+            &mut self.language_settings.preferred_line_length,
             data.editor.preferred_line_length,
         );
 
@@ -257,19 +252,19 @@ pub fn settings_file_json_schema(
         .definitions
         .insert("ThemeName".to_owned(), theme_names_schema);
 
-    // Construct language overrides reference type
-    let language_override_schema_reference = Schema::Object(SchemaObject {
-        reference: Some("#/definitions/LanguageOverride".to_owned()),
+    // Construct language settings reference type
+    let language_settings_schema_reference = Schema::Object(SchemaObject {
+        reference: Some("#/definitions/LanguageSettings".to_owned()),
         ..Default::default()
     });
-    let language_overrides_properties = language_names
+    let language_settings_properties = language_names
         .into_iter()
         .map(|name| {
             (
                 name,
                 Schema::Object(SchemaObject {
                     subschemas: Some(Box::new(SubschemaValidation {
-                        all_of: Some(vec![language_override_schema_reference.clone()]),
+                        all_of: Some(vec![language_settings_schema_reference.clone()]),
                         ..Default::default()
                     })),
                     ..Default::default()
@@ -280,7 +275,7 @@ pub fn settings_file_json_schema(
     let language_overrides_schema = Schema::Object(SchemaObject {
         instance_type: Some(SingleOrVec::Single(Box::new(InstanceType::Object))),
         object: Some(Box::new(ObjectValidation {
-            properties: language_overrides_properties,
+            properties: language_settings_properties,
             ..Default::default()
         })),
         ..Default::default()

crates/zed/src/main.rs 🔗

@@ -73,66 +73,66 @@ fn main() {
     let theme = themes.get(DEFAULT_THEME_NAME).unwrap();
     let default_settings = Settings::new("Zed Mono", &app.font_cache(), theme)
         .unwrap()
-        .with_overrides(
+        .with_language_defaults(
             languages::PLAIN_TEXT.name(),
-            settings::LanguageOverride {
+            settings::LanguageSettings {
                 soft_wrap: Some(settings::SoftWrap::PreferredLineLength),
                 ..Default::default()
             },
         )
-        .with_overrides(
+        .with_language_defaults(
             "C",
-            settings::LanguageOverride {
+            settings::LanguageSettings {
                 tab_size: Some(2),
                 ..Default::default()
             },
         )
-        .with_overrides(
+        .with_language_defaults(
             "C++",
-            settings::LanguageOverride {
+            settings::LanguageSettings {
                 tab_size: Some(2),
                 ..Default::default()
             },
         )
-        .with_overrides(
+        .with_language_defaults(
             "Go",
-            settings::LanguageOverride {
+            settings::LanguageSettings {
                 tab_size: Some(4),
                 hard_tabs: Some(true),
                 ..Default::default()
             },
         )
-        .with_overrides(
+        .with_language_defaults(
             "Markdown",
-            settings::LanguageOverride {
+            settings::LanguageSettings {
                 soft_wrap: Some(settings::SoftWrap::PreferredLineLength),
                 ..Default::default()
             },
         )
-        .with_overrides(
+        .with_language_defaults(
             "Rust",
-            settings::LanguageOverride {
+            settings::LanguageSettings {
                 tab_size: Some(4),
                 ..Default::default()
             },
         )
-        .with_overrides(
+        .with_language_defaults(
             "JavaScript",
-            settings::LanguageOverride {
+            settings::LanguageSettings {
                 tab_size: Some(2),
                 ..Default::default()
             },
         )
-        .with_overrides(
+        .with_language_defaults(
             "TypeScript",
-            settings::LanguageOverride {
+            settings::LanguageSettings {
                 tab_size: Some(2),
                 ..Default::default()
             },
         )
-        .with_overrides(
+        .with_language_defaults(
             "TSX",
-            settings::LanguageOverride {
+            settings::LanguageSettings {
                 tab_size: Some(2),
                 ..Default::default()
             },

crates/zed/src/settings_file.rs 🔗

@@ -93,7 +93,7 @@ pub async fn watch_keymap_file(
 mod tests {
     use super::*;
     use project::FakeFs;
-    use settings::SoftWrap;
+    use settings::{LanguageSettings, SoftWrap};
 
     #[gpui::test]
     async fn test_settings_from_files(cx: &mut gpui::TestAppContext) {
@@ -106,8 +106,10 @@ mod tests {
             {
                 "buffer_font_size": 24,
                 "soft_wrap": "editor_width",
+                "tab_size": 8,
                 "language_overrides": {
                     "Markdown": {
+                        "tab_size": 2,
                         "preferred_line_length": 100,
                         "soft_wrap": "preferred_line_length"
                     }
@@ -123,20 +125,40 @@ mod tests {
         let source2 = WatchedJsonFile::new(fs.clone(), &executor, "/settings2.json".as_ref()).await;
         let source3 = WatchedJsonFile::new(fs.clone(), &executor, "/settings3.json".as_ref()).await;
 
+        let settings = cx.read(Settings::test).with_language_defaults(
+            "JavaScript",
+            LanguageSettings {
+                tab_size: Some(2),
+                ..Default::default()
+            },
+        );
         let mut settings_rx = settings_from_files(
-            cx.read(Settings::test),
+            settings,
             vec![source1, source2, source3],
             ThemeRegistry::new((), cx.font_cache()),
             cx.font_cache(),
         );
 
         let settings = settings_rx.next().await.unwrap();
-        let md_settings = settings.language_overrides.get("Markdown").unwrap();
-        assert_eq!(settings.soft_wrap, SoftWrap::EditorWidth);
         assert_eq!(settings.buffer_font_size, 24.0);
-        assert_eq!(settings.tab_size, 4);
-        assert_eq!(md_settings.soft_wrap, Some(SoftWrap::PreferredLineLength));
-        assert_eq!(md_settings.preferred_line_length, Some(100));
+
+        assert_eq!(settings.soft_wrap(None), SoftWrap::EditorWidth);
+        assert_eq!(
+            settings.soft_wrap(Some("Markdown")),
+            SoftWrap::PreferredLineLength
+        );
+        assert_eq!(
+            settings.soft_wrap(Some("JavaScript")),
+            SoftWrap::EditorWidth
+        );
+
+        assert_eq!(settings.preferred_line_length(None), 80);
+        assert_eq!(settings.preferred_line_length(Some("Markdown")), 100);
+        assert_eq!(settings.preferred_line_length(Some("JavaScript")), 80);
+
+        assert_eq!(settings.tab_size(None), 8);
+        assert_eq!(settings.tab_size(Some("Markdown")), 2);
+        assert_eq!(settings.tab_size(Some("JavaScript")), 8);
 
         fs.save(
             "/settings2.json".as_ref(),
@@ -157,18 +179,46 @@ mod tests {
         .unwrap();
 
         let settings = settings_rx.next().await.unwrap();
-        let md_settings = settings.language_overrides.get("Markdown").unwrap();
-        assert_eq!(settings.soft_wrap, SoftWrap::None);
         assert_eq!(settings.buffer_font_size, 24.0);
-        assert_eq!(settings.tab_size, 2);
-        assert_eq!(md_settings.soft_wrap, Some(SoftWrap::PreferredLineLength));
-        assert_eq!(md_settings.preferred_line_length, Some(120));
+
+        assert_eq!(settings.soft_wrap(None), SoftWrap::None);
+        assert_eq!(
+            settings.soft_wrap(Some("Markdown")),
+            SoftWrap::PreferredLineLength
+        );
+        assert_eq!(settings.soft_wrap(Some("JavaScript")), SoftWrap::None);
+
+        assert_eq!(settings.preferred_line_length(None), 80);
+        assert_eq!(settings.preferred_line_length(Some("Markdown")), 120);
+        assert_eq!(settings.preferred_line_length(Some("JavaScript")), 80);
+
+        assert_eq!(settings.tab_size(None), 2);
+        assert_eq!(settings.tab_size(Some("Markdown")), 2);
+        assert_eq!(settings.tab_size(Some("JavaScript")), 2);
 
         fs.remove_file("/settings2.json".as_ref(), Default::default())
             .await
             .unwrap();
 
         let settings = settings_rx.next().await.unwrap();
-        assert_eq!(settings.tab_size, 4);
+        assert_eq!(settings.buffer_font_size, 24.0);
+
+        assert_eq!(settings.soft_wrap(None), SoftWrap::EditorWidth);
+        assert_eq!(
+            settings.soft_wrap(Some("Markdown")),
+            SoftWrap::PreferredLineLength
+        );
+        assert_eq!(
+            settings.soft_wrap(Some("JavaScript")),
+            SoftWrap::EditorWidth
+        );
+
+        assert_eq!(settings.preferred_line_length(None), 80);
+        assert_eq!(settings.preferred_line_length(Some("Markdown")), 100);
+        assert_eq!(settings.preferred_line_length(Some("JavaScript")), 80);
+
+        assert_eq!(settings.tab_size(None), 8);
+        assert_eq!(settings.tab_size(Some("Markdown")), 2);
+        assert_eq!(settings.tab_size(Some("JavaScript")), 8);
     }
 }