GNU bug report logs -
#58839
29.0.50; project-kill-buffer fails when Eglot is running
Previous Next
Full log
View this message in rfc822 format
João Távora <joaotavora <at> gmail.com> writes:
> Philip Kaludercic <philipk <at> 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.
What would be a co-owner?
My view is that project.el is a manager of buffers, but that isn't
relevant anymore.
>>> 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.
I really just meant "break" as in works until Eglot is enabled, and
nothing more than that.
>>> 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.
Project.el uses the same condition format like `buffer-match-p', which
is quite flexible. Maybe all earmuffed buffers should be spared
("\\`\\*.+\\*\\'"), but in my experience that can be too lenient.
>>> 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.
Ok, great.
>> 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"
I have to still admit that I am uncertain if the general ignoring of all
hidden buffers is justified. I have certainly used hidden buffers
frequently enough but never assumed that they were outside of anyone's
control. They are just regular buffers with a special kind of name
after all.
We ought to be able to define a cleaner way of clarifying what buffers
can and cannot belong to projects. Personally I think a buffer-local
variable/predicate would be a better approach.
> 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.
This bug report was last modified 2 years and 280 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.