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

[Eli Zaretskii]

[Please in the future avoid changing the Subject when discussing bugs: it
makes it harder for me and others to follow the discussion by grouping
messages by Subject.]

> Date: Thu, 1 Dec 2022 17:09:24 -0800
> Cc: larsi@gnus.org, 51993@debbugs.gnu.org, gregory@heytings.org
> From: Jim Porter <jporterbugs@gmail.com>
> 
> > 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.

Thanks.  Surprisingly, the previous version was easier to review and agree
to in some of its parts, because it kept more of the original code, and only
changed the conditions when save-some-buffers or save-buffers-kill-emacs
were called.  This version of the patch completely rewrites the affected
code.  Was that really necessary?

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

Please tell more about why these two situations cannot be handled by
similarly simple changes.

What made the original patch risky from my POV is that it rewrote too much
of the original code.  Would it be possible to retain as much as possible of
the original code, and just change the conditions for calling
save-some-buffers etc., and/or add calls to it where it previously wasn't
called for some reason?

Please understand where I stand: use of emacsclient is very popular and very
widespread, so I'd like to avoid adding regressions to it as result of
enhancements we make.  If I conclude that the new features installed as part
of Emacs 29 development are too complicated and/or include semi-buggy
aspects, I'd rather revert those new features than risk regressions in
emacsclient and in the server behavior.