Skip to content

Commit

Permalink
Fixed bug with edit new (#2806)
Browse files Browse the repository at this point in the history
<!--
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]>
  • Loading branch information
AndreasArvidsson and phillco authored Feb 1, 2025
1 parent 5be3d53 commit 6606f98
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 6 deletions.
62 changes: 62 additions & 0 deletions data/fixtures/recorded/actions/pourStateAirAndLineBat.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
languageId: typescript
command:
version: 7
spokenForm: pour state air and line bat
action:
name: editNewLineAfter
target:
type: list
elements:
- type: primitive
mark: {type: decoratedSymbol, symbolColor: default, character: a}
modifiers:
- type: containingScope
scopeType: {type: statement}
- type: primitive
mark: {type: decoratedSymbol, symbolColor: default, character: b}
modifiers:
- type: containingScope
scopeType: {type: line}
usePrePhraseSnapshot: false
initialState:
documentContents: |
function aaa() {
}
// bbb
selections:
- anchor: {line: 4, character: 0}
active: {line: 4, character: 0}
marks:
default.a:
start: {line: 0, character: 9}
end: {line: 0, character: 12}
default.b:
start: {line: 3, character: 3}
end: {line: 3, character: 6}
finalState:
documentContents: |+
function aaa() {
}
// bbb
selections:
- anchor: {line: 2, character: 0}
active: {line: 2, character: 0}
- anchor: {line: 5, character: 0}
active: {line: 5, character: 0}
thatMark:
- type: UntypedTarget
contentRange:
start: {line: 0, character: 0}
end: {line: 1, character: 1}
isReversed: false
hasExplicitRange: true
- type: UntypedTarget
contentRange:
start: {line: 4, character: 0}
end: {line: 4, character: 6}
isReversed: false
hasExplicitRange: true
12 changes: 9 additions & 3 deletions packages/cursorless-engine/src/actions/EditNew/EditNew.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export class EditNew {
*/
let state: State = {
destinations,
actionTypes: destinations.map((d) => d.getEditNewActionType()),
thatRanges: destinations.map(
({ target }) => target.thatTarget.contentRange,
),
Expand Down Expand Up @@ -63,9 +64,14 @@ export class EditNew {
!useInsertLineAfter,
);

const newSelections = state.destinations.map((destination, index) =>
state.cursorRanges[index]!.toSelection(destination.target.isReversed),
);
const newSelections = state.destinations.map((destination, index) => {
const cursorRange = state.cursorRanges[index];
if (cursorRange == null) {
throw Error("Cursor range is undefined for destination");
}
return cursorRange.toSelection(destination.target.isReversed);
});

await editableEditor.setSelections(newSelections, { focusEditor: true });

return {
Expand Down
12 changes: 11 additions & 1 deletion packages/cursorless-engine/src/actions/EditNew/EditNew.types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import type { Range } from "@cursorless/common";
import type { Destination } from "../../typings/target.types";
import type {
Destination,
EditNewActionType,
} from "../../typings/target.types";

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

/**
* The action types for each destination. It's important to read from this and not
* the destinations themselves, since they are changed as the action runs and
* we make multiple passes.
*/
actionTypes: EditNewActionType[];

/**
* We use this field to track the desired `thatMark` at the end, updating it
* as necessary.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export async function runEditTargets(
): Promise<State> {
const destinations: EditDestination[] = state.destinations
.map((destination, index) => {
if (useAllDestinations || destination.getEditNewActionType() === "edit") {
if (useAllDestinations || state.actionTypes[index] === "edit") {
return {
destination,
index,
Expand Down Expand Up @@ -90,6 +90,7 @@ export async function runEditTargets(

return {
destinations: state.destinations,
actionTypes: state.actionTypes,
thatRanges: updatedThatRanges,
cursorRanges: finalCursorRanges,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export async function runInsertLineAfterTargets(
): Promise<State> {
const destinations: EditDestination[] = state.destinations
.map((destination, index) => {
const actionType = destination.getEditNewActionType();
const actionType = state.actionTypes[index];
if (actionType === "insertLineAfter") {
return {
destination,
Expand Down Expand Up @@ -84,6 +84,7 @@ export async function runInsertLineAfterTargets(
destination.target.withContentRange(updatedTargetRanges[index]),
),
),
actionTypes: state.actionTypes,
thatRanges: updatedThatRanges,
cursorRanges,
};
Expand Down

0 comments on commit 6606f98

Please sign in to comment.