bug#51993: 29.0.50; [PATCH] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files

[Jim Porter]

On 9/9/2022 11:04 AM, Jim Porter wrote:
> On 9/9/2022 10:55 AM, Lars Ingebrigtsen wrote:
> > Gregory Heytings <gregory@heytings.org> writes:
> >
> > > > However, I'm not sure I understand the way forward, as you see
> > > > it. AFAIU, there are fundamental disagreements between you and
> > > > Gregory, and I don't think the disagreements are there because one
> > > > of you doesn't understand the proposals of the other.
> > > >
> > > > So how could these disagreements be reconciled?
> >
> > > I'm in the process of implementing something that should satisfy
> > > everyone, but it's not finished yet.
> >
> > This was half a year ago, and I'm not sure whether anything further was
> > done here?
>
> For what it's worth, I've been brainstorming some ideas for how to 
> abstract things out a bit further to make it easy for users to tune the 
> behavior just to their liking. I haven't gotten very far on that yet, 
> though.

Attached is a very WIP patch that hopefully gets across the idea I had. 
Hopefully the explanation below is sufficiently-clear; if anyone has 
questions, or if I missed some detail, let me know.

(Note: the first patch is just a bugfix so that 'server-delete-client' 
doesn't get called when Emacs *creates* the client.)

The patch adds two new hooks: 'server-before-delete-client-functions' 
and 'server-after-delete-client-functions'. These should give people the 
ability to add whatever behaviors they think make sense when closing an 
emacsclient connection. The default behavior is the same as the current 
default (call 'save-some-buffers' before deleting the client, and do 
nothing after). To do something like the 'delete-frame' configuration of 
the current 'server-stop-automatically' API, you might do something like 
this:

  (add-hook
   'server-before-delete-client-functions
   (lambda (proc arg)
     (when (>= 1 (seq-count
                  (lambda (frame)
                    "Return non-nil if FRAME's client is another process."
                    (not (equal (frame-parameter frame 'client) proc)))
                  (frame-list)))
       ;; If there are no non-daemon frames other than ones owned by
       ;; this client, we want to do all the prompting that
       ;; 'save-buffers-kill-emacs' does, but without really killing.
       ;; HACK ALERT: This is a pretty clumsy way to do this...
       (cl-letf (((symbol-function 'kill-emacs) #'ignore))
         (save-buffers-kill-emacs arg))
       ;; Stop running other hooks.
       t)))

  (add-hook 'server-after-delete-client-functions
            (lambda (proc)
              ;; Kill emacs if the only frame left is the daemon frame.
              (unless (cdr (frame-list))
                (kill-emacs))))

(Some helper functions might make these hooks simpler, of course.)

This interface has some other potentially-interesting uses. For example, 
the after-deletion hook could start a timer and only kill Emacs if no 
new clients connected after N seconds[1]. That would be useful in cases 
like `git rebase -i` where Git might run your $EDITOR multiple times in 
a row. It would be a waste of CPU to shut down and restart the Emacs 
daemon every time.

One thing I'm not sure about yet: should the 'empty' setting for 
'server-stop-automatically' just become a hook on 
'server-after-delete-client-functions'? It's not *quite* the same, since 
the 'empty' setting will also leave the daemon running until any active 
subprocesses finish too. Maybe it would make sense for 
'server-stop-automatically' to stick around, but just be a boolean (when 
non-nil, use the current 'empty' behavior)? Then we keep all the nice 
features of the 'empty' setting, and the other behaviors can be 
implemented via these new hooks.

[1] This might need an additional hook like 
'server-after-create-client-functions'.

0001-Only-delete-server-client-from-the-sentinel-when-the.patch
Description: Text document

0002-WIP-Add-hooks-before-after-deleting-Emacs-clients.patch
Description: Text document