Add tests for visual mode and clean up.

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<Position>; } = {
         "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;

src/mode/modeNormal.ts

@@ -52,8 +52,6 @@ export class NormalMode extends Mode {
 
     async handleActivation(key : string): Promise<void> {
         this.motion.left().move();
-
-        await this.motion;
     }
 
     async handleKeyEvent(key : string): Promise<void>  {

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,15 +27,16 @@ export class VisualMode extends Mode {
     constructor(motion: Motion, modeHandler: ModeHandler) {
         super(ModeName.Visual, motion);
 
+        this._modeHandler = modeHandler;
         this._keysToOperators = {
             // TODO: use DeleteOperator.key()
 
             // TODO: Don't pass in mode handler to DeleteOperators,
             // 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<void> {
         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<boolean> {
-        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);
             }
         }
 

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 {

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<void> {
-        await TextEditor.delete(new vscode.Range(start, end));
+        await new DeleteOperator(this._modeHandler).run(start, end);
 
         this._modeHandler.setCurrentModeByName(ModeName.Insert);
     }

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<void> {
+
+        // 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);

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 {
     /**

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;
     }

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);
+    });
+});

test/testUtils.ts

@@ -40,7 +40,7 @@ export async function setupWorkspace(): Promise<any> {
     assert.ok(vscode.window.activeTextEditor);
 }
 
-export function cleanUpWorkspace(): Promise<any> {
+export async function cleanUpWorkspace(): Promise<any> {
     // 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) {