cursorless
cursorless copied to clipboard
Generic scope handler interfaces
Implements generic scope handlers
Checklist
- [x] Add "identifier" scope type
- [ ] Switch "word" and "char" to use this new setup, with "identifier" as their parent scope type
- [ ] Add test for
"take next word"when cursor is in the whitespace between two tokens, eg"foo | barBaz" - [ ] Link issues that this PR closes and clean up related issues
- [x] Remove old modifier stages
- [x] Figure our grand scopes. Just pass ancestorIndex to
getScopesTouchingPosition - [x] I have added tests
- [x] I have updated the docs and cheatsheet
Ok super helpful to see the code written out; great call to just bang it out.
Here's what I'm thinking. Scope type exposes two functions:
- A function that takes a position and returns containing iteration range, just as a range
- A function that takes an iteration range and returns a list of scopes
I think it would look as follows
interface ScopeDefinition {
getIterationRange(editor: TextEditor, position: Position): Range;
getScopesInIterationRange(editor: TextEditor, range: Range): Scope[];
}
interface Scope {
domain: Range;
getTarget(isBackward: boolean): Target;
getPriority?(): number;
}
Then the base class / intermediary has an interface as follows:
/** For use as return type of {@link ScopeHandler.getTargetsForRange} */
interface IterationScope {
targets: Target[]
containingStartIndex: number;
containingEndIndex: number;
}
class ScopeHandler {
getTargetsForRange(editor: TextEditor, range: Range, isBackward: boolean): IterationScope {
const startIterationRange = self.scopeDefinition.getIterationRange(editor, range.start)
const iterationRange = contains(startIterationRange, range.end) ? startIterationRange : startIterationRange.union(self.scopeDefinition.getIterationRange(editor, range.end)
const scopes = self.scopeDefinition.getScopesInRange(editor, iterationRange)
// Then compute containing scope by intersection with domain for non-empty range
// For empty range, pick weakly containing domain, breaking ties using `scope.getPriority()` if it exists, else prefer right
// Then call `getTarget()` on the ones we want to return for `targets`
}
}
Then
- containing scope modifier stage is implemented by grabbing the
iterationScope.containingIndicesand constructing range target everyscope modifier stage is implemented by grabbing theiterationScope.targetsand just doing an intersection internally ifhasExplicitRangeordinalscope modifier stage is implemented by calling theeverystage, like we do today. I think it is actually good that there is a hard dependency here, because we wanteveryandordinalto behave identicallyrelativescope modifier stage is implemented by looking at containing scope and list of targets and doing its magic from there
Note that the only place that we ever refer to hasExplicitRange is in the every scope modifier stage
wdyt?
One other question is how we handle "every round". I guess maybe we just have it return an empty range for getIterationRange? I'm not sure we want to do any expansion
Then for getIterationScope, if any pairs start or end inside the iteration range, it returns all top-level pairs that it it intersects with. Top-level here means not contained by any other pair intersecting with iteration range.
If no pairs start or end inside the iteration range, we return the smallest pair containing the iteration range, or maybe just error
So I think this works within the api I defined above; just wanted to write it out to see
I guess one question is how "next round" works, because I believe this algorithm won't return enough for it to see the next one. I wonder if pairs want special treatment for next 🤔. Might be better for them to be able to walk forward and figure it out themselves. Ie go until they hit an opening paren and then take that round.
Maybe "every" isn't really the right abstraction to use for implementing "next" 🤔
Eg it's slightly unfortunate that "next token" breaks when it hits a line boundary
For functions, we expand to class in getIterationRange
In getScopesInRange, I think we just check that the range is just one class, and return its functions if so? Otherwise error? Idk
Ok what should the following do?
aaa bbb|. ccc
Cursor is the |; saying "two tokens"? Should it refer to bbb. or . ccc? I think probably the consistent thing is bbb., because it's two tokens starting from "token". One could also argue it should always just look to the right, though that breaks down when the cursor is in the middle of a token, which should obviously be that token and the following.
Unfortunately, I don't think it's as simple as containing and then adding as many scopes as necessary. For example:
{
aaa {bbb} ccc {ddd} eee
{fff}
}
In this case, if the user says "three curlies line air", I think it should be {bbb} past {fff}, but if we treat "<number> <scope>s" as first expanding to containing, it will expand to the big curly containing everything and then have nothing to add
So my thinking is as follows:
If the input target is empty, then index 0 should refer to containing scope. If the input target is not empty, then index 0 should refer to all intersecting scopes. Wdyt?
@AndreasArvidsson fyi https://github.com/cursorless-dev/cursorless/pull/1031/commits/969bc720e98b89a7526656622b40375285b9c5f7. I accidentally imported the legacy v2 ScopeType in a couple places when I did auto-import. Shall we make it a policy not to export any legacy types without a vN suffix?
@AndreasArvidsson fyi I added https://github.com/cursorless-dev/cursorless/pull/1031/commits/d73fae6d2d488775f123547d71e25b2bb9b7001b to convince myself that this setup can work with #124. I think it will work, but happy to roll back that commit if we want to wait until we are actually ready to implement that
fwiw this is what it looks like to add a scope type now: https://github.com/cursorless-dev/cursorless/pull/1031/commits/08188162a8e5b40c96dbc3c55ebb49008b606806 cc/ @AndreasArvidsson
Filed #1052 to track migrating remaining scope types