unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Strange behavior of C-u in the presence of sit-for in p-c-h
@ 2006-10-14 16:44 Stefan Monnier
  2006-10-16 13:50 ` Richard Stallman
  0 siblings, 1 reply; 25+ messages in thread
From: Stefan Monnier @ 2006-10-14 16:44 UTC (permalink / raw)



Try the following:

  emacs -Q --eval "(add-hook 'post-command-hook (lambda () (sit-for 0.5)))"

you'll notice that all works apparently well, except that if you hit the
universal prefix C-u, then any key pressed before the 0.5s delay will
be ignored, and after the 0.5s delay you end up with "C-u-" in the
echo area.


        Stefan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-14 16:44 Strange behavior of C-u in the presence of sit-for in p-c-h Stefan Monnier
@ 2006-10-16 13:50 ` Richard Stallman
  2006-10-16 16:04   ` Stefan Monnier
  0 siblings, 1 reply; 25+ messages in thread
From: Richard Stallman @ 2006-10-16 13:50 UTC (permalink / raw)
  Cc: emacs-devel

Can you debug this?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-16 13:50 ` Richard Stallman
@ 2006-10-16 16:04   ` Stefan Monnier
  2006-10-17  6:54     ` Richard Stallman
  0 siblings, 1 reply; 25+ messages in thread
From: Stefan Monnier @ 2006-10-16 16:04 UTC (permalink / raw)
  Cc: emacs-devel

> Can you debug this?

I don't have time right now.  I looked at it briefly and decided it wasn't
obvious enough.  I must say I haven't followed the fit-for development close
enough to have a good feel for what the problem may be.


        Stefan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-16 16:04   ` Stefan Monnier
@ 2006-10-17  6:54     ` Richard Stallman
  2006-10-17 14:50       ` Chong Yidong
  0 siblings, 1 reply; 25+ messages in thread
From: Richard Stallman @ 2006-10-17  6:54 UTC (permalink / raw)
  Cc: emacs-devel

Can someone else please debug this?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-17  6:54     ` Richard Stallman
@ 2006-10-17 14:50       ` Chong Yidong
  2006-10-17 21:11         ` Chong Yidong
                           ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Chong Yidong @ 2006-10-17 14:50 UTC (permalink / raw)
  Cc: Stefan Monnier, emacs-devel

Here is an easy way to reproduce the underlying bug:

(defun foo ()
  (interactive)
  (universal-argument)
  (push ?a unread-command-events))

M-x foo RET

The expected result is aaaa, but in fact nothing is printed.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-17 14:50       ` Chong Yidong
@ 2006-10-17 21:11         ` Chong Yidong
  2006-10-17 22:12           ` Stefan Monnier
  2006-10-18  1:57           ` Stefan Monnier
  2006-10-19 21:09         ` Kim F. Storm
  2006-10-19 21:09         ` Kim F. Storm
  2 siblings, 2 replies; 25+ messages in thread
From: Chong Yidong @ 2006-10-17 21:11 UTC (permalink / raw)
  Cc: Stefan Monnier, emacs-devel

Chong Yidong <cyd@stupidchicken.com> writes:

> Here is an easy way to reproduce the underlying bug:
>
> (defun foo ()
>   (interactive)
>   (universal-argument)
>   (push ?a unread-command-events))
>
> M-x foo RET
>
> The expected result is aaaa, but in fact nothing is printed.

The problem here is that C-u works through the command
`universal-argument-other-key', which sets prefix-arg and then adds
(this-command-keys) to unread-command-events so that it is executed
with that prefix argument.

This normally works ok.  However, suppose the call to
read_key_sequence in the command loop takes its input from
unread-command-events.  (This happens if the input came during a
sit-for in post-command-hook, as in Stefan's original example, or
directly as in the example above.)  This kind of "reread input" is
explicitly *not* added to this_command_keys.  The rationale for this
is not clear to me, but there may be a good reason since the code
explicitly checks for this; see keyboard.c:789.  Then
`universal-argument-other-key' can't see that input.

