From 0b557a5f5c366b65be0bb9fa60f1197ee37a4ee0 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Thu, 19 Mar 2026 12:53:26 -0400 Subject: [PATCH] workspace: Only suppress auto-save on focus transfer for the Vim/Helix command palette (#51949) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR adjusts the logic that was added in #45166 to just apply to the specific case of interacting with the command palette in Vim and Helix modes (hereafter referred to collectively as "Vim mode"). In that PR, we would suppress the auto-save on focus change for _any_ modal in the workspace, regardless of whether we were actually in Vim mode or not. This would cause issues where moving between files some other way—such as the tab switcher or the file finder—would cause the buffers to never be saved. We now only suppress the auto-save on focus loss behavior when in Vim mode and the active modal is the command palette. In all other cases, we save the file. Closes https://github.com/zed-industries/zed/issues/47968. Supersedes https://github.com/zed-industries/zed/pull/51801 and https://github.com/zed-industries/zed/pull/51802. Note: the way we are identifying the active modal as the command palette isn't the best, but @bennetbo and I didn't have any other cleaner solutions. It's a bit tricky, as the logic lives in the `workspace`, which isn't able to know about the `CommandPalette` due to `command_palette` depending on `workspace`. There may be some other way we could achieve this with more indirection, but it's unclear whether it would be worth it at this time. Release Notes: - Changed `{ "autosave": "on_focus_change" }` to now always save on focus loss, except for when activating the command palette when in Vim/Helix mode. Co-authored-by: Bennet Bo Fenner --- Cargo.lock | 2 ++ crates/command_palette/src/command_palette.rs | 6 +++- crates/vim_mode_setting/Cargo.toml | 1 + .../vim_mode_setting/src/vim_mode_setting.rs | 17 +++++++++++ crates/workspace/Cargo.toml | 1 + crates/workspace/src/item.rs | 28 ++++++++++++++----- crates/workspace/src/modal_layer.rs | 21 ++++++++++++++ crates/workspace/src/workspace.rs | 6 ++++ 8 files changed, 74 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 40bf126b74604675570a006d1b4f72b4bd36a3fc..ddfb654ff50569612390fb9f155dafadb07f6d59 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -19144,6 +19144,7 @@ dependencies = [ name = "vim_mode_setting" version = "0.1.0" dependencies = [ + "gpui", "settings", ] @@ -21486,6 +21487,7 @@ 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 a1a7c33f19549d15075cb9d8f243703d6b1e4cb1..579946f30d88db379f6649fd65b13d7d291e19de 100644 --- a/crates/command_palette/src/command_palette.rs +++ b/crates/command_palette/src/command_palette.rs @@ -33,7 +33,11 @@ pub fn init(cx: &mut App) { cx.observe_new(CommandPalette::register).detach(); } -impl ModalView for CommandPalette {} +impl ModalView for CommandPalette { + fn is_command_palette(&self) -> bool { + true + } +} pub struct CommandPalette { picker: Entity>, diff --git a/crates/vim_mode_setting/Cargo.toml b/crates/vim_mode_setting/Cargo.toml index 0ae75d9d55136a499492893afdf14398073c6df3..6306d125b27a5342a61f503520692c099ab9c4f6 100644 --- a/crates/vim_mode_setting/Cargo.toml +++ b/crates/vim_mode_setting/Cargo.toml @@ -12,4 +12,5 @@ workspace = true path = "src/vim_mode_setting.rs" [dependencies] +gpui.workspace = true settings.workspace = true diff --git a/crates/vim_mode_setting/src/vim_mode_setting.rs b/crates/vim_mode_setting/src/vim_mode_setting.rs index e229913a80b0bedcd4ef7b872f1559b98c803d0c..cb9ab03785c9e00459733c62f1b524cea422bfa1 100644 --- a/crates/vim_mode_setting/src/vim_mode_setting.rs +++ b/crates/vim_mode_setting/src/vim_mode_setting.rs @@ -4,6 +4,7 @@ //! disable Vim/Helix modes without having to depend on the `vim` crate in its //! entirety. +use gpui::App; use settings::{RegisterSetting, Settings, SettingsContent}; #[derive(RegisterSetting)] @@ -15,9 +16,25 @@ impl Settings for VimModeSetting { } } +impl VimModeSetting { + pub fn is_enabled(cx: &App) -> bool { + Self::try_get(cx) + .map(|vim_mode| vim_mode.0) + .unwrap_or(false) + } +} + #[derive(RegisterSetting)] pub struct HelixModeSetting(pub bool); +impl HelixModeSetting { + pub fn is_enabled(cx: &App) -> bool { + Self::try_get(cx) + .map(|helix_mode| helix_mode.0) + .unwrap_or(false) + } +} + impl Settings for HelixModeSetting { fn from_settings(content: &SettingsContent) -> Self { Self(content.helix_mode.unwrap()) diff --git a/crates/workspace/Cargo.toml b/crates/workspace/Cargo.toml index e884b834af1294a368ad67d72057561b42876ce2..d5e9400353eee50b3c5734a31684abdb0149caa0 100644 --- a/crates/workspace/Cargo.toml +++ b/crates/workspace/Cargo.toml @@ -65,6 +65,7 @@ theme.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 d4d31739779e7872e29005b180f2e4682ef808af..8e00753a86d67c819dae38613797ccbeff34edf9 100644 --- a/crates/workspace/src/item.rs +++ b/crates/workspace/src/item.rs @@ -946,15 +946,29 @@ impl ItemHandle for Entity { // Only trigger autosave if focus has truly left the item. // If focus is still within the item's hierarchy (e.g., moved to a context menu), // don't trigger autosave to avoid unwanted formatting and cursor jumps. - // Also skip autosave if focus moved to a modal (e.g., command palette), - // since the user is still interacting with the workspace. let focus_handle = item.item_focus_handle(cx); - if !focus_handle.contains_focused(window, cx) - && !workspace.has_active_modal(window, cx) - { - Pane::autosave_item(&item, workspace.project.clone(), window, cx) - .detach_and_log_err(cx); + if focus_handle.contains_focused(window, cx) { + 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; + } + } + + 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 5949c0b1fffb216f27c939330954ecd8c7343a5c..cb6f21206fc5e1348224dd3e01e5155880e5d883 100644 --- a/crates/workspace/src/modal_layer.rs +++ b/crates/workspace/src/modal_layer.rs @@ -26,6 +26,15 @@ 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 { @@ -33,6 +42,7 @@ 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 { @@ -51,6 +61,10 @@ 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 { @@ -189,6 +203,13 @@ 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 79c2aedc9da40518da32c215dd7edef6a4fa6ef8..5ecd607b785419ac44af2553ad15d9ce8c91ff48 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -6998,6 +6998,12 @@ 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() }