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

[Jim Porter]

On 12/1/2022 9:29 AM, Eli Zaretskii wrote:
> > Date: Mon, 28 Nov 2022 21:31:02 -0800
> > From: Jim Porter <jporterbugs@gmail.com>
> > Cc: larsi@gnus.org, 51993@debbugs.gnu.org, gregory@heytings.org
> >
> > > > > The issue in the quote above is that if you enable automatic server
> > > > > shutdown in Emacs 29, it changes the behavior of exiting an emacsclient
> > > > > even when it wouldn't stop the server (i.e. when there are other active
> > > > > clients). That's surprising to me, I wouldn't expect that setting to
> > > > > affect cases when it decides *not* to kill the Emacs daemon.
> > > >
> > > > Sounds like a bug to me, because it contradicts what the doc string
> > > > says.
> > >
> > > That's how it seems to me too. ...
> > Ok, after quite a delay, here's a patch for this. Previously, the
> > function 'server-stop-automatically--handle-delete-frame' responded to
> > both 'C-x C-c' ('save-buffers-kill-terminal') and 'delete-frame', which
> > made it more complex. I've moved the 'C-x C-c' case into
> > 'server-save-buffers-kill-terminal', which simplifies
> > 'server-stop-automatically--handle-delete-frame'.
> >
> > The updated 'server-save-buffers-kill-terminal' should now make sure
> > that the new stop-automatically behavior only happens when there are no
> > other client processes (or nowait frames).
>
> We want this on the release branch, right?  Then please make it the minimal
> change which fixes the immediate cause of the bug, and does nothing else: no
> refactoring, no reshuffling of the code or making it nicer or less
> complicated -- all that just makes the risk of new bugs higher and the job
> of reviewing the patch harder.

Thanks for taking a look. I believe we'd want this on the release 
branch. Here's the absolute minimum I could manage, although it doesn't 
quite fix everything that my previous patch does. In particular, when 
server-stop-automatically is set to 'kill-terminal' (or 'delete-frame'):

* If you type 'C-x C-c' ('save-buffers-kill-terminal') in a nowait 
client frame, and there are still other (non-daemon) frames, nothing 
happens. You'd have to use 'C-x 5 0' ('delete-frame') instead. Fixing 
this would basically mean doing 90% of my original patch, so it's 
probably too risky.

* If you type 'C-u C-x C-c', it doesn't silently save all the relevant 
buffers. That's because 'server-save-buffers-kill-terminal' doesn't 
forward the prefix arg to 
'server-stop-automatically--handle-delete-frame'. That's a separate (but 
closely related) bug, so I didn't fix that either.

In a followup message, I'll show the breakdown of my previous patch into 
smaller steps with some more detailed explanation of why I think it 
simplifies things enough to be worth making the change on the master branch.

0001-Make-killing-a-non-last-client-work-the-same-no-matt.patch
Description: Text document