bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed when opening another dir

bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed when opening another dir

[Maxim Kim]

1. Create one line ~/.emacs file with

   (setq dired-kill-when-opening-new-dired-buffer t)

2. Run `emacsclient -t -a "" .` to see a dired buffer for a current
working directory.

3. Navigate to a directory and press RET to open it.

Emacs frame is closed. Not sure if this is known/expected behavior.

https://asciinema.org/a/eivCdNRj789vs2SGiXJ26aNBN


In GNU Emacs 29.1.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version
 3.24.37, cairo version 1.16.0) of 2023-07-31 built on AUWW00125
Repository revision: 80c9f491fc8dc068421f32cd15e0b387a6d1ee11
Repository branch: emacs-29
System Description: Debian GNU/Linux 12 (bookworm)

Configured using:
 'configure --with-pgtk --with-native-compilation=aot --with-json
 --with-tree-sitter'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NATIVE_COMP
NOTIFY INOTIFY PDUMPER PGTK PNG RSVG SECCOMP SOUND THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP XIM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  rainbow-delimiters-mode: t
  corfu-terminal-mode: t
  corfu-echo-mode: t
  global-corfu-mode: t
  corfu-mode: t
  marginalia-mode: t
  vertico-mode: t
  override-global-mode: t
  pixel-scroll-precision-mode: t
  repeat-mode: t
  savehist-mode: t
  save-place-mode: t
  delete-selection-mode: t
  electric-pair-mode: t
  shell-dirtrack-mode: t
  recentf-mode: t
  winner-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  abbrev-mode: t

Load-path shadows:
/home/habamax/.config/emacs/elpa/transient-20230810.1716/transient hides /usr/local/share/emacs/29.1.50/lisp/transient

Features:
(shadow sort mail-extr emacsbug message yank-media puny dired
dired-loaddefs rfc822 mml mml-sec epa derived epg rfc6068 epg-config
gnus-util text-property-search mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils mule-util orderless time
term/xterm xterm rainbow-delimiters corfu-terminal popon corfu-echo
corfu marginalia vertico compat use-package-ensure edmacro kmacro
use-package-bind-key bind-key easy-mmode use-package-core disp-table
pixel-scroll cua-base repeat savehist saveplace delsel elec-pair
tramp-cache time-stamp tramp-sh tramp tramp-loaddefs trampver
tramp-integration files-x tramp-compat shell pcomplete comint ansi-osc
ansi-color parse-time iso8601 time-date format-spec recentf tree-widget
wid-edit winner ring comp comp-cstr warnings icons cl-extra help-mode
cape-autoloads corfu-terminal-autoloads corfu-autoloads
devdocs-autoloads dune-autoloads dune-format-autoloads elfeed-autoloads
embark-consult-autoloads consult-autoloads embark-autoloads
emms-autoloads erc-hl-nicks-autoloads fennel-mode-autoloads
gdscript-mode-autoloads htmlize-autoloads iedit-autoloads
lua-mode-autoloads magit-autoloads pcase git-commit-autoloads
magit-section-autoloads dash-autoloads marginalia-autoloads
markdown-mode-autoloads ocamlformat-autoloads orderless-autoloads
paredit-autoloads popon-autoloads rainbow-delimiters-autoloads
rainbow-mode-autoloads sly-autoloads tempel-autoloads
transient-autoloads tuareg-autoloads rx caml-autoloads verb-autoloads
vertico-autoloads webpaste-autoloads request-autoloads wgrep-autoloads
with-editor-autoloads info compat-autoloads xclip-autoloads
zig-mode-autoloads reformatter-autoloads package browse-url url
url-proxy url-privacy url-expand url-methods url-history url-cookie
generate-lisp-file url-domsuf url-util mailcap url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs password-cache json subr-x
map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs cl-lib
wildcharm-theme rmc iso-transl tooltip cconv eldoc paren electric
uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/pgtk-win pgtk-win term/common-win pgtk-dnd tool-bar dnd fontset
image regexp-opt fringe tabulated-list replace newcomment text-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify
dynamic-setting system-font-setting font-render-setting cairo gtk pgtk
lcms2 multi-tty make-network-process native-compile emacs)

