From b23e95209056b70c92a50c6a6996868517b6ee1f Mon Sep 17 00:00:00 2001 From: Dan Schnau Date: Mon, 3 Dec 2018 23:53:15 -0500 Subject: [PATCH] Re-implement `` and '' with jumpTrackerCloses #3138.`` and '' were implemented before the new jumpTracker in PR #1993.This caused these actions to fall out-of-sync, causing issue #3138 tosurface.In addition, the jumpTracker implementation requires jumps to be onseparate lines. As far as I can tell, this behavior is not neccesary.This commit removes that requirement, instead requiring that differentjumps be only on separate positions. --- src/actions/commands/actions.ts | 47 +++++++++++++++------------------ src/jumps/jump.ts | 11 ++++---- src/jumps/jumpTracker.ts | 10 +++---- test/jumpTracker.test.ts | 30 ++++++++++++++++++++- 4 files changed, 61 insertions(+), 37 deletions(-) diff --git a/src/actions/commands/actions.ts b/src/actions/commands/actions.ts index fe68af02180e..b9b8d107cc06 100644 --- a/src/actions/commands/actions.ts +++ b/src/actions/commands/actions.ts @@ -23,6 +23,7 @@ import { RegisterAction } from './../base'; import { BaseAction } from './../base'; import { commandLine } from './../../cmd_line/commandLine'; import * as operator from './../operator'; +import { Jump } from '../../jumps/jump'; export class DocumentContentChangeAction extends BaseAction { contentChanges: { @@ -211,10 +212,10 @@ export abstract class BaseCommand extends BaseAction { const cursorsToIterateOver = vimState.allCursors .map(x => new Range(x.start, x.stop)) .sort((a, b) => - a.start.line > b.start.line || - (a.start.line === b.start.line && a.start.character > b.start.character) - ? 1 - : -1 + a.start.line > b.start.line || + (a.start.line === b.start.line && a.start.character > b.start.character) + ? 1 + : -1 ); let cursorIndex = 0; @@ -2853,15 +2854,7 @@ class CommandNavigateLast extends BaseCommand { isJump = true; public async exec(position: Position, vimState: VimState): Promise { - const oldActiveEditor = vimState.editor; - - await vscode.commands.executeCommand('workbench.action.navigateLast'); - - if (oldActiveEditor === vimState.editor) { - vimState.cursorPosition = Position.FromVSCodePosition(vimState.editor.selection.start); - } - - return vimState; + return vimState.globalState.jumpTracker.jumpBack(position, vimState); } } @@ -2873,17 +2866,19 @@ class CommandNavigateLastBOL extends BaseCommand { return false; } isJump = true; - public async exec(position: Position, vimState: VimState): Promise { - const oldActiveEditor = vimState.editor; - - await vscode.commands.executeCommand('workbench.action.navigateLast'); - - if (oldActiveEditor === vimState.editor) { - const pos = Position.FromVSCodePosition(vimState.editor.selection.start); - vimState.cursorPosition = pos.getFirstLineNonBlankChar(); + const lastJump = vimState.globalState.jumpTracker.end; + if (lastJump == null) { + // This command goes to the last jump, and there is no previous jump, so there's nothing to do. + return vimState; } - + const jump = new Jump({ + editor: vimState.editor, + fileName: vimState.editor.document.fileName, + position: lastJump.position.getLineBegin(), + }); + vimState.globalState.jumpTracker.recordJump(Jump.fromStateNow(vimState), jump); + vimState.cursorPosition = jump.position; return vimState; } } @@ -3276,10 +3271,10 @@ class ActionJoin extends BaseCommand { const cursorsToIterateOver = vimState.allCursors .map(x => new Range(x.start, x.stop)) .sort((a, b) => - a.start.line > b.start.line || - (a.start.line === b.start.line && a.start.character > b.start.character) - ? 1 - : -1 + a.start.line > b.start.line || + (a.start.line === b.start.line && a.start.character > b.start.character) + ? 1 + : -1 ); for (const { start, stop } of cursorsToIterateOver) { diff --git a/src/jumps/jump.ts b/src/jumps/jump.ts index ba1c752be772..4cdb16f04422 100644 --- a/src/jumps/jump.ts +++ b/src/jumps/jump.ts @@ -60,14 +60,15 @@ export class Jump { } /** - * Determine whether another jump matches the same file path and line number, - * regardless of whether the column numbers match. - * + * Determine whether another jump matches the same file path, line number, and character column. * @param other - Another Jump to compare against */ - public onSameLine(other: Jump | null | undefined): boolean { + public isSamePosition(other: Jump | null | undefined): boolean { return ( - !other || (this.fileName === other.fileName && this.position.line === other.position.line) + !other || + (this.fileName === other.fileName && + this.position.line === other.position.line && + this.position.character === other.position.character) ); } } diff --git a/src/jumps/jumpTracker.ts b/src/jumps/jumpTracker.ts index 1687686ecbca..564baed05aa2 100644 --- a/src/jumps/jumpTracker.ts +++ b/src/jumps/jumpTracker.ts @@ -78,7 +78,7 @@ export class JumpTracker { * @param to - File/position jumped to */ public recordJump(from: Jump | null, to?: Jump | null) { - if (from && to && from.onSameLine(to)) { + if (from && to && from.isSamePosition(to)) { return; } @@ -313,10 +313,10 @@ export class JumpTracker { pushJump(from: Jump | null, to?: Jump | null) { if (from) { - this.clearJumpsOnSameLine(from); + this.clearJumpsOnSamePosition(from); } - if (from && !from.onSameLine(to)) { + if (from && !from.isSamePosition(to)) { this._jumps.push(from); } @@ -347,8 +347,8 @@ export class JumpTracker { } } - clearJumpsOnSameLine(jump: Jump): void { - this._jumps = this._jumps.filter(j => j === jump || !j.onSameLine(jump)); + clearJumpsOnSamePosition(jump: Jump): void { + this._jumps = this._jumps.filter(j => j === jump || !j.isSamePosition(jump)); } removeDuplicateJumps() { diff --git a/test/jumpTracker.test.ts b/test/jumpTracker.test.ts index 331ab8c05802..48539e2bfaef 100644 --- a/test/jumpTracker.test.ts +++ b/test/jumpTracker.test.ts @@ -283,10 +283,38 @@ suite('Record and navigate jumps', () => { jumps: ['start', '|end', '{', '}'], }); newJumpTest({ - title: 'TODO name this test after https://github.com/VSCodeVim/Vim/issues/3138', + title: 'Can track one-line `` jumps', start: ['|start', 'var foo = {"a", "b"}', 'end'], keysPressed: 'jf{%r]``r[', end: ['start', 'var foo = |["a", "b"]', 'end'], + jumps: ['var foo = ["a", "b"]', 'var foo = ["a", "b"]'], + }); + newJumpTest({ + title: 'Can track one-line double `` jumps', + start: ['|start', 'var foo = {"a", "b"}', 'end'], + keysPressed: 'jf{%r]``r[``', + end: ['start', 'var foo = ["a", "b"|]', 'end'], + jumps: ['var foo = ["a", "b"]', 'var foo = ["a", "b"]'], + }); + newJumpTest({ + title: "Can track one-line '' jumps", + start: ['|start', 'var foo = {"a", "b"}', 'end'], + keysPressed: "jf{%r]``r[''", + end: ['start', '|var foo = ["a", "b"]', 'end'], + jumps: ['var foo = ["a", "b"]', 'var foo = ["a", "b"]'], + }); + newJumpTest({ + title: "Can track one-line double '' jumps", + start: ['|start', 'var foo = {"a", "b"}', 'end'], + keysPressed: "jf{%r]``r[''''", + end: ['start', '|var foo = ["a", "b"]', 'end'], + jumps: ['var foo = ["a", "b"]', 'var foo = ["a", "b"]'], + }); + newJumpTest({ + title: "Can handle '' jumps with no previous jump", + start: ['|start', 'var foo = {"a", "b"}', 'end'], + keysPressed: "''", + end: ['|start', 'var foo = {"a", "b"}', 'end'], jumps: [], }); });