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

[João Távora]

Philip Kaludercic <philipk@posteo.net> writes:

>> bytes, but it shouldn't rely on their values and certainly can't free()
>> what it didn't malloc().
> But to extend this metaphor, if I kill a programm that allocated malloc,
> I would expect that memory to be cleaned up.

Yes, but to kill a process you have to own it.  project.el is not the
owner (not even a co-owner) of Eglot LSP internal buffers, and so it
can't kill them.

>> I think the command is pretty useful.  But perhaps, I'm just guessing
>> here, Philip is focusing too much it as if it were the only sanctioned
>> way for Emacs users to stop working on a given set of files in a
>> programming project and clean up.
> Of course it isn't, it is just my prefered way and Eglot breaks it.

I don't think we should play the who-broke-what game.  It doesn't help,
and if we decided to look up the dates of introduction of your
project-kill-buffers way and eglot's process management, we might reach
a different conclusion.

>> Neither do I.  But when I M-x cd to other places, project.el thinks that
>> scratch belongs to that project.  It doesn't, I keep stuff of various
>> projects in it.
>
> I agree, this is bad, but that can easily be solved by adding
> `lisp-interaction-mode' to the list of major modes that are not
> killed.

Also *ielm*, the global Elisp repl created by M-x ielm. has the same
problem.  And *Completions*.  I'm quite sure that *sly-scratch* in the
SLY CL IDE would also be targeted.  And the CIDER Clojure IDE, as
someone has already reported.  And probably many more.  This blanket
default-directory heuristic is practical in some common cases but flawed
in many others.

>> Just commit the original tested patch I gave you that exempts hidden
>> buffers without buffer-file-name from project-buffers.  Philip's command
>> will keep working perfectly and we can close this bug.
>
> So if I understand correctly, with `eglot-autoshutdown' enabled as soon
> as all the buffers have been killed, the server will also shut down,
> right?

Yes! This is exactly what the docstring says:

   eglot-autoshutdown is a variable defined in `eglot.el'.

   If non-nil, shut down server after killing last managed buffer.

> Regarding the patch itself, I think it would be better to use
> `project-kill-buffer-conditions', so that if a project.el backend
> defines a new implementation for `project-buffers', the issue doesn't
> pop up again.

Your concern is quite valid.  Fortunately, CLOS generic functions have
us covered.  This is even simpler than the first patch:

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index ac278edd40..f8190eb1fc 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -362,6 +362,13 @@ project-buffers
         (push buf bufs)))
     (nreverse bufs)))
 
+(cl-defmethod project-buffers :around (_project)
+  "Ensure hidden/private buffers do not belong to PROJECT."
+  (cl-remove-if-not (lambda (b)
+                      (and (string-prefix-p " " (buffer-name b))
+                           (not (buffer-file-name b))))
+                    (cl-call-next-method)))
+
 (defgroup project-vc nil
   "Project implementation based on the VC package."
   :version "25.1"


Note this still leaves the *scratch* and *ielm* and other things
uncovered.  It addresses this specific bug and most importantly doesn't
blow up in the users.