bug#59668: 29.0.50; [PATCH] Make 'server-stop-automatically' into a defcustom

[Jim Porter <jporterbugs@gmail.com>]

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

On 12/2/2022 6:42 AM, Eli Zaretskii wrote:
>> Date: Thu, 1 Dec 2022 19:30:03 -0800
>> Cc: 59668@debbugs.gnu.org
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>> @@ -1805,7 +1814,7 @@ server-save-buffers-kill-terminal
>>   	    (t (error "Invalid client frame"))))))
>>   
>>   (defun server-stop-automatically--handle-delete-frame (frame)
>> -  "Handle deletion of FRAME when `server-stop-automatically' is used."
>> +  "Handle deletion of FRAME when `server-stop-automatically' is `delete-frame'."
>>     (when server-stop-automatically
>>       (if (if (and (processp (frame-parameter frame 'client))
>>   		 (eq this-command 'save-buffers-kill-terminal))
>> @@ -1828,7 +1837,7 @@ server-stop-automatically--handle-delete-frame
>>   	  (delete-frame frame)))))
> 
> AFAIU, this delete-frame is called after save-buffers-kill-emacs, which is
> strange: there will be no Emacs to perform this call after that.  What am I
> missing?

Thanks. I think that's just an oversight. I removed it in my patch for 
bug#51993, since this seems more closely-related to that bug.

>> +          (const :tag "When empty" empty)
> 
> "When empty" doesn't explain itself well enough.  Can we come up with a
> better description?

How about "When empty (no clients, unsaved files, or processes)"? That 
seems a bit clumsy to me, but I wanted to keep the word "empty" in there 
somehow, since that's the name of the option value.

Attached is an updated patch (rebased on top of my previous patches for 
bug#51993).

[-- Attachment #2: 0001-Make-server-stop-automatically-into-a-defcustom.patch --]
[-- Type: text/plain, Size: 11341 bytes --]

From 7d9c9bbccf8fdf6851ac3cf5425c564d91919438 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 27 Nov 2022 22:21:10 -0800
Subject: [PATCH] Make 'server-stop-automatically' into a defcustom

This changes the meaning of the (formerly internal) variable
'server-stop-automatically': it now always holds the requested
configuration, even when Emacs was not started as a daemon
(bug#59668).

* lisp/server.el (server-stop-automatically): Convert the variable to
a defcustom, and make the function simply set the defcustom.
(server-stop-automatically--timer): New variable.
(server-apply-stop-automatically): New function...
(server-stop, server-start): ... call it.
(server-save-buffers-kill-terminal): Adjust the conditions for
stopping automatically to account for the change of meaning for
'server-stop-automatically'.
(server-stop-automatically--handle-delete-frame): Remove unnecessary
test of the 'server-stop-automatically' option; this hook is only set
when it should do its job.
(server-stop-automatically--maybe-kill-emacs): Update docstring.

* doc/emacs/misc.texi (Emacs Server): Update documentation.
---
 doc/emacs/misc.texi |  41 ++++++++--------
 lisp/server.el      | 111 +++++++++++++++++++++++++++++---------------
 2 files changed, 93 insertions(+), 59 deletions(-)

diff --git a/doc/emacs/misc.texi b/doc/emacs/misc.texi
index 29c0bed19c0..702c72bac25 100644
--- a/doc/emacs/misc.texi
+++ b/doc/emacs/misc.texi
@@ -1808,31 +1808,28 @@ Emacs Server
   emacs --daemon=foo
 @end example
 
-@findex server-stop-automatically
+@vindex server-stop-automatically
   The Emacs server can optionally be stopped automatically when
-certain conditions are met.  To do this, call the function
-@code{server-stop-automatically} in your init file (@pxref{Init
-File}), with one of the following arguments:
+certain conditions are met.  To do this, set the option
+@code{server-stop-automatically} to one of the following values:
 
-@itemize
-@item
-With the argument @code{empty}, the server is stopped when it has no
-clients, no unsaved file-visiting buffers and no running processes
-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.
-
-@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
+@table @code
+@item empty
+This value causes the server to be stopped when it has no clients, no
+unsaved file-visiting buffers and no running processes anymore.
+
+@item delete-frame
+This value means that 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.
-@end itemize
+
+@item kill-terminal
+This value means that 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.
+@end table
 
 @findex server-eval-at
   If you have defined a server by a unique server name, it is possible
diff --git a/lisp/server.el b/lisp/server.el
index aa62b25a891..97265c1a68c 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -273,6 +273,11 @@ server-client-instructions
   :version "28.1"
   :type 'boolean)
 
+(defvar server-stop-automatically)      ; Defined below to avoid recursive load.
+
+(defvar server-stop-automatically--timer nil
+  "The timer object for `server-stop-automatically--maybe-kill-emacs'.")
+
 ;; We do not use `temporary-file-directory' here, because emacsclient
 ;; does not read the init file.
 (defvar server-socket-dir
@@ -636,7 +641,8 @@ server-stop
       (setq stopped-p t
             server-process nil
             server-mode nil
-            global-minor-modes (delq 'server-mode global-minor-modes)))
+            global-minor-modes (delq 'server-mode global-minor-modes))
+      (server-apply-stop-automatically))
     (unwind-protect
         ;; Delete the socket files made by previous server
         ;; invocations.
@@ -757,6 +763,7 @@ server-start
 			 (list :family 'local
 			       :service server-file
 			       :plist '(:authenticated t)))))
+          (server-apply-stop-automatically)
 	  (unless server-process (error "Could not start server process"))
           (server-log "Started server")
 	  (process-put server-process :server-file server-file)
@@ -1769,9 +1776,6 @@ server-switch-buffer
     (when server-raise-frame
       (select-frame-set-input-focus (window-frame)))))
 
-(defvar server-stop-automatically nil
-  "Internal status variable for `server-stop-automatically'.")
-
 ;;;###autoload
 (defun server-save-buffers-kill-terminal (arg)
   ;; Called from save-buffers-kill-terminal in files.el.
@@ -1779,11 +1783,19 @@ server-save-buffers-kill-terminal
 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."
-  (let ((proc (frame-parameter nil 'client)))
+only these files will be asked to be saved.
+
+When running Emacs as a daemon and with
+`server-stop-automatically' (which see) set to `kill-terminal' or
+`delete-frame', this function may call `save-buffers-kill-emacs'
+if there are no other active clients."
+  (let ((stop-automatically
+         (and (daemonp)
+              (memq server-stop-automatically '(kill-terminal delete-frame))))
+        (proc (frame-parameter nil 'client)))
     (cond ((eq proc 'nowait)
 	   ;; Nowait frames have no client buffer list.
-	   (if (length> (frame-list) (if server-stop-automatically 2 1))
+	   (if (length> (frame-list) (if stop-automatically 2 1))
                ;; If there are any other frames, only delete this one.
                ;; When `server-stop-automatically' is set, don't count
                ;; the daemon frame.
@@ -1792,7 +1804,7 @@ server-save-buffers-kill-terminal
 	     ;; If we're the last frame standing, kill Emacs.
 	     (save-buffers-kill-emacs arg)))
 	  ((processp proc)
-           (if (or (not server-stop-automatically)
+           (if (or (not stop-automatically)
                    (length> server-clients 1)
                    (seq-some
                     (lambda (frame)
@@ -1819,14 +1831,13 @@ server-save-buffers-kill-terminal
 	  (t (error "Invalid client frame")))))
 
 (defun server-stop-automatically--handle-delete-frame (frame)
-  "Handle deletion of FRAME when `server-stop-automatically' is used."
-  (when (and server-stop-automatically
-             (null (cddr (frame-list))))
+  "Handle deletion of FRAME when `server-stop-automatically' is `delete-frame'."
+  (when (null (cddr (frame-list)))
     (let ((server-stop-automatically nil))
       (save-buffers-kill-emacs))))
 
 (defun server-stop-automatically--maybe-kill-emacs ()
-  "Handle closing of Emacs daemon when `server-stop-automatically' is used."
+  "Handle closing of Emacs daemon when `server-stop-automatically' is `empty'."
   (unless (cdr (frame-list))
     (when (and
 	   (not (memq t (mapcar (lambda (b)
@@ -1840,41 +1851,67 @@ server-stop-automatically--maybe-kill-emacs
 				(process-list)))))
       (kill-emacs))))
 
-;;;###autoload
-(defun server-stop-automatically (arg)
-  "Automatically stop server as specified by ARG.
-
-If ARG is the symbol `empty', stop the server when it has no
+(defun server-apply-stop-automatically ()
+  "Apply the current value of `server-stop-automatically'.
+This function adds or removes the necessary helpers to manage
+stopping the Emacs server automatically, depending on the whether
+the server is running or not.  This function only applies when
+running Emacs as a daemon."
+  (when (daemonp)
+    (let (empty-timer-p delete-frame-p)
+      (when server-process
+        (pcase server-stop-automatically
+          ('empty        (setq empty-timer-p t))
+          ('delete-frame (setq delete-frame-p t))))
+      ;; Start or stop the timer.
+      (if empty-timer-p
+          (unless server-stop-automatically--timer
+            (setq server-stop-automatically--timer
+                  (run-with-timer
+                   10 2
+		   #'server-stop-automatically--maybe-kill-emacs)))
+        (when server-stop-automatically--timer
+          (cancel-timer server-stop-automatically--timer)
+          (setq server-stop-automatically--timer nil)))
+      ;; Add or remove the delete-frame hook.
+      (if delete-frame-p
+          (add-hook 'delete-frame-functions
+		    #'server-stop-automatically--handle-delete-frame)
+        (remove-hook 'delete-frame-functions
+                     #'server-stop-automatically--handle-delete-frame)))))
+
+(defcustom server-stop-automatically nil
+  "If non-nil, stop the server under the requested conditions.
+
+If this is the symbol `empty', stop the server when it has no
 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
+If this 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
+If this 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'
-flag can be stopped, and if so, stop the server itself.
-
-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."
-  (when (daemonp)
-    (setq server-stop-automatically arg)
-    (cond
-     ((eq arg 'empty)
-      (setq server-stop-automatically nil)
-      (run-with-timer 10 2
-		      #'server-stop-automatically--maybe-kill-emacs))
-     ((eq arg 'delete-frame)
-      (add-hook 'delete-frame-functions
-		#'server-stop-automatically--handle-delete-frame))
-     ((eq arg 'kill-terminal))
-     (t
-      (error "Unexpected argument")))))
+flag can be stopped, and if so, stop the server itself."
+  :type '(choice
+          (const :tag "Never" nil)
+          (const :tag "When empty (no clients, unsaved files, or processes)"
+                 empty)
+          (const :tag "When killing last terminal" kill-terminal)
+          (const :tag "When killing last terminal or frame" delete-frame))
+  :set (lambda (symbol value)
+         (set-default symbol value)
+         (server-apply-stop-automatically))
+  :version "29.1")
+
+(defun server-stop-automatically (value)
+  "Automatically stop the Emacs server as specified by VALUE.
+This sets the variable `server-stop-automatically' (which see)."
+  (setopt server-stop-automatically value))
 
 (define-key ctl-x-map "#" 'server-edit)
 
-- 
2.25.1