Utilize interior in relative scope modifier #3122
Conversation
|
@pokey Mind having a look at this? |
pokey
left a comment
pokey
left a comment
There was a problem hiding this comment.
Hard to say whether this will be nice or annoying in practice. I think worth shipping to find out
| const interiorRanges = Array.from(interiorScopes) | ||
| .filter((s) => !s.domain.contains(initialPosition)) | ||
| .map((s) => s.domain); |
There was a problem hiding this comment.
Nitpicking here but might as well do map / filter on the stream then convert to list at the end
There was a problem hiding this comment.
You mean using itertools? I can have a look at that.
packages/cursorless-engine/src/processTargets/modifiers/RelativeScopeStage.ts
Outdated
Show resolved
Hide resolved
| return islice(scopes, offset - 1, offset + desiredScopeCount - 1); | ||
| } | ||
|
|
||
| function getInteriorRanges( |
There was a problem hiding this comment.
Could use a jsdoc and a better name. Maybe like "getScopeInteriorRanges"? It's getting the top-level interior ranges for this scope right?
There was a problem hiding this comment.
Agreed
Exactly nested interiors are excluded.
|
@pokey Please have another look now. |
|
Ok I made some tweaks. I expanded the comment with an example, and then tweaked the range exclusion: I think we should exclude the target ranges rather than the domain. Wdyt? |
|
I like your changes. The only thing still on my mind is if we really want to use itertools. At most ifilter can remove a single scope. I think for readability reasons we should probably just go back to normal array operations. |
By utilizing interior scope in relative scope modifier we can skip the interior of scopes. See new test.