Skip to content

Commit 17a8f27

Browse files
New interface for performing edits and updating ranges (#2551)
This introduces a new interface to perform updates. The requirements I had was: 1. Support both selections and ranges 2. Support both edits and callback 3. Support setting range expansion behavior per range/selection 4. Support updating selection internally What this pr doesn't try to do: 1. Improve the range updater logic Example ```ts const { selections: [updatedContentSelections], ranges: [updatedEditRanges] } = await new EditsUpdater(rangeUpdater, editableEditor, edits) .selections(contentSelections) .ranges(editRanges, RangeExpansionBehavior.openOpen) .updateEditorSelections() .run(); ``` ## Ponderings 1. Should updating editor selections be opt out instead? 1. Here I'm using two separate classes `EditsUpdater` and `CallbackUpdater`. I'm a bit tempted to just use a single class and a polymorphic argument. `editsOrCallback: Edit[] | () => Promise<void>`. Something like that. 2. Strict necessary we don't actually need the two separate return lists. Internally the range updater works with selections so we converting ranges to selections internally. We could just return a single list of selections and since all selections are also ranges I don't think any of the actions would actually care? If we embrace this polymorphic nature it could look something like this: ```ts const [updatedEditSelections, updatedContentSelections] = await new EditsUpdater(rangeUpdater, editableEditor, edits) .add(contentSelections) .add(editRanges, RangeExpansionBehavior.openOpen) .updateEditorSelections() .run(); ``` Personally I am a fan of the above since it became cleaner and simpler, but I know that @pokey generally doesn't prefer union arguments if it can be avoided. We could of course get rid of `new` if we wanted to make it even cleaner. ```ts const [updatedEditSelections, updatedContentSelections] = await performUpdates(rangeUpdater, editableEditor, edits) .add(contentSelections) .add(editRanges, RangeExpansionBehavior.openOpen) .updateEditorSelections() .run(); ``` Related to #729 ## 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 --------- Co-authored-by: Pokey Rule <755842+pokey@users.noreply.github.com>
1 parent e709b5f commit 17a8f27

File tree

17 files changed

+427
-594
lines changed

17 files changed

+427
-594
lines changed

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

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {
77
} from "@cursorless/common";
88
import { flatten, zip } from "lodash-es";
99
import type { RangeUpdater } from "../core/updateSelections/RangeUpdater";
10-
import { performEditsAndUpdateRanges } from "../core/updateSelections/updateSelections";
10+
import { performEditsAndUpdateSelections } from "../core/updateSelections/updateSelections";
1111
import { ide } from "../singletons/ide.singleton";
1212
import { Target } from "../typings/target.types";
1313
import { flashTargets, runOnTargetsForEachEditor } from "../util/targetUtils";
@@ -25,13 +25,17 @@ export class BreakLine {
2525
await runOnTargetsForEachEditor(targets, async (editor, targets) => {
2626
const contentRanges = targets.map(({ contentRange }) => contentRange);
2727
const edits = getEdits(editor, contentRanges);
28+
const editableEditor = ide().getEditableTextEditor(editor);
2829

29-
const [updatedRanges] = await performEditsAndUpdateRanges(
30-
this.rangeUpdater,
31-
ide().getEditableTextEditor(editor),
32-
edits,
33-
[contentRanges],
34-
);
30+
const { contentRanges: updatedRanges } =
31+
await performEditsAndUpdateSelections({
32+
rangeUpdater: this.rangeUpdater,
33+
editor: editableEditor,
34+
edits,
35+
selections: {
36+
contentRanges,
37+
},
38+
});
3539

3640
return zip(targets, updatedRanges).map(([target, range]) => ({
3741
editor: target!.editor,

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

Lines changed: 28 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,7 @@ import {
77
} from "@cursorless/common";
88
import { flatten } from "lodash-es";
99
import { RangeUpdater } from "../core/updateSelections/RangeUpdater";
10-
import {
11-
getSelectionInfo,
12-
performEditsAndUpdateFullSelectionInfos,
13-
} from "../core/updateSelections/updateSelections";
10+
import { performEditsAndUpdateSelections } from "../core/updateSelections/updateSelections";
1411
import { ide } from "../singletons/ide.singleton";
1512
import { EditWithRangeUpdater } from "../typings/Types";
1613
import { Destination, Target } from "../typings/target.types";
@@ -158,64 +155,35 @@ abstract class BringMoveSwap {
158155
? edits
159156
: edits.filter(({ isSource }) => !isSource);
160157

161-
// Sources should be closedClosed, because they should be logically
162-
// the same as the original source.
163-
const sourceEditSelectionInfos = sourceEdits.map(
164-
({ edit: { range }, originalTarget }) =>
165-
getSelectionInfo(
166-
editor.document,
167-
range.toSelection(originalTarget.isReversed),
168-
RangeExpansionBehavior.closedClosed,
169-
),
170-
);
171-
172-
// Destinations should be openOpen, because they should grow to contain
173-
// the new text.
174-
const destinationEditSelectionInfos = destinationEdits.map(
175-
({ edit: { range }, originalTarget }) =>
176-
getSelectionInfo(
177-
editor.document,
178-
range.toSelection(originalTarget.isReversed),
179-
RangeExpansionBehavior.openOpen,
180-
),
181-
);
182-
183-
const cursorSelectionInfos = editor.selections.map((selection) =>
184-
getSelectionInfo(
185-
editor.document,
186-
selection,
187-
RangeExpansionBehavior.closedClosed,
188-
),
158+
const sourceEditRanges = sourceEdits.map(({ edit }) => edit.range);
159+
const destinationEditRanges = destinationEdits.map(
160+
({ edit }) => edit.range,
189161
);
190-
191162
const editableEditor = ide().getEditableTextEditor(editor);
192163

193-
const [
194-
updatedSourceEditSelections,
195-
updatedDestinationEditSelections,
196-
cursorSelections,
197-
]: Selection[][] = await performEditsAndUpdateFullSelectionInfos(
198-
this.rangeUpdater,
199-
editableEditor,
200-
filteredEdits.map(({ edit }) => edit),
201-
[
202-
sourceEditSelectionInfos,
203-
destinationEditSelectionInfos,
204-
cursorSelectionInfos,
205-
],
206-
);
207-
208-
// NB: We set the selections here because we don't trust vscode to
209-
// properly move the cursor on a bring. Sometimes it will smear an
210-
// empty selection
211-
await editableEditor.setSelections(cursorSelections);
164+
const {
165+
sourceEditRanges: updatedSourceEditRanges,
166+
destinationEditRanges: updatedDestinationEditRanges,
167+
} = await performEditsAndUpdateSelections({
168+
rangeUpdater: this.rangeUpdater,
169+
editor: editableEditor,
170+
edits: filteredEdits.map(({ edit }) => edit),
171+
selections: {
172+
// Sources should be closedClosed, because they should be logically
173+
// the same as the original source.
174+
sourceEditRanges,
175+
// Destinations should be openOpen, because they should grow to contain
176+
// the new text.
177+
destinationEditRanges: {
178+
selections: destinationEditRanges,
179+
behavior: RangeExpansionBehavior.openOpen,
180+
},
181+
},
182+
});
212183

213184
const marks = [
214-
...this.getMarks(sourceEdits, updatedSourceEditSelections),
215-
...this.getMarks(
216-
destinationEdits,
217-
updatedDestinationEditSelections,
218-
),
185+
...this.getMarks(sourceEdits, updatedSourceEditRanges),
186+
...this.getMarks(destinationEdits, updatedDestinationEditRanges),
219187
];
220188

221189
// Restore original order before split into source and destination
@@ -231,13 +199,10 @@ abstract class BringMoveSwap {
231199
);
232200
}
233201

234-
private getMarks(
235-
edits: ExtendedEdit[],
236-
selections: Selection[],
237-
): MarkEntry[] {
202+
private getMarks(edits: ExtendedEdit[], ranges: Range[]): MarkEntry[] {
238203
return edits.map((edit, index): MarkEntry => {
239-
const selection = selections[index];
240-
const range = edit.edit.updateRange(selection);
204+
const originalRange = ranges[index];
205+
const range = edit.edit.updateRange(originalRange);
241206
const target = edit.originalTarget;
242207
return {
243208
editor: edit.editor,

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

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { EditableTextEditor, FlashStyle, TextEditor } from "@cursorless/common";
22
import { flatten } from "lodash-es";
33
import { selectionToStoredTarget } from "../core/commandRunner/selectionToStoredTarget";
44
import { RangeUpdater } from "../core/updateSelections/RangeUpdater";
5-
import { callFunctionAndUpdateSelections } from "../core/updateSelections/updateSelections";
5+
import { performEditsAndUpdateSelections } from "../core/updateSelections/updateSelections";
66
import { ide } from "../singletons/ide.singleton";
77
import { Target } from "../typings/target.types";
88
import {
@@ -97,13 +97,19 @@ export class CallbackAction {
9797
});
9898
}
9999

100-
const [updatedOriginalSelections, updatedTargetSelections] =
101-
await callFunctionAndUpdateSelections(
102-
this.rangeUpdater,
103-
() => options.callback(editableEditor, targets),
104-
editor.document,
105-
[originalSelections, targetSelections],
106-
);
100+
const {
101+
originalSelections: updatedOriginalSelections,
102+
targetSelections: updatedTargetSelections,
103+
} = await performEditsAndUpdateSelections({
104+
rangeUpdater: this.rangeUpdater,
105+
editor: editableEditor,
106+
callback: () => options.callback(editableEditor, targets),
107+
preserveCursorSelections: true,
108+
selections: {
109+
originalSelections,
110+
targetSelections,
111+
},
112+
});
107113

108114
// Reset original selections
109115
if (options.setSelection && options.restoreSelection) {

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

Lines changed: 26 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,7 @@
1-
import {
2-
EditableTextEditor,
3-
RangeExpansionBehavior,
4-
Selection,
5-
} from "@cursorless/common";
1+
import { EditableTextEditor, RangeExpansionBehavior } from "@cursorless/common";
62
import { zip } from "lodash-es";
73
import { RangeUpdater } from "../../core/updateSelections/RangeUpdater";
8-
import { performEditsAndUpdateSelectionsWithBehavior } from "../../core/updateSelections/updateSelections";
4+
import { performEditsAndUpdateSelections } from "../../core/updateSelections/updateSelections";
95
import { EditDestination, State } from "./EditNew.types";
106

117
/**
@@ -46,10 +42,6 @@ export async function runEditTargets(
4642
destination.destination.constructChangeEdit(""),
4743
);
4844

49-
const thatSelections = {
50-
selections: state.thatRanges.map((r) => r.toSelection(false)),
51-
};
52-
5345
// We need to remove undefined cursor locations. Note that these undefined
5446
// locations will be the locations where our edit targets will go. The only
5547
// cursor positions defined at this point will have come from command targets
@@ -59,44 +51,45 @@ export async function runEditTargets(
5951
.filter(({ range }) => range != null);
6052

6153
const cursorIndices = cursorInfos.map(({ index }) => index);
54+
const cursorRanges = cursorInfos.map(({ range }) => range!);
55+
const editRanges = edits.map((edit) => edit.range);
6256

63-
const cursorSelections = {
64-
selections: cursorInfos.map(({ range }) => range!.toSelection(false)),
65-
};
66-
67-
const editSelections = {
68-
selections: edits.map((edit) => edit.range.toSelection(false)),
69-
rangeBehavior: RangeExpansionBehavior.openOpen,
70-
};
71-
72-
const [
73-
updatedThatSelections,
74-
updatedCursorSelections,
75-
updatedEditSelections,
76-
]: Selection[][] = await performEditsAndUpdateSelectionsWithBehavior(
57+
const {
58+
thatRanges: updatedThatRanges,
59+
cursorRanges: updatedCursorRanges,
60+
editRanges: updatedEditRanges,
61+
} = await performEditsAndUpdateSelections({
7762
rangeUpdater,
7863
editor,
7964
edits,
80-
[thatSelections, cursorSelections, editSelections],
81-
);
65+
preserveCursorSelections: true,
66+
selections: {
67+
thatRanges: state.thatRanges,
68+
cursorRanges,
69+
editRanges: {
70+
selections: editRanges,
71+
behavior: RangeExpansionBehavior.openOpen,
72+
},
73+
},
74+
});
8275

83-
const updatedCursorRanges = [...state.cursorRanges];
76+
const finalCursorRanges = [...state.cursorRanges];
8477

8578
// Update the cursor positions for the command targets
86-
zip(cursorIndices, updatedCursorSelections).forEach(([index, selection]) => {
87-
updatedCursorRanges[index!] = selection;
79+
zip(cursorIndices, updatedCursorRanges).forEach(([index, range]) => {
80+
finalCursorRanges[index!] = range;
8881
});
8982

9083
// Add cursor positions for our edit targets.
9184
destinations.forEach((delimiterTarget, index) => {
9285
const edit = edits[index];
93-
const range = edit.updateRange(updatedEditSelections[index]);
94-
updatedCursorRanges[delimiterTarget.index] = range;
86+
const range = edit.updateRange(updatedEditRanges[index]);
87+
finalCursorRanges[delimiterTarget.index] = range;
9588
});
9689

9790
return {
9891
destinations: state.destinations,
99-
thatRanges: updatedThatSelections,
100-
cursorRanges: updatedCursorRanges,
92+
thatRanges: updatedThatRanges,
93+
cursorRanges: finalCursorRanges,
10194
};
10295
}

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

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { CommandCapabilities, EditableTextEditor } from "@cursorless/common";
22
import { RangeUpdater } from "../../core/updateSelections/RangeUpdater";
3-
import { callFunctionAndUpdateRanges } from "../../core/updateSelections/updateSelections";
3+
import { performEditsAndUpdateSelections } from "../../core/updateSelections/updateSelections";
44
import { EditDestination, State } from "./EditNew.types";
55

66
/**
@@ -39,27 +39,33 @@ export async function runInsertLineAfterTargets(
3939
const contentRanges = destinations.map(
4040
({ destination }) => destination.contentRange,
4141
);
42+
const targetRanges = state.destinations.map(
43+
({ contentRange }) => contentRange,
44+
);
45+
46+
const callback = async () => {
47+
if (acceptsLocation) {
48+
await editor.insertLineAfter(contentRanges);
49+
} else {
50+
await editor.setSelections(
51+
contentRanges.map((range) => range.toSelection(false)),
52+
);
53+
await editor.focus();
54+
await editor.insertLineAfter();
55+
}
56+
};
4257

43-
const [updatedTargetRanges, updatedThatRanges] =
44-
await callFunctionAndUpdateRanges(
58+
const { targetRanges: updatedTargetRanges, thatRanges: updatedThatRanges } =
59+
await performEditsAndUpdateSelections({
4560
rangeUpdater,
46-
async () => {
47-
if (acceptsLocation) {
48-
await editor.insertLineAfter(contentRanges);
49-
} else {
50-
await editor.setSelections(
51-
contentRanges.map((range) => range.toSelection(false)),
52-
);
53-
await editor.focus();
54-
await editor.insertLineAfter();
55-
}
61+
editor,
62+
callback,
63+
preserveCursorSelections: true,
64+
selections: {
65+
targetRanges,
66+
thatRanges: state.thatRanges,
5667
},
57-
editor.document,
58-
[
59-
state.destinations.map(({ contentRange }) => contentRange),
60-
state.thatRanges,
61-
],
62-
);
68+
});
6369

6470
// For each of the given command targets, the cursor will go where it ended
6571
// up after running the command. We add it to the state so that any

0 commit comments

Comments
 (0)