From 4b533c339d24ebd472dfad5acc7d5f56185763cb Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 5 Jul 2022 09:25:22 +0200 Subject: [PATCH 1/9] Introduce a new, language-overridable `Autosave` setting --- crates/settings/src/settings.rs | 17 +++++++++++++++++ crates/zed/src/zed.rs | 1 - 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/crates/settings/src/settings.rs b/crates/settings/src/settings.rs index 1e84e5bec6cfbebf157d9511ff91b46aa21df609..886035c543785e8bc66e12630da3be047036e8a6 100644 --- a/crates/settings/src/settings.rs +++ b/crates/settings/src/settings.rs @@ -39,6 +39,7 @@ pub struct LanguageSettings { pub preferred_line_length: Option, pub format_on_save: Option, pub enable_language_server: Option, + pub autosave: Option, } #[derive(Copy, Clone, Debug, Deserialize, PartialEq, Eq, JsonSchema)] @@ -49,6 +50,15 @@ pub enum SoftWrap { PreferredLineLength, } +#[derive(Copy, Clone, Debug, Deserialize, PartialEq, Eq, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum Autosave { + Off, + AfterDelay { milliseconds: usize }, + OnFocusChange, + OnWindowChange, +} + #[derive(Clone, Debug, Default, Deserialize, JsonSchema)] pub struct SettingsFileContent { #[serde(default)] @@ -128,6 +138,11 @@ impl Settings { .unwrap_or(true) } + pub fn autosave(&self, language: Option<&str>) -> Autosave { + self.language_setting(language, |settings| settings.autosave) + .unwrap_or(Autosave::Off) + } + pub fn enable_language_server(&self, language: Option<&str>) -> bool { self.language_setting(language, |settings| settings.enable_language_server) .unwrap_or(true) @@ -212,6 +227,7 @@ impl Settings { &mut self.language_settings.preferred_line_length, data.editor.preferred_line_length, ); + merge_option(&mut self.language_settings.autosave, data.editor.autosave); for (language_name, settings) in data.language_overrides.clone().into_iter() { let target = self @@ -230,6 +246,7 @@ impl Settings { &mut target.preferred_line_length, settings.preferred_line_length, ); + merge_option(&mut target.autosave, settings.autosave); } } } diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 548b726af9e517b158e1e04ddbda63865979d6a3..7240aaef2f841524ae96cc0958df9058277e609a 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -97,7 +97,6 @@ pub fn init(app_state: &Arc, cx: &mut gpui::MutableAppContext) { cx.add_action({ let app_state = app_state.clone(); move |_: &mut Workspace, _: &OpenSettings, cx: &mut ViewContext| { - println!("open settings"); open_config_file(&SETTINGS_PATH, app_state.clone(), cx); } }); From 7a6010e7dc6492ab16d18d07bf5bfe60b85fb152 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 5 Jul 2022 11:12:18 +0200 Subject: [PATCH 2/9] Fix unsafe memory access when converting entity handles --- crates/gpui/src/app.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index ad38520032560b5b0901eb2ce7702632191f1a6b..8a67351c448782ed7dbfc47f39472f4be638766d 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -4451,7 +4451,7 @@ impl AnyViewHandle { handle_id: self.handle_id, }); unsafe { - Arc::decrement_strong_count(&self.ref_counts); + Arc::decrement_strong_count(Arc::as_ptr(&self.ref_counts)); } std::mem::forget(self); result @@ -4517,8 +4517,9 @@ impl From> for AnyViewHandle { #[cfg(any(test, feature = "test-support"))] handle_id: handle.handle_id, }; + unsafe { - Arc::decrement_strong_count(&handle.ref_counts); + Arc::decrement_strong_count(Arc::as_ptr(&handle.ref_counts)); } std::mem::forget(handle); any_handle @@ -4580,7 +4581,7 @@ impl AnyModelHandle { handle_id: self.handle_id, }); unsafe { - Arc::decrement_strong_count(&self.ref_counts); + Arc::decrement_strong_count(Arc::as_ptr(&self.ref_counts)); } std::mem::forget(self); result From d589017a8088126ad643a11a6e2cacbde012e295 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 5 Jul 2022 12:15:40 +0200 Subject: [PATCH 3/9] Add `ViewContext::observe_window_activation` --- crates/gpui/src/app.rs | 156 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 149 insertions(+), 7 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 8a67351c448782ed7dbfc47f39472f4be638766d..917112062876b1efc07b62961ba72b553f846032 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -783,6 +783,7 @@ type FocusObservationCallback = Box bool>; type GlobalObservationCallback = Box; type ReleaseObservationCallback = Box; type ActionObservationCallback = Box; +type WindowActivationCallback = Box bool>; type DeserializeActionCallback = fn(json: &str) -> anyhow::Result>; type WindowShouldCloseSubscriptionCallback = Box bool>; @@ -809,6 +810,8 @@ pub struct MutableAppContext { global_observations: Arc>>>>, release_observations: Arc>>>, + window_activation_observations: + Arc>>>>, action_dispatch_observations: Arc>>, presenters_and_platform_windows: HashMap>, Box)>, @@ -862,6 +865,7 @@ impl MutableAppContext { focus_observations: Default::default(), release_observations: Default::default(), global_observations: Default::default(), + window_activation_observations: Default::default(), action_dispatch_observations: Default::default(), presenters_and_platform_windows: HashMap::new(), foreground, @@ -1358,6 +1362,24 @@ impl MutableAppContext { } } + fn observe_window_activation(&mut self, window_id: usize, callback: F) -> Subscription + where + F: 'static + FnMut(bool, &mut MutableAppContext) -> bool, + { + let subscription_id = post_inc(&mut self.next_subscription_id); + self.pending_effects + .push_back(Effect::WindowActivationObservation { + window_id, + subscription_id, + callback: Box::new(callback), + }); + Subscription::WindowActivationObservation { + id: subscription_id, + window_id, + observations: Some(Arc::downgrade(&self.window_activation_observations)), + } + } + pub fn defer(&mut self, callback: impl 'static + FnOnce(&mut MutableAppContext)) { self.pending_effects.push_back(Effect::Deferred { callback: Box::new(callback), @@ -1715,19 +1737,16 @@ impl MutableAppContext { self.update(|this| { let window_id = post_inc(&mut this.next_window_id); let root_view = this.add_view(window_id, build_root_view); - this.cx.windows.insert( window_id, Window { root_view: root_view.clone().into(), focused_view_id: Some(root_view.id()), - is_active: true, + is_active: false, invalidation: None, }, ); - root_view.update(this, |view, cx| { - view.on_focus(cx); - }); + root_view.update(this, |view, cx| view.on_focus(cx)); this.open_platform_window(window_id, window_options); (window_id, root_view) @@ -1995,10 +2014,19 @@ impl MutableAppContext { .get_or_insert(WindowInvalidation::default()); } } + Effect::WindowActivationObservation { + window_id, + subscription_id, + callback, + } => self.handle_window_activation_observation_effect( + window_id, + subscription_id, + callback, + ), Effect::ActivateWindow { window_id, is_active, - } => self.handle_activation_effect(window_id, is_active), + } => self.handle_window_activation_effect(window_id, is_active), Effect::RefreshWindows => { refreshing = true; } @@ -2361,7 +2389,31 @@ impl MutableAppContext { } } - fn handle_activation_effect(&mut self, window_id: usize, active: bool) { + fn handle_window_activation_observation_effect( + &mut self, + window_id: usize, + subscription_id: usize, + callback: WindowActivationCallback, + ) { + match self + .window_activation_observations + .lock() + .entry(window_id) + .or_default() + .entry(subscription_id) + { + btree_map::Entry::Vacant(entry) => { + entry.insert(Some(callback)); + } + // Observation was dropped before effect was processed + btree_map::Entry::Occupied(entry) => { + debug_assert!(entry.get().is_none()); + entry.remove(); + } + } + } + + fn handle_window_activation_effect(&mut self, window_id: usize, active: bool) { if self .cx .windows @@ -2385,6 +2437,34 @@ impl MutableAppContext { this.cx.views.insert((window_id, view_id), view); } + let callbacks = this + .window_activation_observations + .lock() + .remove(&window_id); + if let Some(callbacks) = callbacks { + for (id, callback) in callbacks { + if let Some(mut callback) = callback { + let alive = callback(active, this); + if alive { + match this + .window_activation_observations + .lock() + .entry(window_id) + .or_default() + .entry(id) + { + btree_map::Entry::Vacant(entry) => { + entry.insert(Some(callback)); + } + btree_map::Entry::Occupied(entry) => { + entry.remove(); + } + } + } + } + } + } + Some(()) }); } @@ -2842,6 +2922,11 @@ pub enum Effect { window_id: usize, is_active: bool, }, + WindowActivationObservation { + window_id: usize, + subscription_id: usize, + callback: WindowActivationCallback, + }, RefreshWindows, ActionDispatchNotification { action_id: TypeId, @@ -2934,6 +3019,15 @@ impl Debug for Effect { .debug_struct("Effect::RefreshWindow") .field("window_id", window_id) .finish(), + Effect::WindowActivationObservation { + window_id, + subscription_id, + .. + } => f + .debug_struct("Effect::WindowActivationObservation") + .field("window_id", window_id) + .field("subscription_id", subscription_id) + .finish(), Effect::ActivateWindow { window_id, is_active, @@ -3518,6 +3612,24 @@ impl<'a, T: View> ViewContext<'a, T> { }) } + pub fn observe_window_activation(&mut self, mut callback: F) -> Subscription + where + F: 'static + FnMut(&mut T, bool, &mut ViewContext), + { + let observer = self.weak_handle(); + self.app + .observe_window_activation(self.window_id(), move |active, cx| { + if let Some(observer) = observer.upgrade(cx) { + observer.update(cx, |observer, cx| { + callback(observer, active, cx); + }); + true + } else { + false + } + }) + } + pub fn emit(&mut self, payload: T::Event) { self.app.pending_effects.push_back(Effect::Event { entity_id: self.view_id, @@ -4833,6 +4945,12 @@ pub enum Subscription { id: usize, observations: Option>>>, }, + WindowActivationObservation { + id: usize, + window_id: usize, + observations: + Option>>>>>, + }, } impl Subscription { @@ -4859,6 +4977,9 @@ impl Subscription { Subscription::ActionObservation { observations, .. } => { observations.take(); } + Subscription::WindowActivationObservation { observations, .. } => { + observations.take(); + } } } } @@ -4972,6 +5093,27 @@ impl Drop for Subscription { observations.lock().remove(&id); } } + Subscription::WindowActivationObservation { + id, + window_id, + observations, + } => { + if let Some(observations) = observations.as_ref().and_then(Weak::upgrade) { + match observations + .lock() + .entry(*window_id) + .or_default() + .entry(*id) + { + btree_map::Entry::Vacant(entry) => { + entry.insert(None); + } + btree_map::Entry::Occupied(entry) => { + entry.remove(); + } + } + } + } } } } From 885172f4dda7d89a78fd33b8317da03c69e37397 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 5 Jul 2022 13:01:27 +0200 Subject: [PATCH 4/9] Honor `Autosave` setting in `Editor` --- crates/editor/src/editor.rs | 78 ++++++++++++++++++++++++++++----- crates/settings/src/settings.rs | 16 +++---- 2 files changed, 75 insertions(+), 19 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 9c950b27be83cd3d66d1410e737f32bae11e28c6..ec65db3bf783152a60d1ebd7ee921572d0d74e2a 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -17,6 +17,7 @@ use collections::{BTreeMap, Bound, HashMap, HashSet, VecDeque}; pub use display_map::DisplayPoint; use display_map::*; pub use element::*; +use futures::{channel::oneshot, FutureExt}; use fuzzy::{StringMatch, StringMatchCandidate}; use gpui::{ actions, @@ -28,8 +29,8 @@ use gpui::{ impl_actions, impl_internal_actions, platform::CursorStyle, text_layout, AppContext, AsyncAppContext, ClipboardItem, Element, ElementBox, Entity, - ModelHandle, MutableAppContext, RenderContext, Task, View, ViewContext, ViewHandle, - WeakViewHandle, + ModelHandle, MutableAppContext, RenderContext, Subscription, Task, View, ViewContext, + ViewHandle, WeakViewHandle, }; use hover_popover::{hide_hover, HoverState}; pub use language::{char_kind, CharKind}; @@ -48,7 +49,7 @@ use ordered_float::OrderedFloat; use project::{LocationLink, Project, ProjectPath, ProjectTransaction}; use selections_collection::{resolve_multiple, MutableSelectionsCollection, SelectionsCollection}; use serde::{Deserialize, Serialize}; -use settings::Settings; +use settings::{Autosave, Settings}; use smallvec::SmallVec; use smol::Timer; use snippet::Snippet; @@ -436,6 +437,9 @@ pub struct Editor { leader_replica_id: Option, hover_state: HoverState, link_go_to_definition_state: LinkGoToDefinitionState, + pending_autosave: Option>>, + cancel_pending_autosave: Option>, + _subscriptions: Vec, } pub struct EditorSnapshot { @@ -973,17 +977,13 @@ impl Editor { cx, ) }); - cx.observe(&buffer, Self::on_buffer_changed).detach(); - cx.subscribe(&buffer, Self::on_buffer_event).detach(); - cx.observe(&display_map, Self::on_display_map_changed) - .detach(); let selections = SelectionsCollection::new(display_map.clone(), buffer.clone()); let mut this = Self { handle: cx.weak_handle(), - buffer, - display_map, + buffer: buffer.clone(), + display_map: display_map.clone(), selections, columnar_selection_tail: None, add_selections_state: None, @@ -1026,6 +1026,14 @@ impl Editor { leader_replica_id: None, hover_state: Default::default(), link_go_to_definition_state: Default::default(), + pending_autosave: Default::default(), + cancel_pending_autosave: Default::default(), + _subscriptions: vec![ + cx.observe(&buffer, Self::on_buffer_changed), + cx.subscribe(&buffer, Self::on_buffer_event), + cx.observe(&display_map, Self::on_display_map_changed), + cx.observe_window_activation(Self::on_window_activation_changed), + ], }; this.end_selection(cx); @@ -2148,7 +2156,10 @@ impl Editor { .iter() .zip(autoclose_pair.ranges.iter().map(|r| r.to_offset(&buffer))) { - if selection.is_empty() && autoclose_range.is_empty() && selection.start == autoclose_range.start { + if selection.is_empty() + && autoclose_range.is_empty() + && selection.start == autoclose_range.start + { new_selections.push(Selection { id: selection.id, start: selection.start - autoclose_pair.pair.start.len(), @@ -5570,6 +5581,33 @@ impl Editor { self.refresh_active_diagnostics(cx); self.refresh_code_actions(cx); cx.emit(Event::BufferEdited); + if let Autosave::AfterDelay { milliseconds } = cx.global::().autosave { + let pending_autosave = + self.pending_autosave.take().unwrap_or(Task::ready(None)); + if let Some(cancel_pending_autosave) = self.cancel_pending_autosave.take() { + let _ = cancel_pending_autosave.send(()); + } + + let (cancel_tx, mut cancel_rx) = oneshot::channel(); + self.cancel_pending_autosave = Some(cancel_tx); + self.pending_autosave = Some(cx.spawn_weak(|this, mut cx| async move { + let mut timer = futures::future::join( + cx.background().timer(Duration::from_millis(milliseconds)), + pending_autosave, + ) + .fuse(); + futures::select! { + _ = timer => {} + _ = cancel_rx => return None, + } + + this.upgrade(&cx)? + .update(&mut cx, |this, cx| this.autosave(cx)) + .await + .log_err(); + None + })); + } } language::Event::Reparsed => cx.emit(Event::Reparsed), language::Event::DirtyChanged => cx.emit(Event::DirtyChanged), @@ -5588,6 +5626,22 @@ impl Editor { cx.notify(); } + fn on_window_activation_changed(&mut self, active: bool, cx: &mut ViewContext) { + if !active && cx.global::().autosave == Autosave::OnWindowChange { + self.autosave(cx).detach_and_log_err(cx); + } + } + + fn autosave(&mut self, cx: &mut ViewContext) -> Task> { + if let Some(project) = self.project.clone() { + if self.buffer.read(cx).is_dirty(cx) && !self.buffer.read(cx).has_conflict(cx) { + return workspace::Item::save(self, project, cx); + } + } + + Task::ready(Ok(())) + } + pub fn set_searchable(&mut self, searchable: bool) { self.searchable = searchable; } @@ -5805,6 +5859,10 @@ impl View for Editor { hide_hover(self, cx); cx.emit(Event::Blurred); cx.notify(); + + if cx.global::().autosave == Autosave::OnFocusChange { + self.autosave(cx).detach_and_log_err(cx); + } } fn keymap_context(&self, _: &AppContext) -> gpui::keymap::Context { diff --git a/crates/settings/src/settings.rs b/crates/settings/src/settings.rs index 886035c543785e8bc66e12630da3be047036e8a6..0a59d28cc9e845adb1ba4d3fd434245b101a5fbf 100644 --- a/crates/settings/src/settings.rs +++ b/crates/settings/src/settings.rs @@ -25,6 +25,7 @@ pub struct Settings { pub default_buffer_font_size: f32, pub hover_popover_enabled: bool, pub vim_mode: bool, + pub autosave: Autosave, pub language_settings: LanguageSettings, pub language_defaults: HashMap, LanguageSettings>, pub language_overrides: HashMap, LanguageSettings>, @@ -39,7 +40,6 @@ pub struct LanguageSettings { pub preferred_line_length: Option, pub format_on_save: Option, pub enable_language_server: Option, - pub autosave: Option, } #[derive(Copy, Clone, Debug, Deserialize, PartialEq, Eq, JsonSchema)] @@ -54,7 +54,7 @@ pub enum SoftWrap { #[serde(rename_all = "snake_case")] pub enum Autosave { Off, - AfterDelay { milliseconds: usize }, + AfterDelay { milliseconds: u64 }, OnFocusChange, OnWindowChange, } @@ -74,6 +74,8 @@ pub struct SettingsFileContent { #[serde(default)] pub format_on_save: Option, #[serde(default)] + pub autosave: Option, + #[serde(default)] pub enable_language_server: Option, #[serde(flatten)] pub editor: LanguageSettings, @@ -95,6 +97,7 @@ impl Settings { default_buffer_font_size: 15., hover_popover_enabled: true, vim_mode: false, + autosave: Autosave::Off, language_settings: Default::default(), language_defaults: Default::default(), language_overrides: Default::default(), @@ -138,11 +141,6 @@ impl Settings { .unwrap_or(true) } - pub fn autosave(&self, language: Option<&str>) -> Autosave { - self.language_setting(language, |settings| settings.autosave) - .unwrap_or(Autosave::Off) - } - pub fn enable_language_server(&self, language: Option<&str>) -> bool { self.language_setting(language, |settings| settings.enable_language_server) .unwrap_or(true) @@ -172,6 +170,7 @@ impl Settings { default_buffer_font_size: 14., hover_popover_enabled: true, vim_mode: false, + autosave: Autosave::Off, language_settings: Default::default(), language_defaults: Default::default(), language_overrides: Default::default(), @@ -213,6 +212,7 @@ impl Settings { 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.language_settings.format_on_save, data.format_on_save, @@ -227,7 +227,6 @@ impl Settings { &mut self.language_settings.preferred_line_length, data.editor.preferred_line_length, ); - merge_option(&mut self.language_settings.autosave, data.editor.autosave); for (language_name, settings) in data.language_overrides.clone().into_iter() { let target = self @@ -246,7 +245,6 @@ impl Settings { &mut target.preferred_line_length, settings.preferred_line_length, ); - merge_option(&mut target.autosave, settings.autosave); } } } From d43e8b270a21856d9931dd4d9bc3b27ff8fb95fb Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 5 Jul 2022 15:47:45 +0200 Subject: [PATCH 5/9] Add unit test for `ViewContext::observe_window_activation` --- crates/gpui/src/app.rs | 113 ++++++++++++++++++++++++++++++- crates/gpui/src/platform/test.rs | 6 +- 2 files changed, 116 insertions(+), 3 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 917112062876b1efc07b62961ba72b553f846032..505f609f5709f57b0c037ec5be2032729f736266 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -401,9 +401,12 @@ impl TestAppContext { T: View, F: FnOnce(&mut ViewContext) -> T, { - self.cx + let (window_id, view) = self + .cx .borrow_mut() - .add_window(Default::default(), build_root_view) + .add_window(Default::default(), build_root_view); + self.simulate_window_activation(Some(window_id)); + (window_id, view) } pub fn window_ids(&self) -> Vec { @@ -551,6 +554,35 @@ impl TestAppContext { } } + pub fn simulate_window_activation(&self, to_activate: Option) { + let mut handlers = BTreeMap::new(); + { + let mut cx = self.cx.borrow_mut(); + for (window_id, (_, window)) in &mut cx.presenters_and_platform_windows { + let window = window + .as_any_mut() + .downcast_mut::() + .unwrap(); + handlers.insert( + *window_id, + mem::take(&mut window.active_status_change_handlers), + ); + } + }; + let mut handlers = handlers.into_iter().collect::>(); + handlers.sort_unstable_by_key(|(window_id, _)| Some(*window_id) == to_activate); + + for (window_id, mut window_handlers) in handlers { + for window_handler in &mut window_handlers { + window_handler(Some(window_id) == to_activate); + } + + self.window_mut(window_id) + .active_status_change_handlers + .extend(window_handlers); + } + } + pub fn is_window_edited(&self, window_id: usize) -> bool { self.window_mut(window_id).edited } @@ -6992,4 +7024,81 @@ mod tests { ); assert_eq!(presenter.borrow().rendered_views.len(), 1); } + + #[crate::test(self)] + async fn test_window_activation(cx: &mut TestAppContext) { + struct View(&'static str); + + impl super::Entity for View { + type Event = (); + } + + impl super::View for View { + fn ui_name() -> &'static str { + "test view" + } + + fn render(&mut self, _: &mut RenderContext) -> ElementBox { + Empty::new().boxed() + } + } + + let events = Rc::new(RefCell::new(Vec::new())); + let (window_1, _) = cx.add_window(|cx: &mut ViewContext| { + cx.observe_window_activation({ + let events = events.clone(); + move |this, active, _| events.borrow_mut().push((this.0, active)) + }) + .detach(); + View("window 1") + }); + assert_eq!(mem::take(&mut *events.borrow_mut()), [("window 1", true)]); + + let (window_2, _) = cx.add_window(|cx: &mut ViewContext| { + cx.observe_window_activation({ + let events = events.clone(); + move |this, active, _| events.borrow_mut().push((this.0, active)) + }) + .detach(); + View("window 2") + }); + assert_eq!( + mem::take(&mut *events.borrow_mut()), + [("window 1", false), ("window 2", true)] + ); + + let (window_3, _) = cx.add_window(|cx: &mut ViewContext| { + cx.observe_window_activation({ + let events = events.clone(); + move |this, active, _| events.borrow_mut().push((this.0, active)) + }) + .detach(); + View("window 3") + }); + assert_eq!( + mem::take(&mut *events.borrow_mut()), + [("window 2", false), ("window 3", true)] + ); + + cx.simulate_window_activation(Some(window_2)); + assert_eq!( + mem::take(&mut *events.borrow_mut()), + [("window 3", false), ("window 2", true)] + ); + + cx.simulate_window_activation(Some(window_1)); + assert_eq!( + mem::take(&mut *events.borrow_mut()), + [("window 2", false), ("window 1", true)] + ); + + cx.simulate_window_activation(Some(window_3)); + assert_eq!( + mem::take(&mut *events.borrow_mut()), + [("window 1", false), ("window 3", true)] + ); + + cx.simulate_window_activation(Some(window_3)); + assert_eq!(mem::take(&mut *events.borrow_mut()), []); + } } diff --git a/crates/gpui/src/platform/test.rs b/crates/gpui/src/platform/test.rs index eb241797c3b8c570c5e5a1163bd9efd691d78f1a..a58bd603f277a93575ec3fe534538c670f03d9a3 100644 --- a/crates/gpui/src/platform/test.rs +++ b/crates/gpui/src/platform/test.rs @@ -37,6 +37,7 @@ pub struct Window { event_handlers: Vec bool>>, resize_handlers: Vec>, close_handlers: Vec>, + pub(crate) active_status_change_handlers: Vec>, pub(crate) should_close_handler: Option bool>>, pub(crate) title: Option, pub(crate) edited: bool, @@ -191,6 +192,7 @@ impl Window { resize_handlers: Default::default(), close_handlers: Default::default(), should_close_handler: Default::default(), + active_status_change_handlers: Default::default(), scale_factor: 1.0, current_scene: None, title: None, @@ -241,7 +243,9 @@ impl super::Window for Window { self.event_handlers.push(callback); } - fn on_active_status_change(&mut self, _: Box) {} + fn on_active_status_change(&mut self, callback: Box) { + self.active_status_change_handlers.push(callback); + } fn on_resize(&mut self, callback: Box) { self.resize_handlers.push(callback); From ebf4bae173d3c2f6ee1123a1e7f10aba0b061b0b Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 5 Jul 2022 16:19:07 +0200 Subject: [PATCH 6/9] Add unit test for autosave --- crates/editor/src/editor.rs | 78 +++++++++++++++++++++++++++++++++++-- 1 file changed, 74 insertions(+), 4 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index ec65db3bf783152a60d1ebd7ee921572d0d74e2a..1f4b75e7cb1cbf75a2cf6ac2af595af8fb8827fe 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -5634,7 +5634,10 @@ impl Editor { fn autosave(&mut self, cx: &mut ViewContext) -> Task> { if let Some(project) = self.project.clone() { - if self.buffer.read(cx).is_dirty(cx) && !self.buffer.read(cx).has_conflict(cx) { + if self.buffer.read(cx).is_dirty(cx) + && !self.buffer.read(cx).has_conflict(cx) + && workspace::Item::can_save(self, cx) + { return workspace::Item::save(self, project, cx); } } @@ -6276,22 +6279,23 @@ mod tests { use super::*; use futures::StreamExt; use gpui::{ + executor::Deterministic, geometry::rect::RectF, platform::{WindowBounds, WindowOptions}, }; use indoc::indoc; use language::{FakeLspAdapter, LanguageConfig}; use lsp::FakeLanguageServer; - use project::FakeFs; + use project::{FakeFs, Fs}; use settings::LanguageSettings; - use std::{cell::RefCell, rc::Rc, time::Instant}; + use std::{cell::RefCell, path::Path, rc::Rc, time::Instant}; use text::Point; use unindent::Unindent; use util::{ assert_set_eq, test::{marked_text_by, marked_text_ranges, marked_text_ranges_by, sample_text}, }; - use workspace::{FollowableItem, ItemHandle}; + use workspace::{FollowableItem, Item, ItemHandle}; #[gpui::test] fn test_edit_events(cx: &mut MutableAppContext) { @@ -9555,6 +9559,72 @@ mod tests { save.await.unwrap(); } + #[gpui::test] + async fn test_autosave(deterministic: Arc, cx: &mut gpui::TestAppContext) { + deterministic.forbid_parking(); + + let fs = FakeFs::new(cx.background().clone()); + fs.insert_file("/file.rs", Default::default()).await; + + let project = Project::test(fs.clone(), ["/file.rs".as_ref()], cx).await; + let buffer = project + .update(cx, |project, cx| project.open_local_buffer("/file.rs", cx)) + .await + .unwrap(); + + let (_, editor) = cx.add_window(|cx| Editor::for_buffer(buffer, Some(project), cx)); + + // Autosave on window change. + editor.update(cx, |editor, cx| { + cx.update_global(|settings: &mut Settings, _| { + settings.autosave = Autosave::OnWindowChange; + }); + editor.insert("X", cx); + assert!(editor.is_dirty(cx)) + }); + + // Deactivating the window saves the file. + cx.simulate_window_activation(None); + deterministic.run_until_parked(); + assert_eq!(fs.load(Path::new("/file.rs")).await.unwrap(), "X"); + editor.read_with(cx, |editor, cx| assert!(!editor.is_dirty(cx))); + + // Autosave on focus change. + editor.update(cx, |editor, cx| { + cx.focus_self(); + cx.update_global(|settings: &mut Settings, _| { + settings.autosave = Autosave::OnFocusChange; + }); + editor.insert("X", cx); + assert!(editor.is_dirty(cx)) + }); + + // Blurring the editor saves the file. + editor.update(cx, |_, cx| cx.blur()); + deterministic.run_until_parked(); + assert_eq!(fs.load(Path::new("/file.rs")).await.unwrap(), "XX"); + editor.read_with(cx, |editor, cx| assert!(!editor.is_dirty(cx))); + + // Autosave after delay. + editor.update(cx, |editor, cx| { + cx.update_global(|settings: &mut Settings, _| { + settings.autosave = Autosave::AfterDelay { milliseconds: 500 }; + }); + editor.insert("X", cx); + assert!(editor.is_dirty(cx)) + }); + + // Delay hasn't fully expired, so the file is still dirty and unsaved. + deterministic.advance_clock(Duration::from_millis(250)); + assert_eq!(fs.load(Path::new("/file.rs")).await.unwrap(), "XX"); + editor.read_with(cx, |editor, cx| assert!(editor.is_dirty(cx))); + + // After delay expires, the file is saved. + deterministic.advance_clock(Duration::from_millis(250)); + assert_eq!(fs.load(Path::new("/file.rs")).await.unwrap(), "XXX"); + editor.read_with(cx, |editor, cx| assert!(!editor.is_dirty(cx))); + } + #[gpui::test] async fn test_completion(cx: &mut gpui::TestAppContext) { let mut language = Language::new( From a5c39acf4c9cf1bc3277d9e10c15a9970b8fce25 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 5 Jul 2022 17:36:40 +0200 Subject: [PATCH 7/9] Always finish previous autosave before starting a new one --- crates/editor/src/editor.rs | 14 +++++++------- crates/gpui/src/executor.rs | 4 +--- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 1f4b75e7cb1cbf75a2cf6ac2af595af8fb8827fe..f6d62a3c83babc2111c14f4cfa13745973482cc4 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -5591,11 +5591,11 @@ impl Editor { let (cancel_tx, mut cancel_rx) = oneshot::channel(); self.cancel_pending_autosave = Some(cancel_tx); self.pending_autosave = Some(cx.spawn_weak(|this, mut cx| async move { - let mut timer = futures::future::join( - cx.background().timer(Duration::from_millis(milliseconds)), - pending_autosave, - ) - .fuse(); + let mut timer = cx + .background() + .timer(Duration::from_millis(milliseconds)) + .fuse(); + pending_autosave.await; futures::select! { _ = timer => {} _ = cancel_rx => return None, @@ -6634,8 +6634,8 @@ mod tests { editor.read(cx).selections.ranges(cx) ); assert_set_eq!( - cloned_editor.update(cx, |e, cx| dbg!(e.selections.display_ranges(cx))), - editor.update(cx, |e, cx| dbg!(e.selections.display_ranges(cx))) + cloned_editor.update(cx, |e, cx| e.selections.display_ranges(cx)), + editor.update(cx, |e, cx| e.selections.display_ranges(cx)) ); } diff --git a/crates/gpui/src/executor.rs b/crates/gpui/src/executor.rs index 6ed00c238f07504fbac1db6e5bbd896616a1dca3..7c2ba44c44a3cb52efb6c3012a10429de2f9596c 100644 --- a/crates/gpui/src/executor.rs +++ b/crates/gpui/src/executor.rs @@ -675,9 +675,7 @@ impl Background { } } _ => { - log::info!("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"); - - // panic!("this method can only be called on a deterministic executor") + panic!("this method can only be called on a deterministic executor") } } } From f09d2650548e69d4b4ff41a60b97aa11afb5fbf9 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 5 Jul 2022 17:54:54 +0200 Subject: [PATCH 8/9] Remove non-determinism from autosave after delay --- crates/editor/src/editor.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index f6d62a3c83babc2111c14f4cfa13745973482cc4..b601044e55b5b4493749f4b8f8211ab6f0ee8f10 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -5596,9 +5596,9 @@ impl Editor { .timer(Duration::from_millis(milliseconds)) .fuse(); pending_autosave.await; - futures::select! { - _ = timer => {} + futures::select_biased! { _ = cancel_rx => return None, + _ = timer => {} } this.upgrade(&cx)? From f1ffcb626a74e0b8bba513fda18506d2002504c4 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 5 Jul 2022 17:59:40 +0200 Subject: [PATCH 9/9] Fix panics in database tests --- crates/collab/src/db.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index b14f9b14e4c43f9f880f7f754659316160c40acc..e7ef0d579706b48ed5f1dcec387a0f31e00a5ef9 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -1361,7 +1361,7 @@ pub mod tests { use super::*; use anyhow::anyhow; use collections::BTreeMap; - use gpui::executor::Background; + use gpui::executor::{Background, Deterministic}; use lazy_static::lazy_static; use parking_lot::Mutex; use rand::prelude::*; @@ -1376,7 +1376,7 @@ pub mod tests { async fn test_get_users_by_ids() { for test_db in [ TestDb::postgres().await, - TestDb::fake(Arc::new(gpui::executor::Background::new())), + TestDb::fake(build_background_executor()), ] { let db = test_db.db(); @@ -1687,7 +1687,7 @@ pub mod tests { async fn test_recent_channel_messages() { for test_db in [ TestDb::postgres().await, - TestDb::fake(Arc::new(gpui::executor::Background::new())), + TestDb::fake(build_background_executor()), ] { let db = test_db.db(); let user = db.create_user("user", None, false).await.unwrap(); @@ -1726,7 +1726,7 @@ pub mod tests { async fn test_channel_message_nonces() { for test_db in [ TestDb::postgres().await, - TestDb::fake(Arc::new(gpui::executor::Background::new())), + TestDb::fake(build_background_executor()), ] { let db = test_db.db(); let user = db.create_user("user", None, false).await.unwrap(); @@ -1834,7 +1834,7 @@ pub mod tests { async fn test_add_contacts() { for test_db in [ TestDb::postgres().await, - TestDb::fake(Arc::new(gpui::executor::Background::new())), + TestDb::fake(build_background_executor()), ] { let db = test_db.db(); @@ -2836,4 +2836,8 @@ pub mod tests { Some(self) } } + + fn build_background_executor() -> Arc { + Deterministic::new(0).build_background() + } }