(Kind of) fix vertical motion with multiple cursors

The real fix will be to move `desiredColumn` to `Cursor`, but this is a pretty good half-measure
Refs #4797, refs #4585

Author: J-Fields

src/actions/motion.ts

@@ -30,6 +30,19 @@ import { SmartQuoteMatcher, WhichQuotes } from './plugins/targets/smartQuotesMat
 import { useSmartQuotes } from './plugins/targets/targetsConfig';
 import { ModeDataFor } from '../mode/modeData';
 
+function adjustForDesiredColumn(args: {
+  position: Position;
+  desiredColumn: number;
+  multicursorIndex: number | undefined;
+}): Position {
+  const { position, desiredColumn, multicursorIndex } = args;
+  // HACK: until we put `desiredColumn` on `Cursor`, only the first cursor will respect it (except after `$`)
+  if (multicursorIndex && multicursorIndex > 0 && desiredColumn !== Number.POSITIVE_INFINITY) {
+    return position;
+  }
+  return position.with({ character: desiredColumn });
+}
+
 /**
  * A movement is something like 'h', 'k', 'w', 'b', 'gg', etc.
  */
@@ -284,7 +297,11 @@ class MoveDownFoldFix extends MoveByScreenLineMaintainDesiredColumn {
       }
       prev = t;
     }
-    return t.with({ character: vimState.desiredColumn });
+    return adjustForDesiredColumn({
+      position: t,
+      desiredColumn: vimState.desiredColumn,
+      multicursorIndex: this.multicursorIndex,
+    });
   }
 }
 
@@ -313,10 +330,13 @@ class MoveDown extends BaseMovement {
     }
 
     if (position.line < vimState.document.lineCount - 1) {
-      return position.with({ character: vimState.desiredColumn }).getDown();
-    } else {
-      return position;
+      return adjustForDesiredColumn({
+        position,
+        desiredColumn: vimState.desiredColumn,
+        multicursorIndex: this.multicursorIndex,
+      }).getDown();
     }
+    return position;
   }
 
   public override async execActionForOperator(
@@ -353,10 +373,13 @@ class MoveUp extends BaseMovement {
     }
 
     if (position.line > 0) {
-      return position.with({ character: vimState.desiredColumn }).getUp();
-    } else {
-      return position;
+      return adjustForDesiredColumn({
+        position,
+        desiredColumn: vimState.desiredColumn,
+        multicursorIndex: this.multicursorIndex,
+      }).getUp();
     }
+    return position;
   }
 
   public override async execActionForOperator(
@@ -392,7 +415,11 @@ class MoveUpFoldFix extends MoveByScreenLineMaintainDesiredColumn {
       }
       prev = t;
     }
-    return t.with({ character: vimState.desiredColumn });
+    return adjustForDesiredColumn({
+      position: t,
+      desiredColumn: vimState.desiredColumn,
+      multicursorIndex: this.multicursorIndex,
+    });
   }
 }
 
@@ -1201,10 +1228,13 @@ class MoveUpByScreenLineVisualBlock extends BaseMovement {
     vimState: VimState,
   ): Promise<Position | IMovement> {
     if (position.line > 0) {
-      return position.with({ character: vimState.desiredColumn }).getUp();
-    } else {
-      return position;
+      return adjustForDesiredColumn({
+        position,
+        desiredColumn: vimState.desiredColumn,
+        multicursorIndex: this.multicursorIndex,
+      }).getUp();
     }
+    return position;
   }
 
   public override async execActionForOperator(
@@ -1230,10 +1260,13 @@ class MoveDownByScreenLineVisualBlock extends BaseMovement {
     vimState: VimState,
   ): Promise<Position | IMovement> {
     if (position.line < vimState.document.lineCount - 1) {
-      return position.with({ character: vimState.desiredColumn }).getDown();
-    } else {
-      return position;
+      return adjustForDesiredColumn({
+        position,
+        desiredColumn: vimState.desiredColumn,
+        multicursorIndex: this.multicursorIndex,
+      }).getDown();
     }
+    return position;
   }
 
   public override async execActionForOperator(

test/multicursor.test.ts

@@ -2,7 +2,7 @@ import * as assert from 'assert';
 import { getAndUpdateModeHandler } from '../extension';
 import { ModeHandler } from '../src/mode/modeHandler';
 import { assertEqualLines, setupWorkspace } from './testUtils';
-import { newTest } from './testSimplifier';
+import { newTest, newTestSkip } from './testSimplifier';
 
 suite('Multicursor', () => {
   let modeHandler: ModeHandler;
@@ -12,6 +12,45 @@ suite('Multicursor', () => {
     modeHandler = (await getAndUpdateModeHandler())!;
   });
 
+  suite('Motions', () => {
+    for (const foldfix of [true, false]) {
+      suite(`j and k ${foldfix ? '(foldfix)' : ''}`, () => {
+        newTest({
+          title: 'j',
+          config: { foldfix },
+          start: ['l|ine 1', 'lin|e 2', 'line 3'],
+          keysPressed: 'j',
+          end: ['line 1', 'l|ine 2', 'lin|e 3'],
+        });
+
+        newTest({
+          title: 'j (at bottom)',
+          config: { foldfix },
+          start: ['line 1', 'l|ine 2', 'lin|e 3'],
+          keysPressed: 'j',
+          end: ['line 1', 'line 2', 'l|in|e 3'],
+        });
+
+        newTest({
+          title: 'k',
+          config: { foldfix },
+          start: ['line 1', 'l|ine 2', 'lin|e 3'],
+          keysPressed: 'k',
+          end: ['l|ine 1', 'lin|e 2', 'line 3'],
+        });
+
+        // TODO: Fix for foldfix
+        (foldfix ? newTestSkip : newTest)({
+          title: 'k (at top)',
+          config: { foldfix },
+          start: ['l|ine 1', 'lin|e 2', 'line 3'],
+          keysPressed: 'k',
+          end: ['l|in|e 1', 'line 2', 'line 3'],
+        });
+      });
+    }
+  });
+
   test('can add multiple cursors below', async () => {
     await modeHandler.handleMultipleKeyEvents('i11\n22'.split(''));
     await modeHandler.handleMultipleKeyEvents(['<Esc>', 'g', 'g']);