Skip to content

Commit 6606f98

Browse files
Fixed bug with edit new (#2806)
<!-- Thanks for your change! * Remember to *add tests* if you have added a new feature, or fixed a bug. Use the test case recorder: https://www.cursorless.org/docs/contributing/test-case-recorder * Remember to *update the docs* if you've added a new feature. Link: https://github.com/cursorless-dev/cursorless/tree/main/docs * Remember to *test the cheatsheet manually* if you've added a new action, modifier, or scope -- or changed any Talon files. --> If you said `"pour state air and line bat"` where `air` was a multiline statement you got an undefined exception at runtime. I have corrected the problem with our logic and also removed a not null assertion in favor of actually checking for null. ## Release notes `"pour state air and line bat"` (if `air` was a multiline statement) now works instead of raising an exception. --------- Co-authored-by: Phil Cohen <[email protected]>
1 parent 5be3d53 commit 6606f98

File tree

5 files changed

+86
-6
lines changed

5 files changed

+86
-6
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
languageId: typescript
2+
command:
3+
version: 7
4+
spokenForm: pour state air and line bat
5+
action:
6+
name: editNewLineAfter
7+
target:
8+
type: list
9+
elements:
10+
- type: primitive
11+
mark: {type: decoratedSymbol, symbolColor: default, character: a}
12+
modifiers:
13+
- type: containingScope
14+
scopeType: {type: statement}
15+
- type: primitive
16+
mark: {type: decoratedSymbol, symbolColor: default, character: b}
17+
modifiers:
18+
- type: containingScope
19+
scopeType: {type: line}
20+
usePrePhraseSnapshot: false
21+
initialState:
22+
documentContents: |
23+
function aaa() {
24+
}
25+
26+
// bbb
27+
selections:
28+
- anchor: {line: 4, character: 0}
29+
active: {line: 4, character: 0}
30+
marks:
31+
default.a:
32+
start: {line: 0, character: 9}
33+
end: {line: 0, character: 12}
34+
default.b:
35+
start: {line: 3, character: 3}
36+
end: {line: 3, character: 6}
37+
finalState:
38+
documentContents: |+
39+
function aaa() {
40+
}
41+
42+
43+
// bbb
44+
45+
selections:
46+
- anchor: {line: 2, character: 0}
47+
active: {line: 2, character: 0}
48+
- anchor: {line: 5, character: 0}
49+
active: {line: 5, character: 0}
50+
thatMark:
51+
- type: UntypedTarget
52+
contentRange:
53+
start: {line: 0, character: 0}
54+
end: {line: 1, character: 1}
55+
isReversed: false
56+
hasExplicitRange: true
57+
- type: UntypedTarget
58+
contentRange:
59+
start: {line: 4, character: 0}
60+
end: {line: 4, character: 6}
61+
isReversed: false
62+
hasExplicitRange: true

packages/cursorless-engine/src/actions/EditNew/EditNew.ts

+9-3
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ export class EditNew {
3535
*/
3636
let state: State = {
3737
destinations,
38+
actionTypes: destinations.map((d) => d.getEditNewActionType()),
3839
thatRanges: destinations.map(
3940
({ target }) => target.thatTarget.contentRange,
4041
),
@@ -63,9 +64,14 @@ export class EditNew {
6364
!useInsertLineAfter,
6465
);
6566

66-
const newSelections = state.destinations.map((destination, index) =>
67-
state.cursorRanges[index]!.toSelection(destination.target.isReversed),
68-
);
67+
const newSelections = state.destinations.map((destination, index) => {
68+
const cursorRange = state.cursorRanges[index];
69+
if (cursorRange == null) {
70+
throw Error("Cursor range is undefined for destination");
71+
}
72+
return cursorRange.toSelection(destination.target.isReversed);
73+
});
74+
6975
await editableEditor.setSelections(newSelections, { focusEditor: true });
7076

7177
return {

packages/cursorless-engine/src/actions/EditNew/EditNew.types.ts

+11-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import type { Range } from "@cursorless/common";
2-
import type { Destination } from "../../typings/target.types";
2+
import type {
3+
Destination,
4+
EditNewActionType,
5+
} from "../../typings/target.types";
36

47
/**
58
* Internal type to be used for storing a reference to a destination that will use an
@@ -29,6 +32,13 @@ export interface State {
2932
*/
3033
destinations: Destination[];
3134

35+
/**
36+
* The action types for each destination. It's important to read from this and not
37+
* the destinations themselves, since they are changed as the action runs and
38+
* we make multiple passes.
39+
*/
40+
actionTypes: EditNewActionType[];
41+
3242
/**
3343
* We use this field to track the desired `thatMark` at the end, updating it
3444
* as necessary.

packages/cursorless-engine/src/actions/EditNew/runEditTargets.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export async function runEditTargets(
2626
): Promise<State> {
2727
const destinations: EditDestination[] = state.destinations
2828
.map((destination, index) => {
29-
if (useAllDestinations || destination.getEditNewActionType() === "edit") {
29+
if (useAllDestinations || state.actionTypes[index] === "edit") {
3030
return {
3131
destination,
3232
index,
@@ -90,6 +90,7 @@ export async function runEditTargets(
9090

9191
return {
9292
destinations: state.destinations,
93+
actionTypes: state.actionTypes,
9394
thatRanges: updatedThatRanges,
9495
cursorRanges: finalCursorRanges,
9596
};

packages/cursorless-engine/src/actions/EditNew/runInsertLineAfterTargets.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export async function runInsertLineAfterTargets(
2525
): Promise<State> {
2626
const destinations: EditDestination[] = state.destinations
2727
.map((destination, index) => {
28-
const actionType = destination.getEditNewActionType();
28+
const actionType = state.actionTypes[index];
2929
if (actionType === "insertLineAfter") {
3030
return {
3131
destination,
@@ -84,6 +84,7 @@ export async function runInsertLineAfterTargets(
8484
destination.target.withContentRange(updatedTargetRanges[index]),
8585
),
8686
),
87+
actionTypes: state.actionTypes,
8788
thatRanges: updatedThatRanges,
8889
cursorRanges,
8990
};

0 commit comments

Comments
 (0)