bug#51993: 29.0.50; [PATCH] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files

[Jim Porter <jporterbugs@gmail.com>]

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

On 10/24/2022 8:10 PM, Jim Porter wrote:
> On 10/21/2022 11:57 PM, Eli Zaretskii wrote:
>>> Date: Fri, 21 Oct 2022 20:46:47 -0700
>>> Cc: larsi@gnus.org, 51993@debbugs.gnu.org, gregory@heytings.org
>>> From: Jim Porter <jporterbugs@gmail.com>
>>>
>>> The issue in the quote above is that if you enable automatic server
>>> shutdown in Emacs 29, it changes the behavior of exiting an emacsclient
>>> even when it wouldn't stop the server (i.e. when there are other active
>>> clients). That's surprising to me, I wouldn't expect that setting to
>>> affect cases when it decides *not* to kill the Emacs daemon.
>>
>> Sounds like a bug to me, because it contradicts what the doc string
>> says.
> 
> That's how it seems to me too. ...
Ok, after quite a delay, here's a patch for this. Previously, the 
function 'server-stop-automatically--handle-delete-frame' responded to 
both 'C-x C-c' ('save-buffers-kill-terminal') and 'delete-frame', which 
made it more complex. I've moved the 'C-x C-c' case into 
'server-save-buffers-kill-terminal', which simplifies 
'server-stop-automatically--handle-delete-frame'.

The updated 'server-save-buffers-kill-terminal' should now make sure 
that the new stop-automatically behavior only happens when there are no 
other client processes (or nowait frames).

I've also attached a version of the diff excluding the whitespace 
changes to make reviewing the code easier.

