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

From: Gregory Heytings <[email-masked]>
Date: Tue, 23 Nov 2021 09:48:13 +0000

>> When killing an emacsclient terminal via C-x C-c, it should prompt to 
>> save the files initially passed to emacsclient. To see this in action:
>>
>>    $ emacs -Q --daemon
>>    $ emacsclient -a "" -c foo.txt
>>    $ emacsclient -a "" -c bar.txt
>>
>>    ;; In the first client frame:
>>    foobar ;; Insert some text
>>    C-x C-c
>>    ;; Emacs prompts "Save file /path/to/foo.txt?..."
>>
>> Now try the above, but call `(server-stop-automatically 'delete-frame)' 
>> first (or replace `delete-frame' with `kill-terminal'; it doesn't 
>> matter). In this case, Emacs doesn't prompt to save the file. However, 
>> the docstring/comments in `server-save-buffers-kill-terminal' say that 
>> it should: "Offer to save each buffer, then kill the current client. 
>> ... Only files from emacsclient file list."
>
> Gregory, any comments?
>

This is not a bug, this is the intented behavior of that feature, which 
was discussed on emacs-devel and in bug#51377.

But in commit 997ca88ef44 the word "last" disappeared in the explanation 
of the meaning of the symbol 'kill-terminal': "when the last frame is 
being closed" became "when the terminal is killed".  Hence the confusion.

I attached a patch which preserves the intended behavior of that feature, 
and adds a fourth possible behavior, the one Jim now wants.

From 4a3b8cecfc091cb48f77f8a5a3a3934ec383d22d Mon Sep 17 00:00:00 2001
From: Gregory Heytings <[email-masked]>
Date: Tue, 23 Nov 2021 09:17:39 +0000
Subject: [PATCH] Improve and extend server-stop-automatically.

* lisp/server.el (server-stop-automatically): Add another allowed
symbol argument, and rename two existing ones.  Update the docstring.
(server-stop-automatically--handle-delete-frame): Handle the new
symbol argument.  Improve the handling of nowait frames.  Use the
prefix argument of server-save-buffers-kill-terminal.
(server-save-buffers-kill-terminal): Pass the prefix argument to
server-stop-automatically--handle-delete-frame.

* doc/emacs/misc.texi (Emacs Server): Update the documentation with
the new allowed symbol argument.

* etc/NEWS: Update the documentation.
---
 doc/emacs/misc.texi | 26 ++++++++++++++--------
 etc/NEWS            |  2 +-
 lisp/server.el      | 53 ++++++++++++++++++++++++++++++---------------
 3 files changed, 54 insertions(+), 27 deletions(-)

diff --git a/doc/emacs/misc.texi b/doc/emacs/misc.texi
index 1f2c852fac..159e50a6c1 100644
--- a/doc/emacs/misc.texi
+++ b/doc/emacs/misc.texi
@@ -1787,17 +1787,25 @@ Emacs Server
 anymore.
 
 @item
-With the argument @code{delete-frame}, when the last client frame is
-being closed, you are asked whether each unsaved file-visiting buffer
-must be saved and each unfinished process can be stopped, and if so,
-the server is stopped.
+With the argument @code{delete-last-frame}, when the last client frame
+is being closed, you are asked whether each unsaved file-visiting
+buffer must be saved and each unfinished process can be stopped, and
+if so, the server is stopped.
 
 @item
-With the argument @code{kill-terminal}, when the last client frame is
-being closed with @kbd{C-x C-c} (@code{save-buffers-kill-terminal}),
-you are asked whether each unsaved file-visiting buffer must be saved
-and each unfinished process can be stopped, and if so, the server is
-stopped.
+With the argument @code{kill-last-terminal} or @code{kill-terminal},
+when the last client terminal is being closed with @kbd{C-x C-c}
+(@code{save-buffers-kill-terminal}), you are asked whether each
+unsaved file-visiting buffer must be saved and each unfinished process
+can be stopped, and if so, the server is stopped.
+
+@item
+With the argument @code{kill-terminal}, when each client terminal but
+the last one is being closed with @kbd{C-x C-c}
+(@code{save-buffers-kill-terminal}), you are also asked whether
+unsaved file-visiting buffers must be saved, or, if
+@command{emacsclient} was started with a list of files to edit,
+whether these files must be saved.
 @end itemize
 
 @findex server-eval-at
diff --git a/etc/NEWS b/etc/NEWS
index bfea4da8b9..2af40106fe 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -197,7 +197,7 @@ frame if one exists; otherwise it creates a new frame.
 *** 'server-stop-automatically' can be used to automatically stop the server.
 The Emacs server will be automatically stopped when certain conditions
 are met.  The conditions are given by the argument, which can be
-'empty', 'delete-frame' or 'kill-terminal'.
+'empty', 'delete-last-frame', 'kill-last-terminal' or 'kill-terminal'.
 
 * Editing Changes in Emacs 29.1
 
diff --git a/lisp/server.el b/lisp/server.el
index 2f003a380a..e2ada697f9 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1728,7 +1728,8 @@ 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))
+      (server-stop-automatically--handle-delete-frame
+       (selected-frame) arg)
     (let ((proc (frame-parameter nil 'client)))
       (cond ((eq proc 'nowait)
 	     ;; Nowait frames have no client buffer list.
@@ -1751,7 +1752,7 @@ server-save-buffers-kill-terminal
 	       (server-delete-client proc)))
 	    (t (error "Invalid client frame"))))))
 
-(defun server-stop-automatically--handle-delete-frame (frame)
+(defun server-stop-automatically--handle-delete-frame (frame arg)
   "Handle deletion of FRAME when `server-stop-automatically' is used."
   (when server-stop-automatically
     (if (if (and (processp (frame-parameter frame 'client))
@@ -1765,13 +1766,22 @@ server-stop-automatically--handle-delete-frame
 		  (let ((server-stop-automatically nil))
 		    (delete-frame f))))
 	      (if (cddr (frame-list))
-		  (let ((server-stop-automatically nil))
-		    (delete-frame frame)
-		    nil)
+		  (let ((kill-terminal
+			 (eq server-stop-automatically 'kill-terminal)))
+		    (let ((server-stop-automatically nil))
+		      (if kill-terminal
+			  (server-save-buffers-kill-terminal arg)
+			(delete-frame frame))
+		      nil))
 		t))
-	  (null (cddr (frame-list))))
+	  (if (and (eq (frame-parameter frame 'client) 'nowait)
+		   (cddr (frame-list)))
+	      (let ((server-stop-automatically nil))
+		(delete-frame frame)
+		nil)
+	    (null (cddr (frame-list)))))
 	(let ((server-stop-automatically nil))
-	  (save-buffers-kill-emacs)
+	  (save-buffers-kill-emacs arg)
 	  (delete-frame frame)))))
 
 (defun server-stop-automatically--maybe-kill-emacs ()
@@ -1797,17 +1807,25 @@ server-stop-automatically
 remaining clients, no remaining unsaved file-visiting buffers,
 and no running processes with a `query-on-exit' flag.
 
-If ARG is the symbol `delete-frame', ask the user when the last
-frame is deleted whether each unsaved file-visiting buffer must
-be saved and each running process with a `query-on-exit' flag
-can be stopped, and if so, stop the server itself.
-
-If ARG is the symbol `kill-terminal', ask the user when the
-terminal is killed with \\[save-buffers-kill-terminal] \
-whether each unsaved file-visiting
-buffer must be saved and each running process with a `query-on-exit'
+If ARG is the symbol `delete-last-frame', ask the user when the
+last frame is deleted whether each unsaved file-visiting buffer
+must be saved and each running process with a `query-on-exit'
 flag can be stopped, and if so, stop the server itself.
 
+If ARG is the symbol `kill-last-terminal' or `kill-terminal',
+ask the user when the last terminal is killed with \
+\\[save-buffers-kill-terminal]
+whether each unsaved file-visiting buffer must be saved and each
+running process with a `query-on-exit' flag can be stopped, and
+if so, stop the server itself.
+
+If ARG is the symbol `kill-terminal', also ask the user when each
+but the last terminal is killed with \\[save-buffers-kill-terminal] \
+whether unsaved
+file-visiting buffers must be saved, or, if emacsclient was
+started with a list of files to edit, whether these files must be
+saved.
+
 Any other value of ARG will cause this function to signal an error.
 
 This function is meant to be called from the user init file."
@@ -1818,9 +1836,10 @@ server-stop-automatically
       (setq server-stop-automatically nil)
       (run-with-timer 10 2
 		      #'server-stop-automatically--maybe-kill-emacs))
-     ((eq arg 'delete-frame)
+     ((eq arg 'delete-last-frame)
       (add-hook 'delete-frame-functions
 		#'server-stop-automatically--handle-delete-frame))
+     ((eq arg 'kill-last-terminal))
      ((eq arg 'kill-terminal))
      (t
       (error "Unexpected argument")))))
-- 
2.33.0