Repository: vscode-neovim/vscode-neovim
Pull request #1973 (2024-05-17 15:12:41)

fix: reduce text jumpiness when using the substitute command
Partially addresses #329 

Both nvim and vscode-neovim support incremental highlights while doing a substitute command (e.g. `:%s/partia`). However, vscode-neovim's support was a bit jumpy. With this change, things are much improved, but we don't avoid the initial scroll to the top (some more details can be found in my comments on that issue). 

Part of the reason for the jumpiness is every time a user types a character in the command, we completely rewrite what's in the command box. We can solve the common case by simply looking to see if the end of the text was changed, which should reduce jumpiness during simple typing

Before (I am typing `%s/poke` while the first instance is out of viewport range):

https://github.com/vscode-neovim/vscode-neovim/assets/977151/20c0344a-ad67-48d3-ac07-efb57aff2fde

After:

https://github.com/vscode-neovim/vscode-neovim/assets/977151/9a73696d-53ff-4f79-9421-d75b18850d50



xiyaowong: ![image](https://github.com/vscode-neovim/vscode-neovim/assets/47070852/aab49de1-b4f3-4bcd-a16d-5f2579e60d0e)
FYI, the GitHub package mirror source has not yet been synchronized to NVIM 0.10.0, so it will fail. 
ollien: Does this mean we have tests we expect to fail on 0.9?
xiyaowong: Yes, we always use the latest stable version of Neovim. Test failure does not mean that related functions cannot be used on older versions
xiyaowong: Yes, the failed test case was fixed in NVIM 0.10.0. https://github.com/vscode-neovim/vscode-neovim/pull/1968

Patch:
diff --git a/eslint.config.cjs b/eslint.config.cjs
index 4ad6327..569f2b4 100644
--- a/eslint.config.cjs
+++ b/eslint.config.cjs
@@ -1,5 +1,3 @@
-/* eslint-env es2019 */
-
 const eslint = require("@eslint/js");
 const tseslint = require("typescript-eslint");
 const eslintPluginPrettierRecommended = require("eslint-plugin-prettier/recommended");
@@ -8,11 +6,12 @@ const globals = require("globals");
 
 module.exports = tseslint.config(
     eslint.configs.recommended,
-    ...tseslint.configs.recommended,
+    ...tseslint.configs.recommendedTypeChecked,
     {
         plugins: {
             import: importsPlugin,
         },
+        ignores: ["eslint.config.cjs"],
         languageOptions: {
             ecmaVersion: 2019, // Allows for the parsing of modern ECMAScript features
             sourceType: "module", // Allows for the use of imports
@@ -20,6 +19,13 @@ module.exports = tseslint.config(
                 ...globals.node,
                 ...globals.es6,
             },
+            parserOptions: {
+                project: true,
+                tsconfigRootDir: __dirname,
+            },
+        },
+        linterOptions: {
+            reportUnusedDisableDirectives: "error",
         },
         rules: {
             // Place to specify ESLint rules. Can be used to overwrite rules specified from the extended configs
@@ -65,6 +71,16 @@ module.exports = tseslint.config(
             "@typescript-eslint/explicit-function-return-type": "off",
             "@typescript-eslint/no-non-null-assertion": "off",
             "@typescript-eslint/no-explicit-any": "off",
+            "@typescript-eslint/restrict-template-expressions": "off", // too jumpy
+            "@typescript-eslint/no-misused-promises": "off", // too jumpy
+            "@typescript-eslint/no-floating-promises": "off", // jumpy; would be nice to turn on, but we have a lot of these
+            "@typescript-eslint/unbound-method": "off", // jumpy, given how vscode's API binds this. Would be good to remove.
+
+            // This codebase interacts with `any` a lot, so for now, all no-unsafe-* we violate are currently disabled
+            "@typescript-eslint/no-unsafe-argument": "off",
+            "@typescript-eslint/no-unsafe-assignment": "off",
+            "@typescript-eslint/no-unsafe-member-access": "off",
+            "@typescript-eslint/no-unsafe-return": "off",
         },
     },
     {
diff --git a/src/actions_eval.ts b/src/actions_eval.ts
index 34ec78b..1256252 100644
--- a/src/actions_eval.ts
+++ b/src/actions_eval.ts
@@ -24,7 +24,8 @@ void logger;
 export async function eval_for_client(code: string, args: any): Promise<any> {
     void args;
 
-    const result = await eval("async () => {" + code + "}")();
+    const func: () => Promise<any> = eval("async () => {" + code + "}");
+    const result = await func();
 
     let data: string | undefined;
     try {
diff --git a/src/command_line.ts b/src/cmdline/cmdline_controller.ts
similarity index 94%
rename from src/command_line.ts
rename to src/cmdline/cmdline_controller.ts
index dd0ba84..03680ff 100644
--- a/src/command_line.ts
+++ b/src/cmdline/cmdline_controller.ts
@@ -1,9 +1,11 @@
 import { NeovimClient } from "neovim";
 import { Disposable, QuickPick, QuickPickItem, commands, window } from "vscode";
 
-import { GlyphChars } from "./constants";
-import { createLogger } from "./logger";
-import { VSCodeContext, disposeAll } from "./utils";
+import { GlyphChars } from "../constants";
+import { createLogger } from "../logger";
+import { VSCodeContext, disposeAll } from "../utils";
+
+import { commandInputIsCompletable } from "./cmdline_text";
 
 const logger = createLogger("CmdLine");
 
@@ -135,16 +137,7 @@ export class CommandLineController implements Disposable {
             this.input.items = [];
             this.completionItems = [];
         }
-        const useCompletion =
-            mode === ":" &&
-            e.charAt(0) !== "?" &&
-            e.charAt(0) !== "/" &&
-            !e.includes("s/") &&
-            !e.includes("substitute/") &&
-            !e.includes("g/") &&
-            !e.includes("global/") &&
-            !e.includes("v/") &&
-            !e.includes("vglobal/");
+        const useCompletion = mode === ":" && commandInputIsCompletable(e);
         if (!useCompletion) {
             this.cancelCompletions();
         }
diff --git a/src/cmdline/cmdline_text.ts b/src/cmdline/cmdline_text.ts
new file mode 100644
index 0000000..048bd89
--- /dev/null
+++ b/src/cmdline/cmdline_text.ts
@@ -0,0 +1,59 @@
+import { normalizeInputString } from "../utils/text";
+
+type TextChange = NoTextChange | TextEndChange | OtherTextChange;
+
+interface TextEndChange {
+    action: "added" | "removed";
+    char: string;
+}
+
+interface NoTextChange {
+    action: "none";
+}
+
+interface OtherTextChange {
+    action: "other";
+}
+
+export function calculateInputAfterTextChange(oldText: string, newText: string): string {
+    const change = diffLineText(oldText, newText);
+    switch (change.action) {
+        case "added":
+            return normalizeInputString(change.char);
+        case "removed":
+            return "<BS>";
+        case "none":
+            // If no change, type nothing.
+            return "";
+        case "other":
+            // Rewrite the line if it's not a simple change
+            return `<C-u>${normalizeInputString(newText)}`;
+    }
+}
+
+export function commandInputIsCompletable(command: string): boolean {
+    return (
+        command.charAt(0) !== "?" &&
+        command.charAt(0) !== "/" &&
+        !command.includes("s/") &&
+        !command.includes("substitute/") &&
+        !command.includes("g/") &&
+        !command.includes("global/") &&
+        !command.includes("v/") &&
+        !command.includes("vglobal/")
+    );
+}
+
+function diffLineText(oldLine: string, newLine: string): TextChange {
+    const lengthDifference = newLine.length - oldLine.length;
+
+    if (oldLine === newLine) {
+        return { action: "none" };
+    } else if (lengthDifference === 1 && oldLine + newLine[newLine.length - 1] === newLine) {
+        return { action: "added", char: newLine[newLine.length - 1] };
+    } else if (lengthDifference === -1 && oldLine.substring(0, oldLine.length - 1) === newLine) {
+        return { action: "removed", char: oldLine[oldLine.length - 1] };
+    } else {
+        return { action: "other" };
+    }
+}
diff --git a/src/command_line_manager.ts b/src/cmdline_manager.ts
similarity index 87%
rename from src/command_line_manager.ts
rename to src/cmdline_manager.ts
index 7a8ec3e..8ef5d46 100644
--- a/src/command_line_manager.ts
+++ b/src/cmdline_manager.ts
@@ -1,10 +1,11 @@
 import { Disposable } from "vscode";
 
-import { CommandLineController } from "./command_line";
+import { CommandLineController } from "./cmdline/cmdline_controller";
 import { config } from "./config";
 import { EventBusData, eventBus } from "./eventBus";
 import { MainController } from "./main_controller";
-import { disposeAll, normalizeInputString } from "./utils";
+import { disposeAll } from "./utils";
+import { calculateInputAfterTextChange } from "./cmdline/cmdline_text";
 
 export class CommandLineManager implements Disposable {
     private disposables: Disposable[] = [];
@@ -17,6 +18,11 @@ export class CommandLineManager implements Disposable {
      */
     private cmdlineTimer?: NodeJS.Timeout;
 
+    /**
+     * The last text typed in the UI, used to calculate changes
+     */
+    private lastTypedText: string = "";
+
     private get client() {
         return this.main.client;
     }
@@ -80,6 +86,8 @@ export class CommandLineManager implements Disposable {
     }
 
     private showCmd = (content: string, firstc: string, prompt: string): void => {
+        this.lastTypedText = content;
+
         if (!this.commandLine) {
             this.commandLine = new CommandLineController(
                 this.client,
@@ -99,12 +107,15 @@ export class CommandLineManager implements Disposable {
         this.cmdlineTimer = undefined;
     };
 
-    private onCmdChange = async (e: string, complete: boolean): Promise<void> => {
-        let keys = "<C-u>" + normalizeInputString(e);
+    private onCmdChange = async (text: string, complete: boolean): Promise<void> => {
+        let toType = calculateInputAfterTextChange(this.lastTypedText, text);
+        this.lastTypedText = text;
+
         if (complete) {
-            keys += "<C-e>";
+            toType += "<C-e>";
         }
-        await this.client.input(keys);
+
+        await this.client.input(toType);
     };
 
     private onCmdCancel = async (): Promise<void> => {
diff --git a/src/eventBus.ts b/src/eventBus.ts
index a2fb573..b5552bc 100644
--- a/src/eventBus.ts
+++ b/src/eventBus.ts
@@ -1,4 +1,3 @@
-/* eslint-disable @typescript-eslint/no-explicit-any */
 import neovim from "neovim";
 import { Disposable, EventEmitter } from "vscode";
 
diff --git a/src/extension.ts b/src/extension.ts
index 7c3eada..6c8fc44 100644
--- a/src/extension.ts
+++ b/src/extension.ts
@@ -61,7 +61,7 @@ export function deactivate(isRestart = false) {
     if (!isRestart) disposeAll(disposables);
 }
 
-async function verifyExperimentalAffinity(): Promise<void> {
+function verifyExperimentalAffinity(): void {
     const extensionsConfiguration = vscode.workspace.getConfiguration("extensions");
     const affinityConfiguration = extensionsConfiguration.inspect<{ [key: string]: [number] }>("experimental.affinity");
 
diff --git a/src/logger.ts b/src/logger.ts
index 1e31637..2064219 100644
--- a/src/logger.ts
+++ b/src/logger.ts
@@ -1,4 +1,3 @@
-/* eslint-disable @typescript-eslint/no-explicit-any */
 import fs from "fs";
 import { inspect } from "util";
 
diff --git a/src/main_controller.ts b/src/main_controller.ts
index 37ff77d..f3c90c8 100644
--- a/src/main_controller.ts
+++ b/src/main_controller.ts
@@ -8,7 +8,7 @@ import { transports as loggerTransports, createLogger as winstonCreateLogger } f
 
 import actions from "./actions";
 import { BufferManager } from "./buffer_manager";
-import { CommandLineManager } from "./command_line_manager";
+import { CommandLineManager } from "./cmdline_manager";
 import { CommandsController } from "./commands_controller";
 import { config } from "./config";
 import { NVIM_MIN_VERSION } from "./constants";
@@ -280,7 +280,7 @@ export class MainController implements vscode.Disposable {
     private onNeovimNotification = async (method: string, events: [string, ...any[]]) => {
         switch (method) {
             case "vscode-action": {
-                const action = events[0] as string;
+                const action = events[0];
                 let options = events[1] as VSCodeActionOptions | [];
                 if (Array.isArray(options)) options = {}; // empty lua table
 
@@ -353,7 +353,7 @@ export class MainController implements vscode.Disposable {
     ): Promise<void> => {
         switch (method) {
             case "vscode-action": {
-                const action = requestArgs[0] as string;
+                const action = requestArgs[0];
                 let options = requestArgs[1] as Omit<VSCodeActionOptions, "callback"> | [];
                 if (Array.isArray(options)) options = {}; // empty lua table
 
diff --git a/src/typing_manager.ts b/src/typing_manager.ts
index e6ff704..b58e21f 100644
--- a/src/typing_manager.ts
+++ b/src/typing_manager.ts
@@ -152,7 +152,7 @@ export class TypingManager implements Disposable {
         };
 
         this.takeOverVSCodeInput = true;
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+
         const registerCommand = (cmd: string, cb: (...args: any[]) => any) => {
             this.disposables.push(commands.registerCommand(cmd, cb, this));
         };
@@ -184,7 +184,7 @@ export class TypingManager implements Disposable {
         this.useCompositeKeys = this.compositeFirstKeys.length > 0;
     }
 
-    private onModeChange = async (): Promise<void> => {
+    private onModeChange = (): void => {
         if (this.main.modeManager.isInsertMode && this.takeOverVSCodeInput && !this.isRecordingInInsertMode) {
             const editor = window.activeTextEditor;
             const documentPromise = editor && this.main.changeManager.getDocumentChangeCompletionLock(editor.document);
@@ -218,11 +218,11 @@ export class TypingManager implements Disposable {
         }
     };
 
-    async compositeInput(key: string) {
+    compositeInput(key: string) {
         if (!this.compositeMatchedFirstKey) {
             if (this.compositeFirstKeys.includes(key)) {
                 this.compositeMatchedFirstKey = key;
-                this.compositeTimer = setTimeout(async () => {
+                this.compositeTimer = setTimeout(() => {
                     this.compositeTimer = undefined;
                     this.compositeMatchedFirstKey = undefined;
                     this.vscodeDefaultType(key);
diff --git a/src/viewport_manager.ts b/src/viewport_manager.ts
index f70aa12..16440d7 100644
--- a/src/viewport_manager.ts
+++ b/src/viewport_manager.ts
@@ -97,7 +97,7 @@ export class ViewportManager implements Disposable {
         return new Position(view.topline, view.leftcol);
     }
 
-    private async handleRedraw({ name, args }: EventBusData<"redraw">) {
+    private handleRedraw({ name, args }: EventBusData<"redraw">) {
         switch (name) {
             case "win_viewport": {
                 for (const [grid, , topline, botline, curline, curcol] of args) {
@@ -164,7 +164,7 @@ export class ViewportManager implements Disposable {
             trailing: true,
         });
     }
-    private onDidChangeVisibleRange = async (e: TextEditorVisibleRangesChangeEvent): Promise<void> => {
+    private onDidChangeVisibleRange = (e: TextEditorVisibleRangesChangeEvent) => {
         if (!this.debouncedScrollNeovim) {
             this.refreshDebounceTime();
             this.refreshDebounceScroll();


Test patch:
diff --git a/src/test/integ/extmark.test.ts b/src/test/integ/extmark.test.ts
index 4a46da4..372fffe 100644
--- a/src/test/integ/extmark.test.ts
+++ b/src/test/integ/extmark.test.ts
@@ -26,7 +26,7 @@ describe("Test ext mark", () => {
         await closeNvimClient(client);
     });
 
-    afterEach(async () => {
+    afterEach(() => {
         restoreWindow();
     });
 
diff --git a/src/test/integ/integrationUtils.ts b/src/test/integ/integrationUtils.ts
index 38bb09f..bb9ca78 100644
--- a/src/test/integ/integrationUtils.ts
+++ b/src/test/integ/integrationUtils.ts
@@ -49,13 +49,12 @@ export async function attachTestNvimClient(): Promise<NeovimClient> {
     const client = attach({ writer: conn, reader: conn });
     // wait for connection
     await client.channelId;
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+
     (client as any).testConn = conn;
     return client;
 }
 
 export async function closeNvimClient(client: NeovimClient): Promise<void> {
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
     const conn: net.Socket = (client as any).testConn;
 
     // Try to gracefully close the socket first, this prevents noisy errors if it works.
diff --git a/src/test/unit/cmdline/cmdline_text.test.ts b/src/test/unit/cmdline/cmdline_text.test.ts
new file mode 100644
index 0000000..f66c725
--- /dev/null
+++ b/src/test/unit/cmdline/cmdline_text.test.ts
@@ -0,0 +1,61 @@
+import { strict as assert } from "assert";
+
+import { calculateInputAfterTextChange, commandInputIsCompletable } from "../../../cmdline/cmdline_text";
+
+describe("calculateInputAfterTextChange", () => {
+    [
+        {
+            name: "should return an empty string if the text is unchanged",
+            oldText: "",
+            newText: "",
+            expected: "",
+        },
+        {
+            name: "should return the newly typed character if a single character is added to the end",
+            oldText: "%s/worl",
+            newText: "%s/world",
+            expected: "d",
+        },
+        {
+            name: "should return <BS> if a single character is removed from the end",
+            oldText: "%s/world",
+            newText: "%s/worl",
+            expected: "<BS>",
+        },
+        {
+            name: "should force the line to be rewritten if some other change happens",
+            oldText: "%s/hello",
+            newText: ".s/hello",
+            expected: "<C-u>.s/hello",
+        },
+    ].forEach(({ name, oldText, newText, expected }) => {
+        it(name, () => {
+            assert.equal(calculateInputAfterTextChange(oldText, newText), expected);
+        });
+    });
+});
+
+describe("commandInputIsCompletable", () => {
+    [
+        "substitute/blah",
+        "substitute/",
+        "s/blah",
+        "s/",
+        "g/blah",
+        "global/",
+        "v/",
+        "v/blah",
+        "vglobal/",
+        "vglobal/blah",
+    ].forEach((input) => {
+        it(`should not produce completions for user-input commands: '${input}'`, () => {
+            assert.equal(commandInputIsCompletable(input), false);
+        });
+    });
+
+    ["p", "Ins", "nno"].forEach((input) => {
+        it(`should produce completions for normal commands: '${input}'`, () => {
+            assert.equal(commandInputIsCompletable(input), true);
+        });
+    });
+});