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

[GNU bug Tracking System]

Your message dated Tue, 6 Dec 2022 14:20:30 -0800
with message-id <1f9d803c-0ffd-3b16-07f7-8c90fe7ba546@gmail.com>
and subject line Re: bug#51993: 29.0.50; [PATCH for 29.1] Killing emacsclient 
terminal with `server-stop-automatically' doesn't prompt to save files
has caused the debbugs.gnu.org bug report #51993,
regarding 29.0.50; [PATCH] Killing emacsclient terminal with 
`server-stop-automatically' doesn't prompt to save files
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs@gnu.org.)


-- 
51993: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=51993
GNU Bug Tracking System
Contact help-debbugs@gnu.org with problems

--- Begin Message --- Subject: 29.0.50; [PATCH] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files Date: Fri, 19 Nov 2021 20:29:43 -0800 When killing an emacsclient terminal via C-x C-c, it should prompt to save the files initially passed to emacsclient. To see this in action:

  $ emacs -Q --daemon
  $ emacsclient -a "" -c foo.txt
  $ emacsclient -a "" -c bar.txt

  ;; In the first client frame:
  foobar ;; Insert some text
  C-x C-c
  ;; Emacs prompts "Save file /path/to/foo.txt?..."

Now try the above, but call `(server-stop-automatically 'delete-frame)' first (or replace `delete-frame' with `kill-terminal'; it doesn't matter). In this case, Emacs doesn't prompt to save the file. However, the docstring/comments in `server-save-buffers-kill-terminal' say that it should: "Offer to save each buffer, then kill the current client. ... Only files from emacsclient file list."

Attached is a patch to restore this behavior when stopping the server automatically. This puts all of the logic into `server-save-buffers-kill-terminal', which allows `server-stop-automatically--handle-delete-frame' to be simpler. I've also added some more detailed comments explaining the logic here, since there are some pretty subtle aspects to it.

There's a further subtlety that I should probably mention here though: when killing a nowait frame, it would kill Emacs entirely if that were the last frame (even in Emacs 28, and probably earlier). The only way (that I can think of) that this could come up would be to run:

  $ emacs -Q --eval '(start-server)'
  $ emacsclient -n
  C-x 5 0 ;; in the non-client frame
  C-x C-c ;; in the emacsclient frame

However, when doing this with a regular (non-nowait) client, the last step would report the error "Attempt to delete the sole visible or iconified frame". Even more oddly, it would work the *second* time you tried to kill the client terminal, since `server-delete-client' would set the `client' frame-parameter to nil before deleting it; on the second attempt, Emacs thinks the frame is a non-client frame (even though it is).

I've fixed this in the second patch by following the nowait behavior: if you kill a client and *all* the existing frames belong to that client, it kills Emacs entirely. I'm not sure this will come up often in practice, but it's a fairly simple change.

Some tests would be nice to prevent this from regressing, but I'm not sure how to write a test that starts up a daemon and connects clients to it...

0001-Ensure-killing-an-emacsclient-terminal-prompts-to-sa.patch
Description: Text document

0002-Don-t-explicitly-delete-client-frames-when-killing-E.patch
Description: Text document

--- End Message ---

> --- Begin Message ---
>
>
>
> Subject: 
>
> Re: bug#51993: 29.0.50; [PATCH for 29.1] Killing emacsclient terminal with `server-stop-automatically' doesn't prompt to save files
>
>
>
>
> Date: 
>
> Tue, 6 Dec 2022 14:20:30 -0800
>
>
>
>
> On 12/4/2022 2:26 PM, Jim Porter wrote:
> > Thanks again for the reviews. I've merged the first part to the 29 
> > branch as 4bcdb1cc65bf779b6479f99a7aa767ab83b3bae1.
> >
> > Like you said, the second patch (for master) should wait until the next 
> > merge of 29 to master.
>
> And now I've merged the cleanup patch to master as 
> bcf4d96db3a61e0d02a584fa9fceb049cdad6fe8. Closing this bug now.
>
> Thanks once again.
>
>
> --- End Message ---