Skip to content

Commit

Permalink
Re-implement and '' with jumpTrackerCloses VSCodeVim#3138. and '…
Browse files Browse the repository at this point in the history
…' were implemented before the new jumpTracker in PR VSCodeVim#1993.This caused these actions to fall out-of-sync, causing issue VSCodeVim#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.
  • Loading branch information
dsschnau committed Dec 6, 2018
1 parent 7c1cec4 commit b23e952
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 37 deletions.
47 changes: 21 additions & 26 deletions src/actions/commands/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -2853,15 +2854,7 @@ class CommandNavigateLast extends BaseCommand {
isJump = true;

public async exec(position: Position, vimState: VimState): Promise<VimState> {
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);
}
}

Expand All @@ -2873,17 +2866,19 @@ class CommandNavigateLastBOL extends BaseCommand {
return false;
}
isJump = true;

public async exec(position: Position, vimState: VimState): Promise<VimState> {
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;
}
}
Expand Down Expand Up @@ -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) {
Expand Down
11 changes: 6 additions & 5 deletions src/jumps/jump.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
}
}
10 changes: 5 additions & 5 deletions src/jumps/jumpTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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() {
Expand Down
30 changes: 29 additions & 1 deletion test/jumpTracker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: [],
});
});
Expand Down

0 comments on commit b23e952

Please sign in to comment.