Re: [michael.cadilhac-@t-lrde.epita.fr: sit-for (detect_input_pending ?) and postfix input methods.]

Re: [michael.cadilhac-@t-lrde.epita.fr: sit-for (detect_input_pending ?) and postfix input methods.]

[Kenichi Handa]

In article <E1EMIa9-0005hI-PX@fencepost.gnu.org>, "Richard M. Stallman" <rms@gnu.org> writes:

> Would you please look at this, and ack?  (We have no maintainer for
> flyspell now.)

> ------- Start of forwarded message -------
> To: emacs-pretest-bug@gnu.org
> From: Michael Cadilhac <michael.cadilhac-@t-lrde.epita.fr>
> Subject: sit-for (detect_input_pending ?) and postfix input methods.
[...]
> For a test-case :

> (defun test-case-after-change (a b c)
>   (if (sit-for 3)
>       (message "sit-for: t")
>     (message "sit-for: nil")))

> (defun test-case ()
>   (interactive)
>   (set (make-local-variable 'after-change-functions)
>        '(test-case-after-change)))

> With a buffer  `foo', M-x test-case RET and then  with no input method
> specified, just  hit repeatedly (but  don't stay on the  touch), let's
> say, `u'.

> You'll have something like

> sit-for: nil [14 times]
> sit-for: t

> in your *Messages* buffer. This result is expected : sit-for is
> interrupted and return nil, then the last `u' produce a successful
> sit-for.

> Now, clear `foo', and M-x set-input-method RET latin-1-postfix RET

> Now do the same (with `u', as it has suffix options), and it'll result
> in:

> sit-for: t [14 times]

> Outch ! `sit-for' exits with `t', what a mess !

> I think this is a bug  of the C function `detect_input_pending' or the
> way input-method are managed, can you reproduce it ?

This is because sit-for instantly returns t if
unread-command-events is not nil, and the currrent input
method mechanism uses unread-command-events in the following
way:

To make the explanation clear, I'll right the first `u'
event as `u1' and the second `u' event as `u2'.

When you turn on latin-1-postfix, type `u1', `u1' is given
to the function quail-input-method (and the callees).  This
binds inhibit-modification-hooks to t and insert "u" with
underline, then wait for another key sequence.  This
insertion doesn't activate test-case-after-change.

When `u2' is typed, quail-input-method at first delete the
underlined "u", and returns that character `u' as an event
to be processed later.  But, before returning, it set `u2'
in unread-command-events so that it is processed again by
the input method.

Then, the returned event `u' is handled in the normal
command loop and self-insert-command is called, and
test-case-after-change is called.  At this time, as
unread-command-events is not nil, sit-for returns t.


Then, why does sit-for return t if unread-command-events is not
nil?

sit_for does something like this:

  ...
  wait_reading_process_output ()
  return detect_input_pending () ? Qnil : Qt;
}

wait_reading_process_output () returns instantly when
unread-command-events, but detect_input_pending () returns 0
because no input event is pending.

I don't know which is bad, but I found this suspicious
comment for requeued_events_pending_p which is called by
wait_reading_process_output.

/* Return nonzero if there are pending requeued events.
   This isn't used yet.  The hope is to make wait_reading_process_output
   call it, and return if it runs Lisp code that unreads something.
   The problem is, kbd_buffer_get_event needs to be fixed to know what
   to do in that case.  It isn't trivial.  */

At least that comment should be fixed because it's actually
used now.

This is the first time I read these codes, and I don't know
how those functions are used in the other places.  So I'd
like to ask someone else to fix this problem if you think
this is the problem to be fixed.


As for the original problem of flyspell, how about this
workaround?

*** flyspell.el	23 Sep 2005 10:59:25 +0900	1.75
--- flyspell.el	05 Oct 2005 16:19:56 +0900	
***************
*** 770,776 ****
       ((get this-command 'flyspell-delayed)
        ;; the current command is not delayed, that
        ;; is that we must check the word now
!       (sit-for flyspell-delay))
       (t t)))
     (t t)))
  
