fix(vim/#2968): Cursor management in command-line mode (#3020)

__Issue:__ There were a couple related issues with the cursor in command-line mode:
1) When exiting command-line mode, the cursor could 'flash' at the top of the file
2) When running a search command, the viewport would not scroll to the first match

__Defect:__ Both issues stem from the fact that Onivim was ignoring editor-cursor-position changes during search. 

For the first issue, when exiting command-line mode, there would be a brief transitional state where the editor is focused in CommandLine mode, but no cursor would be available - so it would be rendered at the default (0,0) position. This would be rectified once the mode transition completed and the cursor position is reset. 

For the second issue, `libvim` was properly updating the cursor position when searching, but that cursor position was being ignored.

__Fix:__ Track the editor cursor position during the course of the command-line interaction

Fixes #2968 
Related #1551

CHANGES_CURRENT.md

@@ -7,6 +7,7 @@
 - #3011 - Vim: Visual Block - Handle 'I' and 'A' in visual block mode (fixes #1633)
 - #3016 - Extensions: Fix memory leak in extension host language features (fixes #3009)
 - #3019 - Extensions: Fix activation error for Ionide.Ionide-fsharp extension (fixes #2974)
+- #3020 - Vim: Fix incsearch cursor movement (fixes #2968)
 
 ### Performance
 

integration_test/RegressionCommandLineNoCompletionTest.re

@@ -10,7 +10,7 @@ runTest(
   dispatch(KeyboardInput({isText: true, input: ":"}));
 
   wait(~name="Mode switches to command line", (state: State.t) =>
-    Selectors.mode(state) == Vim.Mode.CommandLine
+    Vim.Mode.isCommandLine(Selectors.mode(state))
   );
 
   dispatch(KeyboardInput({isText: true, input: "e"}));

integration_test/VimIncsearchScrollTest.re

@@ -0,0 +1,65 @@
+open Oni_Core;
+open Oni_Model;
+open Oni_IntegrationTestLib;
+
+runTest(~name="VimIncsearchScrollTest", ({dispatch, wait, input, key, _}) => {
+  wait(~name="Initial mode is normal", (state: State.t) =>
+    Selectors.mode(state) |> Vim.Mode.isNormal
+  );
+
+  let largeCFile = getAssetPath("large-c-file.c");
+  dispatch(Actions.OpenFileByPath(largeCFile, None, None));
+
+  // Wait for file to load
+  wait(
+    ~timeout=30.0,
+    ~name="Validate buffer is loaded",
+    (state: State.t) => {
+      let fileType =
+        Selectors.getActiveBuffer(state)
+        |> Option.map(Buffer.getFileType)
+        |> Option.map(Buffer.FileType.toString);
+
+      fileType == Some("c");
+    },
+  );
+
+  // Start searching in the file:
+  input("/");
+
+  // Validate we're now in command line
+  wait(~name="Mode switches to command line", (state: State.t) => {
+    Vim.Mode.isCommandLine(Selectors.mode(state))
+  });
+
+  // Search for something far down, that should trigger a scroll
+  // "xor" is on line 983...
+  input("\"xor");
+
+  // Validate editor has scrolled
+  wait(~name="Validate editor has scrolled some distance", (state: State.t) => {
+    let scrollY =
+      state.layout
+      |> Feature_Layout.activeEditor
+      |> Feature_Editor.Editor.scrollY;
+
+    scrollY > 100.;
+  });
+
+  // Cancel search, we should scroll back up...
+  key(EditorInput.Key.Escape);
+
+  // Validate editor has scrolled back to top
+  wait(
+    ~name="Validate editor is back to start",
+    ~timeout=30.,
+    (state: State.t) => {
+      let scrollY =
+        state.layout
+        |> Feature_Layout.activeEditor
+        |> Feature_Editor.Editor.scrollY;
+
+      scrollY < 1.;
+    },
+  );
+});

integration_test/VimlRemapCmdlineTest.re

@@ -16,7 +16,7 @@ runTest(~name="Viml Remap ö -> :", ({dispatch, wait, runEffects, input, _}) =>
   input("ö");
 
   wait(~name="Mode switches to command line", (state: State.t) => {
-    Selectors.mode(state) == Vim.Mode.CommandLine
+    Vim.Mode.isCommandLine(Selectors.mode(state))
   });
 
   input("e");

integration_test/dune

@@ -22,8 +22,9 @@
    SCMGitTest SyntaxHighlightPhpTest SyntaxHighlightTextMateTest
    SyntaxHighlightTreesitterTest AddRemoveSplitTest TerminalSetPidTitleTest
    TerminalConfigurationTest TypingBatchedTest TypingUnbatchedTest
-   VimSimpleRemapTest VimlRemapCmdlineTest ClipboardChangeTest
-   VimScriptLocalFunctionTest ZenModeSingleFileModeTest ZenModeSplitTest)
+   VimIncsearchScrollTest VimSimpleRemapTest VimlRemapCmdlineTest
+   ClipboardChangeTest VimScriptLocalFunctionTest ZenModeSingleFileModeTest
+   ZenModeSplitTest)
  (libraries Oni_CLI Oni_IntegrationTestLib reason-native-crash-utils.asan))
 
 (install
@@ -58,8 +59,9 @@
    SyntaxServerParentPidCorrectTest.exe SyntaxServerReadExceptionTest.exe
    TerminalSetPidTitleTest.exe TerminalConfigurationTest.exe
    AddRemoveSplitTest.exe TypingBatchedTest.exe TypingUnbatchedTest.exe
-   VimScriptLocalFunctionTest.exe VimSimpleRemapTest.exe
-   VimlRemapCmdlineTest.exe ZenModeSingleFileModeTest.exe
-   ZenModeSplitTest.exe ClipboardChangeTest.exe large-c-file.c lsan.supp
-   some-test-file.json some-test-file.txt test.crlf test.lf test-syntax.php
-   utf8.txt utf8-test-file.htm Inconsolata-Regular.ttf PlugScriptLocal.vim))
+   VimIncsearchScrollTest.exe VimScriptLocalFunctionTest.exe
+   VimSimpleRemapTest.exe VimlRemapCmdlineTest.exe
+   ZenModeSingleFileModeTest.exe ZenModeSplitTest.exe ClipboardChangeTest.exe
+   large-c-file.c lsan.supp some-test-file.json some-test-file.txt test.crlf
+   test.lf test-syntax.php utf8.txt utf8-test-file.htm
+   Inconsolata-Regular.ttf PlugScriptLocal.vim))

