feat(pack): create multiple typescript packs

[owittek]

As described here I am looking to add separate packs for both tsserver, denols & tsserver + denols for TypeScript developers. This is a first attempt at doing it and the changes are yet to be tested. The reason why I already open a PR is so the original authors of the typescript pack can take a look at it and possibly extend the functionality (especially to the denols pack) to create a more mature prduct.

Personally I'd also consider adding eslint_d to the tsserver pack, please let me know your opinions about that.

Resolves #120

[mehalter]

Thanks for starting to take a look at this. I do recommend we don't rename the existing typescript pack. This isn't a horrible name, will cause a lot of breaking changes to existing users, and you can still add the others. Also a bad habit in this community repo to effectively rename other people's contributions/"maintained" packages if you will

[owittek]

Thanks for starting to take a look at this. I do recommend we don't rename the existing typescript pack. This isn't a horrible name, will cause a lot of breaking changes to existing users, and you can still add the others. Also a bad habit in this community repo to effectively rename other people's contributions/"maintained" packages if you will

I have reverted the renaming commit.

@ALL the project root detection is still broken which leads to tsserver being loaded when it shouldn't be.
@mehalter suggested the following when we were discussing this issue on discord. We shouldn't modify the root detection functions. We should be disabling the autostart and the making an autocmd that:

1. calculate the root directories of deno and tsserver
2. see which one is more specific
3. choose the more specific one
4. start the appropriate language server

I'll look into it but suggestions and direct help is always appreciated since I'm not the most experienced person when it comes to nvim/lsp configuration.

[mehalter]

@owittek just heads up, I edited your comment a bit to fix my suggestion with more detail

[mehalter]

As mentioned by @owittek these plugin packs don't work as intended. Marking as changes requested just in case anyone wants to pick this up to finish it. It's pretty close though!

[owittek]

I also want to add the dap & neo-tree configuration to denols before merging it.

[mehalter]

@owittek did a bit of investigation into this. You could look into doing something like this:

vim.api.nvim_create_autocmd("LspAttach", {
  callback = function(args)
    local bufnr = args.buf
    local curr_client = vim.lsp.get_client_by_id(args.data.client_id)
    -- if deno attached, stop all typescript servers
    if curr_client.name == "denols" then
      vim.lsp.for_each_buffer_client(bufnr, function(client, client_id)
        if client.name == "tsserver" then vim.lsp.stop_client(client_id, true) end
      end)
    -- if tsserver attached, stop it if there is a denols server attached
    elseif curr_client.name == "tsserver" then
      for _, client in ipairs(vim.lsp.get_active_clients { bufnr = bufnr }) do
        if client.name == "denols" then
          vim.lsp.stop_client(curr_client.id, true)
          break
        end
      end
    end
  end,
})

You will still need to make sure you update the denols root_dir to be like this

  root_dir = require("lspconfig.util").root_pattern("deno.json", "deno.jsonc"),

This seems to work pretty well, but it definitely feels a bit hacky. It would be much more preferable to just disable auto starting and calculate your own logic to attach the clients to buffers individually.

[owittek]

I have yet to test the changes but I wouldn't mind getting feedback on the approach that I'm taking with the packs.

[owittek]

@mehalter please review the changes & merge it if everything looks good.

EDIT: For some reason the automatic installation does not work.

[luxus]

did the involvement of babu in chat helped?

[owittek]

did the involvement of babu in chat helped?

I didn't get any input unfortunately, I'll see if @folke might be able to help since it might be an issue with config overriding by lazy.nvim.

[folke]

What automatic installation are you referring to? mason-null-ls? is that plugin being loaded?

[owittek]

What automatic installation are you referring to? mason-null-ls? is that plugin being loaded?

Awesome that you've responded here, I was planning to open a help request.

If you look at the typescript-denols-and-tsserver.lua you can see that I'm importing both the denols & tsserver configs. Both configs override the mason-null-ls config (yep, that's a plugin that both configure) & importing both leads to wonky behavior.

I can print from both functions but the config does not seem to work properly.

Edit: ensure_installed is a table within mason-lsp-config & mason-null-ls that tells the plugins to install specific LSPs, linters, formatters & DAPs when they are not present. This seems to not work anymore with my setup where I build the specific configs from a generic configuration that is requires for both LSPs.

[folke]

