none-ls.nvim doesn't integrate so seamlessly with mason-null-ls.nvim anymore

Zeioth commented 5 months ago

FAQ

[X] I have checked the FAQ and it didn't resolve my problem.

Issues

[X] I have checked existing issues and there are no issues with the same problem.

Neovim Version

v0.9.5

Dev Version?

[X] I am using a stable Neovim release version, or if I am using a dev version of Neovim I have confirmed that my issue is reproducible on a stable version.

Operating System

arch

Minimal Config

Steps to Reproduce

x

Reproducibility Check

[ ] I confirm that my minimal config is based on the minimal_init.lua template and that my issue is reproducible by running nvim --clean -u minimal_init.lua and following the steps above.

Expected Behavior

x

Actual Behavior

x

Debug Log

x

Help

No

Implementation Help

Loading external builtins in the new way

require("null-ls").register(shellcheck_code_actions)

Suffices for end users, but it's not a good solution for distro maintainers.

Example

I ship my distro with shellcheck.
The final user decides to open mason and change his linter to a different one.

Result

Both shellcheck and the new installed linter are going to appear as loaded in :LspInfo, as shellcheck is now hardcoded in the config (which was not previously necessary).

Requirements

[X] I have read and followed the instructions above and understand that my issue will be closed if I did not provide the required information.

Zeioth commented 5 months ago

There is no problem in requiring a dependency, but it shouldn't be necessary to hardcode loading clients in the config. It beats the point of none-ls.

This is a problem only for external builtins. Native builtins don't suffer this problem.

In our case the native builtins are ok in 99% of cases, but we would still need to hardcode a couple of deprecated ones:

"gbprod/none-ls-shellcheck.nvim" -- to enable code actions on sh.
"gbprod/none-ls-luacheck.nvim"   -- selene is not a viable replacement for us. It support considerably less lints. It would also imply migrating all our projects to selene, which is not viable.

Zeioth commented 5 months ago

In the end we just migrated from luacheck to selene, and from beautysh to shfmt. While it's true these projects were stable and powerful, they are probably abandoned forever.

Feel free to close this issue if you consider it.

mochaaP commented 5 months ago

@jay-babu do you have time to take a look at this?

citosid commented 5 months ago

Nevermind.... I found my answer: https://github.com/nvimtools/none-ls.nvim/discussions/81

Error before I read...

Do you have any update on this one? It is happening to me for 4 libraries: ![image](https://github.com/nvimtools/none-ls.nvim/assets/201430/94f03db9-0cce-4152-b9a8-021da071d094) In my case I cannot migrate to `shfmt` since it is incompatible with some of our standards. Here is my configuration (using lazy.vim): ```lua return { { "jay-babu/mason-null-ls.nvim", event = { "BufReadPre", "BufNewFile" }, config = function() require("mason-null-ls").setup({ -- Each of one of these needs to be added in the configuration for none-ls.nvim ensure_installed = { -- Diagnostics "beautysh", "eslint_d", "hadolint", "markdownlint", -- This is both, formatter and diagnostics -- Formatters "black", "isort", "jq", "prettier", "shellcheck", "stylua", }, }) end, lazy = true, }, { "nvimtools/none-ls.nvim", dependencies = { "jay-babu/mason-null-ls.nvim", }, event = { "BufReadPre", "BufNewFile" }, lazy = true, opts = function(_, opts) local nls = require("null-ls") opts.sources = vim.list_extend(opts.sources or {}, { -- These come from the configuration for mason-null-ls.nvim -- Diagnostics nls.builtins.diagnostics.beautysh, nls.builtins.diagnostics.eslint_d, nls.builtins.diagnostics.hadolint, nls.builtins.diagnostics.markdownlint, -- Formatter nls.builtins.formatting.black, nls.builtins.formatting.isort, nls.builtins.formatting.jq, nls.builtins.formatting.markdownlint, nls.builtins.formatting.prettier, nls.builtins.formatting.shellcheck, nls.builtins.formatting.stylua, }) opts.on_attach = function(current_client, bufnr) if current_client.supports_method("textDocument/formatting") then vim.api.nvim_clear_autocmds({ buffer = bufnr }) vim.api.nvim_create_autocmd("BufWritePre", { buffer = bufnr, callback = function() vim.lsp.buf.format({ filter = function(client) -- only use null-ls for formatting instead of lsp server return client.name == "null-ls" end, bufnr = bufnr, }) end, }) end end end, }, } ```

Zeioth commented 3 months ago

I was thinking a possible solution would be not attaching a source if the command is not available on the terminal. I'm gonna see if I can pull it off during the weekend.

Zeioth commented 3 months ago

I already got a local workaround working:

  --  none-ls [lsp code formatting]
  --  https://github.com/nvimtools/none-ls.nvim
  {
    "nvimtools/none-ls.nvim",
    dependencies = {
      "jay-babu/mason-null-ls.nvim",
      "nvimtools/none-ls-extras.nvim",
    },
    event = "VeryLazy",
    opts = function()

      -- fix: Register the extra builtin reformat_gherkin only if the command is available.
      local gherkin_builtin = require("none-ls.formatting.reformat_gherkin")
      local gherkin_cmd = gherkin_builtin._opts.command
      if vim.fn.executable(gherkin_cmd) == 1 then
        require("null-ls.sources").register(gherkin_builtin)
      end

      return {} -- opts are not necessary for this example
    end
  },

This code will only try to load the extra builtin if its command is executable.

I don't know if I can implement this into none-ls itself so the user doesn't have to use hacks such as this. I'm gonna try.

Zeioth commented 3 months ago

Issue located in null-ls-mason not having added support for external sources yet. Closing and moving the issue to https://github.com/nvimtools/none-ls.nvim/issues/125

nvimtools / none-ls.nvim