diff --git a/crates/language/src/language_settings.rs b/crates/language/src/language_settings.rs index 77c9a1d18cee14511b9d07fb5b1f9e7886d948df..6121cb6a39a2caa45435e6a056d90ed06856508f 100644 --- a/crates/language/src/language_settings.rs +++ b/crates/language/src/language_settings.rs @@ -1152,6 +1152,13 @@ mod tests { ); } + #[test] + fn test_formatter_deserialization_invalid() { + let raw_auto = "{\"formatter\": {}}"; + let result: Result = serde_json::from_str(raw_auto); + assert!(result.is_err()); + } + #[test] pub fn test_resolve_language_servers() { fn language_server_names(names: &[&str]) -> Vec { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index fcf10d11c2cca7f253b6d8155a5801ebd4d0631f..435c1430243705671e32acdf953bb4ad7dea9af3 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -59,12 +59,14 @@ use node_runtime::NodeRuntime; use parking_lot::{Mutex, RwLock}; use paths::{local_tasks_file_relative_path, local_vscode_tasks_file_relative_path}; pub use prettier_store::PrettierStore; -use project_settings::{ProjectSettings, SettingsObserver}; +use project_settings::{ProjectSettings, SettingsObserver, SettingsObserverEvent}; use remote::SshSession; use rpc::{proto::SSH_PROJECT_ID, AnyProtoClient, ErrorCode}; use search::{SearchInputKind, SearchQuery, SearchResult}; use search_history::SearchHistory; -use settings::{watch_config_file, Settings, SettingsLocation, SettingsStore}; +use settings::{ + watch_config_file, InvalidSettingsError, Settings, SettingsLocation, SettingsStore, +}; use smol::channel::Receiver; use snippet::Snippet; use snippet_provider::SnippetProvider; @@ -230,6 +232,7 @@ pub enum Event { LanguageServerRemoved(LanguageServerId), LanguageServerLog(LanguageServerId, LanguageServerLogType, String), Notification(String), + LocalSettingsUpdated(Result<(), InvalidSettingsError>), LanguageServerPrompt(LanguageServerPromptRequest), LanguageNotFound(Model), ActiveEntryChanged(Option), @@ -644,6 +647,8 @@ impl Project { let settings_observer = cx.new_model(|cx| { SettingsObserver::new_local(fs.clone(), worktree_store.clone(), cx) }); + cx.subscribe(&settings_observer, Self::on_settings_observer_event) + .detach(); let environment = ProjectEnvironment::new(&worktree_store, env, cx); let lsp_store = cx.new_model(|cx| { @@ -729,6 +734,8 @@ impl Project { let settings_observer = cx.new_model(|cx| { SettingsObserver::new_ssh(ssh.clone().into(), worktree_store.clone(), cx) }); + cx.subscribe(&settings_observer, Self::on_settings_observer_event) + .detach(); let environment = ProjectEnvironment::new(&worktree_store, None, cx); let lsp_store = cx.new_model(|cx| { @@ -913,6 +920,8 @@ impl Project { cx.subscribe(&buffer_store, Self::on_buffer_store_event) .detach(); cx.subscribe(&lsp_store, Self::on_lsp_store_event).detach(); + cx.subscribe(&settings_observer, Self::on_settings_observer_event) + .detach(); let mut this = Self { buffer_ordered_messages_tx: tx, @@ -2058,6 +2067,19 @@ impl Project { } } + fn on_settings_observer_event( + &mut self, + _: Model, + event: &SettingsObserverEvent, + cx: &mut ModelContext, + ) { + match event { + SettingsObserverEvent::LocalSettingsUpdated(error) => { + cx.emit(Event::LocalSettingsUpdated(error.clone())) + } + } + } + fn on_worktree_store_event( &mut self, _: Model, diff --git a/crates/project/src/project_settings.rs b/crates/project/src/project_settings.rs index 2eeb8408961186d948412d5ba21be11604bd19c2..9a7c80703c734c6c926cf346934423ae8c0acb54 100644 --- a/crates/project/src/project_settings.rs +++ b/crates/project/src/project_settings.rs @@ -1,11 +1,11 @@ use collections::HashMap; use fs::Fs; -use gpui::{AppContext, AsyncAppContext, BorrowAppContext, Model, ModelContext}; +use gpui::{AppContext, AsyncAppContext, BorrowAppContext, EventEmitter, Model, ModelContext}; use paths::local_settings_file_relative_path; use rpc::{proto, AnyProtoClient, TypedEnvelope}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use settings::{Settings, SettingsSources, SettingsStore}; +use settings::{InvalidSettingsError, Settings, SettingsSources, SettingsStore}; use std::{ path::{Path, PathBuf}, sync::Arc, @@ -176,6 +176,13 @@ pub enum SettingsObserverMode { Remote, } +#[derive(Clone, Debug, PartialEq)] +pub enum SettingsObserverEvent { + LocalSettingsUpdated(Result<(), InvalidSettingsError>), +} + +impl EventEmitter for SettingsObserver {} + pub struct SettingsObserver { mode: SettingsObserverMode, downstream_client: Option, @@ -415,11 +422,16 @@ impl SettingsObserver { ) { let worktree_id = worktree.read(cx).id(); let remote_worktree_id = worktree.read(cx).id(); - cx.update_global::(|store, cx| { + + let result = cx.update_global::>(|store, cx| { for (directory, file_content) in settings_contents { - store - .set_local_settings(worktree_id, directory.clone(), file_content.as_deref(), cx) - .log_err(); + store.set_local_settings( + worktree_id, + directory.clone(), + file_content.as_deref(), + cx, + )?; + if let Some(downstream_client) = &self.downstream_client { downstream_client .send(proto::UpdateWorktreeSettings { @@ -431,6 +443,25 @@ impl SettingsObserver { .log_err(); } } - }) + anyhow::Ok(()) + }); + + match result { + Err(error) => { + if let Ok(error) = error.downcast::() { + if let InvalidSettingsError::LocalSettings { + ref path, + ref message, + } = error + { + log::error!("Failed to set local settings in {:?}: {:?}", path, message); + cx.emit(SettingsObserverEvent::LocalSettingsUpdated(Err(error))); + } + } + } + Ok(()) => { + cx.emit(SettingsObserverEvent::LocalSettingsUpdated(Ok(()))); + } + } } } diff --git a/crates/settings/src/settings.rs b/crates/settings/src/settings.rs index 5ece3f867e4ff4b29bc7219859287ea761ee0289..f1f8591bba4525c842cc85a38b81ee34724216d2 100644 --- a/crates/settings/src/settings.rs +++ b/crates/settings/src/settings.rs @@ -13,7 +13,9 @@ pub use editable_setting_control::*; pub use json_schema::*; pub use keymap_file::KeymapFile; pub use settings_file::*; -pub use settings_store::{Settings, SettingsLocation, SettingsSources, SettingsStore}; +pub use settings_store::{ + InvalidSettingsError, Settings, SettingsLocation, SettingsSources, SettingsStore, +}; #[derive(Copy, Clone, PartialEq, Eq, Debug, Hash, PartialOrd, Ord)] pub struct WorktreeId(usize); diff --git a/crates/settings/src/settings_store.rs b/crates/settings/src/settings_store.rs index 3ef8bffe2d3dedadb51cd1e1f34366e2ad864a12..20bf52f2c57ef0be5463fbbf6de8976bea0c6289 100644 --- a/crates/settings/src/settings_store.rs +++ b/crates/settings/src/settings_store.rs @@ -3,6 +3,7 @@ use collections::{btree_map, hash_map, BTreeMap, HashMap}; use fs::Fs; use futures::{channel::mpsc, future::LocalBoxFuture, FutureExt, StreamExt}; use gpui::{AppContext, AsyncAppContext, BorrowAppContext, Global, Task, UpdateGlobal}; +use paths::local_settings_file_relative_path; use schemars::{gen::SchemaGenerator, schema::RootSchema, JsonSchema}; use serde::{de::DeserializeOwned, Deserialize as _, Serialize}; use smallvec::SmallVec; @@ -10,7 +11,7 @@ use std::{ any::{type_name, Any, TypeId}, fmt::Debug, ops::Range, - path::Path, + path::{Path, PathBuf}, str, sync::{Arc, LazyLock}, }; @@ -694,9 +695,14 @@ impl SettingsStore { .deserialize_setting(&self.raw_extension_settings) .log_err(); - let user_settings = setting_value - .deserialize_setting(&self.raw_user_settings) - .log_err(); + let user_settings = match setting_value.deserialize_setting(&self.raw_user_settings) { + Ok(settings) => Some(settings), + Err(error) => { + return Err(anyhow!(InvalidSettingsError::UserSettings { + message: error.to_string() + })); + } + }; let mut release_channel_settings = None; if let Some(release_settings) = &self @@ -746,34 +752,43 @@ impl SettingsStore { break; } - if let Some(local_settings) = - setting_value.deserialize_setting(local_settings).log_err() - { - paths_stack.push(Some((*root_id, path.as_ref()))); - project_settings_stack.push(local_settings); - - // If a local settings file changed, then avoid recomputing local - // settings for any path outside of that directory. - if changed_local_path.map_or(false, |(changed_root_id, changed_local_path)| { - *root_id != changed_root_id || !path.starts_with(changed_local_path) - }) { - continue; - } - - if let Some(value) = setting_value - .load_setting( - SettingsSources { - default: &default_settings, - extensions: extension_settings.as_ref(), - user: user_settings.as_ref(), - release_channel: release_channel_settings.as_ref(), - project: &project_settings_stack.iter().collect::>(), + match setting_value.deserialize_setting(local_settings) { + Ok(local_settings) => { + paths_stack.push(Some((*root_id, path.as_ref()))); + project_settings_stack.push(local_settings); + + // If a local settings file changed, then avoid recomputing local + // settings for any path outside of that directory. + if changed_local_path.map_or( + false, + |(changed_root_id, changed_local_path)| { + *root_id != changed_root_id || !path.starts_with(changed_local_path) }, - cx, - ) - .log_err() - { - setting_value.set_local_value(*root_id, path.clone(), value); + ) { + continue; + } + + if let Some(value) = setting_value + .load_setting( + SettingsSources { + default: &default_settings, + extensions: extension_settings.as_ref(), + user: user_settings.as_ref(), + release_channel: release_channel_settings.as_ref(), + project: &project_settings_stack.iter().collect::>(), + }, + cx, + ) + .log_err() + { + setting_value.set_local_value(*root_id, path.clone(), value); + } + } + Err(error) => { + return Err(anyhow!(InvalidSettingsError::LocalSettings { + path: path.join(local_settings_file_relative_path()), + message: error.to_string() + })); } } } @@ -782,6 +797,24 @@ impl SettingsStore { } } +#[derive(Debug, Clone, PartialEq)] +pub enum InvalidSettingsError { + LocalSettings { path: PathBuf, message: String }, + UserSettings { message: String }, +} + +impl std::fmt::Display for InvalidSettingsError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + InvalidSettingsError::LocalSettings { message, .. } + | InvalidSettingsError::UserSettings { message } => { + write!(f, "{}", message) + } + } + } +} +impl std::error::Error for InvalidSettingsError {} + impl Debug for SettingsStore { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("SettingsStore") diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 92a85299f47b7059a8ba401d7dbf1e8795e4d017..1fbeab38a2e8b4363066d2f359a56c8aeaadd58f 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -64,7 +64,7 @@ use project::{ use remote::{SshConnectionOptions, SshSession}; use serde::Deserialize; use session::AppSession; -use settings::Settings; +use settings::{InvalidSettingsError, Settings}; use shared_screen::SharedScreen; use sqlez::{ bindable::{Bind, Column, StaticColumnCount}, @@ -832,6 +832,23 @@ impl Workspace { } } + project::Event::LocalSettingsUpdated(result) => { + struct LocalSettingsUpdated; + let id = NotificationId::unique::(); + + match result { + Err(InvalidSettingsError::LocalSettings { message, path }) => { + let full_message = + format!("Failed to set local settings in {:?}:\n{}", path, message); + this.show_notification(id, cx, |cx| { + cx.new_view(|_| MessageNotification::new(full_message.clone())) + }) + } + Err(_) => {} + Ok(_) => this.dismiss_notification(&id, cx), + } + } + project::Event::Notification(message) => { struct ProjectNotification; diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 3104001f9927262dbe9444bc038c964c017a3ab6..6ecdbb224f3d91263ca669980a604d79e4de1eb9 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -34,7 +34,9 @@ use parking_lot::Mutex; use recent_projects::open_ssh_project; use release_channel::{AppCommitSha, AppVersion}; use session::{AppSession, Session}; -use settings::{handle_settings_file_changes, watch_config_file, Settings, SettingsStore}; +use settings::{ + handle_settings_file_changes, watch_config_file, InvalidSettingsError, Settings, SettingsStore, +}; use simplelog::ConfigBuilder; use smol::process::Command; use std::{ @@ -626,20 +628,28 @@ fn handle_settings_changed(error: Option, cx: &mut AppContext) { for workspace in workspace::local_workspace_windows(cx) { workspace - .update(cx, |workspace, cx| match &error { - Some(error) => { - workspace.show_notification(id.clone(), cx, |cx| { - cx.new_view(|_| { - MessageNotification::new(format!("Invalid settings file\n{error}")) + .update(cx, |workspace, cx| { + match error + .as_ref() + .and_then(|error| error.downcast_ref::()) + { + Some(InvalidSettingsError::UserSettings { message }) => { + workspace.show_notification(id.clone(), cx, |cx| { + cx.new_view(|_| { + MessageNotification::new(format!( + "Invalid user settings file\n{message}" + )) .with_click_message("Open settings file") .on_click(|cx| { cx.dispatch_action(zed_actions::OpenSettings.boxed_clone()); cx.emit(DismissEvent); }) - }) - }); + }) + }); + } + None => workspace.dismiss_notification(&id, cx), + _ => {} } - None => workspace.dismiss_notification(&id, cx), }) .log_err(); }