nvim-lua / kickstart.nvim

A launch point for your personal nvim configuration
MIT License
16.93k stars 17.06k forks source link

Kickstart calls `kickstart-lsp-highlight` augroup regardless of support #899

Closed cpiber closed 2 months ago

cpiber commented 2 months ago

Describe the bug

Kickstart registers a autocmd for LspDetach, which calls the augroup kickstart-lsp-highlight, but if the server does not support the capability documentHighlightProvider, this augroup does not exist. The following error is thrown:

Error detected while processing LspDetach Autocommands for "*":
Error executing lua callback: <cut>/.config/nvim/init.lua:618: Invalid 'group': 'kickstart-lsp-highlight'
stack traceback:
        [C]: in function 'nvim_clear_autocmds'
        <cut>/.config/nvim/init.lua:618: in function <<cut>/.config/nvim/init.lua:616>
        [C]: in function 'nvim_exec_autocmds'
        /usr/share/nvim/runtime/lua/vim/lsp.lua:388: in function </usr/share/nvim/runtime/lua/vim/lsp.lua:386>

To Reproduce

  1. Install a LSP server that does not support highlighting like grammarly (I believe)
  2. Open a file for that server, e.g. markdown

Desktop

Neovim Version

NVIM v0.10.0-dev
Build type: RelWithDebInfo
LuaJIT 2.1.0-beta3
feoh commented 2 months ago

Ooh. This is interesting. Thanks for the report!

Would you be willing to take a look around at other people's configurations to see how they might properly support this use case?

cpiber commented 2 months ago

I'm not familiar, and not very involved in the community, so that would be difficult, but I'll have a look

But it should easy to check if the group exists and just not run it then, right? I'll look into this a bit later and open a PR if that is fine

Or just move the group creation up? In case a plugin also relies on the existence of the group

dam9000 commented 2 months ago

This was introduced with: https://github.com/nvim-lua/kickstart.nvim/pull/864 by @sudo-tee Perhaps we can ask @sudo-tee to refine this further.

feoh commented 2 months ago

That would be amazing!

For an honest to god functionality breaking issue like this I and I'm sure others would be happy to help whip it into shape if the first go around isn't perfect.

Thanks so much for being willing to contribute, and thanks again for the report!

feoh commented 2 months ago

@dam9000 The more the merrier :)

cpiber commented 2 months ago

Actually just noticed that it's a bit worse than I had assumed, this also occurs if the LSP crashes, i.e. LspAttach is not executed, only LspDetach (not sure why that is even called...)

feoh commented 2 months ago

Let's give @sudo-tee a couple days to respond, and if it's radio silence we can revert that PR if necessary.

Obviously if anyone has any better ideas that would fix all the use cases I'm definitely up for that too :)

dam9000 commented 2 months ago

Well here is my attempt: https://github.com/nvim-lua/kickstart.nvim/pull/900 moving the code as suggested by @cpiber , seems to work based on my testing. @cpiber please test the PR. Also @sudo-tee please review.