Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for failed motions #466

Merged
merged 2 commits into from
Jul 17, 2016
Merged

Add support for failed motions #466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d13dfd4
Add failed motions.
johnfn Jul 17, 2016
262cda1
Add failure state to inside-delimiter motions.
johnfn Jul 17, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@
"typescript.tsdk": "./node_modules/typescript/lib", // we want to use the TS server from our node_modules folder to control its version
"editor.tabSize": 2,
"editor.insertSpaces": true,
"files.trimTrailingWhitespace": false
"files.trimTrailingWhitespace": true
}
39 changes: 36 additions & 3 deletions src/actions/actions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ export interface IMovement {
start : Position;
stop : Position;

/**
* Whether this motion succeeded. Some commands, like fx when 'x' can't be found,
* will not move the cursor. Furthermore, dfx won't delete *anything*, even though
* deleting to the current character would generally delete 1 character.
*/
failed? : boolean;

// It /so/ annoys me that I have to put this here.
registerMode?: RegisterMode;
}
Expand Down Expand Up @@ -1524,6 +1531,10 @@ class MoveFindForward extends BaseMovement {
const toFind = this.keysPressed[1];
let result = position.findForwards(toFind, count);

if (!result) {
return { start: position, stop: position, failed: true };
}

if (vimState.recordedState.operator) {
result = result.getRight();
}
Expand All @@ -1542,6 +1553,10 @@ class MoveFindBackward extends BaseMovement {
const toFind = this.keysPressed[1];
let result = position.findBackwards(toFind, count);

if (!result) {
return { start: position, stop: position, failed: true };
}

if (vimState.recordedState.operator) {
result = result.getLeft();
}
Expand All @@ -1561,6 +1576,10 @@ class MoveTilForward extends BaseMovement {
const toFind = this.keysPressed[1];
let result = position.tilForwards(toFind, count);

if (!result) {
return { start: position, stop: position, failed: true };
}

if (vimState.recordedState.operator) {
result = result.getRight();
}
Expand All @@ -1579,6 +1598,10 @@ class MoveTilBackward extends BaseMovement {
const toFind = this.keysPressed[1];
let result = position.tilBackwards(toFind, count);

if (!result) {
return { start: position, stop: position, failed: true };
}

if (vimState.recordedState.operator) {
result = result.getLeft();
}
Expand Down Expand Up @@ -2219,10 +2242,18 @@ abstract class MoveInsideCharacter extends BaseMovement {
let endPos = PairMatcher.nextPairedChar(startPlusOne, this.charToMatch, false);

// Poor man's check for whether we found an opening character
if (startPos === position && text[position.character] !== this.charToMatch) {
return position;
if ((startPos === position && text[position.character] !== this.charToMatch) ||
// Make sure the start position is inside the selection
startPos.isAfter(position) ||
endPos.isBefore(position)) {

return {
start : position,
stop : position,
failed: true
};
}
// Make sure the start position is inside the selection

if (startPos.isAfter(position) || endPos.isBefore(position)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this now, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah. Good catch.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed on master, thanks!

return position;
}
Expand All @@ -2232,10 +2263,12 @@ abstract class MoveInsideCharacter extends BaseMovement {
} else {
startPos = startPlusOne;
}

// If the closing character is the first on the line, don't swallow it.
if (endPos.character === 0) {
endPos = endPos.getLeftThroughLineBreaks();
}

return {
start : startPos,
stop : endPos,
Expand Down
15 changes: 10 additions & 5 deletions src/mode/modeHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,10 @@ export class ModeHandler implements vscode.Disposable {
if (result instanceof Position) {
vimState.cursorPosition = result;
} else if (isIMovement(result)) {
if (result.failed) {
vimState.recordedState = new RecordedState();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is the line that is causing #474, since the stacktrace we're getting is coming from modeHandler.ts:

    if (!recordedState.operator) {
      throw new Error("what in god's name");
    }

But more guessing than actually understanding what "recorded state" is.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sorry, RecorededState is an awful name! It used to be called ActionState, which was a little better, but still bad.

}

vimState.cursorPosition = result.stop;
vimState.cursorStartPosition = result.start;

Expand Down Expand Up @@ -740,8 +744,8 @@ export class ModeHandler implements vscode.Disposable {
}

private async executeOperator(vimState: VimState): Promise<VimState> {
let start = vimState.cursorStartPosition;
let stop = vimState.cursorPosition;
let start = vimState.cursorStartPosition;
let stop = vimState.cursorPosition;
let recordedState = vimState.recordedState;

if (!recordedState.operator) {
Expand All @@ -752,9 +756,10 @@ export class ModeHandler implements vscode.Disposable {
[start, stop] = [stop, start];
}

if (vimState.currentMode !== ModeName.Visual &&
vimState.currentMode !== ModeName.VisualLine &&
vimState.currentRegisterMode !== RegisterMode.LineWise) {
if (vimState.currentMode !== ModeName.Visual &&
vimState.currentMode !== ModeName.VisualLine &&
vimState.currentRegisterMode !== RegisterMode.LineWise) {

if (Position.EarlierOf(start, stop) === start) {
stop = stop.getLeft();
} else {
Expand Down
16 changes: 8 additions & 8 deletions src/motion/position.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -594,30 +594,30 @@ export class Position extends vscode.Position {
return undefined;
}

public tilForwards(char: string, count: number = 1): Position {
public tilForwards(char: string, count: number = 1): Position | null {
const position = this.findHelper(char, count, +1);
if (!position) { return this; }
if (!position) { return null; }

return new Position(this.line, position.character - 1);
}

public tilBackwards(char: string, count: number = 1): Position {
public tilBackwards(char: string, count: number = 1): Position | null {
const position = this.findHelper(char, count, -1);
if (!position) { return this; }
if (!position) { return null; }

return new Position(this.line, position.character + 1);
}

public findForwards(char: string, count: number = 1): Position {
public findForwards(char: string, count: number = 1): Position | null {
const position = this.findHelper(char, count, +1);
if (!position) { return this; }
if (!position) { return null; }

return new Position(this.line, position.character);
}

public findBackwards(char: string, count: number = 1): Position {
public findBackwards(char: string, count: number = 1): Position | null {
const position = this.findHelper(char, count, -1);
if (!position) { return this; }
if (!position) { return null; }

return position;
}
Expand Down
16 changes: 16 additions & 0 deletions test/mode/modeNormal.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,22 @@ suite("Mode Normal", () => {
endMode: ModeName.Insert
});

newTest({
title: "will fail when ca( with no ()",
start: ['|blaaah'],
keysPressed: 'ca(',
end: ['|blaaah'],
endMode: ModeName.Normal
});

newTest({
title: "will fail when ca{ with no {}",
start: ['|blaaah'],
keysPressed: 'ca{',
end: ['|blaaah'],
endMode: ModeName.Normal
});

newTest({
title: "Can handle 'df'",
start: ['aext tex|t'],
Expand Down