Merge pull request #3445 from esetnik/bugfix/sentences-backward-2689

[Bugfix] - sentences backward

src/common/motion/position.ts

@@ -639,7 +639,7 @@ export class Position extends vscode.Position {
     if (args.forward) {
       return this.getNextSentenceBeginWithRegex(this._sentenceEndRegex, false);
     } else {
-      return this.getPreviousSentenceBeginWithRegex(this._sentenceEndRegex, false);
+      return this.getPreviousSentenceBeginWithRegex(this._sentenceEndRegex);
     }
   }
 
@@ -972,22 +972,20 @@ export class Position extends vscode.Position {
     return new Position(TextEditor.getLineCount() - 1, 0).getLineEnd();
   }
 
-  private getPreviousSentenceBeginWithRegex(regex: RegExp, inclusive: boolean): Position {
+  private getPreviousSentenceBeginWithRegex(regex: RegExp): Position {
     let paragraphBegin = this.getCurrentParagraphBeginning();
     for (let currentLine = this.line; currentLine >= paragraphBegin.line; currentLine--) {
       let endPositions = this.getAllEndPositions(
         TextEditor.getLineAt(new vscode.Position(currentLine, 0)).text,
         regex
       );
-      let newCharacter = _.find(
-        endPositions.reverse(),
-        index =>
-          (index < this.character &&
-            !inclusive &&
-            new Position(currentLine, index).getRightThroughLineBreaks().compareTo(this)) ||
-          (index <= this.character && inclusive) ||
-          currentLine !== this.line
-      );
+      let newCharacter = _.find(endPositions.reverse(), index => {
+        const newPositionBeforeThis = new Position(currentLine, index)
+          .getRightThroughLineBreaks()
+          .compareTo(this);
+
+        return (newPositionBeforeThis && (index < this.character || currentLine < this.line));
+      });
 
       if (newCharacter !== undefined) {
         return new Position(currentLine, <number>newCharacter).getRightThroughLineBreaks();

test/sentenceMotion.test.ts

@@ -0,0 +1,84 @@
+import { getTestingFunctions } from './testSimplifier';
+import { cleanUpWorkspace, setupWorkspace } from './testUtils';
+
+suite('sentence motion', () => {
+  let { newTest } = getTestingFunctions();
+
+  setup(async () => {
+    await setupWorkspace(undefined, '.js');
+  });
+
+  teardown(cleanUpWorkspace);
+
+  suite('[count] sentences backward', () => {
+    newTest({
+      title: 'move one sentence backward',
+      start: ['lorem ipsum. lorem ipsum|'],
+      keysPressed: '(',
+      end: ['lorem ipsum. |lorem ipsum'],
+    });
+
+    newTest({
+      title: 'move one sentence backward',
+      start: ['lorem ipsum. lorem ipsum|'],
+      keysPressed: '1(',
+      end: ['lorem ipsum. |lorem ipsum'],
+    });
+
+    newTest({
+      title: 'move [count] sentences backward',
+      start: ['lorem ipsum. lorem ipsum. lorem ipsum|'],
+      keysPressed: '2(',
+      end: ['lorem ipsum. |lorem ipsum. lorem ipsum'],
+    });
+
+    newTest({
+      title: 'move one sentence backward single line - middle',
+      start: ['lorem ipsum. |lorem ipsum'],
+      keysPressed: '(',
+      end: ['|lorem ipsum. lorem ipsum'],
+    });
+
+    newTest({
+      title: 'move one sentence backward no space',
+      start: ['lorem ipsum.lorem ipsum|'],
+      keysPressed: '(',
+      end: ['|lorem ipsum.lorem ipsum'],
+    });
+
+    newTest({
+      title: 'move one sentence backward no space - middle',
+      start: ['lorem ipsum.|lorem ipsum'],
+      keysPressed: '(',
+      end: ['|lorem ipsum.lorem ipsum'],
+    });
+
+    newTest({
+      title: 'move one sentence backward - multiline',
+      start: ['lorem ipsum', 'lorem ipsum|'],
+      keysPressed: '(',
+      end: ['|lorem ipsum', 'lorem ipsum'],
+    });
+
+    newTest({
+      title: 'move one sentence backward - multiline - period',
+      start: ['lorem ipsum.', 'lorem ipsum|'],
+      keysPressed: '(',
+      end: ['lorem ipsum.', '|lorem ipsum'],
+    });
+
+    newTest({
+      title: 'move one sentence backward - multiline - previous line',
+      start: ['lorem ipsum', '|lorem ipsum'],
+      keysPressed: '(',
+      end: ['|lorem ipsum', 'lorem ipsum'],
+    });
+
+    newTest({
+      title: 'move one sentence backward - multiline - previous line - period',
+      start: ['lorem ipsum.', '|lorem ipsum'],
+      keysPressed: '(',
+      end: ['|lorem ipsum.', 'lorem ipsum'],
+    });
+  });
+});