[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#59668: 29.0.50; [PATCH] Make 'server-stop-automatically' into a defc
From: |
Jim Porter |
Subject: |
bug#59668: 29.0.50; [PATCH] Make 'server-stop-automatically' into a defcustom |
Date: |
Thu, 1 Dec 2022 10:33:57 -0800 |
On 12/1/2022 9:08 AM, Eli Zaretskii wrote:
Date: Mon, 28 Nov 2022 20:23:17 -0800
From: Jim Porter <jporterbugs@gmail.com>
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. ...
Sounds good to me.
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.
Fine by me. The function will just be a one-liner anyway: (setopt
server-stop-automatically arg).
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.
Will do. I made the minimal set of changes to the manual, but while I'm
here, I agree that it would be good to improve things further.
@@ -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?
It's due to a change in the meaning of the 'server-stop-automatically'
value. Previously, it was an internal variable that was set to nil after
calling "(server-stop-automatically 'empty)", or when calling
'server-stop-automatically' in a non-daemon session. Since
'server-stop-automatically' is now a defcustom, that means that a) it
can really be set to 'empty', and b) it can be non-nil in non-daemon
sessions. So this extra code is just to account for the change in meaning.
I could make a helper function that returns the same value as the *old*
version of the 'server-stop-automatically' variable; either way has the
same meaning. I could also keep the old variable around, possibly
renamed to something like 'server-stop-automatically--kill-terminal',
but I think a helper function would be cleaner.
+(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?
The previous implementation was limited in that you could call it once,
but you couldn't necessarily call it a second time to change the
setting. For example, this would put you in an inconsistent state:
(server-stop-automatically 'delete-frame)
(server-stop-automatically 'empty)
After this, the 'empty' setting would be enabled, and the 'delete-frame'
setting would still be partially-enabled too.
That wasn't so much of a problem when you'd just call this function only
once in your init file, but for the Customize interface, I think it's
important to make sure that users can set the defcustom multiple times,
e.g. if they change their minds while customizing it. The changes for
'server-apply-stop-automatically' are to support that.
As you said, these changes are more extensive than "just" adding a
defcustom, but all the other changes are there to support the Customize
interface.