unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Jim Porter <jporterbugs@gmail.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: larsi@gnus.org, 51993@debbugs.gnu.org, gregory@heytings.org
Subject: bug#51993: 29.0.50; [PATCH explanation] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files
Date: Thu, 1 Dec 2022 17:42:18 -0800	[thread overview]
Message-ID: <30673991-caf6-afeb-ec05-d70d6fc45f48@gmail.com> (raw)
In-Reply-To: <83lenrghhg.fsf@gnu.org>

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

On 12/1/2022 9:29 AM, Eli Zaretskii wrote:
> I'm okay with installing the original changes on master, if you indeed
> believe the new code is much cleaner (but then please explain why you think
> so, because I don't think I see that just by looking at the diffs).  But for
> the release branch, I'm not comfortable with making such serious changes in
> a part of server.el that is already way too complicated, what with all the
> fancy shutdown options we strive to support.  There be dragons, and I have
> no intention to release Emacs 29 with buggy server-client editing.  So for
> the release branch, please prepare a safer version of the change, which only
> changes the code which is the immediate cause of the incorrect behavior.

Attached is a patch series that explains in more detail how I arrived at 
the previous version of my patch. This is basically a reconstruction of 
the steps I took when writing it originally. I'll describe my overall 
plan and then address the specific comments you had after that. (This 
might be overly-verbose, but I wanted to put as much detail in as I 
could in the hopes of addressing all your concerns.)

Prior to my patch, 'server-stop-automatically--handle-delete-frame' 
(henceforth 'SSA--handle-delete-frame') can get called in two 
situations: when someone calls 'delete-frame' (it's a hook on 
'delete-frame-functions') or when someone calls 
'save-buffers-kill-emacs' ('server-save-buffers-kill-emacs' delegates to 
it when configured to). To help make the logic easier to follow, I split 
it into two: one for handling 'delete-frame', and one for handling 
'save-buffers-kill-terminal'. See patches 0001 and 0002.

>>  (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))))
> 
> And here you completely rewrote a function.

In patch 0002, you can see (most of) this change that you mentioned: 
since I made one function for each case as described above, the second 
'if' statement's conditional is always false, so I just got rid of the 
conditional and the then-clause, leaving only the else-clause: (null 
(cddr (frame-list))). I also simplified this function a bit in the last 
patch, 0007.

Now for the rest of the patch series.

The original bug is that the behavior of 
'server-save-buffers-kill-terminal' when there are multiple clients 
should be the same regardless of the SSA setting (it wasn't). So next, I 
made 'SSA--handle-kill-terminal' have the same basic structure as 
'server-save-buffers-kill-terminal' (patch 0003). That makes it easier 
to see the differences between the two.

Patch 0005 is where the real fix is: it makes sure that when we *don't* 
want to kill Emacs, 'SSA--handle-kill-terminal' does the same thing as 
'server-save-buffers-kill-terminal'.

>> @@ -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")))))
> 
> But this one is problematic: it adds save-buffers-kill-emacs which wasn't in
> the original code, and I don't understand why.  The bug wasn't about this,
> was it?

In patch 0005, you can start to see this block of code take shape: 
because we want to handle the "don't kill Emacs" case in 
'SSA--handle-kill-terminal', we add the 'save-some-buffers' + 
'server-delete-client' code there, resulting in something that looks 
similar to the above hunk.

Then, in patch 0006, I just merge 'server-save-buffers-kill-terminal' 
and 'SSA--handle-kill-terminal', since most of the code is shared at 
this point. Finally, patch 0007 is just a bit of cleanup; with all of 
these applied, server.el should be identical to my previous patch.

Hopefully this explains things reasonably well, and doesn't go into too 
much (or too little) detail. If there are any other bits that you have 
concerns about, just let me know.

[-- Attachment #2: 0001-Duplicate-server-stop-automatically-handle-delete-fr.patch --]
[-- Type: text/plain, Size: 2246 bytes --]

From 74c724a46b16782d6827357dd64efbafe8aeb92f Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 1 Dec 2022 11:33:18 -0800
Subject: [PATCH 1/7] Duplicate
 'server-stop-automatically--handle-delete-frame'

* lisp/server.el (server-stop-automatically--handle-delete-frame):
Copy this...
(server-stop-automatically--handle-kill-terminal) ... to this.
(server-save-buffers-kill-terminal): Use it.
---
 lisp/server.el | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/lisp/server.el b/lisp/server.el
index 1b027f88ce..0a59c8496a 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1781,7 +1781,7 @@ 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-kill-terminal (selected-frame))
     (let ((proc (frame-parameter nil 'client)))
       (cond ((eq proc 'nowait)
 	     ;; Nowait frames have no client buffer list.
@@ -1827,6 +1827,29 @@ server-stop-automatically--handle-delete-frame
 	  (save-buffers-kill-emacs)
 	  (delete-frame frame)))))
 
+(defun server-stop-automatically--handle-kill-terminal (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)))))
+
 (defun server-stop-automatically--maybe-kill-emacs ()
   "Handle closing of Emacs daemon when `server-stop-automatically' is used."
   (unless (cdr (frame-list))
-- 
2.25.1


[-- Attachment #3: 0002-Simplify-server-stop-automatically-handlers.patch --]
[-- Type: text/plain, Size: 2139 bytes --]

From be0b4556b9cb1ab882d7e6ce5e85dd20cfdc9d96 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 1 Dec 2022 11:35:00 -0800
Subject: [PATCH 2/7] Simplify server-stop-automatically handlers

* lisp/server.el (server-stop-automatically--handle-delete-frame):
'this-command' is never 'save-buffers-kill-terminal' here; remove
the whole conditional.
(server-stop-automatically--handle-kill-terminal): 'this-command' is
always 'save-buffers-kill-terminal', so remove that test.
---
 lisp/server.el | 20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/lisp/server.el b/lisp/server.el
index 0a59c8496a..64332442d3 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1807,22 +1807,7 @@ server-save-buffers-kill-terminal
 (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))))
+    (if (null (cddr (frame-list)))
 	(let ((server-stop-automatically nil))
 	  (save-buffers-kill-emacs)
 	  (delete-frame frame)))))
