Don't yes-or-no-p query the user when killing plz process buffers

[mkcms]

The current behavior when interrupting a sync request by C-g or in other way, externally - e.g. with with-timeout, Emacs unexpectedly asks if I really want to kill the curl process buffer.
E.g. when interrupting this long-running request with C-g:

(plz 'get "http://httpbin.org/delay/10")

Emacs asks if I want to really kill the curl buffer. This is unexpected, as the library itself should never be interactive.
It even does this in --batch mode, which I caught here: https://github.com/mkcms/compiler-explorer.el/actions/runs/9200670124/job/25307608066#step:6:148

I have signed the FSF papers a couple of years ago and already contributed to Emacs, so this is good to go.

[alphapapa]

Hi Michał,

Ah, this is great! I just had this happen to me yesterday when I was testing something while pressing C-g. I hadn't thought about this variable.

What do you think about using defsubst or define-inline for plz--kill-buffer? It would seem reasonable to me, to avoid an extra function call.

Thanks for submitting this!

cc: @josephmturner

[josephmturner]

Thank you @mkcms! I wonder if it might be more appropriate to temporarily bind kill-buffer-query-functions to (cl-callf2 remq 'process-kill-buffer-query-function kill-buffer-query-functions) instead of nil. OTOH, remq would allow for greater flexibility. OTOH, kill-buffer-query-functions is not a defcustom, so customizability might not be important in this case.

[mkcms]

@josephmturner

I wonder if it might be more appropriate to temporarily bind kill-buffer-query-functions to (cl-callf2 remq 'process-kill-buffer-query-function kill-buffer-query-functions) instead of nil

There's no need, because we actually do want to unconditionally kill the buffer. So we don't want any query. If another user function, apart from process-kill-buffer-query-function remains in there, and actually prevents the buffer from being killed (or just also asks us for confirmation), that won't really fix the problem.

@alphapapa

What do you think about using defsubst or define-inline for plz--kill-buffer? It would seem reasonable to me, to avoid an extra function call.

Sure, I can change that if you want, but I get this warning:

Warning: Optimization failure for plz--kill-buffer: Handler: plz--kill-buffer--inliner
(wrong-type-argument stringp (process-buffer process))

I rarely use these - do you think it's worth it?

[josephmturner]

There's no need, because we actually do want to unconditionally kill the buffer. So we don't want any query. If another user function, apart from process-kill-buffer-query-function remains in there, and actually prevents the buffer from being killed (or just also asks us for confirmation), that won't really fix the problem.

That sounds reasonable to me. I was imagining a scenario where for some reason (debugging, maybe?) the user wants to never kill any process buffers. The user could add a different hook, and that would ensure the buffer stayed alive.

OTOH, if we only remq the default hook, other hooks might prompt for confirmation in an undesired way.

Whatever you guys decide is fine by me. My example user story is pretty far-fetched - I'm just trying to be thorough :)

[alphapapa]

According to the Elisp manual, if generate-new-buffer is called with the inhibit-buffer-hooks argument, kill-buffer-query-functions are not called. So it would probably be better if we just passed that argument when we call generate-new-buffer.

[mkcms]

@alphapapa

if generate-new-buffer is called with the inhibit-buffer-hooks argument, kill-buffer-query-functions are not called

Nice, that's even better. I've updated the PR to do that.

[alphapapa]

@mkcms Thank you!

[josephmturner]

Note that the current solution only works on Emacs 28+. We were not able to backport generate-new-buffer's optional INHIBIT-BUFFER-HOOKS argument in compat. See this comment for details.

[alphapapa]

@josephmturner Thanks. So do we need to bind kill-buffer-query-functions on Emacs 27 after all?

I can't reopen this PR, so we'll have to make a new issue.

[josephmturner]

@alphapapa Yes, I believe so. I think we could apply the same patch that @mkcms wrote, and then add a TODO to deprecate plz--kill-buffer when plz stops supporting Emacs 27.