none-ls.nvim
none-ls.nvim copied to clipboard
🐛fix(client): we dont want to attach a source if its command is not executable.
(for sources which command is different than nil).
Problem
A explicitly registered external source will be attached even if the source command is not actually executable. This create several unwanted scenarios:
- When a user uninstall the mason package of a registered external source, none-ls currently attach it anyway.
- Trying to run the non existing command will display an error.
This is inconsistent with the behavior of builtins, which will only attach if the executable is available. This inconsistent behavior is likely to be a regression.
Example
The next formatter appear attached even though we uninstalled it and rebooted neovim (bottom right of the image).
Trying to run the formatter will of cause an error.
Solution
- By not allowing clients to be attached if their executable do not exist, it's not possible to have an inconsistent state anymore.
- Builtins and external sources now behave the same.
closes https://github.com/nvimtools/none-ls.nvim/issues/90
Correct me if I'm wrong: seems https://github.com/nvimtools/none-ls.nvim/blob/f5632db2491fbe02b54f1a321a98548a8ba2bd15/lua/null-ls/sources.lua?plain=1#L101-L107 already handled this (generator._failed)?
Ok I just discovered the next:
none-ls architecture
- always makes you explicitly load the clients you want to use in
sources. - if you explicitely load a client, and its executable is not available, you will get a warning informing you (as in the screenshot above).
mason-null-ls
- it iterates all available builtins, and load them for you.
- But because mason-null-ls only look for builtins, it won't correctly load external sources automatically. As they live in
require("none-ls.formatting.mysource"),require("none-ls.formatting.mysource"), etc while regular builtins live inrequire("null-ls").builtins.formatting.mysource
This provokes mason-null-ls is not able to find external sources.
Possible solution A
Fixing the issue in mason-null-ls.
Pros: Fixes the issue. Everything keep working the same.
Possible solution B
Fixing the issue in none-ls (as in this PR).
Pros: It fixes the issue. Cons: It won't inform to users if the executable is not present anymore. It will just not load the source.
For now I'm gonna try to implement the solution in mason-null-ls. but I'm keeping the PR open.
I've implemented a plugin to load internal and external sources intelligently when using mason: https://github.com/Zeioth/none-ls-autoload.nvim
But I would still consider merging this PR, so we don't register things we know for a fact are not going to work when trying to run them.
Because the tests are not passing, and we already have the plugin to solve this. Let's close the PR.