OTOH, I don't see a safe way of fixing this.  Any suggestions?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-17 21:11         ` Chong Yidong
@ 2006-10-17 22:12           ` Stefan Monnier
  2006-10-17 22:18             ` Chong Yidong
  2006-10-18  1:57           ` Stefan Monnier
  1 sibling, 1 reply; 25+ messages in thread
From: Stefan Monnier @ 2006-10-17 22:12 UTC (permalink / raw)
  Cc: rms, emacs-devel

> This normally works ok.  However, suppose the call to
> read_key_sequence in the command loop takes its input from
> unread-command-events.  (This happens if the input came during a
> sit-for in post-command-hook, as in Stefan's original example, or
> directly as in the example above.)  This kind of "reread input" is
> explicitly *not* added to this_command_keys.  The rationale for this
> is not clear to me, but there may be a good reason since the code
> explicitly checks for this; see keyboard.c:789.  Then
> `universal-argument-other-key' can't see that input.

I think I understand, but I can't find the code in keyboard.c.
Do you really mean "line 789"?  Of which revision?


        Stefan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-17 22:12           ` Stefan Monnier
@ 2006-10-17 22:18             ` Chong Yidong
  0 siblings, 0 replies; 25+ messages in thread
From: Chong Yidong @ 2006-10-17 22:18 UTC (permalink / raw)
  Cc: rms, emacs-devel

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

>> This normally works ok.  However, suppose the call to
>> read_key_sequence in the command loop takes its input from
>> unread-command-events.  (This happens if the input came during a
>> sit-for in post-command-hook, as in Stefan's original example, or
>> directly as in the example above.)  This kind of "reread input" is
>> explicitly *not* added to this_command_keys.  The rationale for this
>> is not clear to me, but there may be a good reason since the code
>> explicitly checks for this; see keyboard.c:789.  Then
>> `universal-argument-other-key' can't see that input.
>
> I think I understand, but I can't find the code in keyboard.c.
> Do you really mean "line 789"?  Of which revision?

Sorry; by 789, I mean 3262 :-P

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-17 21:11         ` Chong Yidong
  2006-10-17 22:12           ` Stefan Monnier
@ 2006-10-18  1:57           ` Stefan Monnier
  2006-10-18  2:19             ` Chong Yidong
  2006-10-18 17:54             ` Richard Stallman
  1 sibling, 2 replies; 25+ messages in thread
From: Stefan Monnier @ 2006-10-18  1:57 UTC (permalink / raw)
  Cc: rms, emacs-devel

> This normally works ok.  However, suppose the call to
> read_key_sequence in the command loop takes its input from
> unread-command-events.  (This happens if the input came during a
> sit-for in post-command-hook, as in Stefan's original example, or
> directly as in the example above.)  This kind of "reread input" is
> explicitly *not* added to this_command_keys.  The rationale for this
> is not clear to me, but there may be a good reason since the code
> explicitly checks for this; see keyboard.c:789.  Then
> `universal-argument-other-key' can't see that input.

> OTOH, I don't see a safe way of fixing this.  Any suggestions?

The check on line 3262 seems odd.  If we trace it back it was introduced in
revision 1.489 in the following form:

    if (this_command_key_count == 0 || ! reread)

the log comment doesn't say much useful:

    (read_char): Call the input method if appropriate.
    Change logic for distinguishing rereads from new events;
    use local var `reread'.  Take events from
    Vunread_input_method_events and Vunread_post_input_method_events.

I'm not sure exactly what this was trying to avoid, but the test looks odd.
Maybe the "!reread" could make some sense, but the
"this_command_key_count==0" looks positively odd since it may end up
recording the first (and only the first) of a sequence of keys.

Maybe the problem is that `this-command-keys' has several potential uses and
they are incompatible: in one case one wants this-command-keys to list the
keys the user has typed (independently from whether or not some of those
keys were later read&unread&reread&reunread&rereread), whereas in the other
one wants the exact key-sequence which triggered this command, so we can
push it back on unread-command-events to force re-interpretation of
those keys.


        Stefan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-18  1:57           ` Stefan Monnier
@ 2006-10-18  2:19             ` Chong Yidong
  2006-10-18  5:57               ` Stefan Monnier
  2006-10-18 17:54             ` Richard Stallman
  1 sibling, 1 reply; 25+ messages in thread
From: Chong Yidong @ 2006-10-18  2:19 UTC (permalink / raw)
  Cc: rms, emacs-devel

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

> Maybe the problem is that `this-command-keys' has several potential uses and
> they are incompatible: in one case one wants this-command-keys to list the
> keys the user has typed (independently from whether or not some of those
> keys were later read&unread&reread&reunread&rereread), whereas in the other
> one wants the exact key-sequence which triggered this command, so we can
> push it back on unread-command-events to force re-interpretation of
> those keys.

