From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: brickviking Newsgroups: gmane.emacs.bugs Subject: bug#65277: Fwd: bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed when opening another dir Date: Fri, 18 Aug 2023 20:41:17 +1200 Message-ID: References: <87350mpkho.fsf@gmail.com> <83ttsy57g0.fsf@gnu.org> <831qg15w7b.fsf@gnu.org> <83cyzk4x0y.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="00000000000026243d06032e8062" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="35192"; mail-complaints-to="usenet@ciao.gmane.io" To: 65277@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Aug 18 10:43:31 2023 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 1qWv4s-0008yA-Fe for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 18 Aug 2023 10:43:30 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qWv4U-0000gs-NM; Fri, 18 Aug 2023 04:43:06 -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 1qWv4Q-0000gG-E5 for bug-gnu-emacs@gnu.org; Fri, 18 Aug 2023 04:43:03 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qWv4P-0001CP-Qj for bug-gnu-emacs@gnu.org; Fri, 18 Aug 2023 04:43:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qWv4Q-0004AK-3H for bug-gnu-emacs@gnu.org; Fri, 18 Aug 2023 04:43:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: brickviking Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 18 Aug 2023 08:43:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 65277 X-GNU-PR-Package: emacs Original-Received: via spool by 65277-submit@debbugs.gnu.org id=B65277.169234812615927 (code B ref 65277); Fri, 18 Aug 2023 08:43:02 +0000 Original-Received: (at 65277) by debbugs.gnu.org; 18 Aug 2023 08:42:06 +0000 Original-Received: from localhost ([127.0.0.1]:46101 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qWv3V-00048p-Vu for submit@debbugs.gnu.org; Fri, 18 Aug 2023 04:42:06 -0400 Original-Received: from mail-lf1-x133.google.com ([2a00:1450:4864:20::133]:57573) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qWv3Q-00048G-JA for 65277@debbugs.gnu.org; Fri, 18 Aug 2023 04:42:04 -0400 Original-Received: by mail-lf1-x133.google.com with SMTP id 2adb3069b0e04-4ffa248263cso923759e87.2 for <65277@debbugs.gnu.org>; Fri, 18 Aug 2023 01:42:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1692348114; x=1692952914; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc:subject:date:message-id:reply-to; bh=M2VbMp4FKgXXc97VWEyQ0Cqmm9/EwTmKRtzfGd3pn8c=; b=J7pdsKhO6gU3J25GtDkCkGH0iiJawqLvR64Da8wgmpsxwdTv95giUcbqIe4rfdL7fG YP8r4zvcAMaSxOH8Hf087xnVhklEZvIxUvriAB4bGElUZC6Dl9I308GZRMSY/Z+J14n7 Bp7QX8nS398T8OXzTZVJDYA5rh17Ls9Jp/gRFlIJGW4g1qzDxve11Tdnxrd7YJJkVWr/ Dd/msqoXY7y+BlYaQkg8pXLkd+ZRYtmv7QoSsebqA0Ams0SZBg07d29h16mCmVIqt/8U o1TipnnnXfSvaEH7Wcs7L6exD6C9xIgpfrcJC/e7+07b9WqtwR99YRHREG5wPGb68dKs 4V9Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692348114; x=1692952914; h=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=M2VbMp4FKgXXc97VWEyQ0Cqmm9/EwTmKRtzfGd3pn8c=; b=UTeGiELeyoaRHdRM5mqhoQ/3t0TIyni/tJK+zXxA99W7rOy2mrBpJbuVrBfCuDRBTP 3I3epoopMHBHqmtOGwOolw27A5HghzWTYVMGxYYyFGvCJH4x8I7dvPfVVz38coUUwfCM 1sT6dKxiJyruT1S4xAP/du76gJz30/mRP2RiXV7v+g+j4UHscCzSqv59+O303xm/jkDh 4J/5xy/l4y02rES5NRpWIljC3DOTeN5hQo1NO0QNQnRvSpgvffUO7AxqxOp5zdsu0iVH gDHG7RkTdz56ykFobfJ095ruhihx/0TYJ3+3VXeb3YPXLTAkhnTl0SfZx9OarIGlH9ec Brqw== X-Gm-Message-State: AOJu0Yx2LaY3TbqWqphMOi+gFsAVgqvNlba/18L3zliQZmQJtMxjJ1o0 oA5wxUVM8p2aOpN5gsgURsqewgMGL0ksrskVlEaxcfaR X-Google-Smtp-Source: AGHT+IFg7oNNX5nXvjSPpGiGce65ea2kJP7QlHxZJiH6+5zg3BdsJLkFJIvOabzStn3WOQ7ev779fpY7nldh5wIivrE= X-Received: by 2002:a05:6512:39c5:b0:4f8:4216:e91f with SMTP id k5-20020a05651239c500b004f84216e91fmr1648556lfu.63.1692348113624; Fri, 18 Aug 2023 01:41:53 -0700 (PDT) In-Reply-To: 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: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:267741 Archived-At: --00000000000026243d06032e8062 Content-Type: text/plain; charset="UTF-8" Apologies, this didn't quite get to the right place. ---------- Forwarded message --------- From: brickviking Date: Fri, 18 Aug 2023 at 20:02 Subject: Re: bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed when opening another dir To: Eli Zaretskii whoops, we've crossed paths here somewhat. My last reply was for the second patch set you sent through, not this last one. I'm compiling again with the last patch set you provided, and will have some results for you shortly. Regards, brickviking On Fri, 18 Aug 2023 at 19:42, Eli Zaretskii wrote: > > Cc: 65277@debbugs.gnu.org > > Date: Thu, 17 Aug 2023 22:02:16 +0300 > > From: Eli Zaretskii > > > > > Cc: 65277@debbugs.gnu.org > > > Date: Thu, 17 Aug 2023 12:44:47 +0300 > > > From: Eli Zaretskii > > > > > > Thanks for testing. Could you please test the improved patch below? > > > > Sorry, that patch had a silly mistake. Please use the one below > > instead: > > I have now tested the last patch I sent on a GNU/Linux system with > using "emacsclient -t", and found another problem. The updated patch > below should solve it, so p-lease try that when you have time. > > With the patch below, the result of "C-x C-v" in a client > terminal/frame is that the frame stays around, and also emacsclient > doesn't exit. However, the buffer that visits the alternate file is > not considered a "client" buffer, so "C-x #" displays an error message > saying there are no client buffers left and does nothing else, and > "C-x k" doesn't delete the client frame. To delete the client frame > and cause emacsclient to exit, you need to type either "C-x C-c" or > "C-x 5 0". This might be a bit surprising, but OTOH the semantics of > the client buffer after "C-x C-v" is not well defined: should the > buffer visiting the alternate file have the same client property and > share the same kill-buffer-hook? I could argue either way. > > Here's the patch to try. If there are no objections or bugs reported > against it, I will install this on master in a week or so. > > diff --git a/lisp/files.el b/lisp/files.el > index 68c0a10..3466a53 100644 > --- a/lisp/files.el > +++ b/lisp/files.el > @@ -1998,6 +1998,8 @@ kill-buffer-hook > Note: Be careful with let-binding this hook considering it is > frequently used for cleanup.") > > +(defvar find-alternate-file-dont-kill-client nil > + "If non-nil, `server-buffer-done' should not delete the client.") > (defun find-alternate-file (filename &optional wildcards) > "Find file FILENAME, select its buffer, kill previous buffer. > If the current buffer now contains an empty file that you just visited > @@ -2044,7 +2046,8 @@ find-alternate-file > ;; save a modified buffer visiting a file. Rather, `kill-buffer' > ;; asks that itself. Thus, there's no need to temporarily do > ;; `(set-buffer-modified-p nil)' before running this hook. > - (run-hooks 'kill-buffer-hook) > + (let ((find-alternate-file-dont-kill-client 'dont-kill-client)) > + (run-hooks 'kill-buffer-hook)) > ;; Okay, now we can end-of-life the old buffer. > (if (get-buffer " **lose**") > (kill-buffer " **lose**")) > diff --git a/lisp/server.el b/lisp/server.el > index ba7e02d..10f1559 100644 > --- a/lisp/server.el > +++ b/lisp/server.el > @@ -330,6 +330,9 @@ server-with-environment > (defun server-delete-client (proc &optional noframe) > "Delete PROC, including its buffers, terminals and frames. > If NOFRAME is non-nil, let the frames live. > +If NOFRAME is the symbol \\='dont-kill-client, also don't > +delete PROC or its terminals, just kill its buffers: this is > +for when `find-alternate-file' calls this via `kill-buffer-hook'. > Updates `server-clients'." > (server-log (concat "server-delete-client" (if noframe " noframe")) > proc) > ;; Force a new lookup of client (prevents infinite recursion). > @@ -366,23 +369,28 @@ server-delete-client > (set-frame-parameter frame 'client nil) > (delete-frame frame)))) > > - (setq server-clients (delq proc server-clients)) > + (or (eq noframe 'dont-kill-client) > + (setq server-clients (delq proc server-clients))) > > ;; Delete the client's tty, except on Windows (both GUI and > ;; console), where there's only one terminal and does not make > ;; sense to delete it, or if we are explicitly told not. > (unless (or (eq system-type 'windows-nt) > + ;; 'find-alternate-file' caused the last client > + ;; buffer to be killed, but we will reuse the client > + ;; for another buffer. > + (eq noframe 'dont-kill-client) > (process-get proc 'no-delete-terminal)) > (let ((terminal (process-get proc 'terminal))) > ;; Only delete the terminal if it is non-nil. > (when (and terminal (eq (terminal-live-p terminal) t)) > (delete-terminal terminal)))) > > - ;; Delete the client's process. > - (if (eq (process-status proc) 'open) > - (delete-process proc)) > - > - (server-log "Deleted" proc)))) > + ;; Delete the client's process (or don't). > + (unless (eq noframe 'dont-kill-client) > + (if (eq (process-status proc) 'open) > + (delete-process proc)) > + (server-log "Deleted" proc))))) > > (defvar server-log-time-function #'current-time-string > "Function to generate timestamps for `server-buffer'.") > @@ -1590,7 +1598,8 @@ server-buffer-done > ;; frames, which might change the current buffer. We > ;; don't want that (bug#640). > (save-current-buffer > - (server-delete-client proc)) > + (server-delete-client proc > + > find-alternate-file-dont-kill-client)) > (server-delete-client proc)))))) > (when (and (bufferp buffer) (buffer-name buffer)) > ;; We may or may not kill this buffer; > --00000000000026243d06032e8062 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Apologies, this didn't quite get to the right place.
----= ------ Forwarded message ---------
From: brickviking <brickviking@gmail.com>
Date: Fr= i, 18 Aug 2023 at 20:02
Subject: Re: bug#65277: 29.1.50; emacsclient Dir= ed: frame is closed/killed when opening another dir
To: Eli Zaretskii &l= t;eliz@gnu.org>


