bug#4093: Overlay keymap and timers

bug#4093: Overlay keymap and timers

[Mihai Bazon]

Actually, it seems it hasn't anything to do with overlays:

(defun my-set-keymap-with-timer ()
  (interactive)
  (run-with-timer 0.5 nil (lambda()
                            (use-local-map my-keymap)
                            (message "Done setting keymap"))))

If we eval the following and M-x my-set-keymap-with-timer, the keymap
will only be enforced after the first keypress (i.e. first press of M-n
says "M-n is undefined", but subsequently it works).

Found another thread about this issue here:

   http://www.mail-archive.com/emacs-pretest-bug@gnu.org/msg12003.html

Stallman says "we should consider that a serious problem". ;-)

Cheers,
-Mihai

bug#4093: Overlay keymap and timers

[Lennart Borgman]

Yes, this is a really bad thing - sometimes, ie when you need to do just that.

Is there any fundamental difficulty with fixing this?


On Sun, Aug 9, 2009 at 3:49 PM, Mihai Bazon<mihai@bazon.net> wrote:
> Actually, it seems it hasn't anything to do with overlays:
>
> (defun my-set-keymap-with-timer ()
>  (interactive)
>  (run-with-timer 0.5 nil (lambda()
>                            (use-local-map my-keymap)
>                            (message "Done setting keymap"))))
>
> If we eval the following and M-x my-set-keymap-with-timer, the keymap
> will only be enforced after the first keypress (i.e. first press of M-n
> says "M-n is undefined", but subsequently it works).
>
> Found another thread about this issue here:
>
>   http://www.mail-archive.com/emacs-pretest-bug@gnu.org/msg12003.html
>
> Stallman says "we should consider that a serious problem". ;-)

bug#4093: Overlay keymap and timers

[Mihai Bazon]

Sorry, looks like I accidentally started a new bug, but this is really a
follow up to
http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=4081

Should be marked as duplicate, or whatever..

Lennart Borgman <lennart.borgman@gmail.com> wrote:
> Yes, this is a really bad thing

But indeed, it is bad...
I'm calling (top-level) for now, but it's not nice.

Cheers,
-Mihai

bug#4093: Overlay keymap and timers

[Stefan Monnier]

> Yes, this is a really bad thing - sometimes, ie when you need to do
> just that.

The code in keyboard.c reads the set of active keymaps before reading
the next event.  That's most likely the explanation for this behavior.

> Is there any fundamental difficulty with fixing this?

It seems the relevant code is trying to do something clever in case of
low-memory situations, so we may bump into surprises, but other than
that, I can't think of any reason why it should be difficult.


        Stefan

bug#4093: Overlay keymap and timers

[Mihai Bazon]

Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> > Yes, this is a really bad thing - sometimes, ie when you need to do
> > just that.
> 
> The code in keyboard.c reads the set of active keymaps before reading
> the next event.  That's most likely the explanation for this behavior.

What you are saying does actually make sense.  If it set the keymap
*before* serving an event, it should work fine.  But I think what
happens is that it sets the keymap *after* executing an event...

I looked at the code but couldn't figure out where is the keymap
computed.  Could you point it out?

Cheers,
-Mihai

bug#4093: Overlay keymap and timers

[Stefan Monnier]

>> > Yes, this is a really bad thing - sometimes, ie when you need to do
>> > just that.
>> The code in keyboard.c reads the set of active keymaps before reading
>> the next event.  That's most likely the explanation for this behavior.
> What you are saying does actually make sense.  If it set the keymap
> *before* serving an event, it should work fine.  But I think what
> happens is that it sets the keymap *after* executing an event...

> I looked at the code but couldn't figure out where is the keymap
> computed.  Could you point it out?

The relevant code is in .... read_key_sequence! (only those who've had
to deal with this function understand the "....").

You'll see that it first collects all the active keyamps (see where it
calls current_minor_maps), and later on calls read_char (which can do
redisplay, run timers, run process filters, etc...).

Basically the problem in the case of changing the keymap from a timer
comes down to:

  what happens if the use presses C-c, then your code runs then the user
  presses C-d:  should the C-c C-d be looked up in the original keymaps
  or in the new keymaps?

you worry about the case where the timer is run before the C-c, but from
Emacs's current point of view, it's no different whether the timer is
run after 1 key-press, or after 2 key-presses, or after 0 key-presses.

