bug#29478: [Patch] bug#29478: 26.0.90; `C-h k' followed by mouse clicks no longer shows down event

[Alan Mackenzie]

Hello, Stefan.

On Sat, Dec 23, 2017 at 11:33:11 -0500, Stefan Monnier wrote:
[ .... ]

> > OK, I have a provisional fix.  The problem was that the double-click-time
> > loop in help-read-key-sequence was discarding all events but the last
> > one.  I have amended the loop so that _all_ these events are stored in a
> > list, and we then discard the most recent events till we find one with a
> > binding.

> I must say after reading the whole thread and looking at the code,
> I still don't really understand what's the idea behind this patch (nor
> exactly which problems we're trying to solve).

To make C-h c/k <shift><mouse-1> display "mouse-appearance-menu" rather
than "key sequence unbound".  The idea behind the code is to return the
last key sequence which is bound, rather than the very last one, which
may not be.

> The way I understand the problem (which may be naive because I haven't
> tried to play with the code yet, so I'm probably missing some subtlety),
> the code should "simply" read events until the event read is not a down
> event (and skipping pseudo events like mouse-movements and help-echo).
> And then return all those events (it's probably not going to be more
> than 2, but I don't see any benefit in hard-coding the fact that we
> return either 1 or 2 events, we could just say "returns a list of
> events").

Out of all these key sequences, some code somewhere has to select the
one for which the help message will be displayed.  Perhaps arbitrarily,
this is now in help-read-key-sequence.

> BTW, here are some comments about the current code:

> >                    (and no-mouse-movement
> >                         (string-match "mouse-movement" keyname))
> > +                  (progn (push key keys) nil)
> >                    (and (string-match "\\(mouse\\|down\\|click\\|drag\\)"
> >                                       keyname)

> Why string-match on event names rather than use things like
> event-basic-type, and event-modifiers?

Lack of knowledge on the part of the person who wrote it (me).

> >                         (progn
> > @@ -739,13 +740,31 @@ help-read-key-sequence
> >                           (sleep-for 0.01)
> >                           (while (read-event nil nil 0.01))

> We're blindly throwing away events here, right?  Isn't that a problem?

No, it's a filter.  Things like <help-echo>, which otherwise triggered
the sit-for are now discarded, so that only user events are processed.
I know of nobody who can press mouse buttons as fast as 100 times a
second.  All this was diagnosed in the early stages of another bug
report, bug #29272.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).