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

[Jim Porter <jporterbugs@gmail.com>]

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

On 11/2/2022 11:42 AM, Eli Zaretskii wrote:
>> Date: Wed, 2 Nov 2022 11:17:00 -0700
>> Cc: 58909@debbugs.gnu.org
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>>> If that's because you want to support the C-g case, then don't: that
>>> is a separate problem.
>>
>> Yeah, this subthread about 'delete-frame' was just to support the C-g
>> case. Aside from that, I think doing this inside 'server-delete-client'
>> would be fine. However, at least for this bug, adding a hook that runs
>> in 'server-delete-client' doesn't add anything that
>> 'delete-frame-functions' doesn't already allow. ('server-delete-client'
>> is called from a hook in 'delete-frame-functions' in this case anyway.)
> 
> We cannot have behavior specific to server.el coded outside of
> server.el.  So I don't think I understand what you are saying here.
> My suggestion is not to add a hook, it is to add a prompt for the user
> when the last client frame is deleted, and do it optionally.

I just meant that the existing 'delete-frame-functions' hook lets me do 
what I want by writing a hook in my local configuration (except for the 
C-g issue). I agree that for any changes to Emacs, the changes should be 
in server.el as much as possible.

>> If you mean adding a defcustom, I thought we'd agreed not to do that
>> (see the beginning of your message here[1]).
> 
> Yes, but you keep pushing...

Sorry if I seem pushy. I really am open to different solutions here, and 
I just want to present some various options I see to make it 
possible/easy to adjust the behavior of server.el. My goal is really 
just to offer what improvements I can to Emacs; if they don't belong, 
that's ok. I can just use them locally.

In short, consider my messages/patches as just an offer to help.

> I'm not sure I understand the purpose of this minor mode, and we
> ourselves start the server in the daemon mode by a direct call to
> server-start.  Also, server-start is a command, and many users (myself
> included) invoke it interactively.
> 
> Why is it important to have the mode turned on?  What can a mode do
> that we cannot do without a mode?

If starting the server activates a minor mode, then Emacs can use a 
minor-mode keymap for server-mode. That would make it easy for users to 
add keybindings that only take effect when the server is running. In my 
case, I could locally remap 'delete-frame' to some new function when the 
server is running, and then I could make that function do exactly what I 
want. Other users might want to add other keybindings to this keymap 
(maybe one for 'server-edit-abort').

In fact, for my purposes, even just having daemon mode call 
'(server-mode 1)' instead of '(server-start)' would be a good improvement.

There are other ways to do this, but having a minor mode makes it 
easier. I attached a quick patch that shows the changes I'd make so that 
users could have a 'sever-mode-keymap' to add to; if you think any of 
these changes are too risky, I don't mind removing them. If you think 
*all* of them are too risky, that's ok too. I can just do something 
equivalent in my local config.

[-- Attachment #2: 0001-WIP-Try-using-a-keymap-for-server-mode.patch --]
[-- Type: text/plain, Size: 2116 bytes --]

From 0e638e4b1481e60b6d8211b15d55d3fde6291b31 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Wed, 2 Nov 2022 09:22:43 -0700
Subject: [PATCH] [WIP] Try using a keymap for server-mode

---
 lisp/frame.el   | 6 +++++-
 lisp/server.el  | 5 +++++
 lisp/startup.el | 2 +-
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/lisp/frame.el b/lisp/frame.el
index 400f8a44ee..f1888623e4 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -128,7 +128,11 @@ handle-delete-frame
                        (not (frame-parent frame-1))
                        (not (frame-parameter frame-1 'delete-before)))
               (throw 'other-frame t))))
-	(delete-frame frame t)
+        ;; NOTE: This change is optional.  If you think it adds too
+        ;; much complexity, let's remove it.
+        (funcall (or (command-remapping #'delete-frame)
+                     #'delete-frame)
+                 t)
       ;; Gildea@x.org says it is ok to ask questions before terminating.
       (save-buffers-kill-emacs))))
 
diff --git a/lisp/server.el b/lisp/server.el
index 90d97c1538..aec6677f1d 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -796,6 +796,10 @@ server-running-p
 	t)
     (file-error nil)))
 
+;; NOTE: This change is optional, but it makes it easier for users to
+;; add keybindings for `server-mode'.
+(defvar-keymap server-mode-keymap)
+
 ;;;###autoload
 (define-minor-mode server-mode
   "Toggle Server mode.
@@ -805,6 +809,7 @@ server-mode
 `server-start' for details."
   :global t
   :version "22.1"
+  :keymap server-mode-keymap
   ;; Fixme: Should this check for an existing server socket and do
   ;; nothing if there is one (for multiple Emacs sessions)?
   (server-start (not server-mode)))
diff --git a/lisp/startup.el b/lisp/startup.el
index 70267fc857..a494e42d25 100644
--- a/lisp/startup.el
+++ b/lisp/startup.el
@@ -1604,7 +1604,7 @@ command-line
   (let ((dn (daemonp)))
     (when dn
       (when (stringp dn) (setq server-name dn))
-      (server-start)
+      (server-mode 1)
       (if server-process
 	  (daemon-initialized)
 	(if (stringp dn)
-- 
2.25.1