unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#51993: 29.0.50; [PATCH] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files
@ 2021-11-20  4:29 Jim Porter
  2021-11-20  7:13 ` Eli Zaretskii
  2021-11-29  5:39 ` Jim Porter
  0 siblings, 2 replies; 15+ messages in thread
From: Jim Porter @ 2021-11-20  4:29 UTC (permalink / raw)
  To: 51993

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

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."

Attached is a patch to restore this behavior when stopping the server 
automatically. This puts all of the logic into 
`server-save-buffers-kill-terminal', which allows 
`server-stop-automatically--handle-delete-frame' to be simpler. I've 
also added some more detailed comments explaining the logic here, since 
there are some pretty subtle aspects to it.

There's a further subtlety that I should probably mention here though: 
when killing a nowait frame, it would kill Emacs entirely if that were 
the last frame (even in Emacs 28, and probably earlier). The only way 
(that I can think of) that this could come up would be to run:

   $ emacs -Q --eval '(start-server)'
   $ emacsclient -n
   C-x 5 0 ;; in the non-client frame
   C-x C-c ;; in the emacsclient frame

However, when doing this with a regular (non-nowait) client, the last 
step would report the error "Attempt to delete the sole visible or 
iconified frame". Even more oddly, it would work the *second* time you 
tried to kill the client terminal, since `server-delete-client' would 
set the `client' frame-parameter to nil before deleting it; on the 
second attempt, Emacs thinks the frame is a non-client frame (even 
though it is).

I've fixed this in the second patch by following the nowait behavior: if 
you kill a client and *all* the existing frames belong to that client, 
it kills Emacs entirely. I'm not sure this will come up often in 
practice, but it's a fairly simple change.

Some tests would be nice to prevent this from regressing, but I'm not 
sure how to write a test that starts up a daemon and connects clients to 
it...