Memory information:
((conses 16 174342 110568)
 (symbols 48 13948 0)
 (strings 32 44630 19137)
 (string-bytes 1 1543926)
 (vectors 16 23160)
 (vector-slots 8 396866 164847)
 (floats 8 124 1094)
 (intervals 56 342 70)
 (buffers 984 11))

bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed when opening another dir

[Eli Zaretskii]

> From: Maxim Kim <habamax@gmail.com>
> Date: Mon, 14 Aug 2023 09:53:39 +1000
> 
> 
> 1. Create one line ~/.emacs file with
> 
>    (setq dired-kill-when-opening-new-dired-buffer t)
> 
> 2. Run `emacsclient -t -a "" .` to see a dired buffer for a current
> working directory.
> 
> 3. Navigate to a directory and press RET to open it.
> 
> Emacs frame is closed. Not sure if this is known/expected behavior.

There's a more general problem here: any call to find-alternate-file
in the client frame will delete the frame:

  emacs -Q
  M-x server-start

From the shell:

  emacsclient -c SOME-FILE

In the newly-created frame:

  C-x C-v SOME-OTHER-FILE RET

Boom! the frame is deleted.

The patch below should fix that by preventing the frame from being
deleted, but it still deletes the client, so emacsclient exits, and
the client frame no longer has a server.

Does anyone see a problem with the behavior after the patch below is
installed, i.e. with the fact that emacsclient exits?

diff --git a/lisp/files.el b/lisp/files.el
index 03675a3..001a195 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 frame.")
 (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 t))
