From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Dmitry Gutov Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Speed up project-kill-buffers Date: Tue, 25 May 2021 04:24:57 +0300 Message-ID: <44b0b57f-27fd-d0b6-f350-5745375ff2a4@yandex.ru> References: <87mttcgnke.fsf@posteo.net> <871raoezl0.fsf@posteo.net> <86r1ihig9p.fsf@stephe-leake.org> <25bc7f1c-92e5-6b13-a6a4-d48b64b32ad3@yandex.ru> <86im3i76xa.fsf@stephe-leake.org> <86pmxmz45m.fsf@stephe-leake.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="33599"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 Cc: Philip Kaludercic , Stefan Monnier , emacs-devel@gnu.org To: Stephen Leake Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Tue May 25 04:11:48 2021 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1llMXr-0008cS-SO for ged-emacs-devel@m.gmane-mx.org; Tue, 25 May 2021 04:11:47 +0200 Original-Received: from localhost ([::1]:45690 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1llMXq-0006Kg-ED for ged-emacs-devel@m.gmane-mx.org; Mon, 24 May 2021 22:11:46 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:58834) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1llLoe-0006Mw-56 for emacs-devel@gnu.org; Mon, 24 May 2021 21:25:04 -0400 Original-Received: from mail-wm1-x332.google.com ([2a00:1450:4864:20::332]:56092) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1llLob-000571-Mh for emacs-devel@gnu.org; Mon, 24 May 2021 21:25:03 -0400 Original-Received: by mail-wm1-x332.google.com with SMTP id b7so15184820wmh.5 for ; Mon, 24 May 2021 18:25:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=HeMvRaibGTUOO0q/KYOVgSlZOEJwcC057v6NvFcAGfk=; b=J/tQEFQH6qv3lILGawdFBSdwd9C5Kz9xPkmvMgpr3LjiKMTMQGzXSNFH9Hl1KMuMUu bJfF7/a2UGqGIXBXRUEmu4/+ebYw6Eqx+SiwYyCoa1X7tzH4EWMF4yQUDxkoaeCUF8z3 4zKnQW9vLPgqKvREWXzuMGJTC2Qkn8da6VU0zdKLClSxO3SkJCVTTPKm21sbAu1YQPBk SxyBLFg9srozgCPv6XNT4H1c0xodDUXxiAatdxyvElDBLZJjYBsGq1gZUzBcz+ewPXo1 RFwuq5uteLBTwXVmQ3O9mYCcP4z9QEmq9c9TOKYE8hDc7HxQx6hScsbYJ3FjcOzfLiNW +vKQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:subject:to:cc:references:from:message-id :date:user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=HeMvRaibGTUOO0q/KYOVgSlZOEJwcC057v6NvFcAGfk=; b=K+ZJ7hurF+8fYwQ+oVgQ8bWR7Mvk5HI98+O4qJs+tAygQjSgVfzrgkd/+4DUvCnQmn jBRJWAJC2gWdMKDyumqZ4bts5grPN56atEgCVvBhPZTiJnZKUeKFfiDGHAlfzzJu3EQO 68D7kqPwOBtXzOoXgSgytDuMeBq+LA//jgqEkSBpN2ctQoNNB1MADcE0sDwzCHwdhMRJ MUjQIPwkdxP3kWs1aaXWuPo70igOAMgTXbl5MryX06A0akfZEYcJIrQlO7BDrR2DRyxI yMUDsUeKG+8FmdmvNsrw0zuGriFVHpVrLYJdWQjJY3GmT8AdxXVDq2Wu7xtrYC68pR4r 4C5w== X-Gm-Message-State: AOAM533p9uAJvygZveszE/zsuU01U4M+qLd7DsCMClLSYrrITpjocPhz sOaqqy/pGvltjNmD63wsL2ulWnaLdrc= X-Google-Smtp-Source: ABdhPJyMQEjfyp283OH2ytnrl37Gooy1S0M1l+nV+arD+IMrjkEPlkj3g+h0YTzVYvF8vtNX+x0MDw== X-Received: by 2002:a1c:dd08:: with SMTP id u8mr1389096wmg.62.1621905899939; Mon, 24 May 2021 18:24:59 -0700 (PDT) Original-Received: from [192.168.0.6] ([46.251.119.176]) by smtp.googlemail.com with ESMTPSA id i11sm14222546wrq.26.2021.05.24.18.24.58 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 24 May 2021 18:24:59 -0700 (PDT) In-Reply-To: <86pmxmz45m.fsf@stephe-leake.org> Content-Language: en-US Received-SPF: pass client-ip=2a00:1450:4864:20::332; envelope-from=raaahh@gmail.com; helo=mail-wm1-x332.google.com X-Spam_score_int: -14 X-Spam_score: -1.5 X-Spam_bar: - X-Spam_report: (-1.5 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FORGED_FROMDOMAIN=0.249, FREEMAIL_FROM=0.001, HEADER_FROM_DIFFERENT_DOMAINS=0.249, NICE_REPLY_A=-0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:269824 Archived-At: 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.