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.

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 some tests and fix some exceptions during the tests #914

Merged
merged 10 commits into from
Oct 26, 2016
45 changes: 33 additions & 12 deletions src/actions/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4724,6 +4724,17 @@ abstract class MoveInsideCharacter extends BaseMovement {
diff : new PositionDiff(0, startPos === position ? 1 : 0)
};
}

public async execActionForOperator(position: Position, vimState: VimState): Promise<Position | IMovement> {
const result = await this.execAction(position, vimState);
if (isIMovement(result)) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't like the duplication here. Ideally we don't even check for this case, it's just handled upstream in ModeHandler.

Copy link
Member Author

@xconverge xconverge Oct 25, 2016

Choose a reason for hiding this comment

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

by the way, these tests that I was trying to fix with these lines dont throw exceptions on macOS but they do on windows...

if (result.failed) {
vimState.recordedState.hasRunOperator = false;
vimState.recordedState.actionsRun = [];
}
}
return result;
}
}

@RegisterAction
Expand Down Expand Up @@ -4893,12 +4904,17 @@ abstract class MoveQuoteMatch extends BaseMovement {
};
}

public async execActionForOperator(position: Position, vimState: VimState): Promise<IMovement> {
const res = await this.execAction(position, vimState);

res.stop = res.stop.getRight();

return res;
public async execActionForOperator(position: Position, vimState: VimState): Promise<Position | IMovement> {
const result = await this.execAction(position, vimState);
if (isIMovement(result)) {
if (result.failed) {
vimState.recordedState.hasRunOperator = false;
vimState.recordedState.actionsRun = [];
} else {
result.stop = result.stop.getRight();
}
}
return result;
}
}

Expand Down Expand Up @@ -5168,12 +5184,17 @@ abstract class MoveTagMatch extends BaseMovement {
};
}

public async execActionForOperator(position: Position, vimState: VimState): Promise<IMovement> {
const res = await this.execAction(position, vimState);

res.stop = res.stop.getRight();

return res;
public async execActionForOperator(position: Position, vimState: VimState): Promise<Position | IMovement> {
const result = await this.execAction(position, vimState);
if (isIMovement(result)) {
if (result.failed) {
vimState.recordedState.hasRunOperator = false;
vimState.recordedState.actionsRun = [];
} else {
result.stop = result.stop.getRight();
}
}
return result;
}
}

Expand Down
21 changes: 20 additions & 1 deletion test/mode/modeInsert.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,18 @@ suite("Mode Insert", () => {
assertEqual(TextEditor.getSelection().start.character, 4, "<Esc> moved cursor position.");
});

test("", async () => {
test("<C-c> can exit insert", async () => {
Copy link
Member

Choose a reason for hiding this comment

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

Wait, don't we need some special config for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you mean?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's useCtrlKeys

Copy link
Member Author

@xconverge xconverge Oct 26, 2016

Choose a reason for hiding this comment

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

We have C-xxxx keys all throughout our tests though

await modeHandler.handleMultipleKeyEvents([
'i',
't', 'e', 'x', 't',
'<C-c>',
'o'
]);

return assertEqualLines(["text", ""]);
});

test("<Esc> can exit insert", async () => {
await modeHandler.handleMultipleKeyEvents([
'i',
't', 'e', 'x', 't',
Expand All @@ -54,6 +65,14 @@ suite("Mode Insert", () => {
return assertEqualLines(["text", ""]);
});

test("Stay in insert when entering characters", async () => {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this will ever catch the elusive bug unfortunately. It's triggered when there are multiple vimStates for the same file. That's almost impossible to cause in a test.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm.. A better test might be to switch the language to HTML or something and then ensure that there was only ever one mode handler created.

await modeHandler.handleKeyEvent('i');
for (var i = 0; i < 10; i++) {
await modeHandler.handleKeyEvent('1');
assertEqual(modeHandler.currentMode.name === ModeName.Insert, true);
}
});

test("Can handle 'O'", async () => {
await modeHandler.handleMultipleKeyEvents([
'i',
Expand Down
7 changes: 7 additions & 0 deletions test/mode/modeNormal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1321,4 +1321,11 @@ suite("Mode Normal", () => {
end: ["\t|one", "two"],
endMode: ModeName.Normal
});

newTest({
title: "Can handle <Esc> and do nothing",
start: ['te|st'],
keysPressed: '<Esc>',
end: ['te|st'],
});
});