From 8ba69c15d1f5014d31fccb8a69eb19cdc330c6ac Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Wed, 19 Jul 2023 18:54:29 -0600 Subject: [PATCH] refactor: Remove G/Z Namespace support This previously enabled things like `d g g` to work, but we can fix that instead by not clearing out pending vim state on change. Either way, it is unnecessary and causes some user-confusion (zed-industries/community#176), so remove this code for now; and use comments to organize the file a bit instead. --- assets/keymaps/vim.json | 67 ++++++++++++++-------------------------- crates/vim/src/motion.rs | 5 ++- crates/vim/src/normal.rs | 25 ++------------- crates/vim/src/state.rs | 9 ------ crates/vim/src/vim.rs | 7 +++-- 5 files changed, 34 insertions(+), 79 deletions(-) diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 7312c1e2f8ccfb0a8a4d193b6249f3a27137be14..5aa448e9d1ae74a719172fab084951beb61ee21c 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -2,12 +2,6 @@ { "context": "Editor && VimControl && !VimWaiting && !menu", "bindings": { - "g": [ - "vim::PushOperator", - { - "Namespace": "G" - } - ], "i": [ "vim::PushOperator", { @@ -110,6 +104,30 @@ "*": "vim::MoveToNext", "#": "vim::MoveToPrev", "0": "vim::StartOfLine", // When no number operator present, use start of line motion + // "g" commands + "g g": "vim::StartOfDocument", + "g h": "editor::Hover", + "g t": "pane::ActivateNextItem", + "g shift-t": "pane::ActivatePrevItem", + "g d": "editor::GoToDefinition", + "g shift-d": "editor::GoToTypeDefinition", + "g *": [ + "vim::MoveToNext", + { + "partialWord": true + } + ], + "g #": [ + "vim::MoveToPrev", + { + "partialWord": true + } + ], + // z commands + "z t": "editor::ScrollCursorTop", + "z z": "editor::ScrollCursorCenter", + "z b": "editor::ScrollCursorBottom", + // Count support "1": [ "vim::Number", 1 @@ -234,12 +252,6 @@ "vim::PushOperator", "Yank" ], - "z": [ - "vim::PushOperator", - { - "Namespace": "Z" - } - ], "i": [ "vim::SwitchMode", "Insert" @@ -306,29 +318,6 @@ ] } }, - { - "context": "Editor && vim_operator == g", - "bindings": { - "g": "vim::StartOfDocument", - "h": "editor::Hover", - "t": "pane::ActivateNextItem", - "shift-t": "pane::ActivatePrevItem", - "d": "editor::GoToDefinition", - "shift-d": "editor::GoToTypeDefinition", - "*": [ - "vim::MoveToNext", - { - "partialWord": true - } - ], - "#": [ - "vim::MoveToPrev", - { - "partialWord": true - } - ] - } - }, { "context": "Editor && vim_operator == c", "bindings": { @@ -347,14 +336,6 @@ "y": "vim::CurrentLine" } }, - { - "context": "Editor && vim_operator == z", - "bindings": { - "t": "editor::ScrollCursorTop", - "z": "editor::ScrollCursorCenter", - "b": "editor::ScrollCursorBottom" - } - }, { "context": "Editor && VimObject", "bindings": { diff --git a/crates/vim/src/motion.rs b/crates/vim/src/motion.rs index 07b095dd5e186412d566badc8cbdda5be637130b..fb742af3aba5036fd813889d9ee94851bb15d89d 100644 --- a/crates/vim/src/motion.rs +++ b/crates/vim/src/motion.rs @@ -127,9 +127,8 @@ pub fn init(cx: &mut AppContext) { } pub(crate) fn motion(motion: Motion, cx: &mut WindowContext) { - if let Some(Operator::Namespace(_)) - | Some(Operator::FindForward { .. }) - | Some(Operator::FindBackward { .. }) = Vim::read(cx).active_operator() + if let Some(Operator::FindForward { .. }) | Some(Operator::FindBackward { .. }) = + Vim::read(cx).active_operator() { Vim::update(cx, |vim, cx| vim.pop_operator(cx)); } diff --git a/crates/vim/src/normal.rs b/crates/vim/src/normal.rs index 8dcaa5008ef60f1e184ff2b251569d20a5b68943..e0765839a0827617dd9eefa6fb428b597c7a4e3f 100644 --- a/crates/vim/src/normal.rs +++ b/crates/vim/src/normal.rs @@ -107,7 +107,7 @@ pub fn normal_motion( Some(Operator::Delete) => delete_motion(vim, motion, times, cx), Some(Operator::Yank) => yank_motion(vim, motion, times, cx), Some(operator) => { - // Can't do anything for text objects or namespace operators. Ignoring + // Can't do anything for text objects, Ignoring error!("Unexpected normal mode motion operator: {:?}", operator) } } @@ -441,11 +441,8 @@ mod test { use indoc::indoc; use crate::{ - state::{ - Mode::{self, *}, - Namespace, Operator, - }, - test::{ExemptionFeatures, NeovimBackedTestContext, VimTestContext}, + state::Mode::{self, *}, + test::{ExemptionFeatures, NeovimBackedTestContext}, }; #[gpui::test] @@ -610,22 +607,6 @@ mod test { .await; } - #[gpui::test] - async fn test_g_prefix_and_abort(cx: &mut gpui::TestAppContext) { - let mut cx = VimTestContext::new(cx, true).await; - - // Can abort with escape to get back to normal mode - cx.simulate_keystroke("g"); - assert_eq!(cx.mode(), Normal); - assert_eq!( - cx.active_operator(), - Some(Operator::Namespace(Namespace::G)) - ); - cx.simulate_keystroke("escape"); - assert_eq!(cx.mode(), Normal); - assert_eq!(cx.active_operator(), None); - } - #[gpui::test] async fn test_gg(cx: &mut gpui::TestAppContext) { let mut cx = NeovimBackedTestContext::new(cx).await; diff --git a/crates/vim/src/state.rs b/crates/vim/src/state.rs index 23471066cdf8ff7da25aab039faa38997f98395b..bf644c08cc4940feae3e37f231e632ab375af997 100644 --- a/crates/vim/src/state.rs +++ b/crates/vim/src/state.rs @@ -16,16 +16,9 @@ impl Default for Mode { } } -#[derive(Copy, Clone, Debug, PartialEq, Eq, Deserialize)] -pub enum Namespace { - G, - Z, -} - #[derive(Copy, Clone, Debug, PartialEq, Eq, Deserialize)] pub enum Operator { Number(usize), - Namespace(Namespace), Change, Delete, Yank, @@ -126,8 +119,6 @@ impl Operator { pub fn id(&self) -> &'static str { match self { Operator::Number(_) => "n", - Operator::Namespace(Namespace::G) => "g", - Operator::Namespace(Namespace::Z) => "z", Operator::Object { around: false } => "i", Operator::Object { around: true } => "a", Operator::Change => "c", diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index 69b94428ddad7a3f4f96cf4ad40abcbecd5a3124..82d2e752c39c31b44a562b7fdd2d62b77377fa53 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -14,7 +14,7 @@ use anyhow::Result; use collections::CommandPaletteFilter; use editor::{Bias, Editor, EditorMode, Event}; use gpui::{ - actions, impl_actions, keymap_matcher::KeymapContext, AppContext, Subscription, ViewContext, + actions, impl_actions,keymap_matcher::MatchResult, keymap_matcher::KeymapContext, AppContext, Subscription, ViewContext, ViewHandle, WeakViewHandle, WindowContext, }; use language::CursorShape; @@ -90,7 +90,10 @@ pub fn init(cx: &mut AppContext) { } pub fn observe_keystrokes(cx: &mut WindowContext) { - cx.observe_keystrokes(|_keystroke, _result, handled_by, cx| { + cx.observe_keystrokes(|_keystroke, result, handled_by, cx| { + if result == &MatchResult::Pending { + return true; + } if let Some(handled_by) = handled_by { // Keystroke is handled by the vim system, so continue forward if handled_by.namespace() == "vim" {