* bug#6256: 24.0.50; read-event in `repeat' command
@ 2010-05-24 15:11 Drew Adams
2010-05-24 16:28 ` Drew Adams
0 siblings, 1 reply; 29+ messages in thread
From: Drew Adams @ 2010-05-24 15:11 UTC (permalink / raw)
To: 6256
In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
of 2010-05-23 on G41R2F1
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (3.4) --no-opt --cflags -Ic:/xpm/include'
I use popup frames a lot. By default, that is how a buffer gets displayed in my
setup.
I'm also on MS Windows, which has the property that a newly created frame is
always selected (obtains the focus). This means that displaying a buffer is
different, depending on whether its frame already exists. If it does, then the
focus (typically) does not change to the buffer's frame. If it does not, then
the focus moves there. I work around this focus-grabbing annoyance sometimes by
using `select-frame-set-input-focus' at appropriate places. (Ugh!)
Anyway, the problem I have is with the `repeat' command. Presumably,
it tries to be sensitive to actual user events, such as typing a character, but
ignore other, non-user events. This does not work for me in some
cases - an automatic `switch-frame' event is causing it to stop repeating.
I have a command that displays a buffer in its own frame (the same frame or
another, possibly new, frame). I make it repeatable using `repeat', and I bind
it to a key, say `f'. That works fine in general: hitting `f f f...' continues
to display buffers - in new frames or existing frames. No problem.
But in some cases my command also pops up an additional buffer, in another
frame. In those cases the repetition breaks as soon as that second buffer is
displayed. Hitting `f' at that point just inserts an `f' character (in that
buffer if it was selected, else in the first buffer displayed by `f'). It does
not matter whether the second buffer and its frame already exist or not.
I mentioned the frame focus above, but that is apparently not the problem. It
does not matter whether the `f' command opens the first buffer in a new frame or
an existing frame, and likewise for the second buffer. And it does not matter
whether the second buffer is selected (and its frame focused) or not.
It is only when hitting `f' opens also a second buffer that a subsequent `f'
just inserts.
However, if I make a copy of the definition of command `repeat' and change only
its `read-event' to `read-char-exclusive', then the problem goes away. So some
event being read is being tested and found to be different from the `f' last
input event, thus ending the repetition.
I'm guessing that this is what happens: The `read-event' reads the `f' and
repeats the command, which displays the first and second buffers. I would think
there would be a `switch-frame' event associated with each buffer display, but
maybe not. In any case, if there is a `switch-frame' for the first buffer
display it is ignored, but the `switch-frame' for the second buffer display
causes repetition to stop.
It is difficult to use the debugger with `repeat', so I copied the code and
inserted a call to `message' after the `read-event'. It shows clearly that
display of the second buffer causes the event read to be a `switch-frame'.
Do we intentionally use `read-event' here (instead of `read-char-exclusive') in
order to let `repeat' work with other user events besides just char input (e.g.
mouse clicks)? If so, then changing `read-event' to `read-char-exclusive' would
not be the correct fix, but what would? This is not clear to me.
`read-event' has been present in this code since at least Emacs 20, but this
code does not correctly handle `switch-frame' events. Real user events need to
be distinguished from events such as `switch-frame' that can be generated by
code.
Is changing `read-event' to `read-char-exclusive' the proper fix for this bug?
It works for me. If it is not the right fix, then what is?
If it is important for some use reason to keep `read-event', and no fix is found
that would DTRT to distinguish real user events from events such as
`switch-frame', then could we at least use `(funcall repeat-read-function)'
instead of `(read-event)', so that code that wants to be sensitive to only char
events can bind `repeat-read-function' to `read-char-exclusive' around the call
to `repeat'?
The default value of such a var could be `read-event', if that's deemed the best
default, but we at least need some way to make `repeat' ignore non-char events
(if we cannot find a way to make it ignore non-user events).
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-05-24 15:11 bug#6256: 24.0.50; read-event in `repeat' command Drew Adams
@ 2010-05-24 16:28 ` Drew Adams
2010-05-24 23:05 ` Drew Adams
0 siblings, 1 reply; 29+ messages in thread
From: Drew Adams @ 2010-05-24 16:28 UTC (permalink / raw)
To: 6256
> Is changing `read-event' to `read-char-exclusive' the proper
> fix for this bug? It works for me. If it is not the right fix,
> then what is?
>
> If it is important for some use reason to keep `read-event',
> and no fix is found that would DTRT to distinguish real user
> events from events such as `switch-frame', then could we at
> least use `(funcall repeat-read-function)' instead of
> `(read-event)', so that code that wants to be sensitive to
> only char events can bind `repeat-read-function' to
> `read-char-exclusive' around the call to `repeat'?
>
> The default value of such a var could be `read-event', if
> that's deemed the best default, but we at least need some
> way to make `repeat' ignore non-char events
> (if we cannot find a way to make it ignore non-user events).
Actually, I think that using `read-char-exclusive' is the right fix. If the
action were initiated by a mouse event it is unlikely that the next event would
be the same mouse event, so repetition would not occur anyway.
Could you please make this fix: replace the unique occurrence of `read-event' by
`read-char-exclusive' in `repeat'. Thx.
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-05-24 16:28 ` Drew Adams
@ 2010-05-24 23:05 ` Drew Adams
2010-05-25 0:06 ` Lennart Borgman
2010-05-25 2:41 ` Stefan Monnier
0 siblings, 2 replies; 29+ messages in thread
From: Drew Adams @ 2010-05-24 23:05 UTC (permalink / raw)
To: 6256
> > Is changing `read-event' to `read-char-exclusive' the proper
> > fix for this bug? It works for me. If it is not the right fix,
> > then what is?
> >
> > If it is important for some use reason to keep `read-event',
> > and no fix is found that would DTRT to distinguish real user
> > events from events such as `switch-frame', then could we at
> > least use `(funcall repeat-read-function)' instead of
> > `(read-event)', so that code that wants to be sensitive to
> > only char events can bind `repeat-read-function' to
> > `read-char-exclusive' around the call to `repeat'?
> >
> > The default value of such a var could be `read-event', if
> > that's deemed the best default, but we at least need some
> > way to make `repeat' ignore non-char events
> > (if we cannot find a way to make it ignore non-user events).
>
> Actually, I think that using `read-char-exclusive' is the
> right fix. If the
> action were initiated by a mouse event it is unlikely that
> the next event would
> be the same mouse event, so repetition would not occur anyway.
>
> Could you please make this fix: replace the unique occurrence
> of `read-event' by `read-char-exclusive' in `repeat'. Thx.
I take it back. `read-char-exclusive' is not the right fix, because not all
keyboard events are character events. Hitting the key `left', for instance,
does not work.
And in fact, I do not seem to be able to reproduce the problem anymore in Emacs
23. (It does occur in Emacs 22.3, however.) Sorry for the noise.
So I guess this bug could be closed. But I still wonder if the code shouldn't
be tweaked somehow to read an event but ignore non-user (or at least
non-keyboard) events. I don't know how to do that.
Before closing, it would be great if someone knowledgable would reply with some
info about the question. Thx.
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-05-24 23:05 ` Drew Adams
@ 2010-05-25 0:06 ` Lennart Borgman
2010-05-25 2:41 ` Stefan Monnier
1 sibling, 0 replies; 29+ messages in thread
From: Lennart Borgman @ 2010-05-25 0:06 UTC (permalink / raw)
To: Drew Adams; +Cc: 6256
On Tue, May 25, 2010 at 1:05 AM, Drew Adams <drew.adams@oracle.com> wrote:
>
> So I guess this bug could be closed. But I still wonder if the code shouldn't
> be tweaked somehow to read an event but ignore non-user (or at least
> non-keyboard) events. I don't know how to do that.
After a quick look I guess it does that. However the switch-frame
event is a bit special. The handling of it is delayed in keyboard.c. I
guess what you have seen is the result of that something has gone
wrong because of that (but I am not sure).
Maybe this has been fixed in 23.2?
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-05-24 23:05 ` Drew Adams
2010-05-25 0:06 ` Lennart Borgman
@ 2010-05-25 2:41 ` Stefan Monnier
2010-07-03 21:24 ` Drew Adams
1 sibling, 1 reply; 29+ messages in thread
From: Stefan Monnier @ 2010-05-25 2:41 UTC (permalink / raw)
To: Drew Adams; +Cc: 6256
> I take it back. `read-char-exclusive' is not the right fix, because not all
> keyboard events are character events. Hitting the key `left', for instance,
> does not work.
I think the real answer is that both read-event and read-char-exclusive
are the wrong answer. A better answer would be to have some way to
change the way the next key is handled. Something like
a `next-key-map', kind of like the overriding maps, except that it would
be automatically reset before running a command (and it would not
disable the other keymaps, only take precedence over them).
Basically calling read-event and friends and then putting it back onto
the unread-command-events list is almost always wrong in one way or
another (e.g. it postpones running post-command-hook).
Stefan
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-05-25 2:41 ` Stefan Monnier
@ 2010-07-03 21:24 ` Drew Adams
2010-07-04 22:45 ` Stefan Monnier
2010-09-11 18:25 ` Stefan Monnier
0 siblings, 2 replies; 29+ messages in thread
From: Drew Adams @ 2010-07-03 21:24 UTC (permalink / raw)
To: 'Stefan Monnier'; +Cc: 6256
> > I take it back. `read-char-exclusive' is not the right
> > fix, because not all keyboard events are character events.
> > Hitting the key `left', for instance, does not work.
>
> I think the real answer is that both read-event and
> read-char-exclusive are the wrong answer. A better answer
> would be to have some way to change the way the next key is
> handled. Something like a `next-key-map', kind of like the
> overriding maps, except that it would be automatically reset
> before running a command (and it would not disable the other
> keymaps, only take precedence over them).
>
> Basically calling read-event and friends and then putting it
> back onto the unread-command-events list is almost always
> wrong in one way or another (e.g. it postpones running
> post-command-hook).
I don't really understand the code very well, I admit. You obviously understand
it and have an idea for how to improve things in a big way. It's not clear that
that improvement is soon forthcoming, however. I assume that it might not be
trivial or that you are busy doing other things.
While waiting for the ideal fix, can we at least improve this incrementally to
enable `repeat' to work as I would expect it to wrt, say, mouse events?
For example, suppose I do this:
(defun repeat-command (command)
(let ((repeat-previous-repeated-command command)
(repeat-message-function 'ignore)
(last-repeatable-command 'repeat))
(repeat nil)))
(defun foo-repeat (arg)
(interactive "P")
(repeat-command 'foo))
(define-key my-map (vector (list mouse-wheel-up-event)) 'foo-repeat)
And suppose `my-map' is bound to `C-x p'. I would like for `C-x p' followed by
repeated mouse-wheel-up events to repeat command `foo'.
That does not happen, however, because of this restrictive `eq' test in the
definition of function `repeat':
(while (eq (read-event) repeat-repeat-char)
(repeat repeat-arg))
The event read will be something like this, for the wheel action:
(wheel-down (#<window 8 on foo.el> 2051 (118 . 176) 158455015 nil
2051 (59 . 40) nil (26 . 2) (2 . 4)))
I would think that we would want to change the test to this, or similar:
(while (let ((evt (read-event)))
(and (equal (event-basic-type evt) (event-basic-type
repeat-repeat-char))
(equal (event-modifiers evt) (event-modifiers
repeat-repeat-char))))
(repeat repeat-arg))
And that seems to work OK. What do you think - is it reasonable to do that? It
seems like a win, to me (nothing lost, something gained). But perhaps I'm
missing something.
It seems to me that what we want is to check whether the same user event
occurred, and for complex events such as mouse events we would typically just
want the same event type with the same modifiers.
Please let me know what you think.
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-07-03 21:24 ` Drew Adams
@ 2010-07-04 22:45 ` Stefan Monnier
2010-07-07 14:43 ` Drew Adams
2010-07-21 15:54 ` Drew Adams
2010-09-11 18:25 ` Stefan Monnier
1 sibling, 2 replies; 29+ messages in thread
From: Stefan Monnier @ 2010-07-04 22:45 UTC (permalink / raw)
To: Drew Adams; +Cc: 6256
> That does not happen, however, because of this restrictive `eq' test in the
> definition of function `repeat':
> (while (eq (read-event) repeat-repeat-char)
> (repeat repeat-arg))
> The event read will be something like this, for the wheel action:
> (wheel-down (#<window 8 on foo.el> 2051 (118 . 176) 158455015 nil
> 2051 (59 . 40) nil (26 . 2) (2 . 4)))
> I would think that we would want to change the test to this, or similar:
> (while (let ((evt (read-event)))
> (and (equal (event-basic-type evt) (event-basic-type
> repeat-repeat-char))
> (equal (event-modifiers evt) (event-modifiers
> repeat-repeat-char))))
> (repeat repeat-arg))
> And that seems to work OK. What do you think - is it reasonable to do
> that?
That sounds right, yes,
People, feel free to make such a change,
Stefan
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-07-04 22:45 ` Stefan Monnier
@ 2010-07-07 14:43 ` Drew Adams
2010-07-21 15:54 ` Drew Adams
1 sibling, 0 replies; 29+ messages in thread
From: Drew Adams @ 2010-07-07 14:43 UTC (permalink / raw)
To: 'Stefan Monnier'; +Cc: 6256
> People, feel free to make such a change,
I hope someone will. Thanks.
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-07-04 22:45 ` Stefan Monnier
2010-07-07 14:43 ` Drew Adams
@ 2010-07-21 15:54 ` Drew Adams
2010-08-28 15:19 ` Drew Adams
1 sibling, 1 reply; 29+ messages in thread
From: Drew Adams @ 2010-07-21 15:54 UTC (permalink / raw)
To: 'Stefan Monnier'; +Cc: 6256
> From: Stefan Monnier Sent: Sunday, July 04, 2010 3:46 PM
> That sounds right, yes,
> People, feel free to make such a change,
Could someone please commit this change? Thx.
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-07-21 15:54 ` Drew Adams
@ 2010-08-28 15:19 ` Drew Adams
0 siblings, 0 replies; 29+ messages in thread
From: Drew Adams @ 2010-08-28 15:19 UTC (permalink / raw)
To: 'Stefan Monnier'; +Cc: 6256
> > From: Stefan Monnier Sent: Sunday, July 04, 2010 3:46 PM
> > That sounds right, yes,
> > People, feel free to make such a change,
>
> Could someone please commit this change? Thx.
ping
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-07-03 21:24 ` Drew Adams
2010-07-04 22:45 ` Stefan Monnier
@ 2010-09-11 18:25 ` Stefan Monnier
2010-09-11 22:34 ` Drew Adams
1 sibling, 1 reply; 29+ messages in thread
From: Stefan Monnier @ 2010-09-11 18:25 UTC (permalink / raw)
To: Drew Adams; +Cc: 6256
> And suppose `my-map' is bound to `C-x p'. I would like for `C-x p'
> followed by repeated mouse-wheel-up events to repeat command `foo'.
> That does not happen, however, because of this restrictive `eq' test in the
> definition of function `repeat':
> (while (eq (read-event) repeat-repeat-char)
> (repeat repeat-arg))
> The event read will be something like this, for the wheel action:
> (wheel-down (#<window 8 on foo.el> 2051 (118 . 176) 158455015 nil
> 2051 (59 . 40) nil (26 . 2) (2 . 4)))
I installed the patch below in the emacs-23 branch (it seemed slightly
safer than your version of the code).
Please confirm that it fixes your problem,
Stefan
=== modified file 'lisp/repeat.el'
--- lisp/repeat.el 2010-01-13 08:35:10 +0000
+++ lisp/repeat.el 2010-09-11 18:20:32 +0000
@@ -335,7 +335,12 @@
(setq real-last-command 'repeat)
(setq repeat-undo-count 1)
(unwind-protect
- (while (eq (read-event) repeat-repeat-char)
+ (while (let ((evt (read-event))) ;FIXME: read-key maybe?
+ ;; For clicks, we need to strip the meta-data to
+ ;; check the underlying event name.
+ (eq (or (car-safe evt) evt)
+ (or (car-safe repeat-repeat-char)
+ repeat-repeat-char)))
(repeat repeat-arg))
;; Make sure `repeat-undo-count' is reset.
(setq repeat-undo-count nil))
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-09-11 18:25 ` Stefan Monnier
@ 2010-09-11 22:34 ` Drew Adams
2010-09-12 16:06 ` Drew Adams
0 siblings, 1 reply; 29+ messages in thread
From: Drew Adams @ 2010-09-11 22:34 UTC (permalink / raw)
To: 'Stefan Monnier'; +Cc: 6256
Thanks for trying, but no, unfortunately it does not work.
I tested your patch using this release:
GNU Emacs 23.2.1 (i386-mingw-nt5.1.2600)
of 2010-05-08 on G41R2F1
And I can confirm that the code I suggested does work.
> From: Stefan Monnier Sent: Saturday, September 11, 2010 11:26 AM
> > And suppose `my-map' is bound to `C-x p'. I would like for `C-x p'
> > followed by repeated mouse-wheel-up events to repeat command `foo'.
>
> > That does not happen, however, because of this restrictive
> > `eq' test in the definition of function `repeat':
>
> > (while (eq (read-event) repeat-repeat-char)
> > (repeat repeat-arg))
>
> > The event read will be something like this, for the wheel action:
> > (wheel-down (#<window 8 on foo.el> 2051 (118 . 176) 158455015 nil
> > 2051 (59 . 40) nil (26 . 2) (2 . 4)))
>
> I installed the patch below in the emacs-23 branch (it seemed slightly
> safer than your version of the code).
> Please confirm that it fixes your problem,
> + (while (let ((evt (read-event))) ;FIXME: read-key maybe?
> + ;; For clicks, we need to strip the meta-data to
> + ;; check the underlying event name.
> + (eq (or (car-safe evt) evt)
> + (or (car-safe repeat-repeat-char)
> + repeat-repeat-char)))
If you want to test for yourself, then:
1. Load the Bookmark+ files (see below).
2. Use C-x p RET at several places in a file to create some bookmarks.
3. Use C-x p <up> <up> <up>... (or <down> <down>...) to cycle among the
bookmarks. You will see a message showing the position at each bookmark
visited.
4. Now try C-x p followed by repeated mouse-wheel down or up movements. You
should see the same movement to each bookmark in turn, and the same position
messages. With your code the wheel repetition does not work. With the code I
sent it does work. This is the `while' condition I sent (again):
(while (let ((evt (read-event)))
(and (equal (event-basic-type evt)
(event-basic-type repeat-repeat-char))
(equal (event-modifiers evt)
(event-modifiers repeat-repeat-char))))
The Bookmark+ files are these (load in this order, after loading std library
bookmark.el):
bookmark+-mac.el
bookmark+-bmu.el
bookmark+-1.el
bookmark+-lit.el
bookmark+.el
You can find the Bookmark+ files here (Emacs Wiki Elisp area):
http://www.emacswiki.org/cgi-bin/wiki?action=index;match=%5C.(el%7Ctar)(%5C.gz)%
3F%24
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-09-11 22:34 ` Drew Adams
@ 2010-09-12 16:06 ` Drew Adams
2010-09-17 3:34 ` Drew Adams
2010-10-18 18:40 ` Stefan Monnier
0 siblings, 2 replies; 29+ messages in thread
From: Drew Adams @ 2010-09-12 16:06 UTC (permalink / raw)
To: 'Stefan Monnier'; +Cc: 6256
> If you want to test for yourself, then:
>
> 1. Load the Bookmark+ files (see below).
Forgot to mention that you will need to uncomment this code in bookmark+-1.el:
;; (define-key bookmark-map (vector (list mouse-wheel-up-event))
;; 'bmkp-next-bookmark-this-buffer-repeat)
;; (define-key bookmark-map (vector (list mouse-wheel-down-event))
;; 'bmkp-previous-bookmark-this-buffer-repeat)
> 2. Use C-x p RET at several places in a file to create some bookmarks.
> 3. Use C-x p <up> <up> <up>... (or <down> <down>...) to cycle
> among the bookmarks. You will see a message showing the position at
> each bookmark visited.
>
> 4. Now try C-x p followed by repeated mouse-wheel down or up
> movements. You should see the same movement to each bookmark
> in turn, and the same position messages. With your code the wheel
> repetition does not work. With the code I sent it does work.
> This is the `while' condition I sent (again):
>
> (while (let ((evt (read-event)))
> (and (equal (event-basic-type evt)
> (event-basic-type repeat-repeat-char))
> (equal (event-modifiers evt)
> (event-modifiers repeat-repeat-char))))
>
> The Bookmark+ files are these (load in this order, after
> loading std library bookmark.el):
>
> bookmark+-mac.el
> bookmark+-bmu.el
> bookmark+-1.el
> bookmark+-lit.el
> bookmark+.el
>
> You can find the Bookmark+ files here (Emacs Wiki Elisp area):
>
http://www.emacswiki.org/cgi-bin/wiki?action=index;match=%5C.(el%7Ctar)(%5C.gz)%
3F%24
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-09-12 16:06 ` Drew Adams
@ 2010-09-17 3:34 ` Drew Adams
2010-09-22 14:01 ` bug#6256: [PATCH] " Drew Adams
2010-10-18 18:40 ` Stefan Monnier
1 sibling, 1 reply; 29+ messages in thread
From: Drew Adams @ 2010-09-17 3:34 UTC (permalink / raw)
To: 'Stefan Monnier'; +Cc: 6256
ping
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: [PATCH] RE: bug#6256: 24.0.50; read-event in `repeat' command
2010-09-17 3:34 ` Drew Adams
@ 2010-09-22 14:01 ` Drew Adams
2010-09-25 14:30 ` Drew Adams
0 siblings, 1 reply; 29+ messages in thread
From: Drew Adams @ 2010-09-22 14:01 UTC (permalink / raw)
To: 'Stefan Monnier'; +Cc: 6256
ping
Please apply the patch (code) I sent (or equivalent that works). Thx.
> From: Drew Adams Sent: Thursday, September 16, 2010 8:34 PM
>
> ping
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: [PATCH] RE: bug#6256: 24.0.50; read-event in `repeat' command
2010-09-22 14:01 ` bug#6256: [PATCH] " Drew Adams
@ 2010-09-25 14:30 ` Drew Adams
0 siblings, 0 replies; 29+ messages in thread
From: Drew Adams @ 2010-09-25 14:30 UTC (permalink / raw)
To: 'Stefan Monnier'; +Cc: 6256
ping.
I sent a fix for this in May 2010. Stefan, you agreed (in July) with the fix I
sent. You said:
"That sounds right, yes. People, feel free to make such a change."
We waited for someone to commit it. No one did. In September you committed a
different "fix" that does not work. Can you please commit the fix I sent, or at
least something equivalent that actually fixes the bug? Thx.
> > > People, feel free to make such a change
> >
> > Could someone please commit this change? Thx.
> > ping
>
> ping
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-09-12 16:06 ` Drew Adams
2010-09-17 3:34 ` Drew Adams
@ 2010-10-18 18:40 ` Stefan Monnier
2010-10-18 21:12 ` Drew Adams
[not found] ` <jwv4ocjuvm1.fsf-monnier+emacs@gnu.org>
1 sibling, 2 replies; 29+ messages in thread
From: Stefan Monnier @ 2010-10-18 18:40 UTC (permalink / raw)
To: Drew Adams; +Cc: 6256
>> If you want to test for yourself, then:
>> 1. Load the Bookmark+ files (see below).
It's not exactly self-contained and going through all those links to
finally download each file is rather inconvenient. So in order to help
me help you, in the future, please try and make it easier. E.g. with
a single download, and then a single load-file.
>> 4. Now try C-x p followed by repeated mouse-wheel down or up
>> movements. You should see the same movement to each bookmark
>> in turn, and the same position messages. With your code the wheel
>> repetition does not work.
Indeed it doesn't work.
>> With the code I sent it does work.
>> This is the `while' condition I sent (again):
>> (while (let ((evt (read-event)))
>> (and (equal (event-basic-type evt)
>> (event-basic-type repeat-repeat-char))
>> (equal (event-modifiers evt)
>> (event-modifiers repeat-repeat-char))))
But that doesn't work either in my tests: the problem is that the
last-command-event was `mouse-4' (i.e. the up event) whereas read-event
returns `down-mouse-4' (a subsequent read-event would return the
`mouse-4').
For my case, replacing the read-event by `read-key' happens to make
it work (see patch below). Please confirm whether or not it fixes it
for you, and if it doesn't, please show me the values of
`repeat-repeat-char' and `evt' in the above test.
Stefan
=== modified file 'lisp/repeat.el'
--- lisp/repeat.el 2010-09-11 18:23:45 +0000
+++ lisp/repeat.el 2010-10-18 18:34:24 +0000
@@ -335,7 +335,7 @@
(setq real-last-command 'repeat)
(setq repeat-undo-count 1)
(unwind-protect
- (while (let ((evt (read-event))) ;FIXME: read-key maybe?
+ (while (let ((evt (read-key)))
;; For clicks, we need to strip the meta-data to
;; check the underlying event name.
(eq (or (car-safe evt) evt)
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-10-18 18:40 ` Stefan Monnier
@ 2010-10-18 21:12 ` Drew Adams
2010-10-19 1:13 ` Stefan Monnier
[not found] ` <jwv4ocjuvm1.fsf-monnier+emacs@gnu.org>
1 sibling, 1 reply; 29+ messages in thread
From: Drew Adams @ 2010-10-18 21:12 UTC (permalink / raw)
To: 'Stefan Monnier'; +Cc: 6256
> >> With the code I sent it does work.
> >> This is the `while' condition I sent (again):
>
> >> (while (let ((evt (read-event)))
> >> (and (equal (event-basic-type evt)
> >> (event-basic-type repeat-repeat-char))
> >> (equal (event-modifiers evt)
> >> (event-modifiers repeat-repeat-char))))
>
> But that doesn't work either in my tests: the problem is that the
> last-command-event was `mouse-4' (i.e. the up event) whereas
> read-event returns `down-mouse-4' (a subsequent read-event would
> return the `mouse-4').
No doubt that is a difference between Emacs on Windows and Emacs on GNU.
BTW, the behavior you describe doesn't really seem very right for a mouse wheel:
Why should the first wheel event be `down-'? I would think that `down-' would
only be called for when you press the mouse wheel (as in mouse-2 clicking using
the wheel). Whatever.
Seems like the Emacs mouse-wheel behavior on Windows (the events) is generally
superior to that on GNU. What about fixing the latter so that it comes up to
speed? Another problem is that it precludes having `mouse-4' and `mouse-5'
correspond to actual mouse buttons. On Windows I can make good use of those
mouse buttons when I use a mouse that has them. Using `mouse-4' and 5 as
stand-ins for the mouse wheel on GNU seems like an ugly workaround/hack. But I
don't know the details/history.
BTW - Don't know if this is related - if not, ignore for this thread, but you
might want to compare my question in emacs-devel wrt an added `<nil>' when using
the wheel in a standalone minibuffer (thread "mouse wheel events - why an extra
<nil>?").
> For my case, replacing the read-event by `read-key' happens to make
> it work (see patch below). Please confirm whether or not it fixes it
> for you, and if it doesn't, please show me the values of
> `repeat-repeat-char' and `evt' in the above test.
> + (while (let ((evt (read-key)))
> ;; For clicks, we need to strip the meta-data to
> ;; check the underlying event name.
> (eq (or (car-safe evt) evt)
Yes, that works for me too. Please install it if you see no problems with it.
Thx.
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-10-18 21:12 ` Drew Adams
@ 2010-10-19 1:13 ` Stefan Monnier
2010-10-19 6:50 ` Jan Djärv
0 siblings, 1 reply; 29+ messages in thread
From: Stefan Monnier @ 2010-10-19 1:13 UTC (permalink / raw)
To: Drew Adams
>> But that doesn't work either in my tests: the problem is that the
>> last-command-event was `mouse-4' (i.e. the up event) whereas
>> read-event returns `down-mouse-4' (a subsequent read-event would
>> return the `mouse-4').
> No doubt that is a difference between Emacs on Windows and Emacs on GNU.
> BTW, the behavior you describe doesn't really seem very right for
> a mouse wheel: Why should the first wheel event be `down-'?
Under X11, wheel-up and wheel-down events are buttons, so they get
pressed&released like any other (actually, it seems to be the case for
the hardware as well, so it's not a complete invention of the X11
layer). I.e. when you turn up the wheel by 3 steps, you get 6 events.
> I would think that `down-' would only be called for when you press the
> mouse wheel (as in mouse-2 clicking using the wheel). Whatever.
You also get such a down in that case, but that's a down-mouse-2 rather
than a down-mouse-4 or down-mouse-5: pressing on the wheel is
a completely different button from "turning the wheel".
> Seems like the Emacs mouse-wheel behavior on Windows (the events) is
> generally superior to that on GNU.
Yes, tho it's mostly not Emacs's fault. X11's handling of wheels is
not great.
> What about fixing the latter so that it comes up to speed?
> Another problem is that it precludes having `mouse-4' and `mouse-5'
> correspond to actual mouse buttons.
That's a general problem under X11, yes. There are various ways to work
around the problem, but it's a source of annoyances. Typically, the
additional buttons will then appear not as mouse-4/5 but for example as
mouse-6,7,8.
>> For my case, replacing the read-event by `read-key' happens to make
>> it work (see patch below). Please confirm whether or not it fixes it
>> for you, and if it doesn't, please show me the values of
>> `repeat-repeat-char' and `evt' in the above test.
>> + (while (let ((evt (read-key)))
>> ;; For clicks, we need to strip the meta-data to
>> ;; check the underlying event name.
>> (eq (or (car-safe evt) evt)
> Yes, that works for me too. Please install it if you see no problems
> with it. Thx.
Thanks, it's installed in emacs-23, closed,
Stefan
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-10-19 1:13 ` Stefan Monnier
@ 2010-10-19 6:50 ` Jan Djärv
2010-10-19 14:03 ` Drew Adams
0 siblings, 1 reply; 29+ messages in thread
From: Jan Djärv @ 2010-10-19 6:50 UTC (permalink / raw)
To: 6256, monnier
2010-10-19 03:13, Stefan Monnier skrev:
> That's a general problem under X11, yes. There are various ways to work
> around the problem, but it's a source of annoyances. Typically, the
> additional buttons will then appear not as mouse-4/5 but for example as
> mouse-6,7,8.
>
If you have a mouse that scrolls horizontally also it comes as mouse 6 and 7.
So extra buttons in that case would be 8, 9, 10 and so on. This makes it
har do make code that uses extra buttons, there is no unique button numbering
between different mouses except 1-5.
Jan D.
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-10-19 6:50 ` Jan Djärv
@ 2010-10-19 14:03 ` Drew Adams
0 siblings, 0 replies; 29+ messages in thread
From: Drew Adams @ 2010-10-19 14:03 UTC (permalink / raw)
To: 'Jan Djärv', 6256, monnier
> If you have a mouse that scrolls horizontally also it comes
> as mouse 6 and 7.
> So extra buttons in that case would be 8, 9, 10 and so on.
> This makes it hard do make code that uses extra buttons,
> there is no unique button numbering between different
> mouses except 1-5.
Correction: except 1-3, not 1-5 (or is it even just 1-2?).
mouse-4 and mouse-5 on Windows are the 4th and 5th mouse buttons (crazy!, I
know), while on GNU they represent wheel actions.
But apparently there's not much that Emacs can do about this.
(Did I understand that correctly?)
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
[not found] ` <jwvpqv7rp50.fsf-monnier+emacs@gnu.org>
@ 2010-10-19 19:21 ` Drew Adams
2010-10-19 20:54 ` Stefan Monnier
0 siblings, 1 reply; 29+ messages in thread
From: Drew Adams @ 2010-10-19 19:21 UTC (permalink / raw)
To: 'Stefan Monnier'; +Cc: 6256
(Putting this back into the bug-list thread, since some of my reply (#3 below)
still pertains to #6256.)
1. > Yes, tho the only change it requires is to change the lets into setqs.
Including the let-binding of `repeat-message-function'? I assume not. Doing
that would change the value globally from then on. I've left that one as a let
binding.
2. However, you say that your implementation will anyway render a let binding of
`repeat-message-function' ineffective. Do you mean your current fix using
read-key or your future reimplementation that you have locally?
I assume you mean the latter. I can't test that, so I can't say how annoying
the changed message behavior would be.
Not being able to effectively let bind `repeat-message-function' (anywhere)
would significantly reduce the utility of that variable. That's kind of the
point of such a variable - code that uses it is unlikely to just treat it as a
global setting (e.g. setq).
There needs to be some simple way to prevent `repeat' from issuing its own
message for the duration (including during all repetitions). And preferably we
would also provide a way to apply an alternative function - i.e. to do exactly
what `repeat-message' was intended to do and does currently.
It seems like your future replacement would change the notion of a repetition so
that it no longer does everything that is currently done in `repeat' (since
`repeat' is not called repetitively). Dunno. I realize you are still working
on it, but please try not to sacrifice too much in your quest for simplicity.
3. Your current read-key fix for `repeat' does not work in Emacs 23, whereas my
fix using read-event does work.
Yours works only for Emacs 24 - I'm not sure why (can you tell me perhaps?).
Mine works also in Emacs 22 and 23. (Emacs 22 has no `read-key'.) So I cannot
say I'm overjoyed with that solution.
But maybe I will be if I understand its advantages. What was your objection to
the solution I provided using `read-event'? You never stated it, IIRC.
I realize that you do not care much about Emacs 22 or 23, but it seems like a
fair amount of loss for little gain. I'd like to understand the advantage of
your current fix using read-key.
(Granted, an Emacs 22/23 user would need to eval the fixed version of `repeat',
but that's not a big deal.)
4.
> If you look at the corresponding patch included below, you'll see that
> it makes it unnecessary to deal with pre/post-command-hook or with
> undo-boundary. Actually it also handles various other details of the
> top-level loop which the current version of repeat just
> hasn't bothered to try and reproduce (e.g. moving the cursor outside
> of images/compositions/invisible text).
Yes, it looks simpler. (But you didn't show the definition of
`set-temporary-overlay-map'.)
> > I'm not objecting but asking to understand, especially
> > since it breaks the way `repeat' has functioned (its
> > interface), requiring code changes for at least some
> > `repeat' callers such as mine.
>
> Yes, that's a concern.
Consider providing some info in NEWS about how to make code compatible etc.
> I've sent my current implementation to emacs-devel in some
> other thread, but in any case it suffers from a few problems.
>
> > BTW, what's the `t' argument for?
>
> It means "keep this overlay map active as long as the user hits keys
> within that keymap". A nil value means to keep it active only for the
> very next command. You can also provide a predicate function
> to decide when to deactivate the map.
Do you actually use such optional functionality? How/where?
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-10-19 19:21 ` Drew Adams
@ 2010-10-19 20:54 ` Stefan Monnier
2010-10-19 22:17 ` Drew Adams
0 siblings, 1 reply; 29+ messages in thread
From: Stefan Monnier @ 2010-10-19 20:54 UTC (permalink / raw)
To: Drew Adams; +Cc: 6256
> 1. > Yes, tho the only change it requires is to change the lets into setqs.
> Including the let-binding of `repeat-message-function'? I assume not.
> Doing that would change the value globally from then on. I've left
> that one as a let binding.
Good.
> 2. However, you say that your implementation will anyway render a let
> binding of `repeat-message-function' ineffective. Do you mean your
> current fix using read-key or your future reimplementation that you
> have locally?
I mean my future reimplementation.
> I assume you mean the latter. I can't test that, so I can't say how annoying
> the changed message behavior would be.
Don't worry about it: it's a problem for me, as of now.
> It seems like your future replacement would change the notion of
> a repetition so that it no longer does everything that is currently
> done in `repeat' (since `repeat' is not called repetitively). Dunno.
It changes the place where the loop happens, yes: insteads of calling
repeat repetitively from repeat itself, it's called repetitively from
the toplevel command loop.
> I realize you are still working on it, but please try not to sacrifice
> too much in your quest for simplicity.
It's not for simplicity but for correctness (several corner cases fail
to work with read-key/event/char).
> 3. Your current read-key fix for `repeat' does not work in Emacs 23,
> whereas my fix using read-event does work.
I did not understand that from your answer and I'm very surprised, since
AFAIK this part of Emacs hasn't changed significantly. In any case I've
only tested it under Emacs-23 (yup, that's a 3 there).
> But maybe I will be if I understand its advantages. What was your
> objection to the solution I provided using `read-event'? You never
> stated it, IIRC.
Your characterization of what works and what doesn't makes no sense to
me (based on my understand of what the code does, and based on my
tests), so I used the code that I know to work and I understand why.
Since you say it doesn't work for you, please follow my advice:
Please confirm whether or not it fixes it
for you, and if it doesn't, please show me the values of
`repeat-repeat-char' and `evt' in the above test.
because without that info from your tests, I can't help you much further.
Stefan
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-10-19 20:54 ` Stefan Monnier
@ 2010-10-19 22:17 ` Drew Adams
2010-10-20 15:47 ` Stefan Monnier
0 siblings, 1 reply; 29+ messages in thread
From: Drew Adams @ 2010-10-19 22:17 UTC (permalink / raw)
To: 'Stefan Monnier'; +Cc: 6256
> > 3. Your current read-key fix for `repeat' does not work in Emacs 23,
> > whereas my fix using read-event does work.
>
> I did not understand that from your answer and I'm very
> surprised, since
> AFAIK this part of Emacs hasn't changed significantly. In
> any case I've
> only tested it under Emacs-23 (yup, that's a 3 there).
>
> > But maybe I will be if I understand its advantages. What was your
> > objection to the solution I provided using `read-event'? You never
> > stated it, IIRC.
>
> Your characterization of what works and what doesn't makes no sense to
> me (based on my understand of what the code does, and based on my
> tests), so I used the code that I know to work and I understand why.
>
> Since you say it doesn't work for you, please follow my advice:
>
> Please confirm whether or not it fixes it
> for you, and if it doesn't, please show me the values of
> `repeat-repeat-char' and `evt' in the above test.
>
> because without that info from your tests, I can't help you
> much further.
I tried debugging it that way. I added this just after the (let ((evt
(read-key))), that is BEFORE the `and' test of the `while':
(message "EVT: %S, R-R-CHAR: %S" evt repeat-repeat-char)
In my own Emacs setup (which has lots of stuff in it) `C-x p wheel-down' moves
correctly to the first bookmark, but then subsequent wheel-downs just start
scrolling the window. IOW, `repeat' is exited immediately - there is no
repetition at all.
And because it exits immediately _no_ debug message appears in *Messages*. That
tells me that it exited during `read-key', not because the event/key tested did
not match (`while' test = nil). Apparently `read-key' itself exits out to the
`unwind-protect' protection code in this scenario.
I tried fiddling with `read-key-delay' but that didn't help. I used 3 bookmarks,
all in the same buffer, so there should be no switch-frame events interfering
anywhere. I do use a standalone minibuffer in my setup, and that sometimes
causes `handle-switch-frame' events to happen in places one might not expect.
But I don't think that is happening here (we are not even using the minibuffer,
and everything should be taking place in the same frame). Dunno what is causing
the problem.
In emacs -Q, however, with just the Bookmark+ files loaded, I do not see the
problem. That is presumably what you are seeing.
The only way I see a problem then (without my setup) is if I rotate the wheel
more rapidly. That is no doubt because it gets a `double-wheel-down' or
`triple-wheel-down' event instead of a `wheel-down' event. I haven't bound
those events specifically to the bookmark-cycling command, but I could do so to
get around that (assuming that's the cause). For the same reason, I can see
that same rapid-rotation problem with the fix I sent. This rapid-rotation
behavior is not an issue, for me.
The issue is the behavior I see when I use my own setup, and I cannot begin to
pare things down from that setup to determine just what else might be involved.
Presumably the bad behavior has something to do with how `read-key' works -
something is apparently causing it to barf, but how `read-key' works is unclear
to me. I hope that you (who wrote it) can figure out or guess what's happening
here.
I would have guessed that if there were a problem it would have occurred because
`read-key' did not return an event/key that matched - like the problem with the
original code and with the `read-char-exclusive' attempt I made first to fix it.
But no, it never even got to the `and' test of the `while': it seems to have
exited during `read-key'.
Perhaps you can suggest something I can do to determine what is happening that
causes it (presumably) to exit the `while' during the `read-key', jumping out to
the `unwind-protect'. Is there some debug message I can put at the beginning of
the the `unwind-protect' protection code to see what happened? Can I put some
debug stuff into `read-key'?
Coming back to my question -
I see no problems at all when I instead use the solution I sent, which uses
read-event instead of read-key. It just works.
You still have not said anything about what's wrong with that solution. If your
read-key solution worked I would not hesitate to say fine, let's go with it -
believe me. But it doesn't. If we can figure it out and fix it, I'm OK with
that.
I'm willing to try to help determine what's going wrong with the `read-key'
approach, if we can do that by just debugging the `repeat' code (as opposed to
my trying to narrow down my setup).
Again (coming back to my question), this is what I suggested:
(while (let ((evt (read-event)))
(and (equal (event-basic-type evt)
(event-basic-type repeat-repeat-char))
(equal (event-modifiers evt)
(event-modifiers repeat-repeat-char))))
(repeat repeat-arg))
It's pretty simple. It also seems logical, and it says just what we want to be
done: if the event's components are all the same as before then repeat. Dunno
why you have a problem with this. What problems do you see with this approach?
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-10-19 22:17 ` Drew Adams
@ 2010-10-20 15:47 ` Stefan Monnier
2010-10-20 20:55 ` Drew Adams
0 siblings, 1 reply; 29+ messages in thread
From: Stefan Monnier @ 2010-10-20 15:47 UTC (permalink / raw)
To: Drew Adams; +Cc: 6256
> I tried debugging it that way. I added this just after the (let ((evt
> (read-key))), that is BEFORE the `and' test of the `while':
> (message "EVT: %S, R-R-CHAR: %S" evt repeat-repeat-char)
That sounds right.
> And because it exits immediately _no_ debug message appears in *Messages*.
That's very odd.
> That tells me that it exited during `read-key', not
> because the event/key tested did not match (`while' test = nil).
That's one possibility, indeed, tho maybe the code never gets to
read-key at all.
> Apparently `read-key' itself exits out to the `unwind-protect'
> protection code in this scenario.
That's only if it signals an error, which you should then see somewhere.
> In emacs -Q, however, with just the Bookmark+ files loaded, I do not
> see the problem. That is presumably what you are seeing.
Indeed. So it does work for you in Emacs-23, tho only for the case of
"emacs -Q"? If so, you may want to try and do the binary-search dance
on your .emacs to see what's interfering.
> Perhaps you can suggest something I can do to determine what is
> happening that causes it (presumably) to exit the `while' during the
> `read-key', jumping out to the `unwind-protect'. Is there some debug
> message I can put at the beginning of the the `unwind-protect'
> protection code to see what happened? Can I put some debug stuff into
> `read-key'?
You can start by adding various `message' calls around the while, inside
the while, around the read-key call, etc...
Using edebug in this code is sadly problematic, so we're left with
print-debugging.
> You still have not said anything about what's wrong with that solution.
It does not work for me (i.e. for X11 mouse wheel events).
> Again (coming back to my question), this is what I suggested:
> (while (let ((evt (read-event)))
> (and (equal (event-basic-type evt)
> (event-basic-type repeat-repeat-char))
> (equal (event-modifiers evt)
> (event-modifiers repeat-repeat-char))))
> (repeat repeat-arg))
You could also try
(while (let ((evt (read-event)))
(message "EVT: %S, R-R-CHAR: %S" evt repeat-repeat-char)
(and (equal (event-basic-type evt)
(event-basic-type repeat-repeat-char))
(equal (event-modifiers evt)
(event-modifiers repeat-repeat-char))))
(repeat repeat-arg))
> It's pretty simple. It also seems logical, and it says just what we
> want to be done: if the event's components are all the same as before
> then repeat. Dunno why you have a problem with this. What problems
> do you see with this approach?
That on X11, the second event is *not* the same as the first because
mouse wheel send first a down and then an up event.
Stefan
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-10-20 15:47 ` Stefan Monnier
@ 2010-10-20 20:55 ` Drew Adams
2010-10-21 1:08 ` Stefan Monnier
0 siblings, 1 reply; 29+ messages in thread
From: Drew Adams @ 2010-10-20 20:55 UTC (permalink / raw)
To: 'Stefan Monnier'; +Cc: 6256
> > That tells me that it exited during `read-key', not
> > because the event/key tested did not match (`while' test = nil).
>
> That's one possibility, indeed, tho maybe the code never gets to
> read-key at all.
This is what I used:
(unwind-protect
(while (let ((evt (read-key)))
(message "EVT: %S, R-R-CHAR: %S"
evt repeat-repeat-char)
(eq (or (car-safe evt) evt)
(or (car-safe repeat-repeat-char)
repeat-repeat-char)))
(repeat repeat-arg))
(setq repeat-undo-count nil))
The only time that code is not entered is if `repeat-repeat-char' is nil (since
the code is inside the (when repeat-repeat-char...)).
And if that code is entered then the only time `message' is not invoked is if
`read-key' barfs.
> > Apparently `read-key' itself exits out to the `unwind-protect'
> > protection code in this scenario.
>
> That's only if it signals an error, which you should then see
> somewhere.
I saw no error messages in *Messages*.
Perhaps we can conclude that `repeat-repeat-char' was nil. Since I have
`repeat-on-final-keystroke'=t (the default), that in turn means that
`last-command-event' was nil. Does that help you see what went wrong?
`last-command-event' takes us into C code (and perhaps into your code for
`read-key'?). I can't help much with that. As I suggested, this might have to
do with other things such as my using a standalone minibuffer. Dunno.
> > In emacs -Q, however, with just the Bookmark+ files loaded, I do not
> > see the problem. That is presumably what you are seeing.
>
> Indeed. So it does work for you in Emacs-23, tho only for the case of
> "emacs -Q"? If so, you may want to try and do the binary-search dance
> on your .emacs to see what's interfering.
I really don't want to try that for this, if I can avoid it.
> > Perhaps you can suggest something I can do to determine what is
> > happening that causes it (presumably) to exit the `while' during the
> > `read-key', jumping out to the `unwind-protect'. Is there
> > some debug message I can put at the beginning of the the
> > `unwind-protect' protection code to see what happened? Can I
> > put some debug stuff into `read-key'?
>
> You can start by adding various `message' calls around the
> while, inside the while, around the read-key call, etc...
> Using edebug in this code is sadly problematic, so we're left with
> print-debugging.
>
> > You still have not said anything about what's wrong with
> > that solution.
>
> It does not work for me (i.e. for X11 mouse wheel events).
Ah. I see. That's the first I heard of it.
I was thinking that that code would work because it remains abstract (in
principle). It just decomposes the event into its components and compares
those. There is (in principle) nothing platform-specific about it.
But I see from what you say below that the problem is that on X (and maybe on
some other platforms?) wheel events cannot be treated abstractly without some
massaging first. They have a different form depending on whether they are the
first of a series.
That form difference might be useful for X (?), and maybe it could sometimes be
useful for Emacs (?), but it is just an obstacle in this context. In general
(and in particular here) I don't think that Emacs has any need to distinguish
the first wheel event in a given direction from subsequent ones in the same
direction.
IOW, the problem is not that X uses different event names from Windows for its
wheel events. The problem (here) is that X uses different event names for the
_same_ wheel action (rotation in a given direction).
> You could also try
> (while (let ((evt (read-event)))
> (message "EVT: %S, R-R-CHAR: %S" evt repeat-repeat-char)
> (and (equal (event-basic-type evt)
> (event-basic-type repeat-repeat-char))
> (equal (event-modifiers evt)
> (event-modifiers repeat-repeat-char))))
> (repeat repeat-arg))
>
> > It's pretty simple. It also seems logical, and it says just what we
> > want to be done: if the event's components are all the same as before
> > then repeat. Dunno why you have a problem with this. What problems
> > do you see with this approach?
>
> That on X11, the second event is *not* the same as the first because
> mouse wheel send first a down and then an up event.
I see. So it sounds like we need an abstraction to deal with that - I'd think
that would be useful anyway. After all, there is no good reason to distinguish
the first wheel rotation (in a given direction). At least there is no good
reason to _always_ do that, even if someone might find a reason why that might
be useful sometimes.
The modifiers that I wanted to compare in the `while' test are only the
`control', `meta', `shift', `double', etc. modifiers. In the case of a wheel
event we could safely abstract from any `down' or `up' modifiers (here, at
least), AFAIK - they don't mean anything here.
What about defining a function that maps X wheel events to "Emacs" wheel events
that strip the `down' and `up' modifiers? And then using that function here?
But that means being able to recognize a wheel event as such - any wheel event.
We don't want to strip `down' or `up' from any non-wheel mouse events. That
recognition should be possible using the vars `mouse-wheel-(up|down)-event'.
Can you use something like this to make the decomposing-read-event approach work
for X?
(defun wheel-event (event)
"Return EVENT, with modifiers `down' and `up' removed if a wheel event."
(if (memq (event-basic-type event)
(list mouse-wheel-up-event
mouse-wheel-down-event))
event
(event-convert-list
(delq 'down (delq 'up (event-modifiers event))))))
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-10-20 20:55 ` Drew Adams
@ 2010-10-21 1:08 ` Stefan Monnier
2010-10-22 18:43 ` Drew Adams
0 siblings, 1 reply; 29+ messages in thread
From: Stefan Monnier @ 2010-10-21 1:08 UTC (permalink / raw)
To: Drew Adams; +Cc: 6256
> I was thinking that that code would work because it remains abstract (in
> principle). It just decomposes the event into its components and compares
> those. There is (in principle) nothing platform-specific about it.
In principle, your message call should have generated a message.
In principle my
(eq (or (car-safe evt) evt)
(or (car-safe repeat-repeat-char)
repeat-repeat-char))
is 100% equivalent (tho more efficient) to your
(and (equal (event-basic-type evt)
(event-basic-type repeat-repeat-char))
(equal (event-modifiers evt)
(event-modifiers repeat-repeat-char))))
But obviously, there's more at play here.
>> That on X11, the second event is *not* the same as the first because
>> mouse wheel send first a down and then an up event.
> I see. So it sounds like we need an abstraction to deal with that -
> I'd think that would be useful anyway. After all, there is no good
> reason to distinguish the first wheel rotation (in a given direction).
> At least there is no good reason to _always_ do that, even if someone
> might find a reason why that might be useful sometimes.
[ You're misunderstanding. ]
I know how to deal with the X11 side. It's dealt with, the code works.
Let's concentrate on your case. E.g.:
>> You could also try
>> (while (let ((evt (read-event)))
>> (message "EVT: %S, R-R-CHAR: %S" evt repeat-repeat-char)
>> (and (equal (event-basic-type evt)
>> (event-basic-type repeat-repeat-char))
>> (equal (event-modifiers evt)
>> (event-modifiers repeat-repeat-char))))
>> (repeat repeat-arg))
Or adding the suggested `message' calls before/after read-key or
elsewhere to try and see what are the values of repeat-repeat-char at
various places rather than try and guess them.
Stefan
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-10-21 1:08 ` Stefan Monnier
@ 2010-10-22 18:43 ` Drew Adams
2010-10-22 19:47 ` Stefan Monnier
0 siblings, 1 reply; 29+ messages in thread
From: Drew Adams @ 2010-10-22 18:43 UTC (permalink / raw)
To: 'Stefan Monnier'; +Cc: 6256
> >> You could also try
> >> (while (let ((evt (read-event)))
> >> (message "EVT: %S, R-R-CHAR: %S" evt repeat-repeat-char)
> >> (and (equal (event-basic-type evt)
> >> (event-basic-type repeat-repeat-char))
> >> (equal (event-modifiers evt)
> >> (event-modifiers repeat-repeat-char))))
> >> (repeat repeat-arg))
>
> Or adding the suggested `message' calls before/after read-key or
> elsewhere to try and see what are the values of repeat-repeat-char at
> various places rather than try and guess them.
OK, I retested, and I no longer see the problem. In the last few days I've
changed some of the code I use, but not in any way that I would expect would
make a difference here (i.e., in presumably unrelated areas).
So I don't understand it. But the good news is that your code works for me now.
If I had to guess, I'd guess that the problem was that I was loading the new
code but then when I used the mouse-wheel commands the library they were defined
in invoked `(require repeat.el)', and that redefined the function `repeat' as it
was originally.
That would be my guess. But I could have sworn that I used `C-M-x' several
times during testing (e.g. after adding a call to `message'), so I should have
at some point tested with the new definition. When I tested before, it
systematically did not work. Anyway, it works now. Sorry for not testing
correctly the first time.
I checked also that it works with modifiers (e.g. C-S-wheel-up).
(I assume that you tested that also on GNU.)
So I'm OK with your fix. The only disadvantage wrt my fix, for my use (on
Windows), is that it will not work for Emacs prior to 23.2, when `read-key' was
introduced. (I know you don't care about that.)
Thx.
What changes will I need to make to my code, if any, after you move to the new
implementation that uses `set-temporary-overlay-map'? I think you said none,
since I changed the let bindings to setq. Can you confirm or let me know what
will need to change?
^ permalink raw reply [flat|nested] 29+ messages in thread
* bug#6256: 24.0.50; read-event in `repeat' command
2010-10-22 18:43 ` Drew Adams
@ 2010-10-22 19:47 ` Stefan Monnier
0 siblings, 0 replies; 29+ messages in thread
From: Stefan Monnier @ 2010-10-22 19:47 UTC (permalink / raw)
To: Drew Adams; +Cc: 6256
> OK, I retested, and I no longer see the problem. In the last few days I've
> changed some of the code I use, but not in any way that I would expect would
> make a difference here (i.e., in presumably unrelated areas).
Good, thanks.
> What changes will I need to make to my code, if any, after you move to
> the new implementation that uses `set-temporary-overlay-map'?
None for now,
Stefan
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2010-10-22 19:47 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-24 15:11 bug#6256: 24.0.50; read-event in `repeat' command Drew Adams
2010-05-24 16:28 ` Drew Adams
2010-05-24 23:05 ` Drew Adams
2010-05-25 0:06 ` Lennart Borgman
2010-05-25 2:41 ` Stefan Monnier
2010-07-03 21:24 ` Drew Adams
2010-07-04 22:45 ` Stefan Monnier
2010-07-07 14:43 ` Drew Adams
2010-07-21 15:54 ` Drew Adams
2010-08-28 15:19 ` Drew Adams
2010-09-11 18:25 ` Stefan Monnier
2010-09-11 22:34 ` Drew Adams
2010-09-12 16:06 ` Drew Adams
2010-09-17 3:34 ` Drew Adams
2010-09-22 14:01 ` bug#6256: [PATCH] " Drew Adams
2010-09-25 14:30 ` Drew Adams
2010-10-18 18:40 ` Stefan Monnier
2010-10-18 21:12 ` Drew Adams
2010-10-19 1:13 ` Stefan Monnier
2010-10-19 6:50 ` Jan Djärv
2010-10-19 14:03 ` Drew Adams
[not found] ` <jwv4ocjuvm1.fsf-monnier+emacs@gnu.org>
[not found] ` <0658C0CCC79D466BA9DE233F5980CAE5@us.oracle.com>
[not found] ` <jwvpqv7rp50.fsf-monnier+emacs@gnu.org>
2010-10-19 19:21 ` Drew Adams
2010-10-19 20:54 ` Stefan Monnier
2010-10-19 22:17 ` Drew Adams
2010-10-20 15:47 ` Stefan Monnier
2010-10-20 20:55 ` Drew Adams
2010-10-21 1:08 ` Stefan Monnier
2010-10-22 18:43 ` Drew Adams
2010-10-22 19:47 ` Stefan Monnier
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).