Merge pull request #130 from aioutecism/fix/motion-linewise

Fix motion bugs around line breaks
aioutecism · Sep 17, 2016 · 11e7a0e · 11e7a0e
2 parents 81565bf + 3780bf8
commit 11e7a0e
Show file tree

Hide file tree

Showing 11 changed files with 113 additions and 18 deletions.
diff --git a/src/Actions/BlockCursor.ts b/src/Actions/BlockCursor.ts
@@ -29,4 +29,5 @@ export class ActionBlockCursor {
 
         return Promise.resolve(true);
     }
+
 }
diff --git a/src/Actions/Delete.ts b/src/Actions/Delete.ts
@@ -29,14 +29,18 @@ export class ActionDelete {
         let ranges = activeTextEditor.selections.map(selection => {
             const start = selection.active;
             const end = args.motions.reduce((position, motion) => {
-                return motion.apply(position, {isInclusive: true, isChangeAction: args.isChangeAction});
+                return motion.apply(position, {
+                    isInclusive: true,
+                    shouldCrossLines: false,
+                    isChangeAction: args.isChangeAction,
+                });
             }, start);
             return new Range(start, end);
         });
 
-        ranges = ranges.map(range => document.validateRange(
-            range.isSingleLine ? range : UtilRange.toLinewise(range)
-        ));
+        if (args.motions.some(motion => motion.isLinewise)) {
+            ranges = ranges.map(range => document.validateRange(UtilRange.toLinewise(range)));
+        }
 
         ranges = UtilRange.unionOverlaps(ranges);
 

diff --git a/src/Actions/Register.ts b/src/Actions/Register.ts
@@ -44,14 +44,17 @@ export class ActionRegister {
         let ranges = activeTextEditor.selections.map(selection => {
             const start = selection.active;
             const end = args.motions.reduce((position, motion) => {
-                return motion.apply(position, {isInclusive: true});
+                return motion.apply(position, {
+                    isInclusive: true,
+                    shouldCrossLines: false,
+                });
             }, start);
             return new Range(start, end);
         });
 
-        ranges = ranges.map(range => document.validateRange(
-            range.isSingleLine ? range : UtilRange.toLinewise(range)
-        ));
+        if (args.motions.some(motion => motion.isLinewise)) {
+            ranges = ranges.map(range => document.validateRange(UtilRange.toLinewise(range)));
+        }
 
         ranges = UtilRange.unionOverlaps(ranges);
 

diff --git a/src/Motions/Character.ts b/src/Motions/Character.ts
@@ -27,6 +27,7 @@ export class MotionCharacter extends Motion {
         const obj = new MotionCharacter();
         obj.translate(-args.n, 0);
 
+        obj.isLinewise = true;
         obj.isCharacterUpdated = false;
 
         return obj;
@@ -38,6 +39,7 @@ export class MotionCharacter extends Motion {
         const obj = new MotionCharacter();
         obj.translate(+args.n, 0);
 
+        obj.isLinewise = true;
         obj.isCharacterUpdated = false;
 
         return obj;

diff --git a/src/Motions/Document.ts b/src/Motions/Document.ts
@@ -8,6 +8,9 @@ export class MotionDocument extends Motion {
     static toLine(args: {n: number}): Motion {
         const obj = new MotionDocument();
         obj.line = args.n - 1;
+
+        obj.isLinewise = true;
+
         return obj;
     }
 

diff --git a/src/Motions/Motion.ts b/src/Motions/Motion.ts
@@ -3,6 +3,8 @@ import {UtilPosition} from '../Utils/Position';
 
 export abstract class Motion {
 
+    // TODO: Mark as readonly after TypeScript 2
+    isLinewise = false;
     isCharacterUpdated = true;
 
     private lineDelta = 0;

diff --git a/src/Motions/Word.ts b/src/Motions/Word.ts
@@ -47,9 +47,17 @@ export class MotionWord extends Motion {
         return obj;
     }
 
-    apply(from: Position, option: {isInclusive?: boolean, isChangeAction?: boolean} = {}): Position {
+    apply(
+        from: Position,
+        option: {
+            isInclusive?: boolean,
+            isChangeAction?: boolean,
+            shouldCrossLines?: boolean,
+        } = {}
+    ): Position {
         option.isInclusive = option.isInclusive === undefined ? false : option.isInclusive;
         option.isChangeAction = option.isChangeAction === undefined ? false : option.isChangeAction;
+        option.shouldCrossLines = option.shouldCrossLines === undefined ? true : option.shouldCrossLines;
 
         // Match both start and end if used in change action.
         if (option.isChangeAction && this.matchKind === MotionWordMatchKind.Start) {
@@ -123,6 +131,10 @@ export class MotionWord extends Motion {
                     character++;
                 }
 
+                if (! option.shouldCrossLines) {
+                    return document.lineAt(line).range.end;
+                }
+
                 line++;
             }
 
@@ -177,6 +189,10 @@ export class MotionWord extends Motion {
                     character--;
                 }
 
+                if (! option.shouldCrossLines) {
+                    return document.lineAt(line).range.start;
+                }
+
                 line--;
             }
 

diff --git a/test/Actions/Delete.test.ts b/test/Actions/Delete.test.ts
@@ -0,0 +1,58 @@
+import * as assert from 'assert';
+import * as TestUtil from '../Util';
+import {window, Selection} from 'vscode';
+
+import {Configuration} from '../../src/Configuration';
+import {ActionDelete} from '../../src/Actions/Delete';
+import {MotionWord} from '../../src/Motions/Word';
+
+export function run() {
+
+    Configuration.init();
+
+    test('ActionDelete.byMotions', (done) => {
+
+        const testCases = [
+            {
+                message: 'MotionWord.nextStart at line end',
+                motions: [MotionWord.nextStart()],
+                in: 'Foo end\nBar end',
+                selection: new Selection(0, 6, 0, 6),
+                out: 'Foo en\nBar end',
+            },
+            {
+                message: 'MotionWord.prevStart at line start',
+                motions: [MotionWord.prevStart()],
+                in: 'Foo end\nBar end',
+                selection: new Selection(1, 0, 1, 0),
+                out: 'Foo end\nBar end',
+            }
+        ];
+
+        let promise = Promise.resolve();
+
+        while (testCases.length > 0) {
+            const testCase = testCases.shift();
+            promise = promise.then(() => {
+
+                return TestUtil.createTempDocument(testCase.in).then(() => {
+                    TestUtil.setSelection(testCase.selection);
+
+                    return ActionDelete.byMotions({
+                        motions: testCase.motions
+                    }).then(() => {
+                        assert.equal(TestUtil.getDocument().getText(), testCase.out, testCase.message);
+                    });
+                });
+
+            });
+        }
+
+        promise.then(() => {
+            done();
+        }, (error) => {
+            done(error);
+        });
+
+    });
+};
diff --git a/test/Motions/Word.test.ts b/test/Motions/Word.test.ts
@@ -9,7 +9,7 @@ export function run() {
 
     Configuration.init();
 
-    test('MotionWordPosition.NEXT_START', (done) => {
+    test('MotionWord: Next start', (done) => {
         TestUtil.createTempDocument('  foo bar baz    fum-nom').then(() => {
 
             let apply = (fromCharacter) => {
@@ -142,7 +142,7 @@ export function run() {
         });
     });
 
-    test('MotionWordPosition.NEXT_END', (done) => {
+    test('MotionWord: Next end', (done) => {
         TestUtil.createTempDocument('  foo bar baz    fum-nom').then(() => {
 
             let apply = (fromCharacter) => {
@@ -275,7 +275,7 @@ export function run() {
         });
     });
 
-    test('MotionWordPosition.PREV_START', (done) => {
+    test('MotionWord: Prev start', (done) => {
         TestUtil.createTempDocument('  foo bar baz    fum-nom').then(() => {
 
             let apply = (fromCharacter) => {
@@ -408,7 +408,7 @@ export function run() {
         });
     });
 
-    test('MotionWordPosition.PREV_END', (done) => {
+    test('MotionWord: Prev end', (done) => {
         TestUtil.createTempDocument('  foo bar baz    fum-nom').then(() => {
 
             let apply = (fromCharacter) => {
@@ -541,7 +541,7 @@ export function run() {
         });
     });
 
-    test('MotionWordPosition.NEXT_START with blankSeparators', (done) => {
+    test('MotionWord: Next start with blank separated style', (done) => {
         TestUtil.createTempDocument(' <foo-bar> (baz$foo)  bar').then(() => {
 
             let apply = (fromCharacter) => {
@@ -674,7 +674,7 @@ export function run() {
         });
     });
 
-    test('MotionWordPosition.NEXT_END with blankSeparators', (done) => {
+    test('MotionWord: Next end with blank separated style', (done) => {
         TestUtil.createTempDocument(' <foo-bar> (baz$foo)  bar').then(() => {
 
             let apply = (fromCharacter) => {
@@ -807,7 +807,7 @@ export function run() {
         });
     });
 
-    test('MotionWordPosition.PREV_START with blankSeparators', (done) => {
+    test('MotionWord: Prev start with blank separated style', (done) => {
         TestUtil.createTempDocument(' <foo-bar> (baz$foo)  bar').then(() => {
 
             let apply = (fromCharacter) => {
@@ -945,7 +945,7 @@ export function run() {
         });
     });
 
-    test('MotionWordPosition.PREV_END with blankSeparators', (done) => {
+    test('MotionWord: Prev end with blank separated style', (done) => {
         TestUtil.createTempDocument(' <foo-bar> (baz$foo)  bar').then(() => {
 
             let apply = (fromCharacter) => {

diff --git a/test/Util.ts b/test/Util.ts
@@ -1,4 +1,4 @@
-import {workspace, window, Uri, Position, Selection} from 'vscode';
+import {workspace, window, Uri, TextDocument, Position, Selection} from 'vscode';
 
 export function createTempDocument(content?: string): Thenable<boolean> {
     const uri = Uri.parse(`untitled:${__dirname}.${Math.random()}.tmp`);
@@ -16,6 +16,10 @@ export function createTempDocument(content?: string): Thenable<boolean> {
         });
 }
 
+export function getDocument(): TextDocument {
+    return window.activeTextEditor.document;
+}
+
 export function setPosition(position: Position): void {
     setPositions([position]);
 }

diff --git a/test/extension.test.ts b/test/extension.test.ts
@@ -7,13 +7,15 @@ import * as MotionWordTest from './Motions/Word.test';
 import * as MotionIntegrationTest from './Motions/Integration.test';
 import * as TextObjectWordTest from './TextObjects/Word.test';
 import * as ActionSelectionTest from './Actions/Selection.test';
+import * as ActionDeleteTest from './Actions/Delete.test';
 
 // Defines a Mocha test suite to group tests of similar kind together
 suite('Extension Tests', () => {
     MotionWordTest.run();
     MotionIntegrationTest.run();
 
     ActionSelectionTest.run();
+    ActionDeleteTest.run();
 
     TextObjectWordTest.run();
 });