Fix global marks jumping to the wrong position (#8662)

src/actions/motion.ts

@@ -562,10 +562,15 @@ class CommandPreviousSearchMatch extends BaseMovement {
   }
 }
 
-@RegisterAction
-class MarkMovementBOL extends BaseMovement {
-  keys = ["'", '<register>'];
+export abstract class BaseMarkMovement extends BaseMovement {
   override isJump = true;
+  protected registerMode?: RegisterMode;
+
+  private isCurrentDocument(document: vscode.TextDocument): boolean {
+    return document === vscode.window.activeTextEditor?.document;
+  }
+
+  protected abstract getNewPosition(document: vscode.TextDocument, position: Position): Position;
 
   public override async execAction(position: Position, vimState: VimState): Promise<Position> {
     const markName = this.keysPressed[1];
@@ -575,42 +580,64 @@ class MarkMovementBOL extends BaseMovement {
       throw VimError.fromCode(ErrorCode.MarkNotSet);
     }
 
-    vimState.currentRegisterMode = RegisterMode.LineWise;
+    if (
+      mark.isUppercaseMark &&
+      vimState.recordedState.operator &&
+      mark.document !== vimState.document
+    ) {
+      throw VimError.fromCode(ErrorCode.MarkNotSet);
+    }
 
-    if (mark.isUppercaseMark && mark.document !== undefined) {
-      if (vimState.recordedState.operator && mark.document !== vimState.document) {
-        // Operators don't work across files
-        throw VimError.fromCode(ErrorCode.MarkNotSet);
-      }
-      await ensureEditorIsActive(mark.document);
+    if (this.registerMode) {
+      vimState.currentRegisterMode = this.registerMode;
+    }
+
+    const document = mark.isUppercaseMark ? mark.document : vimState.document;
+    const newPosition = this.getNewPosition(document, mark.position);
+
+    // Navigate to mark in another document
+    if (mark.isUppercaseMark && !this.isCurrentDocument(mark.document)) {
+      const options: vscode.TextDocumentShowOptions = {
+        selection: new vscode.Range(newPosition, newPosition),
+      };
+      await vscode.window.showTextDocument(mark.document, options);
+      return newPosition;
     }
 
-    return TextEditor.getFirstNonWhitespaceCharOnLine(vimState.document, mark.position.line);
+    // Navigate to mark in the current document
+    return newPosition;
   }
 }
 
 @RegisterAction
-class MarkMovement extends BaseMovement {
-  keys = ['`', '<register>'];
-  override isJump = true;
+export class MarkMovementBOL extends BaseMarkMovement {
+  keys = ["'", '<register>'];
+  protected override registerMode = RegisterMode.LineWise;
 
-  public override async execAction(position: Position, vimState: VimState): Promise<Position> {
-    const markName = this.keysPressed[1];
-    const mark = vimState.historyTracker.getMark(markName);
+  protected override getNewPosition(document: vscode.TextDocument, position: Position): Position {
+    return TextEditor.getFirstNonWhitespaceCharOnLine(document, position.line);
+  }
+}
 
-    if (mark === undefined) {
-      throw VimError.fromCode(ErrorCode.MarkNotSet);
-    }
+@RegisterAction
+export class MarkMovement extends BaseMarkMovement {
+  keys = ['`', '<register>'];
 
-    if (mark.isUppercaseMark && mark.document !== undefined) {
-      if (vimState.recordedState.operator && mark.document !== vimState.document) {
-        // Operators don't work across files
-        throw VimError.fromCode(ErrorCode.MarkNotSet);
-      }
-      await ensureEditorIsActive(mark.document);
+  /**
+   * If the exact position exists, returns that position.
+   * If the character position is beyond the end of line, returns the end of line.
+   * Otherwise returns the position at the end of the document.
+   */
+  protected override getNewPosition(document: vscode.TextDocument, position: Position): Position {
+    const lastLine = document.lineCount - 1;
+    if (position.line > lastLine) {
+      const lastLineLength = document.lineAt(lastLine).text.length;
+      return new Position(lastLine, lastLineLength);
     }
 
-    return mark.position;
+    const { text } = document.lineAt(position.line);
+    const character = Math.min(position.character, text.length);
+    return new Position(position.line, character);
   }
 }
 
@@ -676,12 +703,6 @@ class PrevMarkLinewise extends BaseMovement {
   }
 }
 
-async function ensureEditorIsActive(document: vscode.TextDocument) {
-  if (document !== vscode.window.activeTextEditor?.document) {
-    await vscode.window.showTextDocument(document);
-  }
-}
-
 @RegisterAction
 class MoveLeft extends BaseMovement {
   keys = [['h'], ['<left>'], ['<BS>'], ['<C-BS>'], ['<S-BS>']];

src/cmd_line/commands/marks.ts

@@ -20,7 +20,7 @@ class MarkQuickPickItem implements QuickPickItem {
   constructor(vimState: VimState, mark: IMark) {
     this.mark = mark;
     this.label = mark.name;
-    if (mark.document && mark.document !== vimState.document) {
+    if (mark.isUppercaseMark && mark.document !== vimState.document) {
       this.description = mark.document.fileName;
     } else {
       this.description = vimState.document.lineAt(mark.position).text.trim();

src/history/historyTracker.ts

@@ -96,13 +96,22 @@ class DocumentChange {
   }
 }
 
-export interface IMark {
+export interface IMarkBase {
   name: string;
   position: Position;
-  isUppercaseMark: boolean;
-  document?: vscode.TextDocument; // only required when using global marks (isUppercaseMark is true)
 }
 
+export interface ILocalMark extends IMarkBase {
+  isUppercaseMark: false;
+}
+
+export interface IGlobalMark extends IMarkBase {
+  isUppercaseMark: true;
+  document: vscode.TextDocument; // Required for global marks
+}
+
+export type IMark = ILocalMark | IGlobalMark;
+
 /**
  * An undo's worth of changes; generally corresponds to a single action.
  */
@@ -143,7 +152,7 @@ class HistoryStep {
   /**
    * "global" marks which operate across files. (when IMark.name is uppercase)
    */
-  static globalMarks: IMark[] = [];
+  static globalMarks: IGlobalMark[] = [];
 
   constructor(init: { marks: IMark[]; changes?: DocumentChange[]; cameFromU?: boolean }) {
     this.changes = init.changes ?? [];
@@ -565,12 +574,18 @@ export class HistoryTracker {
       }
     } else {
       const isUppercaseMark = markName.toUpperCase() === markName;
-      const newMark: IMark = {
-        position,
-        name: markName,
-        isUppercaseMark,
-        document: isUppercaseMark ? document : undefined,
-      };
+      const newMark: IMark = isUppercaseMark
+        ? {
+            position,
+            name: markName,
+            isUppercaseMark,
+            document,
+          }
+        : {
+            position,
+            name: markName,
+            isUppercaseMark,
+          };
       this.putMarkInList(newMark);
     }
   }
@@ -651,8 +666,8 @@ export class HistoryTracker {
    * Gets all local marks.  I.e., marks that are specific for the current
    * editor.
    */
-  public getLocalMarks(): IMark[] {
-    return [...this.undoStack.getCurrentMarkList()];
+  public getLocalMarks(): ILocalMark[] {
+    return [...this.undoStack.getCurrentMarkList().filter((mark) => !mark.isUppercaseMark)];
   }
 
   /**

src/vimscript/lineRange.ts

@@ -149,7 +149,7 @@ export class Address {
           return res;
         case 'mark':
           const mark = vimState.historyTracker.getMark(this.specifier.mark);
-          if (!mark || (mark.document && mark.document !== vimState.document)) {
+          if (!mark || (mark.isUppercaseMark && mark.document !== vimState.document)) {
             throw VimError.fromCode(ErrorCode.MarkNotSet);
           }
           return mark.position.line;