Skip to content

Commit 8874c87

Browse files
Fix bug with collection item scope (#2741)
We didn't properly pop this stack of iteration states. This led to items being grouped incorrectly. ## Checklist - [x] 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 4688ad8 commit 8874c87

File tree

2 files changed

+207
-31
lines changed

2 files changed

+207
-31
lines changed
Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
{
2+
language: {},
3+
choices: {
4+
disabled: true,
5+
items: {
6+
colWidth: [2, 10]
7+
}
8+
},
9+
versions: {}
10+
}
11+
---
12+
13+
[#1 Content] =
14+
[#1 Domain] = 1:4-1:16
15+
>------------<
16+
1| language: {},
17+
18+
[#1 Removal] = 1:4-2:4
19+
>-------------
20+
1| language: {},
21+
2| choices: {
22+
----<
23+
24+
[#1 Trailing delimiter] = 1:16-2:4
25+
>-
26+
1| language: {},
27+
2| choices: {
28+
----<
29+
30+
[#1 Insertion delimiter] = ",\n"
31+
32+
33+
[#2 Content] =
34+
[#2 Domain] = 2:4-7:5
35+
>----------
36+
2| choices: {
37+
3| disabled: true,
38+
4| items: {
39+
5| colWidth: [2, 10]
40+
6| }
41+
7| },
42+
-----<
43+
44+
[#2 Removal] = 2:4-8:4
45+
>----------
46+
2| choices: {
47+
3| disabled: true,
48+
4| items: {
49+
5| colWidth: [2, 10]
50+
6| }
51+
7| },
52+
8| versions: {}
53+
----<
54+
55+
[#2 Leading delimiter] = 1:16-2:4
56+
>-
57+
1| language: {},
58+
2| choices: {
59+
----<
60+
61+
[#2 Trailing delimiter] = 7:5-8:4
62+
>-
63+
7| },
64+
8| versions: {}
65+
----<
66+
67+
[#2 Insertion delimiter] = ",\n"
68+
69+
70+
[#3 Content] =
71+
[#3 Domain] = 3:8-3:22
72+
>--------------<
73+
3| disabled: true,
74+
75+
[#3 Removal] = 3:8-4:8
76+
>---------------
77+
3| disabled: true,
78+
4| items: {
79+
--------<
80+
81+
[#3 Trailing delimiter] = 3:22-4:8
82+
>-
83+
3| disabled: true,
84+
4| items: {
85+
--------<
86+
87+
[#3 Insertion delimiter] = ",\n"
88+
89+
90+
[#4 Content] =
91+
[#4 Domain] = 4:8-6:9
92+
>--------
93+
4| items: {
94+
5| colWidth: [2, 10]
95+
6| }
96+
---------<
97+
98+
[#4 Removal] = 3:22-6:9
99+
>-
100+
3| disabled: true,
101+
4| items: {
102+
5| colWidth: [2, 10]
103+
6| }
104+
---------<
105+
106+
[#4 Leading delimiter] = 3:22-4:8
107+
>-
108+
3| disabled: true,
109+
4| items: {
110+
--------<
111+
112+
[#4 Insertion delimiter] = ",\n"
113+
114+
115+
[#5 Content] =
116+
[#5 Domain] = 5:12-5:29
117+
>-----------------<
118+
5| colWidth: [2, 10]
119+
120+
[#5 Removal] = 5:0-5:29
121+
>-----------------------------<
122+
5| colWidth: [2, 10]
123+
124+
[#5 Leading delimiter] = 5:0-5:12
125+
>------------<
126+
5| colWidth: [2, 10]
127+
128+
[#5 Insertion delimiter] = ",\n"
129+
130+
131+
[#6 Content] =
132+
[#6 Domain] = 5:23-5:24
133+
>-<
134+
5| colWidth: [2, 10]
135+
136+
[#6 Removal] = 5:23-5:26
137+
>---<
138+
5| colWidth: [2, 10]
139+
140+
[#6 Trailing delimiter] = 5:24-5:26
141+
>--<
142+
5| colWidth: [2, 10]
143+
144+
[#6 Insertion delimiter] = ", "
145+
146+
147+
[#7 Content] =
148+
[#7 Domain] = 5:26-5:28
149+
>--<
150+
5| colWidth: [2, 10]
151+
152+
[#7 Removal] = 5:24-5:28
153+
>----<
154+
5| colWidth: [2, 10]
155+
156+
[#7 Leading delimiter] = 5:24-5:26
157+
>--<
158+
5| colWidth: [2, 10]
159+
160+
[#7 Insertion delimiter] = ", "
161+
162+
163+
[#8 Content] =
164+
[#8 Domain] = 8:4-8:16
165+
>------------<
166+
8| versions: {}
167+
168+
[#8 Removal] = 7:5-8:16
169+
>-
170+
7| },
171+
8| versions: {}
172+
----------------<
173+
174+
[#8 Leading delimiter] = 7:5-8:4
175+
>-
176+
7| },
177+
8| versions: {}
178+
----<
179+
180+
[#8 Insertion delimiter] = ",\n"

packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CollectionItemScopeHandler/CollectionItemTextualScopeHandler.ts

Lines changed: 27 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -68,51 +68,46 @@ export class CollectionItemTextualScopeHandler extends BaseScopeHandler {
6868
continue;
6969
}
7070

71-
const currentIterationState =
72-
iterationStatesStack[iterationStatesStack.length - 1];
71+
let currentIterationState: IterationState | undefined;
72+
73+
// Find current iteration state and pop all states not containing the separator
74+
while (iterationStatesStack.length > 0) {
75+
const lastState = iterationStatesStack[iterationStatesStack.length - 1];
76+
if (lastState.iterationRange.contains(separator)) {
77+
currentIterationState = lastState;
78+
break;
79+
}
80+
// We are done with this iteration scope. Add all scopes from it and pop it from the stack.
81+
this.addScopes(scopes, lastState);
82+
iterationStatesStack.pop();
83+
}
7384

7485
// Get range for smallest containing interior
7586
const containingInteriorRange =
7687
interiorRangeFinder.getSmallestContaining(separator)?.range;
7788

78-
// The contain range is either the interior or the line containing the separator
89+
// The containing iteration range is either the interior or the line containing the separator
7990
const containingIterationRange =
8091
containingInteriorRange ??
8192
editor.document.lineAt(separator.start.line).range;
8293

83-
if (currentIterationState != null) {
84-
// The current containing iteration range is the same as the previous one. Just append delimiter.
85-
if (
86-
currentIterationState.iterationRange.isRangeEqual(
87-
containingIterationRange,
88-
)
89-
) {
90-
currentIterationState.delimiters.push(separator);
91-
continue;
92-
}
93-
94-
// The current containing range does not intersect previous one. Add scopes and remove state.
95-
if (!currentIterationState.iterationRange.contains(separator)) {
96-
this.addScopes(scopes, currentIterationState);
97-
// Remove already added state
98-
iterationStatesStack.pop();
99-
}
100-
}
101-
102-
// The current containing range is the same as the previous one. Just append delimiter.
103-
if (iterationStatesStack.length > 0) {
104-
const lastState = iterationStatesStack[iterationStatesStack.length - 1];
105-
if (lastState.iterationRange.isRangeEqual(containingIterationRange)) {
106-
lastState.delimiters.push(separator);
107-
continue;
108-
}
94+
// The current containing iteration range is the same as the previous one. Just append delimiter.
95+
if (
96+
currentIterationState != null &&
97+
currentIterationState.iterationRange.isRangeEqual(
98+
containingIterationRange,
99+
)
100+
) {
101+
currentIterationState.delimiters.push(separator);
102+
continue;
109103
}
110104

111-
// New containing range. Add it to the list.
105+
// New containing range. Add it to the set.
112106
if (containingInteriorRange != null) {
113107
usedInteriors.add(containingInteriorRange);
114108
}
115109

110+
// New containing iteration range. Push it to the stack.
116111
iterationStatesStack.push({
117112
editor,
118113
isEveryScope,
@@ -121,13 +116,14 @@ export class CollectionItemTextualScopeHandler extends BaseScopeHandler {
121116
});
122117
}
123118

119+
// Process any remaining states on the stack
124120
for (const state of iterationStatesStack) {
125121
this.addScopes(scopes, state);
126122
}
127123

128124
// Add interior ranges without a delimiter in them. eg: `[foo]`
129125
for (const interior of interiorRanges) {
130-
if (!usedInteriors.has(interior.range)) {
126+
if (!usedInteriors.has(interior.range) && !interior.range.isEmpty) {
131127
const range = shrinkRangeToFitContent(editor, interior.range);
132128
if (!range.isEmpty) {
133129
scopes.push(

0 commit comments

Comments
 (0)