bug#58909: 29.0.50; [WIP PATCH] Deleting the last frame of an emacsclient doesn't ask to save

[Jim Porter <jporterbugs@gmail.com>]

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

On 10/31/2022 5:44 AM, Eli Zaretskii wrote:
>> Date: Sun, 30 Oct 2022 15:29:30 -0700
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>> However, if you use 'C-x 5 0' instead, that terminates the Emacs client,
>> but *doesn't* prompt to save foo.txt. I think it should prompt in this
>> case too: all three of 'C-x #', 'C-x C-c', and 'C-x 5 0' have the effect
>> (in this simple case) of deleting the Emacs client and returning to the
>> calling process. (A user who wanted to bail out of an Emacs client
>> without saving should use 'sever-edit-abort' instead.)
> 
> I'm uneasy with this incompatible behavior change.  I can think of
> some legitimate use cases where "C-x 5 0" should not prompt, e.g., if
> the user intends to keep editing the file, and no application is
> waiting for the client to finish.  Why break such flows?

After thinking about this some more, I realized that I didn't properly 
address this part of your message. If no application is waiting for the 
client to finish, then the user hopefully used "--no-wait" when starting 
emacsclient. We could avoid prompting in that case.[1]

If an application *is* waiting for the client to finish, then 
"--no-wait" would be unset. In that case, prompting the user has some 
value. See the attached patch. (I changed the code to only prompt when 
deleting the last frame of a *non-nowait* client.)

[1] Currently, 'C-x C-c' from a nowait frame prompts via 
'save-some-buffers' (so long as there's another frame; see 
'server-save-buffers-kill-terminal'). Maybe we could avoid prompting 
then too? After prompting, it just calls 'delete-frame', so it's very 
similar to 'C-x 5 0' in this scenario... Still, as you say, it's 
probably good to be careful about making incompatible changes here.

[-- Attachment #2: 0001-When-deleting-the-last-frame-of-an-Emacs-client-ask-.patch --]
[-- Type: text/plain, Size: 3898 bytes --]

From 68b9f41685256870f3409ea468971ee0b64cf136 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 29 Oct 2022 20:40:49 -0700
Subject: [PATCH] When deleting the last frame of an Emacs client, ask to save

* src/frame.c (delete_frame): Call 'delete-frame-functions' with
'call2' instead of 'safe_call2' to allow hooks to quit, cancelling the
frame's deletion.

* lisp/server.el (server-save-some-buffers): New function...
(server-handle-delete-frame, server-save-buffers-kill-terminal):
... use it.
---
 lisp/server.el | 33 ++++++++++++++++++++-------------
 src/frame.c    |  2 +-
 2 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/lisp/server.el b/lisp/server.el
index 90d97c1538..f04f993426 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -479,12 +479,26 @@ server-unselect-display
       (when (buffer-live-p buffer)
 	(kill-buffer buffer)))))
 
+(defun server-save-some-buffers (proc &optional arg)
+  "Save modified file-visiting buffers from the emacsclient file list.
+PROC is the emacsclient process.  If ARG is non-nil, save all
+with no questions."
+  (let ((buffers (process-get proc 'buffers)))
+    (save-some-buffers
+     arg (if buffers
+             ;; Only files from emacsclient file list.
+             (lambda () (memq (current-buffer) buffers))
+           ;; If there's no emacsclient file list: don't override
+           ;; `save-some-buffers-default-predicate' (unless ARG is
+           ;; non-nil).
+           (and arg t)))))
+
 (defun server-handle-delete-frame (frame)
   "Delete the client connection when the emacsclient frame is deleted.
 \(To be used from `delete-frame-functions'.)"
   (let ((proc (frame-parameter frame 'client)))
     (when (and (frame-live-p frame)
-	       proc
+	       (processp proc)
 	       ;; See if this is the last frame for this client.
                (not (seq-some
                      (lambda (f)
@@ -492,7 +506,9 @@ server-handle-delete-frame
                             (eq proc (frame-parameter f 'client))))
                      (frame-list))))
       (server-log (format "server-handle-delete-frame, frame %s" frame) proc)
-      (server-delete-client proc 'noframe)))) ; Let delete-frame delete the frame later.
+      (server-save-some-buffers proc))
+      ;; Let delete-frame delete the frame later.
+      (server-delete-client proc 'noframe)))
 
 (defun server-handle-suspend-tty (terminal)
   "Notify the client process that its tty device is suspended."
@@ -1752,17 +1768,8 @@ server-save-buffers-kill-terminal
 	       ;; 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)))
+             (server-save-some-buffers proc arg)
+	     (server-delete-client proc))
 	    (t (error "Invalid client frame"))))))
 
 (defun server-stop-automatically--handle-delete-frame (frame)
diff --git a/src/frame.c b/src/frame.c
index f076a5ba54..8a85d5a400 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -2037,7 +2037,7 @@ delete_frame (Lisp_Object frame, Lisp_Object force)
       x_clipboard_manager_save_frame (frame);
 #endif
 
-      safe_call2 (Qrun_hook_with_args, Qdelete_frame_functions, frame);
+      call2 (Qrun_hook_with_args, Qdelete_frame_functions, frame);
     }
 
   /* delete_frame_functions may have deleted any frame, including this
-- 
2.25.1