Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug with collection item scope #2741

Merged
merged 2 commits into from
Jan 21, 2025
Merged

Fix bug with collection item scope #2741

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f6a9479
Fix bug with collection item scope
AndreasArvidsson Jan 21, 2025
eb6b7bc
Update comment
AndreasArvidsson Jan 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
180 changes: 180 additions & 0 deletions data/fixtures/scopes/textual/collectionItem.textual15.scope
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
{
phillco marked this conversation as resolved.
Show resolved Hide resolved
language: {},
choices: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User behavior was with the cursor after the {, "take item" selected both language and choices instead of just choices.

disabled: true,
items: {
colWidth: [2, 10]
}
},
versions: {}
}
---

[#1 Content] =
[#1 Domain] = 1:4-1:16
>------------<
1| language: {},

[#1 Removal] = 1:4-2:4
>-------------
1| language: {},
2| choices: {
----<

[#1 Trailing delimiter] = 1:16-2:4
>-
1| language: {},
2| choices: {
----<

[#1 Insertion delimiter] = ",\n"


[#2 Content] =
[#2 Domain] = 2:4-7:5
>----------
2| choices: {
3| disabled: true,
4| items: {
5| colWidth: [2, 10]
6| }
7| },
-----<

[#2 Removal] = 2:4-8:4
>----------
2| choices: {
3| disabled: true,
4| items: {
5| colWidth: [2, 10]
6| }
7| },
8| versions: {}
----<

[#2 Leading delimiter] = 1:16-2:4
>-
1| language: {},
2| choices: {
----<

[#2 Trailing delimiter] = 7:5-8:4
>-
7| },
8| versions: {}
----<

[#2 Insertion delimiter] = ",\n"


[#3 Content] =
[#3 Domain] = 3:8-3:22
>--------------<
3| disabled: true,

[#3 Removal] = 3:8-4:8
>---------------
3| disabled: true,
4| items: {
--------<

[#3 Trailing delimiter] = 3:22-4:8
>-
3| disabled: true,
4| items: {
--------<

[#3 Insertion delimiter] = ",\n"


[#4 Content] =
[#4 Domain] = 4:8-6:9
>--------
4| items: {
5| colWidth: [2, 10]
6| }
---------<

[#4 Removal] = 3:22-6:9
>-
3| disabled: true,
4| items: {
5| colWidth: [2, 10]
6| }
---------<

[#4 Leading delimiter] = 3:22-4:8
>-
3| disabled: true,
4| items: {
--------<

[#4 Insertion delimiter] = ",\n"


[#5 Content] =
[#5 Domain] = 5:12-5:29
>-----------------<
5| colWidth: [2, 10]

[#5 Removal] = 5:0-5:29
>-----------------------------<
5| colWidth: [2, 10]

[#5 Leading delimiter] = 5:0-5:12
>------------<
5| colWidth: [2, 10]

[#5 Insertion delimiter] = ",\n"


[#6 Content] =
[#6 Domain] = 5:23-5:24
>-<
5| colWidth: [2, 10]

[#6 Removal] = 5:23-5:26
>---<
5| colWidth: [2, 10]

[#6 Trailing delimiter] = 5:24-5:26
>--<
5| colWidth: [2, 10]

[#6 Insertion delimiter] = ", "


[#7 Content] =
[#7 Domain] = 5:26-5:28
>--<
5| colWidth: [2, 10]

[#7 Removal] = 5:24-5:28
>----<
5| colWidth: [2, 10]

[#7 Leading delimiter] = 5:24-5:26
>--<
5| colWidth: [2, 10]

[#7 Insertion delimiter] = ", "


[#8 Content] =
[#8 Domain] = 8:4-8:16
>------------<
8| versions: {}

[#8 Removal] = 7:5-8:16
>-
7| },
8| versions: {}
----------------<

[#8 Leading delimiter] = 7:5-8:4
>-
7| },
8| versions: {}
----<

[#8 Insertion delimiter] = ",\n"
57 changes: 26 additions & 31 deletions ...s/modifiers/scopeHandlers/CollectionItemScopeHandler/CollectionItemTextualScopeHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,51 +68,45 @@ export class CollectionItemTextualScopeHandler extends BaseScopeHandler {
continue;
}

const currentIterationState =
iterationStatesStack[iterationStatesStack.length - 1];
let currentIterationState: IterationState | undefined;

// Find current iteration state and pop all states not containing the separator
while (iterationStatesStack.length > 0) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the logic that was missing before

const lastState = iterationStatesStack[iterationStatesStack.length - 1];
if (lastState.iterationRange.contains(separator)) {
currentIterationState = lastState;
break;
}
this.addScopes(scopes, lastState);
iterationStatesStack.pop();
}

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

// The contain range is either the interior or the line containing the separator
// The containing iteration range is either the interior or the line containing the separator
const containingIterationRange =
containingInteriorRange ??
editor.document.lineAt(separator.start.line).range;

if (currentIterationState != null) {
// The current containing iteration range is the same as the previous one. Just append delimiter.
if (
currentIterationState.iterationRange.isRangeEqual(
containingIterationRange,
)
) {
currentIterationState.delimiters.push(separator);
continue;
}

// The current containing range does not intersect previous one. Add scopes and remove state.
if (!currentIterationState.iterationRange.contains(separator)) {
this.addScopes(scopes, currentIterationState);
// Remove already added state
iterationStatesStack.pop();
}
}

// The current containing range is the same as the previous one. Just append delimiter.
if (iterationStatesStack.length > 0) {
const lastState = iterationStatesStack[iterationStatesStack.length - 1];
if (lastState.iterationRange.isRangeEqual(containingIterationRange)) {
lastState.delimiters.push(separator);
continue;
}
// The current containing iteration range is the same as the previous one. Just append delimiter.
if (
currentIterationState != null &&
currentIterationState.iterationRange.isRangeEqual(
containingIterationRange,
)
) {
currentIterationState.delimiters.push(separator);
continue;
}

// New containing range. Add it to the list.
// New containing range. Add it to the set.
if (containingInteriorRange != null) {
usedInteriors.add(containingInteriorRange);
}

// New containing iteration range. Push it to the stack.
iterationStatesStack.push({
editor,
isEveryScope,
Expand All @@ -121,13 +115,14 @@ export class CollectionItemTextualScopeHandler extends BaseScopeHandler {
});
}

// Process any remaining states on the stack
for (const state of iterationStatesStack) {
this.addScopes(scopes, state);
}

// Add interior ranges without a delimiter in them. eg: `[foo]`
for (const interior of interiorRanges) {
if (!usedInteriors.has(interior.range)) {
if (!usedInteriors.has(interior.range) && !interior.range.isEmpty) {
const range = shrinkRangeToFitContent(editor, interior.range);
if (!range.isEmpty) {
scopes.push(
Expand Down
Loading