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

[Philip Kaludercic]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 01.11.2022 20:44, Philip Kaludercic wrote:
>
>>> Sure, but only after we're ready to drop project.el support for Emacs
>>> older than 29.
>> The functionality can be provided using Compat[0], as is already
>> done for a
>> few core package that are distributed on GNU ELPA (ERC, python-mode).
>> [0] https://elpa.gnu.org/packages/compat.html
>
> I suppose if the performance improvement is shown to be significant,
> we could. I'm a little hesitant to add a new dependency: I haven't
> been following this package, not sure how stable it is.

I am the maintainer, so I am biased, but stability is a high priority,
which is why the library is extensively tested, where-ever possible:

https://git.sr.ht/~pkal/compat/tree/master/item/compat-tests.el

Fun fact, I came up with the idea for this library when working on
project-kill-buffer over two years ago, as a means of extracting the
language out of project.el, as has been done with buffer-match-p.

>>>> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
>>>> index ac278edd40..b55c245368 100644
>>>> --- a/lisp/progmodes/project.el
>>>> +++ b/lisp/progmodes/project.el
>>>> @@ -352,15 +352,28 @@ project--remote-file-names
>>>>                    (concat remote-id file))
>>>>                  local-files))))
>>>>    +(defun project-includes-buffer-p (buffer dir)
>>>> +  "Return non-nil if the `default-directory' of BUFFER is below DIR."
>>>> +  (file-in-directory-p
>>>> +   (buffer-local-value 'default-directory buffer)
>>>> +   dir))
>>>
>>> Not an optimal name, given that we have made project-buffers a generic
>>> function, so that a custom project backend can define their own
>>> buffer-listing strategy. And this one implies that matching by
>>> default-directory is universal.
>> Right, as I said this is just a sketch.
>> But as the diff showed, I think any more specific implementation
>> ought
>> to extend the generic implementation, by using `cl-call-next-method'
>> instead of `buffer-list'.
>
> Or apply the same filters some other way, I guess. But yes, the
> cl-call-next-method call makes sense in your patch.
>
>>>> +(defcustom project-buffer-conditions
>>>
>>> Why not keep considering unknown buffers as part of project by default?
>> What are "unknown buffers"?
>
> Take whatever special buffer belonging to jsonrpc that was the cause
> of this bug report. It can still be useful to be able switch to it
> using project-switch-to-buffer (if, say, one was looking for the
> specific buffer to try to debug a problem with some Eglot feature in
> their project). We don't want to kill it with the rest of the buffers,
> however. Apparently.

Ah, right.  I have to admit that I rarely use project-switch-to-buffer,
so I forgot about that.  In that case, project-kill-buffer-conditions
cannot be deprecated, as these are just a subset of the project buffers.

>>> We'll just stop killing them on cleanup.
>>>
>>> Otherwise, we'll really need an extensible mechanism for major modes
>>> all around the ecosystem to tag themselves as project-visible.
>> Wouldn't a simple buffer local variable suffice?
>
> I guess it will. Only with a more meaningful name than 'project-owned'
> and some proper documentation.

Right.  Does it have to have a "project-" prefix, or would
"belongs-to-project" (hypothetically) be fine too?

>>>> +  '(and (or buffer-file-name
>>>> +            (derived-mode . compilation-mode)
>>>> +            (derived-mode . dired-mode)
>>>> +            (derived-mode . diff-mode)
>>>> +            (derived-mode . comint-mode)
>>>> +            (derived-mode . eshell-mode)
>>>> +            (derived-mode . change-log-mode))
>>>> +        project-includes-buffer-p)
>>>> +  "A buffer predicate for matching what buffers belong to a project."
>>>> +  :type 'buffer-predicate)
>>>
>>> Let's not forget Xref, Occur, VC-Dir, Log-View. Maybe some others.
>> This is my point, I think João is right that this ought to be an
>> enumeration of major modes that are related to projects.  As this is a
>> user option, users can add or remove whatever modes they disagree on and
>> that behaviour would ideally be propagated to all projects.
>
> Being to customize it is a good thing.
>
> But either we provide a reasonably complete list which we regularly
> update, or we say its completeness is up to the user.

I would want to argue that the complete list is preferable.

> And in the latter case, as soon as the user customizes the var, they
> stop getting any updates we might make to it later (to the default
> value).
>
> And if we take the strict "whitelist" approach, I'm pretty sure the
> list will require updating in the future, it will never be complete.

Which is fair, but which can also be combined with