We can probably change the code to collect the list of active keymaps
later (e.g. right after the first key-press).
But maybe an alternative is to provide some way for your Elisp code to
cause a jump back to `replay_sequence' so that you can force the C-c C-d
to be interpreted in the new keymaps even if the C-c had already been
pressed when your code was run.

In any case, this function is a monster, so I'll only consider changes
to it if it makes it simpler (typically by moving code out of it into
some new function).  Maybe the idea of a new "need-to-replay-sequence"
variable could be a good way to simplify the code: we could maybe
arrange for most other "goto replay_sequence" to use this new var
(i.e. replace the current code that checks some relevant condition, by
some piece of code elsewhere (where that condition is created) which
sets the var).


        Stefan

bug#4093: Overlay keymap and timers

[Lennart Borgman]

On Mon, Aug 10, 2009 at 6:25 PM, Stefan Monnier<monnier@iro.umontreal.ca> wrote:

> But maybe an alternative is to provide some way for your Elisp code to
> cause a jump back to `replay_sequence' so that you can force the C-c C-d
> to be interpreted in the new keymaps even if the C-c had already been
> pressed when your code was run.

Sounds good. Would it perhaps be most easy to implement a
`please-replay-sequence' function that tells the command loop to go
one more turn and check the keymaps again and reuse the pending input?

bug#4093: Overlay keymap and timers

[Stefan Monnier]

>> But maybe an alternative is to provide some way for your Elisp code to
>> cause a jump back to `replay_sequence' so that you can force the C-c C-d
>> to be interpreted in the new keymaps even if the C-c had already been
>> pressed when your code was run.
> Sounds good.  Would it perhaps be most easy to implement
> a `please-replay-sequence' function that tells the command loop to go
> one more turn and check the keymaps again and reuse the pending input?

That function couldn't do much more than set a variable and return.
So yes, it could be done and would have the advantage of making it
possible to keep the variable hidden from Elisp.  Not sure if that
advantage is of any significance, tho.


        Stefan

bug#4093: Overlay keymap and timers

[Lennart Borgman]

On Mon, Aug 10, 2009 at 7:38 PM, Stefan Monnier<monnier@iro.umontreal.ca> wrote:
>>> But maybe an alternative is to provide some way for your Elisp code to
>>> cause a jump back to `replay_sequence' so that you can force the C-c C-d
>>> to be interpreted in the new keymaps even if the C-c had already been
>>> pressed when your code was run.
>> Sounds good.  Would it perhaps be most easy to implement
>> a `please-replay-sequence' function that tells the command loop to go
>> one more turn and check the keymaps again and reuse the pending input?
>
> That function couldn't do much more than set a variable and return.
> So yes, it could be done and would have the advantage of making it
> possible to keep the variable hidden from Elisp.  Not sure if that
> advantage is of any significance, tho.

You mean it is the same approach as you were thinking of? Then I think
the "active voice" used to call the function makes it a bit easier to
understand, but maybe that is just me. Also you can't really doubt how
to use it.

bug#4093: Overlay keymap and timers

[Stefan Monnier]

> You mean it is the same approach as you were thinking of?

Pretty much.  I was thinking of (setq need-to-replay-sequence t) instead.

> Then I think the "active voice" used to call the function makes it
> a bit easier to understand, but maybe that is just me.

To me it sounds a bit misleading since the replay does not happen during
the function call.  But `force-mode-line-update' does it like you
propose, so it's not that different.

> Also you can't really doubt how to use it.

Indeed, especially if it doesn't take any arguments ;-).


        Stefan

bug#4093: Overlay keymap and timers

[Lennart Borgman]