@@ -1830,8 +1815,7 @@ server-stop-automatically--handle-delete-frame
 (defun server-stop-automatically--handle-kill-terminal (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))
+    (if (if (processp (frame-parameter frame 'client))
 	    (progn
 	      (dolist (f (frame-list))
 		(when (and (eq (frame-parameter frame 'client)
-- 
2.25.1


[-- Attachment #4: 0003-Restructure-server-stop-automatically-handle-kill-te.patch --]
[-- Type: text/plain, Size: 2820 bytes --]

From db0cffb849c0a0be50db7ad4a3a0d48ff343854b Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 1 Dec 2022 11:42:48 -0800
Subject: [PATCH 3/7] Restructure
 'server-stop-automatically--handle-kill-terminal'

This changes the function to have the same structure as
'server-save-buffers-kill-terminal', but otherwise works exactly the
same as before.  This just takes the nested 'if' statement and brings
it to the top level as 'cond' (note that the 'client' frame parameter
can only be a process or the symbol 'nowait' here, since otherwise
'server-save-buffers-kill-terminal' wouldn't have been called).

* lisp/server.el (server-stop-automatically--handle-kill-terminal):
Restructure.
---
 lisp/server.el | 41 +++++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/lisp/server.el b/lisp/server.el
index 64332442d3..7191387959 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1815,24 +1815,29 @@ server-stop-automatically--handle-delete-frame
 (defun server-stop-automatically--handle-kill-terminal (frame)
   "Handle deletion of FRAME when `server-stop-automatically' is used."
   (when server-stop-automatically
-    (if (if (processp (frame-parameter frame 'client))
-	    (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)))))
+    (let ((proc (frame-parameter frame 'client)))
+      (cond ((eq proc 'nowait)
+             (if (null (cddr (frame-list)))
+                 (let ((server-stop-automatically nil))
+	           (save-buffers-kill-emacs)
+	           (delete-frame frame))))
+            ((processp proc)
+             (if (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))
+	         (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 #5: 0004-Remove-unnecessary-delete-frame-calls-after-save-buf.patch --]
[-- Type: text/plain, Size: 1538 bytes --]

From 694aad432aee73acd2335f5ea5d0f47d0c8379e2 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 1 Dec 2022 11:48:11 -0800
Subject: [PATCH 4/7] Remove unnecessary 'delete-frame' calls after
 'save-buffers-kill-emacs'

No need to delete a frame if we're killing Emacs entirely anyway.

* lisp/server.el (server-stop-automatically--handle-kill-terminal):
Remove a couple 'delete-frame' calls.
---
 lisp/server.el | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/lisp/server.el b/lisp/server.el
index 7191387959..399bf694fd 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1818,9 +1818,7 @@ server-stop-automatically--handle-kill-terminal
     (let ((proc (frame-parameter frame 'client)))
       (cond ((eq proc 'nowait)
              (if (null (cddr (frame-list)))
-                 (let ((server-stop-automatically nil))
-	           (save-buffers-kill-emacs)
-	           (delete-frame frame))))
+	         (save-buffers-kill-emacs)))
             ((processp proc)
              (if (progn
 	           (dolist (f (frame-list))
@@ -1835,9 +1833,7 @@ server-stop-automatically--handle-kill-terminal
 		         (delete-frame frame)
 		         nil)
 		     t))
-	         (let ((server-stop-automatically nil))
-	           (save-buffers-kill-emacs)
-	           (delete-frame frame))))))))
+	         (save-buffers-kill-emacs)))))))
 
 (defun server-stop-automatically--maybe-kill-emacs ()
   "Handle closing of Emacs daemon when `server-stop-automatically' is used."
-- 
2.25.1


[-- Attachment #6: 0005-This-is-the-commit-that-actually-fixes-bug-51993.patch --]
[-- Type: text/plain, Size: 3488 bytes --]

From 3730a0c55ed416ec007397be312c3223ebe078ad Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 1 Dec 2022 12:05:40 -0800
Subject: [PATCH 5/7] This is the commit that actually fixes bug#51993

There are two parts to this:

1. For 'nowait' frames, call 'save-some-buffers' and 'delete-frame'
   when there are other non-daemon frames.

2. For client frames (where the client is a process), don't try to
   delete all the client's frames directly.  Previously, this code
   would delete all *other* frames associated with the client, and
   then either a) delete the current frame if there are still more
   (non-daemon) frames remaining, or b) kill Emacs otherwise.  Now,
   check to see what we should do first: if there are no other clients
   (or frames not owned by the current client), then we should kill
   Emacs; otherwise, only delete this client (after saving).

* lisp/server.el (server-stop-automatically--handle-kill-terminal):
Handle the case when we *don't* kill Emacs, as described above.
---
 lisp/server.el | 41 ++++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/lisp/server.el b/lisp/server.el
index 399bf694fd..9b91fbbbdc 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1817,23 +1817,34 @@ server-stop-automatically--handle-kill-terminal
   (when server-stop-automatically
     (let ((proc (frame-parameter frame 'client)))
       (cond ((eq proc 'nowait)
-             (if (null (cddr (frame-list)))
+             (if (cddr (frame-list))
+                 ;; If there are any other non-daemon frames, don't
+                 ;; kill Emacs, but *do* save some buffers, as in
+                 ;; `server-save-buffers-kill-terminal'.
+                 (progn
+                   (save-some-buffers)
+	           (delete-frame))
+                 ;; If we're the last non-daemon frame standing, kill
+                 ;; Emacs.
 	         (save-buffers-kill-emacs)))
             ((processp proc)
-             (if (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))
-	         (save-buffers-kill-emacs)))))))
+             (if ;; Keep the server alive if...
+                 (or
+                  ;; a) there are any other clients, or...
+                  (length> server-clients 1)
+                  ;; b) there are any frames not owned by this client.
+                  (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
+                    nil (if buffers
+                            ;; Only files from emacsclient file list.
+		            (lambda () (memq (current-buffer) buffers))))
+                   (server-delete-client))
+               (save-buffers-kill-emacs)))))))
 
 (defun server-stop-automatically--maybe-kill-emacs ()
   "Handle closing of Emacs daemon when `server-stop-automatically' is used."
-- 
2.25.1


[-- Attachment #7: 0006-Merge-kill-terminal-implementations.patch --]
[-- Type: text/plain, Size: 5747 bytes --]

From fe6ff62de3f18c2d8f9c0a0032fff39615380299 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 1 Dec 2022 12:19:44 -0800
Subject: [PATCH 6/7] Merge kill-terminal implementations

Now that 'server-stop-automatically--handle-kill-terminal' and
'server-save-buffers-kill-terminal' are almost the same, merge them
into a single function.

* lisp/server.el (server-stop-automatically--handle-kill-terminal):
Merge this...
(server-save-buffers-kill-terminal): ... into this.
---
 lisp/server.el | 92 +++++++++++++++++++-------------------------------
 1 file changed, 35 insertions(+), 57 deletions(-)

diff --git a/lisp/server.el b/lisp/server.el
index 9b91fbbbdc..6eeaa932a7 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1780,29 +1780,41 @@ 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-kill-terminal (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."
@@ -1812,40 +1824,6 @@ server-stop-automatically--handle-delete-frame
 	  (save-buffers-kill-emacs)
 	  (delete-frame frame)))))
 
-(defun server-stop-automatically--handle-kill-terminal (frame)
-  "Handle deletion of FRAME when `server-stop-automatically' is used."
-  (when server-stop-automatically
-    (let ((proc (frame-parameter frame 'client)))
-      (cond ((eq proc 'nowait)
-             (if (cddr (frame-list))
-                 ;; If there are any other non-daemon frames, don't
-                 ;; kill Emacs, but *do* save some buffers, as in
-                 ;; `server-save-buffers-kill-terminal'.
-                 (progn
-                   (save-some-buffers)
-	           (delete-frame))
-                 ;; If we're the last non-daemon frame standing, kill
-                 ;; Emacs.
-	         (save-buffers-kill-emacs)))
-            ((processp proc)
-             (if ;; Keep the server alive if...
-                 (or
-                  ;; a) there are any other clients, or...
-                  (length> server-clients 1)
-                  ;; b) there are any frames not owned by this client.
-                  (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
-                    nil (if buffers
-                            ;; Only files from emacsclient file list.
-		            (lambda () (memq (current-buffer) buffers))))
-                   (server-delete-client))
-               (save-buffers-kill-emacs)))))))
-
 (defun server-stop-automatically--maybe-kill-emacs ()
   "Handle closing of Emacs daemon when `server-stop-automatically' is used."
   (unless (cdr (frame-list))
-- 
2.25.1


[-- Attachment #8: 0007-Simplify-server-stop-automatically-handle-delete-fra.patch --]
[-- Type: text/plain, Size: 1263 bytes --]

From a79b4f34faf2c0bd30f8d66f87e4d84832477371 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 1 Dec 2022 12:23:17 -0800
Subject: [PATCH 7/7] Simplify 'server-stop-automatically--handle-delete-frame'

* lisp/server.el (server-stop-automatically--handle-delete-frame):
Combine the 'when' and 'if' into a single conditional.
---
 lisp/server.el | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lisp/server.el b/lisp/server.el
index 6eeaa932a7..e88cffa8fb 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1818,11 +1818,11 @@ server-save-buffers-kill-terminal
 
 (defun server-stop-automatically--handle-delete-frame (frame)
   "Handle deletion of FRAME when `server-stop-automatically' is used."
-  (when server-stop-automatically
-    (if (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


  parent reply	other threads:[~2022-12-02  1:42 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-20  4:29 bug#51993: 29.0.50; [PATCH] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files Jim Porter
2021-11-20  7:13 ` Eli Zaretskii
2021-11-23  9:48   ` Gregory Heytings
2021-11-23 18:25     ` Jim Porter
2021-11-23 20:37       ` Gregory Heytings
2021-11-23 22:08         ` Jim Porter
2021-11-23 22:49           ` Gregory Heytings
2021-11-23 23:42             ` Jim Porter
2021-11-23 23:59               ` Gregory Heytings
2021-11-24  1:10                 ` Jim Porter
2021-11-29  5:39 ` Jim Porter
2021-11-29 12:41   ` Eli Zaretskii
2021-11-29 13:40     ` Gregory Heytings
2021-11-29 19:31       ` Jim Porter
2022-01-01  0:11         ` Jim Porter
2022-09-09 17:55       ` Lars Ingebrigtsen
2022-09-09 18:04         ` Jim Porter
2022-10-09 22:09           ` Jim Porter
2022-10-10  6:04             ` Eli Zaretskii
2022-10-20  3:14               ` Jim Porter
2022-10-20  6:23                 ` Eli Zaretskii
2022-10-21  5:51                   ` Jim Porter
2022-10-21  6:38                     ` Eli Zaretskii
2022-10-22  3:46                       ` Jim Porter
2022-10-22  6:57                         ` Eli Zaretskii
2022-10-25  3:10                           ` Jim Porter
2022-10-30 22:32                             ` Jim Porter
2022-11-29  5:31                             ` Jim Porter
2022-12-01 17:29                               ` Eli Zaretskii
2022-12-02  1:09                                 ` bug#51993: 29.0.50; [PATCH for 29.1] " Jim Porter
2022-12-02 14:10                                   ` Eli Zaretskii
2022-12-02 21:33                                     ` Jim Porter
2022-12-04 17:56                                       ` Eli Zaretskii
2022-12-04 22:26                                         ` Jim Porter
2022-12-06 22:20                                           ` Jim Porter
2022-12-02  1:42                                 ` Jim Porter [this message]
2022-12-02 14:31                                   ` bug#51993: 29.0.50; [PATCH explanation] " Eli Zaretskii
2021-11-29 19:12     ` bug#51993: 29.0.50; [PATCH] " Jim Porter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=30673991-caf6-afeb-ec05-d70d6fc45f48@gmail.com \
    --to=jporterbugs@gmail.com \
    --cc=51993@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    --cc=gregory@heytings.org \
    --cc=larsi@gnus.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).