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

[Jim Porter <jporterbugs@gmail.com>]

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

On 12/1/2022 9:08 AM, Eli Zaretskii wrote:
> I see no reason to make the function obsolete.  It does not harm to have
> both a variable and a function by the same name.

Ok, fixed. I've added back a 'server-stop-automatically' function that 
just sets the variable.

> This @itemize list should be converted to a @table, formatted like this:
> 
>    @item empty
>    This value caused the server to be stopped when...
> 
>    @item delete-frame
>    This value means that when the last client frame is deleted...

Done.

>> @@ -1780,7 +1784,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
>> +  (if (and (daemonp)
>> +           (memq server-stop-automatically '(kill-terminal delete-frame))
> 
> Why is this needed?  I guess I don't understand why non-trivial code changes
> are in a patch that was supposed to just add a defcustom?

Addressed in my other message, but I've added a defsubst to make this 
clearer (I hope).

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

From c0f245bffa7243ad14118e1b6145c7b0e0b3cd3f 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

* 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-stop-automatically--when-killing-terminal):
(server-apply-stop-automatically): New functions.
(server-stop, server-start): Call 'server-apply-stop-automatically'.
(server-save-buffers-kill-terminal): Adjust
'server-stop-automatically' conditional.
(server-stop-automatically--handle-delete-frame)
(server-stop-automatically--maybe-kill-emacs): Update docstrings.

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

diff --git a/doc/emacs/misc.texi b/doc/emacs/misc.texi
index 29c0bed19c..702c72bac2 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 1b027f88ce..e16a9e464e 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,8 +1776,10 @@ 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'.")
+(defsubst server-stop-automatically--when-killing-terminal ()
+  "Return non-nil if we should potentially stop Emacs when killing a terminal."
+  (and (daemonp)
+       (memq server-stop-automatically '(kill-terminal delete-frame))))
 
 ;;;###autoload
 (defun server-save-buffers-kill-terminal (arg)
@@ -1780,7 +1789,7 @@ server-save-buffers-kill-terminal
 
 If emacsclient was started with a list of filenames to edit, then
 only these files will be asked to be saved."
-  (if server-stop-automatically
+  (if (server-stop-automatically--when-killing-terminal)
       (server-stop-automatically--handle-delete-frame (selected-frame))
     (let ((proc (frame-parameter nil 'client)))
       (cond ((eq proc 'nowait)
@@ -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)))))
 
 (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)
@@ -1842,41 +1851,66 @@ 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" 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