Given that the code has been around for 8 years, changing it at this
point in the release process might be a disaster.  It's not even clear
to me what TRT is in this case.  I suggest leaving the current
behavior as it is for Emacs 22; any user-level Lisp code that bumps
into this can quite easily work around it by checking if this-command
is eq to universal-argument.

Then maybe we can revisit this issue after the release.  WDYT?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-18  2:19             ` Chong Yidong
@ 2006-10-18  5:57               ` Stefan Monnier
  2006-10-18 14:28                 ` Chong Yidong
  0 siblings, 1 reply; 25+ messages in thread
From: Stefan Monnier @ 2006-10-18  5:57 UTC (permalink / raw)
  Cc: rms, emacs-devel

> Given that the code has been around for 8 years, changing it at this
> point in the release process might be a disaster.  It's not even clear
> to me what TRT is in this case.  I suggest leaving the current
> behavior as it is for Emacs 22; any user-level Lisp code that bumps
> into this can quite easily work around it by checking if this-command
> is eq to universal-argument.

Huh?  The problem came up because of a minor mode who uses post-command-hook
with a sit-for inside.  How do you suggest to work around this problem?


        Stefan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-18  5:57               ` Stefan Monnier
@ 2006-10-18 14:28                 ` Chong Yidong
  2006-10-18 15:23                   ` Stefan Monnier
                                     ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Chong Yidong @ 2006-10-18 14:28 UTC (permalink / raw)
  Cc: rms, emacs-devel

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

>> Given that the code has been around for 8 years, changing it at
>> this point in the release process might be a disaster.  It's not
>> even clear to me what TRT is in this case.  I suggest leaving the
>> current behavior as it is for Emacs 22; any user-level Lisp code
>> that bumps into this can quite easily work around it by checking if
>> this-command is eq to universal-argument.
>
> Huh?  The problem came up because of a minor mode who uses
> post-command-hook with a sit-for inside.  How do you suggest to work
> around this problem?

