diff --git a/crates/settings/src/settings_store.rs b/crates/settings/src/settings_store.rs index 709b4982706250f91c7aaefc365ddf7613cdf5f4..e971aedd4cd87b4706a465b532846970c2772e23 100644 --- a/crates/settings/src/settings_store.rs +++ b/crates/settings/src/settings_store.rs @@ -148,9 +148,10 @@ pub struct SettingsStore { _setting_file_updates: Task<()>, setting_file_updates_tx: mpsc::UnboundedSender LocalBoxFuture<'static, Result<()>>>>, + file_errors: BTreeMap, } -#[derive(Clone, PartialEq, Debug)] +#[derive(Clone, PartialEq, Eq, Debug)] pub enum SettingsFile { User, Server, @@ -159,6 +160,34 @@ pub enum SettingsFile { Project((WorktreeId, Arc)), } +impl PartialOrd for SettingsFile { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +/// Sorted in order of precedence +impl Ord for SettingsFile { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + use SettingsFile::*; + use std::cmp::Ordering; + match (self, other) { + (User, User) => Ordering::Equal, + (Server, Server) => Ordering::Equal, + (Default, Default) => Ordering::Equal, + (Project((id1, rel_path1)), Project((id2, rel_path2))) => id1 + .cmp(id2) + .then_with(|| rel_path1.cmp(rel_path2).reverse()), + (Project(_), _) => Ordering::Less, + (_, Project(_)) => Ordering::Greater, + (Server, _) => Ordering::Less, + (_, Server) => Ordering::Greater, + (User, _) => Ordering::Less, + (_, User) => Ordering::Greater, + } + } +} + #[derive(Clone)] pub struct Editorconfig { pub is_root: bool, @@ -228,6 +257,7 @@ impl SettingsStore { (setting_file_update)(cx.clone()).await.log_err(); } }), + file_errors: BTreeMap::default(), } } @@ -586,6 +616,24 @@ impl SettingsStore { (SettingsFile::Default, None) } + + fn handle_potential_file_error( + &mut self, + file: SettingsFile, + result: Result, + ) -> Result { + if let Err(err) = result.as_ref() { + let message = err.to_string(); + self.file_errors.insert(file, message); + } else { + self.file_errors.remove(&file); + } + return result; + } + + pub fn error_for_file(&self, file: SettingsFile) -> Option { + self.file_errors.get(&file).cloned() + } } impl SettingsStore { @@ -658,7 +706,10 @@ impl SettingsStore { let settings: UserSettingsContent = if user_settings_content.is_empty() { parse_json_with_comments("{}")? } else { - parse_json_with_comments(user_settings_content)? + self.handle_potential_file_error( + SettingsFile::User, + parse_json_with_comments(user_settings_content), + )? }; self.user_settings = Some(settings); @@ -691,7 +742,10 @@ impl SettingsStore { let settings: Option = if server_settings_content.is_empty() { None } else { - parse_json_with_comments(server_settings_content)? + self.handle_potential_file_error( + SettingsFile::Server, + parse_json_with_comments(server_settings_content), + )? }; // Rewrite the server settings into a content type @@ -740,20 +794,24 @@ impl SettingsStore { zed_settings_changed = self .local_settings .remove(&(root_id, directory_path.clone())) - .is_some() + .is_some(); + self.file_errors + .remove(&SettingsFile::Project((root_id, directory_path.clone()))); } (LocalSettingsKind::Editorconfig, None) => { self.raw_editorconfig_settings .remove(&(root_id, directory_path.clone())); } (LocalSettingsKind::Settings, Some(settings_contents)) => { - let new_settings = parse_json_with_comments::( - settings_contents, - ) - .map_err(|e| InvalidSettingsError::LocalSettings { - path: directory_path.join(local_settings_file_relative_path()), - message: e.to_string(), - })?; + let new_settings = self + .handle_potential_file_error( + SettingsFile::Project((root_id, directory_path.clone())), + parse_json_with_comments::(settings_contents), + ) + .map_err(|e| InvalidSettingsError::LocalSettings { + path: directory_path.join(local_settings_file_relative_path()), + message: e.to_string(), + })?; match self.local_settings.entry((root_id, directory_path.clone())) { btree_map::Entry::Vacant(v) => { v.insert(SettingsContent { @@ -931,6 +989,7 @@ impl SettingsStore { .to_value() } + // todo -> this function never fails, and should not return a result fn recompute_values( &mut self, changed_local_path: Option<(WorktreeId, &RelPath)>, @@ -2032,4 +2091,45 @@ mod tests { let overrides = store.get_overrides_for_field(SettingsFile::Project(wt0_child1), get); assert_eq!(overrides, vec![]); } + + #[test] + fn test_file_ord() { + let wt0_root = + SettingsFile::Project((WorktreeId::from_usize(0), RelPath::empty().into_arc())); + let wt0_child1 = + SettingsFile::Project((WorktreeId::from_usize(0), rel_path("child1").into_arc())); + let wt0_child2 = + SettingsFile::Project((WorktreeId::from_usize(0), rel_path("child2").into_arc())); + + let wt1_root = + SettingsFile::Project((WorktreeId::from_usize(1), RelPath::empty().into_arc())); + let wt1_subdir = + SettingsFile::Project((WorktreeId::from_usize(1), rel_path("subdir").into_arc())); + + let mut files = vec![ + &wt1_root, + &SettingsFile::Default, + &wt0_root, + &wt1_subdir, + &wt0_child2, + &SettingsFile::Server, + &wt0_child1, + &SettingsFile::User, + ]; + + files.sort(); + pretty_assertions::assert_eq!( + files, + vec![ + &wt0_child2, + &wt0_child1, + &wt0_root, + &wt1_subdir, + &wt1_root, + &SettingsFile::Server, + &SettingsFile::User, + &SettingsFile::Default, + ] + ) + } } diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index 5c76008e834b456b29079626813ec80423d53d6f..0485b2608c99f74fbdc86d1cf0807ebb9727c7e8 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -27,9 +27,9 @@ use std::{ }; use title_bar::platform_title_bar::PlatformTitleBar; use ui::{ - ContextMenu, Divider, DividerColor, DropdownMenu, DropdownStyle, IconButtonShape, KeyBinding, - KeybindingHint, PopoverMenu, Switch, SwitchColor, Tooltip, TreeViewItem, WithScrollbar, - prelude::*, + Banner, ContextMenu, Divider, DividerColor, DropdownMenu, DropdownStyle, IconButtonShape, + KeyBinding, KeybindingHint, PopoverMenu, Switch, SwitchColor, Tooltip, TreeViewItem, + WithScrollbar, prelude::*, }; use ui_input::{NumberField, NumberFieldType}; use util::{ResultExt as _, paths::PathStyle, rel_path::RelPath}; @@ -2436,6 +2436,32 @@ impl SettingsWindow { page_content = (active_page_render_fn)(self, window, cx); } + let mut warning_banner = gpui::Empty.into_any_element(); + if let Some(error) = + SettingsStore::global(cx).error_for_file(self.current_file.to_settings()) + { + warning_banner = v_flex() + .pb_4() + .child( + Banner::new() + .severity(Severity::Warning) + .child( + Label::new("Your Settings File is in an Invalid State. Setting Values May Be Incorrect, and Changes May Be Lost") + .size(LabelSize::Large), + ) + .child(Label::new(error).size(LabelSize::Small).color(Color::Muted)) + .action_slot( + Button::new("fix-in-json", "Fix in settings.json") + .tab_index(0_isize) + .style(ButtonStyle::OutlinedGhost) + .on_click(cx.listener(|this, _, _, cx| { + this.open_current_settings_file(cx); + })), + ), + ) + .into_any_element() + } + return v_flex() .id("Settings-ui-page") .on_action(cx.listener(|this, _: &menu::SelectNext, window, cx| { @@ -2497,6 +2523,7 @@ impl SettingsWindow { } window.focus_prev(); })) + .child(warning_banner) .child(page_header) .when(sub_page_stack().is_empty(), |this| { this.vertical_scrollbar_for(self.list_state.clone(), window, cx)