fix(cs-omnisharp): Fix bug in handler registration caused by wrong lsp name

[ahmtsen]

📑 Description

omnisharp-extended-lsp.nvim handlers are not registered correctly.

Language server name changed from csharp_ls to omnisharp.

Closes #1278

ℹ Additional Information

[github-actions]

Review Checklist

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

Proper conventional commit scoping:

• If you are adding a new plugin, the scope would be the name of the category it is being added into. ex. feat(utility): added noice.nvim plugin

• If you are modifying a pre-existing plugin or pack, the scope would be the name of the plugin folder. ex. fix(noice-nvim): fix LSP handler error

• Pull request title has the appropriate conventional commit type and scope where the scope is the name of the pre-existing directory in the project as described above

• README is properly formatted and uses fenced in links with <url> unless they are inside a [title](url)

• Entry returns a single plugin spec with the new plugin as the only top level spec (not applicable for recipes or packs).

• Proper usage of opts table rather than setting things up with the config function.

• Proper usage of specs table for all specs that are not dependencies of a given plugin (not applicable for recipes or packs).

[gepbird]

AFAIK csharp_ls and omnisharp-roslyn are 2 different projects. Does omnisharp-extended-lsp.nvim work with csharp_ls? Have you tested for example the go to definition function on a symbol that requires decompilation?

Oops I misread the changes, sorry. However I'm still surprised that this fix works. What about using custom commands instead of handlers?

[Uzaaft]

AFAIK csharp_ls and omnisharp-roslyn are 2 different projects. Does omnisharp-extended-lsp.nvim work with csharp_ls? Have you tested for example the go to definition function on a symbol that requires decompilation?

Oops I misread the changes, sorry. However I'm still surprised that this fix works. What about using custom commands instead of handlers?

Waiting on a reply on this before I merge. @ahmtsen

[ahmtsen]

AFAIK csharp_ls and omnisharp-roslyn are 2 different projects. Does omnisharp-extended-lsp.nvim work with csharp_ls? Have you tested for example the go to definition function on a symbol that requires decompilation?
Oops I misread the changes, sorry. However I'm still surprised that this fix works. What about using custom commands instead of handlers?

Waiting on a reply on this before I merge. @ahmtsen

Custom handlers extend the functionality of Neovim's built-in LSP functions, such as vim.lsp.buf.references(). The reason for me to avoid custom commands provided by omnisharp-extended-lsp.nvim is the need to map those commands to specific keys, which can create inconsistencies.

For example, if we map require('omnisharp_extended').lsp_references() to the gr key when omnisharp lsp is attached, but the user has mapped vim.lsp.buf.references() to another key, such as fr, they would expect the fr key to trigger the "go to references" functionality. However, this won't work without additional setup.

By using custom handlers instead of custom commands, this pack extends the functionality of Neovim's built-in functions directly. This approach ensures consistency and avoids conflicts with user-defined key mappings.

I hope this explanation makes things clear.

[gepbird]

@ahmtsen Yes, that make sense, thanks for the explanation.

In my config I was using handlers for a long time, then after a neovim nightly update I ran into this problem (neovim/neovim#30908) and that's why I switched to custom commands which fixed it. And custom commands are marked as optimal while handlers are marked as not suboptimal (https://github.com/Hoffs/omnisharp-extended-lsp.nvim#how-to-use) due to many similar issues.

Thanks for pointing out that in a distro you can't just change default keybindings as people may have remapped that, that's a very good point! However if this issue comes back, maybe some hacky solution has to be implemented, like overriding vim.lsp.buf.defition and others instead of changing keybindings, but this sounds like a bad idea.

If you've properly tested this change, no objections from me, feel free to merge, I see this fixes an issue :) But I don't see how does it fix the 'Cursor position outside of buffer' issue.

(Also sorry for the noise, I came from the neovim issue and I'm not an AstroNvim user)