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

[Stefan Monnier <monnier@IRO.UMontreal.CA>]

> Of course!  read-key-sequence consumes all the events up to the
> S-down-mouse-1, leaving the following S-mouse-1 in the event buffer.
> This is read at the next iteration of the command loop.

Exactly.

>> CC'ing Stefan in the hope that he could have some insights.
>
> 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).

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").

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?

>                         (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?


        Stefan