--- 770,777 ----
       ((get this-command 'flyspell-delayed)
        ;; the current command is not delayed, that
        ;; is that we must check the word now
!       (and (not unread-command-events)
! 	   (sit-for flyspell-delay)))
       (t t)))
     (t t)))
  
---
Kenichi Handa
handa@m17n.org

Re: [michael.cadilhac-@t-lrde.epita.fr: sit-for (detect_input_pending ?) and postfix input methods.]

[Michael Cadilhac]

Kenichi Handa <handa@m17n.org> writes:

> In article <E1EMIa9-0005hI-PX@fencepost.gnu.org>, "Richard M. Stallman" <rms@gnu.org> writes:
>
>> Would you please look at this, and ack?  (We have no maintainer for
>> flyspell now.)
>
> As for the original problem of flyspell, how about this
> workaround?
>
> *** flyspell.el	23 Sep 2005 10:59:25 +0900	1.75
> --- flyspell.el	05 Oct 2005 16:19:56 +0900	
> ***************
> *** 770,776 ****
>        ((get this-command 'flyspell-delayed)
>         ;; the current command is not delayed, that
>         ;; is that we must check the word now
> !       (sit-for flyspell-delay))
>        (t t)))
>      (t t)))
>
> --- 770,777 ----
>        ((get this-command 'flyspell-delayed)
>         ;; the current command is not delayed, that
>         ;; is that we must check the word now
> !       (and (not unread-command-events)
> ! 	   (sit-for flyspell-delay)))
>        (t t)))
>      (t t)))

   Thanks for your deep explanation !

   Note that with your workaround, you're disabling a flyspell feature
   which  is, when you  type a  world and  don't add  delimiters, this
   world will eventually be checked after `flyspell-delay' seconds.

   I think the  behavior of sit-for should be  corrected, or explained
   in the documentation.

   However, I'll use your workaround as this feature is less important
   to me than the bug fix.

   Thanks !

-- 
    Michael Cadilhac, a.k.a. Micha [mika] |
                    Epita/LRDE promo 2007 |  Please note that you should
  2 rue de la Convention | 08.70.65.13.14 |  s/-@t-/@/ my mail address.
94270 Le Kremlin Bicetre | 06.23.20.31.30 |

Re: [michael.cadilhac-@t-lrde.epita.fr: sit-for (detect_input_pending ?) and postfix input methods.]

[Kenichi Handa]

In article <87y858udxg.fsf@mahaena.lrde>, Michael Cadilhac <michael.cadilhac-@t-lrde.epita.fr> writes:

>>  --- 770,777 ----
>>         ((get this-command 'flyspell-delayed)
>>          ;; the current command is not delayed, that
>>          ;; is that we must check the word now
>>  !       (and (not unread-command-events)
>>  ! 	   (sit-for flyspell-delay)))
>>         (t t)))
>>       (t t)))

>    Thanks for your deep explanation !

>    Note that with your workaround, you're disabling a flyspell feature
>    which  is, when you  type a  world and  don't add  delimiters, this
>    world will eventually be checked after `flyspell-delay' seconds.

??? The above change should not disable that feature.  The
reason why "world" is not checked is because when you type
the last "d", you are still in the command loop within input
method (because "d" has a possibility of being tranlated to
ð when you type "/" after it), thus after-change-functions
is bound to nil.  If you type "worlk" instead, it should be
checked after `flyspell-delay' seconds because the last "k"
is committed instantly.

---
Kenichi Handa
handa@m17n.org

Re: [michael.cadilhac-@t-lrde.epita.fr: sit-for (detect_input_pending ?) and postfix input methods.]

[Michael Cadilhac]

Kenichi Handa <handa@m17n.org> writes:

> In article <87y858udxg.fsf@mahaena.lrde>, Michael Cadilhac <michael.cadilhac-@t-lrde.epita.fr> writes:
>
>>>  --- 770,777 ----
>>>         ((get this-command 'flyspell-delayed)
>>>          ;; the current command is not delayed, that
>>>          ;; is that we must check the word now
>>>  !       (and (not unread-command-events)
>>>  ! 	   (sit-for flyspell-delay)))
>>>         (t t)))
>>>       (t t)))
>
>>    Thanks for your deep explanation !
>
>>    Note that with your workaround, you're disabling a flyspell feature
>>    which  is, when you  type a  world and  don't add  delimiters, this
>>    world will eventually be checked after `flyspell-delay' seconds.
>
> ??? The above change should not disable that feature.  The
> reason why "world" is not checked is because when you type
> the last "d", you are still in the command loop within input
> method (because "d" has a possibility of being tranlated to
> ð when you type "/" after it), thus after-change-functions
> is bound to nil.  If you type "worlk" instead, it should be
> checked after `flyspell-delay' seconds because the last "k"
> is committed instantly.

  Yep, you're right. Sorry for the misunderstanding of the behavior.

  So your patch is ok for me. Thanks !

-- 
    Michael Cadilhac, a.k.a. Micha [mika] |
                    Epita/LRDE promo 2007 |  Please note that you should
  2 rue de la Convention | 08.70.65.13.14 |  s/-@t-/@/ my mail address.
94270 Le Kremlin Bicetre | 06.23.20.31.30 |

Re: [michael.cadilhac-@t-lrde.epita.fr: sit-for (detect_input_pending ?) and postfix input methods.]

[Kenichi Handa]

In article <87r7ayu8ii.fsf@mahaena.lrde>, Michael Cadilhac <michael.cadilhac-@t-lrde.epita.fr> writes:
>>  ??? The above change should not disable that feature.  The
>>  reason why "world" is not checked is because when you type
>>  the last "d", you are still in the command loop within input
>>  method (because "d" has a possibility of being tranlated to
>>  ð when you type "/" after it), thus after-change-functions
>>  is bound to nil.  If you type "worlk" instead, it should be
>>  checked after `flyspell-delay' seconds because the last "k"
>>  is committed instantly.

>   Yep, you're right. Sorry for the misunderstanding of the behavior.

>   So your patch is ok for me. Thanks !

Thank you for testing it.  I'll install it if there's no
objection.

---
Kenichi Handa
handa@m17n.org

*** flyspell.el	23 Sep 2005 10:59:25 +0900	1.75
--- flyspell.el	06 Oct 2005 21:44:30 +0900	
***************
*** 770,776 ****
       ((get this-command 'flyspell-delayed)
        ;; the current command is not delayed, that
        ;; is that we must check the word now
!       (sit-for flyspell-delay))
       (t t)))
     (t t)))
  
--- 770,782 ----
       ((get this-command 'flyspell-delayed)
        ;; the current command is not delayed, that
        ;; is that we must check the word now
! 
!       ;; Note: When an input method is activated, it is likely that
!       ;; unread-command-events is non-nil now.  Then, sit-for
!       ;; instantly returns t.  As a workaround for that bug, here we
!       ;; explicitely checks unread-command-events.
!       (and (not unread-command-events)
! 	   (sit-for flyspell-delay)))
       (t t)))
     (t t)))
  

Re: [michael.cadilhac-@t-lrde.epita.fr: sit-for (detect_input_pending ?) and postfix input methods.]

[Stefan Monnier]

> This is because sit-for instantly returns t if
> unread-command-events is not nil, and the currrent input
> method mechanism uses unread-command-events in the following
> way:

...Hmmm... interesting.
Indeed, what happens basically is that when you type

        a b

the `a' is only executed when you type the `b', because in the time between
the two events, quail is waiting for another key in order to decide whether
to really meant to type an `a' or maybe some other char (like à, ä, ...).

In the case where `a' and `b' are bount to self-insert-command, I could
imagine changing Quail such that the first (quail-input-method ?a) returns
'(?a) and that a subsequent (quail-input-method ?\") returns '(?\^? ?ä),
with some added magic to add/remove the underscore.  But since those chars
can be bound to something else than self-insert-command, there's no
guarantee that it'll do the right thing.

Or maybe we should first return '(set-quail-undo-boundary ?a) and then
'(quail-undo-last ?ä) where both set-quail-undo-boundary and quail-undo-last
are special events bound to similarly named functions.  I guess that could
work, although it would need additional hacks to enable/disable the undo-log
and to add/remove the underscore.


        Stefan

