Merge pull request #1337 from zed-industries/respect-hard-tabs-setting

Max Brunsfeld created

Simplify setting merging, fix ignored hard_tabs setting

Change summary

crates/settings/src/settings.rs |  99 +++++++++--------------------
crates/zed/src/main.rs          |  34 ++-------
crates/zed/src/settings_file.rs | 118 +++++++++++++---------------------
3 files changed, 84 insertions(+), 167 deletions(-)

Detailed changes

crates/settings/src/settings.rs 🔗

@@ -136,6 +136,37 @@ impl Settings {
         }
     }
 
+    pub fn set_user_settings(
+        &mut self,
+        data: SettingsFileContent,
+        theme_registry: &ThemeRegistry,
+        font_cache: &FontCache,
+    ) {
+        if let Some(value) = &data.buffer_font_family {
+            if let Some(id) = font_cache.load_family(&[value]).log_err() {
+                self.buffer_font_family = id;
+            }
+        }
+        if let Some(value) = &data.theme {
+            if let Some(theme) = theme_registry.get(&value.to_string()).log_err() {
+                self.theme = theme;
+            }
+        }
+
+        merge(
+            &mut self.projects_online_by_default,
+            data.projects_online_by_default,
+        );
+        merge(&mut self.buffer_font_size, data.buffer_font_size);
+        merge(&mut self.default_buffer_font_size, data.buffer_font_size);
+        merge(&mut self.hover_popover_enabled, data.hover_popover_enabled);
+        merge(&mut self.vim_mode, data.vim_mode);
+        merge(&mut self.autosave, data.autosave);
+
+        self.editor_overrides = data.editor;
+        self.language_overrides = data.languages;
+    }
+
     pub fn with_language_defaults(
         mut self,
         language_name: impl Into<Arc<str>>,
@@ -213,68 +244,6 @@ impl Settings {
             cx.set_global(settings.clone());
         });
     }
-
-    pub fn merge(
-        &mut self,
-        data: &SettingsFileContent,
-        theme_registry: &ThemeRegistry,
-        font_cache: &FontCache,
-    ) {
-        if let Some(value) = &data.buffer_font_family {
-            if let Some(id) = font_cache.load_family(&[value]).log_err() {
-                self.buffer_font_family = id;
-            }
-        }
-        if let Some(value) = &data.theme {
-            if let Some(theme) = theme_registry.get(&value.to_string()).log_err() {
-                self.theme = theme;
-            }
-        }
-
-        merge(
-            &mut self.projects_online_by_default,
-            data.projects_online_by_default,
-        );
-        merge(&mut self.buffer_font_size, data.buffer_font_size);
-        merge(&mut self.default_buffer_font_size, data.buffer_font_size);
-        merge(&mut self.hover_popover_enabled, data.hover_popover_enabled);
-        merge(&mut self.vim_mode, data.vim_mode);
-        merge(&mut self.autosave, data.autosave);
-
-        merge_option(
-            &mut self.editor_overrides.format_on_save,
-            data.editor.format_on_save.clone(),
-        );
-        merge_option(
-            &mut self.editor_overrides.enable_language_server,
-            data.editor.enable_language_server,
-        );
-        merge_option(&mut self.editor_overrides.soft_wrap, data.editor.soft_wrap);
-        merge_option(&mut self.editor_overrides.tab_size, data.editor.tab_size);
-        merge_option(
-            &mut self.editor_overrides.preferred_line_length,
-            data.editor.preferred_line_length,
-        );
-
-        for (language_name, settings) in data.languages.clone().into_iter() {
-            let target = self
-                .language_overrides
-                .entry(language_name.into())
-                .or_default();
-
-            merge_option(&mut target.tab_size, settings.tab_size);
-            merge_option(&mut target.soft_wrap, settings.soft_wrap);
-            merge_option(&mut target.format_on_save, settings.format_on_save);
-            merge_option(
-                &mut target.enable_language_server,
-                settings.enable_language_server,
-            );
-            merge_option(
-                &mut target.preferred_line_length,
-                settings.preferred_line_length,
-            );
-        }
-    }
 }
 
 pub fn settings_file_json_schema(
@@ -351,12 +320,6 @@ fn merge<T: Copy>(target: &mut T, value: Option<T>) {
     }
 }
 
