bug#68796: xterm.c: Convert mouse-4/5/6/7 to wheel-up/down/left/right

bug#68796: xterm.c: Convert mouse-4/5/6/7 to wheel-up/down/left/right

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 1393 bytes --]

Tags: patch

Following up on the `xt-mouse.el` patch (bug#68698), I now propose we do
the same for those `mouse-4/5/6/7` events generated by old-style X11 input.

IIUC Po Lu wasn't favorable to the idea, but maybe seeing the code
will make a convert?

Beside that, the patch has 2 remaining issues:
- Lack of corresponding updates to `etc/NEWS` and `doc/lispref/commands.texi`.
- Lack of testing for the left/right remapping.  This is *really*
  necessary because even after (re)reading the various parts of the code
  that handle horizontal scrolling, I'm still very much unsure if I got
  the direction right[left?], so if you have a mouse with a horizontal wheel,
  please compare the `wheel-left/right` events generated with and
  without building `--without-xinput2`.


        Stefan

 In GNU Emacs 30.0.50 (build 2, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.16.0, Xaw3d scroll bars) of 2024-01-25 built on pastel
Repository revision: 5945fab9af0b52cc55f06b2c7e911d51e34a9cdf
Repository branch: work
Windowing system distributor 'The X.Org Foundation', version 11.0.12101007
System Description: Debian GNU/Linux 12 (bookworm)

Configured using:
 'configure -C --enable-checking --enable-check-lisp-object-type --with-modules --with-cairo --with-tiff=ifavailable
 'CFLAGS=-Wall -g3 -Og -Wno-pointer-sign' --without-native-compilation
 PKG_CONFIG_PATH=/home/monnier/lib/pkgconfig'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-xterm.c-Map-old-style-wheel-buttons-to-actual-wheel-.patch --]
[-- Type: text/patch, Size: 12291 bytes --]

From 60e6903ae12546db93f149b1f184ffc7ccdc30c2 Mon Sep 17 00:00:00 2001
From: Stefan Monnier <monnier@iro.umontreal.ca>
Date: Mon, 29 Jan 2024 09:35:09 -0500
Subject: [PATCH] xterm.c: Map old-style wheel buttons to actual wheel events

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 those variables in `xterm.c` to directly
convert those events into the standard `wheel-up/down/left/right` events
used everywhere else.

This simplifies the work of packages which can now just bind their
commands to `wheel-up/down/left/right`.

* src/keyboard.c (base_mouse_symbol): New function.
* src/keyboard.h (base_mouse_symbol): Declare.
* src/xterm.c (x_construct_mouse_click): Obey `mouse-wheel-*-event` vars.
(syms_of_xterm): Define the `mouse-wheel-*-event` symbols.

* lisp/completion-preview.el (completion-preview--mouse-map): Don't
bind to `mouse-wheel-*-event`s any more.
* lisp/edmacro.el (edmacro-fix-menu-commands): Remove special casing of
`mouse-wheel-*-event`s.
* lisp/mwheel.el (mouse-wheel-down-event, mouse-wheel-up-event): Remove :set.
(mouse-wheel--setup-bindings): Don't bind to `mouse-wheel-*-event`s any more.
* lisp/progmodes/flymake.el (flymake--mode-line-counter-map): Don't
bind to `mouse-wheel-*-event`s any more if Emacs is recent enough.
---
 lisp/completion-preview.el | 10 +---------
 lisp/edmacro.el            |  8 +-------
 lisp/mwheel.el             | 31 +++++++++++--------------------
 lisp/progmodes/flymake.el  | 19 +++++++++----------
 src/fns.c                  |  1 +
 src/keyboard.c             |  9 +++++++++
 src/keyboard.h             |  2 +-
 src/xterm.c                | 31 +++++++++++++++++++++++++++++--
 8 files changed, 62 insertions(+), 49 deletions(-)

diff --git a/lisp/completion-preview.el b/lisp/completion-preview.el
index 6fd60f3c416..6bbf1246a9a 100644
--- a/lisp/completion-preview.el
+++ b/lisp/completion-preview.el
@@ -52,8 +52,6 @@
 
 ;;; Code:
 
-(require 'mwheel)
-
 (defgroup completion-preview nil
   "In-buffer completion preview."
   :group 'completion)
@@ -135,14 +133,8 @@ completion-preview--mouse-map
   "<down-mouse-1>" #'completion-preview-insert
   "C-<down-mouse-1>" #'completion-at-point
   "<down-mouse-2>" #'completion-at-point
-  ;; BEWARE: `mouse-wheel-UP-event' corresponds to `wheel-DOWN' events
-  ;; and vice versa!!
   "<wheel-up>"     #'completion-preview-prev-candidate
-  "<wheel-down>"   #'completion-preview-next-candidate
-  (key-description (vector mouse-wheel-up-event))
-  #'completion-preview-next-candidate
-  (key-description (vector mouse-wheel-down-event))
-  #'completion-preview-prev-candidate)
+  "<wheel-down>"   #'completion-preview-next-candidate)
 
 (defvar-local completion-preview--overlay nil)
 
diff --git a/lisp/edmacro.el b/lisp/edmacro.el
index abfc380d154..a80fe973240 100644
--- a/lisp/edmacro.el
+++ b/lisp/edmacro.el
@@ -725,10 +725,6 @@ edmacro-fix-menu-commands
       (let (result)
         ;; Not preloaded in a --without-x build.
         (require 'mwheel)
-        (defvar mouse-wheel-down-event)
-        (defvar mouse-wheel-up-event)
-        (defvar mouse-wheel-right-event)
-        (defvar mouse-wheel-left-event)
 	;; Make a list of the elements.
 	(setq macro (append macro nil))
 	(dolist (ev macro)
@@ -744,9 +740,7 @@ edmacro-fix-menu-commands
 		;; info is recorded in macros to make this possible.
 		((or (mouse-event-p ev) (mouse-movement-p ev)
 		     (memq (event-basic-type ev)
-			   `( ,mouse-wheel-down-event ,mouse-wheel-up-event
-			      ,mouse-wheel-right-event ,mouse-wheel-left-event
-			      wheel-down wheel-up wheel-left wheel-right)))
+			   '(wheel-down wheel-up wheel-left wheel-right)))
 		 nil)
 		(noerror nil)
 		(t
diff --git a/lisp/mwheel.el b/lisp/mwheel.el
index 66a1fa1a706..82a2c48fd5d 100644
--- a/lisp/mwheel.el
+++ b/lisp/mwheel.el
@@ -65,15 +65,13 @@ mouse-wheel-down-event
   (if mouse-wheel-obey-old-style-wheel-buttons 'mouse-4)
   "Event used for scrolling down, beside `wheel-up', if any."
   :group 'mouse
-  :type 'symbol
-  :set #'mouse-wheel-change-button)
+  :type 'symbol)
 
 (defcustom mouse-wheel-up-event
   (if mouse-wheel-obey-old-style-wheel-buttons 'mouse-5)
   "Event used for scrolling up, beside `wheel-down', if any."
   :group 'mouse
-  :type 'symbol
-  :set #'mouse-wheel-change-button)
+  :type 'symbol)
 
 (defcustom mouse-wheel-click-event 'mouse-2
   "Event that should be temporarily inhibited after mouse scrolling.
@@ -464,25 +462,18 @@ mouse-wheel--setup-bindings
     (cond
      ;; Bindings for changing font size.
      ((and (consp binding) (eq (cdr binding) 'text-scale))
-      (dolist (event (list mouse-wheel-down-event mouse-wheel-up-event
-                           'wheel-down 'wheel-up))
-        (when event
-          (mouse-wheel--add-binding `[,(append (car binding) (list event))]
-                                    'mouse-wheel-text-scale))))
+      (dolist (event '(wheel-down wheel-up))
+        (mouse-wheel--add-binding `[,(append (car binding) (list event))]
+                                  'mouse-wheel-text-scale)))
      ((and (consp binding) (eq (cdr binding) 'global-text-scale))
-      (dolist (event (list mouse-wheel-down-event mouse-wheel-up-event
-                           'wheel-down 'wheel-up))
-        (when event
-          (mouse-wheel--add-binding `[,(append (car binding) (list event))]
-                                    'mouse-wheel-global-text-scale))))
+      (dolist (event '(wheel-down wheel-up))
+        (mouse-wheel--add-binding `[,(append (car binding) (list event))]
+                                  'mouse-wheel-global-text-scale)))
      ;; Bindings for scrolling.
      (t
-      (dolist (event (list mouse-wheel-down-event mouse-wheel-up-event
-                           mouse-wheel-left-event mouse-wheel-right-event
-                           'wheel-down 'wheel-up 'wheel-left 'wheel-right))
-        (when event
-          (dolist (key (mouse-wheel--create-scroll-keys binding event))
-            (mouse-wheel--add-binding key 'mwheel-scroll))))))))
+      (dolist (event '(wheel-down wheel-up wheel-left wheel-right))
+        (dolist (key (mouse-wheel--create-scroll-keys binding event))
+          (mouse-wheel--add-binding key 'mwheel-scroll)))))))
 
 (when mouse-wheel-mode
   (mouse-wheel--setup-bindings))
diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index 5974f076556..1411a669891 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -114,10 +114,6 @@
 (require 'thingatpt) ; end-of-thing
 (require 'warnings) ; warning-numeric-level, display-warning
 (require 'compile) ; for some faces
-;; We need the next `require' to avoid compiler warnings and run-time
-;; errors about mouse-wheel-up/down-event in builds --without-x, where
-;; mwheel is not preloaded.
-(require 'mwheel)
 ;; when-let*, if-let*, hash-table-keys, hash-table-values:
 (eval-when-compile (require 'subr-x))
 (require 'project)
@@ -1637,16 +1633,19 @@ flymake--mode-line-counter-scroll-next
 
 (defvar flymake--mode-line-counter-map
   (let ((map (make-sparse-keymap)))
-    ;; BEWARE: `mouse-wheel-UP-event' corresponds to `wheel-DOWN' events
-    ;; and vice versa!!
-    (define-key map (vector 'mode-line mouse-wheel-down-event)
-                #'flymake--mode-line-counter-scroll-prev)
     (define-key map [mode-line wheel-down]
                 #'flymake--mode-line-counter-scroll-next)
-    (define-key map (vector 'mode-line mouse-wheel-up-event)
-                #'flymake--mode-line-counter-scroll-next)
     (define-key map [mode-line wheel-up]
                 #'flymake--mode-line-counter-scroll-prev)
+    (when (< emacs-major-version 30)
+      (require 'mwheel)
+      (defvar mouse-wheel-down-event) (defvar mouse-wheel-up-event)
+      ;; BEWARE: `mouse-wheel-UP-event' corresponds to `wheel-DOWN' events
+      ;; and vice versa!!
+      (define-key map (vector 'mode-line mouse-wheel-down-event)
+                  #'flymake--mode-line-counter-scroll-prev)
+      (define-key map (vector 'mode-line mouse-wheel-up-event)
+                  #'flymake--mode-line-counter-scroll-next))
     map))
 
 (defun flymake--mode-line-counter-1 (type)
diff --git a/src/fns.c b/src/fns.c
index e4fa8157000..48d7b217cb6 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -4855,6 +4855,7 @@ check_mutable_hash_table (Lisp_Object obj, struct Lisp_Hash_Table *h)
    HASH is a previously computed hash code of KEY.
    Value is the index of the entry in H matching KEY.  */
 
+/* FIXME: Callers shouldn't care about the return value!  */
 ptrdiff_t
 hash_put (struct Lisp_Hash_Table *h, Lisp_Object key, Lisp_Object value,
 	  hash_hash_t hash)
diff --git a/src/keyboard.c b/src/keyboard.c
index 1f7253a7da1..7c1822b3423 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -7597,6 +7597,15 @@ modify_event_symbol (ptrdiff_t symbol_num, int modifiers, Lisp_Object symbol_kin
   /* Apply modifiers to that symbol.  */
   return apply_modifiers (modifiers, value);
 }
+
+Lisp_Object
+base_mouse_symbol (int button)
+{
+   return modify_event_symbol (button, 0,
+			       Qmouse_click, Vlispy_mouse_stem,
+			       NULL, &mouse_syms, ASIZE (mouse_syms));
+}
+
 \f
 /* Convert a list that represents an event type,
    such as (ctrl meta backspace), into the usual representation of that
diff --git a/src/keyboard.h b/src/keyboard.h
index 68e68bc2ae3..083b16e4896 100644
--- a/src/keyboard.h
+++ b/src/keyboard.h
@@ -456,7 +456,7 @@ #define EVENT_HEAD_KIND(event_head) \
 extern Lisp_Object read_char (int, Lisp_Object, Lisp_Object,
                               bool *, struct timespec *);
 extern int parse_solitary_modifier (Lisp_Object symbol);
-
+extern Lisp_Object base_mouse_symbol (int button);
 
 /* This is like Vthis_command, except that commands never set it.  */
 extern Lisp_Object real_this_command;
diff --git a/src/xterm.c b/src/xterm.c
index c8a43785564..fcbe7a1ec4f 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -14553,8 +14553,6 @@ x_construct_mouse_click (struct input_event *result,
   int x = event->x;
   int y = event->y;
 
-  /* Make the event type NO_EVENT; we'll change that when we decide
-     otherwise.  */
   result->kind = MOUSE_CLICK_EVENT;
   result->code = event->button - Button1;
   result->timestamp = event->time;
@@ -14564,6 +14562,30 @@ x_construct_mouse_click (struct input_event *result,
 			  ? up_modifier
 			  : down_modifier));
 
+  /* Convert pre-XInput2 wheel events represented as mouse-clicks.  */
+#ifdef HAVE_XINPUT2
+  if (!dpyinfo->supports_xi2)
+#endif
+    {
+      Lisp_Object base = base_mouse_symbol (result->code);
+      int wheel
+       /* BEWARE: `mouse-wheel-UP-event' corresponds to
+          `wheel-DOWN' events and vice versa!!  */
+        = BASE_EQ   (base, find_symbol_value (Qmouse_wheel_up_event))    ? 0
+          : BASE_EQ (base, find_symbol_value (Qmouse_wheel_down_event))  ? 1
+          : BASE_EQ (base, find_symbol_value (Qmouse_wheel_left_event))  ? 2
+          : BASE_EQ (base, find_symbol_value (Qmouse_wheel_right_event)) ? 3
+          : -1;
+      if (wheel >= 0)
+        {
+          result->kind = (event->type != ButtonRelease ? NO_EVENT
+		          : wheel & 2 ? HORIZ_WHEEL_EVENT : WHEEL_EVENT);
+          result->code = 0;         /* Not used.  */
+          result->modifiers &= ~(up_modifier || down_modifier);
+          result->modifiers |= wheel & 1 ? up_modifier : down_modifier;
+        }
+    }
+
   /* If result->window is not the frame's edit widget (which can
      happen with GTK+ scroll bars, for example), translate the
      coordinates so they appear at the correct position.  */
@@ -32444,6 +32466,11 @@ syms_of_xterm (void)
   DEFSYM (Qexpose, "expose");
   DEFSYM (Qdont_save, "dont-save");
 
+  DEFSYM (Qmouse_wheel_down_event, "mouse-wheel-down-event");
+  DEFSYM (Qmouse_wheel_up_event, "mouse-wheel-up-event");
+  DEFSYM (Qmouse_wheel_left_event, "mouse-wheel-left-event");
+  DEFSYM (Qmouse_wheel_right_event, "mouse-wheel-right-event");
+
 #ifdef USE_GTK
   xg_default_icon_file = build_pure_c_string ("icons/hicolor/scalable/apps/emacs.svg");
   staticpro (&xg_default_icon_file);
-- 
2.43.0

bug#68796: xterm.c: Convert mouse-4/5/6/7 to wheel-up/down/left/right

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

>>From 60e6903ae12546db93f149b1f184ffc7ccdc30c2 Mon Sep 17 00:00:00 2001
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Mon, 29 Jan 2024 09:35:09 -0500
> Subject: [PATCH] xterm.c: Map old-style wheel buttons to actual wheel events
>
> 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 those variables in `xterm.c` to directly
> convert those events into the standard `wheel-up/down/left/right` events
> used everywhere else.
>
> This simplifies the work of packages which can now just bind their
> commands to `wheel-up/down/left/right`.

I'm still opposed to this for reasons I will try to communicate below.
First, there are several logic errors in this patch:

> +  /* Convert pre-XInput2 wheel events represented as mouse-clicks.  */
> +#ifdef HAVE_XINPUT2
> +  if (!dpyinfo->supports_xi2)
> +#endif

dpyinfo->supports_xi2 is no guarantee that exclusively input extension
events will be received, but only indicates that the X display supports
the input extension.  There are several scenarios where it may be true
while core events are still delivered to windows, such as a combination
of GTK 3 and GDK_CORE_DEVICE_EVENTS=1's being enabled.  The correct test
of event type is whether x_construct_mouse_click is being called from
the XI_ButtonPress or XI_ButtonRelease label in handle_one_xevent.

Incidentally, servers with the input extension always assign buttons 4
through 7 to wheel movement.

> +      Lisp_Object base = base_mouse_symbol (result->code);
> +      int wheel
> +       /* BEWARE: `mouse-wheel-UP-event' corresponds to
> +          `wheel-DOWN' events and vice versa!!  */
> +        = BASE_EQ   (base, find_symbol_value (Qmouse_wheel_up_event))    ? 0
> +          : BASE_EQ (base, find_symbol_value (Qmouse_wheel_down_event))  ? 1
> +          : BASE_EQ (base, find_symbol_value (Qmouse_wheel_left_event))  ? 2
> +          : BASE_EQ (base, find_symbol_value (Qmouse_wheel_right_event)) ? 3
> +          : -1;
> +      if (wheel >= 0)
> +        {
> +          result->kind = (event->type != ButtonRelease ? NO_EVENT
> +		          : wheel & 2 ? HORIZ_WHEEL_EVENT : WHEEL_EVENT);
> +          result->code = 0;         /* Not used.  */
> +          result->modifiers &= ~(up_modifier || down_modifier);
> +          result->modifiers |= wheel & 1 ? up_modifier : down_modifier;
> +        }
> +    }

Isn't this insufficient to completely supersede mouse events by wheel
events, considering that such events will still be delivered as mouse
button events to tab bars?

Lastly, plenty of old code that binds just the button events will be
broken, and the code that keeps track of button-down and up events in
read_key_sequence and friends will cease applying to X wheel events.

This area has given me a lot of grief over the years (as it has given
you), so I'm reluctant to see any change there, as no matter how well
the argument in favor of its safety is posed, some unforeseen problem
invariably emerges afterwards.

bug#68796: xterm.c: Convert mouse-4/5/6/7 to wheel-up/down/left/right

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 3764 bytes --]

>> +  /* Convert pre-XInput2 wheel events represented as mouse-clicks.  */
>> +#ifdef HAVE_XINPUT2
>> +  if (!dpyinfo->supports_xi2)
>> +#endif
>
> dpyinfo->supports_xi2 is no guarantee that exclusively input extension
> events will be received, but only indicates that the X display supports
> the input extension.  There are several scenarios where it may be true
> while core events are still delivered to windows, such as a combination
> of GTK 3 and GDK_CORE_DEVICE_EVENTS=1's being enabled.

Hmm... so IIUC you're saying that even when `dpyinfo->supports_xi2` is
true, we may receive `mouse-4/5/6/7` events for wheel motions?
Yuck!

> The correct test of event type is whether x_construct_mouse_click is
> being called from the XI_ButtonPress or XI_ButtonRelease label in
> handle_one_xevent.

OK, that sounds fixable.  See additional patch below.

> Incidentally, servers with the input extension always assign buttons 4
> through 7 to wheel movement.

Interesting.

>> +      Lisp_Object base = base_mouse_symbol (result->code);
>> +      int wheel
>> +       /* BEWARE: `mouse-wheel-UP-event' corresponds to
>> +          `wheel-DOWN' events and vice versa!!  */
>> +        = BASE_EQ   (base, find_symbol_value (Qmouse_wheel_up_event))    ? 0
>> +          : BASE_EQ (base, find_symbol_value (Qmouse_wheel_down_event))  ? 1
>> +          : BASE_EQ (base, find_symbol_value (Qmouse_wheel_left_event))  ? 2
>> +          : BASE_EQ (base, find_symbol_value (Qmouse_wheel_right_event)) ? 3
>> +          : -1;
>> +      if (wheel >= 0)
>> +        {
>> +          result->kind = (event->type != ButtonRelease ? NO_EVENT
>> +		          : wheel & 2 ? HORIZ_WHEEL_EVENT : WHEEL_EVENT);
>> +          result->code = 0;         /* Not used.  */
>> +          result->modifiers &= ~(up_modifier || down_modifier);
>> +          result->modifiers |= wheel & 1 ? up_modifier : down_modifier;
>> +        }
>> +    }
>
> Isn't this insufficient to completely supersede mouse events by wheel
> events, considering that such events will still be delivered as mouse
> button events to tab bars?

Hmm... indeed there was a crash here, which I just fixed in the
patch below.  But with that fix, I do see `wheel-up/down` events on the
tab-tab as well.  Am I missing something?

> Lastly, plenty of old code that binds just the button events will be
> broken,

Indeed.  But that should be a disappearing breed because such bindings
never worked under non-X11 GUIs and now they don't work either under X11
builds that use XInput2, which I expect is the vast majority of Emacs≥29
builds in GNU/Linux distributions.

> and the code that keeps track of button-down and up events in
> read_key_sequence and friends will cease applying to X wheel events.

Hmm... the only code I can think of in `read_key_sequence` which cares
about button up/down is the code that drops the button down events and
the one that turns pairs into drag events.  Is that ever useful
currently with the `mouse-4/5/6/7` events?  No one has lamented this
under non-X11 builds so far nor under XInput2, so it seems unlikely to
be a problem.

> This area has given me a lot of grief over the years (as it has given
> you), so I'm reluctant to see any change there, as no matter how well
> the argument in favor of its safety is posed, some unforeseen problem
> invariably emerges afterwards.

There's no doubt that event representation is a delicate/nasty part of
Emacs code, but I think it's really a trade-off between possible
short-term pain and the longer term simplification of unifying all wheel
handling under `wheel-*` events.
Also, I think we *will* have go through that pain sooner or later.


        Stefan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: xterm-wheel-events.patch --]
[-- Type: text/x-diff, Size: 3744 bytes --]

diff --git a/src/keyboard.c b/src/keyboard.c
index 7c1822b3423..a881c3d6117 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -6628,8 +6628,13 @@ make_lispy_event (struct input_event *event)
 
 	if (CONSP (event->arg))
 	  return list5 (head, position, make_fixnum (double_click_count),
-			XCAR (event->arg), Fcons (XCAR (XCDR (event->arg)),
-						  XCAR (XCDR (XCDR (event->arg)))));
+			XCAR (event->arg),
+			/* FIXME: I don't know what I'm doing here.  */
+			(CONSP (XCDR (event->arg))
+			 && CONSP (XCDR (XCDR (event->arg))))
+			? Fcons (XCAR (XCDR (event->arg)),
+			         XCAR (XCDR (XCDR (event->arg))))
+			: Qnil);
         else if (NUMBERP (event->arg))
           return list4 (head, position, make_fixnum (double_click_count),
                         event->arg);
diff --git a/src/xterm.c b/src/xterm.c
index fcbe7a1ec4f..237b705b555 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -14548,7 +14548,7 @@ x_query_pointer (Display *dpy, Window w, Window *root_return,
 static Lisp_Object
 x_construct_mouse_click (struct input_event *result,
                          const XButtonEvent *event,
-                         struct frame *f)
+                         struct frame *f, bool xi2)
 {
   int x = event->x;
   int y = event->y;
@@ -14563,9 +14563,7 @@ x_construct_mouse_click (struct input_event *result,
 			  : down_modifier));
 
   /* Convert pre-XInput2 wheel events represented as mouse-clicks.  */
-#ifdef HAVE_XINPUT2
-  if (!dpyinfo->supports_xi2)
-#endif
+  if (!xi2)
     {
       Lisp_Object base = base_mouse_symbol (result->code);
       int wheel
@@ -21895,13 +21893,14 @@ handle_one_xevent (struct x_display_info *dpyinfo,
 			&& event->xbutton.time > ignore_next_mouse_click_timeout)
 		      {
 			ignore_next_mouse_click_timeout = 0;
-			x_construct_mouse_click (&inev.ie, &event->xbutton, f);
+			x_construct_mouse_click (&inev.ie, &event->xbutton,
+			                         f, false);
 		      }
 		    if (event->type == ButtonRelease)
 		      ignore_next_mouse_click_timeout = 0;
 		  }
 		else
-		  x_construct_mouse_click (&inev.ie, &event->xbutton, f);
+		  x_construct_mouse_click (&inev.ie, &event->xbutton, f, false);
 
 		*finish = X_EVENT_DROP;
 		goto OTHER;
@@ -21971,13 +21970,15 @@ handle_one_xevent (struct x_display_info *dpyinfo,
                           && event->xbutton.time > ignore_next_mouse_click_timeout)
                         {
                           ignore_next_mouse_click_timeout = 0;
-                          x_construct_mouse_click (&inev.ie, &event->xbutton, f);
+                          x_construct_mouse_click (&inev.ie, &event->xbutton,
+                                                   f, false);
                         }
                       if (event->type == ButtonRelease)
                         ignore_next_mouse_click_timeout = 0;
                     }
                   else
-                    x_construct_mouse_click (&inev.ie, &event->xbutton, f);
+                    x_construct_mouse_click (&inev.ie, &event->xbutton,
+                                             f, false);
 
 		  if (!NILP (tab_bar_arg))
 		    inev.ie.arg = tab_bar_arg;
@@ -23754,13 +23755,13 @@ handle_one_xevent (struct x_display_info *dpyinfo,
 				&& xev->time > ignore_next_mouse_click_timeout)
 			      {
 				ignore_next_mouse_click_timeout = 0;
-				x_construct_mouse_click (&inev.ie, &bv, f);
+				x_construct_mouse_click (&inev.ie, &bv, f, true);
 			      }
 			    if (xev->evtype == XI_ButtonRelease)
 			      ignore_next_mouse_click_timeout = 0;
 			  }
 			else
-			  x_construct_mouse_click (&inev.ie, &bv, f);
+			  x_construct_mouse_click (&inev.ie, &bv, f, true);
 
 			if (!NILP (tab_bar_arg))
 			  inev.ie.arg = tab_bar_arg;

bug#68796: xterm.c: Convert mouse-4/5/6/7 to wheel-up/down/left/right

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

> Hmm... so IIUC you're saying that even when `dpyinfo->supports_xi2` is
> true, we may receive `mouse-4/5/6/7` events for wheel motions?
> Yuck!

Yes, and as always other clients may send whatever they want, although
Emacs is not obliged to handle those events correctly.

>> The correct test of event type is whether x_construct_mouse_click is
>> being called from the XI_ButtonPress or XI_ButtonRelease label in
>> handle_one_xevent.
>
> OK, that sounds fixable.  See additional patch below.

Thanks.  I'm short on time at the moment, but will get around to this.

bug#68796: xterm.c: Convert mouse-4/5/6/7 to wheel-up/down/left/right

[Eli Zaretskii]

Ping!  Can we make some progress here?

> Cc: 68796@debbugs.gnu.org
> Date: Tue, 30 Jan 2024 11:53:58 +0800
> From:  Po Lu via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> > Hmm... so IIUC you're saying that even when `dpyinfo->supports_xi2` is
> > true, we may receive `mouse-4/5/6/7` events for wheel motions?
> > Yuck!
> 
> Yes, and as always other clients may send whatever they want, although
> Emacs is not obliged to handle those events correctly.
> 
> >> The correct test of event type is whether x_construct_mouse_click is
> >> being called from the XI_ButtonPress or XI_ButtonRelease label in
> >> handle_one_xevent.
> >
> > OK, that sounds fixable.  See additional patch below.
> 
> Thanks.  I'm short on time at the moment, but will get around to this.
> 
> 
> 
> 

bug#68796: xterm.c: Convert mouse-4/5/6/7 to wheel-up/down/left/right

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> Ping!  Can we make some progress here?

I converted most of the Lisp code at my organization (where X servers
are configured with the input extension disabled) to use wheel events
under Emacs 30 and arranged for a modified version of Stefan's patch to
be built.  Which was incidentally not straightforward to do cleanly, as
the code in question assumed that events it received would be pairs of
button press and release events, and was very much designed around that
assumption.

Unfortunately the holidays are in full swing, and conclusions from this
cannot be drawn until once enough experience accumulates some time after
they end.  To complicate matters, users at my site don't often work with
Emacs 30, except for a few early adopters.

bug#68796: xterm.c: Convert mouse-4/5/6/7 to wheel-up/down/left/right

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Po Lu [2024-02-10 17:09:14] wrote:
> Eli Zaretskii <eliz@gnu.org> writes:
>> Ping!  Can we make some progress here?
> I converted most of the Lisp code at my organization (where X servers
> are configured with the input extension disabled) to use wheel events
> under Emacs 30 and arranged for a modified version of Stefan's patch to
> be built.  Which was incidentally not straightforward to do cleanly, as
> the code in question assumed that events it received would be pairs of
> button press and release events, and was very much designed around that
> assumption.

Fascinating.  Is there any way I could get to peek at that code?
Also interested in the backstory about why your X11 servers are configured
without XInput2.

Maybe we should provide a better "reverse" story: with the current patch
users can recover their old `mouse-4/5/6/7` events by setting
`mouse-wheel-*-event` to nil, but then they lose mwheel-based scrolling
(plus a few other bindings, mostly one in completion-preview and one in
flymake), so the etc/NEWS entry should describe how to recover that
scrolling behavior.

Or maybe we should keep `mouse-wheel-*-event` untouched (tho, maybe mark
them obsolete) and instead introduce a new set of variables that control
the remapping of `mouse-4/5/6/7` to `wheel-*` events.

That would provide a smoother transition: users can set those new vars
to nil to recover the old behavior.

Another benefit is that the new var(s) could hold just the button
numbers (i.e. an integer) instead of a symbol, which should simplify the
remapping code (both in the `xterm.c` and the `xt-mouse.el` cases).


        Stefan

bug#68796: xterm.c: Convert mouse-4/5/6/7 to wheel-up/down/left/right

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 376 bytes --]

> Or maybe we should keep `mouse-wheel-*-event` untouched (tho, maybe mark
> them obsolete) and instead introduce a new set of variables that control
> the remapping of `mouse-4/5/6/7` to `wheel-*` events.

See patch below.
It would need etc/NEWS and doc changes, of course, and probably some
warning-silencing in those packages using `mouse-wheel-*-event`s.


        Stefan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-mouse-wheel-buttons-Map-old-style-wheel-buttons-to-a.patch --]
[-- Type: text/x-diff, Size: 12027 bytes --]

From 0e94e1140a8ec72bd068648eb5ec1dc861f0884b Mon Sep 17 00:00:00 2001
From: Stefan Monnier <monnier@iro.umontreal.ca>
Date: Mon, 29 Jan 2024 09:35:09 -0500
Subject: [PATCH] (mouse-wheel-buttons): Map old-style wheel buttons to actual
 wheel events

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`.

* lisp/mwheel.el (mouse-wheel-up-event, mouse-wheel-down-event)
(mouse-wheel-left-event, mouse-wheel-right-event): Make obsolete.
(mouse-wheel-obey-old-style-wheel-buttons): Delete variable.
---
 lisp/mouse.el    | 13 ++++++++++++
 lisp/mwheel.el   | 21 ++++++++-----------
 lisp/xt-mouse.el | 20 ++++--------------
 src/keyboard.c   |  9 ++++++--
 src/xterm.c      | 53 +++++++++++++++++++++++++++++++++++++++---------
 5 files changed, 75 insertions(+), 41 deletions(-)

diff --git a/lisp/mouse.el b/lisp/mouse.el
index d1b06c2040d..585891bf699 100644
--- a/lisp/mouse.el
+++ b/lisp/mouse.el
@@ -133,6 +133,19 @@ mouse-drag-mode-line-buffer
   :type 'boolean
   :version "29.1")
 
+(defcustom mouse-wheel-buttons
+  '((4 . wheel-up) (5 . wheel-down) (6 . wheel-left) (7 . wheel-right))
+  "Buttons to remap 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',
+`wheel-left', or `wheel-right'.
+This is used only for events that come from sources known to generate such
+events, such as X11 events when XInput2 is not used, or events coming from
+a text terminal."
+  :type '(alist)
+  :version "30.1")
+
 (defvar mouse--last-down nil)
 
 (defun mouse--down-1-maybe-follows-link (&optional _prompt)
diff --git a/lisp/mwheel.el b/lisp/mwheel.el
index 66a1fa1a706..9fc922eebc9 100644
--- a/lisp/mwheel.el
+++ b/lisp/mwheel.el
@@ -56,20 +56,17 @@ mouse-wheel-change-button
              (bound-and-true-p mouse-wheel-mode))
     (mouse-wheel-mode 1)))
 
-(defvar mouse-wheel-obey-old-style-wheel-buttons t
-  "If non-nil, treat mouse-4/5/6/7 events as mouse wheel events.
-These are the event names used historically in X11 before XInput2.
-They are sometimes generated by things like text-terminals as well.")
+(make-obsolete-variable 'mouse-wheel-up-event    'mouse-wheel-buttons "30.1")
+(make-obsolete-variable 'mouse-wheel-down-event  'mouse-wheel-buttons "30.1")
+(make-obsolete-variable 'mouse-wheel-left-event  'mouse-wheel-buttons "30.1")
+(make-obsolete-variable 'mouse-wheel-right-event 'mouse-wheel-buttons "30.1")
 
-(defcustom mouse-wheel-down-event
-  (if mouse-wheel-obey-old-style-wheel-buttons 'mouse-4)
+(defcustom mouse-wheel-down-event 'mouse-4
   "Event used for scrolling down, beside `wheel-up', if any."
   :group 'mouse
   :type 'symbol
   :set #'mouse-wheel-change-button)
-
-(defcustom mouse-wheel-up-event
-  (if mouse-wheel-obey-old-style-wheel-buttons 'mouse-5)
+(defcustom mouse-wheel-up-event 'mouse-5
   "Event used for scrolling up, beside `wheel-down', if any."
   :group 'mouse
   :type 'symbol
@@ -223,12 +220,10 @@ mwheel-scroll-left-function
 (defvar mwheel-scroll-right-function 'scroll-right
   "Function that does the job of scrolling right.")
 
-(defvar mouse-wheel-left-event
-  (if mouse-wheel-obey-old-style-wheel-buttons 'mouse-6)
+(defvar mouse-wheel-left-event 'mouse-6
   "Event used for scrolling left, beside `wheel-left', if any.")
 
-(defvar mouse-wheel-right-event
-  (if mouse-wheel-obey-old-style-wheel-buttons 'mouse-7)
+(defvar mouse-wheel-right-event 'mouse-7
   "Event used for scrolling right, beside `wheel-right', if any.")
 
 (defun mouse-wheel--get-scroll-window (event)
diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
index 081b8f32456..c27dee7e249 100644
--- a/lisp/xt-mouse.el
+++ b/lisp/xt-mouse.el
@@ -40,8 +40,6 @@
 
 ;;; Code:
 
-(require 'mwheel)
-
 (defvar xterm-mouse-debug-buffer nil)
 
 (defun xterm-mouse-translate (_event)
@@ -195,12 +193,6 @@ xterm-mouse--read-number-from-terminal
           (cons n c))
       (cons (- (setq c (xterm-mouse--read-coordinate)) 32) c))))
 
-(defun xterm-mouse--button-p (event btn)
-  (and (symbolp event)
-       (string-prefix-p "mouse-" (symbol-name event))
-       (eq btn (car (read-from-string (symbol-name event)
-                                      (length "mouse-"))))))
-
 ;; XTerm reports mouse events as
 ;; <EVENT-CODE> <X> <Y> in default mode, and
 ;; <EVENT-CODE> ";" <X> ";" <Y> <"M" or "m"> in extended mode.
@@ -246,14 +238,10 @@ xterm-mouse--read-event-sequence
             (if meta "M-" "")
             (if shift "S-" "")
             (if down "down-" "")
-            (cond
-             ;; BEWARE: `mouse-wheel-UP-event' corresponds to
-             ;; `wheel-DOWN' events and vice versa!!
-             ((xterm-mouse--button-p mouse-wheel-down-event btn)  "wheel-up")
-             ((xterm-mouse--button-p mouse-wheel-up-event btn)    "wheel-down")
-             ((xterm-mouse--button-p mouse-wheel-left-event btn)  "wheel-left")
-             ((xterm-mouse--button-p mouse-wheel-right-event btn) "wheel-right")
-             (t (format "mouse-%d" btn))))))))
+            (let ((remap (alist-get btn mouse-wheel-buttons)))
+              (if remap
+                  (symbol-name remap)
+                (format "mouse-%d" btn))))))))
     (list sym (1- x) (1- y))))
 
 (defun xterm-mouse--set-click-count (event click-count)
diff --git a/src/keyboard.c b/src/keyboard.c
index 1f7253a7da1..4c3e33762af 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -6628,8 +6628,13 @@ make_lispy_event (struct input_event *event)
 
 	if (CONSP (event->arg))
 	  return list5 (head, position, make_fixnum (double_click_count),
-			XCAR (event->arg), Fcons (XCAR (XCDR (event->arg)),
-						  XCAR (XCDR (XCDR (event->arg)))));
+			XCAR (event->arg),
+			/* FIXME: I don't know what I'm doing here.  */
+			(CONSP (XCDR (event->arg))
+			 && CONSP (XCDR (XCDR (event->arg))))
+			? Fcons (XCAR (XCDR (event->arg)),
+			         XCAR (XCDR (XCDR (event->arg))))
+			: Qnil);
         else if (NUMBERP (event->arg))
           return list4 (head, position, make_fixnum (double_click_count),
                         event->arg);
diff --git a/src/xterm.c b/src/xterm.c
index c8a43785564..e75b64ed3d4 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -14543,18 +14543,19 @@ x_query_pointer (Display *dpy, Window w, Window *root_return,
    `x', `y', `x_root' and `y_root'.  This function should not access
    any other fields in EVENT without also initializing the
    corresponding fields in `bv' under the XI_ButtonPress and
-   XI_ButtonRelease labels inside `handle_one_xevent'.  */
+   XI_ButtonRelease labels inside `handle_one_xevent'.
+
+   XI2 indicates that this click comes from XInput2 rather than core
+   event.   */
 
 static Lisp_Object
 x_construct_mouse_click (struct input_event *result,
                          const XButtonEvent *event,
-                         struct frame *f)
+                         struct frame *f, bool xi2)
 {
   int x = event->x;
   int y = event->y;
 
-  /* Make the event type NO_EVENT; we'll change that when we decide
-     otherwise.  */
   result->kind = MOUSE_CLICK_EVENT;
   result->code = event->button - Button1;
   result->timestamp = event->time;
@@ -14564,6 +14565,29 @@ x_construct_mouse_click (struct input_event *result,
 			  ? up_modifier
 			  : down_modifier));
 
+  /* 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)));
+      int wheel
+        = NILP (base) ? -1
+          : BASE_EQ (base, Qwheel_down)  ? 0
+          : BASE_EQ (base, Qwheel_up)    ? 1
+          : BASE_EQ (base, Qwheel_left)  ? 2
+          : BASE_EQ (base, Qwheel_right) ? 3
+          : -1;
+      if (wheel >= 0)
+        {
+          result->kind = (event->type != ButtonRelease ? NO_EVENT
+		          : wheel & 2 ? HORIZ_WHEEL_EVENT : WHEEL_EVENT);
+          result->code = 0;         /* Not used.  */
+          result->modifiers &= ~(up_modifier || down_modifier);
+          result->modifiers |= wheel & 1 ? up_modifier : down_modifier;
+        }
+    }
+
   /* If result->window is not the frame's edit widget (which can
      happen with GTK+ scroll bars, for example), translate the
      coordinates so they appear at the correct position.  */
@@ -21873,13 +21897,14 @@ handle_one_xevent (struct x_display_info *dpyinfo,
 			&& event->xbutton.time > ignore_next_mouse_click_timeout)
 		      {
 			ignore_next_mouse_click_timeout = 0;
-			x_construct_mouse_click (&inev.ie, &event->xbutton, f);
+			x_construct_mouse_click (&inev.ie, &event->xbutton,
+			                         f, false);
 		      }
 		    if (event->type == ButtonRelease)
 		      ignore_next_mouse_click_timeout = 0;
 		  }
 		else
-		  x_construct_mouse_click (&inev.ie, &event->xbutton, f);
+		  x_construct_mouse_click (&inev.ie, &event->xbutton, f, false);
 
 		*finish = X_EVENT_DROP;
 		goto OTHER;
@@ -21949,13 +21974,15 @@ handle_one_xevent (struct x_display_info *dpyinfo,
                           && event->xbutton.time > ignore_next_mouse_click_timeout)
                         {
                           ignore_next_mouse_click_timeout = 0;
-                          x_construct_mouse_click (&inev.ie, &event->xbutton, f);
+                          x_construct_mouse_click (&inev.ie, &event->xbutton,
+                                                   f, false);
                         }
                       if (event->type == ButtonRelease)
                         ignore_next_mouse_click_timeout = 0;
                     }
                   else
-                    x_construct_mouse_click (&inev.ie, &event->xbutton, f);
+                    x_construct_mouse_click (&inev.ie, &event->xbutton,
+                                             f, false);
 
 		  if (!NILP (tab_bar_arg))
 		    inev.ie.arg = tab_bar_arg;
@@ -23732,13 +23759,13 @@ handle_one_xevent (struct x_display_info *dpyinfo,
 				&& xev->time > ignore_next_mouse_click_timeout)
 			      {
 				ignore_next_mouse_click_timeout = 0;
-				x_construct_mouse_click (&inev.ie, &bv, f);
+				x_construct_mouse_click (&inev.ie, &bv, f, true);
 			      }
 			    if (xev->evtype == XI_ButtonRelease)
 			      ignore_next_mouse_click_timeout = 0;
 			  }
 			else
-			  x_construct_mouse_click (&inev.ie, &bv, f);
+			  x_construct_mouse_click (&inev.ie, &bv, f, true);
 
 			if (!NILP (tab_bar_arg))
 			  inev.ie.arg = tab_bar_arg;
@@ -32444,6 +32471,12 @@ syms_of_xterm (void)
   DEFSYM (Qexpose, "expose");
   DEFSYM (Qdont_save, "dont-save");
 
+  DEFSYM (Qmouse_wheel_buttons, "mouse-wheel-buttons");
+  DEFSYM (Qwheel_up,    "wheel-up");
+  DEFSYM (Qwheel_down,  "wheel-down");
+  DEFSYM (Qwheel_left,  "wheel-left");
+  DEFSYM (Qwheel_right, "wheel-right");
+
 #ifdef USE_GTK
   xg_default_icon_file = build_pure_c_string ("icons/hicolor/scalable/apps/emacs.svg");
   staticpro (&xg_default_icon_file);
-- 
2.43.0

bug#68796: xterm.c: Convert mouse-4/5/6/7 to wheel-up/down/left/right

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Ping?

I believe this patch should be much less controversial since it allows
users to get the old behavior back and still keeps the same semantics
for existing `mouse-wheel-*-event` variables in case some other code
still generates such events.  It's basically a half-way step between
what I proposed at first and the status quot.

But I'd like some confirmation that you think it's acceptable, and maybe
if you could comment on the FIXME included in there (I needed it for
clicks on the tab-bar, but I don't understand that code).


        Stefan


Stefan Monnier [2024-02-10 10:33:01] wrote:

>> Or maybe we should keep `mouse-wheel-*-event` untouched (tho, maybe mark
>> them obsolete) and instead introduce a new set of variables that control
>> the remapping of `mouse-4/5/6/7` to `wheel-*` events.
>
> See patch below.
> It would need etc/NEWS and doc changes, of course, and probably some
> warning-silencing in those packages using `mouse-wheel-*-event`s.
>
>
>         Stefan
>
> From 0e94e1140a8ec72bd068648eb5ec1dc861f0884b Mon Sep 17 00:00:00 2001
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Mon, 29 Jan 2024 09:35:09 -0500
> Subject: [PATCH] (mouse-wheel-buttons): Map old-style wheel buttons to actual
>  wheel events
>
> 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`.
>
> * lisp/mwheel.el (mouse-wheel-up-event, mouse-wheel-down-event)
> (mouse-wheel-left-event, mouse-wheel-right-event): Make obsolete.
> (mouse-wheel-obey-old-style-wheel-buttons): Delete variable.
> ---
>  lisp/mouse.el    | 13 ++++++++++++
>  lisp/mwheel.el   | 21 ++++++++-----------
>  lisp/xt-mouse.el | 20 ++++--------------
>  src/keyboard.c   |  9 ++++++--
>  src/xterm.c      | 53 +++++++++++++++++++++++++++++++++++++++---------
>  5 files changed, 75 insertions(+), 41 deletions(-)
>
> diff --git a/lisp/mouse.el b/lisp/mouse.el
> index d1b06c2040d..585891bf699 100644
> --- a/lisp/mouse.el
> +++ b/lisp/mouse.el
> @@ -133,6 +133,19 @@ mouse-drag-mode-line-buffer
>    :type 'boolean
>    :version "29.1")
>  
> +(defcustom mouse-wheel-buttons
> +  '((4 . wheel-up) (5 . wheel-down) (6 . wheel-left) (7 . wheel-right))
> +  "Buttons to remap 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',
> +`wheel-left', or `wheel-right'.
> +This is used only for events that come from sources known to generate such
> +events, such as X11 events when XInput2 is not used, or events coming from
> +a text terminal."
> +  :type '(alist)
> +  :version "30.1")
> +
>  (defvar mouse--last-down nil)
>  
>  (defun mouse--down-1-maybe-follows-link (&optional _prompt)
> diff --git a/lisp/mwheel.el b/lisp/mwheel.el
> index 66a1fa1a706..9fc922eebc9 100644
> --- a/lisp/mwheel.el
> +++ b/lisp/mwheel.el
> @@ -56,20 +56,17 @@ mouse-wheel-change-button
>               (bound-and-true-p mouse-wheel-mode))
>      (mouse-wheel-mode 1)))
>  
> -(defvar mouse-wheel-obey-old-style-wheel-buttons t
> -  "If non-nil, treat mouse-4/5/6/7 events as mouse wheel events.
> -These are the event names used historically in X11 before XInput2.
> -They are sometimes generated by things like text-terminals as well.")
> +(make-obsolete-variable 'mouse-wheel-up-event    'mouse-wheel-buttons "30.1")
> +(make-obsolete-variable 'mouse-wheel-down-event  'mouse-wheel-buttons "30.1")
> +(make-obsolete-variable 'mouse-wheel-left-event  'mouse-wheel-buttons "30.1")
> +(make-obsolete-variable 'mouse-wheel-right-event 'mouse-wheel-buttons "30.1")
>  
> -(defcustom mouse-wheel-down-event
> -  (if mouse-wheel-obey-old-style-wheel-buttons 'mouse-4)
> +(defcustom mouse-wheel-down-event 'mouse-4
>    "Event used for scrolling down, beside `wheel-up', if any."
>    :group 'mouse
>    :type 'symbol
>    :set #'mouse-wheel-change-button)
> -
> -(defcustom mouse-wheel-up-event
> -  (if mouse-wheel-obey-old-style-wheel-buttons 'mouse-5)
> +(defcustom mouse-wheel-up-event 'mouse-5
>    "Event used for scrolling up, beside `wheel-down', if any."
>    :group 'mouse
>    :type 'symbol
> @@ -223,12 +220,10 @@ mwheel-scroll-left-function
>  (defvar mwheel-scroll-right-function 'scroll-right
>    "Function that does the job of scrolling right.")
>  
> -(defvar mouse-wheel-left-event
> -  (if mouse-wheel-obey-old-style-wheel-buttons 'mouse-6)
> +(defvar mouse-wheel-left-event 'mouse-6
>    "Event used for scrolling left, beside `wheel-left', if any.")
>  
> -(defvar mouse-wheel-right-event
> -  (if mouse-wheel-obey-old-style-wheel-buttons 'mouse-7)
> +(defvar mouse-wheel-right-event 'mouse-7
>    "Event used for scrolling right, beside `wheel-right', if any.")
>  
>  (defun mouse-wheel--get-scroll-window (event)
> diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
> index 081b8f32456..c27dee7e249 100644
> --- a/lisp/xt-mouse.el
> +++ b/lisp/xt-mouse.el
> @@ -40,8 +40,6 @@
>  
>  ;;; Code:
>  
> -(require 'mwheel)
> -
>  (defvar xterm-mouse-debug-buffer nil)
>  
>  (defun xterm-mouse-translate (_event)
> @@ -195,12 +193,6 @@ xterm-mouse--read-number-from-terminal
>            (cons n c))
>        (cons (- (setq c (xterm-mouse--read-coordinate)) 32) c))))
>  
> -(defun xterm-mouse--button-p (event btn)
> -  (and (symbolp event)
> -       (string-prefix-p "mouse-" (symbol-name event))
> -       (eq btn (car (read-from-string (symbol-name event)
> -                                      (length "mouse-"))))))
> -
>  ;; XTerm reports mouse events as
>  ;; <EVENT-CODE> <X> <Y> in default mode, and
>  ;; <EVENT-CODE> ";" <X> ";" <Y> <"M" or "m"> in extended mode.
> @@ -246,14 +238,10 @@ xterm-mouse--read-event-sequence
>              (if meta "M-" "")
>              (if shift "S-" "")
>              (if down "down-" "")
> -            (cond
> -             ;; BEWARE: `mouse-wheel-UP-event' corresponds to
> -             ;; `wheel-DOWN' events and vice versa!!
> -             ((xterm-mouse--button-p mouse-wheel-down-event btn)  "wheel-up")
> -             ((xterm-mouse--button-p mouse-wheel-up-event btn)    "wheel-down")
> -             ((xterm-mouse--button-p mouse-wheel-left-event btn)  "wheel-left")
> -             ((xterm-mouse--button-p mouse-wheel-right-event btn) "wheel-right")
> -             (t (format "mouse-%d" btn))))))))
> +            (let ((remap (alist-get btn mouse-wheel-buttons)))
> +              (if remap
> +                  (symbol-name remap)
> +                (format "mouse-%d" btn))))))))
>      (list sym (1- x) (1- y))))
>  
>  (defun xterm-mouse--set-click-count (event click-count)
> diff --git a/src/keyboard.c b/src/keyboard.c
> index 1f7253a7da1..4c3e33762af 100644
> --- a/src/keyboard.c
> +++ b/src/keyboard.c
> @@ -6628,8 +6628,13 @@ make_lispy_event (struct input_event *event)
>  
>  	if (CONSP (event->arg))
>  	  return list5 (head, position, make_fixnum (double_click_count),
> -			XCAR (event->arg), Fcons (XCAR (XCDR (event->arg)),
> -						  XCAR (XCDR (XCDR (event->arg)))));
> +			XCAR (event->arg),
> +			/* FIXME: I don't know what I'm doing here.  */
> +			(CONSP (XCDR (event->arg))
> +			 && CONSP (XCDR (XCDR (event->arg))))
> +			? Fcons (XCAR (XCDR (event->arg)),
> +			         XCAR (XCDR (XCDR (event->arg))))
> +			: Qnil);
>          else if (NUMBERP (event->arg))
>            return list4 (head, position, make_fixnum (double_click_count),
>                          event->arg);
> diff --git a/src/xterm.c b/src/xterm.c
> index c8a43785564..e75b64ed3d4 100644
> --- a/src/xterm.c
> +++ b/src/xterm.c
> @@ -14543,18 +14543,19 @@ x_query_pointer (Display *dpy, Window w, Window *root_return,
>     `x', `y', `x_root' and `y_root'.  This function should not access
>     any other fields in EVENT without also initializing the
>     corresponding fields in `bv' under the XI_ButtonPress and
> -   XI_ButtonRelease labels inside `handle_one_xevent'.  */
> +   XI_ButtonRelease labels inside `handle_one_xevent'.
> +
> +   XI2 indicates that this click comes from XInput2 rather than core
> +   event.   */
>  
>  static Lisp_Object
>  x_construct_mouse_click (struct input_event *result,
>                           const XButtonEvent *event,
> -                         struct frame *f)
> +                         struct frame *f, bool xi2)
>  {
>    int x = event->x;
>    int y = event->y;
>  
> -  /* Make the event type NO_EVENT; we'll change that when we decide
> -     otherwise.  */
>    result->kind = MOUSE_CLICK_EVENT;
>    result->code = event->button - Button1;
>    result->timestamp = event->time;
> @@ -14564,6 +14565,29 @@ x_construct_mouse_click (struct input_event *result,
>  			  ? up_modifier
>  			  : down_modifier));
>  
> +  /* 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)));
> +      int wheel
> +        = NILP (base) ? -1
> +          : BASE_EQ (base, Qwheel_down)  ? 0
> +          : BASE_EQ (base, Qwheel_up)    ? 1
> +          : BASE_EQ (base, Qwheel_left)  ? 2
> +          : BASE_EQ (base, Qwheel_right) ? 3
> +          : -1;
> +      if (wheel >= 0)
> +        {
> +          result->kind = (event->type != ButtonRelease ? NO_EVENT
> +		          : wheel & 2 ? HORIZ_WHEEL_EVENT : WHEEL_EVENT);
> +          result->code = 0;         /* Not used.  */
> +          result->modifiers &= ~(up_modifier || down_modifier);
> +          result->modifiers |= wheel & 1 ? up_modifier : down_modifier;
> +        }
> +    }
> +
>    /* If result->window is not the frame's edit widget (which can
>       happen with GTK+ scroll bars, for example), translate the
>       coordinates so they appear at the correct position.  */
> @@ -21873,13 +21897,14 @@ handle_one_xevent (struct x_display_info *dpyinfo,
>  			&& event->xbutton.time > ignore_next_mouse_click_timeout)
>  		      {
>  			ignore_next_mouse_click_timeout = 0;
> -			x_construct_mouse_click (&inev.ie, &event->xbutton, f);
> +			x_construct_mouse_click (&inev.ie, &event->xbutton,
> +			                         f, false);
>  		      }
>  		    if (event->type == ButtonRelease)
>  		      ignore_next_mouse_click_timeout = 0;
>  		  }
>  		else
> -		  x_construct_mouse_click (&inev.ie, &event->xbutton, f);
> +		  x_construct_mouse_click (&inev.ie, &event->xbutton, f, false);
>  
>  		*finish = X_EVENT_DROP;
>  		goto OTHER;
> @@ -21949,13 +21974,15 @@ handle_one_xevent (struct x_display_info *dpyinfo,
>                            && event->xbutton.time > ignore_next_mouse_click_timeout)
>                          {
>                            ignore_next_mouse_click_timeout = 0;
> -                          x_construct_mouse_click (&inev.ie, &event->xbutton, f);
> +                          x_construct_mouse_click (&inev.ie, &event->xbutton,
> +                                                   f, false);
>                          }
>                        if (event->type == ButtonRelease)
>                          ignore_next_mouse_click_timeout = 0;
>                      }
>                    else
> -                    x_construct_mouse_click (&inev.ie, &event->xbutton, f);
> +                    x_construct_mouse_click (&inev.ie, &event->xbutton,
> +                                             f, false);
>  
>  		  if (!NILP (tab_bar_arg))
>  		    inev.ie.arg = tab_bar_arg;
> @@ -23732,13 +23759,13 @@ handle_one_xevent (struct x_display_info *dpyinfo,
>  				&& xev->time > ignore_next_mouse_click_timeout)
>  			      {
>  				ignore_next_mouse_click_timeout = 0;
> -				x_construct_mouse_click (&inev.ie, &bv, f);
> +				x_construct_mouse_click (&inev.ie, &bv, f, true);
>  			      }
>  			    if (xev->evtype == XI_ButtonRelease)
>  			      ignore_next_mouse_click_timeout = 0;
>  			  }
>  			else
> -			  x_construct_mouse_click (&inev.ie, &bv, f);
> +			  x_construct_mouse_click (&inev.ie, &bv, f, true);
>  
>  			if (!NILP (tab_bar_arg))
>  			  inev.ie.arg = tab_bar_arg;
> @@ -32444,6 +32471,12 @@ syms_of_xterm (void)
>    DEFSYM (Qexpose, "expose");
>    DEFSYM (Qdont_save, "dont-save");
>  
> +  DEFSYM (Qmouse_wheel_buttons, "mouse-wheel-buttons");
> +  DEFSYM (Qwheel_up,    "wheel-up");
> +  DEFSYM (Qwheel_down,  "wheel-down");
> +  DEFSYM (Qwheel_left,  "wheel-left");
> +  DEFSYM (Qwheel_right, "wheel-right");
> +
>  #ifdef USE_GTK
>    xg_default_icon_file = build_pure_c_string ("icons/hicolor/scalable/apps/emacs.svg");
>    staticpro (&xg_default_icon_file);

bug#68796: xterm.c: Convert mouse-4/5/6/7 to wheel-up/down/left/right

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

> Ping?
>
> I believe this patch should be much less controversial since it allows
> users to get the old behavior back and still keeps the same semantics
> for existing `mouse-wheel-*-event` variables in case some other code
> still generates such events.  It's basically a half-way step between
> what I proposed at first and the status quot.
>
> But I'd like some confirmation that you think it's acceptable, and maybe
> if you could comment on the FIXME included in there (I needed it for
> clicks on the tab-bar, but I don't understand that code).

That would be simple if I could find the FIXME, but it doesn't appear to
be present anywhere in that patch...

>> +  /* 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)));
>> +      int wheel
>> +        = NILP (base) ? -1
>> +          : BASE_EQ (base, Qwheel_down)  ? 0
>> +          : BASE_EQ (base, Qwheel_up)    ? 1
>> +          : BASE_EQ (base, Qwheel_left)  ? 2
>> +          : BASE_EQ (base, Qwheel_right) ? 3
>> +          : -1;

This should be surrounded by parentheses.

Thanks.

bug#68796: xterm.c: Convert mouse-4/5/6/7 to wheel-up/down/left/right

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> That would be simple if I could find the FIXME, but it doesn't appear to
> be present anywhere in that patch...

[ Hmm... not sure what happened: `C-s FIXME` finds it for me:  ]

diff --git a/src/keyboard.c b/src/keyboard.c
index 1f7253a7da1..4c3e33762af 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -6628,8 +6628,13 @@ make_lispy_event (struct input_event *event)
 
 	if (CONSP (event->arg))
 	  return list5 (head, position, make_fixnum (double_click_count),
-			XCAR (event->arg), Fcons (XCAR (XCDR (event->arg)),
-						  XCAR (XCDR (XCDR (event->arg)))));
+			XCAR (event->arg),
+			/* FIXME: I don't know what I'm doing here.  */
+			(CONSP (XCDR (event->arg))
+			 && CONSP (XCDR (XCDR (event->arg))))
+			? Fcons (XCAR (XCDR (event->arg)),
+			         XCAR (XCDR (XCDR (event->arg))))
+			: Qnil);
         else if (NUMBERP (event->arg))
           return list4 (head, position, make_fixnum (double_click_count),
                         event->arg);

>>> +  /* 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)));
>>> +      int wheel
>>> +        = NILP (base) ? -1
>>> +          : BASE_EQ (base, Qwheel_down)  ? 0
>>> +          : BASE_EQ (base, Qwheel_up)    ? 1
>>> +          : BASE_EQ (base, Qwheel_left)  ? 2
>>> +          : BASE_EQ (base, Qwheel_right) ? 3
>>> +          : -1;
>
> This should be surrounded by parentheses.

Ah, it might be need for CC-mode, indeed, thanks.
`sm-c-mode` gets this right without the parens :-)


        Stefan