-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
lsp: Gracefully shutdown language servers, fix unwraps in transport #287
Conversation
The shutdown procedure I used in the neovim LSP was decent w.r.t. timings and escalating signals. If you want a reference: function client.stop(force)
local handle = rpc.handle
if handle:is_closing() then
return
end
if force or (not client.initialized) or tried_graceful_shutdown then
handle:kill(15)
return
end
tried_graceful_shutdown = true
-- Sending a signal after a process has exited is acceptable.
rpc.request('shutdown', nil, function(err, _)
if err == nil then
rpc.notify('exit')
else
-- If there was an error in the shutdown request, then term to be safe.
handle:kill(15)
end
end)
end
function lsp._vim_exit_handler()
log.info("exit_handler", active_clients)
for _, client in pairs(uninitialized_clients) do
client.stop(true)
end
-- TODO handle v:dying differently?
if tbl_isempty(active_clients) then
return
end
for _, client in pairs(active_clients) do
client.stop()
end
// Check if everything closed after 500ms in increments of 50ms, or else force close them.
if not vim.wait(500, function() return tbl_isempty(active_clients) end, 50) then
for _, client in pairs(active_clients) do
client.stop(true)
end
end
end |
4d2339b
to
cbce953
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine but there's some conflicts with master
Fixed the conflicts, should be ready to merge |
helix-term/src/application.rs
Outdated
future::join_all( | ||
self.editor | ||
.language_servers | ||
.iter_clients() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than exposing iter_clients
, could all of this including join_all
just be a function on the editor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems like a good idea, I'd have to add a futures_util
dependency to helix-view
though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need the iter_clients
because inner
field of Registry
is private so there is no other way I think to access the clients from Editor
Closes: #266
Closes: #33
Maybe closes?: #278
Tested with clangd and it correctly removes the
preamble...
file from/tmp
on close.