From f96f38dbf1b59d80425903248eed7cefa1d8f9ee Mon Sep 17 00:00:00 2001 From: johnfn Date: Thu, 18 Feb 2016 00:21:53 -0800 Subject: [PATCH] Add tests for visual mode and clean up. --- src/mode/mode.ts | 8 +-- src/mode/modeNormal.ts | 2 - src/mode/modeVisual.ts | 63 +++++++++++++------ src/motion/motion.ts | 6 +- src/operator/change.ts | 13 ++-- src/operator/delete.ts | 27 ++++++-- src/operator/operator.ts | 2 +- src/textEditor.ts | 4 ++ test/mode/modeVisual.test.ts | 117 +++++++++++++++++++++++++++++++++++ test/testUtils.ts | 2 +- 10 files changed, 199 insertions(+), 45 deletions(-) diff --git a/src/mode/mode.ts b/src/mode/mode.ts index fac031103bca..074e772b1cd9 100644 --- a/src/mode/mode.ts +++ b/src/mode/mode.ts @@ -1,7 +1,7 @@ "use strict"; import {Motion} from './../motion/motion'; -import {Position, PositionOptions} from './../motion/position'; +import {Position} from './../motion/position'; export enum ModeName { Normal, @@ -48,8 +48,8 @@ export abstract class Mode { protected keyToNewPosition: { [key: string]: (motion: Position) => Promise; } = { "h" : async (c) => { return c.getLeft(); }, - "j" : async (c) => { return c.getDown(0); }, // TODO - 0 is incorrect here. - "k" : async (c) => { return c.getUp(0); }, // getDown/Up should, by default, maintain the current column. + "j" : async (c) => { return c.getDown(0); }, + "k" : async (c) => { return c.getUp(0); }, "l" : async (c) => { return c.getRight(); }, // "^" : async () => { return vscode.commands.executeCommand("cursorHome"); }, "gg" : async (c) => { @@ -65,7 +65,7 @@ export abstract class Mode { "e" : async (c) => { return c.getCurrentWordEnd(); }, "b" : async (c) => { return c.getWordLeft(); }, "}" : async (c) => { return c.getCurrentParagraphEnd(); }, - "{" : async (c) => { return c.getCurrentParagraphBeginning(); }, + "{" : async (c) => { return c.getCurrentParagraphBeginning(); } }; abstract shouldBeActivated(key : string, currentMode : ModeName) : boolean; diff --git a/src/mode/modeNormal.ts b/src/mode/modeNormal.ts index 84915dd52969..cae49a43f4d1 100644 --- a/src/mode/modeNormal.ts +++ b/src/mode/modeNormal.ts @@ -52,8 +52,6 @@ export class NormalMode extends Mode { async handleActivation(key : string): Promise { this.motion.left().move(); - - await this.motion; } async handleKeyEvent(key : string): Promise { diff --git a/src/mode/modeVisual.ts b/src/mode/modeVisual.ts index 51f08c712b64..f2319759a564 100644 --- a/src/mode/modeVisual.ts +++ b/src/mode/modeVisual.ts @@ -1,19 +1,24 @@ "use strict"; -import * as vscode from 'vscode'; -import * as _ from 'lodash' - -import {ModeName, Mode} from './mode'; -import {TextEditor} from './../textEditor'; -import {Motion} from './../motion/motion'; -import {Position, PositionOptions} from './../motion/position'; -import { Operator } from './../operator/operator' -import { DeleteOperator } from './../operator/delete' -import { ModeHandler } from './modeHandler.ts' -import { ChangeOperator } from './../operator/change' +import * as _ from 'lodash'; + +import { ModeName, Mode } from './mode'; +import { Motion} from './../motion/motion'; +import { Position } from './../motion/position'; +import { Operator } from './../operator/operator'; +import { DeleteOperator } from './../operator/delete'; +import { ModeHandler } from './modeHandler.ts'; +import { ChangeOperator } from './../operator/change'; export class VisualMode extends Mode { + /** + * The part of the selection that stays in the same place when motions are applied. + */ private _selectionStart: Position; + + /** + * The part of the selection that moves. + */ private _selectionStop : Position; private _modeHandler : ModeHandler; @@ -22,6 +27,7 @@ export class VisualMode extends Mode { constructor(motion: Motion, modeHandler: ModeHandler) { super(ModeName.Visual, motion); + this._modeHandler = modeHandler; this._keysToOperators = { // TODO: use DeleteOperator.key() @@ -29,8 +35,8 @@ export class VisualMode extends Mode { // simply allow the operators to say what mode they transition into. 'd': new DeleteOperator(modeHandler), 'x': new DeleteOperator(modeHandler), - 'c': new ChangeOperator(modeHandler), - } + 'c': new ChangeOperator(modeHandler) + }; } shouldBeActivated(key: string, currentMode: ModeName): boolean { @@ -39,9 +45,9 @@ export class VisualMode extends Mode { async handleActivation(key: string): Promise { this._selectionStart = this.motion.position; - this._selectionStop = this._selectionStart.getRight(); + this._selectionStop = this._selectionStart; - this.motion.selectTo(this._selectionStop); + this.motion.select(this._selectionStart, this._selectionStop); } handleDeactivation(): void { @@ -73,7 +79,25 @@ export class VisualMode extends Mode { this._selectionStop = await this.keyToNewPosition[keysPressed](this._selectionStop); this.motion.moveTo(this._selectionStart.line, this._selectionStart.character); - this.motion.selectTo(this._selectionStop.getRight()); + + /** + * Always select the letter that we started visual mode on, no matter + * if we are in front or behind it. Imagine that we started visual mode + * with some text like this: + * + * abc|def + * + * (The | represents the cursor.) If we now press w, we'll select def, + * but if we hit b we expect to select abcd, so we need to getRight() on the + * start of the selection when it precedes where we started visual mode. + */ + + // TODO this could be abstracted out + if (this._selectionStart.compareTo(this._selectionStop) <= 0) { + this.motion.select(this._selectionStart, this._selectionStop); + } else { + this.motion.select(this._selectionStart.getRight(), this._selectionStop); + } this.keyHistory = []; } @@ -82,7 +106,6 @@ export class VisualMode extends Mode { } private async _handleOperator(): Promise { - let keyHandled = false; let keysPressed: string; let operator: Operator; @@ -95,10 +118,10 @@ export class VisualMode extends Mode { } if (operator) { - if (this._selectionStart.compareTo(this._selectionStop)) { - operator.run(this._selectionStart, this._selectionStop.getRight()); + if (this._selectionStart.compareTo(this._selectionStop) <= 0) { + await operator.run(this._selectionStart, this._selectionStop.getRight()); } else { - operator.run(this._selectionStop, this._selectionStart.getRight()); + await operator.run(this._selectionStart.getRight(), this._selectionStop); } } diff --git a/src/motion/motion.ts b/src/motion/motion.ts index 0dc3dd2c46f9..1361ac5d23c1 100644 --- a/src/motion/motion.ts +++ b/src/motion/motion.ts @@ -139,12 +139,12 @@ export class Motion implements vscode.Disposable { } } - public selectTo(other: Position): void { - let selection = new vscode.Selection(this.position, other); + public select(from: Position, to: Position): void { + let selection = new vscode.Selection(from, to); vscode.window.activeTextEditor.selection = selection; - this.highlightBlock(other.getLeft()); + this.highlightBlock(to); } public left() : Motion { diff --git a/src/operator/change.ts b/src/operator/change.ts index ef2bdd5a56b9..9e2a463f43a4 100644 --- a/src/operator/change.ts +++ b/src/operator/change.ts @@ -1,12 +1,9 @@ "use strict"; -import {Position, PositionOptions} from './../motion/position'; -import { TextEditor } from './../textEditor' -import { Operator } from './Operator' -import { ModeHandler } from './../mode/modeHandler.ts' -import { ModeName } from './../mode/mode' - -import * as vscode from 'vscode' +import { Position } from './../motion/position'; +import { DeleteOperator } from './delete'; +import { ModeHandler } from './../mode/modeHandler.ts'; +import { ModeName } from './../mode/mode'; export class ChangeOperator { private _modeHandler: ModeHandler; @@ -21,7 +18,7 @@ export class ChangeOperator { * Run this operator on a range. */ public async run(start: Position, end: Position): Promise { - await TextEditor.delete(new vscode.Range(start, end)); + await new DeleteOperator(this._modeHandler).run(start, end); this._modeHandler.setCurrentModeByName(ModeName.Insert); } diff --git a/src/operator/delete.ts b/src/operator/delete.ts index d9fb30f2ff0d..183c83b756dd 100644 --- a/src/operator/delete.ts +++ b/src/operator/delete.ts @@ -1,12 +1,11 @@ "use strict"; -import {Position, PositionOptions} from './../motion/position'; -import { TextEditor } from './../textEditor' -import { Operator } from './Operator' -import { ModeHandler } from './../mode/modeHandler.ts' -import { ModeName } from './../mode/mode' +import { Position } from './../motion/position'; +import { TextEditor } from './../textEditor'; +import { ModeHandler } from './../mode/modeHandler.ts'; +import { ModeName } from './../mode/mode'; -import * as vscode from 'vscode' +import * as vscode from 'vscode'; export class DeleteOperator { private _modeHandler: ModeHandler; @@ -21,6 +20,22 @@ export class DeleteOperator { * Run this operator on a range. */ public async run(start: Position, end: Position): Promise { + + // Imagine we have selected everything with an X in + // the following text (there is no character on the + // second line at all, just a block cursor): + + // XXXXXXX + // X + // + // If we delete this range, we want to delete the entire first and + // second lines. Therefore we have to advance the cursor to the next + // line. + + if (TextEditor.getLineAt(end).text === "") { + end = end.getDown(0); + } + await TextEditor.delete(new vscode.Range(start, end)); this._modeHandler.setCurrentModeByName(ModeName.Normal); diff --git a/src/operator/operator.ts b/src/operator/operator.ts index e3d4eb670504..9ee591533ec5 100644 --- a/src/operator/operator.ts +++ b/src/operator/operator.ts @@ -1,6 +1,6 @@ "use strict"; -import {Position, PositionOptions} from './../motion/position'; +import { Position } from './../motion/position'; export abstract class Operator { /** diff --git a/src/textEditor.ts b/src/textEditor.ts index bf3e39df1f1f..9fc9209cec7c 100644 --- a/src/textEditor.ts +++ b/src/textEditor.ts @@ -71,6 +71,10 @@ export class TextEditor { return vscode.window.activeTextEditor.document.lineAt(position); } + static getSelection(): vscode.Range { + return vscode.window.activeTextEditor.selection; + } + static isFirstLine(position : vscode.Position): boolean { return position.line === 0; } diff --git a/test/mode/modeVisual.test.ts b/test/mode/modeVisual.test.ts index e69de29bb2d1..c72242a934ca 100644 --- a/test/mode/modeVisual.test.ts +++ b/test/mode/modeVisual.test.ts @@ -0,0 +1,117 @@ +"use strict"; + +import * as assert from 'assert'; +import {ModeHandler} from '../../src/mode/modeHandler'; +import {setupWorkspace, cleanUpWorkspace, assertEqualLines} from './../testUtils'; +import {VisualMode} from '../../src/mode/modeVisual'; +import {ModeName} from '../../src/mode/mode'; +import {Motion, MotionMode} from '../../src/motion/motion'; +import {TextEditor} from '../../src/textEditor'; + +suite("Mode Visual", () => { + let motion: Motion; + let visualMode: VisualMode; + let modeHandler: ModeHandler; + + setup(async () => { + await setupWorkspace(); + + modeHandler = new ModeHandler(); + motion = new Motion(MotionMode.Cursor); + visualMode = new VisualMode(motion, modeHandler); + }); + + teardown(cleanUpWorkspace); + + test("can be activated", () => { + assert.equal(visualMode.shouldBeActivated("v", ModeName.Normal), true, "v didn't trigger visual mode..."); + }); + + test("Can handle w", async () => { + await TextEditor.insert("test test test\ntest\n"); + + motion.moveTo(0, 0); + + await visualMode.handleActivation('v'); + await visualMode.handleKeyEvent("w"); + + const sel = TextEditor.getSelection(); + + assert.equal(sel.start.character, 0); + assert.equal(sel.start.line, 0); + + // The input cursor comes BEFORE the block cursor. Try it out, this + // is how Vim works. + assert.equal(sel.end.character, 5); + assert.equal(sel.end.line, 0); + }); + + test("Can handle wd", async () => { + await TextEditor.insert("one two three"); + motion.moveTo(0, 0); + + await visualMode.handleActivation('v'); + await visualMode.handleKeyEvent("w"); + await visualMode.handleKeyEvent("d"); + + assertEqualLines(["wo three"]); + }); + + test("Can handle x", async () => { + await TextEditor.insert("one two three"); + motion.moveTo(0, 0); + + await visualMode.handleActivation('v'); + await visualMode.handleKeyEvent("x"); + + assertEqualLines(["ne two three"]); + }); + + test("can do vwd in middle of sentence", async () => { + await TextEditor.insert("one two three foar"); + motion.moveTo(0, 4); + + await visualMode.handleActivation('v'); + await visualMode.handleKeyEvent("w"); + await visualMode.handleKeyEvent("d"); + + assertEqualLines(["one hree foar"]); + }); + + test("handles case where we go from selecting on right side to selecting on left side", async () => { + await TextEditor.insert("one two three"); + motion.moveTo(0, 4); + + await visualMode.handleActivation('v'); + await visualMode.handleKeyEvent("w"); + await visualMode.handleKeyEvent("b"); + await visualMode.handleKeyEvent("b"); + await visualMode.handleKeyEvent("d"); + + assertEqualLines(["wo three"]); + }); + + test("delete operator handles empty line", async () => { + await TextEditor.insert("one two\n\nthree four"); + motion.moveTo(0, 0); + + await visualMode.handleActivation('v'); + await visualMode.handleKeyEvent("}"); + await visualMode.handleKeyEvent("d"); + + assertEqualLines(["three four"]); + }); + + test("Change operator", async () => { + await TextEditor.insert("one two three"); + motion.moveTo(0, 0); + + await visualMode.handleActivation('v'); + await visualMode.handleKeyEvent("w"); + await visualMode.handleKeyEvent("c"); + + assertEqualLines(["wo three"]); + + assert.equal(((visualMode as any)._modeHandler as ModeHandler).currentMode.name, ModeName.Insert); + }); +}); diff --git a/test/testUtils.ts b/test/testUtils.ts index 3c2e592d099a..5f6370162e00 100644 --- a/test/testUtils.ts +++ b/test/testUtils.ts @@ -40,7 +40,7 @@ export async function setupWorkspace(): Promise { assert.ok(vscode.window.activeTextEditor); } -export function cleanUpWorkspace(): Promise { +export async function cleanUpWorkspace(): Promise { // https://github.com/Microsoft/vscode/blob/master/extensions/vscode-api-tests/src/utils.ts return new Promise((c, e) => { if (vscode.window.visibleTextEditors.length === 0) {