Idle timers with the active minibuffer

Idle timers with the active minibuffer

[Juri Linkov]

Why idle timers don't run when the minibuffer is activated?

I discovered this behavior when tried using isearch in the original
buffer after switching to it from the minibuffer without exiting it.
The idle timer for lazy-highlighting doesn't run.  Steps to reproduce:

1. activate the minibuffer by e.g. `M-:'
2. switch back to the original buffer by `C-x o'
3. start isearch C-s and search for a string that have multiple
occurrences in the original buffer.

Other occurrences are not highlighted by lazy-highlight face.
Putting a breakpoint on isearch-lazy-highlight-update shows
that run-with-idle-timer from isearch-lazy-highlight-new-loop
never runs it when the minibuffer is not yet exited.

Is it an intentional behavior or a bug?

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: Idle timers with the active minibuffer

[Johan Bockgård]

Juri Linkov <juri@jurta.org> writes:

> Why idle timers don't run when the minibuffer is activated?
>
> I discovered this behavior when tried using isearch in the original
> buffer after switching to it from the minibuffer without exiting it.
> The idle timer for lazy-highlighting doesn't run.  Steps to reproduce:
>
> 1. activate the minibuffer by e.g. `M-:'
> 2. switch back to the original buffer by `C-x o'
> 3. start isearch C-s and search for a string that have multiple
> occurrences in the original buffer.
>
> Other occurrences are not highlighted by lazy-highlight face.
> Putting a breakpoint on isearch-lazy-highlight-update shows
> that run-with-idle-timer from isearch-lazy-highlight-new-loop
> never runs it when the minibuffer is not yet exited.
>
> Is it an intentional behavior or a bug?

It worked in Emacs 21. The problem seems to come from the change
to command_loop_1 in the following set of changes:

revision 1.312
date: 2006-10-10 03:20:20 +0200;  author: cyd;  state: Exp;  lines: +2 -5;
	* dispnew.c (sit_for): Sit forever if TIMEOUT is t.

	* keyboard.c (command_loop_1): Handle non-number values of
	`minibuffer-message-timeout'.
	(Fexecute_extended_command): Fix typo.

	* minibuf.c (temp_echo_area_glyphs): Sit for
	`minibuffer-message-timeout' seconds.

The change in question was this:

