Word in visual mode #385

rebornix · 2016-07-04T03:45:12Z

Enrich aw with executeWithOperator and executeWithCount
Add aW
Enrich iw
Add iW
Normal/Visual mode test cases.

johnfn · 2016-07-04T15:27:23Z

test/mode/modeVisual.test.ts

@@ -5,9 +5,14 @@ import { ModeHandler } from '../../src/mode/modeHandler';
 import { setupWorkspace, cleanUpWorkspace, assertEqualLines, assertEqual } from './../testUtils';
 import { ModeName } from '../../src/mode/mode';
 import { TextEditor } from '../../src/textEditor';
+import { getTestingFunctions } from '../testSimplifier';


Thanks! I need to change the other tests at some point...

Oh we can do this step by step. Each time we add new test cases to old ones, we can do the tweaking at the same time.

rebornix · 2016-07-05T07:16:38Z

Add logic for iw and iW.

Just like I mentioned above, selecting words, sentences, paragraphs, etc all share the same executeWithCount that we use in MovementAWordTextObject. It's different from BaseMovement#execActionWithCount.

In summary, BaseMovement#execActionWithCount is returning a movement, which tells us how cursor moves from old position to new position. While MovementAWordTextObject#executeWithCount is returning a selection, which tells us which range (start and stop) should we operate on. IMovement is not an ideal name for this situation IMHO.

rebornix · 2016-07-05T13:10:16Z

Just realize that IMovement is for selection. The funny thing is the IMovement part of BaseMovement#execActionWithCount is never being called (actually I don't know what kind of movement will use original logic). So I just moved my execActionWithCount into it.

rebornix · 2016-07-12T09:50:53Z

Update and fix conflicts

johnfn · 2016-07-12T18:21:32Z

src/actions/actions.ts

      let result: Position | IMovement = new Position(0, 0); // bogus init to satisfy typechecker
+=======


There are still some merge conflicts here.

johnfn · 2016-07-12T18:22:03Z

Sorry, I've left this hanging for too long. Can you explain what you did? Just add counts for aw iw?

rebornix · 2016-07-13T02:02:52Z

The first thing I did is fixing aw and iw with count and then add aW, iW, the Big Word feature.

johnfn · 2016-07-13T09:09:22Z

src/actions/actions.ts

