feat: Add support for LSP 2 #2560
Conversation
|
@mmhat Rather obvious in hindsight, I've now updated the stack configuration to use the newer |
|
@Vekhir Thank you for doing this! I am about to review the work you did so far. Regarding the Hydra issue: # We assume that the current directory is the root of the checked out work tree.
# Start a container with the work tree mounted under /work.
# (I use podman nowadays; docker should work as well.)
podman run -it --rm -v .:/work -w /work docker.io/nixos/nix
# Now we are in the container. We need to install cabal-install and cabal2nix:
nix-env -i cabal-install -i cabal2nix
# cabal2nix needs a hackage index tarball:
cabal update
# Now we generate the Nix expression for the lsp-2.1.0.0 package and store it in the right location:
cabal2nix cabal://lsp-2.1.0.0 > ./nix/packages/lsp.nix |
| @@ -132,7 +133,7 @@ rangeToJSON (Range (x1,y1) (x2,y2)) = | |||
|
|
|||
| hoverHandler :: Handlers HandlerM | |||
| hoverHandler = | |||
| LSP.requestHandler STextDocumentHover \request respond -> handleErrorWithDefault respond Nothing do | |||
| LSP.requestHandler SMethod_TextDocumentHover \request respond -> handleErrorWithDefault respond (maybeToNull Nothing) do | |||
There was a problem hiding this comment.
| LSP.requestHandler SMethod_TextDocumentHover \request respond -> handleErrorWithDefault respond (maybeToNull Nothing) do | |
| LSP.requestHandler SMethod_TextDocumentHover \request respond -> handleErrorWithDefault respond (InR Null) do |
There was a problem hiding this comment.
Just saying Null here leads to ambiguity about Aeson.Null vs LSP.Types.Null, but otherwise is fine.
| @@ -211,23 +212,23 @@ documentLinkHandler = | |||
| filePath <- localToPath prefix file | |||
| let filePath' = basePath </> filePath -- absolute file path | |||
| let _range = rangeToJSON range_ | |||
| let _target = Just (filePathToUri filePath') | |||
| let _target = Just (Text.pack filePath') | |||
There was a problem hiding this comment.
It appears to me that filePathToUri does a bit more than converting the FilePath to Text.
Maybe
| let _target = Just (Text.pack filePath') | |
| let _target = Just (getUri (filePathToUri filePath')) |
is a bit more sensible here.
|
@mmhat I've implemented your suggestions and fixed the CI build - it will hopefully pass now. |
|
Hydra now works, lets see if the rest works now too |
|
This check will most probably fail for GHC 8.10 since |
lsp 2.x needs 'row-types'
|
Failed due to typo... Fixed it now |
|
@mmhat Looks like we are done. If you don't have any objections, you can merge it. |
|
Regarding a new release: Those are done by @Gabriella439 , so I think we have to be a bit more patient. EDIT: Never mind, Github managed to merge it but the UI didn't show. |
|
@Vekhir Nice, excellent work! Thank you! |
|
It's not urgent anyway. Thanks for your suggestions and reviewing! |
* feat: Add support for LSP 2 * fix: Update CI lsp versions lsp 2.x needs 'row-types' --------- Co-authored-by: Mann mit Hut <[email protected]>
@mmhat
Building upon #2531, this further adds support for
lsp 2.1lsp-types 2.0lsp-test 0.16This is a minimum refactoring by essentially following the compile errors and fixing them one by one without major structural changes. Most changes are just renaming.
No thoughts were given to backwards compatibility within LSP - the above versions are also the minimum requirement for
dhall-lsp-server.This is consistent with GHC 9.6.
Note
I had this diff lying around for a few days and it has been verified in my build for updating Arch packages to GHC 9.6.
Supersedes #2428