Marshall Bowers and Bennet Bo Fenner created
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 <bennetbo@gmx.de>Cargo.lock | 2 +
crates/command_palette/src/command_palette.rs | 6 +++
crates/vim_mode_setting/Cargo.toml | 1
crates/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(-)
@@ -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",
@@ -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<Picker<CommandPaletteDelegate>>,
@@ -12,4 +12,5 @@ workspace = true
path = "src/vim_mode_setting.rs"
[dependencies]
+gpui.workspace = true
settings.workspace = true
@@ -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())
@@ -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]
@@ -946,15 +946,29 @@ impl<T: Item> ItemHandle for Entity<T> {
// 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);
}
},
)
@@ -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<V: ModalView> ModalViewHandle for Entity<V> {
@@ -51,6 +61,10 @@ impl<V: ModalView> ModalViewHandle for Entity<V> {
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 {
@@ -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<V: ManagedView + 'static>(&self, cx: &App) -> Option<Entity<V>> {
self.modal_layer.read(cx).active_modal()
}