Again, is mason-null-ls even loaded? Either way, no idea. I'm pretty sure config merging works as expected. Try adding some print statements inside the mason-null-ls thing to see the resolved config. Also remember that if you're configuring a list at two places, then lists are not going to be merged. To merge them manually, make the relevant opts a function and use vim.list_extend to extend the list

[folke]

just to be clear, add something like:

config = function(_, opts)
  vim.pretty_print(opts)
  require("mason-null-ls").setup(opts)
end

[owittek]

Again, is mason-null-ls even loaded? Either way, no idea. I'm pretty sure config merging works as expected. Try adding some print statements inside the mason-null-ls thing to see the resolved config. Also remember that if you're configuring a list at two places, then lists are not going to be merged. To merge them manually, make the relevant opts a function and use vim.list_extend to extend the list

That's exactly what I'm doing, astrocommunity has a util function called list_insert_unique for that and I'm using it in both functions since they use the same base configuration

[owittek]

just to be clear, add something like:

config = function(_, opts)
  vim.pretty_print(opts)
  require("mason-null-ls").setup(opts)
end

Seems like nothing is being printed right now. A small correction on my part: opts is a function that adds to the ensure_installed table, I'm not changing config.

[folke]

If nothing gets printed after you add that config then it means the plugin isn't loaded, so yes automatic install won't happen.

[owittek]

If nothing gets printed after you add that config then it means the plugin isn't loaded, so yes automatic install won't happen.

Makes sense but what could be the reason for the plugin not being loaded? This is what I return to only configure tsserver for example:

{
  {
    import = "astrocommunity.pack.json"
  },
  { "nvim-treesitter/nvim-treesitter",
    opts = <function 1>
  },
  { "williamboman/mason-lspconfig.nvim",
    config = <function 2>,
    opts = <function 3>
  },
  { "jay-babu/mason-nvim-dap.nvim",
    opts = <function 4>
  },
  { "jay-babu/mason-null-ls.nvim",
    opts = <function 5>
  },
  { "jose-elias-alvarez/typescript.nvim",
    ft = { "javascript", "typescript", "javascriptreact", "typescriptreact" },
    init = <function 6>,
    opts = <function 7>
  },
  { "nvim-neo-tree/neo-tree.nvim",
    opts = {
      event_handlers = { {
          event = "file_moved",
          handler = <function 8>
        }, {
          event = "file_renamed",
          handler = <function 8>
        } }
    }
  },
  { "vuki656/package-info.nvim",
    config = true,
    event = "BufRead package.json",
    requires = "MunifTanjim/nui.nvim"
  },
  { "mfussenegger/nvim-dap",
    config = <function 9>,
    dependencies = { { "mxsdev/nvim-dap-vscode-js",
        opts = {
          adapters = { "pwa-node" },
          debugger_cmd = { "js-debug-adapter" }
        }
      }, { "theHamsta/nvim-dap-virtual-text",
        config = true
      }, { "rcarriga/nvim-dap-ui",
        config = true
      } },
    enabled = true,
    ft = { "javascript", "typescript", "javascriptreact", "typescriptreact" }
  }
}

[folke]

I assume you have config.defaults.lazy = true? If so, it will never be loaded. Add it as a dependency somewhere, or set lazy=false for the plugin

[owittek]

That makes sense but AstroNvim itself configures Mason which requires all the plugins in its config. This works for a regular user configuration when changing the opts of those plugins so it's odd if I have to lazy load it myself. @mehalter could you please confirm/deny this?

-- I shortened the code just for demonstration
config = function(_, opts)
  require("mason").setup(opts)

  for _, plugin in ipairs { "mason-lspconfig", "mason-null-ls", "mason-nvim-dap" } do
    pcall(require, plugin)
  end
end

[folke]

Then there must be a bug in there, since it's not loaded. You can also check with :Lazy to see if the plugin is loaded or not.

[owittek]

Then there must be a bug in there, since it's not loaded. You can also check with :Lazy to see if the plugin is loaded or not.

Looks good at least, I'm really not sure about the cause of opts not being loaded.

Could you please explain how Lazy deals with multiple imports within a file if they override the same plugin specs? So in my case I have file A that imports file B and C but both B and C have a spec for the same plugin with different opts. How does Lazy deal with this situation and what happens if file A also has a spec for the same plugin?

[folke]

Sorry, but this is 100% for sure user error. I'm sure you'll figure it out. I have other stuff to do.

[owittek]

Sorry, but this is 100% for sure user error. I'm sure you'll figure it out. I have other stuff to do.

All good, thanks for your time!

[mehalter]

LGTM!