Re: [PATCH] Speed up project-kill-buffers

[Dmitry Gutov]

On 20.05.2021 02:37, Stephen Leake wrote:

> > At the moment, we call `project-current` in each buffer and compare
> > the returned value to the "current" project instance. That wouldn't
> > work for external roots no matter what option we add because a given
> > file inside "external root" can belong (in an "extended" sense) to
> > several projects at once, so there's no logical way to obtain the
> > project instance we're looking for based on such external file. Right?
>
> There is in my projects; the currently active global project. Which
> means that any buffer will appear to "belong" to that project, unless
> project-find-functions is buffer-local, as it is in my elisp buffers.

Oh. Right. So simply comparing project-current won't work for "global" 
projects like in your setup.

> I view project-current as returning an object useful for project-related
> commands. So in a "notes" text file, where I keep track of things to do
> for a project, I want project-find-file to use that project (which is
> the current globally selected project). project-delete-buffers should
> delete that notes file. But in a higher level notes file, opened on
> Emacs start, that lists all the projects I'm currently working on,
> project-find-file should also use the current global project, but that
> buffer should not be deleted with the project.

Right.

> > A generic like project-contains?
>
> That's one option, with the current predicate in project--buffer-list as
> the default implementation; then I could override that to check
> project-files, or all roots, or something. As long as it passes C-u to
> the backend, my function could also use that to decide about dependent
> libraries.

I'm fairly sure you don't want to close the buffers visiting the 
dependencies. Or if we do, that would be a globally acting modifier, and 
that piece of logic which we would add to project-kill-buffers would 
just see whether the buffer's default-directory is inside any of the 
"external roots", as defined by the backend. Would that work for you?

> It might be better to make the predicate more specific;
> project-delete-this-buffer-p. That way eglot won't try to abuse
> project-contains to pick a server :). Or maybe my delete buffer case
> and eglot's "choose the right server" case really are the same?

I'd rather we try to be more strict with semantics, if possible, and 
'project-contains?' would only return t for buffers "inside" the 
project, not stuff that's outside but related to it (like external 
libraries, system includes, etc, which is what I'd like "external roots" 
to be targeted at).

> > I previously mentioned would have a problem that every project
> > backend's implementation would have to obey such an option, which
> > creates new possibility for bugs and diverging behaviors.
>
> Yes. Things should be as simple as possible, but no simpler.
>
> A default that is useful in many simple projects helps a lot.

It also helps when we can encourage projects not to forget to obey the 
option, some way or other.

> > If the current project-kill-buffers doesn't work for you (please
> > check; IIRC your backend's peculiar behavior might have an upside in
> > this case),
>
> First, M-x project-kill-buffers dies because my definition of
> project-root returns nil. I suppose you'd call that a bug in my project
> backend, so I'll pick some arbitrary directory (essentially (car
> compilation-search-path)) and call it the "primary root".
>
> Then the prompt is:
>
>      Kill 17 buffers in d:/apps/gnat-gpl_2020/lib/gnat? (yes or no)
>
> That directory is something you would call an external root, I guess, so
> you would argue I should pick a different one. But I have lived with
> project-root returning nil for several years now, so I don't see why I
> should be forced to put effort into choosing some "better" root, just so
> project-kill-buffers can detect a remote project. I'd be happy if that
> remote project predicate treated nil as "not remote".

Even if your project is "amorphous" in shape, I wouldn't necessarily 
want to call its directories "external roots".

For project-root, though, you might pick something like the directory 
that hosts the main configuration/build file.

That wouldn't help with the project-list-buffers-function approach, but 
I suppose that just wasn't a great idea.

> In any case, 17 buffers is far too many; there are only three buffers
> open related to the current project. (length (buffer-list)) is 24, so at
> least it's not deleting everything.
>
> Hmm. I just found project-kill-buffer-conditions; maybe I could
> customize that; it allows arbitrary predicate functions. It would be
> more efficient if the predicate function were also passed the project
> object, so it doesn't have to call project-current again (hmm - shades
> of project-delete-this-buffer-p :).

Yes, it's an interesting idea: if all buffers point to the current 
project, you could do post-filtering in this var.

> A better prompt would be the name of the project; the ELPA package wisi
> defines a name for each project, specified by the user as part of the
> project definition. In this case, it is "ada_mode_wisi_parse stephe_6",
> which is much more meaningfull to me, and more meaningfull than some
> "better" "primary root" directory. In fact, I have two different
> projects that share the directory of test files (one for compiling the
> parser, one for running the parser tests), so a directory name does not
> uniquely identify a project. Similarly, there are elisp and Ada files in
> the ada-mode source directory; the elisp files use the elisp project
> (named "elisp"), the Ada (and all other) files use the
> "ada_mode_wisi_parse stephe_6" project.
>
> I think another generic project-name, or possbly project-prompt, would
> be good here. The default could be project-root.

We can add that, but please file that as a separate bug report.

> There should also be a custom boolean to turn off the prompt; it's
> helpful the first few times, then it's just annoying. Currently C-u is
> 'no-confirm'; I hope that will change to 'no-dependencies'.

It *might* change to "include dependencies", since currently it's not 
supposed to include them.

If you want this change to happen, could you outline the cases when you 
would and would not use this capability? Personally, I'd probably never 
kill those buffers.

> Normal Emacs
> style uses a custom variable for suppressing prompts.

See below.

> Even better would to define the key ? at that prompt to put up a list of
> the buffer names; that way the user could learn to trust the command (I
> certainly don't trust it now :). I suspect that would be very hard to do
> (certainly yes-or-no-p can't do that now).

That would require some work (patches welcome?), but shouldn't be too 
difficult

> Instead, the custom variable
> that turns off the prompt could be tri-valued; nil (no prompt), t (short
> prompt), 'verbose (list all buffers, with a hint on how to change the
> prompt behavior). Then 'verbose should be the default. dired has similar
> behavior when deleting one vs several files.

I'd be happy to review a patch. If there are many such buffers, some 
smart formatting of the list would need to be used, though.

> > what we could more easily do is create an option like
> > 'project-list-buffers-function'. The default would delegate to
> > project-current.
>
> I don't see how that is better/easier than a cl-defgeneric; either way
> the project has to specify the correct function, possibly on a
> per-project basis. Either way you have to provide a useful default
> implementation. And they provide the same power to encounter/create
> bugs; they are both dispatching methods. I think it's better to stick to
> one method of dispatching. In my case, the wisi package will provide the
> function.

Consider also this: you suggested a generic like 
project-delete-this-buffer-p.

If in the end the set of buffers that project-switch-to-buffer uses, and 
the one that project-kill-buffer uses, are very different, perhaps there 
is not much benefit in having both of them do this through the project 
API. Because the point of doing that is to be able to reuse the same 
information in different contexts.

Like, if your backend would have to implement a method that's 5x as long 
as the current project-kill-buffers definition, you might as well 
provide an ada-project-kill-buffers command, right?

Just thinking aloud here.

> On the other hand, it appears we already have dispatching via
> project-kill-buffer-conditions; I'll see if I can make that work. I'm
> not sure I can reliably detect C-u from there for dependent libraries.

Let me take a look at your other email.