bug#61176: post-command-hook is not run if minibuffer input is aborted

[Jonas Bernoulli]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> - However, when the command reads from the minibuffer and the user
>>   aborts that, then post-command-hook is NOT run a second time AFTER
>>   the command.
>
> Could you clarify what you mean here?
> Let's say in the following scenario:
>
> - The user hits a key like `M-x` which causes a minibuffer to be entered.
> - the user hits C-g.
> - Emacs exits the minibuffer and doesn't even call the command because
>   the interactive args could not be gathered.
>
> When do you expect `post-command-hook` to be run?

Going back to the examples I provided, if the user does NOT abort, then
this happens:

>> ;; -setup   ([f1]       -command)
>> ;; -post    ([]         -command)
>> ;; -post    ([97]       self-insert-command)
>> ;; -exit    ([return]   exit-minibuffer)
>> ;; -command
>> ;; -post    ([f1]       -command)

And if the user DOES abort, I would like the behavior to be changed like
so:

>> ;; -setup   ([f1]       -command)
>> ;; -post    ([]         -command)
>> ;; -exit    ([7]        abort-minibuffers)
>> ;; Quit
>> ;; -post    ([]         abort-minibuffers)
*NEW* -post    ([f1]       -command)

> There's also the case where the command is called and it enters the
> minibuffer (rather than doing it within the interactive spec).  Not sure
> if it makes a significant difference.

The sequence of events is the same as in the above case.  I posted the
wrong log in the examples I provided.  I posted two instances of the user
not aborting instead providing the output for when they abort, which is:

>> ;; -command
>> ;; -setup   ([f1]       -command)
>> ;; -post    ([]         -command)
>> ;; -exit    ([7]        abort-minibuffers)
>> ;; Quit
>> ;; -post    ([]         abort-minibuffers)

So it seems that iff a command uses the minibuffer, then
post-command-hook is ALWAYS run for it right before the minibuffer is
entered, regardless of where the recursive edit is entered; and iff the
user aborts the minibuffer, then the post-command-hook is NEVER run
"after"/"post" the outer command.

> Also it would help to know what you need `post-command-hook` for.

This is relevant for the Transient package.  The following is a
simplified description of relevant parts of what it does.

Calling a transient prefix command installs a transient keymap and adds
functions to `pre-command-hook' and `post-command-hook'.

The pre-command function is responsible for determining whether a
subsequently called (suffix) command should exit the transient state.
If we are about to exit the transient, then this does also set some
global variables to nil, which are only relevant while the transient is
still active.  However, it does not and cannot unset all variables and
most importantly it does not remove the transient keymap and the pre-
and post-command functions.

This function may (or may not) set other global variables, so that the
command that is about to be called has access to the arguments set in
the transient command.  This is similar to how prefix arguments are
implemented.

However, if I remember correctly, in the case of prefix arguments, there
is some C code that takes care of unsetting the prefix argument, if the
next command is aborted.  Transient on the other hand has to do that in
Elisp, and it used the post-command-hook for that (among other things).

Then the command's interactive spec is processed and then the command is
called with the arguments thus determined.

Once that is done, then the post-command function is called.  It is
responsible for redisplaying the transient buffer that displays the
available suffix commands.  Or if the suffix command should exit the
transient, then it has to remove the transient map and the pre- and
post functions, and unset the variables that serve a similar role to
prefix-arg, as well as some internal variables.

The suffix command may use the minibuffer inside interactive and/or in
its body.  If that happens, then transient has to suspend the transient
keymap and pre- and post-command functions, while the minibuffer is in
use.

There are two kinds of suffix commands that may (or may not) use the
minibuffer:

(1) Commands that are specifically designed to be used to set the value
of some argument in the transient command.  These commands are fully
under our control and are designed to handle the suspension and resuming
of the transient map and the pre- and post-command hooks, using
minibuffer-setup-hook, minibuffer-exit-hook, and unwind-protect.

(2) Arbitrary commands, which may have been written with Transient in
mind, but which more likely do nothing to account for the needs of
Transient, i.e., any command that exists in Emacs.

When a (2) command is invoked and that uses the minibuffer, then our
post-command function detects that because it is called with
this-command being the suffix command and this-command-keys-vector being
an empty vector.  The transient map and pre- and post-command hooks are
then suspended like when a (1) command uses the minibuffer, but it is
not possible to use unwind-protect to ensure that these things are
reinstated (or the transient is fully exited), even if the user
aborts while the minibuffer is active.

If post-command-hook were run ("post" command) regardless of whether
the user aborts the minibuffer, then such aborts would not have to be
handled specifically.

Since that is not the case, the pre-mature invocation of the
post-command function, has to delay the resume-or-exit work until some
other event occurs.  Currently that is being done by adding a new
minibuffer-exit function and *another* post-command function.  The first
is designed perform the resume/exit behavior if the minibuffer is
aborted, and the second function is designed to perform the resume/exit
behavior if the first function did not end up doing that, i.e., if the
minibuffer was not aborted.

This is fragile.  Heuristics have to be used to determine whether the
minibuffer is exited normally or if it was aborted.  (There is only
minibuffer-exit-hook, and as far as I can tell, there is no reliable way
to determine whether that was called because the minibuffer was exited
normally or was aborted.)  This approach works more or less, but a few
times I already though I had finally tweaked it enough to handle all
edge-cases, only to later learn that was not so.  Currently there is one
case where it doesn't work as intended.  And if a third-party completion
framework were used that exits the minibuffer in some highly unexpected
way then it would also not work (but currently no such framework does
that, I believe)).

> [ There are several "alternatives" to `post-command-hook` plus there
>   are cases where code is executed not via a command, yet it can be
>   viewed as a command execution as well (e.g. opening a file via
>   `emacsclient`), so over the years ad-hoc calls to `post-command-hook`
>   have been sprinkled outside of the "command-loop", which makes this
>   whole business even more muddy.  ]

What alternatives are you thinking about?

Piuu, that got a bit long.  I left out details, but I hope it became
clear why I need the post-command-hook to always be run *post* command
(and that that is a legitimate need).

> - The user hits a key like `M-x` which causes a minibuffer to be entered.
> - the user hits C-g.
> - Emacs exits the minibuffer and doesn't even call the command because
>   the interactive args could not be gathered.

That could be used as an argument as to why post-command-hook should
not be run when the interactive minibuffer is aborted: if we never go
"inside" the command, there also is no point in going "post" command.

However, it does not appear that this is actually the reason why
post-command-hook is not being run "post" command, not from a design
perspective at least.  If the user aborts the minibuffer usage of the
following command, which uses the minibuffer in its body, then the
post-command-hook also isn't being run "post" command, even though we
clearly made it "into" the command:

>> (defun -command ()
>>   (interactive)
>>   (message ";; -command")
>>   (read-string "-command: "))
>>
>> ;; -command
>> ;; -setup   ([f1]       -command)
>> ;; -post    ([]         -command)
>> ;; -exit    ([7]        abort-minibuffers)
>> ;; Quit
>> ;; -post    ([]         abort-minibuffers)

Thanks for looking into this!
Jonas