Skip to content

Commit cb66508

Browse files
authored
Fix keyboard broken by #2235 (#2309)
This was not caught by our tests because we mock away command server. Not sure how catch this sort of thing in our tests; it's one of the hazards of mocking This PR introduces a more robust approach, where `undefined` means that we don't know which element is focused, either because there is no command server, or we have not been called by the command server - See also cursorless-dev/command-server#25 ## Checklist - [-] I have added [tests](https://www.cursorless.org/docs/contributing/test-case-recorder/) - [-] I have updated the [docs](https://github.com/cursorless-dev/cursorless/tree/main/docs) and [cheatsheet](https://github.com/cursorless-dev/cursorless/tree/main/cursorless-talon/src/cheatsheet) - [-] I have not broken the cheatsheet
1 parent d7dd800 commit cb66508

File tree

8 files changed

+36
-16
lines changed

8 files changed

+36
-16
lines changed
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
languageId: plaintext
2+
focusedElementType: textEditor
3+
command:
4+
version: 7
5+
spokenForm: take token
6+
action:
7+
name: setSelection
8+
target:
9+
type: primitive
10+
modifiers:
11+
- type: containingScope
12+
scopeType: {type: token}
13+
usePrePhraseSnapshot: true
14+
initialState:
15+
documentContents: foo
16+
selections:
17+
- anchor: {line: 0, character: 0}
18+
active: {line: 0, character: 0}
19+
marks: {}
20+
finalState:
21+
documentContents: foo
22+
selections:
23+
- anchor: {line: 0, character: 0}
24+
active: {line: 0, character: 3}

packages/common/src/FakeCommandServerApi.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ export class FakeCommandServerApi implements CommandServerApi {
1010

1111
constructor() {
1212
this.signals = { prePhrase: { getVersion: async () => null } };
13-
this.focusedElementType = "textEditor";
1413
}
1514

1615
async getFocusedElementType(): Promise<FocusedElementType | undefined> {

packages/common/src/types/CommandServerApi.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export interface CommandServerApi {
99
};
1010
}
1111

12-
export type FocusedElementType = "textEditor" | "terminal";
12+
export type FocusedElementType = "textEditor" | "terminal" | "other";
1313

1414
export interface InboundSignal {
1515
getVersion(): Promise<string | null>;

packages/common/src/types/TestCaseFixture.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ interface TestCaseFixtureBase {
1515
/**
1616
* The type of element that is focused before the command is executed. If undefined default to text editor.
1717
*/
18-
focusedElementType?: FocusedElementType | "other";
18+
focusedElementType?: FocusedElementType;
1919

2020
/**
2121
* A list of marks to check in the case of navigation map test otherwise undefined

packages/cursorless-engine/src/core/getCommandFallback.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,9 @@ export async function getCommandFallback(
1414
runAction: (actionDescriptor: ActionDescriptor) => Promise<ActionReturnValue>,
1515
command: CommandComplete,
1616
): Promise<Fallback | null> {
17-
if (
18-
commandServerApi == null ||
19-
(await commandServerApi.getFocusedElementType()) === "textEditor"
20-
) {
17+
const focusedElementType = await commandServerApi?.getFocusedElementType();
18+
19+
if (focusedElementType == null || focusedElementType === "textEditor") {
2120
return null;
2221
}
2322

packages/cursorless-engine/src/testCaseRecorder/TestCase.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,9 +141,9 @@ export class TestCase {
141141
const fixture: EnforceUndefined<TestCaseFixture> = {
142142
languageId: this.languageId,
143143
focusedElementType:
144-
this.focusedElementType !== "textEditor"
145-
? this.focusedElementType ?? "other"
146-
: undefined,
144+
this.focusedElementType === "textEditor"
145+
? undefined
146+
: this.focusedElementType,
147147
postEditorOpenSleepTimeMs: undefined,
148148
postCommandSleepTimeMs: undefined,
149149
command: this.command,

packages/cursorless-vscode-e2e/src/endToEndTestSetup.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ export function endToEndTestSetup(suite: Mocha.Suite) {
2828
const title = this.test!.fullTitle();
2929
retryCount = title === previousTestTitle ? retryCount + 1 : 0;
3030
previousTestTitle = title;
31-
({ ide, injectIde } = (await getCursorlessApi()).testHelpers!);
31+
const testHelpers = (await getCursorlessApi()).testHelpers!;
32+
({ ide, injectIde } = testHelpers);
33+
testHelpers.commandServerApi.setFocusedElementType(undefined);
3234
spy = new SpyIDE(ide);
3335
injectIde(spy);
3436
});

packages/cursorless-vscode-e2e/src/suite/recorded.vscode.test.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,7 @@ async function runTest(file: string, spyIde: SpyIDE) {
104104
// spyIde.clipboard.writeText(fixture.initialState.clipboard);
105105
}
106106

107-
commandServerApi.setFocusedElementType(
108-
fixture.focusedElementType === "other"
109-
? undefined
110-
: fixture.focusedElementType ?? "textEditor",
111-
);
107+
commandServerApi.setFocusedElementType(fixture.focusedElementType);
112108

113109
// Ensure that the expected hats are present
114110
await hatTokenMap.allocateHats(

0 commit comments

Comments
 (0)