Skip to content

Change scopeType matchers to rely on tree-sitter style scheme queries #620

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

Conversation

pokey
Copy link
Member

@pokey pokey commented Mar 24, 2022

Fixes #616

Checklist

{
"label": "Parse quarry",
"type": "shell",
"command": "npx tree-sitter query scopeTypes.scm ts-test.ts",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't get this one to work but will prob be useful to have a task or launch config to use while hacking. Maybe launch.json is better

@pokey
Copy link
Member Author

pokey commented Mar 24, 2022

Fwiw with this file:

ts-test.ts:

foo = {
  foo: {
    bar: "baz",
  },
};

bar = () => "baz";

Running

npx tree-sitter query queries/typescript/scopeTypes.scm  ts-test.ts

We get the following:

ts-test.ts
  pattern: 0
    capture: map, start: (0, 6), end: (4, 1)
  pattern: 5
    capture: key.iterationScope, start: (0, 6), end: (4, 1)
    capture: key.searchScope, start: (1, 2), end: (3, 3)
    capture: 2 - key, start: (1, 2), end: (1, 5), text: `foo`
  pattern: 3
    capture: key.iterationScope, start: (0, 6), end: (4, 1)
    capture: key.searchScope, start: (1, 2), end: (3, 3)
    capture: 2 - key, start: (1, 2), end: (1, 5), text: `foo`
    capture: 3 - key.removalRange.end, start: (1, 5), end: (1, 6), text: `:`
  pattern: 4
    capture: key.iterationScope, start: (0, 6), end: (4, 1)
    capture: key.searchScope, start: (1, 2), end: (3, 3)
    capture: 2 - key, start: (1, 2), end: (1, 5), text: `foo`
    capture: 6 - key.delimiter.trailing, start: (1, 5), end: (1, 6), text: `:`
  pattern: 0
    capture: map, start: (1, 7), end: (3, 3)
  pattern: 5
    capture: key.iterationScope, start: (1, 7), end: (3, 3)
    capture: 4 - key.searchScope, start: (2, 4), end: (2, 14), text: `bar: "baz"`
    capture: 2 - key, start: (2, 4), end: (2, 7), text: `bar`
  pattern: 3
    capture: key.iterationScope, start: (1, 7), end: (3, 3)
    capture: 4 - key.searchScope, start: (2, 4), end: (2, 14), text: `bar: "baz"`
    capture: 2 - key, start: (2, 4), end: (2, 7), text: `bar`
    capture: 3 - key.removalRange.end, start: (2, 7), end: (2, 8), text: `:`
  pattern: 4
    capture: key.iterationScope, start: (1, 7), end: (3, 3)
    capture: 4 - key.searchScope, start: (2, 4), end: (2, 14), text: `bar: "baz"`
    capture: 2 - key, start: (2, 4), end: (2, 7), text: `bar`
    capture: 6 - key.delimiter.trailing, start: (2, 7), end: (2, 8), text: `:`
  pattern: 1
    capture: 1 - anonymousFunction, start: (6, 6), end: (6, 17), text: `() => "baz"`

; (key: (_) @key
; .
; ":") @key.removalRange
; ) @key.searchScope
Copy link
Member Author

@pokey pokey Mar 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing to note for all our definitions of key is that the trailing , on the pair is not included in the searchScope. We could probably handle that one fairly easily, but I do wonder if we risk people forgetting to do this. As an alternative, if we had the searchScope instead point to the pair scope type, then we could potentially get that delimiter information for free if it's defined there. cc/ @wenkokke @soheeyang @Will-Sommers

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is @key.searchScope meant to do here? Wouldn’t we need to manually match the “,” token with a match, and bind it to, e.g, @key.removalRange?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

key.searchScope enables us to say "take key" from anywhere within the pair that contains the key, instead of the default searching behaviour which only matches if you're inside the scope itself

Copy link
Member Author

@pokey pokey Mar 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I tried the obvious thing for ,:

(
    (object
        (
            (pair
                key: (_) @key
                ":" @key.delimiter.trailing
            )
            ","?
        ) @key.searchScope
    ) @key.iterationScope
)

And the problem is that for sibling matches, it appears to only return the range of the first sibling, rather than the range of the two siblings. @Will-Sommers / @wenkokke have you seen this kind of thing? Do you know if we can get the full range from the web-tree-sitter api?

Here's the output on simplified code.

Query:

(((pair) ",") @test1)
(((pair) (pair)) @test2)
(("," (pair)) @test3)

Code:

foo = { bar: "baz", bongo: "bazman" };

Output:

    pattern:  0, capture: 0 - test1, start: (0, 8), end: (0, 18), text: `bar: "baz"`
    pattern:  1, capture: 1 - test2, start: (0, 8), end: (0, 18), text: `bar: "baz"`
    pattern:  2, capture: 2 - test3, start: (0, 18), end: (0, 19), text: `,`

Note that it is paying attention to the trailing sibling, because that's why it only matches the first pair for the first two patterns, but then it doesn't include that trailing sibling in the match range. Hopefully we can just easily get that info when we're using the api instead of the cli

Copy link
Collaborator

@Will-Sommers Will-Sommers Mar 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heyo — I think what we're looking for here is the [ ] alternator syntax. This will allow for a match of the first case or the second case.

(object
    [
        ((pair) ","? @delim)
    ]* @pair
)

yields separate match groups for each kv pair. If we add a * at the end, it will be grouped together. If we add a tag at the end of the , it will be included in the group as a tag. It would normally be excluded as it isn't a named node.

Is this what we want?

foo = {
  bar: "baz",
  bongo: "bazman",
  yar: "yaz"
};


(object
    [
        ((pair) "," @delim)
        (pair)
    ]* @pair
)

ts-test.ts
  pattern: 2
    capture: 2 - pair, start: (1, 2), end: (1, 12), text: `bar: "baz"`
    capture: 1 - delim, start: (1, 12), end: (1, 13), text: `,`
    capture: 2 - pair, start: (2, 2), end: (2, 17), text: `bongo: "bazman"`
    capture: 1 - delim, start: (2, 17), end: (2, 18), text: `,`
    capture: 2 - pair, start: (3, 2), end: (3, 12), text: `yar: "yaz"`

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still isn't grouping the pair and the delim for a removal range, still looking

Copy link
Collaborator

@Will-Sommers Will-Sommers Mar 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to open a ticket on the main project to check this, alternative in the breach is to check for the presence of a named delim node after a pair/key and add it to the removal list. At least it is in sequential order.

FYIW, it looks like there are some issues already related to anonymous captures being wonky in the project.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. Fwiw my goal for this specific experiment isn't removal range, but search range. Ie I want the following to work:

{foo: "bar", baz: "bongo"}

If your cursor is right after the , I'd like to be able to say "take key" and get foo.

I guess we could use the same workaround we were using for removalRange, where we allow a .end tag. So we could say:

(
    (object
         (pair
             key: (_) @key
             ":" @key.delimiter.trailing
         ) @key.searchScope
         ","? @key.searchScope.end
    ) @key.iterationScope
)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, got it.

(pair
key: (_) @key
.
":" @key.delimiter.trailing
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Advantage of labelling the delimiter itself is that we can then target it with the "trailing" modifier (#309)

Tho we can also do this with the approach below where we use #set!

But I think we should prob support all 3 tbh, as they could all be useful for different things

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question is whether we want to use @key.delimiter.trailing here or just @key.delimiter

@pokey pokey force-pushed the pokey/issue616-Change-scopeType-matchers-to-rely-on-tree-sitter-style-scheme-queries branch from 4d9c07a to 169038b Compare April 27, 2022 13:10
@pokey
Copy link
Member Author

pokey commented Aug 17, 2022

Closing this one in favour of #629, but there might be some interesting ideas in here for implementing more complex scope types

@pokey pokey closed this Aug 17, 2022
@pokey pokey deleted the pokey/issue616-Change-scopeType-matchers-to-rely-on-tree-sitter-style-scheme-queries branch February 16, 2023 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change scopeType matchers to rely on tree-sitter style scheme queries
3 participants