Ws symbols (continues from #510) #520
Conversation
|
linux: |
make clean make -j test
|
On my dev machine |
| function workspaceSymbol(msg: p.RequestMessage) { | ||
| // https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#workspace_symbol | ||
| let result: p.WorkspaceSymbol[] | null = utils.runAnalysis( | ||
| ["workspaceSymbols", process.cwd()], |
There was a problem hiding this comment.
This does not look right: process.cwd is not the current project.
CC @zth
There was a problem hiding this comment.
Yeah this looks off. I wonder what to put there though, I guess we're looking for the current workspace root?
There was a problem hiding this comment.
Current file I guess, if the extension gives it to you? Or whatever relevant info is given by vscode.
There was a problem hiding this comment.
The spec doesn't seem to give us anything of value: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#workspaceSymbolParams
I guess we might have the workspaces by looking at the opened file contents cache. But that feels a bit tricky as that means it won't work if no file has been opened. Not sure how to proceed here.
There was a problem hiding this comment.
If no file open vscode does nothing at all.
Can't expect this feature to be different.
There was a problem hiding this comment.
The only possible choice I think is to look at the list of opened files, and get the list of opened projects from there.
Unless one can also have info about the current file.
What do other extensions do in that regard?
There was a problem hiding this comment.
We can support DidChangeWorkspaceFolders notification and register a cache. ocaml-lsp supports this notification.
There was a problem hiding this comment.
That sounds like a good way to go.
I think it can be done/tested separately, then integrated in the PR?
There was a problem hiding this comment.
I think it can be done/tested separately, then integrated in the PR?
Sure
There was a problem hiding this comment.
Yes, that sounds like a great solution!
rebased on master of #510