-
-
Notifications
You must be signed in to change notification settings - Fork 91
Change interior domain #3099
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.
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
base: main
Are you sure you want to change the base?
Change interior domain #3099
Conversation
data/fixtures/recorded/surroundingPair/parseTree/python/clearMatching10.yml
Outdated
Show resolved
Hide resolved
packages/cursorless-engine/src/processTargets/modifiers/InteriorStage.ts
Outdated
Show resolved
Hide resolved
packages/cursorless-engine/src/spokenForms/defaultSpokenFormMapCore.ts
Outdated
Show resolved
Hide resolved
packages/cursorless-org-docs/src/docs/contributing/scopes/surroundingPairInterior.mdx
Show resolved
Hide resolved
| // eg `inside funk` | ||
| // The reason for this being an every scope when we have an explicit scope | ||
| // type is because we are looking for interiors inside of the scope. We | ||
| // don't want a normal containing behavior that expands. | ||
| if (target.hasExplicitScopeType) { | ||
| const everyModifier = this.modifierHandlerFactory.create({ | ||
| type: "everyScope", | ||
| scopeType: { | ||
| type: "interior", | ||
| }, | ||
| }); |
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'm a bit surprised that this actually works. For example, in:
function aaa(bbb: ccc) {
ddd;
}If I say "clear inside funk air", how come you don't get:
function aaa(|) {
|
}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.
ohh I guess it's because (bbb: ccc) is surroundingPairInterior whereas this is explicitly requestiont interior?
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.
So that means that even for {}-style languages, we still need to define interior in tree-sitter if we want "inside funk" to work?
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.
Ok here's a pathological case:
function aaa(bbb = () => { return "ccc"; }) {
ddd;
}In this case, you'd expect "clear inside air" to give:
function aaa(bbb = () => { return "ccc"; }) {
|
}but instead you'd get:
function aaa(bbb = () => { | }) {
|
}I think this is an ok compromise, but just figured I'd flag to demonstrate why this approach is really just a hack that approximates what we actually want
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.
Exactly. inside scope completely rely on Tree sitter.
Maybe I can work around this. It's not perfect, but I think this is a decent solution for 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.
I had look at this and there's nothing in Tree sitter that can help us with this particular case with my suggested implementation. With that said I'm happy with this behavior for now. I think it's much better than today's implementation.
The only way to properly fix this is to use interior properties. I guess we could do both? Where it matters like function bodies we could use a property and for other things like if statements we use a separate scope? That would probably give us the best of both? Right now I can't think of any other case except functions where you could create the above problem. The question is if it worth the increased complexity for this edge case?
"take inside"with the cursor on a function name will not select function body anymore."take inside funk"will select function body."take inside state"will select all branch bodies in an if statement.