[-- Attachment #2: 0001-Ensure-killing-an-emacsclient-terminal-prompts-to-sa.patch --]
[-- Type: text/plain, Size: 5802 bytes --]

From 563036eaf0924f4777f25e74e9f611419fc031cf Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Fri, 19 Nov 2021 19:49:38 -0800
Subject: [PATCH 1/2] Ensure killing an emacsclient terminal prompts to save
 files

Previously, when using 'server-stop-automatically', killing an
emacsclient terminal wouldn't prompt the user to save the files
initially passed to emacsclient (unless it was the last client).

* lisp/server.el (server-save-buffers-kill-terminal): Check
'server-stop-automatically' and handle killing Emas as needed.
(server-stop-automatically--handle-delete-frame): Remove logic
specific to 'save-buffers-kill-terminal'.
---
 lisp/server.el | 94 +++++++++++++++++++++++++++-----------------------
 1 file changed, 51 insertions(+), 43 deletions(-)

diff --git a/lisp/server.el b/lisp/server.el
index 2f003a380a..e2d20b1b02 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1727,52 +1727,60 @@ 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 (if server-stop-automatically
+                   (cddr (frame-list))
+                 (cdr (frame-list)))
+               ;; If there are any other frames (excluding the daemon
+               ;; frame when `server-stop-automatically' is non-nil),
+               ;; only delete this frame.  FIXME: It would be nice to
+               ;; delete any other frames created by this frame (as
+               ;; when killing the terminal of an ordinary client
+               ;; below), but we can't distinguish separate groups of
+               ;; nowait frames currently.
+               (progn (save-some-buffers arg)
+                      (delete-frame))
+             ;; If we're the last frame standing, kill Emacs.
+             (save-buffers-kill-emacs arg)))
+          ((processp proc)
+           (if (seq-some
+                (lambda (frame)
+                  (let ((p (frame-parameter frame 'client)))
+                    (unless (and server-stop-automatically (null p))
+                      (not (equal proc p)))))
+                (frame-list))
+               ;; If there are any frames from other clients, only
+               ;; delete this client.  If `server-stop-automatically'
+               ;; is nil, check for frames from the server process as
+               ;; well.
+               (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 all frames are from this client, 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 ;; Check that the frame is a client frame.
+             ;; Note: `server-delete-client' sets `client' to nil
+             ;; before calling `delete-frame'. That's good, since we
+             ;; don't want to call `save-buffers-kill-emacs' in that
+             ;; case anyway.
+             (frame-parameter frame 'client)
+             (null (cddr (frame-list))))
+    (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 #3: 0002-Don-t-explicitly-delete-client-frames-when-killing-E.patch --]
[-- Type: text/plain, Size: 2315 bytes --]

From cccba7784d7a5af41b651ab0c29d2339bebc6732 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Fri, 19 Nov 2021 20:14:33 -0800
Subject: [PATCH 2/2] Don't explicitly delete client frames when killing Emacs
 anyway

This resolves an obscure issue where killing an emacsclient terminal
when there were no other frames would fail. Now, killing the terminal
properly kills Emacs.

* lisp/server (server-start): Add 'noframe' option to avoid deleting
frames.
(server-force-stop): Pass 'noframe' to 'server-stop'.
---
 lisp/server.el | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lisp/server.el b/lisp/server.el
index e2d20b1b02..ec32da4fdb 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -609,7 +609,7 @@ server-get-auth-key
     (server-generate-key)))
 
 ;;;###autoload
-(defun server-start (&optional leave-dead inhibit-prompt)
+(defun server-start (&optional leave-dead inhibit-prompt noframe)
   "Allow this Emacs process to be a server for client processes.
 This starts a server communications subprocess through which client
 \"editors\" can send your editing commands to this Emacs job.
@@ -619,6 +619,10 @@ server-start
 Optional argument LEAVE-DEAD (interactively, a prefix arg) means just
 kill any existing server communications subprocess.
 
+If NOFRAME is non-nil, let any existing frames associated with a
+client process live.  This is useful, for example, when killing
+Emacs, in which case the frames will die anyway.
+
 If a server is already running, restart it.  If clients are
 running, ask the user for confirmation first, unless optional
 argument INHIBIT-PROMPT is non-nil.
@@ -681,7 +685,7 @@ server-start
          (setq leave-dead t)))
       ;; If this Emacs already had a server, clear out associated status.
       (while server-clients
-	(server-delete-client (car server-clients)))
+	(server-delete-client (car server-clients) noframe))
       ;; Now any previous server is properly stopped.
       (if leave-dead
 	  (progn
@@ -740,7 +744,7 @@ server-start
 (defun server-force-stop ()
   "Kill all connections to the current server.
 This function is meant to be called from `kill-emacs-hook'."
-  (server-start t t))
+  (server-start t t 'noframe))
 
 ;;;###autoload
 (defun server-force-delete (&optional name)
-- 
2.25.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#51993: 29.0.50; [PATCH] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files
  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-29  5:39 ` Jim Porter
  1 sibling, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2021-11-20  7:13 UTC (permalink / raw)
  To: Jim Porter, Gregory Heytings; +Cc: 51993

> From: Jim Porter <jporterbugs@gmail.com>
> Date: Fri, 19 Nov 2021 20:29:43 -0800
> 
> 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?

Thanks.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#51993: 29.0.50; [PATCH] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files
  2021-11-20  7:13 ` Eli Zaretskii
@ 2021-11-23  9:48   ` Gregory Heytings
  2021-11-23 18:25     ` Jim Porter
  0 siblings, 1 reply; 15+ messages in thread
From: Gregory Heytings @ 2021-11-23  9:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Jim Porter, 51993

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


>> 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.

[-- Attachment #2: Type: text/x-diff, Size: 7496 bytes --]

From 4a3b8cecfc091cb48f77f8a5a3a3934ec383d22d Mon Sep 17 00:00:00 2001
From: Gregory Heytings <gregory@heytings.org>
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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#51993: 29.0.50; [PATCH] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files
  2021-11-23  9:48   ` Gregory Heytings
@ 2021-11-23 18:25     ` Jim Porter
  2021-11-23 20:37       ` Gregory Heytings
  0 siblings, 1 reply; 15+ messages in thread
From: Jim Porter @ 2021-11-23 18:25 UTC (permalink / raw)
  To: Gregory Heytings, Eli Zaretskii; +Cc: larsi, 51993


(Cc'ing Lars since he merged the previous patch, and I want to be sure 
everyone's on the same page here.)

On 11/23/2021 1:48 AM, Gregory Heytings wrote:
> This is not a bug, this is the intented behavior of that feature, which 
> was discussed on emacs-devel and in bug#51377.

I started that discussion[1] (and participated throughout it), and I 
don't think we actually agreed that this was the intended behavior. The 
closest I see is this:

On 10/24/2021 11:08 AM, Jim Porter wrote[2]:
> I don't think this is true in general. The docstring for
> `server-save-buffers-kill-terminal' says: "If emacsclient was started
> with a list of filenames to edit, then only these files will be asked to
> be saved." As a result, some files with unsaved changes may still exist,
> so we'd want to prompt about those *before* the last frame is closed.

However, I should stress that the case I brought up above is just a 
counterexample to show a problem with a previous implementation 
strategy, not a full specification. That's part of why I posted patches 
in bug#51377 in the hopes that an implementation would explain the 
behavior I intend more precisely than prose.

The current behavior on Emacs 29 certainly isn't what I personally 
intended when bringing the idea up on emacs-devel.

On 11/23/2021 1:48 AM, Gregory Heytings wrote:
> I attached a patch which preserves the intended behavior of that 
> feature, and adds a fourth possible behavior, the one Jim now wants.

I'm concerned that we're now up to 4 different behaviors, when I think 
two of them are just the result of a miscommunication between the two of 
us. The way I've interpreted our prior discussion is that you would 
prefer the daemon to be killed invisibly if there's no reason to keep it 
alive; this is the `empty' option in your patches. On the other hand, 
I'd prefer to the daemon to be killed "loudly" when there are no 
non-daemon frames left, including being prompted to save buffers, kill 
processes, etc in all the "usual" cases; this is `delete-last-frame' in 
your patch, plus a couple of other tweaks I have pending. (Note: Just to 
be clear, this isn't a specification; it's only a brief summary.)

I think your message to me in bug#51377 encapsulates this well:

On 10/24/2021 2:37 PM, Gregory Heytings wrote[3]:
> I see.  We have different mental models, I guess.  From my viewpoint
> the Emacs server should stay there until it's not necessary, and I'd be
> surprised to be queried about what to do with buffers opened of
> processes started in a frame I already closed when I want to close
> another frame. But of course I do not object to have both behaviors.

However, in your next two messages in the bug, you added:

On 10/26/2021 3:37 AM, Gregory Heytings wrote[4]:
> If it's now also necessary to kill the daemon when you close the
> last Emacs frame with the window manager close button (I did not see
> this requirement in your original post)...

On 10/26/2021 4:59 AM, Gregory Heytings wrote[5]:
> It just occurred to me that it's very easy to add a third behavior,
> namely the one you expect...

(The "third behavior" is the `delete-frame' option.) As I understand 
things, both the `kill-terminal' and `delete-frame' options were added 
to support my mental model in particular (we were the only two people 
commenting on the bug at that time). From my perspective, 
`kill-terminal` (and now `kill-last-terminal' as well) are just the 
result of some miscommunication between the two of us in bug#51377. 
Unless there's a strong argument for keeping them around, I think it 
would be best to remove them so that there are only 2 options (well, 3 
if you include the default behavior).

I hope the above explains things reasonably thoroughly for everyone 
(hence all the citations to previous discussions). If pressed, I could 
probably create a full specification of the behavior I'd like to see, 
but I find it quite a bit easier to explain by way of a patch. If anyone 
needs clarification on any of the above, just let me know and I'll try 
to elaborate.

[1] https://lists.gnu.org/archive/html/emacs-devel/2021-10/msg01465.html
[2] https://lists.gnu.org/archive/html/bug-gnu-emacs/2021-10/msg02163.html
[3] https://lists.gnu.org/archive/html/bug-gnu-emacs/2021-10/msg02207.html
[4] https://lists.gnu.org/archive/html/bug-gnu-emacs/2021-10/msg02367.html
[5] https://lists.gnu.org/archive/html/bug-gnu-emacs/2021-10/msg02373.html





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#51993: 29.0.50; [PATCH] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files
  2021-11-23 18:25     ` Jim Porter
@ 2021-11-23 20:37       ` Gregory Heytings
  2021-11-23 22:08         ` Jim Porter
  0 siblings, 1 reply; 15+ messages in thread
From: Gregory Heytings @ 2021-11-23 20:37 UTC (permalink / raw)
  To: Jim Porter; +Cc: 51993, larsi


>> This is not a bug, this is the intented behavior of that feature
>
> I started that discussion (and participated throughout it), and I don't 
> think we actually agreed that this was the intended behavior.
>

This is the behavior I intended (and described in the docstring and 
manual), if you prefer.  And you did not make further comments in 
bug#51377, which can be interpreted as a kind of agreement.

>
> I should stress that the case I brought up above is just a 
> counterexample to show a problem with a previous implementation strategy
>

Which problem?

>
> The current behavior on Emacs 29 certainly isn't what I personally 
> intended when bringing the idea up on emacs-devel.
>

Is the current behavior of Emacs 29 with my patch and 
(server-stop-automatically 'kill-terminal) still not what you want?  If 
not, what is missing?

>
> I'm concerned that we're now up to 4 different behaviors, when I think 
> two of them are just the result of a miscommunication between the two of 
> us.
>

They are not, AFAICS.  The four behaviors are four reasonable options, 
each of which can (and is) described in a short paragraph, and corresponds 
to a different user preference.  I see no reason to remove any of the 
current three behaviors because of an unspecified "problem".  Especially 
given that all these behaviors are implemented in only ~50 lines of Lisp.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#51993: 29.0.50; [PATCH] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files
  2021-11-23 20:37       ` Gregory Heytings
@ 2021-11-23 22:08         ` Jim Porter
  2021-11-23 22:49           ` Gregory Heytings
  0 siblings, 1 reply; 15+ messages in thread
From: Jim Porter @ 2021-11-23 22:08 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: larsi, 51993

On 11/23/2021 12:37 PM, Gregory Heytings wrote:
> 
>>> This is not a bug, this is the intented behavior of that feature
>>
>> I started that discussion (and participated throughout it), and I 
>> don't think we actually agreed that this was the intended behavior.
>>
> 
> This is the behavior I intended (and described in the docstring and 
> manual), if you prefer.  And you did not make further comments in 
> bug#51377, which can be interpreted as a kind of agreement.

Unfortunately, I was sidetracked by other things and didn't have a 
chance to comment before Lars merged the patch. Since it had already 
been merged, I thought it best to follow up in a separate bug once I had 
made concise steps to reproduce the issue and a patch to fix it.

>> I should stress that the case I brought up above is just a 
>> counterexample to show a problem with a previous implementation strategy
>>
> 
> Which problem?

Prior to that comment, your proposed implementation would kill Emacs on 
a timer when there were no non-daemon frames left, which could result in 
unsaved changes to files being lost. I replied to point that out and 
showed some steps to reproduce it: 
<https://lists.gnu.org/archive/html/bug-gnu-emacs/2021-10/msg02163.html>.

>> The current behavior on Emacs 29 certainly isn't what I personally 
>> intended when bringing the idea up on emacs-devel.
>>
> 
> Is the current behavior of Emacs 29 with my patch and 
> (server-stop-automatically 'kill-terminal) still not what you want?  If 
> not, what is missing?

If I'm understanding your patch, the behavior I'm looking for is 
essentially a combination of `kill-terminal' and `delete-last-frame'. I 
may be misunderstanding it though, since the call tree in your patch 
confuses me a bit: with `kill-terminal', 
`server-save-buffers-kill-terminal` calls 
`server-stop-automatically--handle-delete-frame', which then calls 
`server-save-buffers-kill-terminal' again.

One of my other goals in my patch was to simplify the logic in 
`server-save-buffers-kill-terminal' and 
`server-stop-automatically--handle-delete-frame' somewhat. Rather than 
to have `server-stop-automatically--handle-delete-frame' check if it was 
called by `save-buffers-kill-terminal', I found that the implementation 
was simpler (to me, anyway) if that logic was lifted up into 
`server-save-buffers-kill-terminal'.

One benefit of this simplification is that it causes fewer changes in 
behavior compared to not using `server-stop-automatically'. For example, 
normally when a user kills an emacsclient terminal, Emacs will prompt 
about saving files *before* deleting any frames. This is nice because it 
allows the user to back out by pressing C-g, leaving Emacs in (almost) 
the same state it was previously. My patch handles that and allows the 
user to press C-g and leave all the current frames open.

With your patch in this bug, using `kill-terminal' and pressing C-x C-c 
will close all frames for the current client but the current one, and 
only then prompt the user to save buffers. Thus, pressing C-g will leave 
the user with only that last client frame still open.

(Note: to test this behavior, you probably need multiple clients open as 
I outlined in the first post to this bug.)

>> I'm concerned that we're now up to 4 different behaviors, when I think 
>> two of them are just the result of a miscommunication between the two 
>> of us.
> 
> They are not, AFAICS.  The four behaviors are four reasonable options, 
> each of which can (and is) described in a short paragraph, and 
> corresponds to a different user preference.  I see no reason to remove 
> any of the current three behaviors because of an unspecified "problem".  
> Especially given that all these behaviors are implemented in only ~50 
> lines of Lisp.

I've specified the problems. I can try to clarify if there's any 
confusion though. This bug is one such problem.

I don't think that a user who opts in to stopping the Emacs daemon 
automatically is *also* opting in to changing the behavior of whether 
Emacs will prompt about saving files when killing a (non-last) client. 
Since there are other clients, the daemon won't be killed, and so the 
behavior should be identical to what happens without 
`server-stop-automatically'. As a user, I would find it very strange 
that enabling `server-stop-automatically' would change Emacs' behavior 
in ways *other than* stopping the server in certain cases.

Of course, a user may indeed want to be able to kill a client (but not 
the daemon) without being prompted to save files, but I think that's 
independent of whether the daemon should be stopped when the last client 
exits. If users *do* want this behavior, we could add a totally separate 
option for it; this would allow users who don't want to be prompted but 
also don't want `server-stop-automatically' to use it.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#51993: 29.0.50; [PATCH] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files
  2021-11-23 22:08         ` Jim Porter
@ 2021-11-23 22:49           ` Gregory Heytings
  2021-11-23 23:42             ` Jim Porter
  0 siblings, 1 reply; 15+ messages in thread
From: Gregory Heytings @ 2021-11-23 22:49 UTC (permalink / raw)
  To: Jim Porter; +Cc: larsi, 51993


>>> I should stress that the case I brought up above is just a 
>>> counterexample to show a problem with a previous implementation 
>>> strategy
>> 
>> Which problem?
>
> Prior to that comment, your proposed implementation would kill Emacs on 
> a timer when there were no non-daemon frames left, which could result in 
> unsaved changes to files being lost. I replied to point that out and 
> showed some steps to reproduce it: 
> <https://lists.gnu.org/archive/html/bug-gnu-emacs/2021-10/msg02163.html>.
>

I don't think it's useful to discuss a problem that was raised and fixed a 
month ago.

>
> If I'm understanding your patch, the behavior I'm looking for is 
> essentially a combination of `kill-terminal' and `delete-last-frame'.
>

If so, a fifth behavior could be implemented.  But you would have to 
describe what you want, and it should be something reasonable.  Could you 
please do that, and explain what you want when:

1. You delete a frame that is not the last one
2. You delete the last frame
3. You kill a terminal that is not the last one
4. You kill a terminal that is the last one

>
> I've specified the problems. I can try to clarify if there's any 
> confusion though. This bug is one such problem.
>

As I said earlier, the problem you described in this bug report was not a 
bug, at least in the sense that it was not something that was not 
explicitly intended by the one who wrote the code (and documented).  And 
the behavior you wanted is handled by the patch I sent, without removing 
any of the other existing behaviors.  But now you apparently want 
something else again.

>
> I don't think that a user who opts in to stopping the Emacs daemon 
> automatically is *also* opting in to changing the behavior of whether 
> Emacs will prompt about saving files when killing a (non-last) client. 
> Since there are other clients, the daemon won't be killed, and so the 
> behavior should be identical to what happens without 
> `server-stop-automatically'. As a user, I would find it very strange 
> that enabling `server-stop-automatically' would change Emacs' behavior 
> in ways *other than* stopping the server in certain cases.
>

Yet this is what you're asking.  If you want Emacs to prompt you whether 
the files should be saved and the process killed when you delete the last 
frame, you want to change the way Emacs prompts about saving files and 
killing processes, because this (namely prompting the user when the last 
frame is deleted) isn't happening without server-stop-automatically.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#51993: 29.0.50; [PATCH] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files
  2021-11-23 22:49           ` Gregory Heytings
@ 2021-11-23 23:42             ` Jim Porter
  2021-11-23 23:59               ` Gregory Heytings
  0 siblings, 1 reply; 15+ messages in thread
From: Jim Porter @ 2021-11-23 23:42 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: larsi, 51993

Eli, Lars: I'm not sure what either of you would like to do about this 
bug, but it seems that most of the conflict is due to a miscommunication 
between me and Gregory (that's my impression, anyway). I hope my 
previous messages have explained my thoughts on the matter fairly 
thoroughly (if not, just let me know). However, I'm not sure there's 
much more I can add to the discussion beyond this message.

On 11/23/2021 2:49 PM, Gregory Heytings wrote:
> As I said earlier, the problem you described in this bug report was not 
> a bug, at least in the sense that it was not something that was not 
> explicitly intended by the one who wrote the code (and documented).  And 
> the behavior you wanted is handled by the patch I sent, without removing 
> any of the other existing behaviors.  But now you apparently want 
> something else again.

I've only wanted one behavior since I started this discussion on Oct 
19[1]. However, rather than making sure we understand each other's 
descriptions (or consulting the patches I've posted throughout the 
discussion) and have properly identified the corner cases to handle, 
you've instead implemented the behavior "for" me, even though I said 
from the beginning that I was looking to write the patch myself. I never 
posted a rigorous specification of the behavior I wanted for that 
reason: I was soliciting feedback to develop a patch that meets my needs 
(along with the needs of anyone who spoke up at the time, if possible).

The fact that you opted to help by authoring your own patches is 
appreciated, but it ultimately doesn't help me because we seem to be 
talking at cross purposes and your impressions of what I want aren't 
what I actually want. Moreover, if our interpretations *don't* match up 
and I bring up an issue with a proposed patch, that doesn't mean that I 
want an additional option or that I've changed my mind; it just means 
that we haven't reached an understanding yet.

I certainly don't expect you to do any additional work here. I'm 
perfectly happy to provide patches implementing the behavior I have in 
mind, and to adjust them as needed if you or anyone else has feedback on 
them. While I could probably construct a rigorous specification for the 
behavior I want so that someone else (e.g. you) could implement it, that 
would probably end up just being a set of test cases extracted from the 
patch I already have.

As an aside, I mentioned this previously, but I think it would be 
valuable to write some automated test cases to verify that things work 
as expected. However, I didn't see a way to test creating/destroying 
Emacs servers/clients via ERT. I'm certainly open to doing so if someone 
points me in the right direction though.

>> I don't think that a user who opts in to stopping the Emacs daemon 
>> automatically is *also* opting in to changing the behavior of whether 
>> Emacs will prompt about saving files when killing a (non-last) client. 
>> Since there are other clients, the daemon won't be killed, and so the 
>> behavior should be identical to what happens without 
>> `server-stop-automatically'. As a user, I would find it very strange 
>> that enabling `server-stop-automatically' would change Emacs' behavior 
>> in ways *other than* stopping the server in certain cases.
>>
> 
> Yet this is what you're asking.  If you want Emacs to prompt you whether 
> the files should be saved and the process killed when you delete the 
> last frame, you want to change the way Emacs prompts about saving files 
> and killing processes, because this (namely prompting the user when the 
> last frame is deleted) isn't happening without server-stop-automatically.

That's not relevant to the case I'm discussing above. I specifically 
said I'm talking about the behavior when killing the *non-last* client. 
In that case, the server won't be stopped, no matter how 
`server-stop-automatically' is configured.

[1] https://lists.gnu.org/archive/html/emacs-devel/2021-10/msg01465.html





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#51993: 29.0.50; [PATCH] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files
  2021-11-23 23:42             ` Jim Porter
@ 2021-11-23 23:59               ` Gregory Heytings
  2021-11-24  1:10                 ` Jim Porter
  0 siblings, 1 reply; 15+ messages in thread
From: Gregory Heytings @ 2021-11-23 23:59 UTC (permalink / raw)
  To: Jim Porter; +Cc: larsi, 51993


>
> I never posted a rigorous specification of the behavior I wanted for 
> that reason: I was soliciting feedback to develop a patch that meets my 
> needs.
>

That's not what you did, you posted a patch claiming that one of the 
existing behaviors had a bug and should be replaced with another one.

>
> you've instead implemented the behavior "for" me
>

That's not what I did, I tried to implement a function that provides 
different possible behaviors corresponding to different needs and 
expectations, not just the behavior you expected.  And I did this based on 
your feedback, what others said on emacs-devel, and my own feelings.  I 
hoped (and thought until today) that the behavior you expected was one of 
them.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#51993: 29.0.50; [PATCH] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files
  2021-11-23 23:59               ` Gregory Heytings
@ 2021-11-24  1:10                 ` Jim Porter
  0 siblings, 0 replies; 15+ messages in thread
From: Jim Porter @ 2021-11-24  1:10 UTC (permalink / raw)
  To: Gregory Heytings; +Cc: larsi, 51993

On 11/23/2021 3:59 PM, Gregory Heytings wrote:
>>
>> I never posted a rigorous specification of the behavior I wanted for 
>> that reason: I was soliciting feedback to develop a patch that meets 
>> my needs.
>>
> 
> That's not what you did, you posted a patch claiming that one of the 
> existing behaviors had a bug and should be replaced with another one.

That's in reference to my original message on Oct 19[1], which I cited 
earlier in the paragraph that you excerpted the quote from. At the time, 
I said:

> I didn't see any options to configure this behavior in Emacs, but 
> looking over the code, it shouldn't be that hard for me to write a patch 
> to add this as an option. Before I started though, I wanted to see what 
> others thought. Is this a behavior others would be interested in? If so, 
> are there any other particulars I should take into account in my patch?

--------------------

On 11/23/2021 3:59 PM, Gregory Heytings wrote:
>> you've instead implemented the behavior "for" me
>>
> 
> That's not what I did, I tried to implement a function that provides 
> different possible behaviors corresponding to different needs and 
> expectations, not just the behavior you expected.  And I did this based 
> on your feedback, what others said on emacs-devel, and my own feelings.  
> I hoped (and thought until today) that the behavior you expected was one 
> of them.

And that's fine. I have no issue supporting behaviors other than the one 
I personally want. (For example, I have no problem with the `empty' 
behavior, even though I wouldn't personally find it useful.) However, I 
also want to be sure that we don't provide *unnecessarily* many options 
here, since that adds to the maintenance burden and may produce 
unexpected behavior for users if they don't quite understand the 
distinction between each possible setting.

In this case, I think it's better to continue prompting users when 
killing a non-last client just like Emacs 28[2] does. There might be 
some value in avoiding that prompt, but I think a user who *doesn't* use 
`server-stop-automatically' is just as likely to want that behavior, so 
if we want to support that, we should provide a separate configuration 
option to do so.

In particular, I think it's valuable to prompt users in this case 
because it comes up when using emacsclient and committing code. If I set 
`EDITOR="emacsclient -c"' and run `git commit', then under Emacs 28, I 
can close the client with `C-x C-c'[3] and it will prompt me if I forgot 
to save the commit message. That's useful because without the prompt, it 
would (usually) just abort the commit due to an empty message.

I think that behavior is worth preserving. (Likewise, I think it's worth 
preserving the ability to keep all your open frames by pressing C-g if 
Emacs prompts you about saving files when you try to kill a client.) 
Thus, I implemented the patch that you can see in the original message.

--------------------

In addition, I'm not certain that we need both `delete-frame' (or 
`delete-last-frame' as it's called in your latest patch) and 
`kill-terminal'. The only difference between the two is that in the 
former, individually closing the last non-daemon frame (e.g. by `C-x 5 
0' or clicking the X on the frame's titlebar) also kills the daemon. I 
prefer the behavior of `delete-frame', since my mental model is that the 
daemon should be killed *whenever* the last non-daemon frame is closed, 
no matter *how* it's closed. Maybe someone has a strong opinion in the 
other direction and actively wants the `kill-terminal' behavior instead. 
I don't have a problem with keeping that option around if someone would 
actually use it, but I'm skeptical of that. In this case though, I'm 
happy to present my argument and then defer to the Emacs maintainers to 
make the final call, whatever that may be. If it stays, that's fine with 
me; if it goes, that's fine too.

--------------------

Since, as you mentioned before, we have different mental models for how 
this works, I'm mainly trying to explain through both prose and patches 
how this should work under my mental model. If I understand your prior 
messages correctly, your mental model is best encapsulated by the 
`empty' configuration. I'm 100% happy to keep that around as-is, even 
though it's not my preferred solution. However, my preferred solution is 
"something close to `delete-frame'".

Your patch in bug#51377 gets most of the way to what I wanted, but there 
are a couple of corner cases that don't work like I expect, hence this 
bug. Whether the behavior is a true "bug", a miscommunication, or 
something else doesn't matter much to me. All I'm looking for is 
something along the following lines (note: this isn't 100% rigorous, so 
don't take it as a precise specification):

1) When killing a non-last client (including by closing the last of its 
frames), the behavior should be exactly the same as the default 
emacsclient behavior. (That's what this bug is about.)

2) When killing the last client (including by closing the last of its 
frames), the behavior should be exactly the same as killing Emacs when 
*not* using a server. (Roughly speaking, that means calling 
`save-buffers-kill-emacs'. It would also be nice to have an option to 
silence the "This Emacs session has clients; exit anyway?" prompt, 
though I can certainly understand others wanting to keep it around in 
this case.)

Again, that may not be 100% precise, but it's the mental model I've used 
while implementing patches for this on my end. The specific 
implementation I use has shifted somewhat over time as I find corner 
cases, and also through consulting your patches. Using 
`delete-frame-functions' as in your code greatly simplified my 
implementation, for example. Thanks for that.

Hopefully my delays in following up on bug#51377 haven't been *too* 
disruptive. As mentioned, I was unfortunately too busy to devote 
sufficient time to it back then (my plan when posting to emacs-devel was 
just to collect feedback and to work on the implementation slowly over 
the next several weeks). Now my schedule is a bit less hectic, and I'm 
hoping to address the last few concerns I had after taking the time to 
test things out more thoroughly.

I'm certainly not trying to step on your toes or undo your patch. Just 
to account for certain corner cases that I didn't have a chance to 
investigate (or resolve) at the time.

[1] https://lists.gnu.org/archive/html/emacs-devel/2021-10/msg01465.html
[2] Or without `server-stop-automatically'
[3] You could also use `C-x #' here, but I find `C-x C-c' easier to 
type, so prefer the latter in cases where either would do what I want.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#51993: 29.0.50; [PATCH] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files
  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-29  5:39 ` Jim Porter
  2021-11-29 12:41   ` Eli Zaretskii
  1 sibling, 1 reply; 15+ messages in thread
From: Jim Porter @ 2021-11-29  5:39 UTC (permalink / raw)
  To: 51993; +Cc: larsi, gregory

To help move things along, I've drafted an initial version of a proposal 
describing possible behaviors of `server-stop-automatically' at a high 
level. Hopefully that will make it easier to discuss how each setting 
should work, and we can refine the proposal until we're happy with it. 
Then any implementation changes should "just" be a matter of following 
the final proposal.

I welcome any comments or suggested additions to this proposal. I 
covered things how I see them, but I may not have provided enough detail 
for everyone to understand all the nuances, and other people might have 
their own use cases they'd like to see addressed.

I can also post this proposal to emacs-devel if people think it would be 
better to get a wider range of feedback on it. Once we can get some 
level of consensus on the proposal, then hopefully we'll know the right 
thing to do about this bug.

--------------------

Automatically Shutting Down the Emacs Daemon
Revision 1

* Introduction

Since Emacs 23, `emacsclient' has had the ability to automatically start 
an Emacs daemon by passing `--alternate-editor=""' (or setting the 
ALTERNATE_EDITOR environment variable to the empty string). In Emacs 29, 
the ability to automatically shut down the daemon was added for 
symmetry. This allows users who prefer to start the Emacs daemon on an 
as-needed basis to configure the daemon to stop when finished. However, 
"finished" is a vague term and doesn't precisely describe the conditions 
that would cause the Emacs daemon to be shut down.

Note: for the discussion below, "clients" refers to both "true clients" 
(members of `server-clients') and "pseudo-clients" (each frame with the 
`client' parameter set to `nowait'). This is essentially the logic used 
by `server-save-buffers-kill-terminal' since Emacs 23.

* Proposed Behaviors

I propose two main ways of determining when the Emacs daemon should be 
shut down automatically. Each has different pros and cons, and so there 
are arguments for supporting one, both, or neither in Emacs going 
forward. However, I think each are distinct enough that supporting both 
would be reasonable.

** Implicit Shutdown

For this behavior, the Emacs daemon will be silently stopped once 
nothing would be lost by stopping it. In particular, this means that the 
Emacs daemon should continue so long as there's at least one a) active 
client, b) unsaved file, or c) running subprocess (with non-nil 
`process-query-on-exit-flag'). If none of these exist, then the Emacs 
daemon will be stopped.

One open question is how this should interact with options that control 
how `save-buffers-kill-emacs' prompts the user. If 
`confirm-kill-processes' is nil, should the Emacs daemon then be stopped 
even if there are active subprocesses?

*** Pros

A benefit of this behavior is that because the shutdown occurs 
implicitly, there's no change to how users interact with Emacs. For 
example, `save-buffers-kill-terminal' will prompt the user to save only 
those files passed to `emacsclient'.

*** Cons

However, the behavior's benefit has another side to it: it can be 
difficult for a user to predict whether the Emacs daemon will be 
stopped. For example, a stray unsaved file that the user forgot about 
could be the difference between the daemon continuing to run or being 
stopped.

Furthermore, since an Emacs daemon with no clients is out of sight, it 
may be some time before the user realizes their mistake, leading to 
confusion about why some other program appears to be reading an 
out-of-date version of the file. That said, this aspect is equally a 
concern when *not* automatically stopping the Emacs daemon at all.

** Explicit Shutdown

(Note: this is the behavior I personally prefer.) In this case, Emacs 
will be stopped as soon as there are no active clients, prompting the 
user to save their work as needed. In other words, when a client is 
about to be deleted for any reason (calling 
`server-save-buffers-kill-terminal', deleting the last frame associated 
with the client, etc), then:

- If this is not the last client, behave as in Emacs 28; that is, prompt 
to save any files passed to `emacsclient' before deleting the client.

- If this is the last client, prompt the user to save all their work 
before killing Emacs entirely; that is, call `save-buffers-kill-emacs'. 
In this case, the behavior should be identical (or as close as possible) 
to killing a "regular", non-server Emacs in a similar state.

One open question is: should Emacs warn the user that it will the daemon 
when deleting the last client? Currently it does so in 
`server-kill-emacs-query-function', asking, "This Emacs session has 
clients; exit anyway?" It may be useful to keep this warning, possibly 
rewording it in this case to explain the situation better. It's also 
possible that it's just extra noise and should be eliminate in this 
case. (It may even make sense to provide a separate configuration option.)

*** Pros

One of the main goals with Explicit Shutdown is to resolve a concern 
mentioned above: an Emacs daemon with no clients is out of sight, so 
it's easy to forget about it. If the daemon is holding onto unsaved 
work, it may take a while until the user realizes this. By prompting to 
save *all* the user's work before closing the last client, it's much 
harder to make this mistake.

*** Cons

This behavior makes it more difficult for a user to know ahead of time 
whether `save-buffers-kill-terminal' (bound by default to `C-x C-c') 
will kill the client or Emacs entirely. The prompt mentioned above in 
`server-kill-emacs-query-function' does let the user bail out though, if 
they didn't mean to kill Emacs entirely.

However, `save-buffers-kill-terminal' already has this complexity 
(almost), since it will kill Emacs entirely when executed from a 
non-client frame. That said, it's possible to tell what will happen in 
this case by looking at the mode line: if it starts with something like 
"U:@---", it's a client frame and thus an observant user will know what 
`save-buffers-kill-terminal' will do.

* Delayed Shutdown

It may also be worth considering whether the Emacs daemon should be 
stopped immediately when the conditions are met, or whether it would be 
better to delay it by a short time. Some operations, such as `git rebase 
-i', can open the $EDITOR, close it, and then reopen it in rapid 
succession. In a case like that, it would be more efficient if the Emacs 
daemon weren't stopped right away.

That said, I don't think this is a critical problem, and believe it 
would be ok to decide what to do about this later.

* Current Behaviors

Currently, Emacs 29 supports Implicit Shutdown (called `empty') and has 
two slightly-different variations on Explicit Shutdown (called 
`kill-terminal' and `delete-frame'). `empty' is implemented as a timer 
that periodically calls `server-stop-automatically--maybe-kill-emacs' to 
check if there's any need to keep the daemon running (non-daemon frames, 
unsaved files, or running processes); if not, it stops the daemon. This 
differs very slightly from the proposed spec above, since it checks for 
frames, not active clients. Some clients (e.g. `emacsclient --eval FOO') 
don't create any frames, so it may be useful to enhance the current 
implementation of Implicit Shutdown to account for this.

`kill-terminal' is implemented in 
`server-stop-automatically--handle-delete-frame' (called by 
`server-save-buffers-kill-terminal'). It first deletes all frames 
associated with the current client *except* the current one. Then, if 
there are any non-daemon frames aside from the current one, it just 
deletes that frame; if it's the last non-daemon frame, it calls 
`save-buffers-kill-emacs'.

`delete-frame' works like `kill-terminal' above, but will also call 
`save-buffers-kill-emacs' when closing the last non-daemon frame using 
other means, such as clicking the "X" in the frame's title bar on a GUI 
system.

As mentioned above, `kill-terminal' and `delete-frame' work similarly to 
the Explicit Shutdown behavior, but there are some differences. For 
example, when killing a non-last client, `kill-terminal' and 
`delete-frame' don't prompt to save files passed to `emacsclient'. When 
killing the last client, they delete all the non-current frames before 
calling `save-buffers-kill-emacs', meaning that pressing `C-g' to cancel 
when prompted will still result in all but one frame going away.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#51993: 29.0.50; [PATCH] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files
  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:12     ` Jim Porter
  0 siblings, 2 replies; 15+ messages in thread
From: Eli Zaretskii @ 2021-11-29 12:41 UTC (permalink / raw)
  To: Jim Porter; +Cc: larsi, 51993, gregory

> From: Jim Porter <jporterbugs@gmail.com>
> Cc: eliz@gnu.org, larsi@gnus.org, gregory@heytings.org
> Date: Sun, 28 Nov 2021 21:39:46 -0800
> 
> To help move things along, I've drafted an initial version of a proposal 
> describing possible behaviors of `server-stop-automatically' at a high 
> level. Hopefully that will make it easier to discuss how each setting 
> should work, and we can refine the proposal until we're happy with it. 
> Then any implementation changes should "just" be a matter of following 
> the final proposal.
> 
> I welcome any comments or suggested additions to this proposal. I 
> covered things how I see them, but I may not have provided enough detail 
> for everyone to understand all the nuances, and other people might have 
> their own use cases they'd like to see addressed.
> 
> I can also post this proposal to emacs-devel if people think it would be 
> better to get a wider range of feedback on it. Once we can get some 
> level of consensus on the proposal, then hopefully we'll know the right 
> thing to do about this bug.

Thanks.

However, I'm not sure I understand the way forward, as you see it.
AFAIU, there are fundamental disagreements between you and Gregory,
and I don't think the disagreements are there because one of you
doesn't understand the proposals of the other.

So how could these disagreements be reconciled?  Either you-two come
up with some compromise that is acceptable by both of you, or Lars and
myself make the decision for you (and we don't guarantee you will like
it), or things are left as they are now.  What will it be?





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#51993: 29.0.50; [PATCH] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files
  2021-11-29 12:41   ` Eli Zaretskii
@ 2021-11-29 13:40     ` Gregory Heytings
  2021-11-29 19:31       ` Jim Porter
  2021-11-29 19:12     ` Jim Porter
  1 sibling, 1 reply; 15+ messages in thread
From: Gregory Heytings @ 2021-11-29 13:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Jim Porter, larsi, 51993


>
> However, I'm not sure I understand the way forward, as you see it. 
> AFAIU, there are fundamental disagreements between you and Gregory, and 
> I don't think the disagreements are there because one of you doesn't 
> understand the proposals of the other.
>
> So how could these disagreements be reconciled?
>

I'm in the process of implementing something that should satisfy everyone, 
but it's not finished yet.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#51993: 29.0.50; [PATCH] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files
  2021-11-29 12:41   ` Eli Zaretskii
  2021-11-29 13:40     ` Gregory Heytings
@ 2021-11-29 19:12     ` Jim Porter
  1 sibling, 0 replies; 15+ messages in thread
From: Jim Porter @ 2021-11-29 19:12 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, 51993, gregory

On 11/29/2021 4:41 AM, Eli Zaretskii wrote:
> However, I'm not sure I understand the way forward, as you see it.
> AFAIU, there are fundamental disagreements between you and Gregory,
> and I don't think the disagreements are there because one of you
> doesn't understand the proposals of the other.

I think there may be some of both. Since, from Gregory's perspective 
I've been changing what I want, and from my perspective I've always 
wanted the same thing, it seems that there's at least some 
miscommunication happening. Hopefully my proposal helps to explain my 
position more clearly (and in a single message, rather than scattered 
across several).

There may still be some disagreements, and I'm happy to incorporate 
anything I've missed into the proposal if people would find that useful. 
On the other hand, if it's easier to just work on a patch that everyone 
would be happy with (or at least be able to tolerate), that's ok too.

> So how could these disagreements be reconciled?  Either you-two come
> up with some compromise that is acceptable by both of you, or Lars and
> myself make the decision for you (and we don't guarantee you will like
> it), or things are left as they are now.  What will it be?

Hopefully Gregory and I can reach a compromise; the Implicit Shutdown 
case in my proposal is an initial attempt at incorporating Gregory's 
desired behavior (it was the behavior Gregory first proposed in the 
thread on emacs-devel). I'm primarily interested in the Explicit 
Shutdown case though; I may have missed some things in that section as 
well, but like I said, I'm happy to add more to it if anyone has things 
they'd like to be added.

That said, no matter if Gregory and I can reach a compromise, I think it 
would be helpful if you and/or Lars could take a look if you have time 
to be sure the behavior makes sense to you and that it's something you'd 
want to be in Emacs. Since this might have relevance for other Emacs 
features (e.g. emacsclient.desktop[1]), I think it would be good to have 
as many experienced eyes on this as possible.

In the end, so long as everyone is clear what the behavior should be, 
and that it's documented as such so there's no confusion about what's a 
bug and what's intended behavior, then I don't have any (major) 
problems. Obviously, I'd be happy if my preferred behavior were a part 
of Emacs, but if you and Lars don't think it should be, then I can just 
continue what I've been doing: add advice to the necessary server.el 
functions in my config so things work how I like.

[1] https://lists.gnu.org/archive/html/emacs-devel/2021-10/msg01846.html





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#51993: 29.0.50; [PATCH] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files
  2021-11-29 13:40     ` Gregory Heytings
@ 2021-11-29 19:31       ` Jim Porter
  0 siblings, 0 replies; 15+ messages in thread
From: Jim Porter @ 2021-11-29 19:31 UTC (permalink / raw)
  To: Gregory Heytings, Eli Zaretskii; +Cc: larsi, 51993

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

On 11/29/2021 5:40 AM, Gregory Heytings wrote:
> 
> I'm in the process of implementing something that should satisfy 
> everyone, but it's not finished yet.

Thanks. Hopefully my proposal helps to explain more precisely my mental 
model and how I'd like things to work. If there's anything that's not 
clear, just let me know and I'll try to elaborate on it.

In case it would help, I've also attached the relevant bits of my Emacs 
configuration, which implements the Explicit Shutdown behavior I've 
described. Overall, it's similar to the first patch I posted in this 
bug, but it's usable in Emacs 27+ (maybe even earlier).

However, I think it has (at least) one problem: if `server-buffer-done' 
(indirectly called by `server-edit' / `C-x #') deletes the last client, 
it doesn't stop the daemon. I haven't had a chance to look into this in 
detail yet though. I think the most consistent behavior would be to stop 
the daemon in this case, since then the logic is simply "stop the daemon 
when the last client would be deleted", and we don't have to worry about 
exceptions to that rule.

[-- Attachment #2: server-advice.el --]
[-- Type: text/plain, Size: 3840 bytes --]

(use-package server
  :defer t
  :preface
  (defun user--server-save-buffers-kill-terminal (arg)
    "Offer to save each buffer, then kill the current client.
With ARG non-nil, silently save all file-visiting buffers, then kill.

If emacsclient was started with a list of filenames to edit, then
only these files will be asked to be saved.

If Emacs was started as a daemon and this is the last client
connected to it, this will call `save-buffers-kill-emacs'."
    (let ((proc (frame-parameter nil 'client)))
      (cond ((eq proc 'nowait)
	     ;; Nowait frames have no client buffer list.
	     (if (if (daemonp) (cddr (frame-list)) (cdr (frame-list)))
                 ;; If there's another (non-daemon) frame, only delete this
                 ;; frame. FIXME: It would be nice to delete any other frames
                 ;; created by this frame (as when killing the terminal of an
                 ;; ordinary client below), but we can't distinguish separate
                 ;; groups of nowait frames currently.
	         (progn (save-some-buffers arg)
		        (delete-frame))
	       ;; If we're the last frame standing, kill Emacs.
	       (save-buffers-kill-emacs arg)))
	    ((processp proc)
             (if (seq-some
                  (lambda (frame)
                    (let ((p (frame-parameter frame 'client)))
                      (unless (and (daemonp) (null p))
                        (not (equal proc p)))))
                  (frame-list))
                 ;; If there's a frame not from this client, only delete the
                 ;; client.
	         (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 all frames are from this client, kill Emacs.
               (save-buffers-kill-emacs arg)))
	    (t (error "Invalid client frame")))))

  (defun user--server-kill-emacs-query-function ()
    "Ask before exiting Emacs if it has live clients.
If Emacs was started as a daemon and the only live client is the
current frame's client, don't bother asking."
    (let ((ignored-proc (and (daemonp) (frame-parameter nil 'client))))
      (or (not (seq-some (lambda (proc)
                           (unless (eq ignored-proc proc)
                             (seq-some #'buffer-live-p
                                       (process-get proc 'buffers))))
                         server-clients))
          (yes-or-no-p "This Emacs session has clients; exit anyway? "))))

  (defun user--handle-delete-frame (frame)
    "When deleting the last non-daemon frame, kill Emacs.
\(To be used from `delete-frame-functions'.)"
    (when (and (daemonp)
               ;; Check that the frame is a client frame.
               ;; Note: `server-delete-client' sets `client' to nil before
               ;; calling `delete-frame', but that's good, since we want to call
               ;; `save-buffers-kill-emacs' before all that anyway.
               (frame-parameter frame 'client)
               (null (cddr (frame-list))))
      (save-buffers-kill-emacs)))

  :config
  (advice-add #'server-save-buffers-kill-terminal :override
              #'user--server-save-buffers-kill-terminal)
  (advice-add #'server-kill-emacs-query-function :override
              #'user--server-kill-emacs-query-function)
  (add-hook 'delete-frame-functions #'user--handle-delete-frame))

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-11-29 19:31 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2021-11-29 19:12     ` Jim Porter

Code repositories for project(s) associated with this 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).