refactor: Remove G/Z Namespace support

Conrad Irwin created 2 years ago

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.

Change summary

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(-)

Detailed changes

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": {

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));
     }

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;

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",

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" {