bug#58404: 29.0.50; [PATCH] When killing Emacs from the last client, don't warn about the session having clients

bug#58404: 29.0.50; [PATCH] When killing Emacs from the last client, don't warn about the session having clients

[Jim Porter]

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

To reproduce:

   $ emacs -Q --daemon
   $ emacsclient foo.txt

   M-x save-buffers-kill-emacs
   => This Emacs session has clients; exit anyway?

I think that's unnecessary. Since we're in the last (only) client, we 
can't accidentally kill other clients that we don't see right now (e.g. 
ones in an SSH connection); they don't exist! Couldn't we just proceed 
ahead without the prompt?

On the other hand, I think it *would* be useful to prompt if you're in 
the last client, but there are other non-client frames. This can happen 
if you start the main Emacs process without --daemon or if you use 
--no-wait. For example:

   $ emacs -Q --daemon
   $ emacsclient foo.txt
   $ emacsclient --no-wait -c bar.txt

   ;; From the first client:
   M-x save-buffers-kill-emacs
   => This Emacs session has clients; exit anyway?

This is ok, except the prompt could be clearer. The real issue is that 
the session has non-client frames that would get killed.

Attached is a patch to do this.

[-- Attachment #2: 0001-Don-t-prompt-when-killing-an-Emacs-client-if-it-s-th.patch --]
[-- Type: text/plain, Size: 2215 bytes --]

From 2fb6c6da99b84c250e59409d4dc0adbdbe704fed Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 9 Oct 2022 15:53:27 -0700
Subject: [PATCH] Don't prompt when killing an Emacs client if it's the last
 client

* lisp/server.el (server-kill-emacs-query-function): Ignore the
current client, if any.  Prompt if there are non-client frames.
---
 lisp/server.el | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/lisp/server.el b/lisp/server.el
index 3caa335c4e..9ebdabbf87 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1589,14 +1589,28 @@ server-done
     (server-buffer-done (current-buffer))))
 
 (defun server-kill-emacs-query-function ()
-  "Ask before exiting Emacs if it has live clients.
+  "Ask before exiting Emacs if it has other live clients or non-client frames.
 A \"live client\" is a client with at least one live buffer
 associated with it."
-  (or (not (seq-some (lambda (proc)
-                       (seq-some #'buffer-live-p
-                                 (process-get proc 'buffers)))
-                     server-clients))
-      (yes-or-no-p "This Emacs session has clients; exit anyway? ")))
+  (let ((this-client (frame-parameter nil 'client)))
+    (if (seq-some (lambda (proc)
+                    ;; Ignore the current client when checking for
+                    ;; live clients.
+                    (unless (eq proc this-client)
+                      (seq-some #'buffer-live-p
+                                (process-get proc 'buffers))))
+                  server-clients)
+        (yes-or-no-p "This Emacs session has other clients; exit anyway? ")
+      (or (not (processp this-client))
+          ;; Check if there are any non-client frames, ignoring the
+          ;; initial frame when in daemon mode.
+          (>= (if (daemonp) 1 0)
+              (seq-count
+               (lambda (frame)
+                 (not (processp (frame-parameter frame 'client))))
+               (frame-list)))
+          (yes-or-no-p
+           "This Emacs session has non-client frames; exit anyway? ")))))
 
 (defun server-kill-buffer ()
   "Remove the current buffer from its clients' buffer list.
-- 
2.25.1

bug#58404: 29.0.50; [PATCH] When killing Emacs from the last client, don't warn about the session having clients

[Eli Zaretskii]

> Date: Sun, 9 Oct 2022 16:32:28 -0700
> From: Jim Porter <jporterbugs@gmail.com>
> 
> On the other hand, I think it *would* be useful to prompt if you're in 
> the last client, but there are other non-client frames. This can happen 
> if you start the main Emacs process without --daemon or if you use 
> --no-wait. For example:
> 
>    $ emacs -Q --daemon
>    $ emacsclient foo.txt
>    $ emacsclient --no-wait -c bar.txt
> 
>    ;; From the first client:
>    M-x save-buffers-kill-emacs
>    => This Emacs session has clients; exit anyway?
> 
> This is ok, except the prompt could be clearer. The real issue is that 
> the session has non-client frames that would get killed.
> 
> Attached is a patch to do this.

IMO, this is an unnecessary annoyance.  We don't by default ask the
user any questions today, when they want to kill an Emacs session.
Why should we change that in this case?  What is the use case where
this command could be invoked by mistake and will risk losing edits or
other valuable work?

bug#58404: 29.0.50; [PATCH] When killing Emacs from the last client, don't warn about the session having clients

[Jim Porter]

On 10/9/2022 11:11 PM, Eli Zaretskii wrote:
> IMO, this is an unnecessary annoyance.  We don't by default ask the 
> user any questions today, when they want to kill an Emacs session.

But Emacs *does* prompt the user today. If you call 
'save-buffers-kill-emacs' from an emacsclient frame, it will (almost) 
always ask you, "This Emacs session has clients; exit anyway?" My patch 
just gets rid of that annoyance when we can be sure it's unnecessary 
(i.e. it won't ask if there are no other emacsclients and no other 
frames not owned by a client).

The patch does also change the prompt to be more-precise when there is 
an open frame not owned by a client[1]. However, it doesn't add any new 
prompts where there were none before; it only removes a prompt in one case.

> What is the use case where this command could be invoked by mistake
> and will risk losing edits or other valuable work?

I didn't write this code initially, but here's my understanding of why 
the prompt is useful.

If you use "emacs --daemon", 'C-x C-c' ('save-buffers-kill-terminal') 
will only close the current emacsclient, not shut down the daemon 
entirely. If you'd like to shut down the daemon too, you can call 
'save-buffers-kill-emacs' instead. Maybe you'd even bind that to a key 
combo or do it automatically in some cases.

So given the above, imagine you're at the office. You start the Emacs 
daemon and connect a GUI emacsclient instance[2]. Then you do a bunch of 
work until it's time to go home, but you leave your emacsclient running 
to pick up where you left off the next day.

When you get home, you remember that you wanted to edit something 
quickly, so you SSH into you work computer and start a second 
emacsclient instance. By now, you forgot about your first emacsclient 
instance you made while at work. If you call 'save-buffers-kill-emacs' 
from your SSH emacsclient, it's nice that it can remind you that you 
have another emacsclient instance still running: since you don't see the 
GUI frames for the other client, it would be easy to forget about them 
and kill the session entirely. Then when you go into work the next day, 
the buffers you had open yesterday would be gone.

In this case, you likely haven't lost changes to any files, since you 
saved before killing the session, but you'd lose non-file buffers, 
window configuration, etc. (Of course, this assumes you're not using 
desktop.el.)

As for the code I added to check for non-client frames, the story is 
pretty much the same, although that code is there to handle situations 
where users start Emacs normally (i.e. by running "emacs") and then 
start the Emacs server via '(server-start)'. When connecting to that 
Emacs session remotely, it would be useful to get a prompt for the same 
reasons above. The only difference is that the GUI frames on your work 
system are "non-client" frames, so the code checks for them in a 
different way.

[1] Excluding the initial frame for "emacs --daemon", that is.

[2] This could be as simple as clicking the "Emacs (client)" icon on 
your desktop.

bug#58404: 29.0.50; [PATCH] When killing Emacs from the last client, don't warn about the session having clients

[Eli Zaretskii]

> Date: Mon, 10 Oct 2022 01:08:17 -0700
> Cc: 58404@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> 
> On 10/9/2022 11:11 PM, Eli Zaretskii wrote:
> > IMO, this is an unnecessary annoyance.  We don't by default ask the 
> > user any questions today, when they want to kill an Emacs session.
> 
> But Emacs *does* prompt the user today. If you call 
> 'save-buffers-kill-emacs' from an emacsclient frame, it will (almost) 
> always ask you, "This Emacs session has clients; exit anyway?" My patch 
> just gets rid of that annoyance when we can be sure it's unnecessary 
> (i.e. it won't ask if there are no other emacsclients and no other 
> frames not owned by a client).

It is okay not to prompt when there's no need.  That wasn't the part
on which I commented, AFAIU.

You said:

> On the other hand, I think it *would* be useful to prompt if you're in 
> the last client, but there are other non-client frames.

I'm saying that no, it will not be useful to prompt in that case: if
the only frames left are non-client frames, we should not prompt,
because we don't prompt when there are no client frames to begin with.
Apologies if I misunderstood the use case.

bug#58404: 29.0.50; [PATCH] When killing Emacs from the last client, don't warn about the session having clients

[Jim Porter]

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

On 10/10/2022 1:53 AM, Eli Zaretskii wrote:
> It is okay not to prompt when there's no need.  That wasn't the part
> on which I commented, AFAIU.
> 
> You said:
> 
>> On the other hand, I think it *would* be useful to prompt if you're in
>> the last client, but there are other non-client frames.
> 
> I'm saying that no, it will not be useful to prompt in that case: if
> the only frames left are non-client frames, we should not prompt,
> because we don't prompt when there are no client frames to begin with.
> Apologies if I misunderstood the use case.

Ah, I see. In that part of my original message, I wanted to say that 
this was a case where I thought the old code made sense, since it 
prompts the user then. However, I also thought that the prompt message 
could be more informative.

Here's why I think prompting then makes sense: when you're in an 
emacsclient frame and there are other non-client frames (i.e. ones 
"owned" by the main Emacs process), that looks very similar to the user 
as when you have a second emacsclient running. In other words, I think 
it would be good for this function to prompt when doing this:

   $ emacs -Q --daemon
   $ emacsclient foo.txt
   $ emacsclient -c bar.txt

   ;; From either frame:
   M-x save-buffers-kill-emacs

Or this:

   $ emacs -Q --daemon
   $ emacsclient foo.txt
   $ emacsclient --no-wait -c bar.txt

   ;; From either frame:
   M-x save-buffers-kill-emacs

Or this:

   $ emacs foo.txt
   M-x server-start
   $ emacsclient -c bar.txt

   ;; From either frame:
   M-x save-buffers-kill-emacs

In all of those cases, the user would have 2 (visible) frames, possibly 
on different displays/terminals. Since they're visually similar, I think 
it makes sense to have similar prompts.

Next, if the user ran 'save-buffers-kill-emacs' a second time in any of 
those examples, it *wouldn't* prompt, since then all the remaining 
frames are associated with a single emacsclient (or no client at all).

Since the use case for this is non-obvious (it actually took me several 
iterations to identify all the possible cases), I updated my patch with 
clearer (to me) code, plus comments explaining the reasoning.

[-- Attachment #2: 0001-Don-t-prompt-when-killing-an-Emacs-client-if-it-s-th.patch --]
[-- Type: text/plain, Size: 2846 bytes --]

From 3f08b120a83b3c06a777823acee604f94fd41210 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 9 Oct 2022 15:53:27 -0700
Subject: [PATCH] Don't prompt when killing an Emacs client if it's the last
 client

Previously, this function prompted whenever there was a live client,
including the current one.  Now, only prompt in one of:

  1. There are live Emacs clients other than the current one, or
  2. The current frame is a client frame and there are also non-client
     frames open.

* lisp/server.el (server-kill-emacs-query-function): Rewrite to avoid
prompting as above (bug#58404).
---
 lisp/server.el | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/lisp/server.el b/lisp/server.el
index 3caa335c4e..f74f26210a 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1589,14 +1589,34 @@ server-done
     (server-buffer-done (current-buffer))))
 
 (defun server-kill-emacs-query-function ()
-  "Ask before exiting Emacs if it has live clients.
+  "Ask before exiting Emacs if it has other live clients or non-client frames.
 A \"live client\" is a client with at least one live buffer
 associated with it."
-  (or (not (seq-some (lambda (proc)
-                       (seq-some #'buffer-live-p
-                                 (process-get proc 'buffers)))
-                     server-clients))
-      (yes-or-no-p "This Emacs session has clients; exit anyway? ")))
+  (let ((this-client (frame-parameter nil 'client)))
+    (cond
+     ;; Check for Emacs clients, excluding the current one (if any).
+     ;; If there are any other clients, prompt the user before killing
+     ;; Emacs.  Other clients might be on different (possibly remote)
+     ;; displays, so it can be hard for users to tell on their own.
+     ((seq-some (lambda (proc)
+                  (unless (eq proc this-client)
+                    (seq-some #'buffer-live-p
+                              (process-get proc 'buffers))))
+                server-clients)
+      (yes-or-no-p "This Emacs session has other clients; exit anyway? "))
+     ;; If the user is in a client frame, prompt if there are any
+     ;; frames created by the main Emacs process.  Like in the case
+     ;; above, those frames might be on different displays.
+     ((and (processp this-client)
+           ;; Don't count the initial frame when in daemon mode.
+           (< (if (daemonp) 1 0)
+              (seq-count
+               (lambda (frame)
+                 (not (processp (frame-parameter frame 'client))))
+               (frame-list))))
+      (yes-or-no-p "This Emacs session has non-client frames; exit anyway? "))
+     ;; Otherwise, we can kill Emacs without prompting.
+     (t t))))
 
 (defun server-kill-buffer ()
   "Remove the current buffer from its clients' buffer list.
-- 
2.25.1

bug#58404: 29.0.50; [PATCH] When killing Emacs from the last client, don't warn about the session having clients

[Eli Zaretskii]

> Date: Mon, 10 Oct 2022 09:43:55 -0700
> Cc: 58404@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> 
> Here's why I think prompting then makes sense: when you're in an 
> emacsclient frame and there are other non-client frames (i.e. ones 
> "owned" by the main Emacs process), that looks very similar to the user 
> as when you have a second emacsclient running.

No, there's a very fundamental difference between the two.  When there
are client frames showing buffers, for each client buffer there's a
process waiting, the process which requested the buffer to be edited.
That's why we prompt: we don't want to fail those waiting processes.

Non-client frames don't have this problem.

bug#58404: 29.0.50; [PATCH] When killing Emacs from the last client, don't warn about the session having clients

[Jim Porter]

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

On 10/10/2022 9:59 AM, Eli Zaretskii wrote:
>> Date: Mon, 10 Oct 2022 09:43:55 -0700
>> Cc: 58404@debbugs.gnu.org
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>> Here's why I think prompting then makes sense: when you're in an
>> emacsclient frame and there are other non-client frames (i.e. ones
>> "owned" by the main Emacs process), that looks very similar to the user
>> as when you have a second emacsclient running.
> 
> No, there's a very fundamental difference between the two.  When there
> are client frames showing buffers, for each client buffer there's a
> process waiting, the process which requested the buffer to be edited.
> That's why we prompt: we don't want to fail those waiting processes.
> 
> Non-client frames don't have this problem.

Ok, I think that makes sense. I was hesitant about removing prompts too 
aggressively, since I didn't want to open users up to losing some Emacs 
state without prompting when they would have gotten a prompt before. 
However, you've convinced me that we don't need to worry about 
non-client frames since they don't have processes waiting on them. 
(Maybe some users would want more prompts in case they accidentally kill 
Emacs, but they can always add to 'kill-emacs-query-functions'.)

Attached is a new patch that removes the prompt when called from the 
last remaining client. I also expanded the docstring to explain why the 
prompt is there.

[-- Attachment #2: 0001-Don-t-prompt-when-killing-an-Emacs-client-if-it-s-th.patch --]
[-- Type: text/plain, Size: 1910 bytes --]

From ea95b409e89c1d87016a36a92070a592b8b2e4d0 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 9 Oct 2022 15:53:27 -0700
Subject: [PATCH] Don't prompt when killing an Emacs client if it's the last
 client

* lisp/server.el (server-kill-emacs-query-function): Ignore the
current client (if any) when checking for live clients (bug#58404).
---
 lisp/server.el | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/lisp/server.el b/lisp/server.el
index 3caa335c4e..15118067e5 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1589,14 +1589,19 @@ server-done
     (server-buffer-done (current-buffer))))
 
 (defun server-kill-emacs-query-function ()
-  "Ask before exiting Emacs if it has live clients.
+  "Ask before exiting Emacs if it has other live clients.
 A \"live client\" is a client with at least one live buffer
-associated with it."
-  (or (not (seq-some (lambda (proc)
-                       (seq-some #'buffer-live-p
-                                 (process-get proc 'buffers)))
-                     server-clients))
-      (yes-or-no-p "This Emacs session has clients; exit anyway? ")))
+associated with it.  These clients were (probably) started by
+external processes that are waiting for some buffers to be
+edited.  If there are any other clients, we don't want to fail
+their waiting processes, so ask the user to be sure."
+  (let ((this-client (frame-parameter nil 'client)))
+    (or (not (seq-some (lambda (proc)
+                         (unless (eq proc this-client)
+                           (seq-some #'buffer-live-p
+                                     (process-get proc 'buffers))))
+                       server-clients))
+          (yes-or-no-p "This Emacs session has other clients; exit anyway? "))))
 
 (defun server-kill-buffer ()
   "Remove the current buffer from its clients' buffer list.
-- 
2.25.1

bug#58404: 29.0.50; [PATCH] When killing Emacs from the last client, don't warn about the session having clients

[Eli Zaretskii]

> Date: Mon, 10 Oct 2022 10:49:05 -0700
> Cc: 58404@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> 
> Attached is a new patch that removes the prompt when called from the 
> last remaining client. I also expanded the docstring to explain why the 
> prompt is there.

LGTM, thanks.

bug#58404: 29.0.50; [PATCH] When killing Emacs from the last client, don't warn about the session having clients

[Jim Porter]

On 10/10/2022 10:57 AM, Eli Zaretskii wrote:
>> Date: Mon, 10 Oct 2022 10:49:05 -0700
>> Cc: 58404@debbugs.gnu.org
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>> Attached is a new patch that removes the prompt when called from the
>> last remaining client. I also expanded the docstring to explain why the
>> prompt is there.
> 
> LGTM, thanks.

Thanks, merged as ebc19f56aaeb98b834eea1ce8768ca13bed8578c. Closing this 
bug.