LunarVim icon indicating copy to clipboard operation
LunarVim copied to clipboard

fix(lsp): create a proper way of removing items from skipped_servers

Open abzcoding opened this issue 3 years ago • 9 comments

Description

vim.tbl_map is unreliable (especially on mac), so just provide a simple function to remove item from map

for example you can add this to your config.lua

local tbl = require "lvim.utils.table"
tbl.remove(lvim.lsp.automatic_configuration.skipped_servers, "tailwindcss")

Fixes #2501

abzcoding avatar Apr 22 '22 11:04 abzcoding

I must've copied the snippet wrong somewhere, because it should've been using vim.tbl_filter, but either of them don't mutate the original table, so the full syntax would end up looking like this

lvim.lsp.automatic_configuration.skipped_servers = vim.tbl_filter(function(s)
  return s ~= "tailwindcss"
end, lvim.lsp.automatic_configuration.skipped_servers)

which as you can see, is not that easy to read/follow 🥲

I've been contemplating adding a function similar to what you have created, but instead have it directly accessible

---remove a server or list of servers from automatic configuration's skiplist
---@param s string|table can be a string or a list of strings, e.g. "tailwindcss" or {"emmet_ls", "eslint"}
local function unskip(s)
  if type(s) == "string" then
    s = { s }
  end
  for _, item in ipairs(s) do
    tbl_remove(lvim.lsp.automatic_configuration.skipped_servers, item)
  end
end
lvim.lsp.automatic_configuration.unskip = unskip -- could instead be called "enable" or something

kylo252 avatar Apr 23 '22 08:04 kylo252

with the function that you suggested, we would have three categories

  • skipped servers
  • unskipped server
  • we don't care servers

which is a bit weird I believe having a skipped list and then removing items from it makes more sense, but it's up to you.

abzcoding avatar Apr 23 '22 10:04 abzcoding

I was thinking of it more of a helper, but you're right, it's adding to the confusion instead of helping 😄

One thing I also realized is that removing servers from the skipped list was only necessary when the installer wasn't allowed to run for them, but that has changed in 5edbd91 (#2243), so it's basically enough to add this call somewhere:

require("lvim.lsp.manager").setup("tailwindcss", {})

pros

  • doesn't require any cache reset
  • ability to customize the setup opts

cons

  • the server still shows under skipped servers in LvimInfo
  • ??

kylo252 avatar Apr 23 '22 11:04 kylo252

I was thinking of it more of a helper, but you're right, it's adding to the confusion instead of helping 😄

One thing I also realized is that removing servers from the skipped list was only necessary when the installer wasn't allowed to run for them, but that has changed in 5edbd91 (#2243), so it's basically enough to add this call somewhere:

require("lvim.lsp.manager").setup("tailwindcss", {})

pros

  • doesn't require any cache reset
  • ability to customize the setup opts

cons

  • the server still shows under skipped servers in LvimInfo
  • ??

hmm, it just got interesting 😅 not sure which route to go tbh, maybe keep this open for a bit

abzcoding avatar Apr 23 '22 11:04 abzcoding

hmm, it just got interesting 😅 not sure which route to go tbh, maybe keep this open for a bit

as a quick and easy solution, I would remove the snippet entirely from config.example.lua and replace it with a commented call to one of the popular servers that are skipped, e.g. tailwindcss/emmet_ls/etc

-- all ts/js servers are skipped by default except for tsserver
require("lvim.lsp.manager").setup("tailwindcss", {})

and then fix this snippet on the site https://www.lunarvim.org/troubleshooting/#is-it-overriden

lvim.lsp.automatic_configuration.skipped_servers = vim.tbl_filter(function(s)
  return s ~= "tailwindcss"
end, lvim.lsp.automatic_configuration.skipped_servers)

kylo252 avatar Apr 23 '22 12:04 kylo252

so which one do you prefer?

lvim.lsp.automatic_configuration.skipped_servers = vim.tbl_filter(function(s)
  return s ~= "tailwindcss"
end, lvim.lsp.automatic_configuration.skipped_servers)

or

local tbl = require "lvim.utils.table"
tbl.remove(lvim.lsp.automatic_configuration.skipped_servers, "tailwindcss")

?

abzcoding avatar Apr 24 '22 05:04 abzcoding

so which one do you prefer?

I'll try to get some opinions on discord about this.

Another potential contender is implementing a tbl.remove_if which should be a little more flexible than tbl.remove and fits more with the theme of table helpers that take a predicate, here's an implementation for it https://github.com/aiq/luazdf/blob/0517286dcbe90d44ceeb05b22c143fa55703d1e9/arr/removeif/removeif.lua

kylo252 avatar Apr 24 '22 07:04 kylo252

Is there a way to skip multiple servers. It would be nice to have something like {"tailwindcss", "eslint", "emmet_ls"}.

I used to do require("lvim.lsp.manager").setup("tailwindcss") but after clean install it stopped working.

namimo avatar Jul 17 '22 05:07 namimo

Is there a way to skip multiple servers. It would be nice to have something like {"tailwindcss", "eslint", "emmet_ls"}.

I used to do require("lvim.lsp.manager").setup("tailwindcss") but after clean install it stopped working.

do you mean for un-skipping? because calling the setup('tailwindcss')is enough,

either way, it's just a simple lua table that you can alter however you want

:lua =lvim.lsp.automatic_configuration.skipped_servers

kylo252 avatar Jul 17 '22 17:07 kylo252

This is not working:

require("lvim.lsp.manager").setup("tailwindcss")

This is working:

local unskipServers = { "tailwindcss", "eslint" }
for _, item in ipairs(unskipServers) do
	lvim.lsp.automatic_configuration.skipped_servers = vim.tbl_filter(function(server)
		return server ~= item
	end, lvim.lsp.automatic_configuration.skipped_servers)
end

The bad thing about this is every project now runs this server whether or not tailwind/eslint is configured in the project. Tailwind is fine but eslint pops this message [lspconfig] unable to find eslint library every time i open a file. And if I add deno to the list node project confilcts with deno errors. Shouldn't it check for root_pattern.

I dont know either neovim config or lua and hacked around using the code above.

namimo avatar Aug 12 '22 08:08 namimo

@namimo, the change to unblock tailwindcss was recent #2870, so you don't need to alter it anymore.

for eslint, you can use this something like

-- in ~/.config/lvim/after/ftplugin/typescript.lua
local util = require "lspconfig.util"
local opts = {
  root_dir = util.root_pattern(
    ".eslintrc",
    ".eslintrc.js",
    ".eslintrc.cjs",
    ".eslintrc.yaml",
    ".eslintrc.yml",
    ".eslintrc.json"
  ),
}
require("lvim.lsp.manager").setup("eslint", opts)

you could duplicate in other ftplugin files (that's how lunarvim works internally), or you could also set it up once in config.lua.

kylo252 avatar Aug 12 '22 15:08 kylo252

Should we close this due to #2887 ? I'm not in favor of adding another function, removing items from a table is straightforward...

lvimuser avatar Sep 04 '22 21:09 lvimuser

the root cause of tis was that the tbl_map was not properly understood/used. I never had a single reliability problem with it, and I worked already on several macs (both x86 and M1). Adding another function that does something is very simple to do with native functions seems like a bad idea

danielo515 avatar Sep 12 '22 15:09 danielo515