From: Dmitry Gutov <dmitry@gutov.dev>
To: "Eli Zaretskii" <eliz@gnu.org>,
sbaugh@janestreet.com, "João Távora" <joaotavora@gmail.com>
Cc: app-emacs-dev@janestreet.com, 70724@debbugs.gnu.org
Subject: bug#70724: 29.2.50; eglot-reconnect errors when the project is deleted
Date: Sun, 19 May 2024 18:08:15 +0300 [thread overview]
Message-ID: <faee5968-77e9-4e0c-a64b-74e2d3bf9f95@gutov.dev> (raw)
In-Reply-To: <8634qffrly.fsf@gnu.org>
Seems like we could use Eglot's maintainer's input.
On 18/05/2024 11:31, Eli Zaretskii wrote:
> Ping! Can we make some progress here?
>
>> Cc: app-emacs-dev@janestreet.com
>> Date: Sat, 4 May 2024 04:09:48 +0300
>> From: Dmitry Gutov <dmitry@gutov.dev>
>>
>> Hi Spencer,
>>
>> On 02/05/2024 22:37, Spencer Baugh wrote:
>> >
>> > In some project /home/foo/proj, with pretty much any LSP server:
>> >
>> > 1. In /home/foo/proj, M-x eglot, starting some LSP server
>> >
>> > 2. Delete the directory /home/foo/proj
>> >
>> > 3. The LSP server will crash/exit
>> >
>> > 4. The process sentinel for the server will run, running
>> > eglot--on-shutdown which by default will call eglot-reconnect
>> >
>> > 5. eglot-reconnect extracts the saved project instance out of the
>> > server, which has a root directory which no longer exists, and calls
>> > eglot--connect with it
>> >
>> > 6. eglot--connect calls project-name on a nonexistent project instance,
>> > which may fail with an error depending on the project implementation
>> > (I have a custom project implementation, but I think this can happen
>> > with project-vc too)
>> >
>> > 7. This causes the process sentinel to error.
>> >
>> > I think the right fix is probably for eglot--on-shutdown (or maybe
>> > eglot-reconnect) to call (project-current nil (project-root pr)) to find
>> > the new project instance. If that returns nil, the project has
>> > disappeared, and eglot should just not try to reconnect. This also
>> > would make eglot behave better if the project layout changes (e.g. if
>> > there are nested projects).
>>
>> I think I like this solution (as long as the nil value returned by
>> project-current on this step is appropriately handled).
>>
>> Something like:
>>
>> diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
>> index 6896baf30ce..7b2461c3ce6 100644
>> --- a/lisp/progmodes/eglot.el
>> +++ b/lisp/progmodes/eglot.el
>> @@ -1426,11 +1426,15 @@ eglot-reconnect
>> (interactive (list (eglot--current-server-or-lose) t))
>> (when (jsonrpc-running-p server)
>> (ignore-errors (eglot-shutdown server interactive nil
>> 'preserve-buffers)))
>> - (eglot--connect (eglot--major-modes server)
>> - (eglot--project server)
>> - (eieio-object-class-name server)
>> - (eglot--saved-initargs server)
>> - (eglot--language-ids server))
>> + (let* ((root (project-root (eglot--project server)))
>> + (project (project-current nil root)))
>> + (if (not project)
>> + (eglot--error "Project in `%s' is gone!" root)
>> + (eglot--connect (eglot--major-modes server)
>> + project
>> + (eieio-object-class-name server)
>> + (eglot--saved-initargs server)
>> + (eglot--language-ids server))))
>> (eglot--message "Reconnected!"))
>>
>> (defvar eglot--managed-mode) ; forward decl
>>
>>
>> Though it also raises a question about the caching strategy for VC-Aware
>> project backend. At the moment is associates a project with a directory
>> more or less indefinitely, and this is a case to watch out for.
>>
>> > Alternatively, maybe eglot--on-shutdown shouldn't automatically
>> > reconnect in the first place? Maybe reconnection should happen
>> > automatically only when some specific buffer tries to interact with the
>> > LSP - then it can run project-current in the context of that specific
>> > buffer, and see there's no project, and fail. Plus, if the user kills
>> > all the buffers in the project (possibly with project-kill-buffers)
>> > before deleting it, this approach would entirely avoid the unnecessary
>> > eglot reconnection attempt.
>>
>> This also sounds good, though it'd probably require more changes
>> overall. Additionally, perhaps I'd change the association from (server
>> -> project) to (server -> project-root), relying on the project
>> backends' internal caches to fetch the project value whenever it's
>> needed. That might be the most reliable approach. Perhaps the slowest in
>> theory, but hopefully not noticeably so.
next prev parent reply other threads:[~2024-05-19 15:08 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-02 19:37 bug#70724: 29.2.50; eglot-reconnect errors when the project is deleted Spencer Baugh
2024-05-04 1:09 ` Dmitry Gutov
2024-05-18 8:31 ` Eli Zaretskii
2024-05-19 15:08 ` Dmitry Gutov [this message]
2024-06-06 20:36 ` Dmitry Gutov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=faee5968-77e9-4e0c-a64b-74e2d3bf9f95@gutov.dev \
--to=dmitry@gutov.dev \
--cc=70724@debbugs.gnu.org \
--cc=app-emacs-dev@janestreet.com \
--cc=eliz@gnu.org \
--cc=joaotavora@gmail.com \
--cc=sbaugh@janestreet.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.