Skip to content
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

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

Closed
4 of 5 tasks
Zeioth opened this issue Mar 6, 2024 · 8 comments
Closed
4 of 5 tasks
Labels
bug Something isn't working external External issue

Comments

@Zeioth
Copy link

Zeioth commented Mar 6, 2024

FAQ

  • I have checked the FAQ and it didn't resolve my problem.

Issues

  • I have checked existing issues and there are no issues with the same problem.

Neovim Version

v0.9.5

Dev Version?

  • 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

x

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

  • 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 Zeioth added the bug Something isn't working label Mar 6, 2024
@Zeioth
Copy link
Author

Zeioth commented Mar 6, 2024

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
Copy link
Author

Zeioth commented Mar 6, 2024

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 mochaaP added the external External issue label Mar 7, 2024
@mochaaP
Copy link
Member

mochaaP commented Mar 7, 2024

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

@citosid
Copy link

citosid commented Mar 12, 2024

Nevermind.... I found my answer: #81

Error before I read...

Do you have any update on this one? It is happening to me for 4 libraries:

image

In my case I cannot migrate to shfmt since it is incompatible with some of our standards.

Here is my configuration (using lazy.vim):

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 Zeioth mentioned this issue May 10, 2024
1 task
@Zeioth
Copy link
Author

Zeioth commented May 10, 2024

See also: #122 (comment)

@Zeioth
Copy link
Author

Zeioth commented May 10, 2024

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
Copy link
Author

Zeioth commented May 11, 2024

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
Copy link
Author

Zeioth commented May 12, 2024

Issue located in null-ls-mason not having added support for external sources yet. Closing and moving the issue to #125

@Zeioth Zeioth closed this as completed May 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working external External issue
Projects
None yet
Development

No branches or pull requests

3 participants