-
-
Notifications
You must be signed in to change notification settings - Fork 170
languages/haskell: remove invalid vim.g options #1191
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
base: v0.8
Are you sure you want to change the base?
Conversation
| servers = { | ||
| hls = { | ||
| enable = false; | ||
| cmd = [(getExe' pkgs.haskellPackages.haskell-language-server "haskell-language-server-wrapper") "--lsp"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just tested it. Without the cmd option, nvim behaves the same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that we should still reference in manually, like the rest of the packages.
cmd = [(getExe' pkgs.haskellPackages.haskell-language-server "haskell-language-server") "--lsp"];so that we are explicit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that the hls is compiled for specific ghc version. If we do specify it like you suggested, then you would be stuck on a specific ghc, and would prevent haskell-tools choosing appropriate hls for haskell projects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a fair point and a little bit of a weak suit for HLS imo haha.
Perhaps we could make it a string instead to allow the user to change it if need be.
e.g.
cmd = "haskell-language-server --lsp"and then the plugin can throw an error if necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we do specify it like you suggested, then you would be stuck on a specific ghc, and would prevent haskell-tools choosing appropriate hls for haskell projects.
I'm pretty sure haskell-tools looks for a usable hls in PATH regardless of whether cmd is set. Or I could be misremembering.
Either way, the purpose of the language modules are to provide plug-and-play defaults. In this case the closest we can get is having a default hls so that there is at least some chance of it working ootb for someone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since haskell-tools supports it, the hls configuration was moved to vim.lsp.servers to allow users to override cmd, and avoid unnecessary dependencies
|
Is this here only to have consistent interface with languages? This option is not used, and if it was, we would have to differentiate between configuration with custom plugin (like I have two proposals:
How is this handled in other languages with similar issues? |
|
@siggsy yes it was purely for consistent interface when I made the changes. At the moment we do not have any tools type interface, such as for rust (with rustaceanvim) or anything else. Both rustaceanvim and haskell-tools.nvim support importing the config from Perhaps an API such as what we have with the Where we have {
vim.languages.haskell.enable = true; # just enables HLS as a purely with `vim.lsp.config`
vim.languages.haskell.extensions = {
haskell-tools-nvim.enable = true; # disables the vim.lsp.enable of hls and uses haskell-tools.
};
# and for rust if we go this route
vim.languages.rust.enable = true; # enables just rust-analyzer via `vim.lsp.config`
vim.languages.rust.extensions = {
rustaceanvim.enable = true; # disable `vim.lsp.enable` of rust-analyzer
crates-nvim.enable = true;
};
}I think that having the command in explicitly is better than not having the command, as this is what we have done for every other language module, and we should keep consistency in that regard. |
We can take it back out for now, if it didn't exist before v0.8.
I vaguely recall @NotAShelf wanting to remove the hard dependency we have on some language plugins (rustaceanvim etc.), perhaps this is an opportunity for that? |
Yeh I think that was on the initial language overhaul branch. Want me to make an issue? |
|
This configuration now works when users have GHC in path (otherwise |
Haskell is not configured with lspconfig, but rather with haskell-tools.nvim. This commit fixes "conversion" made in 39efdc7. Without this, users are met with nvim errors about invalid vim.g options
Theses are already provided by `haskell-language-server`. We also do not prvoide any packages to complement the specified settings, so it seems unecessary.
`haskell-language-server-wrapper` only selects the correct `haskell-language-server`. Snippet from hls documentation: > HLS is a binary that must be compiled with the same version of > GHC as the project you are using it on. For this reason it is > usually distributed as a collection of binaries, along with a > haskell-language-server-wrapper executable that selects the correct > one based on which version of GHC it thinks you are using. By providing `hls-wrapper` we pull A LOT of dependencies, but do not use them (around 6.7GB). This is unecessary since we then have to provide `haskell-language-server` in haskell projects bundled with compatable GHC.
This reverts commit 9181d4f.
hls-wrapper is a simple script to start the correct language server based on the currently oppened project. From nvf perspective, this makes it effectively useless. To allow haskell to work on nvf OOTB, we specify cmd with hls (not wrapper). NOTE: this pins the language server to specific GHC version. To circumvent this, users must override (lib.mkForce) vim.lsp.servers.haskell-tools.cmd with their own, or just specify it as [ "haskell-language-server-wrapper" "--lsp" ].
haskell-tools already starts the server with ftplugin. If server.enable is set, haskell-tools starts even on non haskell files

Haskell is not configured with lspconfig, but rather with haskell-tools.nvim. This commit fixes "conversion" made in 39efdc7.
Without this, users are met with nvim errors about invalid
vim.goptionsSanity Checking
nix fmt).#nix(default package).#maximal.#docs-html(manual, must build).#docs-linkcheck(optional, please build if adding links)x86_64-linuxaarch64-linuxx86_64-darwinaarch64-darwinAdd a 👍 reaction to pull requests you find important.