Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
cpiber opened this issue May 2, 2024 · 8 comments · Fixed by #900
Closed

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

cpiber opened this issue May 2, 2024 · 8 comments · Fixed by #900

Comments

@cpiber
Copy link

cpiber commented May 2, 2024

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

  • OS: Ubuntu 23.10 Linux <cut> 6.5.0-28-generic #29-Ubuntu SMP PREEMPT_DYNAMIC Thu Mar 28 23:46:48 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
  • Terminal: kitty

Neovim Version

NVIM v0.10.0-dev
Build type: RelWithDebInfo
LuaJIT 2.1.0-beta3
@feoh
Copy link
Collaborator

feoh commented May 2, 2024

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
Copy link
Author

cpiber commented May 2, 2024

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
Copy link
Contributor

dam9000 commented May 2, 2024

This was introduced with: #864 by @sudo-tee
Perhaps we can ask @sudo-tee to refine this further.

@feoh
Copy link
Collaborator

feoh commented May 2, 2024

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
Copy link
Collaborator

feoh commented May 2, 2024

@dam9000 The more the merrier :)

@cpiber
Copy link
Author

cpiber commented May 2, 2024

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
Copy link
Collaborator

feoh commented May 2, 2024

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
Copy link
Contributor

dam9000 commented May 2, 2024

Well here is my attempt: #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.

@feoh feoh closed this as completed in #900 May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants