Problem quitting properly from transient keymap with one keystroke

Problem quitting properly from transient keymap with one keystroke

[Bob Weiner]

I am using set-transient-map to create an overriding keymap that
provides quick access to frame and window functions.  I have both a
stay-in-transient-map function and an on-exit function defined that
are sent as parameters to the set-transient-map call.

Everything is working as desired except for disabling the transient map.
Both C-g and q should disable the transient map and run the on-exit
function I have defined.  I am using inhibit-quit set to t to
control the operation of C-g.  Both C-g and q set a flag which informs
the stay-in-transient-map function to disable the keymap.  The on-exit
function then is called after the keymap is disabled and for C-g,
(keyboard-quit) is called.

The problem is that set-transient-map uses pre-command-hook to test the
stay-in-transient-map function and to exit, so if a keystroke sets the
flag read by the stay-in-transient-map function to disable the transient
map, this check is not done until another key is pressed because the
pre-command-hook is not run again until then (so the code doesn't see
the flag change until then).

I can't just add a post-command-hook that calls the transient map
disable function because calling (keyboard-quit) from the
post-command-hook triggers an error and I imagine that is not a proper
usage scenario.

Is there any clean way to exit and abort from a transient keymap upon a
single key press?  I have tried manually invoking (from the
post-command-hook) the disable function that set-transient-keymap
returns, but have not gotten that to work either.

FYI, when I mention a key, I mean a bound key within the current keymaps.


Thanks,

Bob

Re: Problem quitting properly from transient keymap with one keystroke

[Robert Weiner]

