bug#11447: 24.1.50; notifications-notify eats keystrokes

bug#11447: 24.1.50; notifications-notify eats keystrokes

[Peter Münster]

Hello,

Please try the following code:

--8<---------------cut here---------------start------------->8---
(require 'notifications)

(defun test-func ()
  (notifications-notify
   :title     "Test"
   :timeout   1000))

(run-with-timer 2 2 'test-func)                    
--8<---------------cut here---------------end--------------->8---

with "emacs -Q" (latest version from trunk).

Then just type some text and you'll see, that some of the letters, that
you type, will be missing in the buffer.

Cheers, Peter


In GNU Emacs 24.1.50.2 (x86_64-suse-linux-gnu, GTK+ Version 2.22.1)
 of 2012-05-10 on micropit
Bzr revision: 108190 lekktu@gmail.com-20120510143443-msjlhoy2181m22rf
Windowing system distributor `The X.Org Foundation', version 11.0.10903000
Configured using:
 `configure '--without-toolkit-scroll-bars''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: en_GB.utf8
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: nil
  value of $XMODIFIERS: @im=local
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t


-- 
           Peter

bug#11447: 24.1.50; notifications-notify eats keystrokes

[Michael Albinus]

Peter Münster <pmlists@free.fr> writes:

> Hello,

Hi Peter,

> Then just type some text and you'll see, that some of the letters, that
> you type, will be missing in the buffer.

Could you, please, check the following patch:

--8<---------------cut here---------------start------------->8---
*** /home/albinus/src/emacs/lisp/net/dbus.el.~108190~
--- /home/albinus/src/emacs/lisp/net/dbus.el
***************
*** 263,274 ****
  	 (apply
  	  'dbus-message-internal dbus-message-type-method-call
  	  bus service path interface method 'dbus-call-method-handler args)))
      ;; Wait until `dbus-call-method-handler' has put the result into
      ;; `dbus-return-values-table'.  If no timeout is given, use the
!     ;; default 25".
      (with-timeout ((if timeout (/ timeout 1000.0) 25))
        (while (eq (gethash key dbus-return-values-table :ignore) :ignore)
! 	(read-event nil nil 0.1)))
  
      ;; Cleanup `dbus-return-values-table'.  Return the result.
      (prog1
--- 263,278 ----
  	 (apply
  	  'dbus-message-internal dbus-message-type-method-call
  	  bus service path interface method 'dbus-call-method-handler args)))
+ 
      ;; Wait until `dbus-call-method-handler' has put the result into
      ;; `dbus-return-values-table'.  If no timeout is given, use the
!     ;; default 25".  Events which are not from D-Bus must be restored.
      (with-timeout ((if timeout (/ timeout 1000.0) 25))
        (while (eq (gethash key dbus-return-values-table :ignore) :ignore)
! 	(let ((event (let (unread-command-events) (read-event nil nil 0.1))))
! 	  (when (and event (not (ignore-errors (dbus-check-event event))))
! 	    (setq unread-command-events
! 		  (append unread-command-events (list event)))))))
  
      ;; Cleanup `dbus-return-values-table'.  Return the result.
      (prog1
--8<---------------cut here---------------end--------------->8---

> Cheers, Peter

Best regards, Michael.

bug#11447: 24.1.50; notifications-notify eats keystrokes

[Peter Münster]

On Fri, May 11 2012, Michael Albinus wrote:

> Could you, please, check the following patch:

It solves the problem. Thanks!

-- 
           Peter

bug#11447: 24.1.50; notifications-notify eats keystrokes

[Stefan Monnier]

>       (with-timeout ((if timeout (/ timeout 1000.0) 25))
>         (while (eq (gethash key dbus-return-values-table :ignore) :ignore)
> ! 	(let ((event (let (unread-command-events) (read-event nil nil 0.1))))
> ! 	  (when (and event (not (ignore-errors (dbus-check-event event))))
> ! 	    (setq unread-command-events
> ! 		  (append unread-command-events (list event)))))))
  
Shouldn't this code use input-pending-p instead?


        Stefan

bug#11447: 24.1.50; notifications-notify eats keystrokes

[Michael Albinus]

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

>>       (with-timeout ((if timeout (/ timeout 1000.0) 25))
>>         (while (eq (gethash key dbus-return-values-table :ignore) :ignore)
>> ! 	(let ((event (let (unread-command-events) (read-event nil nil 0.1))))
>> ! 	  (when (and event (not (ignore-errors (dbus-check-event event))))
>> ! 	    (setq unread-command-events
>> ! 		  (append unread-command-events (list event)))))))
>   
> Shouldn't this code use input-pending-p instead?

Maybe, but I don't know how. I've tried this:

    (with-timeout ((if timeout (/ timeout 1000.0) 25))
      (while (eq (gethash key dbus-return-values-table :ignore) :ignore)
      (or (input-pending-p) (sit-for 0.1 'nodisp))))

With the example in this bug report, I get blocked then. A key might be
placed in `unread-command-events', and I still must handle it.

What do I miss?

>         Stefan

Best regards, Michael.

bug#11447: 24.1.50; notifications-notify eats keystrokes

[Stefan Monnier]

>>> (with-timeout ((if timeout (/ timeout 1000.0) 25))
>>> (while (eq (gethash key dbus-return-values-table :ignore) :ignore)
>>> ! 	(let ((event (let (unread-command-events) (read-event nil nil 0.1))))
>>> ! 	  (when (and event (not (ignore-errors (dbus-check-event event))))
>>> ! 	    (setq unread-command-events
>>> ! 		  (append unread-command-events (list event)))))))
>> 
>> Shouldn't this code use input-pending-p instead?

> Maybe, but I don't know how. I've tried this:

>     (with-timeout ((if timeout (/ timeout 1000.0) 25))
>       (while (eq (gethash key dbus-return-values-table :ignore) :ignore)
>       (or (input-pending-p) (sit-for 0.1 'nodisp))))

Well, actually if you use sit-for you don't need input-pending-p.

> With the example in this bug report, I get blocked then.

What do you mean by "blocked"?

> A key might be placed in `unread-command-events', and I still must
> handle it.

What do you mean by "handle it"?

Could let-binding unread-command-events around the call to sit-for
address the issue?


        Stefan

bug#11447: 24.1.50; notifications-notify eats keystrokes

[Michael Albinus]

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

>> Maybe, but I don't know how. I've tried this:
>
>>     (with-timeout ((if timeout (/ timeout 1000.0) 25))
>>       (while (eq (gethash key dbus-return-values-table :ignore) :ignore)
>>       (or (input-pending-p) (sit-for 0.1 'nodisp))))
>
> Well, actually if you use sit-for you don't need input-pending-p.

OK.

>> With the example in this bug report, I get blocked then.
>
> What do you mean by "blocked"?

An input char goes into unread-command-event. Since nobody handles it,
the while-loop runs forever.

>> A key might be placed in `unread-command-events', and I still must
>> handle it.
>
> What do you mean by "handle it"?

The char event shall be taken from unread-command-event, and (for
example) inserted into the current buffer.

> Could let-binding unread-command-events around the call to sit-for
> address the issue?

Then we have the initial situation, as reported by Peter. The char is
put into the let-bound unread-command-events, and it is lost when the
let-clause ends.

I simply don't know, who shall be responsible to move an event from
unread-command-events. In the example I've shown above, it is not done.

>         Stefan

Best regards, Michael.

bug#11447: 24.1.50; notifications-notify eats keystrokes

[Stefan Monnier]

>>> (with-timeout ((if timeout (/ timeout 1000.0) 25))
>>> (while (eq (gethash key dbus-return-values-table :ignore) :ignore)
>>> (or (input-pending-p) (sit-for 0.1 'nodisp))))
>> Well, actually if you use sit-for you don't need input-pending-p.
> OK.

>>> With the example in this bug report, I get blocked then.
>> What do you mean by "blocked"?
> An input char goes into unread-command-event.

Why?  Who puts it there?

> Since nobody handles it, the while-loop runs forever.

I think I understand what you mean: you mean that while you do `sit-for'
you actually want to run the code associated with the incoming events,
since that code might be the one which changes dbus-return-values-table.

So, indeed sit-for and input-pending-p won't cut it.  Your code looks
OK, then, though you don't need to use a 0.1 timeout, AFAICT.

And in order to avoid the (let (unread-command-events) ...), I'd do
something like:

   (let ((seen ()))
     (unwind-protect
         (let ((event (read-event)))
           (when (and event (not (ignore-errors (dbus-check-event event))))
             (push event seen)))
       (setq unread-command-events
             (append unread-command-events (nreverse seen)))))


-- Stefan

bug#11447: 24.1.50; notifications-notify eats keystrokes

[Michael Albinus]

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

> I think I understand what you mean: you mean that while you do `sit-for'
> you actually want to run the code associated with the incoming events,
> since that code might be the one which changes dbus-return-values-table.

Yes.

> So, indeed sit-for and input-pending-p won't cut it.  Your code looks
> OK, then, though you don't need to use a 0.1 timeout, AFAICT.

OK, I remove the timeout.

> And in order to avoid the (let (unread-command-events) ...), I'd do
> something like:
>
>    (let ((seen ()))
>      (unwind-protect
>          (let ((event (read-event)))
>            (when (and event (not (ignore-errors (dbus-check-event event))))
>              (push event seen)))
>        (setq unread-command-events
>              (append unread-command-events (nreverse seen)))))

Unfortunately, it doesn't work either. When unread-command-events is
non-nil, read-event takes the events from there instead of reading them
from the input streams. This happens after the first time a character
has been read in the loop, before the awaited D-Bus event did arrive.

I've committed my original patch (minus the read-event timeout). Bug is
closed.

> -- Stefan

Best regards, Michael.

bug#11447: 24.1.50; notifications-notify eats keystrokes

[Michael Albinus]

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

>>> (let ((seen ()))
>>> (unwind-protect
>>> (let ((event (read-event)))
>>> (when (and event (not (ignore-errors (dbus-check-event event))))
>>> (push event seen)))
>>> (setq unread-command-events
>>> (append unread-command-events (nreverse seen)))))
>
>> Unfortunately, it doesn't work either. When unread-command-events is
>> non-nil, read-event takes the events from there instead of reading them
>> from the input streams.
>
> You mean, if unread-command-events is already non-nil when entering
> the loop?  Yes, I guess it's a problem.

That I don't know. Its is rather ...

>> This happens after the first time a character has been read in the
>> loop,
>
> That shouldn't be the case unless *you* put that char (well, event) on
> unread-command-events.

Yes, that happens with your proposed

(setq unread-command-events
      (append unread-command-events (nreverse seen)))))

That's why my patch lets unread-command-events be nil, when calling read-event.

>> I've committed my original patch (minus the read-event timeout). Bug is
>> closed.
>
> OK, thank you,

>         Stefan

Best regards, Michael.

bug#11447: 24.1.50; notifications-notify eats keystrokes

[Stefan Monnier]

> Yes, that happens with your proposed

> (setq unread-command-events
>       (append unread-command-events (nreverse seen)))))

Note that I only do it at the end of the loop, so it shouldn't affect
the sit-for.


        Stefan

bug#11447: 24.1.50; notifications-notify eats keystrokes

[Michael Albinus]

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

>> Yes, that happens with your proposed
>
>> (setq unread-command-events
>>       (append unread-command-events (nreverse seen)))))
>
> Note that I only do it at the end of the loop, so it shouldn't affect
> the sit-for.

You do it inside the loop, every next loop cycle unread-command-events
could be non-nil, which confuses read-event.

Maybe you wanted to cumulate the read events in seen inside the loop,
and you wanted to append all events from seen to unread-command-events
outside the loop. That might work.

(Likely it isn't that important which version of the patch we use ...)

>         Stefan

Best regards, Michael..

bug#11447: 24.1.50; notifications-notify eats keystrokes

[Stefan Monnier]

> You do it inside the loop, every next loop cycle unread-command-events
> could be non-nil, which confuses read-event.

Oh, indeed, I didn't write what I intended.  The intention was to do it
outside of the while loop, of course.

> Maybe you wanted to cumulate the read events in seen inside the loop,
> and you wanted to append all events from seen to unread-command-events
> outside the loop. That might work.

Yes, that was the intention.

> (Likely it isn't that important which version of the patch we use ...)

Indeed,


        Stefan