mfussenegger / nvim-jdtls

Extensions for the built-in LSP support in Neovim for eclipse.jdt.ls
GNU General Public License v3.0
1k stars 62 forks source link

adjusted the interface of `jdtls.setup.start_or_attach` #556

Closed bugabinga closed 9 months ago

bugabinga commented 10 months ago

the function start_or_attach has the same interface as vim.lsp.start now.

this helps to embed nvim-jdtls into nvim lua configurations, that wish to abstract over multiple different LSPs.

bugabinga commented 10 months ago

Hm, should a third argument be passed then?

function M.start_or_attach(config, opts,start_opts)
mfussenegger commented 10 months ago

Could you explain the use-case a bit more? I'm not sure if the opts from vim.lsp.start make sense in this context.

bugabinga commented 10 months ago

I am configuring my LSPs similar to how nvim-lspconfig does it. There is a collection of declarative "config" tables that each describe some LSP. I want one of these LSPs to be jdtls, but managed by nvim-jdtls.

local jdtls_lsp_start = function(config, opts)
  require 'jdtls'.start_or_attach(config, opts)
end

return {
  custom_start = jdtls_lsp_start,
  name = 'jdtls_ss',
  filetypes = 'java',
  command = create_jdtls_command(true),
  root_dir = project.find_java_project_root,
  single_file_support = true,
  workspaces = false,
  settings = { java = java_settings },
}

Ultimately, the config tables essentially are what gets passed into vim.lsp.start as config.

    local lsp_starter = config.custom_start or vim.lsp.start

    lsp_starter( start_config, {
      bufnr = bufnr,
      reuse_client = function ( existing_client, config )
        return existing_client.name == config.name and existing_client.root_dir == config.root_dir;
      end,
    } )

Another possibility to achieve this might be to use the before_init function in vim.lsp.start config, like neodev does it. But I suspect the setup nvim-jdtls does is much more complicated than just augmenting the LSP settings.

bugabinga commented 9 months ago

Should I:

I do not mind maintaining my own fork of nivm-jdtls since it is likely I am the only one with this esoteric requirement ;).

mfussenegger commented 9 months ago

Merged https://github.com/mfussenegger/nvim-jdtls/pull/578