bug#58839: [Patch] Re: bug#58839: 29.0.50; project-kill-buffer fails when Eglot is running

["João Távora" <joaotavora@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5366 bytes --]

On Fri, Oct 28, 2022 at 1:58 PM Philip Kaludercic <philipk@posteo.net>
wrote:

> When wanting to clean up behind a project I like to use C-x p k to get
> rid of everything I have opened related to it.  If I was using Eglot and
> there is still an active LSP server running in the background, killing
> the project fails with these messages:

Thanks Philip.  This was discussed at

https://github.com/joaotavora/eglot/discussions/822

Some more information is needed:

1. The error only happens when eglot-autoshutdown has been set to t by
   the user.

2. When it has not been set to t, then the behavior is still not
   correct, but the user may not notice it.

3. According to Manuel Uberti, the problem also happens with CIDER, a
   Clojure IDE for Emacs.  So it seems it is not exclusive to Eglot.

The problem happens because `project-kill-buffers` uses project.el's
sense of a project buffer, and then endeavours to kill all such buffers.

So far so good, but the determination of project buffers according
to `project-buffers` considers all buffers whose buffer-local default
directory starts with a given root of some project.

This is subtly wrong because it also considers buffers whose name starts
with space and without buffer-file-names, so-called "hidden buffers" which
are deemed "uninteresting" to the user (according to the Elisp manual).
They commonly function as implementation details of other packages, such
as Eglot (and possibly CIDER).  These buffers are not normally visible
to the user in M-x ibuffer, switch-to-buffer, etc.

In Eglot's case, the buffer whose name is " EGLOT process..." is
created by eglot.el and then handed over to jsonrpc.el, which becomes
responsible for it.

Killing this buffer from Lisp using `kill-buffer` is incorrect because
it contradicts Eglot's user preferences eglot-autoreconnect and
eglot-autoshutdown:

1. If eglot-autoshutdown is t, killing the buffer from Lisp kills the
   process and confuses the LSP shutdown logic, which is a polite
   "teardown" conversation with the LSP server.  This is Philip's error.

2. If eglot-autoshutdown is nil but eglot-autoreconnect is non-nil (in
   fact, these are the defaults), killing the buffer has the effect of
   immediately restarting the connection, and thus re-creating the
   buffer.  The best that can happen is that nothing was achieved
   and only time was wasted.

The fact is that the buffer in question is an internal Eglot implementation
detail that other packages should stay clear of.

In fact, I think that all hidden buffers can be considered thusly.
They're just like `--` symbols in obarray or in other symbol's plists:
they're visible to all Lisp packages but they are implementation details
that shouldn't be messed with except by the owner of such details.

Dmitry tells me that there was some discussion where it was determined
that it's somehow useful in project-kill-buffers to also target buffers
that the
user isn't aware of.

But I've not seen evidence of this usefulness.  If there is indeed some,
I propose we come up with some convention so that it is possible for
packages to create buffers which are "definitely hidden and private and
not to me tinkered with".  Such a convention could be starting the
buffer name with two spaces.

Whatever the convention, currently I think that the patch after my
signature is the correct approach to fix this bug.

Thanks,
João

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index ac278edd40..4f542137a8 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -352,14 +352,18 @@ project--remote-file-names
                 (concat remote-id file))
               local-files))))

+(defun project--buffer-uninteresting-p (buf)
+  (and (string-prefix-p " " (buffer-name buf)) (null (buffer-file-name
buf))))
+
 (cl-defgeneric project-buffers (project)
   "Return the list of all live buffers that belong to PROJECT."
   (let ((root (expand-file-name (file-name-as-directory (project-root
project))))
         bufs)
     (dolist (buf (buffer-list))
-      (when (string-prefix-p root (expand-file-name
-                                   (buffer-local-value 'default-directory
buf)))
-        (push buf bufs)))
+      (unless (project--buffer-uninteresting-p buf)
+        (when (string-prefix-p root (expand-file-name
+                                     (buffer-local-value
'default-directory buf)))
+          (push buf bufs))))
     (nreverse bufs)))

 (defgroup project-vc nil
@@ -680,11 +684,12 @@ project-buffers
          dd
          bufs)
     (dolist (buf (buffer-list))
-      (setq dd (expand-file-name (buffer-local-value 'default-directory
buf)))
-      (when (and (string-prefix-p root dd)
-                 (not (cl-find-if (lambda (module) (string-prefix-p module
dd))
-                                  modules)))
-        (push buf bufs)))
+      (unless (project--buffer-uninteresting-p buf)
+        (setq dd (expand-file-name (buffer-local-value 'default-directory
buf)))
+        (when (and (string-prefix-p root dd)
+                   (not (cl-find-if (lambda (module) (string-prefix-p
module dd))
+                                    modules)))
+          (push buf bufs))))
     (nreverse bufs)))

[-- Attachment #2: Type: text/html, Size: 6245 bytes --]