From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Newsgroups: gmane.emacs.bugs Subject: bug#58839: [Patch] Re: bug#58839: 29.0.50; project-kill-buffer fails when Eglot is running Date: Fri, 28 Oct 2022 18:17:18 +0100 Message-ID: References: <87sfj8umwb.fsf@posteo.net> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="000000000000870d2205ec1b6a2e" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="12371"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 58839@debbugs.gnu.org To: Philip Kaludercic , Manuel Uberti , Dmitry Gutov Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Oct 28 19:17:25 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1ooSyy-000304-Dz for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 28 Oct 2022 19:17:24 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ooSyi-0004BV-Jc; Fri, 28 Oct 2022 13:17:08 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ooSyd-0004AQ-Bs for bug-gnu-emacs@gnu.org; Fri, 28 Oct 2022 13:17:04 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1ooSyc-0001MV-Uk for bug-gnu-emacs@gnu.org; Fri, 28 Oct 2022 13:17:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1ooSyc-0007Ha-FF for bug-gnu-emacs@gnu.org; Fri, 28 Oct 2022 13:17:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 28 Oct 2022 17:17:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 58839 X-GNU-PR-Package: emacs Original-Received: via spool by 58839-submit@debbugs.gnu.org id=B58839.166697738827938 (code B ref 58839); Fri, 28 Oct 2022 17:17:02 +0000 Original-Received: (at 58839) by debbugs.gnu.org; 28 Oct 2022 17:16:28 +0000 Original-Received: from localhost ([127.0.0.1]:34194 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ooSy4-0007GX-14 for submit@debbugs.gnu.org; Fri, 28 Oct 2022 13:16:28 -0400 Original-Received: from mail-oi1-f180.google.com ([209.85.167.180]:34436) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ooSy0-0007GF-No for 58839@debbugs.gnu.org; Fri, 28 Oct 2022 13:16:25 -0400 Original-Received: by mail-oi1-f180.google.com with SMTP id y67so6817395oiy.1 for <58839@debbugs.gnu.org>; Fri, 28 Oct 2022 10:16:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=ycMia3wljRFmv1pqtmXYSvdA5HAZB/pKO4DIBamj1C4=; b=CNpoGmX+RFd9K8JthTqdNkXT0/uoa2eVAhG5Z+VfR62fCigA9H3YVeiLwZibO71PIO tZiZRRKiIzRYNwmGw7ZPWHIWdYqxtyiU/ze6FUed1DmOWsowWcU/rwrNlZXkDKz7KM8v aFqNin1qZbgbNyaG73EUS4kH6q6pYJ4sR0BNbnNcv9WH5TfHE/Ew6hRoaFFCPVKyHSpi w4ghq6yS3ItEKJpjBz0YUpH6Ss9bFabprUATi1VarTz1ID/vS5p+SM6NcgE02YpLrCTW EKqbboCD+hk/NzjGKf+1f/c8WDQs8du/De3J4C+q2Qj65QKhmZCkVeSpCjuVsu4GfgsL YVUw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=ycMia3wljRFmv1pqtmXYSvdA5HAZB/pKO4DIBamj1C4=; b=mno5Q5ZQ58+BldCv+fxy5McznwEWNlWaCr45ByejnGIaYU7rcjTYiZnAceYvyExKrY bVmipYl+BK5cOFStNZN3Yf4WWWDYQRP1e/xWnUfTYG/Yr7/VA0L2JBr0Ogvi5WMafkNY OZXkdvRPflu37lrwuzwvNEAOMHdwU7JRp/NvTZC9AXRAOdVr8DU8nw0NHig3GGVsgaUy nqhkzT+IlKV2E25UdXK1yHUQEXWzWKMw2sptMnE9dINskYFpyHYkL3g3bl908+GUROjj B1hPgh9ZxoLAuYpf1LOLcC/rsx8TCZAO36+V4lBnPTrg7cpLNgcDBmCL0t3NNtsDrjBk E4aQ== X-Gm-Message-State: ACrzQf3Q8XXTMQFGnjFW14xMB/21ImA6xOBdYmzQxXIv4AvC+xJa/Rnb UgVJnNqU18RQcahm0kIUXqXhxm+nSKwXdnhuAus= X-Google-Smtp-Source: AMsMyM66powm+w5OoOWbZstjTDqMxtTKAf7jd6zsSzofc53RqvBNXHndd9mOw/xm97vldCxclkY0kUK0uKa9eOpTfn0= X-Received: by 2002:a05:6808:1a1f:b0:354:b33b:8b0d with SMTP id bk31-20020a0568081a1f00b00354b33b8b0dmr8132203oib.171.1666977379059; Fri, 28 Oct 2022 10:16:19 -0700 (PDT) In-Reply-To: <87sfj8umwb.fsf@posteo.net> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: "bug-gnu-emacs" Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:246438 Archived-At: --000000000000870d2205ec1b6a2e Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, Oct 28, 2022 at 1:58 PM Philip Kaludercic wrote: > When wanting to clean up behind a project I like to use C-x p k to get > rid of everything I have opened related to it. If I was using Eglot and > there is still an active LSP server running in the background, killing > the project fails with these messages: Thanks Philip. This was discussed at https://github.com/joaotavora/eglot/discussions/822 Some more information is needed: 1. The error only happens when eglot-autoshutdown has been set to t by the user. 2. When it has not been set to t, then the behavior is still not correct, but the user may not notice it. 3. According to Manuel Uberti, the problem also happens with CIDER, a Clojure IDE for Emacs. So it seems it is not exclusive to Eglot. The problem happens because `project-kill-buffers` uses project.el's sense of a project buffer, and then endeavours to kill all such buffers. So far so good, but the determination of project buffers according to `project-buffers` considers all buffers whose buffer-local default directory starts with a given root of some project. This is subtly wrong because it also considers buffers whose name starts with space and without buffer-file-names, so-called "hidden buffers" which are deemed "uninteresting" to the user (according to the Elisp manual). They commonly function as implementation details of other packages, such as Eglot (and possibly CIDER). These buffers are not normally visible to the user in M-x ibuffer, switch-to-buffer, etc. In Eglot's case, the buffer whose name is " EGLOT process..." is created by eglot.el and then handed over to jsonrpc.el, which becomes responsible for it. Killing this buffer from Lisp using `kill-buffer` is incorrect because it contradicts Eglot's user preferences eglot-autoreconnect and eglot-autoshutdown: 1. If eglot-autoshutdown is t, killing the buffer from Lisp kills the process and confuses the LSP shutdown logic, which is a polite "teardown" conversation with the LSP server. This is Philip's error. 2. If eglot-autoshutdown is nil but eglot-autoreconnect is non-nil (in fact, these are the defaults), killing the buffer has the effect of immediately restarting the connection, and thus re-creating the buffer. The best that can happen is that nothing was achieved and only time was wasted. The fact is that the buffer in question is an internal Eglot implementation detail that other packages should stay clear of. In fact, I think that all hidden buffers can be considered thusly. They're just like `--` symbols in obarray or in other symbol's plists: they're visible to all Lisp packages but they are implementation details that shouldn't be messed with except by the owner of such details. Dmitry tells me that there was some discussion where it was determined that it's somehow useful in project-kill-buffers to also target buffers that the user isn't aware of. But I've not seen evidence of this usefulness. If there is indeed some, I propose we come up with some convention so that it is possible for packages to create buffers which are "definitely hidden and private and not to me tinkered with". Such a convention could be starting the buffer name with two spaces. Whatever the convention, currently I think that the patch after my signature is the correct approach to fix this bug. Thanks, Jo=C3=A3o diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el index ac278edd40..4f542137a8 100644 --- a/lisp/progmodes/project.el +++ b/lisp/progmodes/project.el @@ -352,14 +352,18 @@ project--remote-file-names (concat remote-id file)) local-files)))) +(defun project--buffer-uninteresting-p (buf) + (and (string-prefix-p " " (buffer-name buf)) (null (buffer-file-name buf)))) + (cl-defgeneric project-buffers (project) "Return the list of all live buffers that belong to PROJECT." (let ((root (expand-file-name (file-name-as-directory (project-root project)))) bufs) (dolist (buf (buffer-list)) - (when (string-prefix-p root (expand-file-name - (buffer-local-value 'default-directory buf))) - (push buf bufs))) + (unless (project--buffer-uninteresting-p buf) + (when (string-prefix-p root (expand-file-name + (buffer-local-value 'default-directory buf))) + (push buf bufs)))) (nreverse bufs))) (defgroup project-vc nil @@ -680,11 +684,12 @@ project-buffers dd bufs) (dolist (buf (buffer-list)) - (setq dd (expand-file-name (buffer-local-value 'default-directory buf))) - (when (and (string-prefix-p root dd) - (not (cl-find-if (lambda (module) (string-prefix-p module dd)) - modules))) - (push buf bufs))) + (unless (project--buffer-uninteresting-p buf) + (setq dd (expand-file-name (buffer-local-value 'default-directory buf))) + (when (and (string-prefix-p root dd) + (not (cl-find-if (lambda (module) (string-prefix-p module dd)) + modules))) + (push buf bufs)))) (nreverse bufs))) --000000000000870d2205ec1b6a2e Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Fri, Oct 28, 2022 at 1:58 PM Philip Kaludercic <philipk@posteo.net> wrote:

= > When wanting to clean up behind a project I like to use C-x p k to get=
> rid of everything I have opened related to it.=C2=A0 If I was usin= g Eglot and
> there is still an active LSP server running in the back= ground, killing
> the project fails with these messages:

Thank= s Philip.=C2=A0 This was discussed at

https://github.com/joaotavora/eglot/discu= ssions/822

Some more information is needed:

1. The error = only happens when eglot-autoshutdown has been set to t by
=C2=A0 =C2=A0t= he user.

2. When it has not been set to t, then the behavior is stil= l not
=C2=A0 =C2=A0correct, but the user may not notice it.

3. Ac= cording to Manuel Uberti, the problem also happens with CIDER, a
=C2=A0 = =C2=A0Clojure IDE for Emacs.=C2=A0 So it seems it is not exclusive to Eglot= .

The problem happens because `project-kill-buffers` uses project.el= 's
sense of a project buffer, and then endeavours to kill all such b= uffers.

So far so good, but the determination of pro= ject buffers according
to `project-buffers` considers all buffers whos= e buffer-local default
directory starts with a given root of some projec= t.

This is subtly wrong because it also considers buffers whose name= starts
with space and without buffer-file-names, so-called "hidden= buffers" which
are deemed "uninteresting" to the user (a= ccording to the Elisp manual).
They commonly function as implementation = details of other packages, such
as Eglot (and possibly CIDER).=C2=A0 The= se buffers are not normally visible
to the user in M-x ibuffer, switch-t= o-buffer, etc.