[-- Attachment #1: Type: text/plain, Size: 4393 bytes --]

On Wed, Oct 18, 2017 at 11:13 PM, Bob Weiner <rsw@gnu.org> wrote:

>
> The problem is that set-transient-map uses pre-command-hook to test the
> stay-in-transient-map function and to exit, so if a keystroke sets the
> flag read by the stay-in-transient-map function to disable the transient
> map, this check is not done until another key is pressed because the
> pre-command-hook is not run again until then (so the code doesn't see
> the flag change until then).
>
> I can't just add a post-command-hook that calls the transient map
> disable function because calling (keyboard-quit) from the
> post-command-hook triggers an error and I imagine that is not a proper
> usage scenario.
>
> Is there any clean way to exit and abort from a transient keymap upon a
> single key press?


​I have figured it out though, the resultant code cries out for a
simplification
in handling ​exiting from a transient map.  The most important part
of the solution is that whatever you add to post-command-hook to process
each key press in the transient map must also manually call the function
that set-transient-map returns to force a quit of the transient map (call
this the quit-function).

The quit-function removes the transient map setup and then calls the on-exit
function (optional 3rd argument to set-transient-map) which does
application-specific
exit processing.  The on-exit function is where the full processing of the
application's {C-g} and quit command have to go.  But even that is not
enough.
The application's post-command-hook must catch {C-g} and continue its
processing;
any attempt to do a (keyboard-quit) from post-command-hook will generate an
error
rather than just doing the quit.  The only thing I found that worked was
using
(top-level) to ensure a clean exit from any recursive-levels when aborting
with
{C-g}.

Here is the resulting abbreviated code for reference.  It seems like a lot
to setup to allow
for aborting and quitting while always ensuring that the quit code is
executed.  The Elisp
manual could use some examples of handling both aborts and quits from
transient maps so this
is clearer.  I guess we could create some macros that would at least make
this setup reusable.

Bob

-----

(defvar hycontrol-frames-mode-map
  (let ((map (make-sparse-keymap)))
    (suppress-keymap map) ;; Disable self-inserting keys.
    (define-key map "\C-g"  (lambda () (interactive) (setq
hycontrol--exit-status 'abort-from-frames)))
    (define-key map "q"     (lambda () (interactive) (setq
hycontrol--exit-status 'quit-from-frames)))))

(defvar hycontrol--quit-function nil
  "Stores function auto-generated by a call to `set-transient-map' to
remove the transient-map later.")

(defconst hycontrol--frames-prompt "Frames> ")

(defun hycontrol-frames-post-command-hook ()
  "Added to `post-command-hook' while in HyControl frames mode."
  (condition-case ()
      (funcall #'hycontrol-frames-post-command-hook-body)
    (quit (funcall #'hycontrol-frames-post-command-hook-body))))

(defun hycontrol-frames-post-command-hook-body ()
  (if hycontrol--exit-status
      (progn (remove-hook 'post-command-hook
'hycontrol-frames-post-command-hook)
     (funcall hycontrol--quit-function))
    (message hycontrol--frames-prompt)))

(defun hycontrol-exit-mode ()
  "Run by the HyControl frame or window transient keymap after it is
disabled."
  (setq inhibit-quit nil)
  (pcase hycontrol--exit-status
    ('abort-from-frames   (progn (ding)
  (setq hycontrol--exit-status nil)
;; Use of (keyboard-quit) here would
;; trigger an error in post-command-hook,
;; so don't use.
(top-level)))
    ('quit-from-frames    (message "Finished controlling frames"))))

(defun hycontrol-stay-in-mode ()
  "Return non-nil if HyControl mode should remain active."
  (null hycontrol--exit-status))

(defun hycontrol-frames ()
  "(Excerpt only) Control frames interactively."
  (interactive "p")
  (setq inhibit-quit t
hycontrol--exit-status nil)
  (funcall #'hycontrol-frames-post-command-hook)
  (add-hook 'post-command-hook 'hycontrol-frames-post-command-hook)
  ;; Use normal event loop with transient-map until {C-g} or {q} is
  ;; pressed, then exit.
  (setq hycontrol--quit-function
(set-transient-map hycontrol-frames-mode-map #'hycontrol-stay-in-mode
   #'hycontrol-exit-mode)))

[-- Attachment #2: Type: text/html, Size: 11256 bytes --]

Re: Problem quitting properly from transient keymap with one keystroke

[Stefan Monnier]

> The problem is that set-transient-map uses pre-command-hook to test the
> stay-in-transient-map function and to exit, so if a keystroke sets the
> flag read by the stay-in-transient-map function to disable the transient
> map, this check is not done until another key is pressed because the
> pre-command-hook is not run again until then (so the code doesn't see
> the flag change until then).

Indeed, that's why I made `set-transient-map` return an "exit function"
which you can call when you want to explicitly leave this transient map
at a different time than when the `keep-pred` predicate is checked.


        Stefan

Re: Problem quitting properly from transient keymap with one keystroke

[Stefan Monnier]

> I can't just add a post-command-hook that calls the transient map
> disable function because calling (keyboard-quit) from the
> post-command-hook triggers an error and I imagine that is not a proper
> usage scenario.

Why do you need a post-command-hook?


        Stefan

Re: Problem quitting properly from transient keymap with one keystroke

[Robert Weiner]

[-- Attachment #1: Type: text/plain, Size: 793 bytes --]

On Fri, Oct 20, 2017 at 8:27 AM, Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> > I can't just add a post-command-hook that calls the transient map
> > disable function because calling (keyboard-quit) from the
> > post-command-hook triggers an error and I imagine that is not a proper
> > usage scenario.
>
> ​​
> Why do you need a post-command-hook?
>
​​
HyControl uses post-command-hook to persist a prefix-argument value across
commands and to continually display a help message in the minibuffer until
exit.
​​
​​Just curious, why does set-transient-map use pre-command-hook instead of
post-command-hook​ to test whether or not to keep the transient-map
enabled?  This prevents the test from using the results of the most recent
key binding run.

[-- Attachment #2: Type: text/html, Size: 1714 bytes --]

Re: Problem quitting properly from transient keymap with one keystroke

[Stefan Monnier]

>> > I can't just add a post-command-hook that calls the transient map
>> > disable function because calling (keyboard-quit) from the
>> > post-command-hook triggers an error and I imagine that is not a proper
>> > usage scenario.
>> Why do you need a post-command-hook?
> HyControl uses post-command-hook to persist a prefix-argument value across
> commands and to continually display a help message in the minibuffer until
> exit.

But neither of those activities seem related to calling the exit
function or calling keyboard-quit.

> ​​Just curious, why does set-transient-map use pre-command-hook instead of
> post-command-hook​ to test whether or not to keep the transient-map
> enabled?

Because we need to know which is the next command before we know whether
to exit or not: E.g. after hitting C-u we don't know yet whether to
exit, it's only once the user hits the next key that we know whether to
exit (e.g. she pressed `a`) or stay (e.g. she pressed `5`).

And when she presses `a`, we need to exit *before* running the command
bound to `a`: sometimes it could be OK to linger on until the end of
the command bound to `a`, but not in general.


        Stefan

Re: Problem quitting properly from transient keymap with one keystroke

[Robert Weiner]

[-- Attachment #1: Type: text/plain, Size: 2104 bytes --]

On Fri, Oct 20, 2017 at 11:13 AM, Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> >> > I can't just add a post-command-hook that calls the transient map
> >> > disable function because calling (keyboard-quit) from the
> >> > post-command-hook triggers an error and I imagine that is not a proper
> >> > usage scenario.
> >> Why do you need a post-command-hook?
> > HyControl uses post-command-hook to persist a prefix-argument value
> across
> > commands and to continually display a help message in the minibuffer
> until
> > exit.
>
> But neither of those activities seem related to calling the exit
> function or calling keyboard-quit.
>

​I thought you were just asking about how this library used
post-command-hook with set-transient-map.

When {C-g} is pressed, we want to execute some actions and then terminate
the transient map.  We could
do this in the {C-g} key binding itself rather than in post-command-hook.
I just tested and that does
indeed solve the problem.  Thanks much.

​​
>
> ​​
> > ​​Just curious, why does set-transient-map use pre-command-hook instead
> of
> ​​
> > post-command-hook​ to test whether or not to keep the transient-map
> ​​
> > enabled?
> ​​
>
> ​​
> Because we need to know which is the next command before we know whether
> ​​
> to exit or not: E.g. after hitting C-u we don't know yet whether to
> ​​
> exit, it's only once the user hits the next key that we know whether to
> ​​
> exit (e.g. she pressed `a`) or stay (e.g. she pressed `5`).
> ​​
>
> ​​
> And when she presses `a`, we need to exit *before* running the command
> ​​
> bound to `a`: sometimes it could be OK to linger on until the end of
> ​​
> the command bound to `a`, but not in general.
>

​I understand.  Would it be possible to make the exit function run on both
pre- and post-command hook so that it could use the results of the command​
when needed and eliminate the need in many cases to call the quit function
manually?  Or would that cause problems?

Bob

[-- Attachment #2: Type: text/html, Size: 4718 bytes --]

Re: Problem quitting properly from transient keymap with one keystroke

[Stefan Monnier]

> ​I understand.  Would it be possible to make the exit function run on both
> pre- and post-command hook so that it could use the results of the command​
> when needed and eliminate the need in many cases to call the quit function
> manually?  Or would that cause problems?

I think it would bump into other problems because the keep-pred
predicate expects to be called from pre-command-hook.

IOW, maybe it would have been a valid design, but it would be an
incompatible change.


        Stefan

Re: Problem quitting properly from transient keymap with one keystroke

[Robert Weiner]

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]

On Fri, Oct 20, 2017 at 1:34 PM, Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> > ​I understand.  Would it be possible to make the exit function run on
> both
> > pre- and post-command hook so that it could use the results of the
> command​
> > when needed and eliminate the need in many cases to call the quit
> function
> > manually?  Or would that cause problems?
>
> I think it would bump into other problems because the keep-pred
> predicate expects to be called from pre-command-hook.
>
> IOW, maybe it would have been a valid design, but it would be an
> incompatible change.
>

​Ok.  I have everything working well now and have been able
to eliminate the need for the on-exit function given to
set-transient-map and instead call the quit function generated
by set-transient-map in each key binding that quits from my mode
(this was happening before but indirectly through the setting of
a status flag).

Bob

[-- Attachment #2: Type: text/html, Size: 2164 bytes --]

Re: Problem quitting properly from transient keymap with one keystroke

[Oleh Krehel]

Hi Bob,

> Everything is working as desired except for disabling the transient map.
> Both C-g and q should disable the transient map and run the on-exit
> function I have defined.

Have you looked at the hydra package on ELPA? It's easy to do what you
describe with `defhydra'.
Initially, it relied on the default `set-transient-map', but now it
uses `hydra-set-transient-map' (which makes some things easier).

regards,
Oleh

Re: Problem quitting properly from transient keymap with one keystroke

[Robert Weiner]

[-- Attachment #1: Type: text/plain, Size: 647 bytes --]

On Fri, Oct 20, 2017 at 8:42 AM, Oleh Krehel <ohwoeowho@gmail.com> wrote:

>
> Have you looked at the hydra package on ELPA? It's easy to do what you
> describe with `defhydra'.
> Initially, it relied on the default `set-transient-map', but now it
> uses `hydra-set-transient-map' (which makes some things easier).
>

​Thanks.  Hydra is at the top of my list for new packages to work with; I
just watched your London talk on it which was very clear.​
It would be nice if you could also offer a mini-hydra like your original
prototype which would be less than say 100 lines and would
be easier to use for very basic needs.

Bob

[-- Attachment #2: Type: text/html, Size: 1639 bytes --]

Re: Problem quitting properly from transient keymap with one keystroke

[Robert Weiner]

[-- Attachment #1: Type: text/plain, Size: 550 bytes --]

On Fri, Oct 20, 2017 at 8:42 AM, Oleh Krehel <ohwoeowho@gmail.com> wrote:

> Hi Bob,
>
> > Everything is working as desired except for disabling the transient map.
> > Both C-g and q should disable the transient map and run the on-exit
> > function I have defined.
>
> Have you looked at the hydra package on ELPA? It's easy to do what you
> describe with `defhydra'.
>

​So, I could I bind {C-g} in my hydra map, execute some functions and then
do a (keyboard-quit)
and have that not trigger an error but just signal a quit?

Bob

[-- Attachment #2: Type: text/html, Size: 1475 bytes --]

Re: Problem quitting properly from transient keymap with one keystroke

[Stefan Monnier]

> Have you looked at the hydra package on ELPA? It's easy to do what you
> describe with `defhydra'.
> Initially, it relied on the default `set-transient-map', but now it
> uses `hydra-set-transient-map' (which makes some things easier).

Could you highlight some of the things that are easier with
hydra-set-transient-map (or more generally what were the problems you
encountered with set-transient-map)?


        Stefan

Re: Problem quitting properly from transient keymap with one keystroke

[Oleh Krehel]

> Could you highlight some of the things that are easier with
> hydra-set-transient-map (or more generally what were the problems you
> encountered with set-transient-map)?

Just some customization logic, see `hydra--clearfun' for more detail.
One example is the `hydra-pause-resume' command: it's not in any
transient map, but needs special interaction.
Another example is issuing a warning in some cases about the transient
map, instead of disabling it.

regards,
Oleh

Re: Problem quitting properly from transient keymap with one keystroke

[Stefan Monnier]

>> Could you highlight some of the things that are easier with
>> hydra-set-transient-map (or more generally what were the problems you
>> encountered with set-transient-map)?
> Just some customization logic, see `hydra--clearfun' for more detail.
> One example is the `hydra-pause-resume' command: it's not in any
> transient map, but needs special interaction.
> Another example is issuing a warning in some cases about the transient
> map, instead of disabling it.

Without looking into the details, I'd think that you can put that
functionality into the `keep-pred` function (and hence implement
hydra-set-transient-map on top of set-transient-map).

But since you don't build on top of set-transient-map, I assume that
doing what I suggest leads to problems.  Right?  What were those?


        Stefan