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

[Philip Kaludercic <philipk@posteo.net>]

João Távora <joaotavora@gmail.com> writes:

> Dmitry Gutov <dgutov@yandex.ru> writes:
>
>> This whole discussion is about different shades of OCD. One party
>> wants to clean up as much as possible, another says don't touch my
>> things.
>
> I think this discussion is about mistaking resource access for resource
> ownership.  Just because project.el or any Lisp package can access the
> full list of buffers, doesn't mean it can do whatever it wants with
> them.  Just like a routine in a C program can see the full memory
> address space of the process, and possibly even form pointers to these
> 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.

>> I don't think there is an objective "right" way to do things, only
>> something we're able to agree on in the end. I don't really use this
>> feature much myself: if you're able to come to an agreement with
>> Philip (who took the initiative on adding that command), I'll be
>> happy.
>
> 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.

> So project-k-buffers is useful but it doesn't have that exclusive. If it
> did (but I don't think it will ever have) then Eglot could indeed attach
> connection management to it.
>
>> Most object types are garbage-collected when no live reference to them
>> remains. That's not the case for buffers.
>
> Because there is always at least one live reference to them, obviously.
> But why does this matter?  In this buffer's case there are probably even
> more.  One of these references is the one that Eglot and Jsonrpc control
> the program or network process.  This is held in global variable.  There
> are no resource leaks, as far as I know.
>
>> Is the buffer in question killed when the user calls 'M-x
>> eglot-shutdown'? If so, consider that, after the user calls
>> project-kill-buffers, there won't be any buffers remaining that belong
>> to that project, that the user would be able to call 'M-x
>> eglot-shutdown' from.
>
> Are you sure?  Well you should actually try M-x eglot-shutdown and see
> what happens then :-)
>
>>> I M-x cd in *scratch* all the time.  It's a global scratch pad,
>>> now accessible via scratch-buffer everywhere.
>> And I don't have any projects that "~" belongs to.
>
> 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.

>> Same place you changed the major mode in the last patch:
>> jsonrpc.el. If jsonrpc.el doesn't want its buffers to be killed, it
>> could set that up as described above, through
>> kill-buffer-query-functions.
>
> Why should resource owners go to the hassle of whitelisting themselves
> from other packages' newfound disregard for private property?  Not to
> mention jsonrpc.el wants its buffers to be killed in controlled
> circunstances.  So now it would have to "unprotect itself" in those
> places.  I can't even think of adjectifying this design, let alone deal
> with the corner cases.
>
>>>>> 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.
>>>> I'm unable to find that message.
>>> In the original conversation:
>>> https://github.com/joaotavora/eglot/discussions/822#discussioncomment-2053395
>>
>> It's a reasonable approach too. Just not the one we took here. It
>> would make sense to try to make it work first.
>
> Ibuffer understands ownership, so it comes with bug-free and
> hassle-free.  Sounds more than "reasonable" to me.
>
> 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?

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.

> João