Skip to content
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

Add LSP communication layer #320

Merged
merged 48 commits into from
Apr 25, 2019
Merged

Add LSP communication layer #320

merged 48 commits into from
Apr 25, 2019

Conversation

Krzysztof-Cieslak
Copy link
Member

@Krzysztof-Cieslak Krzysztof-Cieslak commented Dec 31, 2018

This PR is adding communication layer implementing Language Server Protocol.

Implementation status

LSP messages

  • initialize - OK
  • initialized - OK
  • textDocument/hover - OK
  • textDocument/didOpen - OK
  • textDocument/didChange - OK
  • textDocument/completion - OK
  • completionItem/resolve - OK
  • textDocument/rename - OK
  • textDocument/definition - OK
  • textDocument/typeDefinition - OK
  • textDocument/implementation - OK
  • textDocument/codeAction - OK
  • textDocument/codeLens - OK
  • codeLens/resolve - OK
  • textDocument/references - OK
  • textDocument/documentHighlight - OK
  • textDocument/documentLink - In the future
  • documentLink/resolve - Nope
  • textDocument/signatureHelp - OK
  • textDocument/documentColor - Nope
  • textDocument/colorPresentation - Nope
  • textDocument/formatting - ?
  • textDocument/rangeFormatting - ?
  • textDocument/onTypeFormatting - ?
  • textDocument/willSave - Nope
  • textDocument/willSaveWaitUntil - Nope
  • textDocument/didSave - OK
  • textDocument/didClose - Nope
  • textDocument/documentSymbol - OK
  • workspace/didChangeWatchedFiles - OK
  • workspace/didChangeWorkspaceFolders - Nope
  • workspace/didChangeConfiguration - OK
  • workspace/symbol - OK
  • workspace/executeCommand - In the future (fsharp.generateDoc)
  • window/showMessage - OK
  • window/logMessage - OK
  • textDocument/publishDiagnostics - OK

Custom messages

  • fsharp/signature - OK
  • fsharp/signatureData - OK
  • fsharp/lineLens - OK
  • lineLens/resolve - OK
  • fsharp/workspaceLoad - OK
  • fsharp/compilerLocation - OK
  • fsharp/project - OK
  • fsharp/compile - OK
  • fsharp/notifyWorkspace - OK
  • fsharp/notifyWorkspacePeek - OK

Notes:

  • Current implementation was trying to do minimal changes in the other layers of FSAC to ensure it doesn't break other clients. As such, some implementations in LSP layer looks bit clunky. If we agree that LSP is way to go for all clients, we can deprecate other communication protocols, and do bigger refactoring of Core project.
  • Client is responsible for project loading/parsing/refreshing other than initial loading
  • Initial project loading is done automatically if and only if there exists single solution file in the workspace, or there is no solution files in the workspace (in such case all detected projects are loaded)
  • Custom endpoints are using (for messages body) PlainNotification type and string format serialized with exactly same serialization format as old JSON protocol
  • workspace/didChangeWatchedFiles should be used only for removing diagnostics from the files that were deleted.
  • Ionide's LineLenses needs to be implemented in client side using custom endpoints, it's too innovative for MSFT ;]

@Krzysztof-Cieslak Krzysztof-Cieslak changed the title Add LSP communication layer [WIP] Add LSP communication layer Dec 31, 2018
build.fsx Outdated Show resolved Hide resolved
build.fsx Outdated Show resolved Hide resolved
@@ -103,9 +120,14 @@ let sendSymbols (serializer: Serializer) fn (symbols: FSharpSymbolUse[]) =

let getSymbols symbolName =
makePostRequest ("http://localhost:" + (string port) + "/getSymbols") symbolName
|> Async.map (fun n ->
try
Some <| JsonConvert.DeserializeObject<SymbolUseRange[]> n
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe

Some (JsonConvert.DeserializeObject<SymbolUseRange[]> n)

is easier to ready, too many pipes with the generic

@Krzysztof-Cieslak
Copy link
Member Author

Wow, green. At least I haven't broken anything ;-)

@Krzysztof-Cieslak
Copy link
Member Author

Rebased on the latest master

@Krzysztof-Cieslak
Copy link
Member Author

I think this is more or less ready - all endpoints I've wanted to implement are done. From Ionide point of view there are still some... interesting... things about VSCode ordering calls differently than what we've done in Ionide, I feel that lack of control will still cause us problems, but we'll work that out when the Ionide integration will be done.

I still plan to write some tests for it before merging.

@Krzysztof-Cieslak
Copy link
Member Author

This is ready to merge (if CI is green) - if someone wanted to do some kind of code review, I'd welcome it, otherwise, I'll just merge it tomorrow morning and start working on Ionide integration. That should result in a huge set of fixes anyway ;-)

BTW - the new way of writing tests using LSP implementation is really nice and enables us to debug FSAC easily, we should get way more of those.

@cartermp
Copy link
Contributor

Greeeeeeeeen

@Krzysztof-Cieslak Krzysztof-Cieslak changed the title [WIP] Add LSP communication layer Add LSP communication layer Apr 25, 2019
@Krzysztof-Cieslak Krzysztof-Cieslak merged commit 060a242 into master Apr 25, 2019
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.

3 participants