diff --git a/Cargo.lock b/Cargo.lock index a35068e268d2586d9032ac182cbc41028a7e988a..26bc6f36b5376e9fa5125cd94124d77521a4b62a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -21936,7 +21936,6 @@ dependencies = [ "ui", "util", "uuid", - "vim_mode_setting", "windows 0.61.3", "zed_actions", "zlog", diff --git a/crates/command_palette/src/command_palette.rs b/crates/command_palette/src/command_palette.rs index 7b04981b929da1f50ee40da4d773c6618ede28e3..d0c7f052f4de15f831afd0724bac7e6975154a41 100644 --- a/crates/command_palette/src/command_palette.rs +++ b/crates/command_palette/src/command_palette.rs @@ -33,11 +33,7 @@ pub fn init(cx: &mut App) { cx.observe_new(CommandPalette::register).detach(); } -impl ModalView for CommandPalette { - fn is_command_palette(&self) -> bool { - true - } -} +impl ModalView for CommandPalette {} pub struct CommandPalette { picker: Entity>, diff --git a/crates/workspace/Cargo.toml b/crates/workspace/Cargo.toml index 00cd8ebbed433322ce96b01714c2fa1244d483d7..f74f8fdb4156e7b0a09f4ab15e7341e82dd85da3 100644 --- a/crates/workspace/Cargo.toml +++ b/crates/workspace/Cargo.toml @@ -66,7 +66,6 @@ theme_settings.workspace = true ui.workspace = true util.workspace = true uuid.workspace = true -vim_mode_setting.workspace = true zed_actions.workspace = true [target.'cfg(target_os = "windows")'.dependencies] diff --git a/crates/workspace/src/item.rs b/crates/workspace/src/item.rs index ee74d7f5ebd8d5b8e7664a643bc12bcf1996fb85..5cd669473c73fd9fd4c507eb2de903d4d5f7dc2b 100644 --- a/crates/workspace/src/item.rs +++ b/crates/workspace/src/item.rs @@ -953,24 +953,23 @@ impl ItemHandle for Entity { return; } - let vim_mode = vim_mode_setting::VimModeSetting::is_enabled(cx); - let helix_mode = vim_mode_setting::HelixModeSetting::is_enabled(cx); - - if vim_mode || helix_mode { - // We use the command palette for executing commands in Vim and Helix modes (e.g., `:w`), so - // in those cases we don't want to trigger auto-save if the focus has just been transferred - // to the command palette. - // - // This isn't totally perfect, as you could still switch files indirectly via the command - // palette (such as by opening up the tab switcher from it and then switching tabs that - // way). - if workspace.is_active_modal_command_palette(cx) { - return; + // Add the item to a deferred save list. The actual save will happen when + // focus lands on a pane or panel (via handle_pane_focused or + // handle_panel_focused), or when the window deactivates. + // This avoids saving when opening modals and skips saving if focus + // returns to the same item. + workspace.deferred_save_items.push(item.downgrade_item()); + + // Defer the flush to ensure all focus events are processed first. + // This is needed because on_focus_out fires before handle_pane_focused + // when switching items. + cx.defer_in(window, |workspace, window, cx| { + // Don't flush if a modal is active - the user might return + // to the original item when the modal is dismissed. + if !workspace.has_active_modal(window, cx) { + workspace.flush_deferred_saves(window, cx); } - } - - Pane::autosave_item(&item, workspace.project.clone(), window, cx) - .detach_and_log_err(cx); + }); } }, ) diff --git a/crates/workspace/src/modal_layer.rs b/crates/workspace/src/modal_layer.rs index cb6f21206fc5e1348224dd3e01e5155880e5d883..5949c0b1fffb216f27c939330954ecd8c7343a5c 100644 --- a/crates/workspace/src/modal_layer.rs +++ b/crates/workspace/src/modal_layer.rs @@ -26,15 +26,6 @@ pub trait ModalView: ManagedView { fn render_bare(&self) -> bool { false } - - /// Returns whether this [`ModalView`] is the command palette. - /// - /// This breaks the encapsulation of the [`ModalView`] trait a little bit, but there doesn't seem to be an - /// immediate, more elegant way to have the workspace know about the command palette (due to dependency arrow - /// directions). - fn is_command_palette(&self) -> bool { - false - } } trait ModalViewHandle { @@ -42,7 +33,6 @@ trait ModalViewHandle { fn view(&self) -> AnyView; fn fade_out_background(&self, cx: &mut App) -> bool; fn render_bare(&self, cx: &mut App) -> bool; - fn is_command_palette(&self, cx: &App) -> bool; } impl ModalViewHandle for Entity { @@ -61,10 +51,6 @@ impl ModalViewHandle for Entity { fn render_bare(&self, cx: &mut App) -> bool { self.read(cx).render_bare() } - - fn is_command_palette(&self, cx: &App) -> bool { - self.read(cx).is_command_palette() - } } pub struct ActiveModal { @@ -203,13 +189,6 @@ impl ModalLayer { pub fn has_active_modal(&self) -> bool { self.active_modal.is_some() } - - /// Returns whether the active modal is the command palette. - pub fn is_active_modal_command_palette(&self, cx: &App) -> bool { - self.active_modal - .as_ref() - .map_or(false, |modal| modal.modal.is_command_palette(cx)) - } } impl Render for ModalLayer { diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 995023cdd298897ec636c0add7afdfeea7631677..22a86cab0b3449714195a1a87935ed22a2251238 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1398,6 +1398,7 @@ pub struct Workspace { sidebar_focus_handle: Option, multi_workspace: Option>, active_worktree_creation: ActiveWorktreeCreation, + deferred_save_items: Vec>, } impl EventEmitter for Workspace {} @@ -1829,6 +1830,7 @@ impl Workspace { active_worktree_creation: ActiveWorktreeCreation::default(), open_in_dev_container: false, _dev_container_task: None, + deferred_save_items: Vec::new(), } } @@ -5219,6 +5221,8 @@ impl Workspace { window: &mut Window, cx: &mut Context, ) { + self.flush_deferred_saves(window, cx); + // This is explicitly hoisted out of the following check for pane identity as // terminal panel panes are not registered as a center panes. self.status_bar.update(cx, |status_bar, cx| { @@ -5276,9 +5280,26 @@ impl Workspace { } fn handle_panel_focused(&mut self, window: &mut Window, cx: &mut Context) { + self.flush_deferred_saves(window, cx); self.update_active_view_for_followers(window, cx); } + fn flush_deferred_saves(&mut self, window: &mut Window, cx: &mut Context) { + let deferred = std::mem::take(&mut self.deferred_save_items); + for weak_item in deferred { + let Some(item) = weak_item.upgrade() else { + continue; + }; + // Skip if focus returned to this item + let focus_handle = item.item_focus_handle(cx); + if focus_handle.contains_focused(window, cx) { + continue; + } + Pane::autosave_item(item.as_ref(), self.project.clone(), window, cx) + .detach_and_log_err(cx); + } + } + fn handle_pane_event( &mut self, pane: &Entity, @@ -6454,6 +6475,8 @@ impl Workspace { .detach(); } } else { + // When window is deactivated, flush any deferred saves since focus has left the window + self.flush_deferred_saves(window, cx); for pane in &self.panes { pane.update(cx, |pane, cx| { if let Some(item) = pane.active_item() { @@ -7495,12 +7518,6 @@ impl Workspace { self.modal_layer.read(cx).has_active_modal() } - pub fn is_active_modal_command_palette(&self, cx: &mut App) -> bool { - self.modal_layer - .read(cx) - .is_active_modal_command_palette(cx) - } - pub fn active_modal(&self, cx: &App) -> Option> { self.modal_layer.read(cx).active_modal() } @@ -11637,10 +11654,13 @@ mod tests { }); item.is_dirty = true; }); - // Blurring the item saves the file. - item.update_in(cx, |_, window, _| window.blur()); + // Focus leaving the item (via window deactivation) saves the file. + // Deferred autosaves are flushed when focus lands elsewhere (pane, panel) + // or when the window is deactivated. + cx.deactivate_window(); cx.executor().run_until_parked(); item.read_with(cx, |item, _| assert_eq!(item.save_count, 2)); + cx.update(|window, _| window.activate_window()); // Deactivating the window still saves the file. item.update_in(cx, |item, window, cx| { @@ -11792,19 +11812,23 @@ mod tests { ); }); - // Blurring the item saves the file. This is the core regression scenario: + // Focus leaving the item saves the file. This is the core regression scenario: // with `on_blur`, this would NOT trigger because `on_blur` only fires when // the item's own focus handle is the leaf that lost focus. In a multibuffer, // the leaf is always a child focus handle, so `on_blur` never detected // focus leaving the item. - item.update_in(cx, |_, window, _| window.blur()); + // + // With deferred saves, the save happens when focus lands on a pane/panel or + // the window deactivates. + cx.deactivate_window(); cx.executor().run_until_parked(); item.read_with(cx, |item, _| { assert_eq!( item.save_count, 1, - "Blurring should trigger autosave when focus was on a child of the item" + "Window deactivation should trigger autosave when focus was on a child of the item" ); }); + cx.update(|window, _| window.activate_window()); // Deactivating the window should also trigger autosave when a child of // the multibuffer item currently owns focus. @@ -11824,6 +11848,141 @@ mod tests { }); } + #[gpui::test] + async fn test_autosave_deferred_for_modals(cx: &mut gpui::TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let (workspace, cx) = + cx.add_window_view(|window, cx| Workspace::test_new(project, window, cx)); + + let item = cx.new(|cx| { + TestItem::new(cx).with_project_items(&[TestProjectItem::new(1, "1.txt", cx)]) + }); + + workspace.update_in(cx, |workspace, window, cx| { + workspace.add_item_to_active_pane(Box::new(item.clone()), None, true, window, cx); + }); + + item.update_in(cx, |item, window, cx| { + SettingsStore::update_global(cx, |settings, cx| { + settings.update_user_settings(cx, |settings| { + settings.workspace.autosave = Some(AutosaveSetting::OnFocusChange); + }) + }); + item.is_dirty = true; + cx.focus_self(window); + }); + cx.executor().run_until_parked(); + + // Opening a modal moves focus away from the item, but autosave should be + // deferred until focus lands on a pane or panel (not saved immediately). + workspace.update_in(cx, |workspace, window, cx| { + workspace.toggle_modal(window, cx, TestModal::new); + }); + cx.executor().run_until_parked(); + item.read_with(cx, |item, _| { + assert_eq!( + item.save_count, 0, + "Opening a modal should NOT immediately trigger autosave" + ); + }); + + // If focus returns to the same item (modal dismissed), the deferred save + // should be skipped. + workspace.update_in(cx, |workspace, window, cx| { + workspace.modal_layer.update(cx, |modal, cx| { + modal.hide_modal(window, cx); + }); + }); + cx.executor().run_until_parked(); + item.read_with(cx, |item, _| { + assert_eq!( + item.save_count, 0, + "Returning focus to the same item should skip deferred save" + ); + }); + + // Open modal again with a dirty item. + item.update_in(cx, |item, window, cx| { + item.is_dirty = true; + cx.focus_self(window); + }); + workspace.update_in(cx, |workspace, window, cx| { + workspace.toggle_modal(window, cx, TestModal::new); + }); + cx.executor().run_until_parked(); + item.read_with(cx, |item, _| { + assert_eq!(item.save_count, 0, "Modal open should not trigger save"); + }); + + // Window deactivation should flush deferred saves. + cx.deactivate_window(); + cx.executor().run_until_parked(); + item.read_with(cx, |item, _| { + assert_eq!( + item.save_count, 1, + "Window deactivation should flush deferred saves" + ); + }); + } + + #[gpui::test] + async fn test_autosave_deferred_until_pane_focus(cx: &mut gpui::TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let (workspace, cx) = + cx.add_window_view(|window, cx| Workspace::test_new(project, window, cx)); + + let item1 = cx.new(|cx| { + TestItem::new(cx).with_project_items(&[TestProjectItem::new(1, "1.txt", cx)]) + }); + let item2 = cx.new(|cx| { + TestItem::new(cx).with_project_items(&[TestProjectItem::new(2, "2.txt", cx)]) + }); + + let pane = workspace.update_in(cx, |workspace, window, cx| { + workspace.add_item_to_active_pane(Box::new(item1.clone()), None, false, window, cx); + workspace.add_item_to_active_pane(Box::new(item2.clone()), None, false, window, cx); + workspace.active_pane().clone() + }); + // Ensure added_to_pane is called for both items (sets up focus handlers) + cx.executor().run_until_parked(); + + // Activate item1 (at index 0) and focus it. + pane.update_in(cx, |pane, window, cx| { + pane.activate_item(0, true, true, window, cx); + }); + cx.executor().run_until_parked(); + + // Set up OnFocusChange autosave and make item1 dirty. + item1.update(cx, |item, cx| { + SettingsStore::update_global(cx, |settings, cx| { + settings.update_user_settings(cx, |settings| { + settings.workspace.autosave = Some(AutosaveSetting::OnFocusChange); + }) + }); + item.is_dirty = true; + }); + cx.executor().run_until_parked(); + + // Activate item2 via the pane - this should trigger autosave of item1. + pane.update_in(cx, |pane, window, cx| { + pane.activate_item(1, true, true, window, cx); + }); + cx.executor().run_until_parked(); + + item1.read_with(cx, |item, _| { + assert_eq!( + item.save_count, 1, + "Switching to another item should trigger deferred save of the previous item" + ); + }); + } + #[gpui::test] async fn test_pane_navigation(cx: &mut gpui::TestAppContext) { init_test(cx);