[-- Attachment #2: 0001-Make-killing-a-non-last-client-work-the-same-no-matt.patch --]
[-- Type: text/plain, Size: 5307 bytes --]

From 2c0940a86cd11ebad0d1ea7fcf8fe2e9cfbc0fb3 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Mon, 28 Nov 2022 21:17:59 -0800
Subject: [PATCH] Make killing a non-last client work the same no matter the
 auto-stop setting

Previously, if 'server-stop-automatically' was configured for
'kill-terminal' or 'delete-frame', killing a client via
'save-buffers-kill-terminal' wouldn't prompt about the saving files in
the client's buffer list (as it does when not using those settings).
This change ensures that those settings only apply when killing the
last client, as described in the manual (bug#51993).

* lisp/server.el (server-save-buffers-kill-terminal): Handle
'server-stop-automatically' behavior in this function, rather than
calling 'server-stop-automatically--handle-delete-frame'.
(server-stop-automatically--handle-delete-frame): Remove code that
handled the 'save-buffers-kill-terminal' case.
---
 lisp/server.el | 83 ++++++++++++++++++++++++--------------------------
 1 file changed, 40 insertions(+), 43 deletions(-)

diff --git a/lisp/server.el b/lisp/server.el
index 1b027f88ce..e88cffa8fb 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1780,52 +1780,49 @@ server-save-buffers-kill-terminal
 
 If emacsclient was started with a list of filenames to edit, then
 only these files will be asked to be saved."
-  (if server-stop-automatically
-      (server-stop-automatically--handle-delete-frame (selected-frame))
-    (let ((proc (frame-parameter nil 'client)))
-      (cond ((eq proc 'nowait)
-	     ;; Nowait frames have no client buffer list.
-	     (if (cdr (frame-list))
-	         (progn (save-some-buffers arg)
-		        (delete-frame))
-	       ;; If we're the last frame standing, kill Emacs.
-	       (save-buffers-kill-emacs arg)))
-	    ((processp proc)
-	     (let ((buffers (process-get proc 'buffers)))
-	       (save-some-buffers
-	        arg (if buffers
-                        ;; Only files from emacsclient file list.
-		        (lambda () (memq (current-buffer) buffers))
-                      ;; No emacsclient file list: don't override
-                      ;; `save-some-buffers-default-predicate' (unless
-                      ;; ARG is non-nil), since we're not killing
-                      ;; Emacs (unlike `save-buffers-kill-emacs').
-		      (and arg t)))
-	       (server-delete-client proc)))
-	    (t (error "Invalid client frame"))))))
+  (let ((proc (frame-parameter nil 'client)))
+    (cond ((eq proc 'nowait)
+	   ;; Nowait frames have no client buffer list.
+	   (if (length> (frame-list) (if server-stop-automatically 2 1))
+               ;; If there are any other frames, only delete this one.
+               ;; When `server-stop-automatically' is set, don't count
+               ;; the daemon frame.
+	       (progn (save-some-buffers arg)
+		      (delete-frame))
+	     ;; If we're the last frame standing, kill Emacs.
+	     (save-buffers-kill-emacs arg)))
+	  ((processp proc)
+           (if (or (not server-stop-automatically)
+                   (length> server-clients 1)
+                   (seq-some
+                    (lambda (frame)
+                      (when-let ((p (frame-parameter frame 'client)))
+                        (not (eq proc p))))
+                    (frame-list)))
+	       (let ((buffers (process-get proc 'buffers)))
+	         (save-some-buffers
+	          arg (if buffers
+                          ;; Only files from emacsclient file list.
+		          (lambda () (memq (current-buffer) buffers))
+                        ;; No emacsclient file list: don't override
+                        ;; `save-some-buffers-default-predicate' (unless
+                        ;; ARG is non-nil), since we're not killing
+                        ;; Emacs (unlike `save-buffers-kill-emacs').
+		        (and arg t)))
+	         (server-delete-client proc))
+             ;; If `server-stop-automatically' is set, there are no
+             ;; other client processes, and no other client frames
+             ;; (e.g. `nowait' frames), kill Emacs.
+             (save-buffers-kill-emacs arg)))
+	  (t (error "Invalid client frame")))))
 
 (defun server-stop-automatically--handle-delete-frame (frame)
   "Handle deletion of FRAME when `server-stop-automatically' is used."
-  (when server-stop-automatically
-    (if (if (and (processp (frame-parameter frame 'client))
-		 (eq this-command 'save-buffers-kill-terminal))
-	    (progn
-	      (dolist (f (frame-list))
-		(when (and (eq (frame-parameter frame 'client)
-			       (frame-parameter f 'client))
-			   (not (eq frame f)))
-		  (set-frame-parameter f 'client nil)
-		  (let ((server-stop-automatically nil))
-		    (delete-frame f))))
-	      (if (cddr (frame-list))
-		  (let ((server-stop-automatically nil))
-		    (delete-frame frame)
-		    nil)
-		t))
-	  (null (cddr (frame-list))))
-	(let ((server-stop-automatically nil))
-	  (save-buffers-kill-emacs)
-	  (delete-frame frame)))))
+  (when (and server-stop-automatically
+             (null (cddr (frame-list))))
+    (let ((server-stop-automatically nil))
+      (save-buffers-kill-emacs)
+      (delete-frame frame))))
 
 (defun server-stop-automatically--maybe-kill-emacs ()
   "Handle closing of Emacs daemon when `server-stop-automatically' is used."
-- 
2.25.1


[-- Attachment #3: no-whitespace-change.diff --]
[-- Type: text/plain, Size: 3078 bytes --]

diff --git a/lisp/server.el b/lisp/server.el
index 1b027f88ce..e88cffa8fb 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1780,17 +1780,25 @@ server-save-buffers-kill-terminal
 
 If emacsclient was started with a list of filenames to edit, then
 only these files will be asked to be saved."
-  (if server-stop-automatically
-      (server-stop-automatically--handle-delete-frame (selected-frame))
   (let ((proc (frame-parameter nil 'client)))
     (cond ((eq proc 'nowait)
 	   ;; Nowait frames have no client buffer list.
-	     (if (cdr (frame-list))
+	   (if (length> (frame-list) (if server-stop-automatically 2 1))
+               ;; If there are any other frames, only delete this one.
+               ;; When `server-stop-automatically' is set, don't count
+               ;; the daemon frame.
 	       (progn (save-some-buffers arg)
 		      (delete-frame))
 	     ;; If we're the last frame standing, kill Emacs.
 	     (save-buffers-kill-emacs arg)))
 	  ((processp proc)
+           (if (or (not server-stop-automatically)
+                   (length> server-clients 1)
+                   (seq-some
+                    (lambda (frame)
+                      (when-let ((p (frame-parameter frame 'client)))
+                        (not (eq proc p))))
+                    (frame-list)))
 	       (let ((buffers (process-get proc 'buffers)))
 	         (save-some-buffers
 	          arg (if buffers
@@ -1801,31 +1809,20 @@ server-save-buffers-kill-terminal
                         ;; ARG is non-nil), since we're not killing
                         ;; Emacs (unlike `save-buffers-kill-emacs').
 		        (and arg t)))
-	       (server-delete-client proc)))
-	    (t (error "Invalid client frame"))))))
+	         (server-delete-client proc))
+             ;; If `server-stop-automatically' is set, there are no
+             ;; other client processes, and no other client frames
+             ;; (e.g. `nowait' frames), kill Emacs.
+             (save-buffers-kill-emacs arg)))
+	  (t (error "Invalid client frame")))))
 
 (defun server-stop-automatically--handle-delete-frame (frame)
   "Handle deletion of FRAME when `server-stop-automatically' is used."
-  (when server-stop-automatically
-    (if (if (and (processp (frame-parameter frame 'client))
-		 (eq this-command 'save-buffers-kill-terminal))
-	    (progn
-	      (dolist (f (frame-list))
-		(when (and (eq (frame-parameter frame 'client)
-			       (frame-parameter f 'client))
-			   (not (eq frame f)))
-		  (set-frame-parameter f 'client nil)
-		  (let ((server-stop-automatically nil))
-		    (delete-frame f))))
-	      (if (cddr (frame-list))
-		  (let ((server-stop-automatically nil))
-		    (delete-frame frame)
-		    nil)
-		t))
+  (when (and server-stop-automatically
              (null (cddr (frame-list))))
     (let ((server-stop-automatically nil))
       (save-buffers-kill-emacs)
-	  (delete-frame frame)))))
+      (delete-frame frame))))
 
 (defun server-stop-automatically--maybe-kill-emacs ()
   "Handle closing of Emacs daemon when `server-stop-automatically' is used."