Which minor mode is that?  It could avoid performing the sit-for
during universal-argument:

  (unless (eq this-command 'universal-argument)
    (sit-for 0.5))

Or can you think of a safe way to change the keyboard.c code?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-18 14:28                 ` Chong Yidong
@ 2006-10-18 15:23                   ` Stefan Monnier
  2006-10-19  6:41                     ` Richard Stallman
  2006-10-19  6:41                   ` Richard Stallman
  2006-10-19 14:51                   ` Johan Bockgård
  2 siblings, 1 reply; 25+ messages in thread
From: Stefan Monnier @ 2006-10-18 15:23 UTC (permalink / raw)
  Cc: rms, emacs-devel

> Which minor mode is that?

haskell-doc-mode

> It could avoid performing the sit-for during universal-argument:
>   (unless (eq this-command 'universal-argument)
>     (sit-for 0.5))

Oh, I see.  Well, in the case of haskell-doc-mode, the better fix is to use
an idle-timer rather than a post-command-hook.  But it's still a weird bug
introduced in Emacs-22 (it was there before, but the sit-for changes made
it appear in more cases).

> Or can you think of a safe way to change the keyboard.c code?

Add a parameter to this-command-keys which, if non-nil causes it to return
all keys, even those that were `reread'.
Then use this new parameter in universal-argument.


        Stefan

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-18  1:57           ` Stefan Monnier
  2006-10-18  2:19             ` Chong Yidong
@ 2006-10-18 17:54             ` Richard Stallman
  2006-10-18 23:57               ` Chong Yidong
  1 sibling, 1 reply; 25+ messages in thread
From: Richard Stallman @ 2006-10-18 17:54 UTC (permalink / raw)
  Cc: cyd, emacs-devel

    I'm not sure exactly what this was trying to avoid, but the test looks odd.
    Maybe the "!reread" could make some sense, but the
    "this_command_key_count==0" looks positively odd since it may end up
    recording the first (and only the first) of a sequence of keys.

I designed that test to do the right thing in the cases that were
encountered, and both conditions proved to be necessary.

!reread is the usual case of reading a new character.

this_command_key_count==0 is the case you get when a command reads
input with read-event and then unreads it.  This usually IS the first
event of a key sequence; it is supposed to be.

To really do the right thing, we would need to associate each unread
key with a flag indicating whether it was previously added to
this-command-keys.  That would be a very painful incompatibility, so I
never wanted to do it, and I don't want to do it now.

    Maybe the problem is that `this-command-keys' has several potential uses and
    they are incompatible: in one case one wants this-command-keys to list the
    keys the user has typed (independently from whether or not some of those
    keys were later read&unread&reread&reunread&rereread), whereas in the other
    one wants the exact key-sequence which triggered this command, so we can
    push it back on unread-command-events to force re-interpretation of
    those keys.

I think that is true.

I suggest that we add a new primitive that does precisely what
`universal-argument-other-key' needs, and use it there.  That will be
safe, in that the change can't break anything else.

Does anyone disagree?  Would someone like to do this?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-18 17:54             ` Richard Stallman
@ 2006-10-18 23:57               ` Chong Yidong
  2006-10-19  9:01                 ` Kim F. Storm
  0 siblings, 1 reply; 25+ messages in thread
From: Chong Yidong @ 2006-10-18 23:57 UTC (permalink / raw)
  Cc: Stefan Monnier, emacs-devel

Richard Stallman <rms@gnu.org> writes:

> To really do the right thing, we would need to associate each unread
> key with a flag indicating whether it was previously added to
> this-command-keys.  That would be a very painful incompatibility, so I
> never wanted to do it, and I don't want to do it now.
>
>     Maybe the problem is that `this-command-keys' has several potential uses and
>     they are incompatible: in one case one wants this-command-keys to list the
>     keys the user has typed (independently from whether or not some of those
>     keys were later read&unread&reread&reunread&rereread), whereas in the other
>     one wants the exact key-sequence which triggered this command, so we can
>     push it back on unread-command-events to force re-interpretation of
>     those keys.
>
> I think that is true.
>
> I suggest that we add a new primitive that does precisely what
> `universal-argument-other-key' needs, and use it there.  That will be
> safe, in that the change can't break anything else.
>
> Does anyone disagree?

This would involve lugging around an extra set of variables, exactly
like `this_command_keys' and `this_command_key_count' except reread
events get updated there too.  Sounds ugly.

How about a variable that tells the command loop to re-insert reread
commands into `this-command-keys' until the next command?  The command
loop could automagically reset this at the start of the loop, in case
of a quit.  Then universal-argument could toggle this variable.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-18 14:28                 ` Chong Yidong
  2006-10-18 15:23                   ` Stefan Monnier
@ 2006-10-19  6:41                   ` Richard Stallman
  2006-10-19 14:31                     ` Chong Yidong
  2006-10-19 14:51                   ` Johan Bockgård
  2 siblings, 1 reply; 25+ messages in thread
From: Richard Stallman @ 2006-10-19  6:41 UTC (permalink / raw)
  Cc: monnier, emacs-devel

    Which minor mode is that?  It could avoid performing the sit-for
    during universal-argument:

      (unless (eq this-command 'universal-argument)
	(sit-for 0.5))

If that works, I'd rather do that
than make any change at a lower level.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-18 15:23                   ` Stefan Monnier
@ 2006-10-19  6:41                     ` Richard Stallman
  0 siblings, 0 replies; 25+ messages in thread
From: Richard Stallman @ 2006-10-19  6:41 UTC (permalink / raw)
  Cc: cyd, emacs-devel

    Add a parameter to this-command-keys which, if non-nil causes it to return
    all keys, even those that were `reread'.

That would be hard to do, since there's only one data structure
and the keys that you don't get are not in the data structure.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-18 23:57               ` Chong Yidong
@ 2006-10-19  9:01                 ` Kim F. Storm
  0 siblings, 0 replies; 25+ messages in thread
From: Kim F. Storm @ 2006-10-19  9:01 UTC (permalink / raw)
  Cc: emacs-devel, rms, Stefan Monnier

Chong Yidong <cyd@stupidchicken.com> writes:

> How about a variable that tells the command loop to re-insert reread
> commands into `this-command-keys' until the next command?  The command
> loop could automagically reset this at the start of the loop, in case
> of a quit.  Then universal-argument could toggle this variable.

Sounds like a good way to DTRT at this stage...

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-19  6:41                   ` Richard Stallman
@ 2006-10-19 14:31                     ` Chong Yidong
  0 siblings, 0 replies; 25+ messages in thread
From: Chong Yidong @ 2006-10-19 14:31 UTC (permalink / raw)
  Cc: monnier, emacs-devel

Richard Stallman <rms@gnu.org> writes:

>     Which minor mode is that?  It could avoid performing the sit-for
>     during universal-argument:
>
>       (unless (eq this-command 'universal-argument)
> 	(sit-for 0.5))
>
> If that works, I'd rather do that
> than make any change at a lower level.

The minor mode that reported contains the code, haskell-doc-mode, is
not distributed with Emacs; and as Stefan noted it's more correct for
it to use an idle-timer.

I suggest not making a change to Emacs at this stage.  I could add a
TODO item about "implementing a new primitive that is like
this-command-keys only it accepts reread events", and look into it
after the release.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-18 14:28                 ` Chong Yidong
  2006-10-18 15:23                   ` Stefan Monnier
  2006-10-19  6:41                   ` Richard Stallman
@ 2006-10-19 14:51                   ` Johan Bockgård
  2 siblings, 0 replies; 25+ messages in thread
From: Johan Bockgård @ 2006-10-19 14:51 UTC (permalink / raw)


Chong Yidong <cyd@stupidchicken.com> writes:

>  It could avoid performing the sit-for during universal-argument:
>
>   (unless (eq this-command 'universal-argument)
>     (sit-for 0.5))

Ditto for universal-argument-more, negative-argument, digit-argument,
etc...

-- 
Johan Bockgård

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-17 14:50       ` Chong Yidong
  2006-10-17 21:11         ` Chong Yidong
@ 2006-10-19 21:09         ` Kim F. Storm
  2006-10-19 21:25           ` Chong Yidong
  2006-10-19 21:09         ` Kim F. Storm
  2 siblings, 1 reply; 25+ messages in thread
From: Kim F. Storm @ 2006-10-19 21:09 UTC (permalink / raw)
  Cc: emacs-devel, rms, Stefan Monnier

Chong Yidong <cyd@stupidchicken.com> writes:

> Here is an easy way to reproduce the underlying bug:
>
> (defun foo ()
>   (interactive)
>   (universal-argument)
>   (push ?a unread-command-events))
>
> M-x foo RET
>
> The expected result is aaaa, but in fact nothing is printed.


Here is a simple patch which fixes the problem AFAICS.

It uses a special event code (t . EVENT) in unread-command-events
to specify that EVENT is _not_ reread.  Of course, this should be
documented in unread-command-events if people think this fix is ok.


*** keyboard.c	10 Oct 2006 10:33:25 +0200	1.878
--- keyboard.c	19 Oct 2006 23:00:23 +0200	
***************
*** 2537,2542 ****
--- 2537,2554 ----
        c = XCAR (Vunread_command_events);
        Vunread_command_events = XCDR (Vunread_command_events);
  
+       reread = 1;
+ 
+       /* Undo what sit-for did when it unread additional keys
+ 	 inside universal-argument.  */
+ 
+       if (CONSP (c)
+ 	  && EQ (XCAR (c), Qt))
+ 	{
+ 	  reread = 0;
+ 	  c = XCDR (c);
+ 	}
+ 
        /* Undo what read_char_x_menu_prompt did when it unread
  	 additional keys returned by Fx_popup_menu.  */
        if (CONSP (c)
***************
*** 2550,2556 ****
  	  && (EQ (c, Qtool_bar) || EQ (c, Qmenu_bar)))
  	*used_mouse_menu = 1;
  
-       reread = 1;
        goto reread_for_input_method;
      }
  
--- 2562,2567 ----


Index: subr.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/subr.el,v
retrieving revision 1.530
diff -c -r1.530 subr.el
*** subr.el	18 Oct 2006 10:56:46 -0000	1.530
--- subr.el	19 Oct 2006 21:08:24 -0000
***************
*** 1752,1759 ****
      (or nodisp (redisplay))
      (let ((read (read-event nil nil seconds)))
        (or (null read)
! 	  (progn (push read unread-command-events)
! 		 nil))))))
  \f
  ;;; Atomic change groups.
  
--- 1758,1768 ----
      (or nodisp (redisplay))
      (let ((read (read-event nil nil seconds)))
        (or (null read)
! 	  (progn
! 	    (if (eq overriding-terminal-local-map universal-argument-map)
! 		(setq read (cons t read)))
! 	    (push read unread-command-events)
! 	    nil))))))
  \f
  ;;; Atomic change groups.
  

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-17 14:50       ` Chong Yidong
  2006-10-17 21:11         ` Chong Yidong
  2006-10-19 21:09         ` Kim F. Storm
@ 2006-10-19 21:09         ` Kim F. Storm
  2006-10-21  2:03           ` Richard Stallman
  2 siblings, 1 reply; 25+ messages in thread
From: Kim F. Storm @ 2006-10-19 21:09 UTC (permalink / raw)
  Cc: emacs-devel, rms, Stefan Monnier


Chong Yidong <cyd@stupidchicken.com> writes:

> Here is an easy way to reproduce the underlying bug:
>
> (defun foo ()
>   (interactive)
>   (universal-argument)
>   (push ?a unread-command-events))
>
> M-x foo RET
>
> The expected result is aaaa, but in fact nothing is printed.


Here is a simple patch which fixes the problem AFAICS.

It uses a special event code (t . EVENT) in unread-command-events
to specify that EVENT is _not_ reread.  Of course, this should be
documented in unread-command-events if people think this fix is ok.


*** keyboard.c	10 Oct 2006 10:33:25 +0200	1.878
--- keyboard.c	19 Oct 2006 23:00:23 +0200	
***************
*** 2537,2542 ****
--- 2537,2554 ----
        c = XCAR (Vunread_command_events);
        Vunread_command_events = XCDR (Vunread_command_events);
  
+       reread = 1;
+ 
+       /* Undo what sit-for did when it unread additional keys
+ 	 inside universal-argument.  */
+ 
+       if (CONSP (c)
+ 	  && EQ (XCAR (c), Qt))
+ 	{
+ 	  reread = 0;
+ 	  c = XCDR (c);
+ 	}
+ 
        /* Undo what read_char_x_menu_prompt did when it unread
  	 additional keys returned by Fx_popup_menu.  */
        if (CONSP (c)
***************
*** 2550,2556 ****
  	  && (EQ (c, Qtool_bar) || EQ (c, Qmenu_bar)))
  	*used_mouse_menu = 1;
  
-       reread = 1;
        goto reread_for_input_method;
      }
  
--- 2562,2567 ----


Index: subr.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/subr.el,v
retrieving revision 1.530
diff -c -r1.530 subr.el
*** subr.el	18 Oct 2006 10:56:46 -0000	1.530
--- subr.el	19 Oct 2006 21:08:24 -0000
***************
*** 1752,1759 ****
      (or nodisp (redisplay))
      (let ((read (read-event nil nil seconds)))
        (or (null read)
! 	  (progn (push read unread-command-events)
! 		 nil))))))
  \f
  ;;; Atomic change groups.
  
--- 1758,1768 ----
      (or nodisp (redisplay))
      (let ((read (read-event nil nil seconds)))
        (or (null read)
! 	  (progn
! 	    (if (eq overriding-terminal-local-map universal-argument-map)
! 		(setq read (cons t read)))
! 	    (push read unread-command-events)
! 	    nil))))))
  \f
  ;;; Atomic change groups.
  

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-19 21:09         ` Kim F. Storm
@ 2006-10-19 21:25           ` Chong Yidong
  0 siblings, 0 replies; 25+ messages in thread
From: Chong Yidong @ 2006-10-19 21:25 UTC (permalink / raw)
  Cc: emacs-devel, rms, Stefan Monnier

storm@cua.dk (Kim F. Storm) writes:

> Here is a simple patch which fixes the problem AFAICS.
>
> It uses a special event code (t . EVENT) in unread-command-events
> to specify that EVENT is _not_ reread.  Of course, this should be
> documented in unread-command-events if people think this fix is ok.

I think it's an excellent idea.

> *** keyboard.c	10 Oct 2006 10:33:25 +0200	1.878
> --- keyboard.c	19 Oct 2006 23:00:23 +0200	
> ***************
> *** 2537,2542 ****
> --- 2537,2554 ----
>         c = XCAR (Vunread_command_events);
>         Vunread_command_events = XCDR (Vunread_command_events);
>   
> +       reread = 1;
> + 
> +       /* Undo what sit-for did when it unread additional keys
> + 	 inside universal-argument.  */
> + 
> +       if (CONSP (c)
> + 	  && EQ (XCAR (c), Qt))
> + 	{
> + 	  reread = 0;
> + 	  c = XCDR (c);
> + 	}
> + 
>         /* Undo what read_char_x_menu_prompt did when it unread
>   	 additional keys returned by Fx_popup_menu.  */
>         if (CONSP (c)
> ***************
> *** 2550,2556 ****
>   	  && (EQ (c, Qtool_bar) || EQ (c, Qmenu_bar)))
>   	*used_mouse_menu = 1;
>   
> -       reread = 1;
>         goto reread_for_input_method;
>       }
>   
> --- 2562,2567 ----
>
>
> Index: subr.el
> ===================================================================
> RCS file: /cvsroot/emacs/emacs/lisp/subr.el,v
> retrieving revision 1.530
> diff -c -r1.530 subr.el
> *** subr.el	18 Oct 2006 10:56:46 -0000	1.530
> --- subr.el	19 Oct 2006 21:08:24 -0000
> ***************
> *** 1752,1759 ****
>       (or nodisp (redisplay))
>       (let ((read (read-event nil nil seconds)))
>         (or (null read)
> ! 	  (progn (push read unread-command-events)
> ! 		 nil))))))
>   \f
>   ;;; Atomic change groups.
>   
> --- 1758,1768 ----
>       (or nodisp (redisplay))
>       (let ((read (read-event nil nil seconds)))
>         (or (null read)
> ! 	  (progn
> ! 	    (if (eq overriding-terminal-local-map universal-argument-map)
> ! 		(setq read (cons t read)))
> ! 	    (push read unread-command-events)
> ! 	    nil))))))
>   \f
>   ;;; Atomic change groups.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-19 21:09         ` Kim F. Storm
@ 2006-10-21  2:03           ` Richard Stallman
  2006-10-22 22:54             ` Kim F. Storm
  0 siblings, 1 reply; 25+ messages in thread
From: Richard Stallman @ 2006-10-21  2:03 UTC (permalink / raw)
  Cc: cyd, monnier, emacs-devel

Your approach is very good-- what I thought would be unacceptably
incompatible, you've made acceptable.  Please install your patch
and document it.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: Strange behavior of C-u in the presence of sit-for in p-c-h
  2006-10-21  2:03           ` Richard Stallman
