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

[Eli Zaretskii <eliz@gnu.org>]

> Date: Mon, 28 Nov 2022 20:23:17 -0800
> From: Jim Porter <jporterbugs@gmail.com>
> 
>    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...
> 
> It'd be nice if this were a defcustom so that people who prefer the 
> Customize interface could use that instead of editing their init files. 
> Here's a patch for that.

Thanks.

> One question though: should this only go on the master branch, or should 
> it go into the 29 branch? To me, it seems like it could go either way, 
> though I think it'd be nice to make this easier for users in 29. I'll do 
> whatever the maintainers think is best though.

Let's not start installing new features on the release branch.  We have
enough new stuff there already.  Besides, the changes you suggest are hardly
trivial, and saying just "make a function into a defcustom" doesn't come
close to describing it, IMO.

So please install on master, once you take care of the comments below.

> If it goes on the master branch only, I'll add back the function form of 
> 'server-stop-automatically' for compatibility, and then mark it obsolete.

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.

> -@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
> +With the value @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
> +With the value @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.

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

etc., I guess you get the idea.

> @@ -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?

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

And why this significant refactoring of the original function? was there
something wrong with it?