modules/lsp: add inotify-tools package when any lsp server is enabled#4150
modules/lsp: add inotify-tools package when any lsp server is enabled#4150wvffle wants to merge 1 commit into
inotify-tools package when any lsp server is enabled#4150Conversation
f876170 to
de0d3db
Compare
|
Not sure about enabling it for any LSP, I don't have any LSP enabled that uses file watching. I think we can add |
My instinct is that LSPs that benefit from it should enable the dependency. Maybe we can implement a test that greps through I could be convinced that any LSP being installed should enable it if that is impractical to maintain that, though. Assuming installing it by default is relatively harmless & uncontroversial. |
Before this change `:LspInfo` complains about `vim.lsp` using `libuv-watchdirs` for file watcher: ``` vim.lsp: File Watcher ~ - File watch backend: libuv-watchdirs -⚠️ WARNING libuv-watchdirs has known performance issues. Consider installing inotify-tools. ``` This commit adds `inotify-tools` as extra package when any of the lsp servers is enabled via `lsp.servers.*.enable` option, so the warning goes away.
de0d3db to
1487b11
Compare
| dependencies.inotify-tools = lib.mkIf (enabledServers != [ ] && pkgs.stdenv.hostPlatform.isLinux) { | ||
| enable = true; | ||
| }; |
There was a problem hiding this comment.
-
As per plugins/lsp/language-servers/nixd: use dependencies.nixpkgs-fmt #4075 (comment) I'm unsure about continuing to spread usage of
dependenciesuntil we've ironed out how to advertise to users that a particular option/module enables a particular dependency. -
As per modules/lsp: add
inotify-toolspackage when any lsp server is enabled #4150 (comment) and modules/lsp: addinotify-toolspackage when any lsp server is enabled #4150 (comment), I'd ideally prefer to see a way of us maintaining this on a per-LSP-server basis, too.
I'm willing to overlook pt.1 if this is considered an important enough dependency that we need a solution now, before solving the wider dependencies questions.
Currently, I suspect pt.2 would be too high a maintenance burden. In the future if we had a way to assert specific :LspInfo output with nixvim's test module, this could become easier.
Perhaps we can address pt.2 by adding a TODO comment, along the lines of:
Ideally this dependency should be enabled by the servers that depend on inotify-tools, instead of globally for all servers
Before this change
:LspInfocomplains aboutvim.lspusinglibuv-watchdirsfor file watcher:This PR adds
inotify-toolsas extra package when any of the lsp servers is enabled vialsp.servers.*.enableoption, so the warning goes away.