From 980730a4e17adc5765734cdf77090cda2c9b2964 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 6 Jul 2022 15:53:40 +0200 Subject: [PATCH 1/5] Report whether a view was focused or blurred when observing focus --- crates/gpui/src/app.rs | 130 +++++++++++++++++++--------- crates/search/src/project_search.rs | 12 ++- 2 files changed, 97 insertions(+), 45 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 505f609f57..fd447e2469 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -811,7 +811,7 @@ type GlobalActionCallback = dyn FnMut(&dyn Action, &mut MutableAppContext); type SubscriptionCallback = Box bool>; type GlobalSubscriptionCallback = Box; type ObservationCallback = Box bool>; -type FocusObservationCallback = Box bool>; +type FocusObservationCallback = Box bool>; type GlobalObservationCallback = Box; type ReleaseObservationCallback = Box; type ActionObservationCallback = Box; @@ -1305,7 +1305,7 @@ impl MutableAppContext { fn observe_focus(&mut self, handle: &ViewHandle, mut callback: F) -> Subscription where - F: 'static + FnMut(ViewHandle, &mut MutableAppContext) -> bool, + F: 'static + FnMut(ViewHandle, bool, &mut MutableAppContext) -> bool, V: View, { let subscription_id = post_inc(&mut self.next_subscription_id); @@ -1314,9 +1314,9 @@ impl MutableAppContext { self.pending_effects.push_back(Effect::FocusObservation { view_id, subscription_id, - callback: Box::new(move |cx| { + callback: Box::new(move |focused, cx| { if let Some(observed) = observed.upgrade(cx) { - callback(observed, cx) + callback(observed, focused, cx) } else { false } @@ -2525,6 +2525,31 @@ impl MutableAppContext { if let Some(mut blurred_view) = this.cx.views.remove(&(window_id, blurred_id)) { blurred_view.on_blur(this, window_id, blurred_id); this.cx.views.insert((window_id, blurred_id), blurred_view); + + let callbacks = this.focus_observations.lock().remove(&blurred_id); + if let Some(callbacks) = callbacks { + for (id, callback) in callbacks { + if let Some(mut callback) = callback { + let alive = callback(false, this); + if alive { + match this + .focus_observations + .lock() + .entry(blurred_id) + .or_default() + .entry(id) + { + btree_map::Entry::Vacant(entry) => { + entry.insert(Some(callback)); + } + btree_map::Entry::Occupied(entry) => { + entry.remove(); + } + } + } + } + } + } } } @@ -2537,7 +2562,7 @@ impl MutableAppContext { if let Some(callbacks) = callbacks { for (id, callback) in callbacks { if let Some(mut callback) = callback { - let alive = callback(this); + let alive = callback(true, this); if alive { match this .focus_observations @@ -3598,20 +3623,21 @@ impl<'a, T: View> ViewContext<'a, T> { pub fn observe_focus(&mut self, handle: &ViewHandle, mut callback: F) -> Subscription where - F: 'static + FnMut(&mut T, ViewHandle, &mut ViewContext), + F: 'static + FnMut(&mut T, ViewHandle, bool, &mut ViewContext), V: View, { let observer = self.weak_handle(); - self.app.observe_focus(handle, move |observed, cx| { - if let Some(observer) = observer.upgrade(cx) { - observer.update(cx, |observer, cx| { - callback(observer, observed, cx); - }); - true - } else { - false - } - }) + self.app + .observe_focus(handle, move |observed, focused, cx| { + if let Some(observer) = observer.upgrade(cx) { + observer.update(cx, |observer, cx| { + callback(observer, observed, focused, cx); + }); + true + } else { + false + } + }) } pub fn observe_release(&mut self, handle: &H, mut callback: F) -> Subscription @@ -6448,11 +6474,13 @@ mod tests { view_1.update(cx, |_, cx| { cx.observe_focus(&view_2, { let observed_events = observed_events.clone(); - move |this, view, cx| { + move |this, view, focused, cx| { + let label = if focused { "focus" } else { "blur" }; observed_events.lock().push(format!( - "{} observed {}'s focus", + "{} observed {}'s {}", this.name, - view.read(cx).name + view.read(cx).name, + label )) } }) @@ -6461,16 +6489,20 @@ mod tests { view_2.update(cx, |_, cx| { cx.observe_focus(&view_1, { let observed_events = observed_events.clone(); - move |this, view, cx| { + move |this, view, focused, cx| { + let label = if focused { "focus" } else { "blur" }; observed_events.lock().push(format!( - "{} observed {}'s focus", + "{} observed {}'s {}", this.name, - view.read(cx).name + view.read(cx).name, + label )) } }) .detach(); }); + assert_eq!(mem::take(&mut *view_events.lock()), ["view 1 focused"]); + assert_eq!(mem::take(&mut *observed_events.lock()), Vec::<&str>::new()); view_1.update(cx, |_, cx| { // Ensure only the latest focus is honored. @@ -6478,31 +6510,47 @@ mod tests { cx.focus(&view_1); cx.focus(&view_2); }); - view_1.update(cx, |_, cx| cx.focus(&view_1)); - view_1.update(cx, |_, cx| cx.focus(&view_2)); - view_1.update(cx, |_, _| drop(view_2)); - assert_eq!( - *view_events.lock(), - [ - "view 1 focused".to_string(), - "view 1 blurred".to_string(), - "view 2 focused".to_string(), - "view 2 blurred".to_string(), - "view 1 focused".to_string(), - "view 1 blurred".to_string(), - "view 2 focused".to_string(), - "view 1 focused".to_string(), - ], + mem::take(&mut *view_events.lock()), + ["view 1 blurred", "view 2 focused"], ); assert_eq!( - *observed_events.lock(), + mem::take(&mut *observed_events.lock()), [ - "view 1 observed view 2's focus".to_string(), - "view 2 observed view 1's focus".to_string(), - "view 1 observed view 2's focus".to_string(), + "view 2 observed view 1's blur", + "view 1 observed view 2's focus" ] ); + + view_1.update(cx, |_, cx| cx.focus(&view_1)); + assert_eq!( + mem::take(&mut *view_events.lock()), + ["view 2 blurred", "view 1 focused"], + ); + assert_eq!( + mem::take(&mut *observed_events.lock()), + [ + "view 1 observed view 2's blur", + "view 2 observed view 1's focus" + ] + ); + + view_1.update(cx, |_, cx| cx.focus(&view_2)); + assert_eq!( + mem::take(&mut *view_events.lock()), + ["view 1 blurred", "view 2 focused"], + ); + assert_eq!( + mem::take(&mut *observed_events.lock()), + [ + "view 2 observed view 1's blur", + "view 1 observed view 2's focus" + ] + ); + + view_1.update(cx, |_, _| drop(view_2)); + assert_eq!(mem::take(&mut *view_events.lock()), ["view 1 focused"]); + assert_eq!(mem::take(&mut *observed_events.lock()), Vec::<&str>::new()); } #[crate::test(self)] diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index bf862f0d9d..2aa8993285 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -365,8 +365,10 @@ impl ProjectSearchView { cx.emit(ViewEvent::EditorEvent(event.clone())) }) .detach(); - cx.observe_focus(&query_editor, |this, _, _| { - this.results_editor_was_focused = false; + cx.observe_focus(&query_editor, |this, _, focused, _| { + if focused { + this.results_editor_was_focused = false; + } }) .detach(); @@ -377,8 +379,10 @@ impl ProjectSearchView { }); cx.observe(&results_editor, |_, _, cx| cx.emit(ViewEvent::UpdateTab)) .detach(); - cx.observe_focus(&results_editor, |this, _, _| { - this.results_editor_was_focused = true; + cx.observe_focus(&results_editor, |this, _, focused, _| { + if focused { + this.results_editor_was_focused = true; + } }) .detach(); cx.subscribe(&results_editor, |this, _, event, cx| { From b937c1acec6f858ed0b9abd2f383ca52f0877596 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 6 Jul 2022 16:11:06 +0200 Subject: [PATCH 2/5] Move autosave logic up into `Workspace` and `Pane` --- crates/diagnostics/src/diagnostics.rs | 9 +- crates/editor/src/editor.rs | 131 +------------------------- crates/editor/src/items.rs | 4 + crates/search/src/project_search.rs | 8 ++ crates/workspace/src/pane.rs | 12 +++ crates/workspace/src/workspace.rs | 59 +++++++++++- crates/zed/src/zed.rs | 75 +++++++++++++++ 7 files changed, 166 insertions(+), 132 deletions(-) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 94eda67c39..ecc1b2df68 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -568,10 +568,11 @@ impl workspace::Item for ProjectDiagnosticsEditor { } fn should_update_tab_on_event(event: &Event) -> bool { - matches!( - event, - Event::Saved | Event::DirtyChanged | Event::TitleChanged - ) + Editor::should_update_tab_on_event(event) + } + + fn is_edit_event(event: &Self::Event) -> bool { + Editor::is_edit_event(event) } fn set_nav_history(&mut self, nav_history: ItemNavHistory, cx: &mut ViewContext) { diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 31a636fd61..9dd40413fd 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -18,7 +18,6 @@ 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, @@ -51,7 +50,7 @@ use ordered_float::OrderedFloat; use project::{LocationLink, Project, ProjectPath, ProjectTransaction}; use selections_collection::{resolve_multiple, MutableSelectionsCollection, SelectionsCollection}; use serde::{Deserialize, Serialize}; -use settings::{Autosave, Settings}; +use settings::Settings; use smallvec::SmallVec; use smol::Timer; use snippet::Snippet; @@ -439,8 +438,6 @@ 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, } @@ -1028,13 +1025,10 @@ 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); @@ -5584,33 +5578,6 @@ 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 = cx - .background() - .timer(Duration::from_millis(milliseconds)) - .fuse(); - pending_autosave.await; - futures::select_biased! { - _ = cancel_rx => return None, - _ = timer => {} - } - - 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), @@ -5629,25 +5596,6 @@ 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) - && workspace::Item::can_save(self, cx) - { - return workspace::Item::save(self, project, cx); - } - } - - Task::ready(Ok(())) - } - pub fn set_searchable(&mut self, searchable: bool) { self.searchable = searchable; } @@ -5865,10 +5813,6 @@ 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 { @@ -6282,23 +6226,22 @@ 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, Fs}; + use project::FakeFs; use settings::LanguageSettings; - use std::{cell::RefCell, path::Path, rc::Rc, time::Instant}; + use std::{cell::RefCell, 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, Item, ItemHandle}; + use workspace::{FollowableItem, ItemHandle}; #[gpui::test] fn test_edit_events(cx: &mut MutableAppContext) { @@ -9562,72 +9505,6 @@ 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( diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 8e15dce83c..f7aa80beaa 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -445,6 +445,10 @@ impl Item for Editor { Event::Saved | Event::DirtyChanged | Event::TitleChanged ) } + + fn is_edit_event(event: &Self::Event) -> bool { + matches!(event, Event::BufferEdited) + } } impl ProjectItem for Editor { diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index 2aa8993285..5ee2dcbb27 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -329,6 +329,14 @@ impl Item for ProjectSearchView { fn should_update_tab_on_event(event: &ViewEvent) -> bool { matches!(event, ViewEvent::UpdateTab) } + + fn is_edit_event(event: &Self::Event) -> bool { + if let ViewEvent::EditorEvent(editor_event) = event { + Editor::is_edit_event(editor_event) + } else { + false + } + } } impl ProjectSearchView { diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 5e039b8cd0..f8474e41b1 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -718,6 +718,18 @@ impl Pane { Ok(true) } + pub fn autosave_item( + item: &dyn ItemHandle, + project: ModelHandle, + cx: &mut MutableAppContext, + ) -> Task> { + if item.is_dirty(cx) && !item.has_conflict(cx) && item.can_save(cx) { + item.save(project, cx) + } else { + Task::ready(Ok(())) + } + } + pub fn focus_active_item(&mut self, cx: &mut ViewContext) { if let Some(active_item) = self.active_item() { cx.focus(active_item); diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 419998e730..cf1092662f 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -11,6 +11,7 @@ use client::{ }; use clock::ReplicaId; use collections::{hash_map, HashMap, HashSet}; +use futures::{channel::oneshot, FutureExt}; use gpui::{ actions, color::Color, @@ -30,7 +31,7 @@ pub use pane_group::*; use postage::prelude::Stream; use project::{fs, Fs, Project, ProjectEntryId, ProjectPath, ProjectStore, Worktree, WorktreeId}; use serde::Deserialize; -use settings::Settings; +use settings::{Autosave, Settings}; use sidebar::{Side, Sidebar, SidebarButtons, ToggleSidebarItem}; use smallvec::SmallVec; use status_bar::StatusBar; @@ -41,12 +42,14 @@ use std::{ cell::RefCell, fmt, future::Future, + mem, path::{Path, PathBuf}, rc::Rc, sync::{ atomic::{AtomicBool, Ordering::SeqCst}, Arc, }, + time::Duration, }; use theme::{Theme, ThemeRegistry}; pub use toolbar::{ToolbarItemLocation, ToolbarItemView}; @@ -296,6 +299,9 @@ pub trait Item: View { fn should_update_tab_on_event(_: &Self::Event) -> bool { false } + fn is_edit_event(_: &Self::Event) -> bool { + false + } fn act_as_type( &self, type_id: TypeId, @@ -510,6 +516,8 @@ impl ItemHandle for ViewHandle { } } + let mut pending_autosave = None; + let mut cancel_pending_autosave = oneshot::channel::<()>().0; let pending_update = Rc::new(RefCell::new(None)); let pending_update_scheduled = Rc::new(AtomicBool::new(false)); let pane = pane.downgrade(); @@ -570,6 +578,40 @@ impl ItemHandle for ViewHandle { cx.notify(); }); } + + if T::is_edit_event(event) { + if let Autosave::AfterDelay { milliseconds } = cx.global::().autosave { + let prev_autosave = pending_autosave.take().unwrap_or(Task::ready(Some(()))); + let (cancel_tx, mut cancel_rx) = oneshot::channel::<()>(); + let prev_cancel_tx = mem::replace(&mut cancel_pending_autosave, cancel_tx); + let project = workspace.project.downgrade(); + let _ = prev_cancel_tx.send(()); + pending_autosave = Some(cx.spawn_weak(|_, mut cx| async move { + let mut timer = cx + .background() + .timer(Duration::from_millis(milliseconds)) + .fuse(); + prev_autosave.await; + futures::select_biased! { + _ = cancel_rx => return None, + _ = timer => {} + } + + let project = project.upgrade(&cx)?; + cx.update(|cx| Pane::autosave_item(&item, project, cx)) + .await + .log_err(); + None + })); + } + } + }) + .detach(); + + cx.observe_focus(self, move |workspace, item, focused, cx| { + if !focused && cx.global::().autosave == Autosave::OnFocusChange { + Pane::autosave_item(&item, workspace.project.clone(), cx).detach_and_log_err(cx); + } }) .detach(); } @@ -774,6 +816,8 @@ impl Workspace { cx.notify() }) .detach(); + cx.observe_window_activation(Self::on_window_activation_changed) + .detach(); cx.subscribe(&project, move |this, project, event, cx| { match event { @@ -2314,6 +2358,19 @@ impl Workspace { } None } + + fn on_window_activation_changed(&mut self, active: bool, cx: &mut ViewContext) { + if !active && cx.global::().autosave == Autosave::OnWindowChange { + for pane in &self.panes { + pane.update(cx, |pane, cx| { + for item in pane.items() { + Pane::autosave_item(item.as_ref(), self.project.clone(), cx) + .detach_and_log_err(cx); + } + }); + } + } + } } impl Entity for Workspace { diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index f88aee3d7c..4fa2e238bf 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -396,9 +396,11 @@ mod tests { }; use project::{Project, ProjectPath}; use serde_json::json; + use settings::Autosave; use std::{ collections::HashSet, path::{Path, PathBuf}, + time::Duration, }; use theme::{Theme, ThemeRegistry, DEFAULT_THEME_NAME}; use workspace::{ @@ -977,6 +979,79 @@ mod tests { }) } + #[gpui::test] + async fn test_autosave(deterministic: Arc, cx: &mut gpui::TestAppContext) { + let app_state = init(cx); + let fs = app_state.fs.clone(); + fs.as_fake() + .insert_tree("/root", json!({ "a.txt": "" })) + .await; + + let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; + let (_, workspace) = cx.add_window(|cx| Workspace::new(project, cx)); + cx.update(|cx| { + workspace.update(cx, |view, cx| { + view.open_paths(vec![PathBuf::from("/root/a.txt")], true, cx) + }) + }) + .await; + let editor = cx.read(|cx| { + let pane = workspace.read(cx).active_pane().read(cx); + let item = pane.active_item().unwrap(); + item.downcast::().unwrap() + }); + + // 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("/root/a.txt")).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("/root/a.txt")).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("/root/a.txt")).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("/root/a.txt")).await.unwrap(), "XXX"); + editor.read_with(cx, |editor, cx| assert!(!editor.is_dirty(cx))); + } + #[gpui::test] async fn test_setting_language_when_saving_as_single_file_worktree(cx: &mut TestAppContext) { let app_state = init(cx); From 5e00df62672b96baf73812eb8d5d4a4cb7491b70 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 6 Jul 2022 16:55:05 +0200 Subject: [PATCH 3/5] Move autosave tests down into `Workspace` --- crates/workspace/src/workspace.rs | 92 ++++++++++++++++++++++++++++++- crates/zed/src/zed.rs | 75 ------------------------- 2 files changed, 90 insertions(+), 77 deletions(-) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index cf1092662f..2120695760 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2688,7 +2688,7 @@ fn open_new(app_state: &Arc, cx: &mut MutableAppContext) { #[cfg(test)] mod tests { use super::*; - use gpui::{ModelHandle, TestAppContext, ViewContext}; + use gpui::{executor::Deterministic, ModelHandle, TestAppContext, ViewContext}; use project::{FakeFs, Project, ProjectEntryId}; use serde_json::json; @@ -3026,6 +3026,86 @@ mod tests { }); } + #[gpui::test] + async fn test_autosave(deterministic: Arc, cx: &mut gpui::TestAppContext) { + deterministic.forbid_parking(); + + Settings::test_async(cx); + let fs = FakeFs::new(cx.background()); + + let project = Project::test(fs, [], cx).await; + let (window_id, workspace) = cx.add_window(|cx| Workspace::new(project, cx)); + + let item = cx.add_view(window_id, |_| { + let mut item = TestItem::new(); + item.project_entry_ids = vec![ProjectEntryId::from_proto(1)]; + item.is_dirty = true; + item + }); + let item_id = item.id(); + workspace.update(cx, |workspace, cx| { + workspace.add_item(Box::new(item.clone()), cx); + }); + + // Autosave on window change. + item.update(cx, |_, cx| { + cx.update_global(|settings: &mut Settings, _| { + settings.autosave = Autosave::OnWindowChange; + }); + }); + + // Deactivating the window saves the file. + cx.simulate_window_activation(None); + deterministic.run_until_parked(); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 1)); + + // Autosave on focus change. + item.update(cx, |_, cx| { + cx.focus_self(); + cx.update_global(|settings: &mut Settings, _| { + settings.autosave = Autosave::OnFocusChange; + }); + }); + + // Blurring the item saves the file. + item.update(cx, |_, cx| cx.blur()); + deterministic.run_until_parked(); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 2)); + + // Autosave after delay. + item.update(cx, |_, cx| { + cx.update_global(|settings: &mut Settings, _| { + settings.autosave = Autosave::AfterDelay { milliseconds: 500 }; + }); + cx.emit(TestItemEvent::Edit); + }); + + // Delay hasn't fully expired, so the file is still dirty and unsaved. + deterministic.advance_clock(Duration::from_millis(250)); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 2)); + + // After delay expires, the file is saved. + deterministic.advance_clock(Duration::from_millis(250)); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 3)); + + // Autosave on focus change, ensuring closing the tab counts as such. + item.update(cx, |_, cx| { + cx.update_global(|settings: &mut Settings, _| { + settings.autosave = Autosave::OnFocusChange; + }); + }); + + workspace + .update(cx, |workspace, cx| { + let pane = workspace.active_pane().clone(); + Pane::close_items(workspace, pane, cx, move |id| id == item_id) + }) + .await + .unwrap(); + assert!(!cx.has_pending_prompt(window_id)); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 4)); + } + #[derive(Clone)] struct TestItem { save_count: usize, @@ -3038,6 +3118,10 @@ mod tests { is_singleton: bool, } + enum TestItemEvent { + Edit, + } + impl TestItem { fn new() -> Self { Self { @@ -3054,7 +3138,7 @@ mod tests { } impl Entity for TestItem { - type Event = (); + type Event = TestItemEvent; } impl View for TestItem { @@ -3136,5 +3220,9 @@ mod tests { fn should_update_tab_on_event(_: &Self::Event) -> bool { true } + + fn is_edit_event(event: &Self::Event) -> bool { + matches!(event, TestItemEvent::Edit) + } } } diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 4fa2e238bf..f88aee3d7c 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -396,11 +396,9 @@ mod tests { }; use project::{Project, ProjectPath}; use serde_json::json; - use settings::Autosave; use std::{ collections::HashSet, path::{Path, PathBuf}, - time::Duration, }; use theme::{Theme, ThemeRegistry, DEFAULT_THEME_NAME}; use workspace::{ @@ -979,79 +977,6 @@ mod tests { }) } - #[gpui::test] - async fn test_autosave(deterministic: Arc, cx: &mut gpui::TestAppContext) { - let app_state = init(cx); - let fs = app_state.fs.clone(); - fs.as_fake() - .insert_tree("/root", json!({ "a.txt": "" })) - .await; - - let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; - let (_, workspace) = cx.add_window(|cx| Workspace::new(project, cx)); - cx.update(|cx| { - workspace.update(cx, |view, cx| { - view.open_paths(vec![PathBuf::from("/root/a.txt")], true, cx) - }) - }) - .await; - let editor = cx.read(|cx| { - let pane = workspace.read(cx).active_pane().read(cx); - let item = pane.active_item().unwrap(); - item.downcast::().unwrap() - }); - - // 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("/root/a.txt")).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("/root/a.txt")).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("/root/a.txt")).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("/root/a.txt")).await.unwrap(), "XXX"); - editor.read_with(cx, |editor, cx| assert!(!editor.is_dirty(cx))); - } - #[gpui::test] async fn test_setting_language_when_saving_as_single_file_worktree(cx: &mut TestAppContext) { let app_state = init(cx); From 92868931776a1ea1ff2d0843eebf74fb9fdd421b Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 6 Jul 2022 16:55:25 +0200 Subject: [PATCH 4/5] Save item when closing it if autosave on focus change is enabled --- crates/workspace/src/pane.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index f8474e41b1..b98ce49cd9 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -14,7 +14,7 @@ use gpui::{ }; use project::{Project, ProjectEntryId, ProjectPath}; use serde::Deserialize; -use settings::Settings; +use settings::{Autosave, Settings}; use std::{any::Any, cell::RefCell, mem, path::Path, rc::Rc}; use util::ResultExt; @@ -677,7 +677,13 @@ impl Pane { _ => return Ok(false), } } else if is_dirty && (can_save || is_singleton) { - let should_save = if should_prompt_for_save { + let autosave_enabled = cx.read(|cx| { + matches!( + cx.global::().autosave, + Autosave::OnFocusChange | Autosave::OnWindowChange + ) + }); + let should_save = if should_prompt_for_save && !autosave_enabled { let mut answer = pane.update(cx, |pane, cx| { pane.activate_item(item_ix, true, true, cx); cx.prompt( From ab4931da6565842727a13d7fa41a43df819660f0 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 6 Jul 2022 17:25:33 +0200 Subject: [PATCH 5/5] Prevent autosave for deleted files --- crates/workspace/src/pane.rs | 13 +++++++---- crates/workspace/src/workspace.rs | 37 ++++++++++++++++++++++++++----- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index b98ce49cd9..bbc086395b 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -677,13 +677,13 @@ impl Pane { _ => return Ok(false), } } else if is_dirty && (can_save || is_singleton) { - let autosave_enabled = cx.read(|cx| { + let will_autosave = cx.read(|cx| { matches!( cx.global::().autosave, Autosave::OnFocusChange | Autosave::OnWindowChange - ) + ) && Self::can_autosave_item(item.as_ref(), cx) }); - let should_save = if should_prompt_for_save && !autosave_enabled { + let should_save = if should_prompt_for_save && !will_autosave { let mut answer = pane.update(cx, |pane, cx| { pane.activate_item(item_ix, true, true, cx); cx.prompt( @@ -724,12 +724,17 @@ impl Pane { Ok(true) } + fn can_autosave_item(item: &dyn ItemHandle, cx: &AppContext) -> bool { + let is_deleted = item.project_entry_ids(cx).is_empty(); + item.is_dirty(cx) && !item.has_conflict(cx) && item.can_save(cx) && !is_deleted + } + pub fn autosave_item( item: &dyn ItemHandle, project: ModelHandle, cx: &mut MutableAppContext, ) -> Task> { - if item.is_dirty(cx) && !item.has_conflict(cx) && item.can_save(cx) { + if Self::can_autosave_item(item, cx) { item.save(project, cx) } else { Task::ready(Ok(())) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 2120695760..53318d943f 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -3039,7 +3039,6 @@ mod tests { let item = cx.add_view(window_id, |_| { let mut item = TestItem::new(); item.project_entry_ids = vec![ProjectEntryId::from_proto(1)]; - item.is_dirty = true; item }); let item_id = item.id(); @@ -3048,10 +3047,11 @@ mod tests { }); // Autosave on window change. - item.update(cx, |_, cx| { + item.update(cx, |item, cx| { cx.update_global(|settings: &mut Settings, _| { settings.autosave = Autosave::OnWindowChange; }); + item.is_dirty = true; }); // Deactivating the window saves the file. @@ -3060,11 +3060,12 @@ mod tests { item.read_with(cx, |item, _| assert_eq!(item.save_count, 1)); // Autosave on focus change. - item.update(cx, |_, cx| { + item.update(cx, |item, cx| { cx.focus_self(); cx.update_global(|settings: &mut Settings, _| { settings.autosave = Autosave::OnFocusChange; }); + item.is_dirty = true; }); // Blurring the item saves the file. @@ -3073,10 +3074,11 @@ mod tests { item.read_with(cx, |item, _| assert_eq!(item.save_count, 2)); // Autosave after delay. - item.update(cx, |_, cx| { + item.update(cx, |item, cx| { cx.update_global(|settings: &mut Settings, _| { settings.autosave = Autosave::AfterDelay { milliseconds: 500 }; }); + item.is_dirty = true; cx.emit(TestItemEvent::Edit); }); @@ -3089,10 +3091,11 @@ mod tests { item.read_with(cx, |item, _| assert_eq!(item.save_count, 3)); // Autosave on focus change, ensuring closing the tab counts as such. - item.update(cx, |_, cx| { + item.update(cx, |item, cx| { cx.update_global(|settings: &mut Settings, _| { settings.autosave = Autosave::OnFocusChange; }); + item.is_dirty = true; }); workspace @@ -3104,6 +3107,27 @@ mod tests { .unwrap(); assert!(!cx.has_pending_prompt(window_id)); item.read_with(cx, |item, _| assert_eq!(item.save_count, 4)); + + // Add the item again, ensuring autosave is prevented if the underlying file has been deleted. + workspace.update(cx, |workspace, cx| { + workspace.add_item(Box::new(item.clone()), cx); + }); + item.update(cx, |item, cx| { + item.project_entry_ids = Default::default(); + item.is_dirty = true; + cx.blur(); + }); + deterministic.run_until_parked(); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 4)); + + // Ensure autosave is prevented for deleted files also when closing the buffer. + let _close_items = workspace.update(cx, |workspace, cx| { + let pane = workspace.active_pane().clone(); + Pane::close_items(workspace, pane, cx, move |id| id == item_id) + }); + deterministic.run_until_parked(); + assert!(cx.has_pending_prompt(window_id)); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 4)); } #[derive(Clone)] @@ -3195,6 +3219,7 @@ mod tests { _: &mut ViewContext, ) -> Task> { self.save_count += 1; + self.is_dirty = false; Task::ready(Ok(())) } @@ -3205,6 +3230,7 @@ mod tests { _: &mut ViewContext, ) -> Task> { self.save_as_count += 1; + self.is_dirty = false; Task::ready(Ok(())) } @@ -3214,6 +3240,7 @@ mod tests { _: &mut ViewContext, ) -> Task> { self.reload_count += 1; + self.is_dirty = false; Task::ready(Ok(())) }