+      (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..c678924 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1590,7 +1590,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;

bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed when opening another dir

[brickviking]

[-- Attachment #1: Type: text/plain, Size: 812 bytes --]

Relating to the dired problem, I patched the two files Eli stated, and
compiled up a motif client from this git commit
(emacs-29.1-89-g26949819df0) plus the two changes in lisp/files.el and
lisp/server.el. Installed to the usual /usr/local/bin/emacs and duplicated
this set of instructions:

shell1$ emacs -Q
Window opens. I type M-x server-start, window stays around. I switch to
another terminal and:

shell2$ emacsclient -c GNUmakefile
The terminal states "Waiting for Emacs..." and a window opens with content
of buffer.

In that window, I go C-x C-v INSTALL

The terminal returns, but the emacsclient window remains mapped and I'm
able to work in the newly created buffer. Both that window and the original
window with the server started are both mapped and visible.

Hope that helps,

regards, brickviking

[-- Attachment #2: Type: text/html, Size: 1042 bytes --]

bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed when opening another dir

[brickviking]

[-- Attachment #1: Type: text/plain, Size: 1092 bytes --]

Further to the above tests from me which were in graphics mode, I also did
the same test with textmode as follows:

shell1$ emacs -Q  # in graphics mode
A frame renders, I then start up the server:
   M-x server-start

That frame stays around.  Going off to another shell, I started an
emacsclient, but this time in text mode.

   shell2$ emacsclient -t GNUmakefile

The terminal is cleared, and the contents of GNUmakefile appear. A buffer
is assigned, as I can tell in C-x C-b in the original server window.

In the minibuffer (of the emacsclient) I then go:
  C-x C-v INSTALL
and the terminal returns to the shell prompt. Interestingly, the contents
of INSTALL then appear in the original emacs server window, and in the
buffer listing, GNUmakefile has been replaced by INSTALL when I refresh the
buffer listing.

My conclusion is that this bug hasn't exactly been "fixed" with the patches
to files.el and server.el, but at least in graphics mode the frame hangs
around instead of the content turning up in the parent frame with the
server running.

Hope that helps,

Regards,
brickviking

[-- Attachment #2: Type: text/html, Size: 1219 bytes --]

bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed when opening another dir

[Eli Zaretskii]

> From: brickviking <brickviking@gmail.com>
> Date: Wed, 16 Aug 2023 18:37:59 +1200
> 
> Further to the above tests from me which were in graphics mode, I also did the same test with
> textmode as follows:
> 
> shell1$ emacs -Q  # in graphics mode
> A frame renders, I then start up the server:
>    M-x server-start
> 
> That frame stays around.  Going off to another shell, I started an emacsclient, but this time in text
> mode.
> 
>    shell2$ emacsclient -t GNUmakefile
> 
> The terminal is cleared, and the contents of GNUmakefile appear. A buffer is assigned, as I can tell in
> C-x C-b in the original server window.
> 
> In the minibuffer (of the emacsclient) I then go:
>   C-x C-v INSTALL
> and the terminal returns to the shell prompt. Interestingly, the contents of INSTALL then appear in the
> original emacs server window, and in the buffer listing, GNUmakefile has been replaced by INSTALL
> when I refresh the buffer listing.
> 
> My conclusion is that this bug hasn't exactly been "fixed" with the patches to files.el and server.el, but
> at least in graphics mode the frame hangs around instead of the content turning up in the parent
> frame with the server running.

Thanks for testing.  Could you please test the improved patch below?
It includes the original changes you already tested, so please use it
to patch the original files.el and server.el, not the versions already
patched by the changes I sent earlier.

diff --git a/lisp/files.el b/lisp/files.el
index 68c0a10..cc17445 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 t))
+      (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..e616d59 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -372,6 +372,10 @@ server-delete-client
       ;; 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 'find-alternate-file-dont-kill-client)
                   (process-get proc 'no-delete-terminal))
 	(let ((terminal (process-get proc 'terminal)))
 	  ;; Only delete the terminal if it is non-nil.
@@ -1590,7 +1594,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;

bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed when opening another dir

[Eli Zaretskii]

> Cc: 65277@debbugs.gnu.org
> Date: Thu, 17 Aug 2023 12:44:47 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> Thanks for testing.  Could you please test the improved patch below?

Sorry, that patch had a silly mistake.  Please use the one below
instead:

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..1ae1e12 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -372,6 +372,10 @@ server-delete-client
       ;; 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.
@@ -1590,7 +1594,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;

bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed when opening another dir

[Eli Zaretskii]

> Cc: 65277@debbugs.gnu.org
> Date: Thu, 17 Aug 2023 22:02:16 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > Cc: 65277@debbugs.gnu.org
> > Date: Thu, 17 Aug 2023 12:44:47 +0300
> > From: Eli Zaretskii <eliz@gnu.org>
> > 
> > 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;

bug#65277: Fwd: bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed when opening another dir

[brickviking]

[-- Attachment #1: Type: text/plain, Size: 6053 bytes --]

Apologies, this didn't quite get to the right place.

---------- Forwarded message ---------
From: brickviking <brickviking@gmail.com>
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 <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 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 <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: 65277@debbugs.gnu.org
> > > Date: Thu, 17 Aug 2023 12:44:47 +0300
> > > From: Eli Zaretskii <eliz@gnu.org>
> > >
> > > 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;
>

[-- Attachment #2: Type: text/html, Size: 7941 bytes --]

bug#65277: Fwd: bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed when opening another dir

[brickviking]

[-- Attachment #1: Type: text/plain, Size: 1061 bytes --]

Same with this message. Should all be straight now. Eli, sorry about the
mixup.
---------- Forwarded message ---------
From: brickviking <brickviking@gmail.com>
Date: Fri, 18 Aug 2023 at 20:22
Subject: Re: bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed
when opening another dir
To: Eli Zaretskii <eliz@gnu.org>


Okay. Patch applied, compiled, installed, executed emacs from shell1.
----- text -----
    shell1$ emacs -Q
I started server: M-x server-start. Window stays up.

    shell2$ emacsclient -t GNUmakefile
It then opens with contents of GNUmakefile.

I then go C-x C-v INSTALL, hit enter, and get returned back to the prompt.
----- GUI -----
The behaviour differs for the GUI client in that I open emacsclient -C
GNUmakefile after starting the server, and GNUmakefile shows in the window.
When I go C-x C-v INSTALL <RET> the emacsclient window closes (and returns
me to a prompt), and the INSTALL appears in a buffer in the original server
window.
There's nothing strange that appears in the Messages buffer either.

Regards, brickviking

[-- Attachment #2: Type: text/html, Size: 1565 bytes --]

bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed when opening another dir

[Eli Zaretskii]

> From: brickviking <brickviking@gmail.com>
> Date: Fri, 18 Aug 2023 20:22:24 +1200
> 
> Okay. Patch applied, compiled, installed, executed emacs from shell1.
> ----- text -----
>     shell1$ emacs -Q
> I started server: M-x server-start. Window stays up.
> 
>     shell2$ emacsclient -t GNUmakefile
> It then opens with contents of GNUmakefile. 
> 
> I then go C-x C-v INSTALL, hit enter, and get returned back to the prompt.

That's not what I see here.  I see INSTALL, and I don't get back to
the prompt.  If you indeed compiled the latest patch, this is strange.

> The behaviour differs for the GUI client in that I open emacsclient -C GNUmakefile after starting the
> server, and GNUmakefile shows in the window.
> When I go C-x C-v INSTALL <RET> the emacsclient window closes (and returns me to a prompt),
> and the INSTALL appears in a buffer in the original server window.

That's also not what I see.

I suspect you used one of the previous patches, or maybe didn't
install the patch correctly.

Thanks.

bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed when opening another dir

[brickviking]

[-- Attachment #1: Type: text/plain, Size: 2313 bytes --]

Yes, sorry about that. I missed an error when retrieving the patch from the
email. This altered the patch so that one of the hunks didn't succeed (I
missed an initial space off the bottom comment line of the patch).

I've corrected the patch on my end and done some more testing. Configured,
patched, compiled and installed.

---- text ----
    shell1$ emacs -Q

    M-x server-mode
Starts as per normal, window stays up.

    shell2$ emacsclient -t GNUmakefile

This opens a text window with the contents of GNUmakefile.
    C-x C-v INSTALL
INSTALL now opens in the buffer, and I can use it as normal. Trying to
shut the client down with C-x # results in the message
    "No server buffers remain to edit"
(not client buffers as you said will happen) so I just did C-x 5 0 to
terminate the client.

---- GUI ----
Starting up a GUI client with emacsclient -c GNUmakefile shows that file in
the window.
    C-x C-v INSTALL
then shows the install file, and crucially, the terminal doesn't return to
the shell prompt. I still see "Waiting for Emacs..." in the terminal until
I exit the client window.

At least on my end, this appears to work as advertised.

Regards, brickviking


On Sat, 19 Aug 2023 at 00:10, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: brickviking <brickviking@gmail.com>
> > Date: Fri, 18 Aug 2023 20:22:24 +1200
> >
> > Okay. Patch applied, compiled, installed, executed emacs from shell1.
> > ----- text -----
> >     shell1$ emacs -Q
> > I started server: M-x server-start. Window stays up.
> >
> >     shell2$ emacsclient -t GNUmakefile
> > It then opens with contents of GNUmakefile.
> >
> > I then go C-x C-v INSTALL, hit enter, and get returned back to the
> prompt.
>
> That's not what I see here.  I see INSTALL, and I don't get back to
> the prompt.  If you indeed compiled the latest patch, this is strange.
>
> > The behaviour differs for the GUI client in that I open emacsclient -C
> GNUmakefile after starting the
> > server, and GNUmakefile shows in the window.
> > When I go C-x C-v INSTALL <RET> the emacsclient window closes (and
> returns me to a prompt),
> > and the INSTALL appears in a buffer in the original server window.
>
> That's also not what I see.
>
> I suspect you used one of the previous patches, or maybe didn't
> install the patch correctly.
>
> Thanks.
>

[-- Attachment #2: Type: text/html, Size: 2973 bytes --]

bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed when opening another dir

[Eli Zaretskii]

> From: brickviking <brickviking@gmail.com>
> Date: Sat, 19 Aug 2023 12:51:13 +1200
> Cc: 65277@debbugs.gnu.org
> 
> Yes, sorry about that. I missed an error when retrieving the patch from the email. This altered the
> patch so that one of the hunks didn't succeed (I missed an initial space off the bottom comment line
> of the patch).
> 
> I've corrected the patch on my end and done some more testing. Configured, patched, compiled and
> installed.
> 
> ---- text ----
>     shell1$ emacs -Q
> 
>     M-x server-mode
> Starts as per normal, window stays up.
> 
>     shell2$ emacsclient -t GNUmakefile
> 
> This opens a text window with the contents of GNUmakefile. 
>     C-x C-v INSTALL
> INSTALL now opens in the buffer, and I can use it as normal. Trying to
> shut the client down with C-x # results in the message
>     "No server buffers remain to edit"
> (not client buffers as you said will happen) so I just did C-x 5 0 to terminate the client.
> 
> ---- GUI ----
> Starting up a GUI client with emacsclient -c GNUmakefile shows that file in the window. 
>     C-x C-v INSTALL
> then shows the install file, and crucially, the terminal doesn't return to the shell prompt. I still see
> "Waiting for Emacs..." in the terminal until I exit the client window.
> 
> At least on my end, this appears to work as advertised.

Thanks.  So I've now installed the fix on the master branch, and I'm
closing this bug.

bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed when opening another dir

[brickviking]

[-- Attachment #1: Type: text/plain, Size: 1989 bytes --]

One more thing, as it's now fixed in master, would it be a good idea to
push this to the emacs-29 branch as well? I seem to have gathered a tiny
patch file that I apply each time that includes this, and I'd like to trim
down my patch file. Sorry it's taken so long (*) to get to it, I literally
haven't thought about it until now.

(*) regarding your conversation on another topic about release cadences.

Regards, brickviking


On Sat, 19 Aug 2023 at 19:35, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: brickviking <brickviking@gmail.com>
> > Date: Sat, 19 Aug 2023 12:51:13 +1200
> > Cc: 65277@debbugs.gnu.org
> >
> > Yes, sorry about that. I missed an error when retrieving the patch from
> the email. This altered the
> > patch so that one of the hunks didn't succeed (I missed an initial space
> off the bottom comment line
> > of the patch).
> >
> > I've corrected the patch on my end and done some more testing.
> Configured, patched, compiled and
> > installed.
> >
> > ---- text ----
> >     shell1$ emacs -Q
> >
> >     M-x server-mode
> > Starts as per normal, window stays up.
> >
> >     shell2$ emacsclient -t GNUmakefile
> >
> > This opens a text window with the contents of GNUmakefile.
> >     C-x C-v INSTALL
> > INSTALL now opens in the buffer, and I can use it as normal. Trying to
> > shut the client down with C-x # results in the message
> >     "No server buffers remain to edit"
> > (not client buffers as you said will happen) so I just did C-x 5 0 to
> terminate the client.
> >
> > ---- GUI ----
> > Starting up a GUI client with emacsclient -c GNUmakefile shows that file
> in the window.
> >     C-x C-v INSTALL
> > then shows the install file, and crucially, the terminal doesn't return
> to the shell prompt. I still see
> > "Waiting for Emacs..." in the terminal until I exit the client window.
> >
> > At least on my end, this appears to work as advertised.
>
> Thanks.  So I've now installed the fix on the master branch, and I'm
> closing this bug.
>

[-- Attachment #2: Type: text/html, Size: 2726 bytes --]

bug#65277: 29.1.50; emacsclient Dired: frame is closed/killed when opening another dir

[Eli Zaretskii]

> From: brickviking <brickviking@gmail.com>
> Date: Mon, 28 Aug 2023 13:09:12 +1200
> Cc: 65277-done@debbugs.gnu.org
> 
> One more thing, as it's now fixed in master, would it be a good idea to push this to the emacs-29
> branch as well? I seem to have gathered a tiny patch file that I apply each time that includes this, and
> I'd like to trim down my patch file. Sorry it's taken so long (*) to get to it, I literally haven't thought about
> it until now.

Sorry, no.  The change is quite significant, and in a delicate place.
Emacs server and client are used by a lot of users in many different
patterns and use cases, and I'm not comfortable with making such
changes in bug-fix releases, since this issue was with us forever.

However, feel free to modify your local copy of server.el that came
with Emacs 29: since server.el is not preloaded, you can easily do
that for your own use.