From 66486870aaeaa1719ec18b9573e1b6d59a941c8c Mon Sep 17 00:00:00 2001 From: Keith Simmons Date: Thu, 30 Jun 2022 12:32:53 -0700 Subject: [PATCH] Fix vim editor focus selection issues, cancel vim operators on escape and unbound keys --- Cargo.lock | 1 + assets/keymaps/vim.json | 22 +++--- crates/search/src/buffer_search.rs | 2 +- crates/vim/Cargo.toml | 1 + crates/vim/src/editor_events.rs | 13 +++- crates/vim/src/normal.rs | 2 +- crates/vim/src/normal/delete.rs | 40 ++++++++++- crates/vim/src/state.rs | 27 +++++--- crates/vim/src/vim.rs | 104 +++++++++++++++++++++++++---- crates/vim/src/vim_test_context.rs | 26 +++++++- crates/zed/src/zed.rs | 1 + styles/package-lock.json | 1 + 12 files changed, 198 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 07e688c6b8..88e9dc731a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5768,6 +5768,7 @@ dependencies = [ "language", "log", "project", + "search", "serde", "settings", "util", diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 6fcd5d3d12..0d2a611d46 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -37,16 +37,12 @@ "ignorePunctuation": true } ], - "escape": [ - "vim::SwitchMode", - "Normal" - ] + "escape": "editor::Cancel" } }, { - "context": "Editor && vim_mode == normal", + "context": "Editor && vim_mode == normal && vim_operator == none", "bindings": { - "escape": "editor::Cancel", "c": [ "vim::PushOperator", "Change" @@ -92,7 +88,13 @@ "p": "vim::Paste", "u": "editor::Undo", "ctrl-r": "editor::Redo", - "ctrl-o": "pane::GoBack" + "ctrl-o": "pane::GoBack", + "/": [ + "buffer_search::Deploy", + { + "focus": true + } + ] } }, { @@ -146,11 +148,5 @@ "escape": "vim::NormalBefore", "ctrl-c": "vim::NormalBefore" } - }, - { - "context": "Editor && mode == singleline", - "bindings": { - "escape": "editor::Cancel" - } } ] \ No newline at end of file diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index de9e8af9c4..59f78cdc33 100644 --- a/crates/search/src/buffer_search.rs +++ b/crates/search/src/buffer_search.rs @@ -58,7 +58,7 @@ fn add_toggle_option_action(option: SearchOption, cx: &mut MutableApp } pub struct BufferSearchBar { - query_editor: ViewHandle, + pub query_editor: ViewHandle, active_editor: Option>, active_match_index: Option, active_editor_subscription: Option, diff --git a/crates/vim/Cargo.toml b/crates/vim/Cargo.toml index 56b0aec8cc..5bc32cd5bd 100644 --- a/crates/vim/Cargo.toml +++ b/crates/vim/Cargo.toml @@ -14,6 +14,7 @@ command_palette = { path = "../command_palette" } editor = { path = "../editor" } gpui = { path = "../gpui" } language = { path = "../language" } +search = { path = "../search" } serde = { version = "1.0", features = ["derive", "rc"] } settings = { path = "../settings" } workspace = { path = "../workspace" } diff --git a/crates/vim/src/editor_events.rs b/crates/vim/src/editor_events.rs index 6ad0cb77c4..c68e5182b0 100644 --- a/crates/vim/src/editor_events.rs +++ b/crates/vim/src/editor_events.rs @@ -29,8 +29,17 @@ fn editor_focused(EditorFocused(editor): &EditorFocused, cx: &mut MutableAppCont } })); - if editor.read(cx).mode() != EditorMode::Full { - vim.switch_mode(Mode::Insert, cx); + if !vim.enabled { + return; + } + + let editor = editor.read(cx); + if editor.selections.newest::(cx).is_empty() { + if editor.mode() != EditorMode::Full { + vim.switch_mode(Mode::Insert, cx); + } + } else { + vim.switch_mode(Mode::Visual { line: false }, cx); } }); } diff --git a/crates/vim/src/normal.rs b/crates/vim/src/normal.rs index 55c9779581..4c6dfd2d60 100644 --- a/crates/vim/src/normal.rs +++ b/crates/vim/src/normal.rs @@ -1165,7 +1165,7 @@ mod test { The quick brown fox [jump}s over the lazy dog"}, - Mode::Normal, + Mode::Visual { line: false }, ); cx.simulate_keystroke("y"); cx.set_state( diff --git a/crates/vim/src/normal/delete.rs b/crates/vim/src/normal/delete.rs index cea607e9f3..c5c823c79e 100644 --- a/crates/vim/src/normal/delete.rs +++ b/crates/vim/src/normal/delete.rs @@ -40,7 +40,7 @@ pub fn delete_over(vim: &mut Vim, motion: Motion, cx: &mut MutableAppContext) { mod test { use indoc::indoc; - use crate::vim_test_context::VimTestContext; + use crate::{state::Mode, vim_test_context::VimTestContext}; #[gpui::test] async fn test_delete_h(cx: &mut gpui::TestAppContext) { @@ -390,4 +390,42 @@ mod test { the lazy"}, ); } + + #[gpui::test] + async fn test_cancel_delete_operator(cx: &mut gpui::TestAppContext) { + let mut cx = VimTestContext::new(cx, true).await; + cx.set_state( + indoc! {" + The quick brown + fox ju|mps over + the lazy dog"}, + Mode::Normal, + ); + + // Canceling operator twice reverts to normal mode with no active operator + cx.simulate_keystrokes(["d", "escape", "k"]); + assert_eq!(cx.active_operator(), None); + assert_eq!(cx.mode(), Mode::Normal); + cx.assert_editor_state(indoc! {" + The qu|ick brown + fox jumps over + the lazy dog"}); + } + + #[gpui::test] + async fn test_unbound_command_cancels_pending_operator(cx: &mut gpui::TestAppContext) { + let mut cx = VimTestContext::new(cx, true).await; + cx.set_state( + indoc! {" + The quick brown + fox ju|mps over + the lazy dog"}, + Mode::Normal, + ); + + // Canceling operator twice reverts to normal mode with no active operator + cx.simulate_keystrokes(["d", "y"]); + assert_eq!(cx.active_operator(), None); + assert_eq!(cx.mode(), Mode::Normal); + } } diff --git a/crates/vim/src/state.rs b/crates/vim/src/state.rs index a08b8bd2d2..e36cb7203d 100644 --- a/crates/vim/src/state.rs +++ b/crates/vim/src/state.rs @@ -37,7 +37,14 @@ pub struct VimState { impl VimState { pub fn cursor_shape(&self) -> CursorShape { match self.mode { - Mode::Normal | Mode::Visual { .. } => CursorShape::Block, + Mode::Normal => { + if self.operator_stack.is_empty() { + CursorShape::Block + } else { + CursorShape::Underscore + } + } + Mode::Visual { .. } => CursorShape::Block, Mode::Insert => CursorShape::Bar, } } @@ -73,20 +80,20 @@ impl VimState { context.set.insert("VimControl".to_string()); } - if let Some(operator) = &self.operator_stack.last() { - operator.set_context(&mut context); - } + Operator::set_context(self.operator_stack.last(), &mut context); + context } } impl Operator { - pub fn set_context(&self, context: &mut Context) { - let operator_context = match self { - Operator::Namespace(Namespace::G) => "g", - Operator::Change => "c", - Operator::Delete => "d", - Operator::Yank => "y", + pub fn set_context(operator: Option<&Operator>, context: &mut Context) { + let operator_context = match operator { + Some(Operator::Namespace(Namespace::G)) => "g", + Some(Operator::Change) => "c", + Some(Operator::Delete) => "d", + Some(Operator::Yank) => "y", + None => "none", } .to_owned(); diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index 82567f9829..5655e51e29 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -11,7 +11,7 @@ mod visual; use collections::HashMap; use command_palette::CommandPaletteFilter; -use editor::{Bias, CursorShape, Editor, Input}; +use editor::{Bias, Cancel, CursorShape, Editor, Input}; use gpui::{impl_actions, MutableAppContext, Subscription, ViewContext, WeakViewHandle}; use serde::Deserialize; @@ -34,6 +34,7 @@ pub fn init(cx: &mut MutableAppContext) { insert::init(cx); motion::init(cx); + // Vim Actions cx.add_action(|_: &mut Workspace, &SwitchMode(mode): &SwitchMode, cx| { Vim::update(cx, |vim, cx| vim.switch_mode(mode, cx)) }); @@ -42,7 +43,11 @@ pub fn init(cx: &mut MutableAppContext) { Vim::update(cx, |vim, cx| vim.push_operator(operator, cx)) }, ); + + // Editor Actions cx.add_action(|_: &mut Editor, _: &Input, cx| { + // If we have an unbound input with an active operator, cancel that operator. Otherwise forward + // the input to the editor if Vim::read(cx).active_operator().is_some() { // Defer without updating editor MutableAppContext::defer(cx, |cx| Vim::update(cx, |vim, cx| vim.clear_operator(cx))) @@ -50,6 +55,20 @@ pub fn init(cx: &mut MutableAppContext) { cx.propagate_action() } }); + cx.add_action(|_: &mut Editor, _: &Cancel, cx| { + // If we are in a non normal mode or have an active operator, swap to normal mode + // Otherwise forward cancel on to the editor + let vim = Vim::read(cx); + if vim.state.mode != Mode::Normal || vim.active_operator().is_some() { + MutableAppContext::defer(cx, |cx| { + Vim::update(cx, |state, cx| { + state.switch_mode(Mode::Normal, cx); + }); + }); + } else { + cx.propagate_action(); + } + }); // Sync initial settings with the rest of the app Vim::update(cx, |state, cx| state.sync_vim_settings(cx)); @@ -97,9 +116,46 @@ impl Vim { } fn switch_mode(&mut self, mode: Mode, cx: &mut MutableAppContext) { + let previous_mode = self.state.mode; self.state.mode = mode; self.state.operator_stack.clear(); + + // Sync editor settings like clip mode self.sync_vim_settings(cx); + + // Adjust selections + for editor in self.editors.values() { + if let Some(editor) = editor.upgrade(cx) { + editor.update(cx, |editor, cx| { + editor.change_selections(None, cx, |s| { + s.move_with(|map, selection| { + // If empty selections + if self.state.empty_selections_only() { + let new_head = map.clip_point(selection.head(), Bias::Left); + selection.collapse_to(new_head, selection.goal) + } else { + if matches!(mode, Mode::Visual { line: false }) + && !matches!(previous_mode, Mode::Visual { .. }) + && !selection.reversed + && !selection.is_empty() + { + // Mode wasn't visual mode before, but is now. We need to move the end + // back by one character so that the region to be modifed stays the same + *selection.end.column_mut() = + selection.end.column().saturating_sub(1); + selection.end = map.clip_point(selection.end, Bias::Left); + } + + selection.set_head( + map.clip_point(selection.head(), Bias::Left), + selection.goal, + ); + } + }); + }) + }) + } + } } fn push_operator(&mut self, operator: Operator, cx: &mut MutableAppContext) { @@ -127,7 +183,7 @@ impl Vim { self.enabled = enabled; self.state = Default::default(); if enabled { - self.state.mode = Mode::Normal; + self.switch_mode(Mode::Normal, cx); } self.sync_vim_settings(cx); } @@ -156,17 +212,6 @@ impl Vim { matches!(state.mode, Mode::Visual { line: true }); let context_layer = state.keymap_context_layer(); editor.set_keymap_context_layer::(context_layer); - editor.change_selections(None, cx, |s| { - s.move_with(|map, selection| { - selection.set_head( - map.clip_point(selection.head(), Bias::Left), - selection.goal, - ); - if state.empty_selections_only() { - selection.collapse_to(selection.head(), selection.goal) - } - }); - }) } else { editor.set_cursor_shape(CursorShape::Bar, cx); editor.set_clip_at_line_ends(false, cx); @@ -182,6 +227,9 @@ impl Vim { #[cfg(test)] mod test { + use indoc::indoc; + use search::BufferSearchBar; + use crate::{state::Mode, vim_test_context::VimTestContext}; #[gpui::test] @@ -226,4 +274,34 @@ mod test { cx.enable_vim(); assert_eq!(cx.mode(), Mode::Normal); } + + #[gpui::test] + async fn test_buffer_search_switches_mode(cx: &mut gpui::TestAppContext) { + let mut cx = VimTestContext::new(cx, true).await; + + cx.set_state( + indoc! {" + The quick brown + fox ju|mps over + the lazy dog"}, + Mode::Normal, + ); + cx.simulate_keystroke("/"); + + assert_eq!(cx.mode(), Mode::Visual { line: false }); + + let search_bar = cx.workspace(|workspace, cx| { + workspace + .active_pane() + .read(cx) + .toolbar() + .read(cx) + .item_of_type::() + .expect("Buffer search bar should be deployed") + }); + + search_bar.read_with(cx.cx, |bar, cx| { + assert_eq!(bar.query_editor.read(cx).text(cx), "jumps"); + }) + } } diff --git a/crates/vim/src/vim_test_context.rs b/crates/vim/src/vim_test_context.rs index 52d4778b38..57d0174703 100644 --- a/crates/vim/src/vim_test_context.rs +++ b/crates/vim/src/vim_test_context.rs @@ -1,14 +1,16 @@ use std::ops::{Deref, DerefMut}; use editor::test::EditorTestContext; -use gpui::json::json; +use gpui::{json::json, AppContext, ViewHandle}; use project::Project; +use search::{BufferSearchBar, ProjectSearchBar}; use workspace::{pane, AppState, WorkspaceHandle}; use crate::{state::Operator, *}; pub struct VimTestContext<'a> { cx: EditorTestContext<'a>, + workspace: ViewHandle, } impl<'a> VimTestContext<'a> { @@ -16,6 +18,7 @@ impl<'a> VimTestContext<'a> { cx.update(|cx| { editor::init(cx); pane::init(cx); + search::init(cx); crate::init(cx); settings::KeymapFileContent::load("keymaps/vim.json", cx).unwrap(); @@ -37,6 +40,19 @@ impl<'a> VimTestContext<'a> { .await; let (window_id, workspace) = cx.add_window(|cx| Workspace::new(project.clone(), cx)); + + // Setup search toolbars + workspace.update(cx, |workspace, cx| { + workspace.active_pane().update(cx, |pane, cx| { + pane.toolbar().update(cx, |toolbar, cx| { + let buffer_search_bar = cx.add_view(|cx| BufferSearchBar::new(cx)); + toolbar.add_item(buffer_search_bar, cx); + let project_search_bar = cx.add_view(|_| ProjectSearchBar::new()); + toolbar.add_item(project_search_bar, cx); + }) + }); + }); + project .update(cx, |project, cx| { project.find_or_create_local_worktree("/root", true, cx) @@ -64,9 +80,17 @@ impl<'a> VimTestContext<'a> { window_id, editor, }, + workspace, } } + pub fn workspace(&mut self, read: F) -> T + where + F: FnOnce(&Workspace, &AppContext) -> T, + { + self.workspace.read_with(self.cx.cx, read) + } + pub fn enable_vim(&mut self) { self.cx.update(|cx| { cx.update_global(|settings: &mut Settings, _| { diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 7240aaef2f..548b726af9 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -97,6 +97,7 @@ pub fn init(app_state: &Arc, cx: &mut gpui::MutableAppContext) { cx.add_action({ let app_state = app_state.clone(); move |_: &mut Workspace, _: &OpenSettings, cx: &mut ViewContext| { + println!("open settings"); open_config_file(&SETTINGS_PATH, app_state.clone(), cx); } }); diff --git a/styles/package-lock.json b/styles/package-lock.json index 2eb6d3a1bf..49304dc2fa 100644 --- a/styles/package-lock.json +++ b/styles/package-lock.json @@ -5,6 +5,7 @@ "requires": true, "packages": { "": { + "name": "styles", "version": "1.0.0", "license": "ISC", "dependencies": {