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

[Jim Porter <jporterbugs@gmail.com>]

On 11/23/2021 3:59 PM, Gregory Heytings wrote:
>>
>> I never posted a rigorous specification of the behavior I wanted for 
>> that reason: I was soliciting feedback to develop a patch that meets 
>> my needs.
>>
> 
> That's not what you did, you posted a patch claiming that one of the 
> existing behaviors had a bug and should be replaced with another one.

That's in reference to my original message on Oct 19[1], which I cited 
earlier in the paragraph that you excerpted the quote from. At the time, 
I said:

> I didn't see any options to configure this behavior in Emacs, but 
> looking over the code, it shouldn't be that hard for me to write a patch 
> to add this as an option. Before I started though, I wanted to see what 
> others thought. Is this a behavior others would be interested in? If so, 
> are there any other particulars I should take into account in my patch?

--------------------

On 11/23/2021 3:59 PM, Gregory Heytings wrote:
>> you've instead implemented the behavior "for" me
>>
> 
> That's not what I did, I tried to implement a function that provides 
> different possible behaviors corresponding to different needs and 
> expectations, not just the behavior you expected.  And I did this based 
> on your feedback, what others said on emacs-devel, and my own feelings.  
> I hoped (and thought until today) that the behavior you expected was one 
> of them.

And that's fine. I have no issue supporting behaviors other than the one 
I personally want. (For example, I have no problem with the `empty' 
behavior, even though I wouldn't personally find it useful.) However, I 
also want to be sure that we don't provide *unnecessarily* many options 
here, since that adds to the maintenance burden and may produce 
unexpected behavior for users if they don't quite understand the 
distinction between each possible setting.

In this case, I think it's better to continue prompting users when 
killing a non-last client just like Emacs 28[2] does. There might be 
some value in avoiding that prompt, but I think a user who *doesn't* use 
`server-stop-automatically' is just as likely to want that behavior, so 
if we want to support that, we should provide a separate configuration 
option to do so.

In particular, I think it's valuable to prompt users in this case 
because it comes up when using emacsclient and committing code. If I set 
`EDITOR="emacsclient -c"' and run `git commit', then under Emacs 28, I 
can close the client with `C-x C-c'[3] and it will prompt me if I forgot 
to save the commit message. That's useful because without the prompt, it 
would (usually) just abort the commit due to an empty message.

I think that behavior is worth preserving. (Likewise, I think it's worth 
preserving the ability to keep all your open frames by pressing C-g if 
Emacs prompts you about saving files when you try to kill a client.) 
Thus, I implemented the patch that you can see in the original message.

--------------------

In addition, I'm not certain that we need both `delete-frame' (or 
`delete-last-frame' as it's called in your latest patch) and 
`kill-terminal'. The only difference between the two is that in the 
former, individually closing the last non-daemon frame (e.g. by `C-x 5 
0' or clicking the X on the frame's titlebar) also kills the daemon. I 
prefer the behavior of `delete-frame', since my mental model is that the 
daemon should be killed *whenever* the last non-daemon frame is closed, 
no matter *how* it's closed. Maybe someone has a strong opinion in the 
other direction and actively wants the `kill-terminal' behavior instead. 
I don't have a problem with keeping that option around if someone would 
actually use it, but I'm skeptical of that. In this case though, I'm 
happy to present my argument and then defer to the Emacs maintainers to 
make the final call, whatever that may be. If it stays, that's fine with 
me; if it goes, that's fine too.

--------------------

Since, as you mentioned before, we have different mental models for how 
this works, I'm mainly trying to explain through both prose and patches 
how this should work under my mental model. If I understand your prior 
messages correctly, your mental model is best encapsulated by the 
`empty' configuration. I'm 100% happy to keep that around as-is, even 
though it's not my preferred solution. However, my preferred solution is 
"something close to `delete-frame'".

Your patch in bug#51377 gets most of the way to what I wanted, but there 
are a couple of corner cases that don't work like I expect, hence this 
bug. Whether the behavior is a true "bug", a miscommunication, or 
something else doesn't matter much to me. All I'm looking for is 
something along the following lines (note: this isn't 100% rigorous, so 
don't take it as a precise specification):

1) When killing a non-last client (including by closing the last of its 
frames), the behavior should be exactly the same as the default 
emacsclient behavior. (That's what this bug is about.)

2) When killing the last client (including by closing the last of its 
frames), the behavior should be exactly the same as killing Emacs when 
*not* using a server. (Roughly speaking, that means calling 
`save-buffers-kill-emacs'. It would also be nice to have an option to 
silence the "This Emacs session has clients; exit anyway?" prompt, 
though I can certainly understand others wanting to keep it around in 
this case.)

Again, that may not be 100% precise, but it's the mental model I've used 
while implementing patches for this on my end. The specific 
implementation I use has shifted somewhat over time as I find corner 
cases, and also through consulting your patches. Using 
`delete-frame-functions' as in your code greatly simplified my 
implementation, for example. Thanks for that.

Hopefully my delays in following up on bug#51377 haven't been *too* 
disruptive. As mentioned, I was unfortunately too busy to devote 
sufficient time to it back then (my plan when posting to emacs-devel was 
just to collect feedback and to work on the implementation slowly over 
the next several weeks). Now my schedule is a bit less hectic, and I'm 
hoping to address the last few concerns I had after taking the time to 
test things out more thoroughly.

I'm certainly not trying to step on your toes or undo your patch. Just 
to account for certain corner cases that I didn't have a chance to 
investigate (or resolve) at the time.

[1] https://lists.gnu.org/archive/html/emacs-devel/2021-10/msg01465.html
[2] Or without `server-stop-automatically'
[3] You could also use `C-x #' here, but I find `C-x C-c' easier to 
type, so prefer the latter in cases where either would do what I want.