whoops, we've crossed paths here somewhat. My last reply = was for the second patch set you sent through, not this last one. I'm c= ompiling again with the last patch set you provided, and will have some res= ults for you shortly.

Regards, brickviking

On Fri, 18 Aug 2023 at 19:42, Eli Zaretskii <eliz@gnu.org> wrote:
> Cc: 65277@debbugs.gnu.org
> Date: Thu, 17 Aug 2023 22:02:16 +0300
> From: Eli Zaretskii <eliz@gnu.org>
>
> > Cc: 65= 277@debbugs.gnu.org
> > Date: Thu, 17 Aug 2023 12:44:47 +0300
> > From: Eli Zaretskii <eliz@gnu.org>
> >
> > Thanks for testing.=C2=A0 Could you please test the improved patc= h below?
>
> Sorry, that patch had a silly mistake.=C2=A0 Please use the one below<= br> > instead:

I have now tested the last patch I sent on a GNU/Linux system with
using "emacsclient -t", and found another problem.=C2=A0 The upda= ted patch
below should solve it, so p-lease try that when you have time.

With the patch below, the result of "C-x C-v" in a client
terminal/frame is that the frame stays around, and also emacsclient
doesn't exit.=C2=A0 However, the buffer that visits the alternate file = is
not considered a "client" buffer, so "C-x #" displays a= n error message
saying there are no client buffers left and does nothing else, and
"C-x k" doesn't delete the client frame.=C2=A0 To delete the = client frame
and cause emacsclient to exit, you need to type either "C-x C-c" = or
"C-x 5 0".=C2=A0 This might be a bit surprising, but OTOH the sem= antics of
the client buffer after "C-x C-v" is not well defined: should the=
buffer visiting the alternate file have the same client property and
share the same kill-buffer-hook?=C2=A0 I could argue either way.

