Subject: bug#68796: Stefan's wheel event change

From: Eli Zaretskii <[email-masked]>
Date: Thu, 04 Apr 2024 08:21:43 +0300

> Cc: [email-masked]
> Date: Wed, 03 Apr 2024 15:27:54 -0400
> From:  Stefan Monnier via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <[email-masked]>
> 
> > I'm now satisfied that Stefan's change to return wheel events on all
> > platforms does not produce the adverse effects I feared, but I have also
> > misplaced the number of the bug report where it was discussed, so,
> > Stefan, please post your version of the patch again before it is
> > installed.  Thanks in advance.
> 
> Here it is, with a brand new etc/NEWS,

Thanks, a few minor comments below.

> Change the handling of the old X11 convention to use mouse-4/5/6/7
> events to represent wheel events: instead of asking downstream
> packages to use the `mouse-wheel-*-event` variables to know which events
> represent wheel events, use new var `mouse-wheel-buttons` to directly
> convert those events into the standard `wheel-up/down/left/right` events
> used everywhere else.
> 
> This will simplify the work of packages which can thus just bind their
> commands to `wheel-up/down/left/right`.
> 
> * lisp/mouse.el (mouse-wheel-buttons): New custom variable.
> 
> * src/keyboard.c (make_lispy_event): Adjust for "wheel-clicks" on the tab-bar.
> * src/xterm.c (x_construct_mouse_click): Add `xi2` argument and
> obey `mouse-wheel-buttons` variable.
> (handle_one_xevent): Adjust calls accordingly.
> (syms_of_xterm): Define the `mouse-wheel-buttons` and the
> `wheel-up/down/left/right`symbols.
> 
> * lisp/xt-mouse.el: Don't require `mwheel` any more.
> (xterm-mouse--same-button-p): Delete function.
> (xterm-mouse--read-event-sequence): Use `mouse-wheel-buttons`.

Nit-picking: can we please NOT use Markdown-style quoting `like this`
in our docs and change logs?

> +** Mouse wheel events should now always be 'wheel-up/down/left/right'.
> +At those places where the old 'mouse-4/5/6/7' events could still occur
> +(i.e. X11 input in the absence of XInput2, and `xterm-mouse-mode`),
> +we remap them to the corresponding 'wheel-up/down/left/right' event,
> +according to the new variable 'mouse-wheel-buttons'.

Likewise here.

> +(defcustom mouse-wheel-buttons
> +  '((4 . wheel-up) (5 . wheel-down) (6 . wheel-left) (7 . wheel-right))
> +  "Buttons to remap to wheel events.

Suggest to rephrase:

  "How to remap mouse button numbers to wheel events."

> +This is an alist of (NUMBER . SYMBOL) used to remap old-style mouse wheel
> +events represented as mouse button events.  It remaps mouse button event
> +NUMBER to the event SYMBOL.  SYMBOL must be one of `wheel-up', `wheel-down',

Not "mouse event NUMBER", but "mouse button NUMBER", I think.
Alternatively, you could say "mouse button event 'mouse-NUMBER'".
IOW, NUMBER is not a mouse event, it's a number.

> +  /* Convert pre-XInput2 wheel events represented as mouse-clicks.  */
> +  if (!xi2)
> +    {
> +      Lisp_Object base
> +        = Fcdr_safe (Fassq (make_fixnum (result->code + 1),
> +                            Fsymbol_value (Qmouse_wheel_buttons)));

Should this test mouse-wheel-buttons for being bound and have a list
value, before calling assq?  mouse.el is preloaded relatively late in
loadup, so I think we had better played it safe, no?