Add automatic download for haskell-language-server #97
Conversation
|
Also need to figure out how to do versioning, e.g. what happens when a new version of hls is released. Probably best to just see what the other languages are doing. If they aren't doing any version checks then it may just be fine to tell users to |
There was a problem hiding this comment.
Looks nice! A few things for discussion.
Also need to figure out how to do versioning, e.g. what happens when a new version of hls is released. Probably best to just see what the other languages are doing.
install-server gets this update? parameter that gets passed down to download-server-fn. I think if that's set you're supposed to force re-installing it. So that's a simpler button to push than wiping out the cache dir.
lsp-haskell.el
Outdated
| "GitHub releases url for haskell-language-server and its binaries. Make sure to include the trailing directory slash." | ||
| :type 'string | ||
| :group 'lsp-haskell | ||
| :package-version '(lsp-mode . "7.1")) |
lsp-haskell.el
Outdated
| (defun lsp-haskell--project-ghc-version () | ||
| (let* ((path-exe (executable-find "haskell-language-server-wrapper")) | ||
| (downloaded-exe (lsp-package-path 'haskell-language-server-wrapper)) | ||
| (wrapper-exe (if path-exe path-exe downloaded-exe))) |
There was a problem hiding this comment.
Doesn't the :system alternative for lsp-dependency handle this? I thought that meant that it would get it as a "system binary" (i.e. from the path) if possible.
| (let* ((proj-ghc-ver (lsp-haskell--project-ghc-version)) | ||
| (server-name (format "haskell-language-server-%s" proj-ghc-ver)) | ||
| (path-exe (executable-find server-name)) | ||
| (downloaded-exe |
There was a problem hiding this comment.
If I understand right, this will actually download the server when we try and start it up. That's not ideal, but I can see the problem: we don't know which version of the server to install until this point, and anyway the user might have multiple projects with different GHC versions.
I think it would be nicer if the downloading happened only in download-server-fn. But then I guess we'd just have to download the whole big archive. I think that's okay, though, it's not so big, and I don't see what else we can do to avoid having people download things on startup...
| ;; Should run under haskell-mode and haskell-literate-mode. We need to list the | ||
| ;; latter even though it's a derived mode of the former | ||
| :major-modes '(haskell-mode haskell-literate-mode) | ||
| ;; This is arbitrary. | ||
| :server-id 'lsp-haskell | ||
| :server-id 'haskell-language-server |
There was a problem hiding this comment.
Why this? Does it matter? In particular, I'm pretty sure it doesn't have to match the name used for lsp-depdendency.
| 'haskell-language-server-wrapper | ||
| (lambda () | ||
| (let* | ||
| ((proj-ghc-ver (lsp-haskell--project-ghc-version)) |
There was a problem hiding this comment.
So this will only DTRT if it's run from the project directory? I'm not sure what the right behavior for install-server should be, especially in a case like this where we effectively have multiple servers. Maybe needing to run it multiple times isn't crazy? Not sure.
(Again, this would be less of an issue if we just downloaded everything up front)
| (lsp-package-ensure | ||
| 'haskell-language-server-wrapper | ||
| (lambda () | ||
| (let* |
There was a problem hiding this comment.
Maybe a named function install-server :: Version -> IO () would be good, might be independently useful (users can call it directly if they want).
| :new-connection (lsp-stdio-connection | ||
| (lambda () | ||
| (let* ((proj-ghc-ver (lsp-haskell--project-ghc-version)) | ||
| (server-name (format "haskell-language-server-%s" proj-ghc-ver)) |
There was a problem hiding this comment.
I think we should support bare haskell-language-server (this insists it has the version suffix). People (like me!) who build HLS themselves will just have a binary called haskell-language-server.
Possibly this is a use for lsp-haskell-server-path: it could be used here if non-nil.
| :new-connection (lsp-stdio-connection (lambda () (lsp-haskell--server-command))) | ||
| :new-connection (lsp-stdio-connection | ||
| (lambda () | ||
| (let* ((proj-ghc-ver (lsp-haskell--project-ghc-version)) |
There was a problem hiding this comment.
For users who've set up the server binary themselves (and maybe set it with lsp-haskell-server-path) it would be nice to not force the use and download of the wrapper.
There was a problem hiding this comment.
Yeah, this is something that I've yet to figure out, don't take this code as the intended behaviour! I'm wondering how we should decide to do the automatic download: Check for lsp-haskell-server-path first and see if it exists?
There was a problem hiding this comment.
Yeah, something like that makes sense. So lsp-haskell-server-path would be how you signal "I know what I'm doing, please just get out of my way and run this binary". And if it's nil, we do the automatic downloading.
|
Do yall mind if I pick this up and continue this work? I'd like to start contributing to the haskell language server effort, and since I know emacs lisp farily well, this is an obvious place to start |
|
By all means! Do have a look at what |
Closes #92
Wont work on Windows yet because lsp-mode upstream doesn't support decompressing gzips yet there. Also not sure what to do with
lsp-haskell-server-pathvariables and friends. Probably needs a bit of a discussion