fix race movePrimaryCursorIntoVisibleRange (#2011)

src/commands/move.ts

@@ -299,7 +299,7 @@ export class EndOfBuffer extends EmacsCommand {
   }
 }
 
-function movePrimaryCursorIntoVisibleRange(
+export function movePrimaryCursorIntoVisibleRange(
   textEditor: TextEditor,
   isInMarkMode: boolean,
   emacsController: IEmacsController,
@@ -351,12 +351,10 @@ export class ScrollUpCommand extends EmacsCommand {
     }
 
     if (Configuration.instance.strictEmacsMove) {
-      return vscode.commands
-        .executeCommand<void>("editorScroll", {
-          to: "down",
-          by: "page",
-        })
-        .then(() => movePrimaryCursorIntoVisibleRange(textEditor, isInMarkMode, this.emacsController));
+      return vscode.commands.executeCommand<void>("editorScroll", {
+        to: "down",
+        by: "page",
+      });
     } else {
       return vscode.commands.executeCommand<void>(isInMarkMode ? "cursorPageDownSelect" : "cursorPageDown");
     }
@@ -378,12 +376,10 @@ export class ScrollDownCommand extends EmacsCommand {
     }
 
     if (Configuration.instance.strictEmacsMove) {
-      return vscode.commands
-        .executeCommand<void>("editorScroll", {
-          to: "up",
-          by: "page",
-        })
-        .then(() => movePrimaryCursorIntoVisibleRange(textEditor, isInMarkMode, this.emacsController));
+      return vscode.commands.executeCommand<void>("editorScroll", {
+        to: "up",
+        by: "page",
+      });
     } else {
       return vscode.commands.executeCommand<void>(isInMarkMode ? "cursorPageUpSelect" : "cursorPageUp");
     }

src/emulator.ts

@@ -171,6 +171,16 @@ export class EmacsEmulator implements IEmacsController, vscode.Disposable {
     this.commandRegistry.register(new MoveCommands.EndOfBuffer(this));
     this.commandRegistry.register(new MoveCommands.ScrollUpCommand(this));
     this.commandRegistry.register(new MoveCommands.ScrollDownCommand(this));
+    vscode.window.onDidChangeTextEditorVisibleRanges(
+      () => {
+        if (Configuration.instance.strictEmacsMove) {
+          // Keep the primary cursor in the visible range when scrolling
+          MoveCommands.movePrimaryCursorIntoVisibleRange(this.textEditor, this.isInMarkMode, this);
+        }
+      },
+      this,
+      this.disposables,
+    );
     this.commandRegistry.register(new MoveCommands.ForwardParagraph(this));
     this.commandRegistry.register(new MoveCommands.BackwardParagraph(this));
     this.commandRegistry.register(new EditCommands.DeleteBackwardChar(this));

src/test/suite/commands/move.test.ts

@@ -301,6 +301,31 @@ suite("scroll-up/down-command", () => {
       );
       assertCursorsEqual(activeTextEditor, [activeTextEditor.visibleRanges[0]?.start.line as number, 0]);
     });
+
+    test("it scrolls with the specified number of lines by the prefix argument and moves the cursor if it goes outside the visible range, keeping the selection", async () => {
+      setEmptyCursors(activeTextEditor, [visibleRange.start.line, 0]); // This line will be outside the visible range after scrolling.
+
+      const initVisibleStartLine = visibleRange.start.line;
+      const initCursorPosition = activeTextEditor.selection.active;
+
+      emulator.setMarkCommand();
+
+      await emulator.universalArgument();
+      await emulator.subsequentArgumentDigit(12);
+      await emulator.runCommand("scrollUpCommand");
+
+      assert.equal(
+        activeTextEditor.visibleRanges[0]?.start.line,
+        initVisibleStartLine + 12,
+        "Expected the visibleRange has been scrolled 2 lines",
+      );
+      assertSelectionsEqual(activeTextEditor, [
+        initCursorPosition.line,
+        initCursorPosition.character,
+        activeTextEditor.visibleRanges[0]?.start.line as number,
+        0,
+      ]);
+    });
   });
 
   suite("scroll-down-command", () => {
@@ -366,6 +391,31 @@ suite("scroll-up/down-command", () => {
       );
       assertCursorsEqual(activeTextEditor, [activeTextEditor.visibleRanges[0]?.end.line as number, 0]);
     });
+
+    test("it scrolls with the specified number of lines by the prefix argument and moves the cursor if it goes outside the visible range, keeping the selection", async () => {
+      setEmptyCursors(activeTextEditor, [visibleRange.end.line, 0]); // This line will be outside the visible range after scrolling.
+
+      const initVisibleStartLine = visibleRange.start.line;
+      const initCursorPosition = activeTextEditor.selection.active;
+
+      emulator.setMarkCommand();
+
+      await emulator.universalArgument();
+      await emulator.subsequentArgumentDigit(12);
+      await emulator.runCommand("scrollDownCommand");
+
+      assert.equal(
+        activeTextEditor.visibleRanges[0]?.start.line,
+        initVisibleStartLine - 12,
+        "Expected the visibleRange has been scrolled 2 lines",
+      );
+      assertSelectionsEqual(activeTextEditor, [
+        initCursorPosition.line,
+        initCursorPosition.character,
+        activeTextEditor.visibleRanges[0]?.end.line as number,
+        0,
+      ]);
+    });
   });
 });
 

src/test/suite/keep-cursor-range.test.ts

@@ -0,0 +1,50 @@
+import assert from "assert";
+import * as vscode from "vscode";
+import { EmacsEmulator } from "../../emulator";
+import { cleanUpWorkspace, setupWorkspace, setEmptyCursors } from "./utils";
+import { Configuration } from "../../configuration/configuration";
+
+suite("onDidChangeTextEditorVisibleRanges event listener with strictEmacsMove", () => {
+  let activeTextEditor: vscode.TextEditor;
+  let emulator: EmacsEmulator;
+
+  setup(async () => {
+    const initialText = "x\n".repeat(1000);
+    activeTextEditor = await setupWorkspace(initialText);
+    emulator = new EmacsEmulator(activeTextEditor); // `EmacsEmulator`'s constructor registers the event listener
+  });
+
+  teardown(async () => {
+    await cleanUpWorkspace();
+    emulator.dispose();
+  });
+
+  setup(() => {});
+  teardown(() => {
+    Configuration.reload();
+  });
+
+  test("it should keep cursor position in the visible range when scrolling when strictEmacsMove = true", async () => {
+    Configuration.instance.strictEmacsMove = true;
+
+    setEmptyCursors(activeTextEditor, [0, 0]);
+
+    await vscode.commands.executeCommand("editorScroll", { to: "down", by: "page", value: 3 });
+
+    assert.strictEqual(activeTextEditor.selection.active.line, activeTextEditor.visibleRanges[0]!.start.line);
+
+    Configuration.reload();
+  });
+
+  test("it shouldn't keep cursor position in the visible range when scrolling when strictEmacsMove = false", async () => {
+    Configuration.instance.strictEmacsMove = false;
+
+    setEmptyCursors(activeTextEditor, [0, 0]);
+
+    await vscode.commands.executeCommand("editorScroll", { to: "down", by: "page", value: 3 });
+
+    assert.strictEqual(activeTextEditor.selection.active.line, 0);
+
+    Configuration.reload();
+  });
+});