Re: [michael.cadilhac-@t-lrde.epita.fr: sit-for (detect_input_pending ?) and postfix input methods.]

[Kenichi Handa]

In article <jwvachn3mvh.fsf-monnier+emacs@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Indeed, what happens basically is that when you type

>         a b

> the `a' is only executed when you type the `b', because in the time between
> the two events, quail is waiting for another key in order to decide whether
> to really meant to type an `a' or maybe some other char (like à, ä, ...).

> In the case where `a' and `b' are bount to self-insert-command, I could
> imagine changing Quail such that the first (quail-input-method ?a) returns
> '(?a) and that a subsequent (quail-input-method ?\") returns '(?\^? ?ä),
> with some added magic to add/remove the underscore.  But since those chars
> can be bound to something else than self-insert-command, there's no
> guarantee that it'll do the right thing.

> Or maybe we should first return '(set-quail-undo-boundary ?a) and then
> '(quail-undo-last ?ä) where both set-quail-undo-boundary and quail-undo-last
> are special events bound to similarly named functions.  I guess that could
> work, although it would need additional hacks to enable/disable the undo-log
> and to add/remove the underscore.

You are quite right.  I was also thinking how we can avoid
the command loop within inpt-method function, but, for the
moment, I don't have a good idea.  It's dangerous to insert
that temporary "a" outside of input-method function because
that will run various hook functions, and simply suppressing
them on insertion will cause another problem.  :-(

---
Kenichi Handa
handa@m17n.org

Re: [michael.cadilhac-@t-lrde.epita.fr: sit-for (detect_input_pending ?) and postfix input methods.]

[Richard M. Stallman]

    Then, why does sit-for return t if unread-command-events is not
    nil?

That is a good question.  Should it return nil immediately
without waiting when unread-command-events is nonempty?
That would be more coherent than the current behavior.
It would also be an incompatible change, and might break something.

There are about 550 calls to sit-for in the Emacs sources.
To check all of them would be a lot of work.

Perhaps we should make this change now, so that the pretesting
for the release will help us see if anything breaks.
What do people think?


Anyway, I installed your flyspell.el change as a work-around.

Re: [michael.cadilhac-@t-lrde.epita.fr: sit-for (detect_input_pending ?) and postfix input methods.]

[Kim F. Storm]

"Richard M. Stallman" <rms@gnu.org> writes:

>     Then, why does sit-for return t if unread-command-events is not
>     nil?
>
> That is a good question.  Should it return nil immediately
> without waiting when unread-command-events is nonempty?
> That would be more coherent than the current behavior.
> It would also be an incompatible change, and might break something.
>
> There are about 550 calls to sit-for in the Emacs sources.
> To check all of them would be a lot of work.
>
> Perhaps we should make this change now, so that the pretesting
> for the release will help us see if anything breaks.
> What do people think?

Why take the risk of breaking currently working code this close (YMMV)
to the release...?

IMO, this is not the time for such a change in 22.x -- we know of one
place (flyspell) that was affected by the present behaviuor and it has
been fixed now.  

OTOH, you can install the change now on the unicode-2 branch for 23.x.

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

Re: [michael.cadilhac-@t-lrde.epita.fr: sit-for (detect_input_pending ?) and postfix input methods.]

[David Kastrup]

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

> Why take the risk of breaking currently working code this close
> (YMMV) to the release...?
>
> IMO, this is not the time for such a change in 22.x -- we know of
> one place (flyspell) that was affected by the present behaviuor and
> it has been fixed now.
>
> OTOH, you can install the change now on the unicode-2 branch for
> 23.x.

I think considering "unicode-2" as equivalent to "next release" not
really a good idea.  unicode-2 is not a substitute for a release fork
unless I am mistaken.  We have several branches, like multi-tty and
unicode-2, and I don't think it is completely obvious what exactly
will come after 22.1.  It won't make merging the branch easier, if all
kind of unrelated stuff gets put into it.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum