-
-
Notifications
You must be signed in to change notification settings - Fork 57
feat: add tree-sitter-based token provider for GritQL #795
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?
Conversation
cbb9bb8
to
c6569b9
Compare
6f315cb
to
11c60c1
Compare
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.
Good start! I left some comments to address
src/gritql-token-provider.ts
Outdated
@@ -0,0 +1,113 @@ | |||
import * as vscode from "vscode"; | |||
|
|||
// TODO: Use tree-sitter for more accurate parsing once tree-sitter-gritql exports wasm file |
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.
This Todo should also explain how to address itself. Like, there's a reason why you couldn't implement it in this PR
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.
Thanks! I added more context in the TODO comment.
Essentially, honeycombio/tree-sitter-gritql#28 is the blocker.
d962d57
to
96d310c
Compare
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.
Thanks @ematipico for reviewing. I cleaned up my the PR. PTAL!
I got my prototype for tree-sitter integration working in daivinhtran#1. honeycombio/tree-sitter-gritql#28 is the blocker before we can move forward with tree-sitter.
Anyways, this PR is a stepping stone and will make the work easier down the road.
src/gritql-token-provider.ts
Outdated
@@ -0,0 +1,113 @@ | |||
import * as vscode from "vscode"; | |||
|
|||
// TODO: Use tree-sitter for more accurate parsing once tree-sitter-gritql exports wasm file |
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.
Thanks! I added more context in the TODO comment.
Essentially, honeycombio/tree-sitter-gritql#28 is the blocker.
d9ccbe6
to
f6b507e
Compare
@ematipico I reworked the PR to integrate with tree-sitter since honeycombio/tree-sitter-gritql#28 is now resolved. |
VSCode provides several ways to support highlighting
a. Using Regex to determine tokens
b. Tree-sitter integration: Use tree-sitter-gritql and map syntax nodes to semantic tokens
This PR aims for 2.b. and implements a mapper from
tree-sitter-gritql
's syntax nodes [1] to VSCode's sematic tokens [2]. This is the cleanest way since we get to avoid the whole legwork of comprehending the GritQL grammar from the ground up.[1] https://github.com/honeycombio/tree-sitter-gritql/blob/main/src/node-types.json
[2] https://code.visualstudio.com/api/language-extensions/semantic-highlight-guide#standard-token-types-and-modifiers
Fixes: #686
Checklist