From ea3a1745f56ed819aca7c3b235e123a5bedff0b9 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Thu, 7 Sep 2023 22:48:01 -0600 Subject: [PATCH 01/14] Add vim-specific interactions to command This mostly adds the commonly requested set (:wq and friends) and a few that I use frequently : to go to a line number :vsp / :sp to create a split :cn / :cp to go to diagnostics --- assets/keymaps/vim.json | 1 + crates/command_palette/src/command_palette.rs | 47 +++- crates/vim/src/command.rs | 219 ++++++++++++++++++ crates/vim/src/vim.rs | 9 + crates/workspace/src/pane.rs | 32 ++- crates/workspace/src/workspace.rs | 103 ++++++-- crates/zed/src/menus.rs | 21 +- 7 files changed, 406 insertions(+), 26 deletions(-) create mode 100644 crates/vim/src/command.rs diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 9e69240d27200c591e110fd5578a85b13fe96668..1a6a752e23f931658f352f86b4d8a201cbedd067 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -18,6 +18,7 @@ } } ], + ":": "command_palette::Toggle", "h": "vim::Left", "left": "vim::Left", "backspace": "vim::Backspace", diff --git a/crates/command_palette/src/command_palette.rs b/crates/command_palette/src/command_palette.rs index 4f9bb231ce1aaa68c4fa994bf4f85a09db05fb95..06f76bbc5c68a848de11823d6746e693e600c1e5 100644 --- a/crates/command_palette/src/command_palette.rs +++ b/crates/command_palette/src/command_palette.rs @@ -18,6 +18,15 @@ actions!(command_palette, [Toggle]); pub type CommandPalette = Picker; +pub type CommandPaletteInterceptor = + Box Option>; + +pub struct CommandInterceptResult { + pub action: Box, + pub string: String, + pub positions: Vec, +} + pub struct CommandPaletteDelegate { actions: Vec, matches: Vec, @@ -136,7 +145,7 @@ impl PickerDelegate for CommandPaletteDelegate { char_bag: command.name.chars().collect(), }) .collect::>(); - let matches = if query.is_empty() { + let mut matches = if query.is_empty() { candidates .into_iter() .enumerate() @@ -158,6 +167,40 @@ impl PickerDelegate for CommandPaletteDelegate { ) .await }; + let intercept_result = cx.read(|cx| { + if cx.has_global::() { + cx.global::()(&query, cx) + } else { + None + } + }); + if let Some(CommandInterceptResult { + action, + string, + positions, + }) = intercept_result + { + if let Some(idx) = matches + .iter() + .position(|m| actions[m.candidate_id].action.id() == action.id()) + { + matches.remove(idx); + } + actions.push(Command { + name: string.clone(), + action, + keystrokes: vec![], + }); + matches.insert( + 0, + StringMatch { + candidate_id: actions.len() - 1, + string, + positions, + score: 0.0, + }, + ) + } picker .update(&mut cx, |picker, _| { let delegate = picker.delegate_mut(); @@ -254,7 +297,7 @@ impl PickerDelegate for CommandPaletteDelegate { } } -fn humanize_action_name(name: &str) -> String { +pub fn humanize_action_name(name: &str) -> String { let capacity = name.len() + name.chars().filter(|c| c.is_uppercase()).count(); let mut result = String::with_capacity(capacity); for char in name.chars() { diff --git a/crates/vim/src/command.rs b/crates/vim/src/command.rs new file mode 100644 index 0000000000000000000000000000000000000000..a1f4777ec6b8d8fcf588eec4834eb08b0da76939 --- /dev/null +++ b/crates/vim/src/command.rs @@ -0,0 +1,219 @@ +use command_palette::{humanize_action_name, CommandInterceptResult}; +use gpui::{actions, impl_actions, Action, AppContext, AsyncAppContext, ViewContext}; +use itertools::Itertools; +use serde::{Deserialize, Serialize}; +use workspace::{SaveBehavior, Workspace}; + +use crate::{ + motion::{motion, Motion}, + normal::JoinLines, + Vim, +}; + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct GoToLine { + pub line: u32, +} + +impl_actions!(vim, [GoToLine]); + +pub fn init(cx: &mut AppContext) { + cx.add_action(|_: &mut Workspace, action: &GoToLine, cx| { + Vim::update(cx, |vim, cx| { + vim.push_operator(crate::state::Operator::Number(action.line as usize), cx) + }); + motion(Motion::StartOfDocument, cx) + }); +} + +pub fn command_interceptor(mut query: &str, _: &AppContext) -> Option { + while query.starts_with(":") { + query = &query[1..]; + } + + let (name, action) = match query { + // :w + "w" | "wr" | "wri" | "writ" | "write" => ( + "write", + workspace::Save { + save_behavior: Some(SaveBehavior::PromptOnConflict), + } + .boxed_clone(), + ), + "w!" | "wr!" | "wri!" | "writ!" | "write!" => ( + "write", + workspace::Save { + save_behavior: Some(SaveBehavior::SilentlyOverwrite), + } + .boxed_clone(), + ), + + // :q + "q" | "qu" | "qui" | "quit" => ( + "quit", + workspace::CloseActiveItem { + save_behavior: Some(SaveBehavior::PromptOnWrite), + } + .boxed_clone(), + ), + "q!" | "qu!" | "qui!" | "quit!" => ( + "quit!", + workspace::CloseActiveItem { + save_behavior: Some(SaveBehavior::DontSave), + } + .boxed_clone(), + ), + + // :wq + "wq" => ( + "wq", + workspace::CloseActiveItem { + save_behavior: Some(SaveBehavior::PromptOnConflict), + } + .boxed_clone(), + ), + "wq!" => ( + "wq!", + workspace::CloseActiveItem { + save_behavior: Some(SaveBehavior::SilentlyOverwrite), + } + .boxed_clone(), + ), + // :x + "x" | "xi" | "xit" | "exi" | "exit" => ( + "exit", + workspace::CloseActiveItem { + save_behavior: Some(SaveBehavior::PromptOnConflict), + } + .boxed_clone(), + ), + "x!" | "xi!" | "xit!" | "exi!" | "exit!" => ( + "xit", + workspace::CloseActiveItem { + save_behavior: Some(SaveBehavior::SilentlyOverwrite), + } + .boxed_clone(), + ), + + // :wa + "wa" | "wal" | "wall" => ( + "wall", + workspace::SaveAll { + save_behavior: Some(SaveBehavior::PromptOnConflict), + } + .boxed_clone(), + ), + "wa!" | "wal!" | "wall!" => ( + "wall!", + workspace::SaveAll { + save_behavior: Some(SaveBehavior::SilentlyOverwrite), + } + .boxed_clone(), + ), + + // :qa + "qa" | "qal" | "qall" | "quita" | "quital" | "quitall" => ( + "quitall", + workspace::CloseAllItemsAndPanes { + save_behavior: Some(SaveBehavior::PromptOnWrite), + } + .boxed_clone(), + ), + "qa!" | "qal!" | "qall!" | "quita!" | "quital!" | "quitall!" => ( + "quitall!", + workspace::CloseAllItemsAndPanes { + save_behavior: Some(SaveBehavior::DontSave), + } + .boxed_clone(), + ), + + // :cq + "cq" | "cqu" | "cqui" | "cquit" | "cq!" | "cqu!" | "cqui!" | "cquit!" => ( + "cquit!", + workspace::CloseAllItemsAndPanes { + save_behavior: Some(SaveBehavior::DontSave), + } + .boxed_clone(), + ), + + // :xa + "xa" | "xal" | "xall" => ( + "xall", + workspace::CloseAllItemsAndPanes { + save_behavior: Some(SaveBehavior::PromptOnConflict), + } + .boxed_clone(), + ), + "xa!" | "xal!" | "xall!" => ( + "zall!", + workspace::CloseAllItemsAndPanes { + save_behavior: Some(SaveBehavior::SilentlyOverwrite), + } + .boxed_clone(), + ), + + // :wqa + "wqa" | "wqal" | "wqall" => ( + "wqall", + workspace::CloseAllItemsAndPanes { + save_behavior: Some(SaveBehavior::PromptOnConflict), + } + .boxed_clone(), + ), + "wqa!" | "wqal!" | "wqall!" => ( + "wqall!", + workspace::CloseAllItemsAndPanes { + save_behavior: Some(SaveBehavior::SilentlyOverwrite), + } + .boxed_clone(), + ), + + "j" | "jo" | "joi" | "join" => ("join", JoinLines.boxed_clone()), + + "sp" | "spl" | "spli" | "split" => ("split", workspace::SplitUp.boxed_clone()), + "vs" | "vsp" | "vspl" | "vspli" | "vsplit" => { + ("vsplit", workspace::SplitLeft.boxed_clone()) + } + "cn" | "cne" | "cnex" | "cnext" => ("cnext", editor::GoToDiagnostic.boxed_clone()), + "cp" | "cpr" | "cpre" | "cprev" => ("cprev", editor::GoToPrevDiagnostic.boxed_clone()), + + _ => { + if let Ok(line) = query.parse::() { + (query, GoToLine { line }.boxed_clone()) + } else { + return None; + } + } + }; + + let string = ":".to_owned() + name; + let positions = generate_positions(&string, query); + + Some(CommandInterceptResult { + action, + string, + positions, + }) +} + +fn generate_positions(string: &str, query: &str) -> Vec { + let mut positions = Vec::new(); + let mut chars = query.chars().into_iter(); + + let Some(mut current) = chars.next() else { + return positions; + }; + + for (i, c) in string.chars().enumerate() { + if c == current { + positions.push(i); + if let Some(c) = chars.next() { + current = c; + } else { + break; + } + } + } + + positions +} diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index e2fa6e989ab8ca674e322cb1e5fcaebef7a0471b..6ff997a16163234a5c66bd4568ccb803901542a4 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -1,6 +1,7 @@ #[cfg(test)] mod test; +mod command; mod editor_events; mod insert; mod mode_indicator; @@ -13,6 +14,7 @@ mod visual; use anyhow::Result; use collections::{CommandPaletteFilter, HashMap}; +use command_palette::CommandPaletteInterceptor; use editor::{movement, Editor, EditorMode, Event}; use gpui::{ actions, impl_actions, keymap_matcher::KeymapContext, keymap_matcher::MatchResult, Action, @@ -63,6 +65,7 @@ pub fn init(cx: &mut AppContext) { insert::init(cx); object::init(cx); motion::init(cx); + command::init(cx); // Vim Actions cx.add_action(|_: &mut Workspace, &SwitchMode(mode): &SwitchMode, cx| { @@ -469,6 +472,12 @@ impl Vim { } }); + if self.enabled { + cx.set_global::(Box::new(command::command_interceptor)); + } else if cx.has_global::() { + let _ = cx.remove_global::(); + } + cx.update_active_window(|cx| { if self.enabled { let active_editor = cx diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index a3e6a547ddfd73c1fd67c50c4b6c4df2d6ff51d1..5275a2664ad2b2b6b9b15cf2b7a8ed988b9e9390 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -78,10 +78,17 @@ pub struct CloseItemsToTheRightById { } #[derive(Clone, PartialEq, Debug, Deserialize, Default)] +#[serde(rename_all = "camelCase")] pub struct CloseActiveItem { pub save_behavior: Option, } +#[derive(Clone, PartialEq, Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct CloseAllItems { + pub save_behavior: Option, +} + actions!( pane, [ @@ -92,7 +99,6 @@ actions!( CloseCleanItems, CloseItemsToTheLeft, CloseItemsToTheRight, - CloseAllItems, GoBack, GoForward, ReopenClosedItem, @@ -103,7 +109,7 @@ actions!( ] ); -impl_actions!(pane, [ActivateItem, CloseActiveItem]); +impl_actions!(pane, [ActivateItem, CloseActiveItem, CloseAllItems]); const MAX_NAVIGATION_HISTORY_LEN: usize = 1024; @@ -829,14 +835,18 @@ impl Pane { pub fn close_all_items( &mut self, - _: &CloseAllItems, + action: &CloseAllItems, cx: &mut ViewContext, ) -> Option>> { if self.items.is_empty() { return None; } - Some(self.close_items(cx, SaveBehavior::PromptOnWrite, |_| true)) + Some(self.close_items( + cx, + action.save_behavior.unwrap_or(SaveBehavior::PromptOnWrite), + |_| true, + )) } pub fn close_items( @@ -1175,7 +1185,12 @@ impl Pane { ContextMenuItem::action("Close Clean Items", CloseCleanItems), ContextMenuItem::action("Close Items To The Left", CloseItemsToTheLeft), ContextMenuItem::action("Close Items To The Right", CloseItemsToTheRight), - ContextMenuItem::action("Close All Items", CloseAllItems), + ContextMenuItem::action( + "Close All Items", + CloseAllItems { + save_behavior: None, + }, + ), ] } else { // In the case of the user right clicking on a non-active tab, for some item-closing commands, we need to provide the id of the tab, for the others, we can reuse the existing command. @@ -1219,7 +1234,12 @@ impl Pane { } } }), - ContextMenuItem::action("Close All Items", CloseAllItems), + ContextMenuItem::action( + "Close All Items", + CloseAllItems { + save_behavior: None, + }, + ), ] }, cx, diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index feab53d0941d1823fc79930d3697c39e54822207..c297962684b803f8f2edc6a9abbdc0312455b332 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -122,13 +122,11 @@ actions!( Open, NewFile, NewWindow, - CloseWindow, CloseInactiveTabsAndPanes, AddFolderToProject, Unfollow, - Save, SaveAs, - SaveAll, + ReloadActiveItem, ActivatePreviousPane, ActivateNextPane, FollowNextCollaborator, @@ -158,6 +156,30 @@ pub struct ActivatePane(pub usize); #[derive(Clone, Deserialize, PartialEq)] pub struct ActivatePaneInDirection(pub SplitDirection); +#[derive(Clone, PartialEq, Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct SaveAll { + pub save_behavior: Option, +} + +#[derive(Clone, PartialEq, Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct Save { + pub save_behavior: Option, +} + +#[derive(Clone, PartialEq, Debug, Deserialize, Default)] +#[serde(rename_all = "camelCase")] +pub struct CloseWindow { + pub save_behavior: Option, +} + +#[derive(Clone, PartialEq, Debug, Deserialize, Default)] +#[serde(rename_all = "camelCase")] +pub struct CloseAllItemsAndPanes { + pub save_behavior: Option, +} + #[derive(Deserialize)] pub struct Toast { id: usize, @@ -210,7 +232,16 @@ pub struct OpenTerminal { impl_actions!( workspace, - [ActivatePane, ActivatePaneInDirection, Toast, OpenTerminal] + [ + ActivatePane, + ActivatePaneInDirection, + Toast, + OpenTerminal, + SaveAll, + Save, + CloseWindow, + CloseAllItemsAndPanes, + ] ); pub type WorkspaceId = i64; @@ -251,6 +282,7 @@ pub fn init(app_state: Arc, cx: &mut AppContext) { cx.add_async_action(Workspace::follow_next_collaborator); cx.add_async_action(Workspace::close); cx.add_async_action(Workspace::close_inactive_items_and_panes); + cx.add_async_action(Workspace::close_all_items_and_panes); cx.add_global_action(Workspace::close_global); cx.add_global_action(restart); cx.add_async_action(Workspace::save_all); @@ -1262,11 +1294,15 @@ impl Workspace { pub fn close( &mut self, - _: &CloseWindow, + action: &CloseWindow, cx: &mut ViewContext, ) -> Option>> { let window = cx.window(); - let prepare = self.prepare_to_close(false, cx); + let prepare = self.prepare_to_close( + false, + action.save_behavior.unwrap_or(SaveBehavior::PromptOnWrite), + cx, + ); Some(cx.spawn(|_, mut cx| async move { if prepare.await? { window.remove(&mut cx); @@ -1323,8 +1359,17 @@ impl Workspace { }) } - fn save_all(&mut self, _: &SaveAll, cx: &mut ViewContext) -> Option>> { - let save_all = self.save_all_internal(SaveBehavior::PromptOnConflict, cx); + fn save_all( + &mut self, + action: &SaveAll, + cx: &mut ViewContext, + ) -> Option>> { + let save_all = self.save_all_internal( + action + .save_behavior + .unwrap_or(SaveBehavior::PromptOnConflict), + cx, + ); Some(cx.foreground().spawn(async move { save_all.await?; Ok(()) @@ -1691,24 +1736,52 @@ impl Workspace { &mut self, _: &CloseInactiveTabsAndPanes, cx: &mut ViewContext, + ) -> Option>> { + self.close_all_internal(true, SaveBehavior::PromptOnWrite, cx) + } + + pub fn close_all_items_and_panes( + &mut self, + action: &CloseAllItemsAndPanes, + cx: &mut ViewContext, + ) -> Option>> { + self.close_all_internal( + false, + action.save_behavior.unwrap_or(SaveBehavior::PromptOnWrite), + cx, + ) + } + + fn close_all_internal( + &mut self, + retain_active_pane: bool, + save_behavior: SaveBehavior, + cx: &mut ViewContext, ) -> Option>> { let current_pane = self.active_pane(); let mut tasks = Vec::new(); - if let Some(current_pane_close) = current_pane.update(cx, |pane, cx| { - pane.close_inactive_items(&CloseInactiveItems, cx) - }) { - tasks.push(current_pane_close); - }; + if retain_active_pane { + if let Some(current_pane_close) = current_pane.update(cx, |pane, cx| { + pane.close_inactive_items(&CloseInactiveItems, cx) + }) { + tasks.push(current_pane_close); + }; + } for pane in self.panes() { - if pane.id() == current_pane.id() { + if retain_active_pane && pane.id() == current_pane.id() { continue; } if let Some(close_pane_items) = pane.update(cx, |pane: &mut Pane, cx| { - pane.close_all_items(&CloseAllItems, cx) + pane.close_all_items( + &CloseAllItems { + save_behavior: Some(save_behavior), + }, + cx, + ) }) { tasks.push(close_pane_items) } diff --git a/crates/zed/src/menus.rs b/crates/zed/src/menus.rs index 6b5f7b3a35868ffc20e9a26bc6b694bbe2a9ecba..da3f7e4c32e1fbc535dcb071e9a2fa55df1b4ae6 100644 --- a/crates/zed/src/menus.rs +++ b/crates/zed/src/menus.rs @@ -38,16 +38,31 @@ pub fn menus() -> Vec> { MenuItem::action("Open Recent...", recent_projects::OpenRecent), MenuItem::separator(), MenuItem::action("Add Folder to Project…", workspace::AddFolderToProject), - MenuItem::action("Save", workspace::Save), + MenuItem::action( + "Save", + workspace::Save { + save_behavior: None, + }, + ), MenuItem::action("Save As…", workspace::SaveAs), - MenuItem::action("Save All", workspace::SaveAll), + MenuItem::action( + "Save All", + workspace::SaveAll { + save_behavior: None, + }, + ), MenuItem::action( "Close Editor", workspace::CloseActiveItem { save_behavior: None, }, ), - MenuItem::action("Close Window", workspace::CloseWindow), + MenuItem::action( + "Close Window", + workspace::CloseWindow { + save_behavior: None, + }, + ), ], }, Menu { From ba5d84f7e82433b427e53d566a28173a391c8a23 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 19 Sep 2023 21:27:22 -0600 Subject: [PATCH 02/14] Fix vim tests on my machine In a rare case of "it broke on my machine" I haven't been able to run the vim tests locally for a few days; turns out I ran out of swap file names... --- crates/vim/src/test/neovim_connection.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/vim/src/test/neovim_connection.rs b/crates/vim/src/test/neovim_connection.rs index e44e8d0e4cb67dd13a7906767e62a72379b5aeec..38af2d1555e3af892a732e8d9b5ced60638313da 100644 --- a/crates/vim/src/test/neovim_connection.rs +++ b/crates/vim/src/test/neovim_connection.rs @@ -65,7 +65,13 @@ impl NeovimConnection { // Ensure we don't create neovim connections in parallel let _lock = NEOVIM_LOCK.lock(); let (nvim, join_handle, child) = new_child_cmd( - &mut Command::new("nvim").arg("--embed").arg("--clean"), + &mut Command::new("nvim") + .arg("--embed") + .arg("--clean") + // disable swap (otherwise after about 1000 test runs you run out of swap file names) + .arg("-n") + // disable writing files (just in case) + .arg("-m"), handler, ) .await From e27b7d78120b8dcdc2cc0982b4c7e60baa0acb85 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 19 Sep 2023 22:20:20 -0600 Subject: [PATCH 03/14] Ensure the picker waits for pending updates Particularly in development builds (and in tests), when typing in the command palette, I tend to hit enter before the suggestions have settled. --- crates/picker/src/picker.rs | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/crates/picker/src/picker.rs b/crates/picker/src/picker.rs index 700b69117ae07767a449db4b2d856974a6f0b5de..89bfaa4b707c677f269558944bab6b8f008326c7 100644 --- a/crates/picker/src/picker.rs +++ b/crates/picker/src/picker.rs @@ -25,7 +25,8 @@ pub struct Picker { max_size: Vector2F, theme: Arc theme::Picker>>>, confirmed: bool, - pending_update_matches: Task>, + pending_update_matches: Option>>, + confirm_on_update: Option, has_focus: bool, } @@ -208,7 +209,8 @@ impl Picker { max_size: vec2f(540., 420.), theme, confirmed: false, - pending_update_matches: Task::ready(None), + pending_update_matches: None, + confirm_on_update: None, has_focus: false, }; this.update_matches(String::new(), cx); @@ -263,11 +265,13 @@ impl Picker { pub fn update_matches(&mut self, query: String, cx: &mut ViewContext) { let update = self.delegate.update_matches(query, cx); self.matches_updated(cx); - self.pending_update_matches = cx.spawn(|this, mut cx| async move { + self.pending_update_matches = Some(cx.spawn(|this, mut cx| async move { update.await; - this.update(&mut cx, |this, cx| this.matches_updated(cx)) - .log_err() - }); + this.update(&mut cx, |this, cx| { + this.matches_updated(cx); + }) + .log_err() + })); } fn matches_updated(&mut self, cx: &mut ViewContext) { @@ -278,6 +282,11 @@ impl Picker { ScrollTarget::Show(index) }; self.list_state.scroll_to(target); + self.pending_update_matches = None; + if let Some(secondary) = self.confirm_on_update.take() { + self.confirmed = true; + self.delegate.confirm(secondary, cx) + } cx.notify(); } @@ -331,13 +340,21 @@ impl Picker { } pub fn confirm(&mut self, _: &Confirm, cx: &mut ViewContext) { - self.confirmed = true; - self.delegate.confirm(false, cx); + if self.pending_update_matches.is_some() { + self.confirm_on_update = Some(false) + } else { + self.confirmed = true; + self.delegate.confirm(false, cx); + } } pub fn secondary_confirm(&mut self, _: &SecondaryConfirm, cx: &mut ViewContext) { - self.confirmed = true; - self.delegate.confirm(true, cx); + if self.pending_update_matches.is_some() { + self.confirm_on_update = Some(true) + } else { + self.confirmed = true; + self.delegate.confirm(true, cx); + } } fn cancel(&mut self, _: &Cancel, cx: &mut ViewContext) { From a4f96e64528ecc88840a864fbd45ccee2bcefc85 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 19 Sep 2023 22:36:13 -0600 Subject: [PATCH 04/14] tests: wait deterministically after simulating_keystrokes --- crates/editor/src/test/editor_test_context.rs | 15 +++++++++++++-- crates/vim/src/test.rs | 3 --- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/crates/editor/src/test/editor_test_context.rs b/crates/editor/src/test/editor_test_context.rs index 0bae32f1f7087b05b67f166554671ca6359a6104..0ef54dc3d557843d79d1e457d1651f7bf84e1af1 100644 --- a/crates/editor/src/test/editor_test_context.rs +++ b/crates/editor/src/test/editor_test_context.rs @@ -3,8 +3,8 @@ use crate::{ }; use futures::Future; use gpui::{ - keymap_matcher::Keystroke, AnyWindowHandle, AppContext, ContextHandle, ModelContext, - ViewContext, ViewHandle, + executor::Foreground, keymap_matcher::Keystroke, AnyWindowHandle, AppContext, ContextHandle, + ModelContext, ViewContext, ViewHandle, }; use indoc::indoc; use language::{Buffer, BufferSnapshot}; @@ -114,6 +114,7 @@ impl<'a> EditorTestContext<'a> { let keystroke = Keystroke::parse(keystroke_text).unwrap(); self.cx.dispatch_keystroke(self.window, keystroke, false); + keystroke_under_test_handle } @@ -126,6 +127,16 @@ impl<'a> EditorTestContext<'a> { for keystroke_text in keystroke_texts.into_iter() { self.simulate_keystroke(keystroke_text); } + // it is common for keyboard shortcuts to kick off async actions, so this ensures that they are complete + // before returning. + // NOTE: we don't do this in simulate_keystroke() because a possible cause of bugs is that typing too + // quickly races with async actions. + if let Foreground::Deterministic { cx_id: _, executor } = self.cx.foreground().as_ref() { + executor.run_until_parked(); + } else { + unreachable!(); + } + keystrokes_under_test_handle } diff --git a/crates/vim/src/test.rs b/crates/vim/src/test.rs index e43b0ab22bc237146b1edecb1afbd4b45b4a4c95..82e4cc68630b5f4d5fb197f937192439801ece35 100644 --- a/crates/vim/src/test.rs +++ b/crates/vim/src/test.rs @@ -186,9 +186,6 @@ async fn test_selection_on_search(cx: &mut gpui::TestAppContext) { assert_eq!(bar.query(cx), "cc"); }); - // wait for the query editor change event to fire. - search_bar.next_notification(&cx).await; - cx.update_editor(|editor, cx| { let highlights = editor.all_text_background_highlights(cx); assert_eq!(3, highlights.len()); From 88a32ae48d81dad90c5b30515ce2e8b595c29cb4 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 19 Sep 2023 23:48:21 -0600 Subject: [PATCH 05/14] Merge Workspace::save_item into Pane::save_item These methods were slightly different which caused (for example) there to be no "Discard" option in the conflict case at the workspace level. To make this work, a new SaveBehavior (::PromptForNewPath) was added to support SaveAs. --- crates/workspace/src/pane.rs | 31 ++++++++--- crates/workspace/src/workspace.rs | 91 ++++++++++++------------------- 2 files changed, 58 insertions(+), 64 deletions(-) diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 5275a2664ad2b2b6b9b15cf2b7a8ed988b9e9390..285cf1da603c64977c6942131105c0fe2462b263 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -46,13 +46,15 @@ use theme::{Theme, ThemeSettings}; #[derive(PartialEq, Clone, Copy, Deserialize, Debug)] #[serde(rename_all = "camelCase")] pub enum SaveBehavior { - /// ask before overwriting conflicting files (used by default with %s) + /// ask before overwriting conflicting files (used by default with cmd-s) PromptOnConflict, - /// ask before writing any file that wouldn't be auto-saved (used by default with %w) + /// ask for a new path before writing (used with cmd-shift-s) + PromptForNewPath, + /// ask before writing any file that wouldn't be auto-saved (used by default with cmd-w) PromptOnWrite, /// never prompt, write on conflict (used with vim's :w!) SilentlyOverwrite, - /// skip all save-related behaviour (used with vim's :cq) + /// skip all save-related behaviour (used with vim's :q!) DontSave, } @@ -1019,7 +1021,7 @@ impl Pane { return Ok(true); } - let (has_conflict, is_dirty, can_save, is_singleton) = cx.read(|cx| { + let (mut has_conflict, mut is_dirty, mut can_save, is_singleton) = cx.read(|cx| { ( item.has_conflict(cx), item.is_dirty(cx), @@ -1028,6 +1030,12 @@ impl Pane { ) }); + if save_behavior == SaveBehavior::PromptForNewPath { + has_conflict = false; + is_dirty = true; + can_save = false; + } + if has_conflict && can_save { if save_behavior == SaveBehavior::SilentlyOverwrite { pane.update(cx, |_, cx| item.save(project, cx))?.await?; @@ -2589,10 +2597,17 @@ mod tests { add_labeled_item(&pane, "C", false, cx); assert_item_labels(&pane, ["A", "B", "C*"], cx); - pane.update(cx, |pane, cx| pane.close_all_items(&CloseAllItems, cx)) - .unwrap() - .await - .unwrap(); + pane.update(cx, |pane, cx| { + pane.close_all_items( + &CloseAllItems { + save_behavior: None, + }, + cx, + ) + }) + .unwrap() + .await + .unwrap(); assert_item_labels(&pane, [], cx); } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index c297962684b803f8f2edc6a9abbdc0312455b332..a8e7f12b3a1d3d84c2a50f22ab823fa2341cb947 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -122,6 +122,7 @@ actions!( Open, NewFile, NewWindow, + CloseWindow, CloseInactiveTabsAndPanes, AddFolderToProject, Unfollow, @@ -168,12 +169,6 @@ pub struct Save { pub save_behavior: Option, } -#[derive(Clone, PartialEq, Debug, Deserialize, Default)] -#[serde(rename_all = "camelCase")] -pub struct CloseWindow { - pub save_behavior: Option, -} - #[derive(Clone, PartialEq, Debug, Deserialize, Default)] #[serde(rename_all = "camelCase")] pub struct CloseAllItemsAndPanes { @@ -236,10 +231,9 @@ impl_actions!( ActivatePane, ActivatePaneInDirection, Toast, - OpenTerminal, + OpenTerminal, SaveAll, Save, - CloseWindow, CloseAllItemsAndPanes, ] ); @@ -294,13 +288,22 @@ pub fn init(app_state: Arc, cx: &mut AppContext) { }, ); cx.add_action( - |workspace: &mut Workspace, _: &Save, cx: &mut ViewContext| { - workspace.save_active_item(false, cx).detach_and_log_err(cx); + |workspace: &mut Workspace, action: &Save, cx: &mut ViewContext| { + workspace + .save_active_item( + action + .save_behavior + .unwrap_or(SaveBehavior::PromptOnConflict), + cx, + ) + .detach_and_log_err(cx); }, ); cx.add_action( |workspace: &mut Workspace, _: &SaveAs, cx: &mut ViewContext| { - workspace.save_active_item(true, cx).detach_and_log_err(cx); + workspace + .save_active_item(SaveBehavior::PromptForNewPath, cx) + .detach_and_log_err(cx); }, ); cx.add_action(|workspace: &mut Workspace, _: &ActivatePreviousPane, cx| { @@ -1294,15 +1297,11 @@ impl Workspace { pub fn close( &mut self, - action: &CloseWindow, + _: &CloseWindow, cx: &mut ViewContext, ) -> Option>> { let window = cx.window(); - let prepare = self.prepare_to_close( - false, - action.save_behavior.unwrap_or(SaveBehavior::PromptOnWrite), - cx, - ); + let prepare = self.prepare_to_close(false, cx); Some(cx.spawn(|_, mut cx| async move { if prepare.await? { window.remove(&mut cx); @@ -1685,51 +1684,31 @@ impl Workspace { pub fn save_active_item( &mut self, - force_name_change: bool, + save_behavior: SaveBehavior, cx: &mut ViewContext, ) -> Task> { let project = self.project.clone(); - if let Some(item) = self.active_item(cx) { - if !force_name_change && item.can_save(cx) { - if item.has_conflict(cx) { - const CONFLICT_MESSAGE: &str = "This file has changed on disk since you started editing it. Do you want to overwrite it?"; + let pane = self.active_pane(); + let item_ix = pane.read(cx).active_item_index(); + let item = pane.read(cx).active_item(); + let pane = pane.downgrade(); - let mut answer = cx.prompt( - PromptLevel::Warning, - CONFLICT_MESSAGE, - &["Overwrite", "Cancel"], - ); - cx.spawn(|this, mut cx| async move { - let answer = answer.recv().await; - if answer == Some(0) { - this.update(&mut cx, |this, cx| item.save(this.project.clone(), cx))? - .await?; - } - Ok(()) - }) - } else { - item.save(self.project.clone(), cx) - } - } else if item.is_singleton(cx) { - let worktree = self.worktrees(cx).next(); - let start_abs_path = worktree - .and_then(|w| w.read(cx).as_local()) - .map_or(Path::new(""), |w| w.abs_path()) - .to_path_buf(); - let mut abs_path = cx.prompt_for_new_path(&start_abs_path); - cx.spawn(|this, mut cx| async move { - if let Some(abs_path) = abs_path.recv().await.flatten() { - this.update(&mut cx, |_, cx| item.save_as(project, abs_path, cx))? - .await?; - } - Ok(()) - }) + cx.spawn(|_, mut cx| async move { + if let Some(item) = item { + Pane::save_item( + project, + &pane, + item_ix, + item.as_ref(), + save_behavior, + &mut cx, + ) + .await + .map(|_| ()) } else { - Task::ready(Ok(())) + Ok(()) } - } else { - Task::ready(Ok(())) - } + }) } pub fn close_inactive_items_and_panes( From 6ad1f19a214f212b391c12c9ae6b1ba5291fbb5d Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Wed, 20 Sep 2023 15:00:41 -0600 Subject: [PATCH 06/14] Add NewFileInDirection --- assets/keymaps/vim.json | 12 +++++++++++- crates/editor/src/editor.rs | 25 +++++++++++++++++++++++-- crates/workspace/src/workspace.rs | 15 ++++++++++++--- 3 files changed, 46 insertions(+), 6 deletions(-) diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 1a6a752e23f931658f352f86b4d8a201cbedd067..5db0fe748ef6885f697ec71c321814c44e1a9fcb 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -319,7 +319,17 @@ "ctrl-w c": "pane::CloseAllItems", "ctrl-w ctrl-c": "pane::CloseAllItems", "ctrl-w q": "pane::CloseAllItems", - "ctrl-w ctrl-q": "pane::CloseAllItems" + "ctrl-w ctrl-q": "pane::CloseAllItems", + "ctrl-w o": "workspace::CloseInactiveTabsAndPanes", + "ctrl-w ctrl-o": "workspace::CloseInactiveTabsAndPanes", + "ctrl-w n": [ + "workspace::NewFileInDirection", + "Up" + ], + "ctrl-w ctrl-n": [ + "workspace::NewFileInDirection", + "Up" + ] } }, { diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 0827e1326402e39bfeeab389ad7a9df6d8eb5587..446ddfeab009d51055b5e4aa7e588211b3640f8d 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -103,7 +103,7 @@ use sum_tree::TreeMap; use text::Rope; use theme::{DiagnosticStyle, Theme, ThemeSettings}; use util::{post_inc, RangeExt, ResultExt, TryFutureExt}; -use workspace::{ItemNavHistory, ViewId, Workspace}; +use workspace::{ItemNavHistory, SplitDirection, ViewId, Workspace}; use crate::git::diff_hunk_to_display; @@ -363,6 +363,7 @@ pub fn init_settings(cx: &mut AppContext) { pub fn init(cx: &mut AppContext) { init_settings(cx); cx.add_action(Editor::new_file); + cx.add_action(Editor::new_file_in_direction); cx.add_action(Editor::cancel); cx.add_action(Editor::newline); cx.add_action(Editor::newline_above); @@ -1627,6 +1628,26 @@ impl Editor { } } + pub fn new_file_in_direction( + workspace: &mut Workspace, + action: &workspace::NewFileInDirection, + cx: &mut ViewContext, + ) { + let project = workspace.project().clone(); + if project.read(cx).is_remote() { + cx.propagate_action(); + } else if let Some(buffer) = project + .update(cx, |project, cx| project.create_buffer("", None, cx)) + .log_err() + { + workspace.split_item( + action.0, + Box::new(cx.add_view(|cx| Editor::for_buffer(buffer, Some(project.clone()), cx))), + cx, + ); + } + } + pub fn replica_id(&self, cx: &AppContext) -> ReplicaId { self.buffer.read(cx).replica_id() } @@ -7130,7 +7151,7 @@ impl Editor { ); }); if split { - workspace.split_item(Box::new(editor), cx); + workspace.split_item(SplitDirection::Right, Box::new(editor), cx); } else { workspace.add_item(Box::new(editor), cx); } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index a8e7f12b3a1d3d84c2a50f22ab823fa2341cb947..6043b946214257642082194043395a75d7417138 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -157,6 +157,9 @@ pub struct ActivatePane(pub usize); #[derive(Clone, Deserialize, PartialEq)] pub struct ActivatePaneInDirection(pub SplitDirection); +#[derive(Clone, Deserialize, PartialEq)] +pub struct NewFileInDirection(pub SplitDirection); + #[derive(Clone, PartialEq, Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct SaveAll { @@ -230,6 +233,7 @@ impl_actions!( [ ActivatePane, ActivatePaneInDirection, + NewFileInDirection, Toast, OpenTerminal, SaveAll, @@ -1991,8 +1995,13 @@ impl Workspace { .update(cx, |pane, cx| pane.add_item(item, true, true, None, cx)); } - pub fn split_item(&mut self, item: Box, cx: &mut ViewContext) { - let new_pane = self.split_pane(self.active_pane.clone(), SplitDirection::Right, cx); + pub fn split_item( + &mut self, + split_direction: SplitDirection, + item: Box, + cx: &mut ViewContext, + ) { + let new_pane = self.split_pane(self.active_pane.clone(), split_direction, cx); new_pane.update(cx, move |new_pane, cx| { new_pane.add_item(item, true, true, None, cx) }) @@ -2170,7 +2179,7 @@ impl Workspace { } let item = cx.add_view(|cx| T::for_project_item(self.project().clone(), project_item, cx)); - self.split_item(Box::new(item.clone()), cx); + self.split_item(SplitDirection::Right, Box::new(item.clone()), cx); item } From 2d9db0fed15449a3390aabf70ffc0fd3f98ed8a6 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Wed, 20 Sep 2023 15:24:31 -0600 Subject: [PATCH 07/14] Flesh out v1.0 of vim : --- Cargo.lock | 2 + crates/command_palette/src/command_palette.rs | 2 +- crates/fs/src/fs.rs | 2 +- crates/gpui/src/app/test_app_context.rs | 2 +- crates/search/src/buffer_search.rs | 32 ++- crates/vim/Cargo.toml | 2 + crates/vim/src/command.rs | 258 +++++++++++++++--- crates/vim/src/normal.rs | 9 +- crates/vim/src/normal/search.rs | 196 ++++++++++++- .../neovim_backed_binding_test_context.rs | 17 +- .../src/test/neovim_backed_test_context.rs | 16 +- crates/vim/test_data/test_command_basics.json | 6 + crates/vim/test_data/test_command_goto.json | 5 + .../vim/test_data/test_command_replace.json | 22 ++ crates/zed/src/menus.rs | 7 +- crates/zed/src/zed.rs | 20 +- 16 files changed, 516 insertions(+), 82 deletions(-) create mode 100644 crates/vim/test_data/test_command_basics.json create mode 100644 crates/vim/test_data/test_command_goto.json create mode 100644 crates/vim/test_data/test_command_replace.json diff --git a/Cargo.lock b/Cargo.lock index 3cced78c4272e5fc9f571d8023719a1ea56d06d2..bfae8cae0b08a8a5cc446f21fe54dd639ae75d32 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8860,6 +8860,7 @@ dependencies = [ "async-trait", "collections", "command_palette", + "diagnostics", "editor", "futures 0.3.28", "gpui", @@ -8881,6 +8882,7 @@ dependencies = [ "tokio", "util", "workspace", + "zed-actions", ] [[package]] diff --git a/crates/command_palette/src/command_palette.rs b/crates/command_palette/src/command_palette.rs index 06f76bbc5c68a848de11823d6746e693e600c1e5..90c155cdf8caba22d945d3b982cd4ec88b339828 100644 --- a/crates/command_palette/src/command_palette.rs +++ b/crates/command_palette/src/command_palette.rs @@ -126,7 +126,7 @@ impl PickerDelegate for CommandPaletteDelegate { } }) .collect::>(); - let actions = cx.read(move |cx| { + let mut actions = cx.read(move |cx| { let hit_counts = cx.optional_global::(); actions.sort_by_key(|action| { ( diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index ecaee4534e4b4d4ae11391c688d44caa1bdd0fa0..97175cb55e7f1c8edb494857d1e28ad16d4ee6d1 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -507,7 +507,7 @@ impl FakeFs { state.emit_event(&[path]); } - fn write_file_internal(&self, path: impl AsRef, content: String) -> Result<()> { + pub fn write_file_internal(&self, path: impl AsRef, content: String) -> Result<()> { let mut state = self.state.lock(); let path = path.as_ref(); let inode = state.next_inode; diff --git a/crates/gpui/src/app/test_app_context.rs b/crates/gpui/src/app/test_app_context.rs index 0dc1d1eba437fb67beddbde47bd10bc497ed928a..5b15b5274c60bb835c22e58882f6d7eb95910151 100644 --- a/crates/gpui/src/app/test_app_context.rs +++ b/crates/gpui/src/app/test_app_context.rs @@ -33,7 +33,7 @@ use super::{ #[derive(Clone)] pub struct TestAppContext { - cx: Rc>, + pub cx: Rc>, foreground_platform: Rc, condition_duration: Option, pub function_name: String, diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index 142b548a9341cf459e12eb3072ac57c75183e9ba..97f9d7a7c8f44f2d78e3b362d5f2878c3002a492 100644 --- a/crates/search/src/buffer_search.rs +++ b/crates/search/src/buffer_search.rs @@ -539,6 +539,23 @@ impl BufferSearchBar { .map(|searchable_item| searchable_item.query_suggestion(cx)) } + pub fn set_replacement(&mut self, replacement: Option<&str>, cx: &mut ViewContext) { + if replacement.is_none() { + self.replace_is_active = false; + return; + } + self.replace_is_active = true; + self.replacement_editor + .update(cx, |replacement_editor, cx| { + replacement_editor + .buffer() + .update(cx, |replacement_buffer, cx| { + let len = replacement_buffer.len(cx); + replacement_buffer.edit([(0..len, replacement.unwrap())], None, cx); + }); + }); + } + pub fn search( &mut self, query: &str, @@ -679,6 +696,19 @@ impl BufferSearchBar { } } + pub fn select_last_match(&mut self, cx: &mut ViewContext) { + if let Some(searchable_item) = self.active_searchable_item.as_ref() { + if let Some(matches) = self + .searchable_items_with_matches + .get(&searchable_item.downgrade()) + { + let new_match_index = matches.len() - 1; + searchable_item.update_matches(matches, cx); + searchable_item.activate_match(new_match_index, matches, cx); + } + } + } + fn select_next_match_on_pane( pane: &mut Pane, action: &SelectNextMatch, @@ -934,7 +964,7 @@ impl BufferSearchBar { } } } - fn replace_all(&mut self, _: &ReplaceAll, cx: &mut ViewContext) { + pub fn replace_all(&mut self, _: &ReplaceAll, cx: &mut ViewContext) { if !self.dismissed && self.active_search.is_some() { if let Some(searchable_item) = self.active_searchable_item.as_ref() { if let Some(query) = self.active_search.as_ref() { diff --git a/crates/vim/Cargo.toml b/crates/vim/Cargo.toml index 5d40032024b5f78758e25d6ba0a6e865c827cf5b..509efc58257ce8499d5b27d8e63408ea189d5da6 100644 --- a/crates/vim/Cargo.toml +++ b/crates/vim/Cargo.toml @@ -34,6 +34,8 @@ settings = { path = "../settings" } workspace = { path = "../workspace" } theme = { path = "../theme" } language_selector = { path = "../language_selector"} +diagnostics = { path = "../diagnostics" } +zed-actions = { path = "../zed-actions" } [dev-dependencies] indoc.workspace = true diff --git a/crates/vim/src/command.rs b/crates/vim/src/command.rs index a1f4777ec6b8d8fcf588eec4834eb08b0da76939..10c2599dd9af76d98792898e33a97af1695bf4ac 100644 --- a/crates/vim/src/command.rs +++ b/crates/vim/src/command.rs @@ -1,16 +1,21 @@ -use command_palette::{humanize_action_name, CommandInterceptResult}; -use gpui::{actions, impl_actions, Action, AppContext, AsyncAppContext, ViewContext}; -use itertools::Itertools; -use serde::{Deserialize, Serialize}; +use command_palette::CommandInterceptResult; +use editor::{SortLinesCaseInsensitive, SortLinesCaseSensitive}; +use gpui::{impl_actions, Action, AppContext}; +use serde_derive::Deserialize; use workspace::{SaveBehavior, Workspace}; use crate::{ - motion::{motion, Motion}, - normal::JoinLines, + motion::{EndOfDocument, Motion}, + normal::{ + move_cursor, + search::{FindCommand, ReplaceCommand}, + JoinLines, + }, + state::Mode, Vim, }; -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Deserialize)] pub struct GoToLine { pub line: u32, } @@ -20,19 +25,28 @@ impl_actions!(vim, [GoToLine]); pub fn init(cx: &mut AppContext) { cx.add_action(|_: &mut Workspace, action: &GoToLine, cx| { Vim::update(cx, |vim, cx| { - vim.push_operator(crate::state::Operator::Number(action.line as usize), cx) + vim.switch_mode(Mode::Normal, false, cx); + move_cursor(vim, Motion::StartOfDocument, Some(action.line as usize), cx); }); - motion(Motion::StartOfDocument, cx) }); } pub fn command_interceptor(mut query: &str, _: &AppContext) -> Option { + // Note: this is a very poor simulation of vim's command palette. + // In the future we should adjust it to handle parsing range syntax, + // and then calling the appropriate commands with/without ranges. + // + // We also need to support passing arguments to commands like :w + // (ideally with filename autocompletion). + // + // For now, you can only do a replace on the % range, and you can + // only use a specific line number range to "go to line" while query.starts_with(":") { query = &query[1..]; } let (name, action) = match query { - // :w + // save and quit "w" | "wr" | "wri" | "writ" | "write" => ( "write", workspace::Save { @@ -41,14 +55,12 @@ pub fn command_interceptor(mut query: &str, _: &AppContext) -> Option ( - "write", + "write!", workspace::Save { save_behavior: Some(SaveBehavior::SilentlyOverwrite), } .boxed_clone(), ), - - // :q "q" | "qu" | "qui" | "quit" => ( "quit", workspace::CloseActiveItem { @@ -63,8 +75,6 @@ pub fn command_interceptor(mut query: &str, _: &AppContext) -> Option ( "wq", workspace::CloseActiveItem { @@ -79,7 +89,6 @@ pub fn command_interceptor(mut query: &str, _: &AppContext) -> Option ( "exit", workspace::CloseActiveItem { @@ -88,14 +97,12 @@ pub fn command_interceptor(mut query: &str, _: &AppContext) -> Option ( - "xit", + "exit!", workspace::CloseActiveItem { save_behavior: Some(SaveBehavior::SilentlyOverwrite), } .boxed_clone(), ), - - // :wa "wa" | "wal" | "wall" => ( "wall", workspace::SaveAll { @@ -110,8 +117,6 @@ pub fn command_interceptor(mut query: &str, _: &AppContext) -> Option ( "quitall", workspace::CloseAllItemsAndPanes { @@ -126,17 +131,6 @@ pub fn command_interceptor(mut query: &str, _: &AppContext) -> Option ( - "cquit!", - workspace::CloseAllItemsAndPanes { - save_behavior: Some(SaveBehavior::DontSave), - } - .boxed_clone(), - ), - - // :xa "xa" | "xal" | "xall" => ( "xall", workspace::CloseAllItemsAndPanes { @@ -145,14 +139,12 @@ pub fn command_interceptor(mut query: &str, _: &AppContext) -> Option ( - "zall!", + "xall!", workspace::CloseAllItemsAndPanes { save_behavior: Some(SaveBehavior::SilentlyOverwrite), } .boxed_clone(), ), - - // :wqa "wqa" | "wqal" | "wqall" => ( "wqall", workspace::CloseAllItemsAndPanes { @@ -167,18 +159,89 @@ pub fn command_interceptor(mut query: &str, _: &AppContext) -> Option { + ("cquit!", zed_actions::Quit.boxed_clone()) + } - "j" | "jo" | "joi" | "join" => ("join", JoinLines.boxed_clone()), - + // pane management "sp" | "spl" | "spli" | "split" => ("split", workspace::SplitUp.boxed_clone()), "vs" | "vsp" | "vspl" | "vspli" | "vsplit" => { ("vsplit", workspace::SplitLeft.boxed_clone()) } + "new" => ( + "new", + workspace::NewFileInDirection(workspace::SplitDirection::Up).boxed_clone(), + ), + "vne" | "vnew" => ( + "vnew", + workspace::NewFileInDirection(workspace::SplitDirection::Left).boxed_clone(), + ), + "tabe" | "tabed" | "tabedi" | "tabedit" => ("tabedit", workspace::NewFile.boxed_clone()), + "tabnew" => ("tabnew", workspace::NewFile.boxed_clone()), + + "tabn" | "tabne" | "tabnex" | "tabnext" => { + ("tabnext", workspace::ActivateNextItem.boxed_clone()) + } + "tabp" | "tabpr" | "tabpre" | "tabprev" | "tabprevi" | "tabprevio" | "tabpreviou" + | "tabprevious" => ("tabprevious", workspace::ActivatePrevItem.boxed_clone()), + "tabN" | "tabNe" | "tabNex" | "tabNext" => { + ("tabNext", workspace::ActivatePrevItem.boxed_clone()) + } + "tabc" | "tabcl" | "tabclo" | "tabclos" | "tabclose" => ( + "tabclose", + workspace::CloseActiveItem { + save_behavior: Some(SaveBehavior::PromptOnWrite), + } + .boxed_clone(), + ), + + // quickfix / loclist (merged together for now) + "cl" | "cli" | "clis" | "clist" => ("clist", diagnostics::Deploy.boxed_clone()), + "cc" => ("cc", editor::Hover.boxed_clone()), + "ll" => ("ll", editor::Hover.boxed_clone()), "cn" | "cne" | "cnex" | "cnext" => ("cnext", editor::GoToDiagnostic.boxed_clone()), - "cp" | "cpr" | "cpre" | "cprev" => ("cprev", editor::GoToPrevDiagnostic.boxed_clone()), + "lne" | "lnex" | "lnext" => ("cnext", editor::GoToDiagnostic.boxed_clone()), + + "cpr" | "cpre" | "cprev" | "cprevi" | "cprevio" | "cpreviou" | "cprevious" => { + ("cprevious", editor::GoToPrevDiagnostic.boxed_clone()) + } + "cN" | "cNe" | "cNex" | "cNext" => ("cNext", editor::GoToPrevDiagnostic.boxed_clone()), + "lp" | "lpr" | "lpre" | "lprev" | "lprevi" | "lprevio" | "lpreviou" | "lprevious" => { + ("lprevious", editor::GoToPrevDiagnostic.boxed_clone()) + } + "lN" | "lNe" | "lNex" | "lNext" => ("lNext", editor::GoToPrevDiagnostic.boxed_clone()), + + // modify the buffer (should accept [range]) + "j" | "jo" | "joi" | "join" => ("join", JoinLines.boxed_clone()), + "d" | "de" | "del" | "dele" | "delet" | "delete" | "dl" | "dell" | "delel" | "deletl" + | "deletel" | "dp" | "dep" | "delp" | "delep" | "deletp" | "deletep" => { + ("delete", editor::DeleteLine.boxed_clone()) + } + "sor" | "sor " | "sort" | "sort " => ("sort", SortLinesCaseSensitive.boxed_clone()), + "sor i" | "sort i" => ("sort i", SortLinesCaseInsensitive.boxed_clone()), + + // goto (other ranges handled under _ => ) + "$" => ("$", EndOfDocument.boxed_clone()), _ => { - if let Ok(line) = query.parse::() { + if query.starts_with("/") || query.starts_with("?") { + ( + query, + FindCommand { + query: query[1..].to_string(), + backwards: query.starts_with("?"), + } + .boxed_clone(), + ) + } else if query.starts_with("%") { + ( + query, + ReplaceCommand { + query: query.to_string(), + } + .boxed_clone(), + ) + } else if let Ok(line) = query.parse::() { (query, GoToLine { line }.boxed_clone()) } else { return None; @@ -217,3 +280,120 @@ fn generate_positions(string: &str, query: &str) -> Vec { positions } + +#[cfg(test)] +mod test { + use std::path::Path; + + use crate::test::{NeovimBackedTestContext, VimTestContext}; + use gpui::{executor::Foreground, TestAppContext}; + use indoc::indoc; + + #[gpui::test] + async fn test_command_basics(cx: &mut TestAppContext) { + if let Foreground::Deterministic { cx_id: _, executor } = cx.foreground().as_ref() { + executor.run_until_parked(); + } + let mut cx = NeovimBackedTestContext::new(cx).await; + + cx.set_shared_state(indoc! {" + ˇa + b + c"}) + .await; + + cx.simulate_shared_keystrokes([":", "j", "enter"]).await; + + // hack: our cursor positionining after a join command is wrong + cx.simulate_shared_keystrokes(["^"]).await; + cx.assert_shared_state(indoc! { + "ˇa b + c" + }) + .await; + } + + #[gpui::test] + async fn test_command_goto(cx: &mut TestAppContext) { + let mut cx = NeovimBackedTestContext::new(cx).await; + + cx.set_shared_state(indoc! {" + ˇa + b + c"}) + .await; + cx.simulate_shared_keystrokes([":", "3", "enter"]).await; + cx.assert_shared_state(indoc! {" + a + b + ˇc"}) + .await; + } + + #[gpui::test] + async fn test_command_replace(cx: &mut TestAppContext) { + let mut cx = NeovimBackedTestContext::new(cx).await; + + cx.set_shared_state(indoc! {" + ˇa + b + c"}) + .await; + cx.simulate_shared_keystrokes([":", "%", "s", "/", "b", "/", "d", "enter"]) + .await; + cx.assert_shared_state(indoc! {" + a + ˇd + c"}) + .await; + cx.simulate_shared_keystrokes([ + ":", "%", "s", ":", ".", ":", "\\", "0", "\\", "0", "enter", + ]) + .await; + cx.assert_shared_state(indoc! {" + aa + dd + ˇcc"}) + .await; + } + + #[gpui::test] + async fn test_command_write(cx: &mut TestAppContext) { + let mut cx = VimTestContext::new(cx, true).await; + let path = Path::new("/root/dir/file.rs"); + let fs = cx.workspace(|workspace, cx| workspace.project().read(cx).fs().clone()); + + cx.simulate_keystrokes(["i", "@", "escape"]); + cx.simulate_keystrokes([":", "w", "enter"]); + + assert_eq!(fs.load(&path).await.unwrap(), "@\n"); + + fs.as_fake() + .write_file_internal(path, "oops\n".to_string()) + .unwrap(); + + // conflict! + cx.simulate_keystrokes(["i", "@", "escape"]); + cx.simulate_keystrokes([":", "w", "enter"]); + let window = cx.window; + assert!(window.has_pending_prompt(cx.cx)); + // "Cancel" + window.simulate_prompt_answer(0, cx.cx); + assert_eq!(fs.load(&path).await.unwrap(), "oops\n"); + assert!(!window.has_pending_prompt(cx.cx)); + // force overwrite + cx.simulate_keystrokes([":", "w", "!", "enter"]); + assert!(!window.has_pending_prompt(cx.cx)); + assert_eq!(fs.load(&path).await.unwrap(), "@@\n"); + } + + #[gpui::test] + async fn test_command_quit(cx: &mut TestAppContext) { + let mut cx = VimTestContext::new(cx, true).await; + + cx.simulate_keystrokes([":", "n", "e", "w", "enter"]); + cx.workspace(|workspace, cx| assert_eq!(workspace.items(cx).count(), 2)); + cx.simulate_keystrokes([":", "q", "enter"]); + cx.workspace(|workspace, cx| assert_eq!(workspace.items(cx).count(), 1)); + } +} diff --git a/crates/vim/src/normal.rs b/crates/vim/src/normal.rs index c8d12f8ee33047c2148710b79360735d8ae9c114..a23091c7a7a6433a49d715d498118a47d9e0ec99 100644 --- a/crates/vim/src/normal.rs +++ b/crates/vim/src/normal.rs @@ -4,7 +4,7 @@ mod delete; mod paste; pub(crate) mod repeat; mod scroll; -mod search; +pub(crate) mod search; pub mod substitute; mod yank; @@ -168,7 +168,12 @@ pub fn normal_object(object: Object, cx: &mut WindowContext) { }) } -fn move_cursor(vim: &mut Vim, motion: Motion, times: Option, cx: &mut WindowContext) { +pub(crate) fn move_cursor( + vim: &mut Vim, + motion: Motion, + times: Option, + cx: &mut WindowContext, +) { vim.update_active_editor(cx, |editor, cx| { editor.change_selections(Some(Autoscroll::fit()), cx, |s| { s.move_cursors_with(|map, cursor, goal| { diff --git a/crates/vim/src/normal/search.rs b/crates/vim/src/normal/search.rs index c9c04007d1a4c0237047bb24f686668d40002290..a9e5e66b9e1bef7d31a764067bf9e37e13823756 100644 --- a/crates/vim/src/normal/search.rs +++ b/crates/vim/src/normal/search.rs @@ -1,9 +1,9 @@ use gpui::{actions, impl_actions, AppContext, ViewContext}; use search::{buffer_search, BufferSearchBar, SearchMode, SearchOptions}; use serde_derive::Deserialize; -use workspace::{searchable::Direction, Pane, Workspace}; +use workspace::{searchable::Direction, Pane, Toast, Workspace}; -use crate::{state::SearchState, Vim}; +use crate::{motion::Motion, normal::move_cursor, state::SearchState, Vim}; #[derive(Clone, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] @@ -25,7 +25,29 @@ pub(crate) struct Search { backwards: bool, } -impl_actions!(vim, [MoveToNext, MoveToPrev, Search]); +#[derive(Debug, Clone, PartialEq, Deserialize)] +pub struct FindCommand { + pub query: String, + pub backwards: bool, +} + +#[derive(Debug, Clone, PartialEq, Deserialize)] +pub struct ReplaceCommand { + pub query: String, +} + +#[derive(Debug)] +struct Replacement { + search: String, + replacement: String, + should_replace_all: bool, + is_case_sensitive: bool, +} + +impl_actions!( + vim, + [MoveToNext, MoveToPrev, Search, FindCommand, ReplaceCommand] +); actions!(vim, [SearchSubmit]); pub(crate) fn init(cx: &mut AppContext) { @@ -34,6 +56,9 @@ pub(crate) fn init(cx: &mut AppContext) { cx.add_action(search); cx.add_action(search_submit); cx.add_action(search_deploy); + + cx.add_action(find_command); + cx.add_action(replace_command); } fn move_to_next(workspace: &mut Workspace, action: &MoveToNext, cx: &mut ViewContext) { @@ -65,6 +90,7 @@ fn search(workspace: &mut Workspace, action: &Search, cx: &mut ViewContext) { + let pane = workspace.active_pane().clone(); + pane.update(cx, |pane, cx| { + if let Some(search_bar) = pane.toolbar().read(cx).item_of_type::() { + let search = search_bar.update(cx, |search_bar, cx| { + if !search_bar.show(cx) { + return None; + } + let mut query = action.query.clone(); + if query == "" { + query = search_bar.query(cx); + }; + + search_bar.activate_search_mode(SearchMode::Regex, cx); + Some(search_bar.search(&query, Some(SearchOptions::CASE_SENSITIVE), cx)) + }); + let Some(search) = search else { return }; + let search_bar = search_bar.downgrade(); + cx.spawn(|_, mut cx| async move { + search.await?; + search_bar.update(&mut cx, |search_bar, cx| { + search_bar.select_match(Direction::Next, 1, cx) + })?; + anyhow::Ok(()) + }) + .detach_and_log_err(cx); + } + }) +} + +fn replace_command( + workspace: &mut Workspace, + action: &ReplaceCommand, + cx: &mut ViewContext, +) { + let replacement = match parse_replace_all(&action.query) { + Ok(replacement) => replacement, + Err(message) => { + cx.handle().update(cx, |workspace, cx| { + workspace.show_toast(Toast::new(1544, message), cx) + }); + return; + } + }; + let pane = workspace.active_pane().clone(); + pane.update(cx, |pane, cx| { + if let Some(search_bar) = pane.toolbar().read(cx).item_of_type::() { + let search = search_bar.update(cx, |search_bar, cx| { + if !search_bar.show(cx) { + return None; + } + + let mut options = SearchOptions::default(); + if replacement.is_case_sensitive { + options.set(SearchOptions::CASE_SENSITIVE, true) + } + let search = if replacement.search == "" { + search_bar.query(cx) + } else { + replacement.search + }; + + search_bar.set_replacement(Some(&replacement.replacement), cx); + search_bar.activate_search_mode(SearchMode::Regex, cx); + Some(search_bar.search(&search, Some(options), cx)) + }); + let Some(search) = search else { return }; + let search_bar = search_bar.downgrade(); + cx.spawn(|_, mut cx| async move { + search.await?; + search_bar.update(&mut cx, |search_bar, cx| { + if replacement.should_replace_all { + search_bar.select_last_match(cx); + search_bar.replace_all(&Default::default(), cx); + Vim::update(cx, |vim, cx| { + move_cursor( + vim, + Motion::StartOfLine { + display_lines: false, + }, + None, + cx, + ) + }) + } + })?; + anyhow::Ok(()) + }) + .detach_and_log_err(cx); + } + }) +} + +fn parse_replace_all(query: &str) -> Result { + let mut chars = query.chars(); + if Some('%') != chars.next() || Some('s') != chars.next() { + return Err("unsupported pattern".to_string()); + } + + let Some(delimeter) = chars.next() else { + return Err("unsupported pattern".to_string()); + }; + if delimeter == '\\' || !delimeter.is_ascii_punctuation() { + return Err(format!("cannot use {:?} as a search delimeter", delimeter)); + } + + let mut search = String::new(); + let mut replacement = String::new(); + let mut flags = String::new(); + + let mut buffer = &mut search; + + let mut escaped = false; + let mut phase = 0; + + for c in chars { + if escaped { + escaped = false; + if phase == 1 && c.is_digit(10) { + // help vim users discover zed regex syntax + // (though we don't try and fix arbitrary patterns for them) + buffer.push('$') + } else if phase == 0 && c == '(' || c == ')' { + // un-escape parens + } else if c != delimeter { + buffer.push('\\') + } + buffer.push(c) + } else if c == '\\' { + escaped = true; + } else if c == delimeter { + if phase == 0 { + buffer = &mut replacement; + phase = 1; + } else if phase == 1 { + buffer = &mut flags; + phase = 2; + } else { + return Err("trailing characters".to_string()); + } + } else { + buffer.push(c) + } + } + + let mut replacement = Replacement { + search, + replacement, + should_replace_all: true, + is_case_sensitive: true, + }; + + for c in flags.chars() { + match c { + 'g' | 'I' => {} // defaults, + 'c' | 'n' => replacement.should_replace_all = false, + 'i' => replacement.is_case_sensitive = false, + _ => return Err(format!("unsupported flag {:?}", c)), + } + } + + Ok(replacement) +} + #[cfg(test)] mod test { use std::sync::Arc; diff --git a/crates/vim/src/test/neovim_backed_binding_test_context.rs b/crates/vim/src/test/neovim_backed_binding_test_context.rs index 18de029fdc90ea0f70f1f71b19548c51e4ad6e56..15fce99aad3f4ea0e03129342a4bca48fba4166f 100644 --- a/crates/vim/src/test/neovim_backed_binding_test_context.rs +++ b/crates/vim/src/test/neovim_backed_binding_test_context.rs @@ -1,7 +1,5 @@ use std::ops::{Deref, DerefMut}; -use gpui::ContextHandle; - use crate::state::Mode; use super::{ExemptionFeatures, NeovimBackedTestContext, SUPPORTED_FEATURES}; @@ -33,26 +31,17 @@ impl<'a, const COUNT: usize> NeovimBackedBindingTestContext<'a, COUNT> { self.consume().binding(keystrokes) } - pub async fn assert( - &mut self, - marked_positions: &str, - ) -> Option<(ContextHandle, ContextHandle)> { + pub async fn assert(&mut self, marked_positions: &str) { self.cx .assert_binding_matches(self.keystrokes_under_test, marked_positions) - .await + .await; } - pub async fn assert_exempted( - &mut self, - marked_positions: &str, - feature: ExemptionFeatures, - ) -> Option<(ContextHandle, ContextHandle)> { + pub async fn assert_exempted(&mut self, marked_positions: &str, feature: ExemptionFeatures) { if SUPPORTED_FEATURES.contains(&feature) { self.cx .assert_binding_matches(self.keystrokes_under_test, marked_positions) .await - } else { - None } } diff --git a/crates/vim/src/test/neovim_backed_test_context.rs b/crates/vim/src/test/neovim_backed_test_context.rs index e58f805a026f32473ab18b4dccc9eb662ae6e541..227d39bb6354c4c2dd3a8c5ce38dc75c567e513e 100644 --- a/crates/vim/src/test/neovim_backed_test_context.rs +++ b/crates/vim/src/test/neovim_backed_test_context.rs @@ -106,26 +106,25 @@ impl<'a> NeovimBackedTestContext<'a> { pub async fn simulate_shared_keystrokes( &mut self, keystroke_texts: [&str; COUNT], - ) -> ContextHandle { + ) { for keystroke_text in keystroke_texts.into_iter() { self.recent_keystrokes.push(keystroke_text.to_string()); self.neovim.send_keystroke(keystroke_text).await; } - self.simulate_keystrokes(keystroke_texts) + self.simulate_keystrokes(keystroke_texts); } - pub async fn set_shared_state(&mut self, marked_text: &str) -> ContextHandle { + pub async fn set_shared_state(&mut self, marked_text: &str) { let mode = if marked_text.contains("»") { Mode::Visual } else { Mode::Normal }; - let context_handle = self.set_state(marked_text, mode); + self.set_state(marked_text, mode); self.last_set_state = Some(marked_text.to_string()); self.recent_keystrokes = Vec::new(); self.neovim.set_state(marked_text).await; self.is_dirty = true; - context_handle } pub async fn set_shared_wrap(&mut self, columns: u32) { @@ -288,18 +287,18 @@ impl<'a> NeovimBackedTestContext<'a> { &mut self, keystrokes: [&str; COUNT], initial_state: &str, - ) -> Option<(ContextHandle, ContextHandle)> { + ) { if let Some(possible_exempted_keystrokes) = self.exemptions.get(initial_state) { match possible_exempted_keystrokes { Some(exempted_keystrokes) => { if exempted_keystrokes.contains(&format!("{keystrokes:?}")) { // This keystroke was exempted for this insertion text - return None; + return; } } None => { // All keystrokes for this insertion text are exempted - return None; + return; } } } @@ -307,7 +306,6 @@ impl<'a> NeovimBackedTestContext<'a> { let _state_context = self.set_shared_state(initial_state).await; let _keystroke_context = self.simulate_shared_keystrokes(keystrokes).await; self.assert_state_matches().await; - Some((_state_context, _keystroke_context)) } pub async fn assert_binding_matches_all( diff --git a/crates/vim/test_data/test_command_basics.json b/crates/vim/test_data/test_command_basics.json new file mode 100644 index 0000000000000000000000000000000000000000..669d34409f218f6205f69a0d13c4dc9bf75bd5b8 --- /dev/null +++ b/crates/vim/test_data/test_command_basics.json @@ -0,0 +1,6 @@ +{"Put":{"state":"ˇa\nb\nc"}} +{"Key":":"} +{"Key":"j"} +{"Key":"enter"} +{"Key":"^"} +{"Get":{"state":"ˇa b\nc","mode":"Normal"}} diff --git a/crates/vim/test_data/test_command_goto.json b/crates/vim/test_data/test_command_goto.json new file mode 100644 index 0000000000000000000000000000000000000000..2f7ed10eeb27017ca3fbdaa94f6a3f8f2d2ce316 --- /dev/null +++ b/crates/vim/test_data/test_command_goto.json @@ -0,0 +1,5 @@ +{"Put":{"state":"ˇa\nb\nc"}} +{"Key":":"} +{"Key":"3"} +{"Key":"enter"} +{"Get":{"state":"a\nb\nˇc","mode":"Normal"}} diff --git a/crates/vim/test_data/test_command_replace.json b/crates/vim/test_data/test_command_replace.json new file mode 100644 index 0000000000000000000000000000000000000000..13928d5c7e138af7628e613f612a199a760d0798 --- /dev/null +++ b/crates/vim/test_data/test_command_replace.json @@ -0,0 +1,22 @@ +{"Put":{"state":"ˇa\nb\nc"}} +{"Key":":"} +{"Key":"%"} +{"Key":"s"} +{"Key":"/"} +{"Key":"b"} +{"Key":"/"} +{"Key":"d"} +{"Key":"enter"} +{"Get":{"state":"a\nˇd\nc","mode":"Normal"}} +{"Key":":"} +{"Key":"%"} +{"Key":"s"} +{"Key":":"} +{"Key":"."} +{"Key":":"} +{"Key":"\\"} +{"Key":"0"} +{"Key":"\\"} +{"Key":"0"} +{"Key":"enter"} +{"Get":{"state":"aa\ndd\nˇcc","mode":"Normal"}} diff --git a/crates/zed/src/menus.rs b/crates/zed/src/menus.rs index da3f7e4c32e1fbc535dcb071e9a2fa55df1b4ae6..acffbc29abbe1bf4486548e859a397bc7fb3a62d 100644 --- a/crates/zed/src/menus.rs +++ b/crates/zed/src/menus.rs @@ -57,12 +57,7 @@ pub fn menus() -> Vec> { save_behavior: None, }, ), - MenuItem::action( - "Close Window", - workspace::CloseWindow { - save_behavior: None, - }, - ), + MenuItem::action("Close Window", workspace::CloseWindow), ], }, Menu { diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index d968a92646c1d3854193e28f196d7f5597268bc8..72804cb523ddb8d335339dda7c07622b4d3cf32e 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -947,7 +947,9 @@ mod tests { assert!(editor.text(cx).is_empty()); }); - let save_task = workspace.update(cx, |workspace, cx| workspace.save_active_item(false, cx)); + let save_task = workspace.update(cx, |workspace, cx| { + workspace.save_active_item(SaveBehavior::PromptOnConflict, cx) + }); app_state.fs.create_dir(Path::new("/root")).await.unwrap(); cx.simulate_new_path_selection(|_| Some(PathBuf::from("/root/the-new-name"))); save_task.await.unwrap(); @@ -1311,7 +1313,9 @@ mod tests { .await; cx.read(|cx| assert!(editor.is_dirty(cx))); - let save_task = workspace.update(cx, |workspace, cx| workspace.save_active_item(false, cx)); + let save_task = workspace.update(cx, |workspace, cx| { + workspace.save_active_item(SaveBehavior::PromptOnConflict, cx) + }); window.simulate_prompt_answer(0, cx); save_task.await.unwrap(); editor.read_with(cx, |editor, cx| { @@ -1353,7 +1357,9 @@ mod tests { }); // Save the buffer. This prompts for a filename. - let save_task = workspace.update(cx, |workspace, cx| workspace.save_active_item(false, cx)); + let save_task = workspace.update(cx, |workspace, cx| { + workspace.save_active_item(SaveBehavior::PromptOnConflict, cx) + }); cx.simulate_new_path_selection(|parent_dir| { assert_eq!(parent_dir, Path::new("/root")); Some(parent_dir.join("the-new-name.rs")) @@ -1377,7 +1383,9 @@ mod tests { editor.handle_input(" there", cx); assert!(editor.is_dirty(cx)); }); - let save_task = workspace.update(cx, |workspace, cx| workspace.save_active_item(false, cx)); + let save_task = workspace.update(cx, |workspace, cx| { + workspace.save_active_item(SaveBehavior::PromptOnConflict, cx) + }); save_task.await.unwrap(); assert!(!cx.did_prompt_for_new_path()); editor.read_with(cx, |editor, cx| { @@ -1444,7 +1452,9 @@ mod tests { }); // Save the buffer. This prompts for a filename. - let save_task = workspace.update(cx, |workspace, cx| workspace.save_active_item(false, cx)); + let save_task = workspace.update(cx, |workspace, cx| { + workspace.save_active_item(SaveBehavior::PromptOnConflict, cx) + }); cx.simulate_new_path_selection(|_| Some(PathBuf::from("/root/the-new-name.rs"))); save_task.await.unwrap(); // The buffer is not dirty anymore and the language is assigned based on the path. From a25fcfdfa7116ad21d3fdc38f0be0297aed5119d Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Wed, 20 Sep 2023 16:30:45 -0600 Subject: [PATCH 08/14] Iron out some edge-cases --- Cargo.toml | 1 + crates/search/src/buffer_search.rs | 3 ++ crates/vim/src/command.rs | 3 ++ crates/vim/src/normal/search.rs | 31 ++++++------------- .../vim/test_data/test_command_replace.json | 7 +++++ 5 files changed, 24 insertions(+), 21 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c1876434ad46fc7f8c5eddf59d46255fcde4136e..3e4c5911ed644014bc70aef383d357b5552c67d6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -158,6 +158,7 @@ core-graphics = { git = "https://github.com/servo/core-foundation-rs", rev = "07 [profile.dev] split-debuginfo = "unpacked" +panic = "abort" [profile.release] debug = true diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index 97f9d7a7c8f44f2d78e3b362d5f2878c3002a492..11c35a6d75cd50cada606dbc52617d7441c56d0e 100644 --- a/crates/search/src/buffer_search.rs +++ b/crates/search/src/buffer_search.rs @@ -702,6 +702,9 @@ impl BufferSearchBar { .searchable_items_with_matches .get(&searchable_item.downgrade()) { + if matches.len() == 0 { + return; + } let new_match_index = matches.len() - 1; searchable_item.update_matches(matches, cx); searchable_item.activate_match(new_match_index, matches, cx); diff --git a/crates/vim/src/command.rs b/crates/vim/src/command.rs index 10c2599dd9af76d98792898e33a97af1695bf4ac..7aaf3d8024acea6e5826fabaf08ae9a955105fca 100644 --- a/crates/vim/src/command.rs +++ b/crates/vim/src/command.rs @@ -355,6 +355,9 @@ mod test { dd ˇcc"}) .await; + + cx.simulate_shared_keystrokes([":", "%", "s", "/", "/", "/", "enter"]) + .await; } #[gpui::test] diff --git a/crates/vim/src/normal/search.rs b/crates/vim/src/normal/search.rs index a9e5e66b9e1bef7d31a764067bf9e37e13823756..8ef0718c326dc660fb064e492ca35d1faadaa062 100644 --- a/crates/vim/src/normal/search.rs +++ b/crates/vim/src/normal/search.rs @@ -1,7 +1,7 @@ use gpui::{actions, impl_actions, AppContext, ViewContext}; use search::{buffer_search, BufferSearchBar, SearchMode, SearchOptions}; use serde_derive::Deserialize; -use workspace::{searchable::Direction, Pane, Toast, Workspace}; +use workspace::{searchable::Direction, Pane, Workspace}; use crate::{motion::Motion, normal::move_cursor, state::SearchState, Vim}; @@ -36,7 +36,7 @@ pub struct ReplaceCommand { pub query: String, } -#[derive(Debug)] +#[derive(Debug, Default)] struct Replacement { search: String, replacement: String, @@ -212,15 +212,7 @@ fn replace_command( action: &ReplaceCommand, cx: &mut ViewContext, ) { - let replacement = match parse_replace_all(&action.query) { - Ok(replacement) => replacement, - Err(message) => { - cx.handle().update(cx, |workspace, cx| { - workspace.show_toast(Toast::new(1544, message), cx) - }); - return; - } - }; + let replacement = parse_replace_all(&action.query); let pane = workspace.active_pane().clone(); pane.update(cx, |pane, cx| { if let Some(search_bar) = pane.toolbar().read(cx).item_of_type::() { @@ -270,18 +262,15 @@ fn replace_command( }) } -fn parse_replace_all(query: &str) -> Result { +fn parse_replace_all(query: &str) -> Replacement { let mut chars = query.chars(); if Some('%') != chars.next() || Some('s') != chars.next() { - return Err("unsupported pattern".to_string()); + return Replacement::default(); } let Some(delimeter) = chars.next() else { - return Err("unsupported pattern".to_string()); + return Replacement::default(); }; - if delimeter == '\\' || !delimeter.is_ascii_punctuation() { - return Err(format!("cannot use {:?} as a search delimeter", delimeter)); - } let mut search = String::new(); let mut replacement = String::new(); @@ -315,7 +304,7 @@ fn parse_replace_all(query: &str) -> Result { buffer = &mut flags; phase = 2; } else { - return Err("trailing characters".to_string()); + break; } } else { buffer.push(c) @@ -331,14 +320,14 @@ fn parse_replace_all(query: &str) -> Result { for c in flags.chars() { match c { - 'g' | 'I' => {} // defaults, + 'g' | 'I' => {} 'c' | 'n' => replacement.should_replace_all = false, 'i' => replacement.is_case_sensitive = false, - _ => return Err(format!("unsupported flag {:?}", c)), + _ => {} } } - Ok(replacement) + replacement } #[cfg(test)] diff --git a/crates/vim/test_data/test_command_replace.json b/crates/vim/test_data/test_command_replace.json index 13928d5c7e138af7628e613f612a199a760d0798..91827c0285b3b74d3d7366047d408cba36879228 100644 --- a/crates/vim/test_data/test_command_replace.json +++ b/crates/vim/test_data/test_command_replace.json @@ -20,3 +20,10 @@ {"Key":"0"} {"Key":"enter"} {"Get":{"state":"aa\ndd\nˇcc","mode":"Normal"}} +{"Key":":"} +{"Key":"%"} +{"Key":"s"} +{"Key":"/"} +{"Key":"/"} +{"Key":"/"} +{"Key":"enter"} From a59da3634bb3b84eb4a1d5a9ebeb1afe0f0a89ae Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Wed, 20 Sep 2023 16:41:40 -0600 Subject: [PATCH 09/14] Fix backward search from command --- crates/vim/src/command.rs | 27 ++++++++++++++++++- crates/vim/src/normal/search.rs | 7 ++++- crates/vim/test_data/test_command_search.json | 11 ++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 crates/vim/test_data/test_command_search.json diff --git a/crates/vim/src/command.rs b/crates/vim/src/command.rs index 7aaf3d8024acea6e5826fabaf08ae9a955105fca..2ad8f0522eb84ac1a94348d4397f99259828e4d6 100644 --- a/crates/vim/src/command.rs +++ b/crates/vim/src/command.rs @@ -355,8 +355,33 @@ mod test { dd ˇcc"}) .await; + } + + #[gpui::test] + async fn test_command_search(cx: &mut TestAppContext) { + let mut cx = NeovimBackedTestContext::new(cx).await; - cx.simulate_shared_keystrokes([":", "%", "s", "/", "/", "/", "enter"]) + cx.set_shared_state(indoc! {" + ˇa + b + a + c"}) + .await; + cx.simulate_shared_keystrokes([":", "/", "b", "enter"]) + .await; + cx.assert_shared_state(indoc! {" + a + ˇb + a + c"}) + .await; + cx.simulate_shared_keystrokes([":", "?", "a", "enter"]) + .await; + cx.assert_shared_state(indoc! {" + ˇa + b + a + c"}) .await; } diff --git a/crates/vim/src/normal/search.rs b/crates/vim/src/normal/search.rs index 8ef0718c326dc660fb064e492ca35d1faadaa062..e76da1dfc5dd7dc1d7803c6fe0127a85fb089238 100644 --- a/crates/vim/src/normal/search.rs +++ b/crates/vim/src/normal/search.rs @@ -195,10 +195,15 @@ fn find_command(workspace: &mut Workspace, action: &FindCommand, cx: &mut ViewCo }); let Some(search) = search else { return }; let search_bar = search_bar.downgrade(); + let direction = if action.backwards { + Direction::Prev + } else { + Direction::Next + }; cx.spawn(|_, mut cx| async move { search.await?; search_bar.update(&mut cx, |search_bar, cx| { - search_bar.select_match(Direction::Next, 1, cx) + search_bar.select_match(direction, 1, cx) })?; anyhow::Ok(()) }) diff --git a/crates/vim/test_data/test_command_search.json b/crates/vim/test_data/test_command_search.json new file mode 100644 index 0000000000000000000000000000000000000000..705ce51fb75f19dfa831bb97c6df22b6c1e20e94 --- /dev/null +++ b/crates/vim/test_data/test_command_search.json @@ -0,0 +1,11 @@ +{"Put":{"state":"ˇa\nb\na\nc"}} +{"Key":":"} +{"Key":"/"} +{"Key":"b"} +{"Key":"enter"} +{"Get":{"state":"a\nˇb\na\nc","mode":"Normal"}} +{"Key":":"} +{"Key":"?"} +{"Key":"a"} +{"Key":"enter"} +{"Get":{"state":"ˇa\nb\na\nc","mode":"Normal"}} From 7a7ff4bb960faa05b5fadbd2cd9baf494685a949 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Wed, 20 Sep 2023 20:13:52 -0600 Subject: [PATCH 10/14] Fix save related tests, and refactor saves again --- crates/vim/src/command.rs | 43 ++++--- crates/workspace/src/item.rs | 6 +- crates/workspace/src/pane.rs | 195 +++++++++++++++--------------- crates/workspace/src/workspace.rs | 47 +++---- crates/zed/src/zed.rs | 28 +++-- 5 files changed, 156 insertions(+), 163 deletions(-) diff --git a/crates/vim/src/command.rs b/crates/vim/src/command.rs index 2ad8f0522eb84ac1a94348d4397f99259828e4d6..6bed7f5bb4b995df69bfb055b9b7a6acc64847ca 100644 --- a/crates/vim/src/command.rs +++ b/crates/vim/src/command.rs @@ -2,7 +2,7 @@ use command_palette::CommandInterceptResult; use editor::{SortLinesCaseInsensitive, SortLinesCaseSensitive}; use gpui::{impl_actions, Action, AppContext}; use serde_derive::Deserialize; -use workspace::{SaveBehavior, Workspace}; +use workspace::{SaveIntent, Workspace}; use crate::{ motion::{EndOfDocument, Motion}, @@ -50,112 +50,119 @@ pub fn command_interceptor(mut query: &str, _: &AppContext) -> Option ( "write", workspace::Save { - save_behavior: Some(SaveBehavior::PromptOnConflict), + save_behavior: Some(SaveIntent::Save), } .boxed_clone(), ), "w!" | "wr!" | "wri!" | "writ!" | "write!" => ( "write!", workspace::Save { - save_behavior: Some(SaveBehavior::SilentlyOverwrite), + save_behavior: Some(SaveIntent::Overwrite), } .boxed_clone(), ), "q" | "qu" | "qui" | "quit" => ( "quit", workspace::CloseActiveItem { - save_behavior: Some(SaveBehavior::PromptOnWrite), + save_behavior: Some(SaveIntent::Close), } .boxed_clone(), ), "q!" | "qu!" | "qui!" | "quit!" => ( "quit!", workspace::CloseActiveItem { - save_behavior: Some(SaveBehavior::DontSave), + save_behavior: Some(SaveIntent::Skip), } .boxed_clone(), ), "wq" => ( "wq", workspace::CloseActiveItem { - save_behavior: Some(SaveBehavior::PromptOnConflict), + save_behavior: Some(SaveIntent::Save), } .boxed_clone(), ), "wq!" => ( "wq!", workspace::CloseActiveItem { - save_behavior: Some(SaveBehavior::SilentlyOverwrite), + save_behavior: Some(SaveIntent::Overwrite), } .boxed_clone(), ), "x" | "xi" | "xit" | "exi" | "exit" => ( "exit", workspace::CloseActiveItem { - save_behavior: Some(SaveBehavior::PromptOnConflict), + save_behavior: Some(SaveIntent::Save), } .boxed_clone(), ), "x!" | "xi!" | "xit!" | "exi!" | "exit!" => ( "exit!", workspace::CloseActiveItem { - save_behavior: Some(SaveBehavior::SilentlyOverwrite), + save_behavior: Some(SaveIntent::Overwrite), + } + .boxed_clone(), + ), + "up" | "upd" | "upda" | "updat" | "update" => ( + "update", + workspace::Save { + save_behavior: Some(SaveIntent::SaveAll), } .boxed_clone(), ), "wa" | "wal" | "wall" => ( "wall", workspace::SaveAll { - save_behavior: Some(SaveBehavior::PromptOnConflict), + save_behavior: Some(SaveIntent::SaveAll), } .boxed_clone(), ), "wa!" | "wal!" | "wall!" => ( "wall!", workspace::SaveAll { - save_behavior: Some(SaveBehavior::SilentlyOverwrite), + save_behavior: Some(SaveIntent::Overwrite), } .boxed_clone(), ), "qa" | "qal" | "qall" | "quita" | "quital" | "quitall" => ( "quitall", workspace::CloseAllItemsAndPanes { - save_behavior: Some(SaveBehavior::PromptOnWrite), + save_behavior: Some(SaveIntent::Close), } .boxed_clone(), ), "qa!" | "qal!" | "qall!" | "quita!" | "quital!" | "quitall!" => ( "quitall!", workspace::CloseAllItemsAndPanes { - save_behavior: Some(SaveBehavior::DontSave), + save_behavior: Some(SaveIntent::Skip), } .boxed_clone(), ), "xa" | "xal" | "xall" => ( "xall", workspace::CloseAllItemsAndPanes { - save_behavior: Some(SaveBehavior::PromptOnConflict), + save_behavior: Some(SaveIntent::SaveAll), } .boxed_clone(), ), "xa!" | "xal!" | "xall!" => ( "xall!", workspace::CloseAllItemsAndPanes { - save_behavior: Some(SaveBehavior::SilentlyOverwrite), + save_behavior: Some(SaveIntent::Overwrite), } .boxed_clone(), ), "wqa" | "wqal" | "wqall" => ( "wqall", workspace::CloseAllItemsAndPanes { - save_behavior: Some(SaveBehavior::PromptOnConflict), + save_behavior: Some(SaveIntent::SaveAll), } .boxed_clone(), ), "wqa!" | "wqal!" | "wqall!" => ( "wqall!", workspace::CloseAllItemsAndPanes { - save_behavior: Some(SaveBehavior::SilentlyOverwrite), + save_behavior: Some(SaveIntent::Overwrite), } .boxed_clone(), ), @@ -190,7 +197,7 @@ pub fn command_interceptor(mut query: &str, _: &AppContext) -> Option ( "tabclose", workspace::CloseActiveItem { - save_behavior: Some(SaveBehavior::PromptOnWrite), + save_behavior: Some(SaveIntent::Close), } .boxed_clone(), ), diff --git a/crates/workspace/src/item.rs b/crates/workspace/src/item.rs index ea747b3a364720c653f91ce7b8bc609750509fe3..24bed4c8d1427f0497ca0151a16b14133abbb514 100644 --- a/crates/workspace/src/item.rs +++ b/crates/workspace/src/item.rs @@ -475,11 +475,7 @@ impl ItemHandle for ViewHandle { match item_event { ItemEvent::CloseItem => { pane.update(cx, |pane, cx| { - pane.close_item_by_id( - item.id(), - crate::SaveBehavior::PromptOnWrite, - cx, - ) + pane.close_item_by_id(item.id(), crate::SaveIntent::Close, cx) }) .detach_and_log_err(cx); return; diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 285cf1da603c64977c6942131105c0fe2462b263..40e6d36f3bea4419d69be1b6e7dd31b6435cabcd 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -45,17 +45,21 @@ use theme::{Theme, ThemeSettings}; #[derive(PartialEq, Clone, Copy, Deserialize, Debug)] #[serde(rename_all = "camelCase")] -pub enum SaveBehavior { - /// ask before overwriting conflicting files (used by default with cmd-s) - PromptOnConflict, - /// ask for a new path before writing (used with cmd-shift-s) - PromptForNewPath, - /// ask before writing any file that wouldn't be auto-saved (used by default with cmd-w) - PromptOnWrite, - /// never prompt, write on conflict (used with vim's :w!) - SilentlyOverwrite, - /// skip all save-related behaviour (used with vim's :q!) - DontSave, +pub enum SaveIntent { + /// write all files (even if unchanged) + /// prompt before overwriting on-disk changes + Save, + /// write any files that have local changes + /// prompt before overwriting on-disk changes + SaveAll, + /// always prompt for a new path + SaveAs, + /// prompt "you have unsaved changes" before writing + Close, + /// write all dirty files, don't prompt on conflict + Overwrite, + /// skip all save-related behavior + Skip, } #[derive(Clone, Deserialize, PartialEq)] @@ -82,13 +86,13 @@ pub struct CloseItemsToTheRightById { #[derive(Clone, PartialEq, Debug, Deserialize, Default)] #[serde(rename_all = "camelCase")] pub struct CloseActiveItem { - pub save_behavior: Option, + pub save_behavior: Option, } #[derive(Clone, PartialEq, Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct CloseAllItems { - pub save_behavior: Option, + pub save_behavior: Option, } actions!( @@ -730,7 +734,7 @@ impl Pane { let active_item_id = self.items[self.active_item_index].id(); Some(self.close_item_by_id( active_item_id, - action.save_behavior.unwrap_or(SaveBehavior::PromptOnWrite), + action.save_behavior.unwrap_or(SaveIntent::Close), cx, )) } @@ -738,7 +742,7 @@ impl Pane { pub fn close_item_by_id( &mut self, item_id_to_close: usize, - save_behavior: SaveBehavior, + save_behavior: SaveIntent, cx: &mut ViewContext, ) -> Task> { self.close_items(cx, save_behavior, move |view_id| { @@ -756,11 +760,9 @@ impl Pane { } let active_item_id = self.items[self.active_item_index].id(); - Some( - self.close_items(cx, SaveBehavior::PromptOnWrite, move |item_id| { - item_id != active_item_id - }), - ) + Some(self.close_items(cx, SaveIntent::Close, move |item_id| { + item_id != active_item_id + })) } pub fn close_clean_items( @@ -773,11 +775,9 @@ impl Pane { .filter(|item| !item.is_dirty(cx)) .map(|item| item.id()) .collect(); - Some( - self.close_items(cx, SaveBehavior::PromptOnWrite, move |item_id| { - item_ids.contains(&item_id) - }), - ) + Some(self.close_items(cx, SaveIntent::Close, move |item_id| { + item_ids.contains(&item_id) + })) } pub fn close_items_to_the_left( @@ -802,7 +802,7 @@ impl Pane { .take_while(|item| item.id() != item_id) .map(|item| item.id()) .collect(); - self.close_items(cx, SaveBehavior::PromptOnWrite, move |item_id| { + self.close_items(cx, SaveIntent::Close, move |item_id| { item_ids.contains(&item_id) }) } @@ -830,7 +830,7 @@ impl Pane { .take_while(|item| item.id() != item_id) .map(|item| item.id()) .collect(); - self.close_items(cx, SaveBehavior::PromptOnWrite, move |item_id| { + self.close_items(cx, SaveIntent::Close, move |item_id| { item_ids.contains(&item_id) }) } @@ -846,7 +846,7 @@ impl Pane { Some(self.close_items( cx, - action.save_behavior.unwrap_or(SaveBehavior::PromptOnWrite), + action.save_behavior.unwrap_or(SaveIntent::Close), |_| true, )) } @@ -854,7 +854,7 @@ impl Pane { pub fn close_items( &mut self, cx: &mut ViewContext, - save_behavior: SaveBehavior, + save_behavior: SaveIntent, should_close: impl 'static + Fn(usize) -> bool, ) -> Task> { // Find the items to close. @@ -1010,18 +1010,18 @@ impl Pane { pane: &WeakViewHandle, item_ix: usize, item: &dyn ItemHandle, - save_behavior: SaveBehavior, + save_behavior: SaveIntent, cx: &mut AsyncAppContext, ) -> Result { const CONFLICT_MESSAGE: &str = "This file has changed on disk since you started editing it. Do you want to overwrite it?"; const DIRTY_MESSAGE: &str = "This file contains unsaved edits. Do you want to save it?"; - if save_behavior == SaveBehavior::DontSave { + if save_behavior == SaveIntent::Skip { return Ok(true); } - let (mut has_conflict, mut is_dirty, mut can_save, is_singleton) = cx.read(|cx| { + let (mut has_conflict, mut is_dirty, mut can_save, can_save_as) = cx.read(|cx| { ( item.has_conflict(cx), item.is_dirty(cx), @@ -1030,73 +1030,76 @@ impl Pane { ) }); - if save_behavior == SaveBehavior::PromptForNewPath { - has_conflict = false; + // when saving a single buffer, we ignore whether or not it's dirty. + if save_behavior == SaveIntent::Save { is_dirty = true; + } + + if save_behavior == SaveIntent::SaveAs { + is_dirty = true; + has_conflict = false; can_save = false; } + if save_behavior == SaveIntent::Overwrite { + has_conflict = false; + } + if has_conflict && can_save { - if save_behavior == SaveBehavior::SilentlyOverwrite { - pane.update(cx, |_, cx| item.save(project, cx))?.await?; - } else { - let mut answer = pane.update(cx, |pane, cx| { - pane.activate_item(item_ix, true, true, cx); - cx.prompt( - PromptLevel::Warning, - CONFLICT_MESSAGE, - &["Overwrite", "Discard", "Cancel"], - ) - })?; - match answer.next().await { - Some(0) => pane.update(cx, |_, cx| item.save(project, cx))?.await?, - Some(1) => pane.update(cx, |_, cx| item.reload(project, cx))?.await?, - _ => return Ok(false), - } + let mut answer = pane.update(cx, |pane, cx| { + pane.activate_item(item_ix, true, true, cx); + cx.prompt( + PromptLevel::Warning, + CONFLICT_MESSAGE, + &["Overwrite", "Discard", "Cancel"], + ) + })?; + match answer.next().await { + Some(0) => pane.update(cx, |_, cx| item.save(project, cx))?.await?, + Some(1) => pane.update(cx, |_, cx| item.reload(project, cx))?.await?, + _ => return Ok(false), } - } else if is_dirty && (can_save || is_singleton) { - let will_autosave = cx.read(|cx| { - matches!( - settings::get::(cx).autosave, - AutosaveSetting::OnFocusChange | AutosaveSetting::OnWindowChange - ) && Self::can_autosave_item(&*item, cx) - }); - let should_save = if save_behavior == SaveBehavior::PromptOnWrite && !will_autosave { - let mut answer = pane.update(cx, |pane, cx| { - pane.activate_item(item_ix, true, true, cx); - cx.prompt( - PromptLevel::Warning, - DIRTY_MESSAGE, - &["Save", "Don't Save", "Cancel"], - ) - })?; - match answer.next().await { - Some(0) => true, - Some(1) => false, - _ => return Ok(false), + } else if is_dirty && (can_save || can_save_as) { + if save_behavior == SaveIntent::Close { + let will_autosave = cx.read(|cx| { + matches!( + settings::get::(cx).autosave, + AutosaveSetting::OnFocusChange | AutosaveSetting::OnWindowChange + ) && Self::can_autosave_item(&*item, cx) + }); + if !will_autosave { + let mut answer = pane.update(cx, |pane, cx| { + pane.activate_item(item_ix, true, true, cx); + cx.prompt( + PromptLevel::Warning, + DIRTY_MESSAGE, + &["Save", "Don't Save", "Cancel"], + ) + })?; + match answer.next().await { + Some(0) => {} + Some(1) => return Ok(true), // Don't save this file + _ => return Ok(false), // Cancel + } } - } else { - true - }; + } - if should_save { - if can_save { - pane.update(cx, |_, cx| item.save(project, cx))?.await?; - } else if is_singleton { - let start_abs_path = project - .read_with(cx, |project, cx| { - let worktree = project.visible_worktrees(cx).next()?; - Some(worktree.read(cx).as_local()?.abs_path().to_path_buf()) - }) - .unwrap_or_else(|| Path::new("").into()); + if can_save { + pane.update(cx, |_, cx| item.save(project, cx))?.await?; + } else if can_save_as { + let start_abs_path = project + .read_with(cx, |project, cx| { + let worktree = project.visible_worktrees(cx).next()?; + Some(worktree.read(cx).as_local()?.abs_path().to_path_buf()) + }) + .unwrap_or_else(|| Path::new("").into()); - let mut abs_path = cx.update(|cx| cx.prompt_for_new_path(&start_abs_path)); - if let Some(abs_path) = abs_path.next().await.flatten() { - pane.update(cx, |_, cx| item.save_as(project, abs_path, cx))? - .await?; - } else { - return Ok(false); - } + let mut abs_path = cx.update(|cx| cx.prompt_for_new_path(&start_abs_path)); + if let Some(abs_path) = abs_path.next().await.flatten() { + pane.update(cx, |_, cx| item.save_as(project, abs_path, cx))? + .await?; + } else { + return Ok(false); } } } @@ -1210,7 +1213,7 @@ impl Pane { pane.update(cx, |pane, cx| { pane.close_item_by_id( target_item_id, - SaveBehavior::PromptOnWrite, + SaveIntent::Close, cx, ) .detach_and_log_err(cx); @@ -1367,12 +1370,8 @@ impl Pane { .on_click(MouseButton::Middle, { let item_id = item.id(); move |_, pane, cx| { - pane.close_item_by_id( - item_id, - SaveBehavior::PromptOnWrite, - cx, - ) - .detach_and_log_err(cx); + pane.close_item_by_id(item_id, SaveIntent::Close, cx) + .detach_and_log_err(cx); } }) .on_down( @@ -1580,7 +1579,7 @@ impl Pane { cx.window_context().defer(move |cx| { if let Some(pane) = pane.upgrade(cx) { pane.update(cx, |pane, cx| { - pane.close_item_by_id(item_id, SaveBehavior::PromptOnWrite, cx) + pane.close_item_by_id(item_id, SaveIntent::Close, cx) .detach_and_log_err(cx); }); } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 6043b946214257642082194043395a75d7417138..a853691b7644028e570e7b7b78176d71d2f1aca2 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -163,19 +163,19 @@ pub struct NewFileInDirection(pub SplitDirection); #[derive(Clone, PartialEq, Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct SaveAll { - pub save_behavior: Option, + pub save_behavior: Option, } #[derive(Clone, PartialEq, Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct Save { - pub save_behavior: Option, + pub save_behavior: Option, } #[derive(Clone, PartialEq, Debug, Deserialize, Default)] #[serde(rename_all = "camelCase")] pub struct CloseAllItemsAndPanes { - pub save_behavior: Option, + pub save_behavior: Option, } #[derive(Deserialize)] @@ -294,19 +294,14 @@ pub fn init(app_state: Arc, cx: &mut AppContext) { cx.add_action( |workspace: &mut Workspace, action: &Save, cx: &mut ViewContext| { workspace - .save_active_item( - action - .save_behavior - .unwrap_or(SaveBehavior::PromptOnConflict), - cx, - ) + .save_active_item(action.save_behavior.unwrap_or(SaveIntent::Save), cx) .detach_and_log_err(cx); }, ); cx.add_action( |workspace: &mut Workspace, _: &SaveAs, cx: &mut ViewContext| { workspace - .save_active_item(SaveBehavior::PromptForNewPath, cx) + .save_active_item(SaveIntent::SaveAs, cx) .detach_and_log_err(cx); }, ); @@ -1356,7 +1351,7 @@ impl Workspace { Ok(this .update(&mut cx, |this, cx| { - this.save_all_internal(SaveBehavior::PromptOnWrite, cx) + this.save_all_internal(SaveIntent::Close, cx) })? .await?) }) @@ -1367,12 +1362,8 @@ impl Workspace { action: &SaveAll, cx: &mut ViewContext, ) -> Option>> { - let save_all = self.save_all_internal( - action - .save_behavior - .unwrap_or(SaveBehavior::PromptOnConflict), - cx, - ); + let save_all = + self.save_all_internal(action.save_behavior.unwrap_or(SaveIntent::SaveAll), cx); Some(cx.foreground().spawn(async move { save_all.await?; Ok(()) @@ -1381,7 +1372,7 @@ impl Workspace { fn save_all_internal( &mut self, - save_behaviour: SaveBehavior, + save_behaviour: SaveIntent, cx: &mut ViewContext, ) -> Task> { if self.project.read(cx).is_read_only() { @@ -1688,7 +1679,7 @@ impl Workspace { pub fn save_active_item( &mut self, - save_behavior: SaveBehavior, + save_behavior: SaveIntent, cx: &mut ViewContext, ) -> Task> { let project = self.project.clone(); @@ -1720,7 +1711,7 @@ impl Workspace { _: &CloseInactiveTabsAndPanes, cx: &mut ViewContext, ) -> Option>> { - self.close_all_internal(true, SaveBehavior::PromptOnWrite, cx) + self.close_all_internal(true, SaveIntent::Close, cx) } pub fn close_all_items_and_panes( @@ -1728,17 +1719,13 @@ impl Workspace { action: &CloseAllItemsAndPanes, cx: &mut ViewContext, ) -> Option>> { - self.close_all_internal( - false, - action.save_behavior.unwrap_or(SaveBehavior::PromptOnWrite), - cx, - ) + self.close_all_internal(false, action.save_behavior.unwrap_or(SaveIntent::Close), cx) } fn close_all_internal( &mut self, retain_active_pane: bool, - save_behavior: SaveBehavior, + save_behavior: SaveIntent, cx: &mut ViewContext, ) -> Option>> { let current_pane = self.active_pane(); @@ -4433,7 +4420,7 @@ mod tests { let item1_id = item1.id(); let item3_id = item3.id(); let item4_id = item4.id(); - pane.close_items(cx, SaveBehavior::PromptOnWrite, move |id| { + pane.close_items(cx, SaveIntent::Close, move |id| { [item1_id, item3_id, item4_id].contains(&id) }) }); @@ -4571,7 +4558,7 @@ mod tests { // prompts, the task should complete. let close = left_pane.update(cx, |pane, cx| { - pane.close_items(cx, SaveBehavior::PromptOnWrite, move |_| true) + pane.close_items(cx, SaveIntent::Close, move |_| true) }); cx.foreground().run_until_parked(); left_pane.read_with(cx, |pane, cx| { @@ -4689,7 +4676,7 @@ mod tests { }); pane.update(cx, |pane, cx| { - pane.close_items(cx, SaveBehavior::PromptOnWrite, move |id| id == item_id) + pane.close_items(cx, SaveIntent::Close, move |id| id == item_id) }) .await .unwrap(); @@ -4712,7 +4699,7 @@ mod tests { // Ensure autosave is prevented for deleted files also when closing the buffer. let _close_items = pane.update(cx, |pane, cx| { - pane.close_items(cx, SaveBehavior::PromptOnWrite, move |id| id == item_id) + pane.close_items(cx, SaveIntent::Close, move |id| id == item_id) }); deterministic.run_until_parked(); assert!(window.has_pending_prompt(cx)); diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 72804cb523ddb8d335339dda7c07622b4d3cf32e..5363262e3f610d5a5bd073132ee26df29feb5dae 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -744,7 +744,7 @@ mod tests { use theme::{ThemeRegistry, ThemeSettings}; use workspace::{ item::{Item, ItemHandle}, - open_new, open_paths, pane, NewFile, SaveBehavior, SplitDirection, WorkspaceHandle, + open_new, open_paths, pane, NewFile, SaveIntent, SplitDirection, WorkspaceHandle, }; #[gpui::test] @@ -945,12 +945,14 @@ mod tests { editor.update(cx, |editor, cx| { assert!(editor.text(cx).is_empty()); + assert!(!editor.is_dirty(cx)); }); let save_task = workspace.update(cx, |workspace, cx| { - workspace.save_active_item(SaveBehavior::PromptOnConflict, cx) + workspace.save_active_item(SaveIntent::Save, cx) }); app_state.fs.create_dir(Path::new("/root")).await.unwrap(); + cx.foreground().run_until_parked(); cx.simulate_new_path_selection(|_| Some(PathBuf::from("/root/the-new-name"))); save_task.await.unwrap(); editor.read_with(cx, |editor, cx| { @@ -1314,7 +1316,7 @@ mod tests { cx.read(|cx| assert!(editor.is_dirty(cx))); let save_task = workspace.update(cx, |workspace, cx| { - workspace.save_active_item(SaveBehavior::PromptOnConflict, cx) + workspace.save_active_item(SaveIntent::Save, cx) }); window.simulate_prompt_answer(0, cx); save_task.await.unwrap(); @@ -1358,8 +1360,9 @@ mod tests { // Save the buffer. This prompts for a filename. let save_task = workspace.update(cx, |workspace, cx| { - workspace.save_active_item(SaveBehavior::PromptOnConflict, cx) + workspace.save_active_item(SaveIntent::Save, cx) }); + cx.foreground().run_until_parked(); cx.simulate_new_path_selection(|parent_dir| { assert_eq!(parent_dir, Path::new("/root")); Some(parent_dir.join("the-new-name.rs")) @@ -1384,7 +1387,7 @@ mod tests { assert!(editor.is_dirty(cx)); }); let save_task = workspace.update(cx, |workspace, cx| { - workspace.save_active_item(SaveBehavior::PromptOnConflict, cx) + workspace.save_active_item(SaveIntent::Save, cx) }); save_task.await.unwrap(); assert!(!cx.did_prompt_for_new_path()); @@ -1453,8 +1456,9 @@ mod tests { // Save the buffer. This prompts for a filename. let save_task = workspace.update(cx, |workspace, cx| { - workspace.save_active_item(SaveBehavior::PromptOnConflict, cx) + workspace.save_active_item(SaveIntent::Save, cx) }); + cx.foreground().run_until_parked(); cx.simulate_new_path_selection(|_| Some(PathBuf::from("/root/the-new-name.rs"))); save_task.await.unwrap(); // The buffer is not dirty anymore and the language is assigned based on the path. @@ -1692,7 +1696,7 @@ mod tests { pane.update(cx, |pane, cx| { let editor3_id = editor3.id(); drop(editor3); - pane.close_item_by_id(editor3_id, SaveBehavior::PromptOnWrite, cx) + pane.close_item_by_id(editor3_id, SaveIntent::Close, cx) }) .await .unwrap(); @@ -1727,7 +1731,7 @@ mod tests { pane.update(cx, |pane, cx| { let editor2_id = editor2.id(); drop(editor2); - pane.close_item_by_id(editor2_id, SaveBehavior::PromptOnWrite, cx) + pane.close_item_by_id(editor2_id, SaveIntent::Close, cx) }) .await .unwrap(); @@ -1884,28 +1888,28 @@ mod tests { // Close all the pane items in some arbitrary order. pane.update(cx, |pane, cx| { - pane.close_item_by_id(file1_item_id, SaveBehavior::PromptOnWrite, cx) + pane.close_item_by_id(file1_item_id, SaveIntent::Close, cx) }) .await .unwrap(); assert_eq!(active_path(&workspace, cx), Some(file4.clone())); pane.update(cx, |pane, cx| { - pane.close_item_by_id(file4_item_id, SaveBehavior::PromptOnWrite, cx) + pane.close_item_by_id(file4_item_id, SaveIntent::Close, cx) }) .await .unwrap(); assert_eq!(active_path(&workspace, cx), Some(file3.clone())); pane.update(cx, |pane, cx| { - pane.close_item_by_id(file2_item_id, SaveBehavior::PromptOnWrite, cx) + pane.close_item_by_id(file2_item_id, SaveIntent::Close, cx) }) .await .unwrap(); assert_eq!(active_path(&workspace, cx), Some(file3.clone())); pane.update(cx, |pane, cx| { - pane.close_item_by_id(file3_item_id, SaveBehavior::PromptOnWrite, cx) + pane.close_item_by_id(file3_item_id, SaveIntent::Close, cx) }) .await .unwrap(); From 4bf4c780be8d75705d269a9ee7c75399ac5f4c3c Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Wed, 20 Sep 2023 20:50:22 -0600 Subject: [PATCH 11/14] Revert accidental Cargo change --- Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 3e4c5911ed644014bc70aef383d357b5552c67d6..c1876434ad46fc7f8c5eddf59d46255fcde4136e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -158,7 +158,6 @@ core-graphics = { git = "https://github.com/servo/core-foundation-rs", rev = "07 [profile.dev] split-debuginfo = "unpacked" -panic = "abort" [profile.release] debug = true From 32f87333138617cd7525285d8bc5441dc7fcb728 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Wed, 20 Sep 2023 20:54:30 -0600 Subject: [PATCH 12/14] Code review changes --- assets/keymaps/vim.json | 4 +- crates/command_palette/src/command_palette.rs | 2 +- crates/file_finder/src/file_finder.rs | 9 +- crates/gpui/src/app/test_app_context.rs | 2 +- crates/terminal_view/src/terminal_view.rs | 7 +- crates/vim/src/command.rs | 36 +++---- crates/vim/src/normal/search.rs | 100 ++++++++++-------- crates/workspace/src/pane.rs | 92 +++++----------- crates/workspace/src/workspace.rs | 35 +++--- crates/zed/src/menus.rs | 18 +--- crates/zed/src/zed.rs | 9 +- 11 files changed, 127 insertions(+), 187 deletions(-) diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 5db0fe748ef6885f697ec71c321814c44e1a9fcb..9453607ce997e5d822f12df8abc529f2f91a3ce7 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -206,13 +206,13 @@ "shift-z shift-q": [ "pane::CloseActiveItem", { - "saveBehavior": "dontSave" + "saveIntent": "skip" } ], "shift-z shift-z": [ "pane::CloseActiveItem", { - "saveBehavior": "promptOnConflict" + "saveIntent": "saveAll" } ], // Count support diff --git a/crates/command_palette/src/command_palette.rs b/crates/command_palette/src/command_palette.rs index 90c155cdf8caba22d945d3b982cd4ec88b339828..90c448137453fa0be95744958972c6dc63d85405 100644 --- a/crates/command_palette/src/command_palette.rs +++ b/crates/command_palette/src/command_palette.rs @@ -297,7 +297,7 @@ impl PickerDelegate for CommandPaletteDelegate { } } -pub fn humanize_action_name(name: &str) -> String { +fn humanize_action_name(name: &str) -> String { let capacity = name.len() + name.chars().filter(|c| c.is_uppercase()).count(); let mut result = String::with_capacity(capacity); for char in name.chars() { diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index c2d8cc52b26f7483a40982cac04a96c4c8d92bb0..64ef31cd307dc8e5fbf82d01c98f3890d761c1c2 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -1528,13 +1528,8 @@ mod tests { let active_pane = cx.read(|cx| workspace.read(cx).active_pane().clone()); active_pane .update(cx, |pane, cx| { - pane.close_active_item( - &workspace::CloseActiveItem { - save_behavior: None, - }, - cx, - ) - .unwrap() + pane.close_active_item(&workspace::CloseActiveItem { save_intent: None }, cx) + .unwrap() }) .await .unwrap(); diff --git a/crates/gpui/src/app/test_app_context.rs b/crates/gpui/src/app/test_app_context.rs index 5b15b5274c60bb835c22e58882f6d7eb95910151..0dc1d1eba437fb67beddbde47bd10bc497ed928a 100644 --- a/crates/gpui/src/app/test_app_context.rs +++ b/crates/gpui/src/app/test_app_context.rs @@ -33,7 +33,7 @@ use super::{ #[derive(Clone)] pub struct TestAppContext { - pub cx: Rc>, + cx: Rc>, foreground_platform: Rc, condition_duration: Option, pub function_name: String, diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index b79f655f815a71b3985eb26450b2b5edf9837c26..cd939b5604716a1b6c0f523db53acce735cc9ac1 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -284,12 +284,7 @@ impl TerminalView { pub fn deploy_context_menu(&mut self, position: Vector2F, cx: &mut ViewContext) { let menu_entries = vec![ ContextMenuItem::action("Clear", Clear), - ContextMenuItem::action( - "Close", - pane::CloseActiveItem { - save_behavior: None, - }, - ), + ContextMenuItem::action("Close", pane::CloseActiveItem { save_intent: None }), ]; self.context_menu.update(cx, |menu, cx| { diff --git a/crates/vim/src/command.rs b/crates/vim/src/command.rs index 6bed7f5bb4b995df69bfb055b9b7a6acc64847ca..092d72c2fcd19a4f3dcacf78c0f6487015a0a379 100644 --- a/crates/vim/src/command.rs +++ b/crates/vim/src/command.rs @@ -50,119 +50,119 @@ pub fn command_interceptor(mut query: &str, _: &AppContext) -> Option ( "write", workspace::Save { - save_behavior: Some(SaveIntent::Save), + save_intent: Some(SaveIntent::Save), } .boxed_clone(), ), "w!" | "wr!" | "wri!" | "writ!" | "write!" => ( "write!", workspace::Save { - save_behavior: Some(SaveIntent::Overwrite), + save_intent: Some(SaveIntent::Overwrite), } .boxed_clone(), ), "q" | "qu" | "qui" | "quit" => ( "quit", workspace::CloseActiveItem { - save_behavior: Some(SaveIntent::Close), + save_intent: Some(SaveIntent::Close), } .boxed_clone(), ), "q!" | "qu!" | "qui!" | "quit!" => ( "quit!", workspace::CloseActiveItem { - save_behavior: Some(SaveIntent::Skip), + save_intent: Some(SaveIntent::Skip), } .boxed_clone(), ), "wq" => ( "wq", workspace::CloseActiveItem { - save_behavior: Some(SaveIntent::Save), + save_intent: Some(SaveIntent::Save), } .boxed_clone(), ), "wq!" => ( "wq!", workspace::CloseActiveItem { - save_behavior: Some(SaveIntent::Overwrite), + save_intent: Some(SaveIntent::Overwrite), } .boxed_clone(), ), "x" | "xi" | "xit" | "exi" | "exit" => ( "exit", workspace::CloseActiveItem { - save_behavior: Some(SaveIntent::Save), + save_intent: Some(SaveIntent::SaveAll), } .boxed_clone(), ), "x!" | "xi!" | "xit!" | "exi!" | "exit!" => ( "exit!", workspace::CloseActiveItem { - save_behavior: Some(SaveIntent::Overwrite), + save_intent: Some(SaveIntent::Overwrite), } .boxed_clone(), ), "up" | "upd" | "upda" | "updat" | "update" => ( "update", workspace::Save { - save_behavior: Some(SaveIntent::SaveAll), + save_intent: Some(SaveIntent::SaveAll), } .boxed_clone(), ), "wa" | "wal" | "wall" => ( "wall", workspace::SaveAll { - save_behavior: Some(SaveIntent::SaveAll), + save_intent: Some(SaveIntent::SaveAll), } .boxed_clone(), ), "wa!" | "wal!" | "wall!" => ( "wall!", workspace::SaveAll { - save_behavior: Some(SaveIntent::Overwrite), + save_intent: Some(SaveIntent::Overwrite), } .boxed_clone(), ), "qa" | "qal" | "qall" | "quita" | "quital" | "quitall" => ( "quitall", workspace::CloseAllItemsAndPanes { - save_behavior: Some(SaveIntent::Close), + save_intent: Some(SaveIntent::Close), } .boxed_clone(), ), "qa!" | "qal!" | "qall!" | "quita!" | "quital!" | "quitall!" => ( "quitall!", workspace::CloseAllItemsAndPanes { - save_behavior: Some(SaveIntent::Skip), + save_intent: Some(SaveIntent::Skip), } .boxed_clone(), ), "xa" | "xal" | "xall" => ( "xall", workspace::CloseAllItemsAndPanes { - save_behavior: Some(SaveIntent::SaveAll), + save_intent: Some(SaveIntent::SaveAll), } .boxed_clone(), ), "xa!" | "xal!" | "xall!" => ( "xall!", workspace::CloseAllItemsAndPanes { - save_behavior: Some(SaveIntent::Overwrite), + save_intent: Some(SaveIntent::Overwrite), } .boxed_clone(), ), "wqa" | "wqal" | "wqall" => ( "wqall", workspace::CloseAllItemsAndPanes { - save_behavior: Some(SaveIntent::SaveAll), + save_intent: Some(SaveIntent::SaveAll), } .boxed_clone(), ), "wqa!" | "wqal!" | "wqall!" => ( "wqall!", workspace::CloseAllItemsAndPanes { - save_behavior: Some(SaveIntent::Overwrite), + save_intent: Some(SaveIntent::Overwrite), } .boxed_clone(), ), @@ -197,7 +197,7 @@ pub fn command_interceptor(mut query: &str, _: &AppContext) -> Option ( "tabclose", workspace::CloseActiveItem { - save_behavior: Some(SaveIntent::Close), + save_intent: Some(SaveIntent::Close), } .boxed_clone(), ), diff --git a/crates/vim/src/normal/search.rs b/crates/vim/src/normal/search.rs index e76da1dfc5dd7dc1d7803c6fe0127a85fb089238..f74625c8b30586ae6a05a637134f9e4c3f1f4191 100644 --- a/crates/vim/src/normal/search.rs +++ b/crates/vim/src/normal/search.rs @@ -220,53 +220,58 @@ fn replace_command( let replacement = parse_replace_all(&action.query); let pane = workspace.active_pane().clone(); pane.update(cx, |pane, cx| { - if let Some(search_bar) = pane.toolbar().read(cx).item_of_type::() { - let search = search_bar.update(cx, |search_bar, cx| { - if !search_bar.show(cx) { - return None; - } + let Some(search_bar) = pane.toolbar().read(cx).item_of_type::() else { + return; + }; + let search = search_bar.update(cx, |search_bar, cx| { + if !search_bar.show(cx) { + return None; + } - let mut options = SearchOptions::default(); - if replacement.is_case_sensitive { - options.set(SearchOptions::CASE_SENSITIVE, true) - } - let search = if replacement.search == "" { - search_bar.query(cx) - } else { - replacement.search - }; + let mut options = SearchOptions::default(); + if replacement.is_case_sensitive { + options.set(SearchOptions::CASE_SENSITIVE, true) + } + let search = if replacement.search == "" { + search_bar.query(cx) + } else { + replacement.search + }; - search_bar.set_replacement(Some(&replacement.replacement), cx); - search_bar.activate_search_mode(SearchMode::Regex, cx); - Some(search_bar.search(&search, Some(options), cx)) - }); - let Some(search) = search else { return }; - let search_bar = search_bar.downgrade(); - cx.spawn(|_, mut cx| async move { - search.await?; - search_bar.update(&mut cx, |search_bar, cx| { - if replacement.should_replace_all { - search_bar.select_last_match(cx); - search_bar.replace_all(&Default::default(), cx); - Vim::update(cx, |vim, cx| { - move_cursor( - vim, - Motion::StartOfLine { - display_lines: false, - }, - None, - cx, - ) - }) - } - })?; - anyhow::Ok(()) - }) - .detach_and_log_err(cx); - } + search_bar.set_replacement(Some(&replacement.replacement), cx); + search_bar.activate_search_mode(SearchMode::Regex, cx); + Some(search_bar.search(&search, Some(options), cx)) + }); + let Some(search) = search else { return }; + let search_bar = search_bar.downgrade(); + cx.spawn(|_, mut cx| async move { + search.await?; + search_bar.update(&mut cx, |search_bar, cx| { + if replacement.should_replace_all { + search_bar.select_last_match(cx); + search_bar.replace_all(&Default::default(), cx); + Vim::update(cx, |vim, cx| { + move_cursor( + vim, + Motion::StartOfLine { + display_lines: false, + }, + None, + cx, + ) + }) + } + })?; + anyhow::Ok(()) + }) + .detach_and_log_err(cx); }) } +// convert a vim query into something more usable by zed. +// we don't attempt to fully convert between the two regex syntaxes, +// but we do flip \( and \) to ( and ) (and vice-versa) in the pattern, +// and convert \0..\9 to $0..$9 in the replacement so that common idioms work. fn parse_replace_all(query: &str) -> Replacement { let mut chars = query.chars(); if Some('%') != chars.next() || Some('s') != chars.next() { @@ -284,17 +289,18 @@ fn parse_replace_all(query: &str) -> Replacement { let mut buffer = &mut search; let mut escaped = false; + // 0 - parsing search + // 1 - parsing replacement + // 2 - parsing flags let mut phase = 0; for c in chars { if escaped { escaped = false; if phase == 1 && c.is_digit(10) { - // help vim users discover zed regex syntax - // (though we don't try and fix arbitrary patterns for them) buffer.push('$') + // unescape escaped parens } else if phase == 0 && c == '(' || c == ')' { - // un-escape parens } else if c != delimeter { buffer.push('\\') } @@ -312,6 +318,10 @@ fn parse_replace_all(query: &str) -> Replacement { break; } } else { + // escape unescaped parens + if phase == 0 && c == '(' || c == ')' { + buffer.push('\\') + } buffer.push(c) } } diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 40e6d36f3bea4419d69be1b6e7dd31b6435cabcd..fbe018409b4008d3675146c44c3cc9f3c81b3784 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -86,13 +86,13 @@ pub struct CloseItemsToTheRightById { #[derive(Clone, PartialEq, Debug, Deserialize, Default)] #[serde(rename_all = "camelCase")] pub struct CloseActiveItem { - pub save_behavior: Option, + pub save_intent: Option, } #[derive(Clone, PartialEq, Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct CloseAllItems { - pub save_behavior: Option, + pub save_intent: Option, } actions!( @@ -734,7 +734,7 @@ impl Pane { let active_item_id = self.items[self.active_item_index].id(); Some(self.close_item_by_id( active_item_id, - action.save_behavior.unwrap_or(SaveIntent::Close), + action.save_intent.unwrap_or(SaveIntent::Close), cx, )) } @@ -742,12 +742,10 @@ impl Pane { pub fn close_item_by_id( &mut self, item_id_to_close: usize, - save_behavior: SaveIntent, + save_intent: SaveIntent, cx: &mut ViewContext, ) -> Task> { - self.close_items(cx, save_behavior, move |view_id| { - view_id == item_id_to_close - }) + self.close_items(cx, save_intent, move |view_id| view_id == item_id_to_close) } pub fn close_inactive_items( @@ -844,17 +842,17 @@ impl Pane { return None; } - Some(self.close_items( - cx, - action.save_behavior.unwrap_or(SaveIntent::Close), - |_| true, - )) + Some( + self.close_items(cx, action.save_intent.unwrap_or(SaveIntent::Close), |_| { + true + }), + ) } pub fn close_items( &mut self, cx: &mut ViewContext, - save_behavior: SaveIntent, + save_intent: SaveIntent, should_close: impl 'static + Fn(usize) -> bool, ) -> Task> { // Find the items to close. @@ -912,7 +910,7 @@ impl Pane { &pane, item_ix, &*item, - save_behavior, + save_intent, &mut cx, ) .await? @@ -1010,14 +1008,14 @@ impl Pane { pane: &WeakViewHandle, item_ix: usize, item: &dyn ItemHandle, - save_behavior: SaveIntent, + save_intent: SaveIntent, cx: &mut AsyncAppContext, ) -> Result { const CONFLICT_MESSAGE: &str = "This file has changed on disk since you started editing it. Do you want to overwrite it?"; const DIRTY_MESSAGE: &str = "This file contains unsaved edits. Do you want to save it?"; - if save_behavior == SaveIntent::Skip { + if save_intent == SaveIntent::Skip { return Ok(true); } @@ -1031,17 +1029,17 @@ impl Pane { }); // when saving a single buffer, we ignore whether or not it's dirty. - if save_behavior == SaveIntent::Save { + if save_intent == SaveIntent::Save { is_dirty = true; } - if save_behavior == SaveIntent::SaveAs { + if save_intent == SaveIntent::SaveAs { is_dirty = true; has_conflict = false; can_save = false; } - if save_behavior == SaveIntent::Overwrite { + if save_intent == SaveIntent::Overwrite { has_conflict = false; } @@ -1060,7 +1058,7 @@ impl Pane { _ => return Ok(false), } } else if is_dirty && (can_save || can_save_as) { - if save_behavior == SaveIntent::Close { + if save_intent == SaveIntent::Close { let will_autosave = cx.read(|cx| { matches!( settings::get::(cx).autosave, @@ -1188,9 +1186,7 @@ impl Pane { vec![ ContextMenuItem::action( "Close Active Item", - CloseActiveItem { - save_behavior: None, - }, + CloseActiveItem { save_intent: None }, ), ContextMenuItem::action("Close Inactive Items", CloseInactiveItems), ContextMenuItem::action("Close Clean Items", CloseCleanItems), @@ -1198,9 +1194,7 @@ impl Pane { ContextMenuItem::action("Close Items To The Right", CloseItemsToTheRight), ContextMenuItem::action( "Close All Items", - CloseAllItems { - save_behavior: None, - }, + CloseAllItems { save_intent: None }, ), ] } else { @@ -1247,9 +1241,7 @@ impl Pane { }), ContextMenuItem::action( "Close All Items", - CloseAllItems { - save_behavior: None, - }, + CloseAllItems { save_intent: None }, ), ] }, @@ -2182,12 +2174,7 @@ mod tests { pane.update(cx, |pane, cx| { assert!(pane - .close_active_item( - &CloseActiveItem { - save_behavior: None - }, - cx - ) + .close_active_item(&CloseActiveItem { save_intent: None }, cx) .is_none()) }); } @@ -2439,12 +2426,7 @@ mod tests { assert_item_labels(&pane, ["A", "B", "1*", "C", "D"], cx); pane.update(cx, |pane, cx| { - pane.close_active_item( - &CloseActiveItem { - save_behavior: None, - }, - cx, - ) + pane.close_active_item(&CloseActiveItem { save_intent: None }, cx) }) .unwrap() .await @@ -2455,12 +2437,7 @@ mod tests { assert_item_labels(&pane, ["A", "B", "C", "D*"], cx); pane.update(cx, |pane, cx| { - pane.close_active_item( - &CloseActiveItem { - save_behavior: None, - }, - cx, - ) + pane.close_active_item(&CloseActiveItem { save_intent: None }, cx) }) .unwrap() .await @@ -2468,12 +2445,7 @@ mod tests { assert_item_labels(&pane, ["A", "B*", "C"], cx); pane.update(cx, |pane, cx| { - pane.close_active_item( - &CloseActiveItem { - save_behavior: None, - }, - cx, - ) + pane.close_active_item(&CloseActiveItem { save_intent: None }, cx) }) .unwrap() .await @@ -2481,12 +2453,7 @@ mod tests { assert_item_labels(&pane, ["A", "C*"], cx); pane.update(cx, |pane, cx| { - pane.close_active_item( - &CloseActiveItem { - save_behavior: None, - }, - cx, - ) + pane.close_active_item(&CloseActiveItem { save_intent: None }, cx) }) .unwrap() .await @@ -2597,12 +2564,7 @@ mod tests { assert_item_labels(&pane, ["A", "B", "C*"], cx); pane.update(cx, |pane, cx| { - pane.close_all_items( - &CloseAllItems { - save_behavior: None, - }, - cx, - ) + pane.close_all_items(&CloseAllItems { save_intent: None }, cx) }) .unwrap() .await diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index a853691b7644028e570e7b7b78176d71d2f1aca2..092286973e867007cedbb0b37c94030a6e906fe6 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -163,19 +163,19 @@ pub struct NewFileInDirection(pub SplitDirection); #[derive(Clone, PartialEq, Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct SaveAll { - pub save_behavior: Option, + pub save_intent: Option, } #[derive(Clone, PartialEq, Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct Save { - pub save_behavior: Option, + pub save_intent: Option, } #[derive(Clone, PartialEq, Debug, Deserialize, Default)] #[serde(rename_all = "camelCase")] pub struct CloseAllItemsAndPanes { - pub save_behavior: Option, + pub save_intent: Option, } #[derive(Deserialize)] @@ -294,7 +294,7 @@ pub fn init(app_state: Arc, cx: &mut AppContext) { cx.add_action( |workspace: &mut Workspace, action: &Save, cx: &mut ViewContext| { workspace - .save_active_item(action.save_behavior.unwrap_or(SaveIntent::Save), cx) + .save_active_item(action.save_intent.unwrap_or(SaveIntent::Save), cx) .detach_and_log_err(cx); }, ); @@ -1363,7 +1363,7 @@ impl Workspace { cx: &mut ViewContext, ) -> Option>> { let save_all = - self.save_all_internal(action.save_behavior.unwrap_or(SaveIntent::SaveAll), cx); + self.save_all_internal(action.save_intent.unwrap_or(SaveIntent::SaveAll), cx); Some(cx.foreground().spawn(async move { save_all.await?; Ok(()) @@ -1372,7 +1372,7 @@ impl Workspace { fn save_all_internal( &mut self, - save_behaviour: SaveIntent, + save_intent: SaveIntent, cx: &mut ViewContext, ) -> Task> { if self.project.read(cx).is_read_only() { @@ -1407,7 +1407,7 @@ impl Workspace { &pane, ix, &*item, - save_behaviour, + save_intent, &mut cx, ) .await? @@ -1679,7 +1679,7 @@ impl Workspace { pub fn save_active_item( &mut self, - save_behavior: SaveIntent, + save_intent: SaveIntent, cx: &mut ViewContext, ) -> Task> { let project = self.project.clone(); @@ -1690,16 +1690,9 @@ impl Workspace { cx.spawn(|_, mut cx| async move { if let Some(item) = item { - Pane::save_item( - project, - &pane, - item_ix, - item.as_ref(), - save_behavior, - &mut cx, - ) - .await - .map(|_| ()) + Pane::save_item(project, &pane, item_ix, item.as_ref(), save_intent, &mut cx) + .await + .map(|_| ()) } else { Ok(()) } @@ -1719,13 +1712,13 @@ impl Workspace { action: &CloseAllItemsAndPanes, cx: &mut ViewContext, ) -> Option>> { - self.close_all_internal(false, action.save_behavior.unwrap_or(SaveIntent::Close), cx) + self.close_all_internal(false, action.save_intent.unwrap_or(SaveIntent::Close), cx) } fn close_all_internal( &mut self, retain_active_pane: bool, - save_behavior: SaveIntent, + save_intent: SaveIntent, cx: &mut ViewContext, ) -> Option>> { let current_pane = self.active_pane(); @@ -1748,7 +1741,7 @@ impl Workspace { if let Some(close_pane_items) = pane.update(cx, |pane: &mut Pane, cx| { pane.close_all_items( &CloseAllItems { - save_behavior: Some(save_behavior), + save_intent: Some(save_intent), }, cx, ) diff --git a/crates/zed/src/menus.rs b/crates/zed/src/menus.rs index acffbc29abbe1bf4486548e859a397bc7fb3a62d..4e01693dbf6980c10d99c2fc727eeb1ad642b31b 100644 --- a/crates/zed/src/menus.rs +++ b/crates/zed/src/menus.rs @@ -38,24 +38,12 @@ pub fn menus() -> Vec> { MenuItem::action("Open Recent...", recent_projects::OpenRecent), MenuItem::separator(), MenuItem::action("Add Folder to Project…", workspace::AddFolderToProject), - MenuItem::action( - "Save", - workspace::Save { - save_behavior: None, - }, - ), + MenuItem::action("Save", workspace::Save { save_intent: None }), MenuItem::action("Save As…", workspace::SaveAs), - MenuItem::action( - "Save All", - workspace::SaveAll { - save_behavior: None, - }, - ), + MenuItem::action("Save All", workspace::SaveAll { save_intent: None }), MenuItem::action( "Close Editor", - workspace::CloseActiveItem { - save_behavior: None, - }, + workspace::CloseActiveItem { save_intent: None }, ), MenuItem::action("Close Window", workspace::CloseWindow), ], diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 5363262e3f610d5a5bd073132ee26df29feb5dae..11e80ffb4a5917f1c1b4f814a48140987ee90633 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -1318,6 +1318,7 @@ mod tests { let save_task = workspace.update(cx, |workspace, cx| { workspace.save_active_item(SaveIntent::Save, cx) }); + cx.foreground().run_until_parked(); window.simulate_prompt_answer(0, cx); save_task.await.unwrap(); editor.read_with(cx, |editor, cx| { @@ -1522,9 +1523,7 @@ mod tests { }); cx.dispatch_action( window.into(), - workspace::CloseActiveItem { - save_behavior: None, - }, + workspace::CloseActiveItem { save_intent: None }, ); cx.foreground().run_until_parked(); @@ -1535,9 +1534,7 @@ mod tests { cx.dispatch_action( window.into(), - workspace::CloseActiveItem { - save_behavior: None, - }, + workspace::CloseActiveItem { save_intent: None }, ); cx.foreground().run_until_parked(); window.simulate_prompt_answer(1, cx); From 5c75450a77b0579649bbf79365c9cef5a2c1110f Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Mon, 25 Sep 2023 11:41:09 -0600 Subject: [PATCH 13/14] Revert "workspace: Improve save prompt. (#3025)" This reverts commit 0a491e773b689a74f96b7555070cf5a3bf245543. --- crates/util/src/util.rs | 14 ---- crates/workspace/src/pane.rs | 128 +++++++----------------------- crates/workspace/src/workspace.rs | 39 ++------- 3 files changed, 33 insertions(+), 148 deletions(-) diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index 629f9500147533a85154995fe2462f5b6d5f5297..3f83f8e37a5c493062d2291b7f56995ca527254b 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -41,8 +41,6 @@ pub fn truncate(s: &str, max_chars: usize) -> &str { } } -/// Removes characters from the end of the string if it's length is greater than `max_chars` and -/// appends "..." to the string. Returns string unchanged if it's length is smaller than max_chars. pub fn truncate_and_trailoff(s: &str, max_chars: usize) -> String { debug_assert!(max_chars >= 5); @@ -53,18 +51,6 @@ pub fn truncate_and_trailoff(s: &str, max_chars: usize) -> String { } } -/// Removes characters from the front of the string if it's length is greater than `max_chars` and -/// prepends the string with "...". Returns string unchanged if it's length is smaller than max_chars. -pub fn truncate_and_remove_front(s: &str, max_chars: usize) -> String { - debug_assert!(max_chars >= 5); - - let truncation_ix = s.char_indices().map(|(i, _)| i).nth_back(max_chars); - match truncation_ix { - Some(length) => "…".to_string() + &s[length..], - None => s.to_string(), - } -} - pub fn post_inc + AddAssign + Copy>(value: &mut T) -> T { let prev = *value; *value += T::from(1); diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index a191adcc05e0798316d311b26b5368b711eebee5..a3e6a547ddfd73c1fd67c50c4b6c4df2d6ff51d1 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -42,7 +42,6 @@ use std::{ }, }; use theme::{Theme, ThemeSettings}; -use util::truncate_and_remove_front; #[derive(PartialEq, Clone, Copy, Deserialize, Debug)] #[serde(rename_all = "camelCase")] @@ -840,45 +839,10 @@ impl Pane { Some(self.close_items(cx, SaveBehavior::PromptOnWrite, |_| true)) } - pub(super) fn file_names_for_prompt( - items: &mut dyn Iterator>, - all_dirty_items: usize, - cx: &AppContext, - ) -> String { - /// Quantity of item paths displayed in prompt prior to cutoff.. - const FILE_NAMES_CUTOFF_POINT: usize = 10; - let mut file_names: Vec<_> = items - .filter_map(|item| { - item.project_path(cx).and_then(|project_path| { - project_path - .path - .file_name() - .and_then(|name| name.to_str().map(ToOwned::to_owned)) - }) - }) - .take(FILE_NAMES_CUTOFF_POINT) - .collect(); - let should_display_followup_text = - all_dirty_items > FILE_NAMES_CUTOFF_POINT || file_names.len() != all_dirty_items; - if should_display_followup_text { - let not_shown_files = all_dirty_items - file_names.len(); - if not_shown_files == 1 { - file_names.push(".. 1 file not shown".into()); - } else { - file_names.push(format!(".. {} files not shown", not_shown_files).into()); - } - } - let file_names = file_names.join("\n"); - format!( - "Do you want to save changes to the following {} files?\n{file_names}", - all_dirty_items - ) - } - pub fn close_items( &mut self, cx: &mut ViewContext, - mut save_behavior: SaveBehavior, + save_behavior: SaveBehavior, should_close: impl 'static + Fn(usize) -> bool, ) -> Task> { // Find the items to close. @@ -897,25 +861,6 @@ impl Pane { let workspace = self.workspace.clone(); cx.spawn(|pane, mut cx| async move { - if save_behavior == SaveBehavior::PromptOnWrite && items_to_close.len() > 1 { - let mut answer = pane.update(&mut cx, |_, cx| { - let prompt = Self::file_names_for_prompt( - &mut items_to_close.iter(), - items_to_close.len(), - cx, - ); - cx.prompt( - PromptLevel::Warning, - &prompt, - &["Save all", "Discard all", "Cancel"], - ) - })?; - match answer.next().await { - Some(0) => save_behavior = SaveBehavior::PromptOnConflict, - Some(1) => save_behavior = SaveBehavior::DontSave, - _ => {} - } - } let mut saved_project_items_ids = HashSet::default(); for item in items_to_close.clone() { // Find the item's current index and its set of project item models. Avoid @@ -1058,6 +1003,7 @@ impl Pane { ) -> Result { const CONFLICT_MESSAGE: &str = "This file has changed on disk since you started editing it. Do you want to overwrite it?"; + const DIRTY_MESSAGE: &str = "This file contains unsaved edits. Do you want to save it?"; if save_behavior == SaveBehavior::DontSave { return Ok(true); @@ -1100,10 +1046,9 @@ impl Pane { let should_save = if save_behavior == SaveBehavior::PromptOnWrite && !will_autosave { let mut answer = pane.update(cx, |pane, cx| { pane.activate_item(item_ix, true, true, cx); - let prompt = dirty_message_for(item.project_path(cx)); cx.prompt( PromptLevel::Warning, - &prompt, + DIRTY_MESSAGE, &["Save", "Don't Save", "Cancel"], ) })?; @@ -2190,15 +2135,6 @@ impl Element for PaneBackdrop { } } -fn dirty_message_for(buffer_path: Option) -> String { - let path = buffer_path - .as_ref() - .and_then(|p| p.path.to_str()) - .unwrap_or(&"Untitled buffer"); - let path = truncate_and_remove_front(path, 80); - format!("{path} contains unsaved edits. Do you want to save it?") -} - #[cfg(test)] mod tests { use super::*; @@ -2543,14 +2479,12 @@ mod tests { set_labeled_items(&pane, ["A", "B", "C*", "D", "E"], cx); - let task = pane - .update(cx, |pane, cx| { - pane.close_inactive_items(&CloseInactiveItems, cx) - }) - .unwrap(); - cx.foreground().run_until_parked(); - window.simulate_prompt_answer(2, cx); - task.await.unwrap(); + pane.update(cx, |pane, cx| { + pane.close_inactive_items(&CloseInactiveItems, cx) + }) + .unwrap() + .await + .unwrap(); assert_item_labels(&pane, ["C*"], cx); } @@ -2571,12 +2505,10 @@ mod tests { add_labeled_item(&pane, "E", false, cx); assert_item_labels(&pane, ["A^", "B", "C^", "D", "E*"], cx); - let task = pane - .update(cx, |pane, cx| pane.close_clean_items(&CloseCleanItems, cx)) + pane.update(cx, |pane, cx| pane.close_clean_items(&CloseCleanItems, cx)) + .unwrap() + .await .unwrap(); - cx.foreground().run_until_parked(); - window.simulate_prompt_answer(2, cx); - task.await.unwrap(); assert_item_labels(&pane, ["A^", "C*^"], cx); } @@ -2592,14 +2524,12 @@ mod tests { set_labeled_items(&pane, ["A", "B", "C*", "D", "E"], cx); - let task = pane - .update(cx, |pane, cx| { - pane.close_items_to_the_left(&CloseItemsToTheLeft, cx) - }) - .unwrap(); - cx.foreground().run_until_parked(); - window.simulate_prompt_answer(2, cx); - task.await.unwrap(); + pane.update(cx, |pane, cx| { + pane.close_items_to_the_left(&CloseItemsToTheLeft, cx) + }) + .unwrap() + .await + .unwrap(); assert_item_labels(&pane, ["C*", "D", "E"], cx); } @@ -2615,14 +2545,12 @@ mod tests { set_labeled_items(&pane, ["A", "B", "C*", "D", "E"], cx); - let task = pane - .update(cx, |pane, cx| { - pane.close_items_to_the_right(&CloseItemsToTheRight, cx) - }) - .unwrap(); - cx.foreground().run_until_parked(); - window.simulate_prompt_answer(2, cx); - task.await.unwrap(); + pane.update(cx, |pane, cx| { + pane.close_items_to_the_right(&CloseItemsToTheRight, cx) + }) + .unwrap() + .await + .unwrap(); assert_item_labels(&pane, ["A", "B", "C*"], cx); } @@ -2641,12 +2569,10 @@ mod tests { add_labeled_item(&pane, "C", false, cx); assert_item_labels(&pane, ["A", "B", "C*"], cx); - let t = pane - .update(cx, |pane, cx| pane.close_all_items(&CloseAllItems, cx)) + pane.update(cx, |pane, cx| pane.close_all_items(&CloseAllItems, cx)) + .unwrap() + .await .unwrap(); - cx.foreground().run_until_parked(); - window.simulate_prompt_answer(2, cx); - t.await.unwrap(); assert_item_labels(&pane, [], cx); } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 263652184a89c85810861831d6b1b660454875df..feab53d0941d1823fc79930d3697c39e54822207 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1333,12 +1333,13 @@ impl Workspace { fn save_all_internal( &mut self, - mut save_behaviour: SaveBehavior, + save_behaviour: SaveBehavior, cx: &mut ViewContext, ) -> Task> { if self.project.read(cx).is_read_only() { return Task::ready(Ok(true)); } + let dirty_items = self .panes .iter() @@ -1354,27 +1355,7 @@ impl Workspace { .collect::>(); let project = self.project.clone(); - cx.spawn(|workspace, mut cx| async move { - // Override save mode and display "Save all files" prompt - if save_behaviour == SaveBehavior::PromptOnWrite && dirty_items.len() > 1 { - let mut answer = workspace.update(&mut cx, |_, cx| { - let prompt = Pane::file_names_for_prompt( - &mut dirty_items.iter().map(|(_, handle)| handle), - dirty_items.len(), - cx, - ); - cx.prompt( - PromptLevel::Warning, - &prompt, - &["Save all", "Discard all", "Cancel"], - ) - })?; - match answer.next().await { - Some(0) => save_behaviour = SaveBehavior::PromptOnConflict, - Some(1) => save_behaviour = SaveBehavior::DontSave, - _ => {} - } - } + cx.spawn(|_, mut cx| async move { for (pane, item) in dirty_items { let (singleton, project_entry_ids) = cx.read(|cx| (item.is_singleton(cx), item.project_entry_ids(cx))); @@ -4339,9 +4320,7 @@ mod tests { }); let task = workspace.update(cx, |w, cx| w.prepare_to_close(false, cx)); cx.foreground().run_until_parked(); - window.simulate_prompt_answer(2, cx); // cancel save all - cx.foreground().run_until_parked(); - window.simulate_prompt_answer(2, cx); // cancel save all + window.simulate_prompt_answer(2, cx); // cancel cx.foreground().run_until_parked(); assert!(!window.has_pending_prompt(cx)); assert!(!task.await.unwrap()); @@ -4399,15 +4378,13 @@ mod tests { }); cx.foreground().run_until_parked(); - assert!(window.has_pending_prompt(cx)); - // Ignore "Save all" prompt - window.simulate_prompt_answer(2, cx); - cx.foreground().run_until_parked(); // There's a prompt to save item 1. pane.read_with(cx, |pane, _| { assert_eq!(pane.items_len(), 4); assert_eq!(pane.active_item().unwrap().id(), item1.id()); }); + assert!(window.has_pending_prompt(cx)); + // Confirm saving item 1. window.simulate_prompt_answer(0, cx); cx.foreground().run_until_parked(); @@ -4535,10 +4512,6 @@ mod tests { let close = left_pane.update(cx, |pane, cx| { pane.close_items(cx, SaveBehavior::PromptOnWrite, move |_| true) }); - cx.foreground().run_until_parked(); - // Discard "Save all" prompt - window.simulate_prompt_answer(2, cx); - cx.foreground().run_until_parked(); left_pane.read_with(cx, |pane, cx| { assert_eq!( From 359847d04758beb30822242ca6ccba19b5e3325d Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Mon, 25 Sep 2023 12:18:03 -0600 Subject: [PATCH 14/14] Revert "Revert "workspace: Improve save prompt. (#3025)"" This reverts commit 5c75450a77b0579649bbf79365c9cef5a2c1110f. --- crates/search/src/buffer_search.rs | 4 +- crates/util/src/util.rs | 14 +++ crates/workspace/src/pane.rs | 136 ++++++++++++++++++++++------- crates/workspace/src/workspace.rs | 39 +++++++-- 4 files changed, 154 insertions(+), 39 deletions(-) diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index 72893656d61a82327ecdd36e242fd1381e0b39ec..4f0deda6e0e95e2006240c88f19e3f71dc800c7e 100644 --- a/crates/search/src/buffer_search.rs +++ b/crates/search/src/buffer_search.rs @@ -541,10 +541,10 @@ impl BufferSearchBar { pub fn set_replacement(&mut self, replacement: Option<&str>, cx: &mut ViewContext) { if replacement.is_none() { - self.replace_is_active = false; + self.replace_enabled = false; return; } - self.replace_is_active = true; + self.replace_enabled = true; self.replacement_editor .update(cx, |replacement_editor, cx| { replacement_editor diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index 3f83f8e37a5c493062d2291b7f56995ca527254b..629f9500147533a85154995fe2462f5b6d5f5297 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -41,6 +41,8 @@ pub fn truncate(s: &str, max_chars: usize) -> &str { } } +/// Removes characters from the end of the string if it's length is greater than `max_chars` and +/// appends "..." to the string. Returns string unchanged if it's length is smaller than max_chars. pub fn truncate_and_trailoff(s: &str, max_chars: usize) -> String { debug_assert!(max_chars >= 5); @@ -51,6 +53,18 @@ pub fn truncate_and_trailoff(s: &str, max_chars: usize) -> String { } } +/// Removes characters from the front of the string if it's length is greater than `max_chars` and +/// prepends the string with "...". Returns string unchanged if it's length is smaller than max_chars. +pub fn truncate_and_remove_front(s: &str, max_chars: usize) -> String { + debug_assert!(max_chars >= 5); + + let truncation_ix = s.char_indices().map(|(i, _)| i).nth_back(max_chars); + match truncation_ix { + Some(length) => "…".to_string() + &s[length..], + None => s.to_string(), + } +} + pub fn post_inc + AddAssign + Copy>(value: &mut T) -> T { let prev = *value; *value += T::from(1); diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index fbe018409b4008d3675146c44c3cc9f3c81b3784..e27100863732b68607e6e6dce0c14ea008bb3298 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -42,6 +42,7 @@ use std::{ }, }; use theme::{Theme, ThemeSettings}; +use util::truncate_and_remove_front; #[derive(PartialEq, Clone, Copy, Deserialize, Debug)] #[serde(rename_all = "camelCase")] @@ -849,10 +850,45 @@ impl Pane { ) } + pub(super) fn file_names_for_prompt( + items: &mut dyn Iterator>, + all_dirty_items: usize, + cx: &AppContext, + ) -> String { + /// Quantity of item paths displayed in prompt prior to cutoff.. + const FILE_NAMES_CUTOFF_POINT: usize = 10; + let mut file_names: Vec<_> = items + .filter_map(|item| { + item.project_path(cx).and_then(|project_path| { + project_path + .path + .file_name() + .and_then(|name| name.to_str().map(ToOwned::to_owned)) + }) + }) + .take(FILE_NAMES_CUTOFF_POINT) + .collect(); + let should_display_followup_text = + all_dirty_items > FILE_NAMES_CUTOFF_POINT || file_names.len() != all_dirty_items; + if should_display_followup_text { + let not_shown_files = all_dirty_items - file_names.len(); + if not_shown_files == 1 { + file_names.push(".. 1 file not shown".into()); + } else { + file_names.push(format!(".. {} files not shown", not_shown_files).into()); + } + } + let file_names = file_names.join("\n"); + format!( + "Do you want to save changes to the following {} files?\n{file_names}", + all_dirty_items + ) + } + pub fn close_items( &mut self, cx: &mut ViewContext, - save_intent: SaveIntent, + mut save_intent: SaveIntent, should_close: impl 'static + Fn(usize) -> bool, ) -> Task> { // Find the items to close. @@ -871,6 +907,25 @@ impl Pane { let workspace = self.workspace.clone(); cx.spawn(|pane, mut cx| async move { + if save_intent == SaveIntent::Close && items_to_close.len() > 1 { + let mut answer = pane.update(&mut cx, |_, cx| { + let prompt = Self::file_names_for_prompt( + &mut items_to_close.iter(), + items_to_close.len(), + cx, + ); + cx.prompt( + PromptLevel::Warning, + &prompt, + &["Save all", "Discard all", "Cancel"], + ) + })?; + match answer.next().await { + Some(0) => save_intent = SaveIntent::Save, + Some(1) => save_intent = SaveIntent::Skip, + _ => {} + } + } let mut saved_project_items_ids = HashSet::default(); for item in items_to_close.clone() { // Find the item's current index and its set of project item models. Avoid @@ -1013,7 +1068,6 @@ impl Pane { ) -> Result { const CONFLICT_MESSAGE: &str = "This file has changed on disk since you started editing it. Do you want to overwrite it?"; - const DIRTY_MESSAGE: &str = "This file contains unsaved edits. Do you want to save it?"; if save_intent == SaveIntent::Skip { return Ok(true); @@ -1068,15 +1122,16 @@ impl Pane { if !will_autosave { let mut answer = pane.update(cx, |pane, cx| { pane.activate_item(item_ix, true, true, cx); + let prompt = dirty_message_for(item.project_path(cx)); cx.prompt( PromptLevel::Warning, - DIRTY_MESSAGE, + &prompt, &["Save", "Don't Save", "Cancel"], ) })?; match answer.next().await { Some(0) => {} - Some(1) => return Ok(true), // Don't save this file + Some(1) => return Ok(true), // Don't save his file _ => return Ok(false), // Cancel } } @@ -2154,6 +2209,15 @@ impl Element for PaneBackdrop { } } +fn dirty_message_for(buffer_path: Option) -> String { + let path = buffer_path + .as_ref() + .and_then(|p| p.path.to_str()) + .unwrap_or(&"Untitled buffer"); + let path = truncate_and_remove_front(path, 80); + format!("{path} contains unsaved edits. Do you want to save it?") +} + #[cfg(test)] mod tests { use super::*; @@ -2473,12 +2537,14 @@ mod tests { set_labeled_items(&pane, ["A", "B", "C*", "D", "E"], cx); - pane.update(cx, |pane, cx| { - pane.close_inactive_items(&CloseInactiveItems, cx) - }) - .unwrap() - .await - .unwrap(); + let task = pane + .update(cx, |pane, cx| { + pane.close_inactive_items(&CloseInactiveItems, cx) + }) + .unwrap(); + cx.foreground().run_until_parked(); + window.simulate_prompt_answer(2, cx); + task.await.unwrap(); assert_item_labels(&pane, ["C*"], cx); } @@ -2499,10 +2565,12 @@ mod tests { add_labeled_item(&pane, "E", false, cx); assert_item_labels(&pane, ["A^", "B", "C^", "D", "E*"], cx); - pane.update(cx, |pane, cx| pane.close_clean_items(&CloseCleanItems, cx)) - .unwrap() - .await + let task = pane + .update(cx, |pane, cx| pane.close_clean_items(&CloseCleanItems, cx)) .unwrap(); + cx.foreground().run_until_parked(); + window.simulate_prompt_answer(2, cx); + task.await.unwrap(); assert_item_labels(&pane, ["A^", "C*^"], cx); } @@ -2518,12 +2586,14 @@ mod tests { set_labeled_items(&pane, ["A", "B", "C*", "D", "E"], cx); - pane.update(cx, |pane, cx| { - pane.close_items_to_the_left(&CloseItemsToTheLeft, cx) - }) - .unwrap() - .await - .unwrap(); + let task = pane + .update(cx, |pane, cx| { + pane.close_items_to_the_left(&CloseItemsToTheLeft, cx) + }) + .unwrap(); + cx.foreground().run_until_parked(); + window.simulate_prompt_answer(2, cx); + task.await.unwrap(); assert_item_labels(&pane, ["C*", "D", "E"], cx); } @@ -2539,12 +2609,14 @@ mod tests { set_labeled_items(&pane, ["A", "B", "C*", "D", "E"], cx); - pane.update(cx, |pane, cx| { - pane.close_items_to_the_right(&CloseItemsToTheRight, cx) - }) - .unwrap() - .await - .unwrap(); + let task = pane + .update(cx, |pane, cx| { + pane.close_items_to_the_right(&CloseItemsToTheRight, cx) + }) + .unwrap(); + cx.foreground().run_until_parked(); + window.simulate_prompt_answer(2, cx); + task.await.unwrap(); assert_item_labels(&pane, ["A", "B", "C*"], cx); } @@ -2563,12 +2635,14 @@ mod tests { add_labeled_item(&pane, "C", false, cx); assert_item_labels(&pane, ["A", "B", "C*"], cx); - pane.update(cx, |pane, cx| { - pane.close_all_items(&CloseAllItems { save_intent: None }, cx) - }) - .unwrap() - .await - .unwrap(); + let t = pane + .update(cx, |pane, cx| { + pane.close_all_items(&CloseAllItems { save_intent: None }, cx) + }) + .unwrap(); + cx.foreground().run_until_parked(); + window.simulate_prompt_answer(2, cx); + t.await.unwrap(); assert_item_labels(&pane, [], cx); } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 092286973e867007cedbb0b37c94030a6e906fe6..f081eb9efae77150de27840816828acd8dd72914 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1372,13 +1372,12 @@ impl Workspace { fn save_all_internal( &mut self, - save_intent: SaveIntent, + mut save_intent: SaveIntent, cx: &mut ViewContext, ) -> Task> { if self.project.read(cx).is_read_only() { return Task::ready(Ok(true)); } - let dirty_items = self .panes .iter() @@ -1394,7 +1393,27 @@ impl Workspace { .collect::>(); let project = self.project.clone(); - cx.spawn(|_, mut cx| async move { + cx.spawn(|workspace, mut cx| async move { + // Override save mode and display "Save all files" prompt + if save_intent == SaveIntent::Close && dirty_items.len() > 1 { + let mut answer = workspace.update(&mut cx, |_, cx| { + let prompt = Pane::file_names_for_prompt( + &mut dirty_items.iter().map(|(_, handle)| handle), + dirty_items.len(), + cx, + ); + cx.prompt( + PromptLevel::Warning, + &prompt, + &["Save all", "Discard all", "Cancel"], + ) + })?; + match answer.next().await { + Some(0) => save_intent = SaveIntent::Save, + Some(1) => save_intent = SaveIntent::Skip, + _ => {} + } + } for (pane, item) in dirty_items { let (singleton, project_entry_ids) = cx.read(|cx| (item.is_singleton(cx), item.project_entry_ids(cx))); @@ -4361,7 +4380,9 @@ mod tests { }); let task = workspace.update(cx, |w, cx| w.prepare_to_close(false, cx)); cx.foreground().run_until_parked(); - window.simulate_prompt_answer(2, cx); // cancel + window.simulate_prompt_answer(2, cx); // cancel save all + cx.foreground().run_until_parked(); + window.simulate_prompt_answer(2, cx); // cancel save all cx.foreground().run_until_parked(); assert!(!window.has_pending_prompt(cx)); assert!(!task.await.unwrap()); @@ -4419,13 +4440,15 @@ mod tests { }); cx.foreground().run_until_parked(); + assert!(window.has_pending_prompt(cx)); + // Ignore "Save all" prompt + window.simulate_prompt_answer(2, cx); + cx.foreground().run_until_parked(); // There's a prompt to save item 1. pane.read_with(cx, |pane, _| { assert_eq!(pane.items_len(), 4); assert_eq!(pane.active_item().unwrap().id(), item1.id()); }); - assert!(window.has_pending_prompt(cx)); - // Confirm saving item 1. window.simulate_prompt_answer(0, cx); cx.foreground().run_until_parked(); @@ -4553,6 +4576,10 @@ mod tests { let close = left_pane.update(cx, |pane, cx| { pane.close_items(cx, SaveIntent::Close, move |_| true) }); + cx.foreground().run_until_parked(); + // Discard "Save all" prompt + window.simulate_prompt_answer(2, cx); + cx.foreground().run_until_parked(); left_pane.read_with(cx, |pane, cx| { assert_eq!(