none-ls.nvim icon indicating copy to clipboard operation
none-ls.nvim copied to clipboard

🐛fix(client): we dont want to attach a source if its command is not executable.

Open Zeioth opened this issue 1 year ago • 5 comments

(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). screenshot_2024-05-10_18-49-50_149890376

Trying to run the formatter will of cause an error. screenshot_2024-05-10_18-51-58_029003400

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.

Zeioth avatar May 11 '24 03:05 Zeioth

closes https://github.com/nvimtools/none-ls.nvim/issues/90

Zeioth avatar May 11 '24 03:05 Zeioth

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 in require("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.

Zeioth avatar May 12 '24 16:05 Zeioth

For now I'm gonna try to implement the solution in mason-null-ls. but I'm keeping the PR open.

Zeioth avatar May 12 '24 16:05 Zeioth

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.

Zeioth avatar May 20 '24 21:05 Zeioth

Because the tests are not passing, and we already have the plugin to solve this. Let's close the PR.

Zeioth avatar Jun 11 '24 11:06 Zeioth