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

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:

> (defun eglot-before-kill-special ()
>   (eglot-shutdown)
>   t)
>
> ;; somewhere inside that special buffer's setup:
> (add-hook 'kill-buffer-query-functions #'eglot-before-kill-special nil t)
>
> Or use kill-buffer-hook, no need to watch the return value then.

Thanks, but this is simply wrong.  Besides missing a required argument,
it doesn't respect the preference of eglot-autoshutdown: I explained
that in the second message of this thread replying to Philip.

I also explained in that message that Eglot doesn't "own" the process
buffer, jsonrpc.el does.  Clients of jsonrpc.el don't even know there's
a process buffer, they only own a handle to a jsonrpc "connection",
which may or may not use buffers underneath.  So your "somewhere
inside..." is a big problem.

> In either case, it will also cover the scenario of the user killing
> the background buffer some other way.

The "background buffer" is hidden.  Users don't see it unless they go
considerably out of their way.  Even M-x project-switch-to-buffer
somehow doesn't list it.

Let me ask you this: can you conceive to that some buffers in Emacs's
buffer list simply don't belong to _any_ project?  If you agree that
there are such cases, then it should become clear that the buffer in
question must be at the top of that list.

There are more hints that the concept of "buffer belonging to a project"
was not fully thought through, even in cases unrelated to this bug
report.

* Take the *scratch* buffer.  It has a default-directory.  Does this
  also make *scratch* belong to a project?  It doesn't make any sense to
  me that it would.  Yet it is caught by project-buffers.

* project-buffers also catches the one-time *Completions* buffers, the
  kind produced by hitting TAB after C-x p b.  If you type C-x p b
  again, it quite comically offers the stale *Completions* buffer as a
  candidate to switch to.

But back to *scratch*.  Somehow *scratch* is not killed by M-x
project-kill-buffers.  I think it's because it doesn't have a
buffer-file-name.  But then neither does the Eglot/Jsonrpc's "background
buffers"!  It seems it is being targeted merely because it uses
fundamental-mode, a most reasonable mode to use for exchanging messages
via standard streams.

I guess this means that the hack below is enough to fix the issue, but
it is also decidedly silly.

So please consider fixing this in project.el.  As Manuel pointed out,
the venerable ibuffer.el's ibuffer-kill-filter-group also kills project
buffers and handles this whole thing very well.  We should just take a
hint from it.

João

diff --git a/lisp/jsonrpc.el b/lisp/jsonrpc.el
index 90833e1c1d..2914d380e9 100644
--- a/lisp/jsonrpc.el
+++ b/lisp/jsonrpc.el
@@ -365,6 +365,9 @@ jsonrpc-process-connection
 :ON-SHUTDOWN (optional), a function of one argument, the
 connection object, called when the process dies.")
 
+(define-derived-mode jsonrpc--fundamental-mode fundamental-mode ""
+  "Make project.el's stay out of our internal buffers.")
+
 (cl-defmethod initialize-instance ((conn jsonrpc-process-connection) slots)
   (cl-call-next-method)
   (cl-destructuring-bind (&key ((:process proc)) name &allow-other-keys) slots
@@ -377,6 +380,7 @@ initialize-instance
     ;; (but maybe not a slot).
     (let ((calling-buffer (current-buffer)))
       (with-current-buffer (get-buffer-create (format "*%s stderr*" name))
+        (jsonrpc--fundamental-mode)
         (let ((inhibit-read-only t)
               (hidden-name (concat " " (buffer-name))))
           (erase-buffer)
@@ -411,6 +415,7 @@ initialize-instance
     (set-process-filter proc #'jsonrpc--process-filter)
     (set-process-sentinel proc #'jsonrpc--process-sentinel)
     (with-current-buffer (process-buffer proc)
+      (jsonrpc--fundamental-mode)
       (buffer-disable-undo)
       (set-marker (process-mark proc) (point-min))
       (let ((inhibit-read-only t))