Here's the patch to try.=C2=A0 If there are no objections or bugs repor= ted
against it, I will install this on master in a week or so.

diff --git a/lisp/files.el b/lisp/files.el
index 68c0a10..3466a53 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -1998,6 +1998,8 @@ kill-buffer-hook
=C2=A0Note: Be careful with let-binding this hook considering it is
=C2=A0frequently used for cleanup.")

+(defvar find-alternate-file-dont-kill-client nil
+=C2=A0 "If non-nil, `server-buffer-done' should not delete the cl= ient.")
=C2=A0(defun find-alternate-file (filename &optional wildcards)
=C2=A0 =C2=A0"Find file FILENAME, select its buffer, kill previous buf= fer.
=C2=A0If the current buffer now contains an empty file that you just visite= d
@@ -2044,7 +2046,8 @@ find-alternate-file
=C2=A0 =C2=A0 =C2=A0;; save a modified buffer visiting a file.=C2=A0 Rather= , `kill-buffer'
=C2=A0 =C2=A0 =C2=A0;; asks that itself.=C2=A0 Thus, there's no need to= temporarily do
=C2=A0 =C2=A0 =C2=A0;; `(set-buffer-modified-p nil)' before running thi= s hook.
-=C2=A0 =C2=A0 (run-hooks 'kill-buffer-hook)
+=C2=A0 =C2=A0 (let ((find-alternate-file-dont-kill-client 'dont-kill-c= lient))
+=C2=A0 =C2=A0 =C2=A0 (run-hooks 'kill-buffer-hook))
=C2=A0 =C2=A0 =C2=A0;; Okay, now we can end-of-life the old buffer.
=C2=A0 =C2=A0 =C2=A0(if (get-buffer " **lose**")
=C2=A0 =C2=A0 =C2=A0 =C2=A0 (kill-buffer " **lose**"))
diff --git a/lisp/server.el b/lisp/server.el
index ba7e02d..10f1559 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -330,6 +330,9 @@ server-with-environment
=C2=A0(defun server-delete-client (proc &optional noframe)
=C2=A0 =C2=A0"Delete PROC, including its buffers, terminals and frames= .
=C2=A0If NOFRAME is non-nil, let the frames live.
+If NOFRAME is the symbol \\=3D'dont-kill-client, also don't
+delete PROC or its terminals, just kill its buffers: this is
+for when `find-alternate-file' calls this via `kill-buffer-hook'.<= br> =C2=A0Updates `server-clients'."
=C2=A0 =C2=A0(server-log (concat "server-delete-client" (if nofra= me " noframe")) proc)
=C2=A0 =C2=A0;; Force a new lookup of client (prevents infinite recursion).=
@@ -366,23 +369,28 @@ server-delete-client
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (set-frame-parameter frame 'c= lient nil)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (delete-frame frame))))

-=C2=A0 =C2=A0 =C2=A0 (setq server-clients (delq proc server-clients))
+=C2=A0 =C2=A0 =C2=A0 (or (eq noframe 'dont-kill-client)
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (setq server-clients (delq proc server-= clients)))

=C2=A0 =C2=A0 =C2=A0 =C2=A0;; Delete the client's tty, except on Window= s (both GUI and
=C2=A0 =C2=A0 =C2=A0 =C2=A0;; console), where there's only one terminal= and does not make
=C2=A0 =C2=A0 =C2=A0 =C2=A0;; sense to delete it, or if we are explicitly t= old not.
=C2=A0 =C2=A0 =C2=A0 =C2=A0(unless (or (eq system-type 'windows-nt)
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ;; 'fin= d-alternate-file' caused the last client
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ;; buffer t= o be killed, but we will reuse the client
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ;; for anot= her buffer.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (eq noframe= 'dont-kill-client)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(proce= ss-get proc 'no-delete-terminal))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 (let ((terminal (process-get proc 'terminal= )))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ;; Only delete the terminal if it is non= -nil.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (when (and terminal (eq (terminal-live-p= terminal) t))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (delete-terminal terminal))))

-=C2=A0 =C2=A0 =C2=A0 ;; Delete the client's process.
-=C2=A0 =C2=A0 =C2=A0 (if (eq (process-status proc) 'open)
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(delete-process proc))
-
-=C2=A0 =C2=A0 =C2=A0 (server-log "Deleted" proc))))
+=C2=A0 =C2=A0 =C2=A0 ;; Delete the client's process (or don't). +=C2=A0 =C2=A0 =C2=A0 (unless (eq noframe 'dont-kill-client)
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 (if (eq (process-status proc) 'open)
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(delete-process proc))
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 (server-log "Deleted" proc)))))

=C2=A0(defvar server-log-time-function #'current-time-string
=C2=A0 =C2=A0"Function to generate timestamps for `server-buffer'.= ")
@@ -1590,7 +1598,8 @@ server-buffer-done
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ;; frames, which mi= ght change the current buffer.=C2=A0 We
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ;; don't want t= hat (bug#640).
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (save-current-buffe= r
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(server-dele= te-client proc))
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(server-dele= te-client proc
+=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 =C2=A0 =C2=A0 find-alt= ernate-file-dont-kill-client))
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (server-delete-client proc= ))))))
=C2=A0 =C2=A0 =C2=A0(when (and (bufferp buffer) (buffer-name buffer))
=C2=A0 =C2=A0 =C2=A0 =C2=A0;; We may or may not kill this buffer;
--00000000000026243d06032e8062--