Contiguous scope #2101
+604
−24
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,164 @@ | ||
| import { | ||
| Direction, | ||
| Position, | ||
| Range, | ||
| ScopeType, | ||
| TextEditor, | ||
| next, | ||
| } from "@cursorless/common"; | ||
| import { Target } from "../../../typings/target.types"; | ||
| import { ensureSingleTarget } from "../../../util/targetUtils"; | ||
| import { constructScopeRangeTarget } from "../constructScopeRangeTarget"; | ||
| import { BaseScopeHandler } from "./BaseScopeHandler"; | ||
| import type { TargetScope } from "./scope.types"; | ||
| import type { | ||
| CustomScopeType, | ||
| ScopeHandler, | ||
| ScopeIteratorRequirements, | ||
| } from "./scopeHandler.types"; | ||
|
|
||
| export class ContiguousScopeHandler extends BaseScopeHandler { | ||
| protected readonly isHierarchical = false; | ||
|
|
||
| constructor(private scopeHandler: ScopeHandler) { | ||
| super(); | ||
| } | ||
|
|
||
| get scopeType(): ScopeType | undefined { | ||
| return this.scopeHandler.scopeType; | ||
| } | ||
|
|
||
| get iterationScopeType(): ScopeType | CustomScopeType { | ||
| return this.scopeHandler.iterationScopeType; | ||
| } | ||
|
|
||
| *generateScopeCandidates( | ||
| editor: TextEditor, | ||
| position: Position, | ||
| direction: Direction, | ||
AndreasArvidsson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| _hints: ScopeIteratorRequirements, | ||
| ): Iterable<TargetScope> { | ||
| let targetRangeOpposite = next( | ||
| generateTargetRangesInDirection( | ||
| this.scopeHandler, | ||
| editor, | ||
| position, | ||
| direction === "forward" ? "backward" : "forward", | ||
| ), | ||
| ); | ||
|
|
||
| const targetRangesIter = generateTargetRangesInDirection( | ||
| this.scopeHandler, | ||
| editor, | ||
| position, | ||
| direction, | ||
| ); | ||
|
|
||
| for (const targetRange of targetRangesIter) { | ||
| if ( | ||
| targetRangeOpposite != null && | ||
| isAdjacent(targetRangeOpposite.proximal, targetRange.proximal) | ||
| ) { | ||
| yield combineScopes(targetRangeOpposite.distal, targetRange.distal); | ||
| targetRangeOpposite = undefined; | ||
| } else { | ||
| yield combineScopes(targetRange.proximal, targetRange.distal); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function combineScopes(scope1: TargetScope, scope2: TargetScope): TargetScope { | ||
| if (scope1.domain.isRangeEqual(scope2.domain)) { | ||
| return scope1; | ||
| } | ||
|
|
||
| return { | ||
| editor: scope1.editor, | ||
| domain: scope1.domain.union(scope2.domain), | ||
| getTargets: (isReversed) => { | ||
| return constructScopeRangeTarget(isReversed, scope1, scope2); | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| function* generateTargetRangesInDirection( | ||
|
There was a problem hiding this comment. I'm not convinced this function is correct. Eg corner cases like single scope not adjacent to others that's last in the file. To me it looks like if you've already yielded anything, then the final yield after the loop body won't fire. Some proper unit tests for There was a problem hiding this comment. Actually might make sense to hold off on unit tests for the following reasons:
There was a problem hiding this comment. Update from discussion today:
There was a problem hiding this comment.
looking at these test cases, i think it should die There was a problem hiding this comment. Sounds good. Any opinion on making single-line comments behave this way by default? (Ie auto expand to all comments that aren't separated by an empty line) There was a problem hiding this comment. Update from meet-up: let's do the following: (
(comment) @comment
(#match? @comment "^//")
(#contiguous! @comment)
)
(comment) @commentWe both don't love this option or the option in this PR, but it's the best we can do with what we have today. Fwiw we would prefer something like (
(comment) @comment
($if
(#match? @comment "^//")
(#contiguous! @comment)
)
)but that's a bit out of scope here 😅 |
||
| scopeHandler: ScopeHandler, | ||
| editor: TextEditor, | ||
| position: Position, | ||
| direction: Direction, | ||
| ): Iterable<{ proximal: TargetScope; distal: TargetScope }> { | ||
| let proximal, distal: TargetScope | undefined; | ||
|
|
||
| const generator = scopeHandler.generateScopes(editor, position, direction, { | ||
| allowAdjacentScopes: true, | ||
| skipAncestorScopes: true, | ||
| }); | ||
|
|
||
| for (const scope of generator) { | ||
| if (proximal == null) { | ||
| proximal = scope; | ||
| } | ||
|
|
||
| if (distal != null) { | ||
| if (!isAdjacent(distal, scope)) { | ||
| yield { proximal, distal }; | ||
| proximal = scope; | ||
| } | ||
| } | ||
|
|
||
| distal = scope; | ||
| } | ||
|
|
||
| if (proximal != null && distal != null) { | ||
| yield { proximal, distal }; | ||
| } | ||
| } | ||
|
|
||
| function isAdjacent(scope1: TargetScope, scope2: TargetScope): boolean { | ||
| if (!scope1.contiguous || !scope2.contiguous) { | ||
| return false; | ||
| } | ||
|
|
||
| if (scope1.domain.isRangeEqual(scope2.domain)) { | ||
| return true; | ||
| } | ||
|
|
||
| const [startTarget, endTarget] = getTargetsInDocumentOrder( | ||
|
There was a problem hiding this comment. This file is now general enough to be used with most scopes. If we wanted to we could remove a bunch of code here and rely on the fact that we only use it for comments. I could go either way. |
||
| ensureSingleTarget(scope1.getTargets(false)), | ||
| ensureSingleTarget(scope2.getTargets(false)), | ||
| ); | ||
|
|
||
| const leadingRange = | ||
| startTarget.getTrailingDelimiterTarget()?.contentRange ?? | ||
| startTarget.contentRange; | ||
| const trailingRange = | ||
| endTarget.getLeadingDelimiterTarget()?.contentRange ?? | ||
| endTarget.contentRange; | ||
|
|
||
| if (leadingRange.intersection(trailingRange) != null) { | ||
| return true; | ||
| } | ||
|
|
||
| // Non line targets are excluded if they are separated by more than one line | ||
| if ( | ||
| !startTarget.isLine && | ||
| trailingRange.start.line - leadingRange.end.line > 1 | ||
| ) { | ||
| return false; | ||
| } | ||
|
|
||
| // Finally targets are excluded if there is non whitespace text between them | ||
| const rangeBetween = new Range(leadingRange.end, trailingRange.start); | ||
| const text = startTarget.editor.document.getText(rangeBetween); | ||
| return /^\s*$/.test(text); | ||
| } | ||
|
|
||
| function getTargetsInDocumentOrder( | ||
| target1: Target, | ||
| target2: Target, | ||
| ): [Target, Target] { | ||
| return target1.contentRange.start.isBefore(target2.contentRange.start) | ||
| ? [target1, target2] | ||
| : [target2, target1]; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought you were using a query predicate for this now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use both. I don't know how to check it at this stage. When we actually fetch the matched scope it's to late to inject this handler. Not without a lot of rewrites at least.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I'm not following how you would use both. When do you use one vs the other?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code decides to use the handler. Then the scope.contiguous is used to determine when to merge two scopes. That way we can ignore block comments.
cursorless/packages/cursorless-engine/src/processTargets/modifiers/scopeHandlers/ContiguousScopeHandler.ts
Line 119 in b6ac60f