feat(lsp): Add textDocument/completion to LSP#2398
Conversation
00796d1 to
92c4ca7
Compare
There was a problem hiding this comment.
Going through and adding some clarifying comments throughout. Had to expand this separately since the implementation for native vs js backends for grain-tree-sitter.ml use different underlying libraries for each target (ocaml tree_sitter bindings vs wasm tree-sitter-web bindings). Not sure if there's a better way to do it here or in the layer I wrote but open to feedback
There was a problem hiding this comment.
@spotandjake mentioned wanted to have a discussion about how is best to test the lsp and completions going forwards. Obviously completions need to be thoroughly tested but the current manual positioning method could probably use some work. Will leave it up to Jake to explain more but my current thoughts are to have a helper function and a position and/or range character
for example
let x: §
where the section sign (§) specifies where the cursor is. And for ranges to use « and » for ranges
| @@ -1,4 +1,4 @@ | |||
| (lang dune 3.0) | |||
| (lang dune 3.7) | |||
There was a problem hiding this comment.
This had to be changed to allow for (include_subdirs qualified) (https://dune.readthedocs.io/en/stable/reference/dune/include_subdirs.html#qualified) to make it part of a module hierarchy. This was mainly done for organizational reasons. In my testing I haven't observed anything breaking due to this change. Happy to discuss further
| }, | ||
| "dependencies": { | ||
| "@grain/binaryen.ml": ">= 0.34.0 < 0.35.0", | ||
| "@grain/grain-tree-sitter.ml": "*", |
There was a problem hiding this comment.
the intent here would be to ideally have grain-tree-sitter.ml and tree-sitter-grain (which I should probably change the name of one of them, lol) to become part of the grain project (not the repo but rather than org and under the grain umbrella)
| %token EXCEPT [@keyword] FROM [@keyword] STAR | ||
| %token SLASH DASH PIPE | ||
| %token EOL EOF | ||
|
|
There was a problem hiding this comment.
on L46 there's the reserved keywords not used yet. Whenever these are added, they'd need a attribute applied as well
| "assets": "bin/*.js" | ||
| "assets": [ | ||
| "bin/*.js", | ||
| "bin/*.wasm" |
There was a problem hiding this comment.
wasm file needed for js side of the completions
|
A couple of other things I want to bring up related to tree-sitter. I think tree-sitter gives us great opportunity elsewhere within the language server.
|
| | Some(_) => | ||
| let command: Protocol.command = { | ||
| title: "", | ||
| command: "editor.action.triggerSuggest", |
There was a problem hiding this comment.
when doing snippets, unfortunately we can't force the LSP to automatically suggest options. This is the closest I was able to figure out but unfortunately editor.action.triggerSuggest is vscode (and derivative) specific.
Kara-Zor-El
changed the title
textDocument/Completions to LSPtextDocument/completion to LSP
…er keyword generator
| type t = {capabilities: lsp_capabilities}; | ||
|
|
||
| let capabilities = { | ||
| position_encoding: "utf-8", |
There was a problem hiding this comment.
NOTE: This is used because grain assume UTF-8. It should be noted that the LSP protocol requires UTF-16 support so this is not spec compliant. The only major editor I am aware of that does not have UTF-8 support is Zed.
7ebfe9a to
8043c41
Compare
only done since its mandatory and vscode-languageclient doesn't support it
ffd3f53 to
e3ebe83
Compare
1 participant
Hey, been working on getting completions working in the LSP. So far its going well but hopefully much more cleanup and work to come.
This pr adds context-aware
textDocument/completionsupport to the LSP.Completions are driven by Tree-sitter for syntax context (so they still work on incomplete / broken code while editing) and supplements it with the typed environment when we have a successful compilation. Depending on where the cursor is, we can offer keywords, in-scope values/types, import paths, module names, graindoc attributes, etc.
Huge credit to @spotandjake for helping out and answering questions throughout as well as his immense help with the tree-sitter work. Work here is in part based on his earlier work in #1900
Currently I am using:
Currently tree-sitter-grain is licensed as MIT. I think currently Jake and Oscar mentioned we might be able to change the license for the parser code to MIT. Will wait for confirmation but can change to LGPL-3 for the time being.
I do still have a couples areas I am trying to smoothen out and squash a few bugs so will leave this as a draft for now and would love some feedback even if not in a complete state