From f6a9479e58c9bf90423758487f36ed7f89c14e2f Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Tue, 21 Jan 2025 13:09:07 +0100 Subject: [PATCH 1/2] Fix bug with collection item scope --- .../textual/collectionItem.textual15.scope | 180 ++++++++++++++++++ .../CollectionItemTextualScopeHandler.ts | 57 +++--- 2 files changed, 206 insertions(+), 31 deletions(-) create mode 100644 data/fixtures/scopes/textual/collectionItem.textual15.scope diff --git a/data/fixtures/scopes/textual/collectionItem.textual15.scope b/data/fixtures/scopes/textual/collectionItem.textual15.scope new file mode 100644 index 0000000000..0aa55f1680 --- /dev/null +++ b/data/fixtures/scopes/textual/collectionItem.textual15.scope @@ -0,0 +1,180 @@ +{ + language: {}, + 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" diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CollectionItemScopeHandler/CollectionItemTextualScopeHandler.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CollectionItemScopeHandler/CollectionItemTextualScopeHandler.ts index fb3f251e6b..979e0c22ac 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CollectionItemScopeHandler/CollectionItemTextualScopeHandler.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CollectionItemScopeHandler/CollectionItemTextualScopeHandler.ts @@ -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) { + 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, @@ -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( From eb6b7bc83b94caf2d4b1879afacd23b0034e8a95 Mon Sep 17 00:00:00 2001 From: Andreas Arvidsson Date: Tue, 21 Jan 2025 13:11:50 +0100 Subject: [PATCH 2/2] Update comment --- .../CollectionItemTextualScopeHandler.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CollectionItemScopeHandler/CollectionItemTextualScopeHandler.ts b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CollectionItemScopeHandler/CollectionItemTextualScopeHandler.ts index 979e0c22ac..b290edb0d2 100644 --- a/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CollectionItemScopeHandler/CollectionItemTextualScopeHandler.ts +++ b/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/CollectionItemScopeHandler/CollectionItemTextualScopeHandler.ts @@ -77,6 +77,7 @@ export class CollectionItemTextualScopeHandler extends BaseScopeHandler { currentIterationState = lastState; break; } + // We are done with this iteration scope. Add all scopes from it and pop it from the stack. this.addScopes(scopes, lastState); iterationStatesStack.pop(); }