In Eglot's case, the buffer whose name is " = EGLOT process..." is
created by eglot.el and then handed over to js= onrpc.el, which becomes
responsible for it.

Killing this buffer f= rom Lisp using `kill-buffer` is incorrect because
it contradicts Eglot&#= 39;s user preferences eglot-autoreconnect and
eglot-autoshutdown:
1. If eglot-autoshutdown is t, killing the buffer from Lisp kills the
= =C2=A0 =C2=A0process and confuses the LSP shutdown logic, which is a polite=
=C2=A0 =C2=A0"teardown" conversation with the LSP server.=C2= =A0 This is Philip's error.

2. If eglot-autoshutdown is nil but = eglot-autoreconnect is non-nil (in
=C2=A0 =C2=A0fact, these are the defa= ults), killing the buffer has the effect of
=C2=A0 =C2=A0immediately res= tarting the connection, and thus re-creating the
=C2=A0 =C2=A0buffe= r.=C2=A0 The best that can happen is that nothing was achieved
=C2=A0=C2=A0 and only time was wasted.

The fact is that th= e buffer in question is an internal Eglot implementation
det= ail that other packages should stay clear of.=C2=A0

In fact, = I think that all hidden buffers can be considered thusly.
They're ju= st like `--` symbols in obarray or in other symbol's plists:
they= 9;re visible to all Lisp packages but they are implementation details
that shouldn't be messed with except by the owner of such details.
Dmitry tells me that there was some discussion where it was determi= ned
that it's somehow useful in project-kill-buffers to also ta= rget buffers that the
user isn't aware of.

