feat: automatically attach

[seblj]

Problem

It feels unecessary to manually setup nvim-navic even when calling the setup function of nvim-navic.

Solution

Make it automatically attach to a language server if it supports documentSymbolProvider by using the LspAttach autocmd. A user could opt-in to this by using auto_attach = true in the setup table.

Context

It looks like others want it too judging from these issues:

https://github.com/SmiteshP/nvim-navic/issues/11 https://github.com/folke/lazy.nvim/issues/704

[seblj]

If you don't like it attaching by default when calling setup, I can also change it to needing to pass attach = true

I do however feel that one would want it to attach when calling setup. And if a user don't want it to attch, the user should instead just not call setup at all.

This means that I actually don't see the need for the attach option that I added, but I added it for now in case that's something you would like to have.

Let me know what you think!

[SmiteshP]

Hi @seblj ! Yep I am definitely planning on adding this feature. I would implement it similar to how I did it in navbuddy. I would keep it opt in feature, since I still believe manual attachment is the way to go for better control. Would also like to add the prefence list for lsps incase there are more than one lsp to choose from. Only case I am worried about is that when there is multiple lsps attached to buffer and there is no preference list provided by the user, what should nvim-navic do in that scenario? 🤔 Sliently attach to random lsp? or throw error? Since, unlike nvim-navbuddy, nvim-navic is a plugin that works "in the background", throwing error or opening dialog box asking for user input would be too intrusive I guess.

[seblj]

I would keep it opt in feature, since I still believe manual attachment is the way to go for better control.

Totally understand! I'll change this right away

Would also like to add the prefence list for lsps incase there are more than one lsp to choose from

This also makes sense! Do you have any idea how this would work? What about having a table the user can pass, where the key is the name of the server, and value is priority? Then all of them can have a default value of 100 for example, and the user might either increase or lower the priority of certian language servers

Only case I am worried about is that when there is multiple lsps attached to buffer and there is no preference list provided by the user, what should nvim-navic do in that scenario? 🤔

This is also a valid case to think about! Do you have any idea how usual there is to attach multiple language servers to a buffer where both of them supports documentSymbolProvider? In the case of a warning, it would be quite rare to get it since I would suppose it doesn't happen that often, and especially if the user then lowers/increases the priority of one of them, then the warning would be gone. What do you think?

Thank you for taking the time to review!

[seblj]

I actually found a bug in the implementation with regards to multiple servers where the attach didn't work like expected. It sends a request with vim.lsp.buf.request_all which will send a request to all servers attached to the buffer regardless of them calling the attach-function from nvim-navic or not.

I'll go ahead and fix that bug while I am at it with calling the request-method on the client instead!

[seblj]

I implemented automatically attach with a user needing to opt-in to it. I also added a priority rank where a user could pass a priority to make it attach over another server.

I also added a detach function that could be called by the user, but is meant to be called before attaching a new server if it has higher priority than the one already attached.

Lastly I fixed the bug with it not working properly with two servers attached to the buffer by calling the request method on the specific client.

Let me know what you think about the changes! @SmiteshP

TODO: I need to add docs, but I'll wait with that until the code looks good and we have decided on if this is how we want to handle automatic attaching😄

[SmiteshP]

This also makes sense! Do you have any idea how this would work? What about having a table the user can pass, where the key is the name of the server, and value is priority? Then all of them can have a default value of 100 for example, and the user might either increase or lower the priority of certian language servers

Lets just have simple list where users can write all the lsp names in order, earlier the name, higher the priority. Explained this more in review comments.

This is also a valid case to think about! Do you have any idea how usual there is to attach multiple language servers to a buffer where both of them supports documentSymbolProvider? In the case of a warning, it would be quite rare to get it since I would suppose it doesn't happen that often, and especially if the user then lowers/increases the priority of one of them, then the warning would be gone. What do you think?

Not sure how common this would be, I have had some issues open though were there were requests to handle multiple LSP at same time. So I guess atleast some people will have multiple LSP set. But yeah, I think throwing an error is the best way to handle it. Because the users are not manually attaching the LSP, they should atleast file the preference option.

TODO: I need to add docs, but I'll wait with that until the code looks good and we have decided on if this is how we want to handle automatic attachingsmile

Yep, docs can be done at the end :)

[seblj]

I think I resolved all the comments now! Let me know if there is anything else that needs to be changed 😄

[SmiteshP]

Pushed a minor change, rest looks good to me 👍🏽

[seblj]

Awesome! I'll go ahead and update the docs then in a few minutes

[seblj]

Awesome! I'll go ahead and update the docs then in a few minutes

Done

[SmiteshP]

Merged! Thanks @seblj :)