src/Feature/Editor/Editor.re

@@ -1328,7 +1328,8 @@ let mapCursor = (~f, editor) => {
     // When scrolling in operator pending, cancel the pending operator
     | Operator({cursor, _}) => Vim.Mode.Normal({cursor: f(cursor)})
     // Don't do anything for command line mode
-    | CommandLine => CommandLine
+    | CommandLine({cursor, text, commandCursor, commandType}) =>
+      CommandLine({text, commandCursor, commandType, cursor: f(cursor)})
     | Normal({cursor}) => Normal({cursor: f(cursor)})
     | Visual(curr) =>
       Visual(Vim.VisualRange.{...curr, cursor: f(curr.cursor)})

src/Model/ModeManager.re

@@ -6,7 +6,7 @@ module Internal = {
     | Vim.Mode.Visual(range) =>
       Mode.TerminalVisual({range: Oni_Core.VisualRange.ofVim(range)})
     | Vim.Mode.Operator({pending, _}) => Mode.Operator({pending: pending})
-    | Vim.Mode.CommandLine => Mode.CommandLine
+    | Vim.Mode.CommandLine(_) => Mode.CommandLine
     | _ => Mode.TerminalNormal;
 };
 
@@ -44,7 +44,7 @@ let current: State.t => Oni_Core.Mode.t =
            | Vim.Mode.Replace({cursor}) => Mode.Replace({cursor: cursor})
            | Vim.Mode.Operator({pending, _}) =>
              Mode.Operator({pending: pending})
-           | Vim.Mode.CommandLine => Mode.CommandLine
+           | Vim.Mode.CommandLine(_) => Mode.CommandLine
            },
        );
   };

src/Service/Vim/Service_Vim.re

@@ -31,7 +31,7 @@ let quitAll = () =>
 module Effects = {
   let paste = (~toMsg, text) => {
     Isolinear.Effect.createWithDispatch(~name="vim.clipboardPaste", dispatch => {
-      let isCmdLineMode = Vim.Mode.current() == Vim.Mode.CommandLine;
+      let isCmdLineMode = Vim.Mode.isCommandLine(Vim.Mode.current());
       let isInsertMode = Vim.Mode.isInsert(Vim.Mode.current());
 
       if (isInsertMode || isCmdLineMode) {
@@ -123,7 +123,13 @@ module Effects = {
         })
       | Replace({cursor}) =>
         Replace({cursor: adjustBytePositionForEdit(cursor, edit)})
-      | CommandLine => CommandLine
+      | CommandLine({commandType, text, commandCursor, cursor}) =>
+        CommandLine({
+          commandType,
+          text,
+          commandCursor,
+          cursor: adjustBytePositionForEdit(cursor, edit),
+        })
       | Operator({cursor, pending}) =>
         Operator({cursor: adjustBytePositionForEdit(cursor, edit), pending})
       | Visual(_) as vis => vis

src/Store/VimStoreConnector.re

@@ -486,56 +486,45 @@ let start =
   let lastCompletionMeet = ref(None);
   let isCompleting = ref(false);
 
-  let checkCommandLineCompletions = () => {
+  let checkCommandLineCompletions = (~text: string, ~position: int) => {
     Log.debug("checkCommandLineCompletions");
 
-    let position = Vim.CommandLine.getPosition();
-    Vim.CommandLine.getText()
-    |> Option.iter(commandStr =>
-         if (position == String.length(commandStr)
-             && !StringEx.isEmpty(commandStr)) {
-           let context = Oni_Model.VimContext.current(getState());
-           let completions = Vim.CommandLine.getCompletions(~context, ());
-
-           Log.debugf(m =>
-             m("  got %n completions.", Array.length(completions))
-           );
-
-           let items =
-             Array.map(
-               name =>
-                 Actions.{
-                   name,
-                   category: None,
-                   icon: None,
-                   command: () => Noop,
-                   highlight: [],
-                   handle: None,
-                 },
-               completions,
-             );
+    if (position == String.length(text) && !StringEx.isEmpty(text)) {
+      let context = Oni_Model.VimContext.current(getState());
+      let completions = Vim.CommandLine.getCompletions(~context, ());
+
+      Log.debugf(m => m("  got %n completions.", Array.length(completions)));
+
+      let items =
+        Array.map(
+          name =>
+            Actions.{
+              name,
+              category: None,
+              icon: None,
+              command: () => Noop,
+              highlight: [],
+              handle: None,
+            },
+          completions,
+        );
 
-           dispatch(Actions.QuickmenuUpdateFilterProgress(items, Complete));
-         }
-       );
+      dispatch(Actions.QuickmenuUpdateFilterProgress(items, Complete));
+    };
   };
 
   let _: unit => unit =
-    Vim.CommandLine.onUpdate(({text, position: cursorPosition, _}) => {
+    Vim.CommandLine.onUpdate(({text, position: cursorPosition, cmdType}) => {
       dispatch(Actions.QuickmenuCommandlineUpdated(text, cursorPosition));
 
-      let cmdlineType = Vim.CommandLine.getType();
+      let cmdlineType = cmdType;
       switch (cmdlineType) {
       | Ex =>
-        let text =
-          switch (Vim.CommandLine.getText()) {
-          | Some(v) => v
-          | None => ""
-          };
         let meet = Feature_Vim.CommandLine.getCompletionMeet(text);
         lastCompletionMeet := meet;
 
-        isCompleting^ ? () : checkCommandLineCompletions();
+        isCompleting^
+          ? () : checkCommandLineCompletions(~position=cursorPosition, ~text);
 
       | SearchForward
       | SearchReverse =>

src/reason-libvim/CommandLine.re

@@ -7,12 +7,8 @@ let getCompletions = (~context: Context.t=Context.current(), ()) => {
   completions;
 };
 
-let getText = Native.vimCommandLineGetText;
-
 let getPosition = Native.vimCommandLineGetPosition;
 
-let getType = Native.vimCommandLineGetType;
-
 let onEnter = f => Event.add(f, Listeners.commandLineEnter);
 let onLeave = f => Event.add(f, Listeners.commandLineLeave);
 let onUpdate = f => Event.add(f, Listeners.commandLineUpdate);

src/reason-libvim/Mode.re

@@ -3,7 +3,12 @@ open EditorCoreTypes;
 type t =
   | Normal({cursor: BytePosition.t})
   | Insert({cursors: list(BytePosition.t)})
-  | CommandLine
+  | CommandLine({
+      text: string,
+      commandCursor: ByteIndex.t,
+      commandType: Types.cmdlineType,
+      cursor: BytePosition.t,
+    })
   | Replace({cursor: BytePosition.t})
   | Visual(VisualRange.t)
   | Operator({
@@ -16,7 +21,7 @@ let show = (mode: t) => {
   switch (mode) {
   | Normal(_) => "Normal"
   | Visual(_) => "Visual"
-  | CommandLine => "CommandLine"
+  | CommandLine(_) => "CommandLine"
   | Replace(_) => "Replace"
   | Operator(_) => "Operator"
   | Insert(_) => "Insert"
@@ -32,7 +37,7 @@ let cursors =
   | Visual(range) => [range |> VisualRange.cursor]
   | Operator({cursor, _}) => [cursor]
   | Select(range) => [range |> VisualRange.cursor]
-  | CommandLine => [];
+  | CommandLine({cursor, _}) => [cursor];
 
 let current = () => {
   let nativeMode: Native.mode = Native.vimGetMode();
@@ -41,7 +46,11 @@ let current = () => {
   switch (nativeMode) {
   | Native.Normal => Normal({cursor: cursor})
   | Native.Visual => Visual(VisualRange.current())
-  | Native.CommandLine => CommandLine
+  | Native.CommandLine =>
+    let commandCursor = Native.vimCommandLineGetPosition() |> ByteIndex.ofInt;
+    let commandType = Native.vimCommandLineGetType();
+    let text = Native.vimCommandLineGetText() |> Option.value(~default="");
+    CommandLine({cursor, commandCursor, commandType, text});
   | Native.Replace => Replace({cursor: cursor})
   | Native.Operator =>
     Operator({
@@ -68,6 +77,11 @@ let isInsert =
   | Insert(_) => true
   | _ => false;
 
+let isCommandLine =
+  fun
+  | CommandLine(_) => true
+  | _ => false;
+
 let isNormal =
   fun
   | Normal(_) => true
@@ -139,7 +153,7 @@ let trySet = newMode => {
   // These modes cannot be explicitly transitioned to currently
   | Operator(_)
   | Replace(_)
-  | CommandLine => ()
+  | CommandLine(_) => ()
   };
 
   current();

src/reason-libvim/Vim.re

@@ -206,16 +206,16 @@ let runWith = (~context: Context.t, f) => {
 
   BufferInternal.checkCurrentBufferForUpdate();
 
-  if (mode != prevMode) {
-    if (mode == CommandLine) {
+  if (!Mode.isCommandLine(mode) || !Mode.isCommandLine(prevMode)) {
+    if (Mode.isCommandLine(mode)) {
       Event.dispatch(
         CommandLineInternal.getState(),
         Listeners.commandLineEnter,
       );
-    } else if (prevMode == CommandLine) {
+    } else if (Mode.isCommandLine(prevMode)) {
       Event.dispatch((), Listeners.commandLineLeave);
     };
-  } else if (mode == CommandLine) {
+  } else if (Mode.isCommandLine(mode)) {
     Event.dispatch(
       CommandLineInternal.getState(),
       Listeners.commandLineUpdate,

src/reason-libvim/Vim.rei

@@ -102,7 +102,12 @@ module Mode: {
   type t =
     | Normal({cursor: BytePosition.t})
     | Insert({cursors: list(BytePosition.t)})
-    | CommandLine
+    | CommandLine({
+        text: string,
+        commandCursor: ByteIndex.t,
+        commandType: Types.cmdlineType,
+        cursor: BytePosition.t,
+      })
     | Replace({cursor: BytePosition.t})
     | Visual(VisualRange.t)
     | Operator({
@@ -113,6 +118,7 @@ module Mode: {
 
   let current: unit => t;
 
+  let isCommandLine: t => bool;
   let isInsert: t => bool;
   let isNormal: t => bool;
   let isVisual: t => bool;
@@ -191,9 +197,8 @@ module CommandLine: {
   type t = Types.cmdline;
 
   let getCompletions: (~context: Context.t=?, unit) => array(string);
-  let getText: unit => option(string);
+
   let getPosition: unit => int;
-  let getType: unit => Types.cmdlineType;
 
   let onEnter: (Event.handler(t), unit) => unit;
   let onLeave: (Event.handler(unit), unit) => unit;