On Mon, Aug 10, 2009 at 8:17 PM, Stefan Monnier<monnier@iro.umontreal.ca> wrote:
>> You mean it is the same approach as you were thinking of?
>
> Pretty much.  I was thinking of (setq need-to-replay-sequence t) instead.
>
>> Then I think the "active voice" used to call the function makes it
>> a bit easier to understand, but maybe that is just me.
>
> To me it sounds a bit misleading since the replay does not happen during
> the function call.  But `force-mode-line-update' does it like you
> propose, so it's not that different.


Maybe a better name. But this is of course not as important as the feature.

I do not think that there are many people who dares to touch this part
of the code, but I may be misunderstanding that totally...

bug#4093: Overlay keymap and timers

[Mihai Bazon]

Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> 
> Basically the problem in the case of changing the keymap from a timer
> comes down to:
> 
>   what happens if the use presses C-c, then your code runs then the user
>   presses C-d:  should the C-c C-d be looked up in the original keymaps
>   or in the new keymaps?

That's a good question.  As an user, if I start typing a combined key
binding then I would expect it to work regardless of whatever happened
in a timer in between.  So indeed, it can be a problem to allow a timer
to set a new keymap in this case.

But if there's no prefix key already typed, it would be great if
use-local-map actually did what it's supposed to do, even when ran with
a timer.

Perhaps the proper solution would involve saving the current keymap when
a prefix key has been pressed, and use that keymap until it results in a
command or it's canceled.  If a timer changes the keymap in between, the
new keymap should apply only after any pending key binding has been
finished.  Not sure if this is doable...

I'm sorry I don't have any knowledge of Emacs internals, and only
limited Lisp knowledge anyway, but I'd love to see a decent work-around
for this...

Cheers,
-Mihai

bug#4093: Overlay keymap and timers

[Mihai Bazon]

Indeed, that function looks frightening.

It's worth noting that XEmacs doesn't have this problem.
But the code differs drastically..  Hard to figure out a simple fix.

-M

Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> >> > Yes, this is a really bad thing - sometimes, ie when you need to do
> >> > just that.
> >> The code in keyboard.c reads the set of active keymaps before reading
> >> the next event.  That's most likely the explanation for this behavior.
> > What you are saying does actually make sense.  If it set the keymap
> > *before* serving an event, it should work fine.  But I think what
> > happens is that it sets the keymap *after* executing an event...
> 
> > I looked at the code but couldn't figure out where is the keymap
> > computed.  Could you point it out?
> 
> The relevant code is in .... read_key_sequence! (only those who've had
> to deal with this function understand the "....").
> 
> You'll see that it first collects all the active keyamps (see where it
> calls current_minor_maps), and later on calls read_char (which can do
> redisplay, run timers, run process filters, etc...).
> 
> Basically the problem in the case of changing the keymap from a timer
> comes down to:
> 
>   what happens if the use presses C-c, then your code runs then the user
>   presses C-d:  should the C-c C-d be looked up in the original keymaps
>   or in the new keymaps?
> 
> you worry about the case where the timer is run before the C-c, but from
> Emacs's current point of view, it's no different whether the timer is
> run after 1 key-press, or after 2 key-presses, or after 0 key-presses.
> 
> We can probably change the code to collect the list of active keymaps
> later (e.g. right after the first key-press).
> But maybe an alternative is to provide some way for your Elisp code to
> cause a jump back to `replay_sequence' so that you can force the C-c C-d
> to be interpreted in the new keymaps even if the C-c had already been
> pressed when your code was run.
> 
> In any case, this function is a monster, so I'll only consider changes
> to it if it makes it simpler (typically by moving code out of it into
> some new function).  Maybe the idea of a new "need-to-replay-sequence"
> variable could be a good way to simplify the code: we could maybe
> arrange for most other "goto replay_sequence" to use this new var
> (i.e. replace the current code that checks some relevant condition, by
> some piece of code elsewhere (where that condition is created) which
> sets the var).
> 
> 
>         Stefan

bug#4093: Overlay keymap and timers

[Stefan Monnier]

> Indeed, that function looks frightening.

I hoped you'd find it challenging.  Oh well!

> It's worth noting that XEmacs doesn't have this problem.

IIRC XEmacs's equivalent function restarts the whole work after every
event, so they look up the keymaps anew, and look them all up anew
(kind of like always jumping back to replay_sequence except that it
additionally recomputes the active keymaps).

> But the code differs drastically..

Yes, the algorithm is completely different.
I have a feeling it's related to the "frightening" above... nah!


        Stefan

bug#4093: Overlay keymap and timers

[Mihai Bazon]

Stefan Monnier <monnier@IRO.UMontreal.CA> wrote:
> > Indeed, that function looks frightening.
> 
> I hoped you'd find it challenging.  Oh well!

I'd love to find it challenging, but I don't have time for this,
sorry... :-(

If I felt competitive enough to fix it within hours, or even days, I'd
get on it.  But my C skills, though once pretty good, are long forgotten
-- not to mention that that code requires knowledge far beyond "C", you
actually have to know some intimate Emacs internals.

We'll live with it.

-M.