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

LspStopServer: Stop all and stop specific #1491

Merged

Conversation

ilya-bobyr
Copy link
Contributor

It is convenient to be able to stop all LSP servers, regardless of the currently active buffer.

Also, it seems confusing that when a server name is specified, it is only stopped if it is also one handling the current buffer type. I would imagine it is quite rare to have more than one server handing a specific buffer type. And if one explicitly specified a name, it seems reasonable to stop this particular server, regardless of the currently active buffer.

@prabirshrestha
Copy link
Owner

could you fix the lint issue?

@ilya-bobyr
Copy link
Contributor Author

ilya-bobyr commented Sep 8, 2023

could you fix the lint issue?

What is the issue?
The CI seems to show all checks as successful:

All checks have passed
18 successful checks


Update.
Ah, sorry, I can see it now - in the review view.

@ilya-bobyr
Copy link
Contributor Author

Vim documentation says that default values for arguments are fair game.
And I am actually using this code for a few weeks now. It works correctly.
:LspStopServer works both with and without an argument.

I can change lsp#ui#vim#stop_server(stop_all, name = '') to lsp#ui#vim#stop_server(stop_all, ...) and then use a:0, a:1 or a:000 to deal with the optional name.
But as an optional name is what I want and as Vim script supports it, this would be a workaround just to avoid the linter parser limitation.

Do you want to make this change?

@prabirshrestha
Copy link
Owner

We should make the change. Not sure if older versions of vim will have issues. Some devs use the one that ships with OS and they are not fan of updating to latest version. Specially for those using LTS versions that barely update vim.

@ilya-bobyr ilya-bobyr force-pushed the pr/LspStopServer-stop-all branch 4 times, most recently from fc05014 to 92ebd4a Compare September 8, 2023 21:41
It is convenient to be able to stop all LSP servers, regardless of the
currently active buffer.

Also, it seems confusing that when a server name is specified, it is
only stopped if it is also one handling the current buffer type.  I
would imagine it is quite rare to have more than one server handing a
specific buffer type.  And if one explicitly specified a name, it seems
reasonable to stop this particular server, regardless of the currently
active buffer.
@ilya-bobyr
Copy link
Contributor Author

Rebased, addressed two linter issues, and added quotes around server names.
Here is a complete diff: https://github.com/prabirshrestha/vim-lsp/compare/0b0db777524f127f965768286623407f7ada5992..6e35c234a44e4d32ea578adb1fdcf3f05b784c5c

@prabirshrestha Please, take a look.

@prabirshrestha prabirshrestha merged commit aa93b2a into prabirshrestha:master Sep 9, 2023
18 checks passed
@prabirshrestha
Copy link
Owner

prabirshrestha commented Sep 9, 2023

Merged. Thanks!

@ilya-bobyr ilya-bobyr deleted the pr/LspStopServer-stop-all branch September 10, 2023 08:11
ompugao pushed a commit to ompugao/vim-lsp that referenced this pull request Dec 9, 2023
It is convenient to be able to stop all LSP servers, regardless of the
currently active buffer.

Also, it seems confusing that when a server name is specified, it is
only stopped if it is also one handling the current buffer type.  I
would imagine it is quite rare to have more than one server handing a
specific buffer type.  And if one explicitly specified a name, it seems
reasonable to stop this particular server, regardless of the currently
active buffer.
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 this pull request may close these issues.

2 participants