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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
891c3be
Initial query experiments
pokey Mar 24, 2022
169038b
Tweak formatting
pokey Mar 24, 2022
41facd4
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .vscode/tasks.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@
"reveal": "never"
},
"group": "build"
},
{
"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

"presentation": {}
}
]
}
57 changes: 57 additions & 0 deletions queries/typescript/scopeTypes.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
(object) @map

(arrow_function) @anonymousFunction
(function) @anonymousFunction

; We could also have used an alternation here, but the above style is clean and
; seems to be more commonly used.
; [
; (arrow_function)
; (function)
; ] @anonymousFunction

; The following doesn't work because we can't have a `(` before the field name (`key`)
; (
; (object
; (pair
; (key: (_) @key
; .
; ":") @key.removalRange
; ) @key.searchScope
Copy link
Member Author

@pokey pokey Mar 24, 2022 •
edited
Loading

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

Copy link

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 •
edited
Loading

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 •
edited
Loading

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 •
edited
Loading

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.

; ) @key.iterationScope
; )

; As a workaround, we could allow defining the end of an annotation
(
(object
(pair
key: (_) @key
.
":" @key.removalRange.end
) @key.searchScope
) @key.iterationScope
)

; We could instead tag the delimiter itself. Gives us a bit more information,
; so maybe that's better?
(
(object
(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

) @key.searchScope
) @key.iterationScope
)

; We could also try setting an attribute. This approach is likely how we'd
; handle comma-separated lists, because they'd be a bit painful to define as
; parse tree patterns
(
(object
(pair
key: (_) @key
(#set! "delimiter" ":")
) @key.searchScope
) @key.iterationScope
)