@@ -171,7 +171,12 @@ export abstract class BaseMovement extends BaseAction {
   */
  public async execActionWithCount(position: Position, vimState: VimState, count: number): Promise<Position | IMovement> {
      let recordedState = vimState.recordedState;
-      let result: Position | IMovement = new Position(0, 0); // bogus init to satisfy typechecker
+      let resultPosition: Position = new Position(0, 0);


Isn't resultPosition the exact same thing as position?

rebornix · 2016-07-14T12:30:51Z

Fix the comment by @johnfn , I was doing wrong with Union Type but now turned around.

johnfn · 2016-07-16T06:01:10Z

src/actions/actions.ts

+            if (lastIteration) {
+              (result as IMovement).stop = temporaryResult.stop;
+            } else {
+              position = temporaryResult.stop.getRightThroughLineBreaks();


This getRight concerns me. Why is it necessary?

yup, it's necessary for IMovment. This is also why the logic of IMovement and Position is totally different.

When we run a single movement without an operator, like w, we want to move cursor to the beginning of next word. While if we run a single movement with an operator, like aw, we want to move the end cursor to the end of current word (aka, the left-side character of the beginning of next word). Then we sucessfully get a selection of the whole word.

Above is about single movement w/ operator. Then if we run movements with an operator multple times, like 2aw, after running the first aw, the end of seletion or the cursor is at the end of first word. As we need to include next following word into the selection, we need to pass the position of the begining of next word to aw command, then we can get the correct end position of the second aw. Lastly, we combine the beginning of first word and the end of last word and return the result selection.

Like I mentioned before, Movement (contains a start and a stop) is likely a selection. When executing selection with numeric prefix, I think we always need to move cursor to next character of the end of current selection.

johnfn · 2016-07-16T06:03:54Z

Just want to hear back on my 2 comments and we should be good to merge!

rebornix · 2016-07-17T05:01:19Z

@johnfn rebased code and replied to your comments :)

rebornix · 2016-07-19T14:00:17Z

Rebase again to resolve conflicts.

johnfn · 2016-07-23T06:34:07Z

Woo finally! Sorry for taking so long, and thanks @rebornix!

johnfn reviewed Jul 4, 2016
View reviewed changes

rebornix force-pushed the WordInVisualMode branch 2 times, most recently from 51c281d to 5b889b2 Compare July 5, 2016 13:08

rebornix force-pushed the WordInVisualMode branch from 5b889b2 to a7f321c Compare July 12, 2016 09:50

rebornix force-pushed the WordInVisualMode branch from a7f321c to a4357a9 Compare July 12, 2016 09:53

johnfn reviewed Jul 12, 2016
View reviewed changes

rebornix force-pushed the WordInVisualMode branch from a4357a9 to f672298 Compare July 13, 2016 01:04

johnfn reviewed Jul 13, 2016
View reviewed changes

rebornix force-pushed the WordInVisualMode branch from f672298 to 87a8990 Compare July 14, 2016 12:28

johnfn reviewed Jul 16, 2016
View reviewed changes

rebornix mentioned this pull request Jul 17, 2016

All text objects #287

Closed

7 tasks

rebornix force-pushed the WordInVisualMode branch from 87a8990 to f391e53 Compare July 17, 2016 05:00

rebornix force-pushed the WordInVisualMode branch from f391e53 to 7c67e7d Compare July 17, 2016 05:02

rebornix added 7 commits July 19, 2016 21:59

refactor and minor fix aw

425f705

Fix leading spaces of aw

abd5c31

Fix numeric aw

b15438e

Add aW big word selection

78b4355

aw, aW visual mode test

74aa90d

Refactor execActionForOperator for aw

be4a6f3

aw, aW Normal mode test

7e6762e

rebornix added 6 commits July 19, 2016 21:59

fix whitespace and update roadmap status

b6a333d

iw and iW

fec23f8

When select chars in reverse order, {i/a}{w/W} works differently.

c168fa8

iw, iW test cases.

70c115f

Refactor

c3bcc56

Fix null check for union type.

6c779f5

rebornix force-pushed the WordInVisualMode branch from 7c67e7d to 6c779f5 Compare July 19, 2016 13:59

johnfn merged commit 6c25501 into VSCodeVim:master Jul 23, 2016

rebornix deleted the WordInVisualMode branch July 23, 2016 15:20

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Word in visual mode #385

Word in visual mode #385

rebornix commented Jul 4, 2016 •

edited

Loading

johnfn Jul 4, 2016

rebornix Jul 5, 2016

rebornix commented Jul 5, 2016

rebornix commented Jul 5, 2016

rebornix commented Jul 12, 2016

johnfn Jul 12, 2016

johnfn commented Jul 12, 2016

rebornix commented Jul 13, 2016

johnfn Jul 13, 2016

rebornix commented Jul 14, 2016

johnfn Jul 16, 2016

rebornix Jul 17, 2016

johnfn commented Jul 16, 2016

rebornix commented Jul 17, 2016

rebornix commented Jul 19, 2016

johnfn commented Jul 23, 2016

		let result: Position \| IMovement = new Position(0, 0); // bogus init to satisfy typechecker
		=======

Word in visual mode #385

Word in visual mode #385

Conversation

rebornix commented Jul 4, 2016 • edited Loading

johnfn Jul 4, 2016

Choose a reason for hiding this comment

rebornix Jul 5, 2016

Choose a reason for hiding this comment

rebornix commented Jul 5, 2016

rebornix commented Jul 5, 2016

rebornix commented Jul 12, 2016

johnfn Jul 12, 2016

Choose a reason for hiding this comment

johnfn commented Jul 12, 2016

rebornix commented Jul 13, 2016

johnfn Jul 13, 2016

Choose a reason for hiding this comment

rebornix commented Jul 14, 2016

johnfn Jul 16, 2016

Choose a reason for hiding this comment

rebornix Jul 17, 2016

Choose a reason for hiding this comment

johnfn commented Jul 16, 2016

rebornix commented Jul 17, 2016

rebornix commented Jul 19, 2016

johnfn commented Jul 23, 2016

rebornix commented Jul 4, 2016 •

edited

Loading