-fn merge_option<T>(target: &mut Option<T>, value: Option<T>) {
-    if value.is_some() {
-        *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()),

crates/zed/src/main.rs 🔗

@@ -45,7 +45,7 @@ use zed::{
     self, build_window_options,
     fs::RealFs,
     initialize_workspace, languages, menus,
-    settings_file::{settings_from_files, watch_keymap_file, WatchedJsonFile},
+    settings_file::{watch_keymap_file, watch_settings_file, WatchedJsonFile},
 };
 
 fn main() {
@@ -126,36 +126,13 @@ fn main() {
 
         let db = cx.background().block(db);
         let (settings_file, keymap_file) = cx.background().block(config_files).unwrap();
-        let mut settings_rx = settings_from_files(
-            default_settings,
-            vec![settings_file],
-            themes.clone(),
-            cx.font_cache().clone(),
-        );
 
         cx.spawn(|cx| watch_themes(fs.clone(), themes.clone(), cx))
             .detach();
         cx.spawn(|cx| watch_keymap_file(keymap_file, cx)).detach();
+        cx.spawn(|cx| watch_settings_file(default_settings, settings_file, themes.clone(), cx))
+            .detach();
 
-        let settings = cx.background().block(settings_rx.next()).unwrap();
-        cx.spawn(|mut cx| async move {
-            while let Some(settings) = settings_rx.next().await {
-                cx.update(|cx| {
-                    cx.update_global(|s, _| *s = settings);
-                    cx.refresh_windows();
-                });
-            }
-        })
-        .detach();
-
-        cx.observe_global::<Settings, _>({
-            let languages = languages.clone();
-            move |cx| {
-                languages.set_theme(cx.global::<Settings>().theme.clone());
-            }
-        })
-        .detach();
-        cx.set_global(settings);
         cx.spawn({
             let languages = languages.clone();
             |cx| async move {
@@ -164,6 +141,11 @@ fn main() {
             }
         })
         .detach();
+        cx.observe_global::<Settings, _>({
+            let languages = languages.clone();
+            move |cx| languages.set_theme(cx.global::<Settings>().theme.clone())
+        })
+        .detach();
 
         let project_store = cx.add_model(|_| ProjectStore::new(db.clone()));
         let app_state = Arc::new(AppState {

crates/zed/src/settings_file.rs 🔗

@@ -1,5 +1,5 @@
-use futures::{stream, StreamExt};
-use gpui::{executor, AsyncAppContext, FontCache};
+use futures::StreamExt;
+use gpui::{executor, AsyncAppContext};
 use postage::sink::Sink as _;
 use postage::{prelude::Stream, watch};
 use project::Fs;
@@ -51,29 +51,20 @@ where
     }
 }
 
-pub fn settings_from_files(
+pub async fn watch_settings_file(
     defaults: Settings,
-    sources: Vec<WatchedJsonFile<SettingsFileContent>>,
+    mut file: WatchedJsonFile<SettingsFileContent>,
     theme_registry: Arc<ThemeRegistry>,
-    font_cache: Arc<FontCache>,
-) -> impl futures::stream::Stream<Item = Settings> {
-    stream::select_all(sources.iter().enumerate().map(|(i, source)| {
-        let mut rx = source.0.clone();
-        // Consume the initial item from all of the constituent file watches but one.
-        // This way, the stream will yield exactly one item for the files' initial
-        // state, and won't return any more items until the files change.
-        if i > 0 {
-            rx.try_recv().ok();
-        }
-        rx
-    }))
-    .map(move |_| {
-        let mut settings = defaults.clone();
-        for source in &sources {
-            settings.merge(&*source.0.borrow(), &theme_registry, &font_cache);
-        }
-        settings
-    })
+    mut cx: AsyncAppContext,
+) {
+    while let Some(content) = file.0.recv().await {
+        cx.update(|cx| {
+            let mut settings = defaults.clone();
+            settings.set_user_settings(content, &theme_registry, &cx.font_cache());
+            cx.set_global(settings);
+            cx.refresh_windows();
+        });
+    }
 }
 
 pub async fn watch_keymap_file(
@@ -96,12 +87,13 @@ mod tests {
     use settings::{EditorSettings, SoftWrap};
 
     #[gpui::test]
-    async fn test_settings_from_files(cx: &mut gpui::TestAppContext) {
+    async fn test_watch_settings_files(cx: &mut gpui::TestAppContext) {
         let executor = cx.background();
         let fs = FakeFs::new(executor.clone());
+        let font_cache = cx.font_cache();
 
         fs.save(
-            "/settings1.json".as_ref(),
+            "/settings.json".as_ref(),
             &r#"
             {
                 "buffer_font_size": 24,
@@ -122,25 +114,27 @@ mod tests {
         .await
         .unwrap();
 
-        let source1 = WatchedJsonFile::new(fs.clone(), &executor, "/settings1.json".as_ref()).await;
-        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 source = WatchedJsonFile::new(fs.clone(), &executor, "/settings.json".as_ref()).await;
 
-        let settings = cx.read(Settings::test).with_language_defaults(
+        let default_settings = cx.read(Settings::test).with_language_defaults(
             "JavaScript",
             EditorSettings {
                 tab_size: Some(2.try_into().unwrap()),
                 ..Default::default()
             },
         );
-        let mut settings_rx = settings_from_files(
-            settings,
-            vec![source1, source2, source3],
-            ThemeRegistry::new((), cx.font_cache()),
-            cx.font_cache(),
-        );
-
-        let settings = settings_rx.next().await.unwrap();
+        cx.spawn(|cx| {
+            watch_settings_file(
+                default_settings.clone(),
+                source,
+                ThemeRegistry::new((), font_cache),
+                cx,
+            )
+        })
+        .detach();
+
+        cx.foreground().run_until_parked();
+        let settings = cx.read(|cx| cx.global::<Settings>().clone());
         assert_eq!(settings.buffer_font_size, 24.0);
 
         assert_eq!(settings.soft_wrap(None), SoftWrap::EditorWidth);
@@ -162,47 +156,18 @@ mod tests {
         assert_eq!(settings.tab_size(Some("JavaScript")).get(), 8);
 
         fs.save(
-            "/settings2.json".as_ref(),
-            &r#"
-            {
-                "tab_size": 2,
-                "soft_wrap": "none",
-                "language_overrides": {
-                    "Markdown": {
-                        "preferred_line_length": 120
-                    }
-                }
-            }
-            "#
-            .into(),
+            "/settings.json".as_ref(),
+            &"(garbage)".into(),
             Default::default(),
         )
         .await
         .unwrap();
+        // fs.remove_file("/settings.json".as_ref(), Default::default())
+        //     .await
+        //     .unwrap();
 
-        let settings = settings_rx.next().await.unwrap();
-        assert_eq!(settings.buffer_font_size, 24.0);
-
-        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).get(), 2);
-        assert_eq!(settings.tab_size(Some("Markdown")).get(), 2);
-        assert_eq!(settings.tab_size(Some("JavaScript")).get(), 2);
-
-        fs.remove_file("/settings2.json".as_ref(), Default::default())
-            .await
-            .unwrap();
-
-        let settings = settings_rx.next().await.unwrap();
+        cx.foreground().run_until_parked();
+        let settings = cx.read(|cx| cx.global::<Settings>().clone());
         assert_eq!(settings.buffer_font_size, 24.0);
 
         assert_eq!(settings.soft_wrap(None), SoftWrap::EditorWidth);
@@ -222,5 +187,12 @@ mod tests {
         assert_eq!(settings.tab_size(None).get(), 8);
         assert_eq!(settings.tab_size(Some("Markdown")).get(), 2);
         assert_eq!(settings.tab_size(Some("JavaScript")).get(), 8);
+
+        fs.remove_file("/settings.json".as_ref(), Default::default())
+            .await
+            .unwrap();
+        cx.foreground().run_until_parked();
+        let settings = cx.read(|cx| cx.global::<Settings>().clone());
+        assert_eq!(settings.buffer_font_size, default_settings.buffer_font_size);
     }
 }