AstroNvim / astrocommunity

A community repository of common plugin specifications
GNU General Public License v3.0
1.04k stars 214 forks source link

feat(cs): add `csharpls-extended-lsp.nvim` for better go to definition #1018

Closed mehalter closed 3 weeks ago

mehalter commented 1 month ago

Closes #951

📑 Description

This is untested because I don't have a C# environment. @Cretezy this is the correct way to set it up, it's the same way you mentioned you tried it. If it doesn't work it could just be the plugin is broken and we shouldn't add it. Could you give this a try?

ℹ Additional Information

github-actions[bot] commented 1 month ago

Review Checklist

Does this PR follow the [Contribution Guidelines](development guidelines)? Following is a partial checklist:

Proper conventional commit scoping:

Uzaaft commented 1 month ago

testing this since I have a working dotnet env on my pc

Cretezy commented 1 month ago

I'm not going to be doing C# until next Tuesday, I can test and review then

Uzaaft commented 1 month ago

Didnt work for me, the LSP in the pack is broken for Mac🥹

ahmtsen commented 3 weeks ago

I think the problem is caused by telescope as mentioned here https://github.com/Decodetalkers/csharpls-extended-lsp.nvim/issues/7. Maybe overriding gd to use vim.lsp.buf.definition() instead of telescope methods will solve the problem.

ahmtsen commented 3 weeks ago

I think the problem is caused by telescope as mentioned here Decodetalkers/csharpls-extended-lsp.nvim#7. Maybe overriding gd to use vim.lsp.buf.definition() instead of telescope methods will solve the problem.

The following configuration works for me. The fix is to override gd to use vim.lsp.buf.definition.

  {
    "Decodetalkers/csharpls-extended-lsp.nvim",
    dependencies = {
      {
        "AstroNvim/astrolsp",
        opts = {
          config = {
            csharp_ls = {
              handlers = {
                ["textDocument/definition"] = function(...) require("csharpls_extended").handler(...) end,
                ["textDocument/typeDefinition"] = function(...) require("csharpls_extended").handler(...) end,
              },
            },
          },
          on_attach = function(client, bufnr)
            if client.name ~= "csharp_ls" then return end

            vim.keymap.set(
              "n",
              "gd",
              vim.lsp.buf.definition,
              { buffer = bufnr, noremap = true, silent = true, desc = "Go to definition" }
            )
          end,
        },
      },
    },
  },
mehalter commented 3 weeks ago

Thanks for pointing this out @ahmtsen ! I am going to be removing the telescope LSP mappings in the next patch release of AstroNvim because apparently there are many problems with them. Also thanks for the workaround! Once the next release is made I'll go ahead and merge this in.

Also sidenote, you know you can also add on_attach functions to the config of language servers themselves to add settings for specific language servers. Rather than checking the client.name you can do something like this:

{
  "Decodetalkers/csharpls-extended-lsp.nvim",
  dependencies = {
    {
      "AstroNvim/astrolsp",
      opts = {
        config = {
          csharp_ls = {
            handlers = {
              ["textDocument/definition"] = function(...) require("csharpls_extended").handler(...) end,
              ["textDocument/typeDefinition"] = function(...) require("csharpls_extended").handler(...) end,
            },
            on_attach = function(_, bufnr)
              vim.keymap.set("n", "gd", vim.lsp.buf.definition, { buffer = bufnr, desc = "Go to definition" })
            end,
          },
        },
      },
    },
  },
},
mehalter commented 3 weeks ago

This should work as intended now that AstroNvim v4.10.2 is released that doesn't use Telescope for LSP mappings out of the box.