@ 2006-10-22 22:54             ` Kim F. Storm
  0 siblings, 0 replies; 25+ messages in thread
From: Kim F. Storm @ 2006-10-22 22:54 UTC (permalink / raw)
  Cc: cyd, monnier, emacs-devel

Richard Stallman <rms@gnu.org> writes:

> Your approach is very good-- what I thought would be unacceptably
> incompatible, you've made acceptable.  Please install your patch
> and document it.

Done.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2006-10-22 22:54 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-10-14 16:44 Strange behavior of C-u in the presence of sit-for in p-c-h Stefan Monnier
2006-10-16 13:50 ` Richard Stallman
2006-10-16 16:04   ` Stefan Monnier
2006-10-17  6:54     ` Richard Stallman
2006-10-17 14:50       ` Chong Yidong
2006-10-17 21:11         ` Chong Yidong
2006-10-17 22:12           ` Stefan Monnier
2006-10-17 22:18             ` Chong Yidong
2006-10-18  1:57           ` Stefan Monnier
2006-10-18  2:19             ` Chong Yidong
2006-10-18  5:57               ` Stefan Monnier
2006-10-18 14:28                 ` Chong Yidong
2006-10-18 15:23                   ` Stefan Monnier
2006-10-19  6:41                     ` Richard Stallman
2006-10-19  6:41                   ` Richard Stallman
2006-10-19 14:31                     ` Chong Yidong
2006-10-19 14:51                   ` Johan Bockgård
2006-10-18 17:54             ` Richard Stallman
2006-10-18 23:57               ` Chong Yidong
2006-10-19  9:01                 ` Kim F. Storm
2006-10-19 21:09         ` Kim F. Storm
2006-10-19 21:25           ` Chong Yidong
2006-10-19 21:09         ` Kim F. Storm
2006-10-21  2:03           ` Richard Stallman
2006-10-22 22:54             ` Kim F. Storm

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).