But I= 've not seen evidence of this usefulness.=C2=A0 If there is indeed some= ,
I propose we come up with some convention so that it is possible forpackages to create buffers which are "definitely hidden and private = and
not to me tinkered with".=C2=A0 Such a convention could be star= ting the
buffer name with two spaces.

Whatever the convention, cu= rrently I think that the patch after my
signature is the correct ap= proach to fix this bug.

Thanks,
Jo= =C3=A3o

diff --git a/lisp/progmodes/project.el= b/lisp/progmodes/project.el
index ac278edd40..4f542137a8 100644
--- = a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -352,1= 4 +352,18 @@ project--remote-file-names
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(concat remote-id file))
=C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0local-files))))
=C2=A0
+(def= un project--buffer-uninteresting-p (buf)
+ =C2=A0(and (string-prefix-p &= quot; " (buffer-name buf)) (null (buffer-file-name buf))))
+
=C2= =A0(cl-defgeneric project-buffers (project)
=C2=A0 =C2=A0"Return th= e list of all live buffers that belong to PROJECT."
=C2=A0 =C2=A0(l= et ((root (expand-file-name (file-name-as-directory (project-root project))= ))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0bufs)
=C2=A0 =C2=A0 =C2=A0(dolis= t (buf (buffer-list))
- =C2=A0 =C2=A0 =C2=A0(when (string-prefix-p root = (expand-file-name
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (buffer-= local-value 'default-directory buf)))
- =C2=A0 =C2=A0 =C2=A0 =C2=A0(= push buf bufs)))
+ =C2=A0 =C2=A0 =C2=A0(unless (project--buffer-unintere= sting-p buf)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0(when (string-prefix-p root (e= xpand-file-name
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (= buffer-local-value 'default-directory buf)))
+ =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0(push buf bufs))))
=C2=A0 =C2=A0 =C2=A0(nreverse bufs)))=C2=A0
=C2=A0(defgroup project-vc nil
@@ -680,11 +684,12 @@ project-= buffers
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dd
=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 bufs)
=C2=A0 =C2=A0 =C2=A0(dolist (buf (buffer-list))
-= =C2=A0 =C2=A0 =C2=A0(setq dd (expand-file-name (buffer-local-value 'de= fault-directory buf)))
- =C2=A0 =C2=A0 =C2=A0(when (and (string-prefix-p= root dd)
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (not= (cl-find-if (lambda (module) (string-prefix-p module dd))
- =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0modules)))
- =C2=A0 =C2=A0 =C2=A0 =C2= =A0(push buf bufs)))
+ =C2=A0 =C2=A0 =C2=A0(unless (project--buffer-unin= teresting-p buf)
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0(setq dd (expand-file-name= (buffer-local-value 'default-directory buf)))
+ =C2=A0 =C2=A0 =C2= =A0 =C2=A0(when (and (string-prefix-p root dd)
+ =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (not (cl-find-if (lambda (module)= (string-prefix-p module dd))
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0modules)))
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(push buf bu= fs))))
=C2=A0 =C2=A0 =C2=A0(nreverse bufs)))
=C2=A0
=C2=A0=0C
--000000000000870d2205ec1b6a2e--