--- keyboard.c	15 Sep 2006 07:19:14 -0000	1.877
+++ keyboard.c	10 Oct 2006 01:20:20 -0000	1.878
@@ -1546,15 +1546,17 @@
 
       if (minibuf_level
 	  && !NILP (echo_area_buffer[0])
-	  && EQ (minibuf_window, echo_area_window)
-	  && NUMBERP (Vminibuffer_message_timeout))
+	  && EQ (minibuf_window, echo_area_window))
 	{
 	  /* Bind inhibit-quit to t so that C-g gets read in
 	     rather than quitting back to the minibuffer.  */
 	  int count = SPECPDL_INDEX ();
 	  specbind (Qinhibit_quit, Qt);
 
-	  sit_for (Vminibuffer_message_timeout, 0, 2);
+	  if (NUMBERP (Vminibuffer_message_timeout))
+	    sit_for (Vminibuffer_message_timeout, 0, 2);
+	  else
+	    sit_for (Qt, 0, 2);
 
 	  /* Clear the echo area.  */
 	  message2 (0, 0, 0);

I thinks it was made in response to this bug report

[minibuffer-message-timeout has no effect]
http://lists.gnu.org/archive/html/emacs-devel/2006-10/msg00216.html

Reverting the change fixes the isearch problem. The bug report contained
no test case, but the following example works correctly without the
change to command_loop_1 (the fix to temp_echo_area_glyphs is enough).

    (defun foo ()
      (interactive)
      (let ((minibuffer-message-timeout 0.2))
        (minibuffer-message "foo")))

    (global-set-key [f7] 'foo)

    ;; M-: <f7>

-- 
Johan Bockgård

Re: Idle timers with the active minibuffer

[Richard Stallman]

    I discovered this behavior when tried using isearch in the original
    buffer after switching to it from the minibuffer without exiting it.
    The idle timer for lazy-highlighting doesn't run.  Steps to reproduce:

It is a bug.  I can't see anything in the source that would cause it.
Would someone please debug why read_char fails to call timer_start_idle
in this case?

    1. activate the minibuffer by e.g. `M-:'
    2. switch back to the original buffer by `C-x o'
    3. start isearch C-s and search for a string that have multiple
    occurrences in the original buffer.

    Other occurrences are not highlighted by lazy-highlight face.

Re: Idle timers with the active minibuffer

[Richard Stallman]

	    * keyboard.c (command_loop_1): Handle non-number values of
	    `minibuffer-message-timeout'.

That change seems to be needed for this:

	;; Save the original value of `minibuffer-message-timeout', and
	;; set it to nil so that isearch's messages don't get timed out.
	isearch-original-minibuffer-message-timeout minibuffer-message-timeout
	minibuffer-message-timeout nil)

Without the change in keyboard.c, won't sit_for crash
given nil as an argument?

Re: Idle timers with the active minibuffer

[Johan Bockgård]

Richard Stallman <rms@gnu.org> writes:

> Without the change in keyboard.c, won't sit_for crash given nil as an
> argument?

There is a NUMBERP test in both versions. Here is the diff again (in
context format)

*** keyboard.c	15 Sep 2006 07:19:14 -0000	1.877
--- keyboard.c	10 Oct 2006 01:20:20 -0000	1.878
***************
*** 1546,1560 ****
  
        if (minibuf_level
  	  && !NILP (echo_area_buffer[0])
! 	  && EQ (minibuf_window, echo_area_window)
! 	  && NUMBERP (Vminibuffer_message_timeout))
  	{
  	  /* Bind inhibit-quit to t so that C-g gets read in
  	     rather than quitting back to the minibuffer.  */
  	  int count = SPECPDL_INDEX ();
  	  specbind (Qinhibit_quit, Qt);
  
! 	  sit_for (Vminibuffer_message_timeout, 0, 2);
  
  	  /* Clear the echo area.  */
  	  message2 (0, 0, 0);
--- 1546,1562 ----
  
        if (minibuf_level
  	  && !NILP (echo_area_buffer[0])
! 	  && EQ (minibuf_window, echo_area_window))
  	{
  	  /* Bind inhibit-quit to t so that C-g gets read in
  	     rather than quitting back to the minibuffer.  */
  	  int count = SPECPDL_INDEX ();
  	  specbind (Qinhibit_quit, Qt);
  
! 	  if (NUMBERP (Vminibuffer_message_timeout))
! 	    sit_for (Vminibuffer_message_timeout, 0, 2);
! 	  else
! 	    sit_for (Qt, 0, 2);
  
  	  /* Clear the echo area.  */
  	  message2 (0, 0, 0);


-- 
Johan Bockgård

[Emacs 22.2] Re: Idle timers with the active minibuffer

[Juri Linkov]

>     There is a NUMBERP test in both versions.
>
> That is correct.  (I find it hard to read unified format diffs.)
> So it won't crash.
>
> It looks like reverting the keyboard.c change is ok.
> But first let's see what Yidong has to say.
> There must have been a reason why he made that change.

It seems this bug should be fixed before releasing Emacs 22.2,
because it causes the regression comparing with Emacs 21.  But
I don't see what implications revering keyboard.c will have.

-- 
Juri Linkov
http://www.jurta.org/emacs/

Re: [Emacs 22.2] Re: Idle timers with the active minibuffer

[Chong Yidong]

Juri Linkov <juri@jurta.org> writes:

>>     There is a NUMBERP test in both versions.
>>
>> That is correct.  (I find it hard to read unified format diffs.)
>> So it won't crash.
>>
>> It looks like reverting the keyboard.c change is ok.
>> But first let's see what Yidong has to say.
>> There must have been a reason why he made that change.
>
> It seems this bug should be fixed before releasing Emacs 22.2,
> because it causes the regression comparing with Emacs 21.  But
> I don't see what implications revering keyboard.c will have.

It appears to be a mistake, and I believe it can be safely reverted.