From 7d561b3f9aee121ebdf53cdeeebae4852ecf1183 Mon Sep 17 00:00:00 2001 From: CVVPK Date: Thu, 30 Jan 2020 17:08:47 -0500 Subject: [PATCH 1/2] Add tests for } motion --- test/mode/modeNormal.test.ts | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/test/mode/modeNormal.test.ts b/test/mode/modeNormal.test.ts index 603f2093c40..9749bfb459f 100644 --- a/test/mode/modeNormal.test.ts +++ b/test/mode/modeNormal.test.ts @@ -1003,7 +1003,7 @@ suite('Mode Normal', () => { }); newTest({ - title: 'Can handle d}', + title: 'Can handle d} at beginning of line', start: ['|foo', 'bar', '', 'fun'], keysPressed: 'd}', end: ['|', 'fun'], @@ -1018,6 +1018,30 @@ suite('Mode Normal', () => { endMode: Mode.Normal, }); + newTest({ + title: 'Can handle d} when not at beginning of line', + start: ['f|oo', 'bar', '', 'fun'], + keysPressed: 'd}', + end: ['|f', '', 'fun'], + endMode: Mode.Normal, + }); + + newTest({ + title: 'Can handle } with operator and count, at beginning of line', + start: ['|foo', '', 'bar', '', 'fun'], + keysPressed: 'd2}', + end: ['|', 'fun'], + endMode: Mode.Normal, + }); + + newTest({ + title: 'Can handle } with operator and count, and not at beginning of line', + start: ['f|oo', '', 'bar', '', 'fun'], + keysPressed: 'd2}', + end: ['|f', '', 'fun'], + endMode: Mode.Normal, + }); + newTest({ title: 'Select sentence with trailing spaces', start: ["That's my sec|ret, Captain. I'm always angry."], From 9d4d7ad83805fcf5c710ca103f674e2778842daf Mon Sep 17 00:00:00 2001 From: CVVPK Date: Thu, 30 Jan 2020 17:18:58 -0500 Subject: [PATCH 2/2] Fix '}'(MoveParagraphEnd) behaviour --- src/actions/baseMotion.ts | 4 +++ src/actions/motion.ts | 57 ++++++++++++++++++++++++++++++++------- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/src/actions/baseMotion.ts b/src/actions/baseMotion.ts index aa658f11f7f..d025ff3dd84 100644 --- a/src/actions/baseMotion.ts +++ b/src/actions/baseMotion.ts @@ -119,6 +119,10 @@ export abstract class BaseMovement extends BaseAction { result = await this.createMovementResult(position, vimState, recordedState, lastIteration); if (result instanceof Position) { + /** + * This position will be passed to the `motion` on the next iteration, + * it may cause some issues when count > 1. + */ position = result; } else if (isIMovement(result)) { if (prevResult && result.failed) { diff --git a/src/actions/motion.ts b/src/actions/motion.ts index 16f5cf393e6..5474b497cd6 100644 --- a/src/actions/motion.ts +++ b/src/actions/motion.ts @@ -1,6 +1,6 @@ import * as vscode from 'vscode'; -import { ChangeOperator, DeleteOperator, YankOperator } from './operator'; +import { ChangeOperator, DeleteOperator, YankOperator, BaseOperator } from './operator'; import { CursorMoveByUnit, CursorMovePosition, TextEditor } from './../textEditor'; import { Mode } from './../mode/mode'; import { PairMatcher } from './../common/matching/matcher'; @@ -1257,17 +1257,54 @@ class MoveNextSentenceBegin extends BaseMovement { class MoveParagraphEnd extends BaseMovement { keys = ['}']; isJump = true; + iteration = 0; + isFirstLineWise = false; public async execAction(position: Position, vimState: VimState): Promise { - const isLineWise = - position.isLineBeginning() && - vimState.currentMode === Mode.Normal && - vimState.recordedState.operator; - let paragraphEnd = position.getCurrentParagraphEnd(); - vimState.currentRegisterMode = isLineWise - ? RegisterMode.LineWise - : RegisterMode.AscertainFromCurrentMode; - return isLineWise ? paragraphEnd.getLeftThroughLineBreaks(true) : paragraphEnd; + const hasOperator = vimState.recordedState.operator; + const paragraphEnd = position.getCurrentParagraphEnd(); + + if (hasOperator) { + /** + * When paired with an `operator` and a `count` this move will be executed + * multiple times which could cause issues like https://github.com/VSCodeVim/Vim/issues/4488 + * because subsequent runs will receive back whatever position we return + * (See comment in `BaseMotion.execActionWithCount()`). + * + * We keep track of the iteration we are in, this way we can + * return the correct position when on the last iteration, and we don't + * accidentally set the `registerMode` incorrectly. + */ + this.iteration++; + + const isLineWise = position.isLineBeginning() && vimState.currentMode === Mode.Normal; + + const isLastIteration = vimState.recordedState.count + ? vimState.recordedState.count === this.iteration + : true; + + /** + * `position` may not represent the position of the cursor from which the command was initiated. + * In the case that we will be repeating this move more than once + * we want to respect whether the starting position was at the beginning of line or not. + */ + this.isFirstLineWise = this.iteration === 1 ? isLineWise : this.isFirstLineWise; + + vimState.currentRegisterMode = this.isFirstLineWise + ? RegisterMode.LineWise + : RegisterMode.AscertainFromCurrentMode; + + /** + * `paragraphEnd` is the first blank line after the last word in the + * current paragraph, we want the position just before that one to + * accurately emulate Vim's behaviour, unless we are at EOF. + */ + return isLastIteration && !paragraphEnd.isAtDocumentEnd() + ? paragraphEnd.getLeftThroughLineBreaks(true) + : paragraphEnd; + } + + return paragraphEnd; } }