bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

Running ‘make -k -C test check-lisp’ on master with a build
configured without X produces the following error:

  cd emacs
  ./autogen.sh
  ./configure --without-x
  make -j$(nproc)
  make -k -C test check-lisp
  …
  In toplevel form:
  lisp/completion-preview-tests.el:23:2: Error: Symbol's value
  as variable is void: mouse-wheel-up-event
  make[3]: *** [Makefile:159: lisp/completion-preview-tests.elc] Error 1
  make[3]: Target 'lisp/completion-preview-tests.log' not remade because of errors.
  …
  SUMMARY OF TEST RESULTS
  -----------------------
  Files examined: 118
  Ran 2209 tests, 2194 results as expected, 0 unexpected, 15 skipped
  1 files did not contain any tests:
    lisp/completion-preview-tests.log
  make[2]: *** [Makefile:344: check-doit] Error 2
  make[2]: Leaving directory '/home/jm/src/emacs/test'
  make[1]: *** [Makefile:310: check] Error 2
  make[1]: Leaving directory '/home/jm/src/emacs/test'
  make: *** [Makefile:262: check-lisp] Error 2

While running ‘make’ I notice some possibly related warnings:

  In toplevel form:
  completion-preview.el:136:18: Warning: reference to free
  variable ‘mouse-wheel-up-event’
  completion-preview.el:137:18: Warning: reference to free
  variable ‘mouse-wheel-up-alternate-event’
  completion-preview.el:138:18: Warning: reference to free
  variable ‘mouse-wheel-down-event’
  completion-preview.el:139:18: Warning: reference to free
  variable ‘mouse-wheel-down-alternate-event’

My desktop is Fedora but I also tried on Debian testing and the
error is the same there.

In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu) of 2024-01-02 built
 on localhost
Repository revision: 35b5775d422eafc120964575094c0ea597a88758
Repository branch: builds
System Description: Fedora Linux 39 (Thirty Nine)

Configured using:
 'configure --without-x --without-native-compilation'

Configured features:
ACL DBUS GMP GNUTLS GPM JSON LIBSELINUX LIBSYSTEMD LIBXML2 MODULES
NOTIFY INOTIFY PDUMPER SECCOMP SOUND SQLITE3 THREADS TREE_SITTER XIM
ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  minibuffer-regexp-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort regexp-opt mail-extr emacsbug message mailcap yank-media
puny dired dnd dired-loaddefs rfc822 mml mml-sec password-cache epa
derived epg rfc6068 epg-config gnus-util text-property-search time-date
subr-x mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader cl-loaddefs cl-lib sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils term/xterm xterm byte-opt gv
bytecomp byte-compile rmc iso-transl tooltip cconv eldoc paren electric
uniquify ediff-hook vc-hooks lisp-float-type elisp-mode tabulated-list
replace newcomment text-mode lisp-mode prog-mode register page tab-bar
menu-bar rfn-eshadow isearch easymenu timer select mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer nadvice seq
simple cl-generic indonesian philippine cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite emoji-zwj charscript charprop case-table
epa-hook jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button
loaddefs theme-loaddefs faces cus-face macroexp files window
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget keymap hashtable-print-readable backquote threads dbusbind
inotify multi-tty make-network-process emacs)

Memory information:
((conses 16 49337 8918) (symbols 48 5807 0) (strings 32 14981 1821)
 (string-bytes 1 436773) (vectors 16 8571)
 (vector-slots 8 107831 8025) (floats 8 24 50) (intervals 56 212 7)
 (buffers 984 10))

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

[Eli Zaretskii]

> Date: Tue, 02 Jan 2024 10:42:59 -0600
> From:  john muhl via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Running ‘make -k -C test check-lisp’ on master with a build
> configured without X produces the following error:
> 
>   cd emacs
>   ./autogen.sh
>   ./configure --without-x
>   make -j$(nproc)
>   make -k -C test check-lisp
>   …
>   In toplevel form:
>   lisp/completion-preview-tests.el:23:2: Error: Symbol's value
>   as variable is void: mouse-wheel-up-event
>   make[3]: *** [Makefile:159: lisp/completion-preview-tests.elc] Error 1
>   make[3]: Target 'lisp/completion-preview-tests.log' not remade because of errors.

Thanks, please try again, I hope I fixed this.

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

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

Hi John, Eli,

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Tue, 02 Jan 2024 10:42:59 -0600
>> From:  john muhl via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> Running ‘make -k -C test check-lisp’ on master with a build
>> configured without X produces the following error:
>> 
>>   cd emacs
>>   ./autogen.sh
>>   ./configure --without-x
>>   make -j$(nproc)
>>   make -k -C test check-lisp
>>   …
>>   In toplevel form:
>>   lisp/completion-preview-tests.el:23:2: Error: Symbol's value
>>   as variable is void: mouse-wheel-up-event
>>   make[3]: *** [Makefile:159: lisp/completion-preview-tests.elc] Error 1
>>   make[3]: Target 'lisp/completion-preview-tests.log' not remade because of errors.
>
> Thanks, please try again, I hope I fixed this.

FWIW, I was going to suggest the attached patch, following how
flymake.el handles this.  But if the current solution works, that seems
just as good.


Thanks,

Eshel


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Explicitly-require-mwheel-in-completion-preview.el.patch --]
[-- Type: text/x-patch, Size: 715 bytes --]

From e4c13af2cc6c3a9839af5fb11b9e1a9e7da87475 Mon Sep 17 00:00:00 2001
From: Eshel Yaron <me@eshelyaron.com>
Date: Tue, 2 Jan 2024 18:13:33 +0100
Subject: [PATCH] Explicitly require 'mwheel' in completion-preview.el

* lisp/completion-preview.el: Require 'mwheel' in favor of
'--without-x' builds.  (Bug#68213)
---
 lisp/completion-preview.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lisp/completion-preview.el b/lisp/completion-preview.el
index da4afb8f66a..26e5af1287d 100644
--- a/lisp/completion-preview.el
+++ b/lisp/completion-preview.el
@@ -52,6 +52,8 @@
 
 ;;; Code:
 
+(require 'mwheel)
+
 (defgroup completion-preview nil
   "In-buffer completion preview."
   :group 'completion)
-- 
2.42.0

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

Eshel Yaron <me@eshelyaron.com> writes:

> Hi John, Eli,
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> Date: Tue, 02 Jan 2024 10:42:59 -0600
>>> From:  john muhl via "Bug reports for GNU Emacs,
>>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>>> 
>>> Running ‘make -k -C test check-lisp’ on master with a build
>>> configured without X produces the following error:
>>> 
>>>   cd emacs
>>>   ./autogen.sh
>>>   ./configure --without-x
>>>   make -j$(nproc)
>>>   make -k -C test check-lisp
>>>   …
>>>   In toplevel form:
>>>   lisp/completion-preview-tests.el:23:2: Error: Symbol's value
>>>   as variable is void: mouse-wheel-up-event
>>>   make[3]: *** [Makefile:159: lisp/completion-preview-tests.elc] Error 1
>>>   make[3]: Target 'lisp/completion-preview-tests.log' not remade because of errors.
>>
>> Thanks, please try again, I hope I fixed this.
>
> FWIW, I was going to suggest the attached patch, following how
> flymake.el handles this.  But if the current solution works, that seems
> just as good.
>
>
> Thanks,
>
> Eshel
>
> [2. text/x-patch; 0001-Explicitly-require-mwheel-in-completion-preview.el.patch]...

That was my first thought too but it led to another error:

  lisp/completion-preview-tests.el:23:2: Error: Duplicate
  definition for key: "<mouse-5>" (keymap (mouse-5 .
  completion-preview-prev-candidate) (down-mouse-2 .
  completion-at-point) (C-down-mouse-1 . completion-at-point)
  (down-mouse-1 . completion-preview-insert))

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

[Eli Zaretskii]

> From: Eshel Yaron <me@eshelyaron.com>
> Cc: john muhl <jm@pub.pink>,  68213@debbugs.gnu.org
> Date: Tue, 02 Jan 2024 18:20:20 +0100
> 
> > Thanks, please try again, I hope I fixed this.
> 
> FWIW, I was going to suggest the attached patch, following how
> flymake.el handles this.  But if the current solution works, that seems
> just as good.

I don't think loading mwheel in a build --without-x is TRT.  If it
were, we'd probably preload mwheel on such a build as well.

In general, it is not safe to load packages that assume GUI
capabilities in a build that excludes those capabilities.

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Tue, 02 Jan 2024 10:42:59 -0600
>> From:  john muhl via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> Running ‘make -k -C test check-lisp’ on master with a build
>> configured without X produces the following error:
>> 
>>   cd emacs
>>   ./autogen.sh
>>   ./configure --without-x
>>   make -j$(nproc)
>>   make -k -C test check-lisp
>>   …
>>   In toplevel form:
>>   lisp/completion-preview-tests.el:23:2: Error: Symbol's value
>>   as variable is void: mouse-wheel-up-event
>>   make[3]: *** [Makefile:159: lisp/completion-preview-tests.elc] Error 1
>>   make[3]: Target 'lisp/completion-preview-tests.log' not remade because of errors.
>
> Thanks, please try again, I hope I fixed this.

That removes the warnings during make but unfortunately the test
still fails with the same error.

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

[Eli Zaretskii]

> From: john muhl <jm@pub.pink>
> Cc: 68213@debbugs.gnu.org
> Date: Tue, 02 Jan 2024 11:48:49 -0600
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Date: Tue, 02 Jan 2024 10:42:59 -0600
> >> From:  john muhl via "Bug reports for GNU Emacs,
> >>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> >> 
> >> Running ‘make -k -C test check-lisp’ on master with a build
> >> configured without X produces the following error:
> >> 
> >>   cd emacs
> >>   ./autogen.sh
> >>   ./configure --without-x
> >>   make -j$(nproc)
> >>   make -k -C test check-lisp
> >>   …
> >>   In toplevel form:
> >>   lisp/completion-preview-tests.el:23:2: Error: Symbol's value
> >>   as variable is void: mouse-wheel-up-event
> >>   make[3]: *** [Makefile:159: lisp/completion-preview-tests.elc] Error 1
> >>   make[3]: Target 'lisp/completion-preview-tests.log' not remade because of errors.
> >
> > Thanks, please try again, I hope I fixed this.
> 
> That removes the warnings during make

I see error, not warning.

> but unfortunately the test still fails with the same error.

Which error?

Please say

  $ cd test && make lisp/completion-preview-tests.log

and then post the part of the log with the full error message.

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: john muhl <jm@pub.pink>
>> Cc: 68213@debbugs.gnu.org
>> Date: Tue, 02 Jan 2024 11:48:49 -0600
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> Date: Tue, 02 Jan 2024 10:42:59 -0600
>> >> From:  john muhl via "Bug reports for GNU Emacs,
>> >>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> >> 
>> >> Running ‘make -k -C test check-lisp’ on master with a build
>> >> configured without X produces the following error:
>> >> 
>> >>   cd emacs
>> >>   ./autogen.sh
>> >>   ./configure --without-x
>> >>   make -j$(nproc)
>> >>   make -k -C test check-lisp
>> >>   …
>> >>   In toplevel form:
>> >>   lisp/completion-preview-tests.el:23:2: Error: Symbol's value
>> >>   as variable is void: mouse-wheel-up-event
>> >>   make[3]: *** [Makefile:159: lisp/completion-preview-tests.elc] Error 1
>> >>   make[3]: Target 'lisp/completion-preview-tests.log' not remade because of errors.
>> >
>> > Thanks, please try again, I hope I fixed this.
>> 
>> That removes the warnings during make
>
> I see error, not warning.
>
>> but unfortunately the test still fails with the same error.
>
> Which error?
>
> Please say
>
>   $ cd test && make lisp/completion-preview-tests.log
>
> and then post the part of the log with the full error message.

  $ cd test && make lisp/completion-preview-tests.log
    ELC      lisp/completion-preview-tests.elc

  In toplevel form:
  lisp/completion-preview-tests.el:23:2: Error: Symbol's value as
  variable is void: mouse-wheel-up-event
  make: *** [Makefile:159: lisp/completion-preview-tests.elc] Error 1

  $ cat lisp/completion-preview-tests.log
  cat: lisp/completion-preview-tests.log: No such file or directory

  $ git rev-parse HEAD
  0bc42eec9836d5f977d4187d57c829895726b862

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

john muhl <jm@pub.pink> writes:

> Eshel Yaron <me@eshelyaron.com> writes:
>
>> FWIW, I was going to suggest the attached patch, following how
>> flymake.el handles this.  But if the current solution works, that seems
>> just as good.
>>
>   lisp/completion-preview-tests.el:23:2: Error: Duplicate
>   definition for key: "<mouse-5>" (keymap (mouse-5 .
>   completion-preview-prev-candidate) (down-mouse-2 .
>   completion-at-point) (C-down-mouse-1 . completion-at-point)
>   (down-mouse-1 . completion-preview-insert))

Ah I can see that, thanks.  IMO `defvar-keymap` is too strict in this
case: the "duplicate" definition is not erroneous, it just happens to be
redundant with some values of the `mwheel` variables.  There's been some
discussion around that in bug#56873, FWIW.  So ISTM that the best way
forward would be to make `defvar-keymap` more lax, perhaps optionally
with a `:lax` keyword argument or some such.  Any other ideas?

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

[Eli Zaretskii]

> From: Eshel Yaron <me@eshelyaron.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  68213@debbugs.gnu.org
> Date: Wed, 03 Jan 2024 08:20:37 +0100
> 
> john muhl <jm@pub.pink> writes:
> 
> > Eshel Yaron <me@eshelyaron.com> writes:
> >
> >> FWIW, I was going to suggest the attached patch, following how
> >> flymake.el handles this.  But if the current solution works, that seems
> >> just as good.
> >>
> >   lisp/completion-preview-tests.el:23:2: Error: Duplicate
> >   definition for key: "<mouse-5>" (keymap (mouse-5 .
> >   completion-preview-prev-candidate) (down-mouse-2 .
> >   completion-at-point) (C-down-mouse-1 . completion-at-point)
> >   (down-mouse-1 . completion-preview-insert))
> 
> Ah I can see that, thanks.  IMO `defvar-keymap` is too strict in this
> case: the "duplicate" definition is not erroneous, it just happens to be
> redundant with some values of the `mwheel` variables.  There's been some
> discussion around that in bug#56873, FWIW.  So ISTM that the best way
> forward would be to make `defvar-keymap` more lax, perhaps optionally
> with a `:lax` keyword argument or some such.  Any other ideas?

Please tell more, as I don't think I follow.  Specifically:

  . does the problem happen only in a --without-x build?
  . what "duplicate" definitions do we have there and why?

Thanks.

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Eshel Yaron <me@eshelyaron.com>
>>
>> john muhl <jm@pub.pink> writes:
>>
>> > Eshel Yaron <me@eshelyaron.com> writes:
>> >
>> >> FWIW, I was going to suggest the attached patch, following how
>> >> flymake.el handles this.  But if the current solution works, that seems
>> >> just as good.
>> >>
>> >   lisp/completion-preview-tests.el:23:2: Error: Duplicate
>> >   definition for key: "<mouse-5>" (keymap (mouse-5 .
>> >   completion-preview-prev-candidate) (down-mouse-2 .
>> >   completion-at-point) (C-down-mouse-1 . completion-at-point)
>> >   (down-mouse-1 . completion-preview-insert))
>>
>> Ah I can see that, thanks.  IMO `defvar-keymap` is too strict in this
>> case: the "duplicate" definition is not erroneous, it just happens to be
>> redundant with some values of the `mwheel` variables.  There's been some
>> discussion around that in bug#56873, FWIW.  So ISTM that the best way
>> forward would be to make `defvar-keymap` more lax, perhaps optionally
>> with a `:lax` keyword argument or some such.  Any other ideas?
>
> Please tell more, as I don't think I follow.

Sure,

> Specifically:
>
>   . does the problem happen only in a --without-x build?
>   . what "duplicate" definitions do we have there and why?

The definition of `completion-preview--mouse-map` binds the events
specified by both `mouse-wheel-up-event` and
`mouse-wheel-up-alternate-event` to `completion-preview-prev-candidate`
(using `defvar-keymap`).  When these two variables have the same value,
as apparently happens in John's `--without-x` build but could probably
also occur in other setups, `defvar-keymap` complains (signals an error)
about a duplicate definition.  The duplication itself is harmless in
this case, but there doesn't seem to be a way to tell `defvar-keymap`
not to worry about it.

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

[Eli Zaretskii]

> From: Eshel Yaron <me@eshelyaron.com>
> Cc: jm@pub.pink,  68213@debbugs.gnu.org
> Date: Wed, 03 Jan 2024 19:45:39 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >   . does the problem happen only in a --without-x build?
> >   . what "duplicate" definitions do we have there and why?
> 
> The definition of `completion-preview--mouse-map` binds the events
> specified by both `mouse-wheel-up-event` and
> `mouse-wheel-up-alternate-event` to `completion-preview-prev-candidate`
> (using `defvar-keymap`).  When these two variables have the same value,
> as apparently happens in John's `--without-x` build but could probably
> also occur in other setups, `defvar-keymap` complains (signals an error)
> about a duplicate definition.  The duplication itself is harmless in
> this case, but there doesn't seem to be a way to tell `defvar-keymap`
> not to worry about it.

We should try not to produce duplicate bindings here.  Is there a real
possibility that these will be the same events in GUI builds?  If so,
the definition of bindings should detect that and avoid duplicate
bindings.  But if this can only happen in a --without-x build, then a
solution can be much simpler: avoid binding any mwheel events at all.
That's why I asked that first question.

In any case, I don't think this issue is significant enough to justify
any infrastructure changes in defvar-keymap.

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

Hi,

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Eshel Yaron <me@eshelyaron.com>
>>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>> >   . does the problem happen only in a --without-x build?
>> >   . what "duplicate" definitions do we have there and why?
>>
>> The definition of `completion-preview--mouse-map` binds the events
>> specified by both `mouse-wheel-up-event` and
>> `mouse-wheel-up-alternate-event` to `completion-preview-prev-candidate`
>> (using `defvar-keymap`).  When these two variables have the same value,
>> as apparently happens in John's `--without-x` build but could probably
>> also occur in other setups, `defvar-keymap` complains (signals an error)
>> about a duplicate definition.  The duplication itself is harmless in
>> this case, but there doesn't seem to be a way to tell `defvar-keymap`
>> not to worry about it.
>
> We should try not to produce duplicate bindings here.

I'm not sure how to do that without giving up on using `defvar-keymap`
to define this keymap, which seems less elegant.  That's why I asked for
other ideas a couple of messages ago :)

> Is there a real possibility that these will be the same events in GUI
> builds?

I don't know.  It's certainly possible, but I can't tell how common it
is for it to happen in practice.

> If so, the definition of bindings should detect that and avoid
> duplicate bindings.  But if this can only happen in a --without-x
> build, then a solution can be much simpler: avoid binding any mwheel
> events at all.  That's why I asked that first question.

Either way, `defvar-keymap` doesn't let us make this kind of choices.
Should we drop it and instead use good old `defvar` along with
`make-sparse-keymap` and `define-key` in this case?

> In any case, I don't think this issue is significant enough to justify
> any infrastructure changes in defvar-keymap.

IIUC, `defvar-keymap` is not flexible enough to support this use case,
or any case in which the programmer cannot guarantee in advance that all
keys are distinct, which they can mostly do only if all keys are static.
It's fairly easy to lift this limitation and make `defvar-keymap` useful
in more situations, including this one, but if you prefer to keep
`defvar-keymap` intact, I can propose a patch for this specific instance
that'll make use of lower level interfaces instead of `defvar-keymap`.


Cheers,

Eshel

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

[Eli Zaretskii]

> From: Eshel Yaron <me@eshelyaron.com>
> Cc: jm@pub.pink,  68213@debbugs.gnu.org
> Date: Fri, 05 Jan 2024 08:17:32 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Eshel Yaron <me@eshelyaron.com>
> >>
> >> Eli Zaretskii <eliz@gnu.org> writes:
> >>
> >> >   . does the problem happen only in a --without-x build?
> >> >   . what "duplicate" definitions do we have there and why?
> >>
> >> The definition of `completion-preview--mouse-map` binds the events
> >> specified by both `mouse-wheel-up-event` and
> >> `mouse-wheel-up-alternate-event` to `completion-preview-prev-candidate`
> >> (using `defvar-keymap`).  When these two variables have the same value,
> >> as apparently happens in John's `--without-x` build but could probably
> >> also occur in other setups, `defvar-keymap` complains (signals an error)
> >> about a duplicate definition.  The duplication itself is harmless in
> >> this case, but there doesn't seem to be a way to tell `defvar-keymap`
> >> not to worry about it.
> >
> > We should try not to produce duplicate bindings here.
> 
> I'm not sure how to do that without giving up on using `defvar-keymap`
> to define this keymap, which seems less elegant.  That's why I asked for
> other ideas a couple of messages ago :)
> 
> > Is there a real possibility that these will be the same events in GUI
> > builds?
> 
> I don't know.  It's certainly possible, but I can't tell how common it
> is for it to happen in practice.

I think the fact that we didn't hear about such problems means the
answer is NO.  This problem happens only on TTY frames.

> > If so, the definition of bindings should detect that and avoid
> > duplicate bindings.  But if this can only happen in a --without-x
> > build, then a solution can be much simpler: avoid binding any mwheel
> > events at all.  That's why I asked that first question.
> 
> Either way, `defvar-keymap` doesn't let us make this kind of choices.
> Should we drop it and instead use good old `defvar` along with
> `make-sparse-keymap` and `define-key` in this case?
> 
> > In any case, I don't think this issue is significant enough to justify
> > any infrastructure changes in defvar-keymap.
> 
> IIUC, `defvar-keymap` is not flexible enough to support this use case,
> or any case in which the programmer cannot guarantee in advance that all
> keys are distinct, which they can mostly do only if all keys are static.
> It's fairly easy to lift this limitation and make `defvar-keymap` useful
> in more situations, including this one, but if you prefer to keep
> `defvar-keymap` intact, I can propose a patch for this specific instance
> that'll make use of lower level interfaces instead of `defvar-keymap`.

Adding Stefan to the discussion in case he has any suggestions.

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

[Eli Zaretskii]

> From: john muhl <jm@pub.pink>
> Cc: 68213@debbugs.gnu.org
> Date: Tue, 02 Jan 2024 16:49:15 -0600
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: john muhl <jm@pub.pink>
> >> Cc: 68213@debbugs.gnu.org
> >> Date: Tue, 02 Jan 2024 11:48:49 -0600
> >> 
> >> Eli Zaretskii <eliz@gnu.org> writes:
> >> 
> >> >> Date: Tue, 02 Jan 2024 10:42:59 -0600
> >> >> From:  john muhl via "Bug reports for GNU Emacs,
> >> >>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> >> >> 
> >> >> Running ‘make -k -C test check-lisp’ on master with a build
> >> >> configured without X produces the following error:
> >> >> 
> >> >>   cd emacs
> >> >>   ./autogen.sh
> >> >>   ./configure --without-x
> >> >>   make -j$(nproc)
> >> >>   make -k -C test check-lisp
> >> >>   …
> >> >>   In toplevel form:
> >> >>   lisp/completion-preview-tests.el:23:2: Error: Symbol's value
> >> >>   as variable is void: mouse-wheel-up-event
> >> >>   make[3]: *** [Makefile:159: lisp/completion-preview-tests.elc] Error 1
> >> >>   make[3]: Target 'lisp/completion-preview-tests.log' not remade because of errors.
> >> >
> >> > Thanks, please try again, I hope I fixed this.
> >> 
> >> That removes the warnings during make
> >
> > I see error, not warning.
> >
> >> but unfortunately the test still fails with the same error.
> >
> > Which error?
> >
> > Please say
> >
> >   $ cd test && make lisp/completion-preview-tests.log
> >
> > and then post the part of the log with the full error message.
> 
>   $ cd test && make lisp/completion-preview-tests.log
>     ELC      lisp/completion-preview-tests.elc
> 
>   In toplevel form:
>   lisp/completion-preview-tests.el:23:2: Error: Symbol's value as
>   variable is void: mouse-wheel-up-event
>   make: *** [Makefile:159: lisp/completion-preview-tests.elc] Error 1
> 
>   $ cat lisp/completion-preview-tests.log
>   cat: lisp/completion-preview-tests.log: No such file or directory
> 
>   $ git rev-parse HEAD
>   0bc42eec9836d5f977d4187d57c829895726b862

I don't understand how that can happen when completion-preview.el has
these symbols defvar'ed.  Stefan, what am I missing here?

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

> In general, it is not safe to load packages that assume GUI
> capabilities in a build that excludes those capabilities.

While that tens to be the case, I consider such things bugs and we
should try to fix them.

Emacs *will* load ELisp files "on a whim" in some circumstances, so
ELisp files should always strive to be "harmless".


        Stefan

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

> The definition of `completion-preview--mouse-map` binds the events
> specified by both `mouse-wheel-up-event` and
> `mouse-wheel-up-alternate-event` to `completion-preview-prev-candidate`
> (using `defvar-keymap`).

Hmm... I don't understand the purpose of
`mouse-wheel-up/down/left/right-alternate-event` (nor why they're
`defcustom`s).

Po Lu?


        Stefan

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

> I don't understand how that can happen when completion-preview.el has
> these symbols defvar'ed.  Stefan, what am I missing here?

(defvar VAR) does not define the variable, it only tells the byte
compiler that the var is probably defined elsewhere, and that it should
be considered as dynamically scoped rather than statically scoped.

IOW, it just silenced the warnings.


        Stefan

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: john muhl <jm@pub.pink>,  68213@debbugs.gnu.org
> Date: Sun, 07 Jan 2024 12:03:45 -0500
> 
> > I don't understand how that can happen when completion-preview.el has
> > these symbols defvar'ed.  Stefan, what am I missing here?
> 
> (defvar VAR) does not define the variable, it only tells the byte
> compiler that the var is probably defined elsewhere, and that it should
> be considered as dynamically scoped rather than statically scoped.
> 
> IOW, it just silenced the warnings.

So in that case we need something like

  (or (boundp 'mouse-wheel-up-event)
      (defvar mouse-wheel-up-event nil))

?

But I guess I will wait until Po Lu explains why these are defined as
they are before doing something about this test failure.

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

> IIUC, `defvar-keymap` is not flexible enough to support this use case,
> or any case in which the programmer cannot guarantee in advance that all
> keys are distinct, which they can mostly do only if all keys are static.

Yeah, I think signaling an error like we do now is a bit drastic.
Why not demote it to a warning?


        Stefan

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  jm@pub.pink,  68213@debbugs.gnu.org
> Date: Sun, 07 Jan 2024 12:15:25 -0500
> 
> > IIUC, `defvar-keymap` is not flexible enough to support this use case,
> > or any case in which the programmer cannot guarantee in advance that all
> > keys are distinct, which they can mostly do only if all keys are static.
> 
> Yeah, I think signaling an error like we do now is a bit drastic.
> Why not demote it to a warning?

I don't mind, but that's a separate issue.  Even a warning in running
a test is something we should avoid, so I'm interested in solving that
problem first.

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

> So in that case we need something like
>
>   (or (boundp 'mouse-wheel-up-event)
>       (defvar mouse-wheel-up-event nil))

That's basically equivalent to

    (defvar mouse-wheel-up-event nil)

But no, it would be wrong for completion-preview.el to define
mwheel.el's variables.

Instead we should either (require 'mwheel) or test `boundp` when we
*use* the variable.

Sadly, `defvar-keymap` does not offer any convenient way to specify
conditional bindings, tho we could use a hack like

    (key-description
     (vector (or (bound-and-true-p mouse-wheel-up-event)
                 'completion-preview--dummy-wheel-up)))

Overall, we can work around the limitations of `defvar-keymap` with the
ugly patch below.

> But I guess I will wait until Po Lu explains why these are defined as
> they are before doing something about this test failure.

I have the impression that `mwheel.el` should just blindly always use
`wheel-up/down/..` events (i.e. hard code them), and optionally
also obey `mouse-5/4` on those systems that need them (based on
`mouse-wheel-up/down-event` custom vars).


        Stefan


diff --git a/lisp/completion-preview.el b/lisp/completion-preview.el
index baadb4714b1..8d7ad97d2c3 100644
--- a/lisp/completion-preview.el
+++ b/lisp/completion-preview.el
@@ -128,19 +128,31 @@ completion-preview-active-mode-map
   ;; "M-p" #'completion-preview-prev-candidate
   )
 
-(defvar mouse-wheel-up-event)
-(defvar mouse-wheel-up-alternate-event)
-(defvar mouse-wheel-down-event)
-(defvar mouse-wheel-down-alternate-event)
 (defvar-keymap completion-preview--mouse-map
   :doc "Keymap for mouse clicks on the completion preview."
   "<down-mouse-1>" #'completion-preview-insert
   "C-<down-mouse-1>" #'completion-at-point
   "<down-mouse-2>" #'completion-at-point
-  (format "<%s>" mouse-wheel-up-event)             #'completion-preview-prev-candidate
-  (format "<%s>" mouse-wheel-up-alternate-event)   #'completion-preview-prev-candidate
-  (format "<%s>" mouse-wheel-down-event)           #'completion-preview-next-candidate
-  (format "<%s>" mouse-wheel-down-alternate-event) #'completion-preview-next-candidate)
+  (key-description (vector (or (bound-and-true-p mouse-wheel-up-event)
+                               'completion-preview--dummy-wheel-up)))
+  #'completion-preview-prev-candidate
+  (key-description
+   (vector (let ((event (bound-and-true-p mouse-wheel-up-alternate-event)))
+             (if (and event (not (eq event (bound-and-true-p
+                                            mouse-wheel-up-event))))
+                 event
+               'completion-preview--dummy-wheel-up-alt))))
+  #'completion-preview-prev-candidate
+  (key-description (vector (or (bound-and-true-p mouse-wheel-down-event)
+                               'completion-preview--dummy-wheel-down)))
+  #'completion-preview-next-candidate
+  (key-description
+   (vector (let ((event (bound-and-true-p mouse-wheel-down-alternate-event)))
+             (if (and event (not (eq event (bound-and-true-p
+                                            mouse-wheel-down-event))))
+                 event
+               'completion-preview--dummy-wheel-down-alt))))
+  #'completion-preview-next-candidate)
 
 (defvar-local completion-preview--overlay nil)
 

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

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

>> The definition of `completion-preview--mouse-map` binds the events
>> specified by both `mouse-wheel-up-event` and
>> `mouse-wheel-up-alternate-event` to `completion-preview-prev-candidate`
>> (using `defvar-keymap`).
>
> Hmm... I don't understand the purpose of
> `mouse-wheel-up/down/left/right-alternate-event` (nor why they're
> `defcustom`s).
>
> Po Lu?
>
>
>         Stefan

The events generated for mouse wheel events vary by window system, and
at times Emacs's choice is incorrect, such as when mouse-4 through
mouse-7 are true mouse buttons rather than representations of the mouse
wheel.

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

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

>>> The definition of `completion-preview--mouse-map` binds the events
>>> specified by both `mouse-wheel-up-event` and
>>> `mouse-wheel-up-alternate-event` to `completion-preview-prev-candidate`
>>> (using `defvar-keymap`).
>>
>> Hmm... I don't understand the purpose of
>> `mouse-wheel-up/down/left/right-alternate-event` (nor why they're
>> `defcustom`s).
>>
>> Po Lu?
>>
>>
>>         Stefan
>
> The events generated for mouse wheel events vary by window system, and
> at times Emacs's choice is incorrect, such as when mouse-4 through
> mouse-7 are true mouse buttons rather than representations of the mouse
> wheel.

But why is `mouse-4` sometimes on `mouse-wheel-down-event` and sometimes
on `mouse-wheel-down-alternate-event`?  What difference does it make?

But, AFAICT the `wheel-up/down` events always mean the same, so I think
`mwheel.el` should just always bind `wheel-DIR` events and then the
`mouse-wheel-DIR-event` vars are used only for those cases where other
events also need to be bound, like in the patch below, WDYT?


        Stefan

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

diff --git a/lisp/mwheel.el b/lisp/mwheel.el
index b75b6f27d53..50ac632b4eb 100644
--- a/lisp/mwheel.el
+++ b/lisp/mwheel.el
@@ -60,46 +60,24 @@ mouse-wheel-down-event
   (if (or (featurep 'w32-win) (featurep 'ns-win)
           (featurep 'haiku-win) (featurep 'pgtk-win)
           (featurep 'android-win))
-      'wheel-up
+      nil
     'mouse-4)
-  "Event used for scrolling down."
+  "Event used for scrolling down, beside `wheel-down', if any."
   :group 'mouse
   :type 'symbol
   :set 'mouse-wheel-change-button)
 
-(defcustom mouse-wheel-down-alternate-event
-  (if (featurep 'xinput2)
-      'wheel-up
-    (unless (featurep 'x)
-      'mouse-4))
-  "Alternative wheel down event to consider."
-  :group 'mouse
-  :type 'symbol
-  :version "29.1"
-  :set 'mouse-wheel-change-button)
-
 (defcustom mouse-wheel-up-event
   (if (or (featurep 'w32-win) (featurep 'ns-win)
           (featurep 'haiku-win) (featurep 'pgtk-win)
           (featurep 'android-win))
-      'wheel-down
+      nil
     'mouse-5)
-  "Event used for scrolling up."
+  "Event used for scrolling up, beside `wheel-up', if any."
   :group 'mouse
   :type 'symbol
   :set 'mouse-wheel-change-button)
 
-(defcustom mouse-wheel-up-alternate-event
-  (if (featurep 'xinput2)
-      'wheel-down
-    (unless (featurep 'x)
-      'mouse-5))
-  "Alternative wheel up event to consider."
-  :group 'mouse
-  :type 'symbol
-  :version "29.1"
-  :set 'mouse-wheel-change-button)
-
 (defcustom mouse-wheel-click-event 'mouse-2
   "Event that should be temporarily inhibited after mouse scrolling.
 The mouse wheel is typically on the mouse-2 button, so it may easily
@@ -222,8 +200,8 @@ mwheel-event-button
     (if (eq 'mouse-wheel x)
         (let ((amount (car (cdr (cdr (cdr event))))))
           (if (< amount 0)
-              mouse-wheel-up-event
-            mouse-wheel-down-event))
+              'wheel-up
+            'wheel-down))
       x)))
 
 (defun mwheel-event-window (event)
@@ -258,31 +236,17 @@ mouse-wheel-left-event
   (if (or (featurep 'w32-win) (featurep 'ns-win)
           (featurep 'haiku-win) (featurep 'pgtk-win)
           (featurep 'android-win))
-      'wheel-left
+      nil
     'mouse-6)
-  "Event used for scrolling left.")
-
-(defvar mouse-wheel-left-alternate-event
-  (if (featurep 'xinput2)
-      'wheel-left
-    (unless (featurep 'x)
-      'mouse-6))
-  "Alternative wheel left event to consider.")
+  "Event used for scrolling left, beside `wheel-left', if any.")
 
 (defvar mouse-wheel-right-event
   (if (or (featurep 'w32-win) (featurep 'ns-win)
           (featurep 'haiku-win) (featurep 'pgtk-win)
           (featurep 'android-win))
-      'wheel-right
+      nil
     'mouse-7)
-  "Event used for scrolling right.")
-
-(defvar mouse-wheel-right-alternate-event
-  (if (featurep 'xinput2)
-      'wheel-right
-    (unless (featurep 'x)
-      'mouse-7))
-  "Alternative wheel right event to consider.")
+  "Event used for scrolling right, beside `wheel-right', if any.")
 
 (defun mouse-wheel--get-scroll-window (event)
   "Return window for mouse wheel event EVENT.
@@ -311,6 +275,15 @@ mouse-wheel--get-scroll-window
                frame nil t)))))
       (mwheel-event-window event)))
 
+(defmacro mwheel--is-dir-p (dir button)
+  (declare (debug (sexp form)))
+  (let ((custom-var (intern (format "mouse-wheel-%s-event" dir)))
+        (event (intern (format "wheel-%s" dir))))
+    (macroexp-let2 nil butsym button
+      `(or (eq ,butsym ',event)
+           ;; We presume here `button' is never nil.
+           (eq ,butsym ,custom-var)))))
+
 (defun mwheel-scroll (event &optional arg)
   "Scroll up or down according to the EVENT.
 This should be bound only to mouse buttons 4, 5, 6, and 7 on
@@ -348,17 +321,17 @@ mwheel-scroll
     (condition-case nil
         (unwind-protect
 	    (let ((button (mwheel-event-button event)))
-              (cond ((and (eq amt 'hscroll) (memq button (list mouse-wheel-down-event
-                                                               mouse-wheel-down-alternate-event)))
+              (cond ((and (eq amt 'hscroll)
+                          (mwheel--is-dir-p down button))
                      (when (and (natnump arg) (> arg 0))
                        (setq mouse-wheel-scroll-amount-horizontal arg))
                      (funcall (if mouse-wheel-flip-direction
                                   mwheel-scroll-left-function
                                 mwheel-scroll-right-function)
                               mouse-wheel-scroll-amount-horizontal))
-                    ((memq button (list mouse-wheel-down-event
-                                        mouse-wheel-down-alternate-event))
-                     (condition-case nil (funcall mwheel-scroll-down-function amt)
+                    ((mwheel--is-dir-p down button)
+                     (condition-case nil
+                         (funcall mwheel-scroll-down-function amt)
                        ;; Make sure we do indeed scroll to the beginning of
                        ;; the buffer.
                        (beginning-of-buffer
@@ -372,31 +345,32 @@ mwheel-scroll
                           ;; for a reason that escapes me.  This problem seems
                           ;; to only affect scroll-down.  --Stef
                           (set-window-start (selected-window) (point-min))))))
-                    ((and (eq amt 'hscroll) (memq button (list mouse-wheel-up-event
-                                                               mouse-wheel-up-alternate-event)))
+                    ((and (eq amt 'hscroll)
+                          (mwheel--is-dir-p up button))
                      (when (and (natnump arg) (> arg 0))
                        (setq mouse-wheel-scroll-amount-horizontal arg))
                      (funcall (if mouse-wheel-flip-direction
                                   mwheel-scroll-right-function
                                 mwheel-scroll-left-function)
                               mouse-wheel-scroll-amount-horizontal))
-                    ((memq button (list mouse-wheel-up-event
-                                        mouse-wheel-up-alternate-event))
-                     (condition-case nil (funcall mwheel-scroll-up-function amt)
+                    ((mwheel--is-dir-p up button)
+                     (condition-case nil
+                         (funcall mwheel-scroll-up-function amt)
                        ;; Make sure we do indeed scroll to the end of the buffer.
-                       (end-of-buffer (while t (funcall mwheel-scroll-up-function)))))
-                    ((memq button (list mouse-wheel-left-event
-                                        mouse-wheel-left-alternate-event)) ; for tilt scroll
+                       (end-of-buffer
+                        (while t (funcall mwheel-scroll-up-function)))))
+                    ((mwheel--is-dir-p left button) ; for tilt scroll
                      (when mouse-wheel-tilt-scroll
                        (funcall (if mouse-wheel-flip-direction
                                     mwheel-scroll-right-function
-                                  mwheel-scroll-left-function) amt)))
-                    ((memq button (list mouse-wheel-right-event
-                                        mouse-wheel-right-alternate-event)) ; for tilt scroll
+                                  mwheel-scroll-left-function)
+                                amt)))
+                    ((mwheel--is-dir-p right button) ; for tilt scroll
                      (when mouse-wheel-tilt-scroll
                        (funcall (if mouse-wheel-flip-direction
                                     mwheel-scroll-left-function
-                                  mwheel-scroll-right-function) amt)))
+                                  mwheel-scroll-right-function)
+                                amt)))
 		    (t (error "Bad binding in mwheel-scroll"))))
           (if (eq scroll-window selected-window)
               ;; If there is a temporarily active region, deactivate it if
@@ -437,11 +411,9 @@ mouse-wheel-text-scale
         (button (mwheel-event-button event)))
     (select-window scroll-window 'mark-for-redisplay)
     (unwind-protect
-        (cond ((memq button (list mouse-wheel-down-event
-                                  mouse-wheel-down-alternate-event))
+        (cond ((mwheel--is-dir-p down button)
                (text-scale-increase 1))
-              ((memq button (list mouse-wheel-up-event
-                                  mouse-wheel-up-alternate-event))
+              ((mwheel--is-dir-p up button)
                (text-scale-decrease 1)))
       (select-window selected-window))))
 
@@ -451,11 +423,9 @@ mouse-wheel-global-text-scale
 This invokes `global-text-scale-adjust', which see."
   (interactive (list last-input-event))
   (let ((button (mwheel-event-button event)))
-    (cond ((memq button (list mouse-wheel-down-event
-                              mouse-wheel-down-alternate-event))
+    (cond ((mwheel--is-dir-p down button)
            (global-text-scale-adjust 1))
-          ((memq button (list mouse-wheel-up-event
-                              mouse-wheel-up-alternate-event))
+          ((mwheel--is-dir-p up button)
            (global-text-scale-adjust -1)))))
 
 (defun mouse-wheel--add-binding (key fun)
@@ -507,15 +477,13 @@ mouse-wheel--setup-bindings
      ;; Bindings for changing font size.
      ((and (consp binding) (eq (cdr binding) 'text-scale))
       (dolist (event (list mouse-wheel-down-event mouse-wheel-up-event
-                           mouse-wheel-down-alternate-event
-                           mouse-wheel-up-alternate-event))
+                           'wheel-down 'wheel-up))
         (when event
           (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
-                           mouse-wheel-down-alternate-event
-                           mouse-wheel-up-alternate-event))
+                           'wheel-down 'wheel-up))
         (when event
           (mouse-wheel--add-binding `[,(append (car binding) (list event))]
                                     'mouse-wheel-global-text-scale))))
@@ -523,10 +491,7 @@ mouse-wheel--setup-bindings
      (t
       (dolist (event (list mouse-wheel-down-event mouse-wheel-up-event
                            mouse-wheel-left-event mouse-wheel-right-event
-                           mouse-wheel-down-alternate-event
-                           mouse-wheel-up-alternate-event
-                           mouse-wheel-left-alternate-event
-                           mouse-wheel-right-alternate-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))))))))

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

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

> But why is `mouse-4` sometimes on `mouse-wheel-down-event` and sometimes
> on `mouse-wheel-down-alternate-event`?  What difference does it make?
>
> But, AFAICT the `wheel-up/down` events always mean the same, so I think
> `mwheel.el` should just always bind `wheel-DIR` events and then the
> `mouse-wheel-DIR-event` vars are used only for those cases where other
> events also need to be bound, like in the patch below, WDYT?

I think we should let sleeping dogs lie, because the last time changes
were made to these options in mwheel.el, several obscure problems
emerged under text terminals that required another round of changes,
yielding the current values.

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Eshel Yaron <me@eshelyaron.com>,  Eli Zaretskii <eliz@gnu.org>,
>   jm@pub.pink,  68213@debbugs.gnu.org
> Date: Mon, 08 Jan 2024 14:16:54 +0800
> 
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> > But why is `mouse-4` sometimes on `mouse-wheel-down-event` and sometimes
> > on `mouse-wheel-down-alternate-event`?  What difference does it make?
> >
> > But, AFAICT the `wheel-up/down` events always mean the same, so I think
> > `mwheel.el` should just always bind `wheel-DIR` events and then the
> > `mouse-wheel-DIR-event` vars are used only for those cases where other
> > events also need to be bound, like in the patch below, WDYT?
> 
> I think we should let sleeping dogs lie, because the last time changes
> were made to these options in mwheel.el, several obscure problems
> emerged under text terminals that required another round of changes,
> yielding the current values.

There's one dog we cannot let lie: the one that fails the test in
completion-preview-tests to fail to compile -- this is what started
this bug report, and I would like to solve that problem.

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Po Lu <luangruo@yahoo.com>
>> Cc: Eshel Yaron <me@eshelyaron.com>,  Eli Zaretskii <eliz@gnu.org>,
>>   jm@pub.pink,  68213@debbugs.gnu.org
>> Date: Mon, 08 Jan 2024 14:16:54 +0800
>> 
>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> 
>> > But why is `mouse-4` sometimes on `mouse-wheel-down-event` and sometimes
>> > on `mouse-wheel-down-alternate-event`?  What difference does it make?
>> >
>> > But, AFAICT the `wheel-up/down` events always mean the same, so I think
>> > `mwheel.el` should just always bind `wheel-DIR` events and then the
>> > `mouse-wheel-DIR-event` vars are used only for those cases where other
>> > events also need to be bound, like in the patch below, WDYT?
>> 
>> I think we should let sleeping dogs lie, because the last time changes
>> were made to these options in mwheel.el, several obscure problems
>> emerged under text terminals that required another round of changes,
>> yielding the current values.
>
> There's one dog we cannot let lie: the one that fails the test in
> completion-preview-tests to fail to compile -- this is what started
> this bug report, and I would like to solve that problem.

From a cursory reading of this bug report, I gather that m-w-*-event and
m-w-*-alternate-event are at times defined to identical values, which
causes defvar-keymap to signal?  In that situation the correct course of
action is using [remap ...] to bind the mwheel commands to whatever
completion-preview needs it set to, right?

HTH.

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

>> But why is `mouse-4` sometimes on `mouse-wheel-down-event` and sometimes
>> on `mouse-wheel-down-alternate-event`?  What difference does it make?

Any chance you could try and help me figure out answers to those questions?

At the very least we have a documentation problem because, as a user
reading the docstring of `mouse-wheel-down-alternate-event`, I have no
idea what this means or does and how it differs from
`mouse-wheel-down-event`.

`mouse-wheel-down-event` is a defcustom because X11 failed
to define events for scroll wheel, and depending on your mouse, those
were not always mapped to the same `mouse-N` events.  That doesn't seem
to apply to `mouse-wheel-down-alternate-event`.

>> But, AFAICT the `wheel-up/down` events always mean the same, so I think
>> `mwheel.el` should just always bind `wheel-DIR` events and then the
>> `mouse-wheel-DIR-event` vars are used only for those cases where other
>> events also need to be bound, like in the patch below, WDYT?
> I think we should let sleeping dogs lie,

These vars were new in Emacs-29, so they're still pretty awake, AFAIC.

> because the last time changes were made to these options in mwheel.el,
> several obscure problems emerged under text terminals that required
> another round of changes, yielding the current values.

Do you happen to remember the bug numbers of bug titles or something to
help me find info about those obscure problems?


        Stefan

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: jm@pub.pink,  me@eshelyaron.com,  monnier@iro.umontreal.ca,
>   68213@debbugs.gnu.org
> Date: Mon, 08 Jan 2024 22:20:17 +0800
> 
> > There's one dog we cannot let lie: the one that fails the test in
> > completion-preview-tests to fail to compile -- this is what started
> > this bug report, and I would like to solve that problem.
> 
> >From a cursory reading of this bug report, I gather that m-w-*-event and
> m-w-*-alternate-event are at times defined to identical values, which
> causes defvar-keymap to signal?  In that situation the correct course of
> action is using [remap ...] to bind the mwheel commands to whatever
> completion-preview needs it set to, right?

I don't know.  Stefan, can you help?

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

>> There's one dog we cannot let lie: the one that fails the test in
>> completion-preview-tests to fail to compile -- this is what started
>> this bug report, and I would like to solve that problem.
>
> From a cursory reading of this bug report, I gather that m-w-*-event and
> m-w-*-alternate-event are at times defined to identical values, which
> causes defvar-keymap to signal?  In that situation the correct course of
> action is using [remap ...] to bind the mwheel commands to whatever
> completion-preview needs it set to, right?

I considered doing that, but it's not great:

- The same command is used for up and down, so we'd have to change the
  completion-preview code to have a new command that looks at the
  invoking event to decide whether to scroll up or down.
- The same command is also used for left/right, which we currently
  don't override in completion-preview, but which a `remap` would
  also affect.
- I'm not sure it's quite the semantics we want (I don't think we want
  these completion-preview bindings to depend on
  `mouse-wheel-scroll-amount`).

None of those problems are deal breaking, of course, but overall it's
not a much better option.  And I suspect that once we sort out the
m-w-*-event problems, fixing the completion-preview problems will be
even easier.


        Stefan

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

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

> Any chance you could try and help me figure out answers to those questions?
>
> At the very least we have a documentation problem because, as a user
> reading the docstring of `mouse-wheel-down-alternate-event`, I have no
> idea what this means or does and how it differs from
> `mouse-wheel-down-event`.
>
> `mouse-wheel-down-event` is a defcustom because X11 failed
> to define events for scroll wheel, and depending on your mouse, those
> were not always mapped to the same `mouse-N` events.  That doesn't seem
> to apply to `mouse-wheel-down-alternate-event`.

mouse-wheel-*-alternate-event were initially meant to reflect a second
type of event generated by the window system in response to scroll wheel
motion.  At the time, it was set on XInput2 builds and no more, where
wheel-* events are generated in addition to the customary mouse-*
events.

This arrangement was later modified before the release to set
mouse-wheel-*-alternate-event to mouse-* wherever it had previously been
nil, to provide for xterm-mouse-mode generating such events whatever the
default value of mouse-wheel-*-event.

As such, I suppose the difference is that between the mouse event
generally emitted by the window system, and events which might be
encountered under special circumstances.

> Do you happen to remember the bug numbers of bug titles or something to
> help me find info about those obscure problems?

I think searching the bug tracker for "xterm-mouse-mode" and "mwheel"
will help, and I also recall that the PGTK port was involved in some
manner.

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

>> Any chance you could try and help me figure out answers to those questions?
>>
>> At the very least we have a documentation problem because, as a user
>> reading the docstring of `mouse-wheel-down-alternate-event`, I have no
>> idea what this means or does and how it differs from
>> `mouse-wheel-down-event`.
>>
>> `mouse-wheel-down-event` is a defcustom because X11 failed
>> to define events for scroll wheel, and depending on your mouse, those
>> were not always mapped to the same `mouse-N` events.  That doesn't seem
>> to apply to `mouse-wheel-down-alternate-event`.
>
> mouse-wheel-*-alternate-event were initially meant to reflect a second
> type of event generated by the window system in response to scroll wheel
> motion.  At the time, it was set on XInput2 builds and no more, where
> wheel-* events are generated in addition to the customary mouse-*
> events.
>
> This arrangement was later modified before the release to set
> mouse-wheel-*-alternate-event to mouse-* wherever it had previously been
> nil, to provide for xterm-mouse-mode generating such events whatever the
> default value of mouse-wheel-*-event.
>
> As such, I suppose the difference is that between the mouse event
> generally emitted by the window system, and events which might be
> encountered under special circumstances.

You're describing the difference in terms of what they're currently set
to, but I'm asking the difference in their effect.  I.e. what's the
difference between

    (setq mouse-wheel-*-alternate-event A)
    (setq mouse-wheel-*-event B)

and

    (setq mouse-wheel-*-alternate-event B)
    (setq mouse-wheel-*-event A)

My patch is based on the idea that the difference doesn't matter and
that one of the two can always hold `wheel-up/down` so we can
hardcode it.

>> Do you happen to remember the bug numbers of bug titles or something to
>> help me find info about those obscure problems?
> I think searching the bug tracker for "xterm-mouse-mode" and "mwheel"
> will help, and I also recall that the PGTK port was involved in some
> manner.

Thanks.  I see that bug#50321 suggests we keep `mouse-4/5` in the
`mouse-wheel-*-event` for any build that can use `xterm-mouse-mode`.
It should be easy to adjust my patch to support that.
Tho bug#49803 suggests we could fix it in `xt-mouse.el` as well and make
`xt-mouse.el` emit `wheel-up/down` events instead.


        Stefa

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

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

> Thanks.  I see that bug#50321 suggests we keep `mouse-4/5` in the
> `mouse-wheel-*-event` for any build that can use `xterm-mouse-mode`.
> It should be easy to adjust my patch to support that.
> Tho bug#49803 suggests we could fix it in `xt-mouse.el` as well and make
> `xt-mouse.el` emit `wheel-up/down` events instead.

The patch below still seems to handle simultaneous mixes of wheel-up and
mouse-4/5 buttons via tty+GUI frames.  And based on my understanding of
the code, it should behave exactly like the current code in pretty much
all circumstances.

It also gets rid of `mwheel-event-button` which has been obsolete for
the last 20 years, apparently.  This is notable, because
`mwheel-event-button` is the only part of the code that distinguishes
between `mouse-wheel-*-event` and
`mouse-wheel-*-alternate-event`, AFAICT.

It still contains a FIXME, tho:

    (defcustom mouse-wheel-use-old-style-wheel-buttons
      ;; FIXME: Is this ever non-nil in practice?
      (not (and (or (featurep 'w32-win) (featurep 'ns-win)
                    (featurep 'haiku-win) (featurep 'pgtk-win)
                    (featurep 'android-win))
                (or (featurep 'xinput2)
                    (featurep 'x))))
      "If non-nil, treat mouse-4/5/6/7 as wheel buttons.
    These are the event names used historically in X11 before XInput2.
    They are sometimes used by things like `xterm-mouse-mode' as well."
      :group 'mouse
      :type 'boolean)

The above code is a slight shuffling of the current code which uses

    (if (or (featurep 'w32-win) (featurep 'ns-win)
            (featurep 'haiku-win) (featurep 'pgtk-win)
            (featurep 'android-win))
        'wheel-up
      'mouse-4)
    [...]
    (if (featurep 'xinput2)
        'wheel-up
      (unless (featurep 'x)
        'mouse-4))

repeated 4 times.

Is it worth worrying about the case where this monstrosity is non-nil?


        Stefan

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

diff --git a/lisp/mwheel.el b/lisp/mwheel.el
index b75b6f27d53..ca6d3cccb97 100644
--- a/lisp/mwheel.el
+++ b/lisp/mwheel.el
@@ -56,49 +56,32 @@ mouse-wheel-change-button
              (bound-and-true-p mouse-wheel-mode))
     (mouse-wheel-mode 1)))
 
-(defcustom mouse-wheel-down-event
-  (if (or (featurep 'w32-win) (featurep 'ns-win)
-          (featurep 'haiku-win) (featurep 'pgtk-win)
-          (featurep 'android-win))
-      'wheel-up
-    'mouse-4)
-  "Event used for scrolling down."
+(defcustom mouse-wheel-use-old-style-wheel-buttons
+  ;; FIXME: Is this ever non-nil in practice?
+  (not (and (or (featurep 'w32-win) (featurep 'ns-win)
+                (featurep 'haiku-win) (featurep 'pgtk-win)
+                (featurep 'android-win))
+            (or (featurep 'xinput2)
+                (featurep 'x))))
+  "If non-nil, treat mouse-4/5/6/7 as wheel buttons.
+These are the event names used historically in X11 before XInput2.
+They are sometimes used by things like `xterm-mouse-mode' as well."
   :group 'mouse
-  :type 'symbol
-  :set 'mouse-wheel-change-button)
-
-(defcustom mouse-wheel-down-alternate-event
-  (if (featurep 'xinput2)
-      'wheel-up
-    (unless (featurep 'x)
-      'mouse-4))
-  "Alternative wheel down event to consider."
+  :type 'boolean)
+
+(defcustom mouse-wheel-down-event
+  (if mouse-wheel-use-old-style-wheel-buttons 'mouse-4)
+  "Event used for scrolling down, beside `wheel-down', if any."
   :group 'mouse
   :type 'symbol
-  :version "29.1"
-  :set 'mouse-wheel-change-button)
+  :set #'mouse-wheel-change-button)
 
 (defcustom mouse-wheel-up-event
-  (if (or (featurep 'w32-win) (featurep 'ns-win)
-          (featurep 'haiku-win) (featurep 'pgtk-win)
-          (featurep 'android-win))
-      'wheel-down
-    'mouse-5)
-  "Event used for scrolling up."
+  (if mouse-wheel-use-old-style-wheel-buttons 'mouse-5)
+  "Event used for scrolling up, beside `wheel-up', if any."
   :group 'mouse
   :type 'symbol
-  :set 'mouse-wheel-change-button)
-
-(defcustom mouse-wheel-up-alternate-event
-  (if (featurep 'xinput2)
-      'wheel-down
-    (unless (featurep 'x)
-      'mouse-5))
-  "Alternative wheel up event to consider."
-  :group 'mouse
-  :type 'symbol
-  :version "29.1"
-  :set 'mouse-wheel-change-button)
+  :set #'mouse-wheel-change-button)
 
 (defcustom mouse-wheel-click-event 'mouse-2
   "Event that should be temporarily inhibited after mouse scrolling.
@@ -108,7 +91,7 @@ mouse-wheel-click-event
 set to the event sent when clicking on the mouse wheel button."
   :group 'mouse
   :type 'symbol
-  :set 'mouse-wheel-change-button)
+  :set #'mouse-wheel-change-button)
 
 (defcustom mouse-wheel-inhibit-click-time 0.35
   "Time in seconds to inhibit clicking on mouse wheel button after scroll."
@@ -165,7 +148,7 @@ mouse-wheel-scroll-amount
                     (const :tag "Scroll horizontally" :value hscroll)
                     (const :tag "Change buffer face size" :value text-scale)
                     (const :tag "Change global face size" :value global-text-scale)))))
-  :set 'mouse-wheel-change-button
+  :set #'mouse-wheel-change-button
   :version "28.1")
 
 (defcustom mouse-wheel-progressive-speed t
@@ -216,15 +199,10 @@ mouse-wheel-flip-direction
   :type 'boolean
   :version "26.1")
 
-(defun mwheel-event-button (event)
-  (let ((x (event-basic-type event)))
-    ;; Map mouse-wheel events to appropriate buttons
-    (if (eq 'mouse-wheel x)
-        (let ((amount (car (cdr (cdr (cdr event))))))
-          (if (< amount 0)
-              mouse-wheel-up-event
-            mouse-wheel-down-event))
-      x)))
+
+;; This function used to handle the `mouse-wheel` event which was
+;; removed in 2003 by commit 9eb28007fb27, thus making it obsolete.
+(define-obsolete-function-alias 'mwheel-event-button #'event-basic-type "30.1")
 
 (defun mwheel-event-window (event)
   (posn-window (event-start event)))
@@ -255,34 +233,12 @@ mwheel-scroll-right-function
   "Function that does the job of scrolling right.")
 
 (defvar mouse-wheel-left-event
-  (if (or (featurep 'w32-win) (featurep 'ns-win)
-          (featurep 'haiku-win) (featurep 'pgtk-win)
-          (featurep 'android-win))
-      'wheel-left
-    'mouse-6)
-  "Event used for scrolling left.")
-
-(defvar mouse-wheel-left-alternate-event
-  (if (featurep 'xinput2)
-      'wheel-left
-    (unless (featurep 'x)
-      'mouse-6))
-  "Alternative wheel left event to consider.")
+  (if mouse-wheel-use-old-style-wheel-buttons 'mouse-6)
+  "Event used for scrolling left, beside `wheel-left', if any.")
 
 (defvar mouse-wheel-right-event
-  (if (or (featurep 'w32-win) (featurep 'ns-win)
-          (featurep 'haiku-win) (featurep 'pgtk-win)
-          (featurep 'android-win))
-      'wheel-right
-    'mouse-7)
-  "Event used for scrolling right.")
-
-(defvar mouse-wheel-right-alternate-event
-  (if (featurep 'xinput2)
-      'wheel-right
-    (unless (featurep 'x)
-      'mouse-7))
-  "Alternative wheel right event to consider.")
+  (if mouse-wheel-use-old-style-wheel-buttons 'mouse-7)
+  "Event used for scrolling right, beside `wheel-right', if any.")
 
 (defun mouse-wheel--get-scroll-window (event)
   "Return window for mouse wheel event EVENT.
@@ -311,6 +267,15 @@ mouse-wheel--get-scroll-window
                frame nil t)))))
       (mwheel-event-window event)))
 
+(defmacro mwheel--is-dir-p (dir button)
+  (declare (debug (sexp form)))
+  (let ((custom-var (intern (format "mouse-wheel-%s-event" dir)))
+        (event (intern (format "wheel-%s" dir))))
+    (macroexp-let2 nil butsym button
+      `(or (eq ,butsym ',event)
+           ;; We presume here `button' is never nil.
+           (eq ,butsym ,custom-var)))))
+
 (defun mwheel-scroll (event &optional arg)
   "Scroll up or down according to the EVENT.
 This should be bound only to mouse buttons 4, 5, 6, and 7 on
@@ -347,18 +312,18 @@ mwheel-scroll
     (when (numberp amt) (setq amt (* amt (event-line-count event))))
     (condition-case nil
         (unwind-protect
-	    (let ((button (mwheel-event-button event)))
-              (cond ((and (eq amt 'hscroll) (memq button (list mouse-wheel-down-event
-                                                               mouse-wheel-down-alternate-event)))
+	    (let ((button (event-basic-type event)))
+              (cond ((and (eq amt 'hscroll)
+                          (mwheel--is-dir-p down button))
                      (when (and (natnump arg) (> arg 0))
                        (setq mouse-wheel-scroll-amount-horizontal arg))
                      (funcall (if mouse-wheel-flip-direction
                                   mwheel-scroll-left-function
                                 mwheel-scroll-right-function)
                               mouse-wheel-scroll-amount-horizontal))
-                    ((memq button (list mouse-wheel-down-event
-                                        mouse-wheel-down-alternate-event))
-                     (condition-case nil (funcall mwheel-scroll-down-function amt)
+                    ((mwheel--is-dir-p down button)
+                     (condition-case nil
+                         (funcall mwheel-scroll-down-function amt)
                        ;; Make sure we do indeed scroll to the beginning of
                        ;; the buffer.
                        (beginning-of-buffer
@@ -372,31 +337,32 @@ mwheel-scroll
                           ;; for a reason that escapes me.  This problem seems
                           ;; to only affect scroll-down.  --Stef
                           (set-window-start (selected-window) (point-min))))))
-                    ((and (eq amt 'hscroll) (memq button (list mouse-wheel-up-event
-                                                               mouse-wheel-up-alternate-event)))
+                    ((and (eq amt 'hscroll)
+                          (mwheel--is-dir-p up button))
                      (when (and (natnump arg) (> arg 0))
                        (setq mouse-wheel-scroll-amount-horizontal arg))
                      (funcall (if mouse-wheel-flip-direction
                                   mwheel-scroll-right-function
                                 mwheel-scroll-left-function)
                               mouse-wheel-scroll-amount-horizontal))
-                    ((memq button (list mouse-wheel-up-event
-                                        mouse-wheel-up-alternate-event))
-                     (condition-case nil (funcall mwheel-scroll-up-function amt)
+                    ((mwheel--is-dir-p up button)
+                     (condition-case nil
+                         (funcall mwheel-scroll-up-function amt)
                        ;; Make sure we do indeed scroll to the end of the buffer.
-                       (end-of-buffer (while t (funcall mwheel-scroll-up-function)))))
-                    ((memq button (list mouse-wheel-left-event
-                                        mouse-wheel-left-alternate-event)) ; for tilt scroll
+                       (end-of-buffer
+                        (while t (funcall mwheel-scroll-up-function)))))
+                    ((mwheel--is-dir-p left button) ; for tilt scroll
                      (when mouse-wheel-tilt-scroll
                        (funcall (if mouse-wheel-flip-direction
                                     mwheel-scroll-right-function
-                                  mwheel-scroll-left-function) amt)))
-                    ((memq button (list mouse-wheel-right-event
-                                        mouse-wheel-right-alternate-event)) ; for tilt scroll
+                                  mwheel-scroll-left-function)
+                                amt)))
+                    ((mwheel--is-dir-p right button) ; for tilt scroll
                      (when mouse-wheel-tilt-scroll
                        (funcall (if mouse-wheel-flip-direction
                                     mwheel-scroll-left-function
-                                  mwheel-scroll-right-function) amt)))
+                                  mwheel-scroll-right-function)
+                                amt)))
 		    (t (error "Bad binding in mwheel-scroll"))))
           (if (eq scroll-window selected-window)
               ;; If there is a temporarily active region, deactivate it if
@@ -434,14 +400,12 @@ mouse-wheel-text-scale
   (interactive (list last-input-event))
   (let ((selected-window (selected-window))
         (scroll-window (mouse-wheel--get-scroll-window event))
-        (button (mwheel-event-button event)))
+        (button (event-basic-type event)))
     (select-window scroll-window 'mark-for-redisplay)
     (unwind-protect
-        (cond ((memq button (list mouse-wheel-down-event
-                                  mouse-wheel-down-alternate-event))
+        (cond ((mwheel--is-dir-p down button)
                (text-scale-increase 1))
-              ((memq button (list mouse-wheel-up-event
-                                  mouse-wheel-up-alternate-event))
+              ((mwheel--is-dir-p up button)
                (text-scale-decrease 1)))
       (select-window selected-window))))
 
@@ -450,12 +414,10 @@ mouse-wheel-global-text-scale
   "Increase or decrease the global font size according to the EVENT.
 This invokes `global-text-scale-adjust', which see."
   (interactive (list last-input-event))
-  (let ((button (mwheel-event-button event)))
-    (cond ((memq button (list mouse-wheel-down-event
-                              mouse-wheel-down-alternate-event))
+  (let ((button (event-basic-type event)))
+    (cond ((mwheel--is-dir-p down button)
            (global-text-scale-adjust 1))
-          ((memq button (list mouse-wheel-up-event
-                              mouse-wheel-up-alternate-event))
+          ((mwheel--is-dir-p up button)
            (global-text-scale-adjust -1)))))
 
 (defun mouse-wheel--add-binding (key fun)
@@ -507,15 +469,13 @@ mouse-wheel--setup-bindings
      ;; Bindings for changing font size.
      ((and (consp binding) (eq (cdr binding) 'text-scale))
       (dolist (event (list mouse-wheel-down-event mouse-wheel-up-event
-                           mouse-wheel-down-alternate-event
-                           mouse-wheel-up-alternate-event))
+                           'wheel-down 'wheel-up))
         (when event
           (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
-                           mouse-wheel-down-alternate-event
-                           mouse-wheel-up-alternate-event))
+                           'wheel-down 'wheel-up))
         (when event
           (mouse-wheel--add-binding `[,(append (car binding) (list event))]
                                     'mouse-wheel-global-text-scale))))
@@ -523,10 +483,7 @@ mouse-wheel--setup-bindings
      (t
       (dolist (event (list mouse-wheel-down-event mouse-wheel-up-event
                            mouse-wheel-left-event mouse-wheel-right-event
-                           mouse-wheel-down-alternate-event
-                           mouse-wheel-up-alternate-event
-                           mouse-wheel-left-alternate-event
-                           mouse-wheel-right-alternate-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))))))))

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

[Drew Adams]

> It also gets rid of `mwheel-event-button` which has been obsolete for
> the last 20 years, apparently.  This is notable, because
> `mwheel-event-button` is the only part of the code that distinguishes
> between `mouse-wheel-*-event` and
> `mouse-wheel-*-alternate-event`, AFAICT.

I'm not following this thread.  But I did
happen to see that bit.  I have code that uses
`mwheel-event-button', so I'd need to know what
to replace it with.

E.g., I have this `cond' test:

((and (consp evnt)
      (member (event-basic-type (car evnt))
              `(,wheel-up   ,wheel-down
                ,wheel-left ,wheel-right)))
 (let ((button  (mwheel-event-button evnt)))
   (cond ((memq button (list wheel-down wheel-left))
          (setq new-incr  incr))
         ((memq button (list wheel-up   wheel-right))
          (setq new-incr  (if (atom incr)
                              (- incr)
                            (mapcar #'- incr))))

BTW - I don't have any Emacs 30 build, but in Emacs
29.1.2 (and earlier releases) I see nothing about
`mwheel-event-button' being "obsolete".  Why do you
say that's been the case for 20 years now?

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

I just pushed to `scratch/mwheel-no-alts` a set of patches which
I intend to push to `master` soon.

They should fix the test failure in `completion-preview`, and they get
rid of the `mouse-wheel-*-alternate-event`s.

Any comments/objections while I work on the corresponding etc/NEWS and
Texinfo changes?


        Stefan


commit ee2a8fd4cff84cd5bd672fdde8ec3e0800f132be
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Date:   Tue Jan 9 11:34:05 2024 -0500

    (mouse-wheel-*-event): Minor cleanups
    
    * lisp/mwheel.el (mwheel-event-button): Mark as obsolete alias.
    Change all callers.
    
    * lisp/edmacro.el (mouse-wheel-*-event): Move declarations to ...
    (edmacro-fix-menu-commands): ... where we do know that they should
    be defined.  Obey `mouse-wheel-*-alternate-event`s as well.

commit a764b503e126a60ff4ea1266da924de7b020637e
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Date:   Fri Jan 12 17:50:09 2024 -0500

    (mwheel--is-dir-p): New macro to reduce code duplication
    
    It also slightly reduces memory allocation.
    
    * lisp/mwheel.el (mwheel--is-dir-p): New macro.
    (mwheel-scroll, mouse-wheel-text-scale)
    (mouse-wheel-global-text-scale): Use it.

commit 3bd8e963f7c8d762551c2528b2f907b613218a08
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Date:   Fri Jan 12 18:05:14 2024 -0500

    * lisp/keymap.el (define-keymap): Demote "duplicate def" to a warning

commit b9959e94d26bdd406d506ef355612627a0b1406b
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Date:   Fri Jan 12 18:08:31 2024 -0500

    * lisp/completion-preview.el: Fix use in non-GUI session
    
    Fix loading in non-GUI sessions where `mwheel` is not preloaded.
    Not requiring `mwheel` would be a lot more complex, since it would
    require delaying the construction of `completion-preview--mouse-map`.
    
    * lisp/completion-preview.el (<toplevel>): Require `mwheel`.
    Remove correspondingly redundant `defvar`s.
    (completion-preview--mouse-map): Use `key-description` rather than mimicking
    it with `format`.

commit b0f04ce4d3495de8593e3b64ddd218cd3bad221c
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Date:   Fri Jan 12 18:28:12 2024 -0500

    mwheel.el: Unconditionally use the `wheel-up/down/...` events
    
    The `mouse-wheel-DIR-event` vars were introduced because under X11
    we get different `mouse-N` events depending on the users' mouse and
    those same events can be used for other things for other rodents, so we
    can't unconditionally treat those events as mouse-wheel events.
    
    But this does not apply to the `wheel-up/down/...` events.
    So hard code them.
    
    * lisp/mwheel.el (mwheel--is-dir-p): Always consider the `wheel-DIR` events.
    (mouse-wheel--setup-bindings): Always bind the `wheel-DIR` events.
    
    * lisp/completion-preview.el (completion-preview--mouse-map):
    Unconditionally bind the `wheel-DIR` events.
    * lisp/edmacro.el (edmacro-fix-menu-commands): Hard code the
    `wheel-DIR` events as mouse events regardless of `mouse-wheel-*-event`s.
    * lisp/progmodes/flymake.el (flymake--mode-line-counter-map):
    Do nothing, because it's already been done in commit e5be6c7ae309.

commit 8cb8b9736532fdd8f2fc734b08ed55c17b922806
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Date:   Fri Jan 12 18:52:37 2024 -0500

    mwheel.el: Remove `mouse-wheel-*-alternate-event` vars
    
    Now that `wheel-DIR` events are hardcoded, we never need more than
    one variable (which we actually never needed anyway, we could have
    let `mouse-wheel-*-event` vars hold lists of events instead), so
    remove the `mouse-wheel-*-alternate-event` vars by merging their
    default value into that of the corresponding `mouse-wheel-*-event`.
    
    * lisp/mwheel.el (mouse-wheel-down-event, mouse-wheel-up-event)
    (mouse-wheel-left-event, mouse-wheel-right-event): Don't bother holding
    `wheel-DIR` events since these are already handled anyway.
    Hold the event that would have been held in
    `mouse-wheel-DIR-alternate-event` instead.
    (mouse-wheel-down-alternate-event, mouse-wheel-up-alternate-event)
    (mouse-wheel-left-alternate-event, mouse-wheel-right-alternate-event):
    Delete vars.
    (mwheel--is-dir-p, mouse-wheel--setup-bindings):
    * lisp/edmacro.el (edmacro-fix-menu-commands):
    * lisp/completion-preview.el (completion-preview--mouse-map):
    Don't use `mouse-wheel-up/down-alternate-event` any more.
    * lisp/progmodes/flymake.el (flymake--mode-line-counter-map):
    Do nothing, because it already ignored those vars.

commit f355557bb9e93f5d5e7c3fae17e4da398226efb4
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Date:   Fri Jan 12 19:05:24 2024 -0500

    mwheel.el: Code clean to reduce duplication
    
    * lisp/mwheel.el (mouse-wheel-obey-old-style-wheel-buttons): New var,
    extracted from `mouse-wheel-*-event` definitions.
    (mouse-wheel-down-event, mouse-wheel-up-event)
    (mouse-wheel-left-event, mouse-wheel-right-event): Use it.

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: john muhl <jm@pub.pink>,  68213@debbugs.gnu.org
> Date: Fri, 12 Jan 2024 19:16:02 -0500
> 
> I just pushed to `scratch/mwheel-no-alts` a set of patches which
> I intend to push to `master` soon.
> 
> They should fix the test failure in `completion-preview`, and they get
> rid of the `mouse-wheel-*-alternate-event`s.
> 
> Any comments/objections while I work on the corresponding etc/NEWS and
> Texinfo changes?

You expect me to have an opinion by just reading the log messages?  Or
do "git diff" myself?  Or something else?

Po Lu, any comments?

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

[Stefan Kangas]

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

Eli Zaretskii <eliz@gnu.org> writes:

> You expect me to have an opinion by just reading the log messages?  Or
> do "git diff" myself?  Or something else?

I've attached the patches below.

[-- Attachment #2: 0001-mouse-wheel-event-Minor-cleanups.patch --]
[-- Type: text/x-diff, Size: 4345 bytes --]

From ee2a8fd4cff84cd5bd672fdde8ec3e0800f132be Mon Sep 17 00:00:00 2001
From: Stefan Monnier <monnier@iro.umontreal.ca>
Date: Tue, 9 Jan 2024 11:34:05 -0500
Subject: [PATCH 1/7] (mouse-wheel-*-event): Minor cleanups

* lisp/mwheel.el (mwheel-event-button): Mark as obsolete alias.
Change all callers.

* lisp/edmacro.el (mouse-wheel-*-event): Move declarations to ...
(edmacro-fix-menu-commands): ... where we do know that they should
be defined.  Obey `mouse-wheel-*-alternate-event`s as well.
---
 lisp/edmacro.el | 20 +++++++++++++-------
 lisp/mwheel.el  | 18 ++++++------------
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/lisp/edmacro.el b/lisp/edmacro.el
index 362ec0ecbb4..5bd0c1892e5 100644
--- a/lisp/edmacro.el
+++ b/lisp/edmacro.el
@@ -720,17 +720,19 @@ edmacro-sanitize-for-string
            (setf (aref seq i) (logand (aref seq i) 127)))
   seq)
 
-;; These are needed in a --without-x build.
-(defvar mouse-wheel-down-event)
-(defvar mouse-wheel-up-event)
-(defvar mouse-wheel-right-event)
-(defvar mouse-wheel-left-event)
-
 (defun edmacro-fix-menu-commands (macro &optional noerror)
   (if (vectorp macro)
       (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)
+        (defvar mouse-wheel-down-alternate-event)
+        (defvar mouse-wheel-up-alternate-event)
+        (defvar mouse-wheel-right-alternate-event)
+        (defvar mouse-wheel-left-alternate-event)
 	;; Make a list of the elements.
 	(setq macro (append macro nil))
 	(dolist (ev macro)
@@ -748,7 +750,11 @@ edmacro-fix-menu-commands
 		     (memq (event-basic-type ev)
 			   (list mouse-wheel-down-event mouse-wheel-up-event
 				 mouse-wheel-right-event
-				 mouse-wheel-left-event)))
+				 mouse-wheel-left-event
+				 mouse-wheel-down-alternate-event
+				 mouse-wheel-up-alternate-event
+				 mouse-wheel-right-alternate-event
+				 mouse-wheel-left-alternate-event)))
 		 nil)
 		(noerror nil)
 		(t
diff --git a/lisp/mwheel.el b/lisp/mwheel.el
index b75b6f27d53..735adf42f68 100644
--- a/lisp/mwheel.el
+++ b/lisp/mwheel.el
@@ -216,15 +216,9 @@ mouse-wheel-flip-direction
   :type 'boolean
   :version "26.1")
 
-(defun mwheel-event-button (event)
-  (let ((x (event-basic-type event)))
-    ;; Map mouse-wheel events to appropriate buttons
-    (if (eq 'mouse-wheel x)
-        (let ((amount (car (cdr (cdr (cdr event))))))
-          (if (< amount 0)
-              mouse-wheel-up-event
-            mouse-wheel-down-event))
-      x)))
+;; This function used to handle the `mouse-wheel` event which was
+;; removed in 2003 by commit 9eb28007fb27, thus making it obsolete.
+(define-obsolete-function-alias 'mwheel-event-button #'event-basic-type "30.1")
 
 (defun mwheel-event-window (event)
   (posn-window (event-start event)))
@@ -347,7 +341,7 @@ mwheel-scroll
     (when (numberp amt) (setq amt (* amt (event-line-count event))))
     (condition-case nil
         (unwind-protect
-	    (let ((button (mwheel-event-button event)))
+	    (let ((button (event-basic-type event)))
               (cond ((and (eq amt 'hscroll) (memq button (list mouse-wheel-down-event
                                                                mouse-wheel-down-alternate-event)))
                      (when (and (natnump arg) (> arg 0))
@@ -434,7 +428,7 @@ mouse-wheel-text-scale
   (interactive (list last-input-event))
   (let ((selected-window (selected-window))
         (scroll-window (mouse-wheel--get-scroll-window event))
-        (button (mwheel-event-button event)))
+        (button (event-basic-type event)))
     (select-window scroll-window 'mark-for-redisplay)
     (unwind-protect
         (cond ((memq button (list mouse-wheel-down-event
@@ -450,7 +444,7 @@ mouse-wheel-global-text-scale
   "Increase or decrease the global font size according to the EVENT.
 This invokes `global-text-scale-adjust', which see."
   (interactive (list last-input-event))
-  (let ((button (mwheel-event-button event)))
+  (let ((button (event-basic-type event)))
     (cond ((memq button (list mouse-wheel-down-event
                               mouse-wheel-down-alternate-event))
            (global-text-scale-adjust 1))
-- 
2.39.2


[-- Attachment #3: 0002-mwheel-is-dir-p-New-macro-to-reduce-code-duplication.patch --]
[-- Type: text/x-diff, Size: 6505 bytes --]

From a764b503e126a60ff4ea1266da924de7b020637e Mon Sep 17 00:00:00 2001
From: Stefan Monnier <monnier@iro.umontreal.ca>
Date: Fri, 12 Jan 2024 17:50:09 -0500
Subject: [PATCH 2/7] (mwheel--is-dir-p): New macro to reduce code duplication

It also slightly reduces memory allocation.

* lisp/mwheel.el (mwheel--is-dir-p): New macro.
(mwheel-scroll, mouse-wheel-text-scale)
(mouse-wheel-global-text-scale): Use it.
---
 lisp/mwheel.el | 45 +++++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/lisp/mwheel.el b/lisp/mwheel.el
index 735adf42f68..84679f5c33f 100644
--- a/lisp/mwheel.el
+++ b/lisp/mwheel.el
@@ -305,6 +305,15 @@ mouse-wheel--get-scroll-window
                frame nil t)))))
       (mwheel-event-window event)))
 
+(defmacro mwheel--is-dir-p (dir button)
+  (declare (debug (sexp form)))
+  (let ((custom-var (intern (format "mouse-wheel-%s-event" dir)))
+        (custom-var-alt (intern (format "mouse-wheel-%s-alternate-event" dir))))
+    (macroexp-let2 nil butsym button
+      `(or (eq ,butsym ,custom-var)
+           ;; We presume here `button' is never nil.
+           (eq ,butsym ,custom-var-alt)))))
+
 (defun mwheel-scroll (event &optional arg)
   "Scroll up or down according to the EVENT.
 This should be bound only to mouse buttons 4, 5, 6, and 7 on
@@ -342,16 +351,14 @@ mwheel-scroll
     (condition-case nil
         (unwind-protect
 	    (let ((button (event-basic-type event)))
-              (cond ((and (eq amt 'hscroll) (memq button (list mouse-wheel-down-event
-                                                               mouse-wheel-down-alternate-event)))
+              (cond ((and (eq amt 'hscroll) (mwheel--is-dir-p down button))
                      (when (and (natnump arg) (> arg 0))
                        (setq mouse-wheel-scroll-amount-horizontal arg))
                      (funcall (if mouse-wheel-flip-direction
                                   mwheel-scroll-left-function
                                 mwheel-scroll-right-function)
                               mouse-wheel-scroll-amount-horizontal))
-                    ((memq button (list mouse-wheel-down-event
-                                        mouse-wheel-down-alternate-event))
+                    ((mwheel--is-dir-p down button)
                      (condition-case nil (funcall mwheel-scroll-down-function amt)
                        ;; Make sure we do indeed scroll to the beginning of
                        ;; the buffer.
@@ -366,31 +373,29 @@ mwheel-scroll
                           ;; for a reason that escapes me.  This problem seems
                           ;; to only affect scroll-down.  --Stef
                           (set-window-start (selected-window) (point-min))))))
-                    ((and (eq amt 'hscroll) (memq button (list mouse-wheel-up-event
-                                                               mouse-wheel-up-alternate-event)))
+                    ((and (eq amt 'hscroll) (mwheel--is-dir-p up button))
                      (when (and (natnump arg) (> arg 0))
                        (setq mouse-wheel-scroll-amount-horizontal arg))
                      (funcall (if mouse-wheel-flip-direction
                                   mwheel-scroll-right-function
                                 mwheel-scroll-left-function)
                               mouse-wheel-scroll-amount-horizontal))
-                    ((memq button (list mouse-wheel-up-event
-                                        mouse-wheel-up-alternate-event))
+                    ((mwheel--is-dir-p up button)
                      (condition-case nil (funcall mwheel-scroll-up-function amt)
                        ;; Make sure we do indeed scroll to the end of the buffer.
                        (end-of-buffer (while t (funcall mwheel-scroll-up-function)))))
-                    ((memq button (list mouse-wheel-left-event
-                                        mouse-wheel-left-alternate-event)) ; for tilt scroll
+                    ((mwheel--is-dir-p left button) ; for tilt scroll
                      (when mouse-wheel-tilt-scroll
                        (funcall (if mouse-wheel-flip-direction
                                     mwheel-scroll-right-function
-                                  mwheel-scroll-left-function) amt)))
-                    ((memq button (list mouse-wheel-right-event
-                                        mouse-wheel-right-alternate-event)) ; for tilt scroll
+                                  mwheel-scroll-left-function)
+                                amt)))
+                    ((mwheel--is-dir-p right button) ; for tilt scroll
                      (when mouse-wheel-tilt-scroll
                        (funcall (if mouse-wheel-flip-direction
                                     mwheel-scroll-left-function
-                                  mwheel-scroll-right-function) amt)))
+                                  mwheel-scroll-right-function)
+                                amt)))
 		    (t (error "Bad binding in mwheel-scroll"))))
           (if (eq scroll-window selected-window)
               ;; If there is a temporarily active region, deactivate it if
@@ -431,11 +436,9 @@ mouse-wheel-text-scale
         (button (event-basic-type event)))
     (select-window scroll-window 'mark-for-redisplay)
     (unwind-protect
-        (cond ((memq button (list mouse-wheel-down-event
-                                  mouse-wheel-down-alternate-event))
+        (cond ((mwheel--is-dir-p down button)
                (text-scale-increase 1))
-              ((memq button (list mouse-wheel-up-event
-                                  mouse-wheel-up-alternate-event))
+              ((mwheel--is-dir-p up button)
                (text-scale-decrease 1)))
       (select-window selected-window))))
 
@@ -445,11 +448,9 @@ mouse-wheel-global-text-scale
 This invokes `global-text-scale-adjust', which see."
   (interactive (list last-input-event))
   (let ((button (event-basic-type event)))
-    (cond ((memq button (list mouse-wheel-down-event
-                              mouse-wheel-down-alternate-event))
+    (cond ((mwheel--is-dir-p down button)
            (global-text-scale-adjust 1))
-          ((memq button (list mouse-wheel-up-event
-                              mouse-wheel-up-alternate-event))
+          ((mwheel--is-dir-p up button)
            (global-text-scale-adjust -1)))))
 
 (defun mouse-wheel--add-binding (key fun)
-- 
2.39.2


[-- Attachment #4: 0003-lisp-keymap.el-define-keymap-Demote-duplicate-def-to.patch --]
[-- Type: text/x-diff, Size: 1416 bytes --]

From 3bd8e963f7c8d762551c2528b2f907b613218a08 Mon Sep 17 00:00:00 2001
From: Stefan Monnier <monnier@iro.umontreal.ca>
Date: Fri, 12 Jan 2024 18:05:14 -0500
Subject: [PATCH 3/7] * lisp/keymap.el (define-keymap): Demote "duplicate def"
 to a warning

---
 lisp/keymap.el | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/lisp/keymap.el b/lisp/keymap.el
index 065c59da74c..d2544e30ce0 100644
--- a/lisp/keymap.el
+++ b/lisp/keymap.el
@@ -577,9 +577,15 @@ define-keymap
           (let ((def (pop definitions)))
             (if (eq key :menu)
                 (easy-menu-define nil keymap "" def)
-              (if (member key seen-keys)
-                  (error "Duplicate definition for key: %S %s" key keymap)
-                (push key seen-keys))
+              (when (member key seen-keys)
+                ;; Since the keys can be computed dynamically, it can
+                ;; very well happen that we get duplicate definitions
+                ;; due to some unfortunate configuration rather than
+                ;; due to an actual bug.  While such duplicates are
+                ;; not desirable, they shouldn't prevent the users
+                ;; from getting their job done.
+                (message "Duplicate definition for key: %S %s" key keymap))
+              (push key seen-keys)
               (keymap-set keymap key def)))))
       keymap)))
 
-- 
2.39.2


[-- Attachment #5: 0004-lisp-completion-preview.el-Fix-use-in-non-GUI-sessio.patch --]
[-- Type: text/x-diff, Size: 2368 bytes --]

From b9959e94d26bdd406d506ef355612627a0b1406b Mon Sep 17 00:00:00 2001
From: Stefan Monnier <monnier@iro.umontreal.ca>
Date: Fri, 12 Jan 2024 18:08:31 -0500
Subject: [PATCH 4/7] * lisp/completion-preview.el: Fix use in non-GUI session

Fix loading in non-GUI sessions where `mwheel` is not preloaded.
Not requiring `mwheel` would be a lot more complex, since it would
require delaying the construction of `completion-preview--mouse-map`.

* lisp/completion-preview.el (<toplevel>): Require `mwheel`.
Remove correspondingly redundant `defvar`s.
(completion-preview--mouse-map): Use `key-description` rather than mimicking
it with `format`.
---
 lisp/completion-preview.el | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/lisp/completion-preview.el b/lisp/completion-preview.el
index baadb4714b1..3bb5ef24e9d 100644
--- a/lisp/completion-preview.el
+++ b/lisp/completion-preview.el
@@ -52,6 +52,8 @@
 
 ;;; Code:
 
+(require 'mwheel)
+
 (defgroup completion-preview nil
   "In-buffer completion preview."
   :group 'completion)
@@ -128,19 +130,19 @@ completion-preview-active-mode-map
   ;; "M-p" #'completion-preview-prev-candidate
   )
 
-(defvar mouse-wheel-up-event)
-(defvar mouse-wheel-up-alternate-event)
-(defvar mouse-wheel-down-event)
-(defvar mouse-wheel-down-alternate-event)
 (defvar-keymap completion-preview--mouse-map
   :doc "Keymap for mouse clicks on the completion preview."
   "<down-mouse-1>" #'completion-preview-insert
   "C-<down-mouse-1>" #'completion-at-point
   "<down-mouse-2>" #'completion-at-point
-  (format "<%s>" mouse-wheel-up-event)             #'completion-preview-prev-candidate
-  (format "<%s>" mouse-wheel-up-alternate-event)   #'completion-preview-prev-candidate
-  (format "<%s>" mouse-wheel-down-event)           #'completion-preview-next-candidate
-  (format "<%s>" mouse-wheel-down-alternate-event) #'completion-preview-next-candidate)
+  (key-description (vector mouse-wheel-up-event))
+  #'completion-preview-prev-candidate
+  (key-description (vector mouse-wheel-up-alternate-event))
+  #'completion-preview-prev-candidate
+  (key-description (vector mouse-wheel-down-event))
+  #'completion-preview-next-candidate
+  (key-description (vector mouse-wheel-down-alternate-event))
+  #'completion-preview-next-candidate)
 
 (defvar-local completion-preview--overlay nil)
 
-- 
2.39.2


[-- Attachment #6: 0005-mwheel.el-Unconditionally-use-the-wheel-up-down-.-ev.patch --]
[-- Type: text/x-diff, Size: 5401 bytes --]

From b0f04ce4d3495de8593e3b64ddd218cd3bad221c Mon Sep 17 00:00:00 2001
From: Stefan Monnier <monnier@iro.umontreal.ca>
Date: Fri, 12 Jan 2024 18:28:12 -0500
Subject: [PATCH 5/7] mwheel.el: Unconditionally use the `wheel-up/down/...`
 events

The `mouse-wheel-DIR-event` vars were introduced because under X11
we get different `mouse-N` events depending on the users' mouse and
those same events can be used for other things for other rodents, so we
can't unconditionally treat those events as mouse-wheel events.

But this does not apply to the `wheel-up/down/...` events.
So hard code them.

* lisp/mwheel.el (mwheel--is-dir-p): Always consider the `wheel-DIR` events.
(mouse-wheel--setup-bindings): Always bind the `wheel-DIR` events.

* lisp/completion-preview.el (completion-preview--mouse-map):
Unconditionally bind the `wheel-DIR` events.
* lisp/edmacro.el (edmacro-fix-menu-commands): Hard code the
`wheel-DIR` events as mouse events regardless of `mouse-wheel-*-event`s.
* lisp/progmodes/flymake.el (flymake--mode-line-counter-map):
Do nothing, because it's already been done in commit e5be6c7ae309.
---
 lisp/completion-preview.el |  2 ++
 lisp/edmacro.el            | 15 ++++++++-------
 lisp/mwheel.el             | 15 ++++++++++-----
 3 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/lisp/completion-preview.el b/lisp/completion-preview.el
index 3bb5ef24e9d..48b6a4fd822 100644
--- a/lisp/completion-preview.el
+++ b/lisp/completion-preview.el
@@ -135,6 +135,8 @@ completion-preview--mouse-map
   "<down-mouse-1>" #'completion-preview-insert
   "C-<down-mouse-1>" #'completion-at-point
   "<down-mouse-2>" #'completion-at-point
+  "<wheel-up>"     #'completion-preview-prev-candidate
+  "<wheel-down>"   #'completion-preview-next-candidate
   (key-description (vector mouse-wheel-up-event))
   #'completion-preview-prev-candidate
   (key-description (vector mouse-wheel-up-alternate-event))
diff --git a/lisp/edmacro.el b/lisp/edmacro.el
index 5bd0c1892e5..9ade554f559 100644
--- a/lisp/edmacro.el
+++ b/lisp/edmacro.el
@@ -748,13 +748,14 @@ 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)
-			   (list mouse-wheel-down-event mouse-wheel-up-event
-				 mouse-wheel-right-event
-				 mouse-wheel-left-event
-				 mouse-wheel-down-alternate-event
-				 mouse-wheel-up-alternate-event
-				 mouse-wheel-right-alternate-event
-				 mouse-wheel-left-alternate-event)))
+			   `( ,mouse-wheel-down-event ,mouse-wheel-up-event
+			      ,mouse-wheel-right-event
+			      ,mouse-wheel-left-event
+			      ,mouse-wheel-down-alternate-event
+			      ,mouse-wheel-up-alternate-event
+			      ,mouse-wheel-right-alternate-event
+			      ,mouse-wheel-left-alternate-event
+			      wheel-down wheel-up wheel-left wheel-right)))
 		 nil)
 		(noerror nil)
 		(t
diff --git a/lisp/mwheel.el b/lisp/mwheel.el
index 84679f5c33f..f50376c72b5 100644
--- a/lisp/mwheel.el
+++ b/lisp/mwheel.el
@@ -308,9 +308,11 @@ mouse-wheel--get-scroll-window
 (defmacro mwheel--is-dir-p (dir button)
   (declare (debug (sexp form)))
   (let ((custom-var (intern (format "mouse-wheel-%s-event" dir)))
-        (custom-var-alt (intern (format "mouse-wheel-%s-alternate-event" dir))))
+        (custom-var-alt (intern (format "mouse-wheel-%s-alternate-event" dir)))
+        (event (intern (format "wheel-%s" dir))))
     (macroexp-let2 nil butsym button
-      `(or (eq ,butsym ,custom-var)
+      `(or (eq ,butsym ',event)
+           (eq ,butsym ,custom-var)
            ;; We presume here `button' is never nil.
            (eq ,butsym ,custom-var-alt)))))
 
@@ -503,14 +505,16 @@ mouse-wheel--setup-bindings
      ((and (consp binding) (eq (cdr binding) 'text-scale))
       (dolist (event (list mouse-wheel-down-event mouse-wheel-up-event
                            mouse-wheel-down-alternate-event
-                           mouse-wheel-up-alternate-event))
+                           mouse-wheel-up-alternate-event
+                           'wheel-down 'wheel-up))
         (when event
           (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
                            mouse-wheel-down-alternate-event
-                           mouse-wheel-up-alternate-event))
+                           mouse-wheel-up-alternate-event
+                           'wheel-down 'wheel-up))
         (when event
           (mouse-wheel--add-binding `[,(append (car binding) (list event))]
                                     'mouse-wheel-global-text-scale))))
@@ -521,7 +525,8 @@ mouse-wheel--setup-bindings
                            mouse-wheel-down-alternate-event
                            mouse-wheel-up-alternate-event
                            mouse-wheel-left-alternate-event
-                           mouse-wheel-right-alternate-event))
+                           mouse-wheel-right-alternate-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))))))))
-- 
2.39.2


[-- Attachment #7: 0006-mwheel.el-Remove-mouse-wheel-alternate-event-vars.patch --]
[-- Type: text/x-diff, Size: 8649 bytes --]

From 8cb8b9736532fdd8f2fc734b08ed55c17b922806 Mon Sep 17 00:00:00 2001
From: Stefan Monnier <monnier@iro.umontreal.ca>
Date: Fri, 12 Jan 2024 18:52:37 -0500
Subject: [PATCH 6/7] mwheel.el: Remove `mouse-wheel-*-alternate-event` vars

Now that `wheel-DIR` events are hardcoded, we never need more than
one variable (which we actually never needed anyway, we could have
let `mouse-wheel-*-event` vars hold lists of events instead), so
remove the `mouse-wheel-*-alternate-event` vars by merging their
default value into that of the corresponding `mouse-wheel-*-event`.

* lisp/mwheel.el (mouse-wheel-down-event, mouse-wheel-up-event)
(mouse-wheel-left-event, mouse-wheel-right-event): Don't bother holding
`wheel-DIR` events since these are already handled anyway.
Hold the event that would have been held in
`mouse-wheel-DIR-alternate-event` instead.
(mouse-wheel-down-alternate-event, mouse-wheel-up-alternate-event)
(mouse-wheel-left-alternate-event, mouse-wheel-right-alternate-event):
Delete vars.
(mwheel--is-dir-p, mouse-wheel--setup-bindings):
* lisp/edmacro.el (edmacro-fix-menu-commands):
* lisp/completion-preview.el (completion-preview--mouse-map):
Don't use `mouse-wheel-up/down-alternate-event` any more.
* lisp/progmodes/flymake.el (flymake--mode-line-counter-map):
Do nothing, because it already ignored those vars.
---
 lisp/completion-preview.el |  4 --
 lisp/edmacro.el            | 11 +-----
 lisp/mwheel.el             | 76 +++++++++++---------------------------
 3 files changed, 22 insertions(+), 69 deletions(-)

diff --git a/lisp/completion-preview.el b/lisp/completion-preview.el
index 48b6a4fd822..f552db7aa8e 100644
--- a/lisp/completion-preview.el
+++ b/lisp/completion-preview.el
@@ -139,11 +139,7 @@ completion-preview--mouse-map
   "<wheel-down>"   #'completion-preview-next-candidate
   (key-description (vector mouse-wheel-up-event))
   #'completion-preview-prev-candidate
-  (key-description (vector mouse-wheel-up-alternate-event))
-  #'completion-preview-prev-candidate
   (key-description (vector mouse-wheel-down-event))
-  #'completion-preview-next-candidate
-  (key-description (vector mouse-wheel-down-alternate-event))
   #'completion-preview-next-candidate)
 
 (defvar-local completion-preview--overlay nil)
diff --git a/lisp/edmacro.el b/lisp/edmacro.el
index 9ade554f559..9d185d79142 100644
--- a/lisp/edmacro.el
+++ b/lisp/edmacro.el
@@ -729,10 +729,6 @@ edmacro-fix-menu-commands
         (defvar mouse-wheel-up-event)
         (defvar mouse-wheel-right-event)
         (defvar mouse-wheel-left-event)
-        (defvar mouse-wheel-down-alternate-event)
-        (defvar mouse-wheel-up-alternate-event)
-        (defvar mouse-wheel-right-alternate-event)
-        (defvar mouse-wheel-left-alternate-event)
 	;; Make a list of the elements.
 	(setq macro (append macro nil))
 	(dolist (ev macro)
@@ -749,12 +745,7 @@ edmacro-fix-menu-commands
 		((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
-			      ,mouse-wheel-down-alternate-event
-			      ,mouse-wheel-up-alternate-event
-			      ,mouse-wheel-right-alternate-event
-			      ,mouse-wheel-left-alternate-event
+			      ,mouse-wheel-right-event ,mouse-wheel-left-event
 			      wheel-down wheel-up wheel-left wheel-right)))
 		 nil)
 		(noerror nil)
diff --git a/lisp/mwheel.el b/lisp/mwheel.el
index f50376c72b5..438ca5f84d5 100644
--- a/lisp/mwheel.el
+++ b/lisp/mwheel.el
@@ -60,44 +60,28 @@ mouse-wheel-down-event
   (if (or (featurep 'w32-win) (featurep 'ns-win)
           (featurep 'haiku-win) (featurep 'pgtk-win)
           (featurep 'android-win))
-      'wheel-up
+      (if (featurep 'xinput2)
+          nil
+        (unless (featurep 'x)
+         'mouse-4))
     'mouse-4)
-  "Event used for scrolling down."
+  "Event used for scrolling down, beside `wheel-down', if any."
   :group 'mouse
   :type 'symbol
   :set 'mouse-wheel-change-button)
 
-(defcustom mouse-wheel-down-alternate-event
-  (if (featurep 'xinput2)
-      'wheel-up
-    (unless (featurep 'x)
-      'mouse-4))
-  "Alternative wheel down event to consider."
-  :group 'mouse
-  :type 'symbol
-  :version "29.1"
-  :set 'mouse-wheel-change-button)
-
 (defcustom mouse-wheel-up-event
   (if (or (featurep 'w32-win) (featurep 'ns-win)
           (featurep 'haiku-win) (featurep 'pgtk-win)
           (featurep 'android-win))
-      'wheel-down
+      (if (featurep 'xinput2)
+          nil
+        (unless (featurep 'x)
+          'mouse-5))
     'mouse-5)
-  "Event used for scrolling up."
-  :group 'mouse
-  :type 'symbol
-  :set 'mouse-wheel-change-button)
-
-(defcustom mouse-wheel-up-alternate-event
-  (if (featurep 'xinput2)
-      'wheel-down
-    (unless (featurep 'x)
-      'mouse-5))
-  "Alternative wheel up event to consider."
+  "Event used for scrolling up, beside `wheel-up', if any."
   :group 'mouse
   :type 'symbol
-  :version "29.1"
   :set 'mouse-wheel-change-button)
 
 (defcustom mouse-wheel-click-event 'mouse-2
@@ -252,31 +236,23 @@ mouse-wheel-left-event
   (if (or (featurep 'w32-win) (featurep 'ns-win)
           (featurep 'haiku-win) (featurep 'pgtk-win)
           (featurep 'android-win))
-      'wheel-left
+      (if (featurep 'xinput2)
+          nil
+        (unless (featurep 'x)
+          'mouse-6))
     'mouse-6)
-  "Event used for scrolling left.")
-
-(defvar mouse-wheel-left-alternate-event
-  (if (featurep 'xinput2)
-      'wheel-left
-    (unless (featurep 'x)
-      'mouse-6))
-  "Alternative wheel left event to consider.")
+  "Event used for scrolling left, beside `wheel-left', if any.")
 
 (defvar mouse-wheel-right-event
   (if (or (featurep 'w32-win) (featurep 'ns-win)
           (featurep 'haiku-win) (featurep 'pgtk-win)
           (featurep 'android-win))
-      'wheel-right
+      (if (featurep 'xinput2)
+          nil
+        (unless (featurep 'x)
+          'mouse-7))
     'mouse-7)
-  "Event used for scrolling right.")
-
-(defvar mouse-wheel-right-alternate-event
-  (if (featurep 'xinput2)
-      'wheel-right
-    (unless (featurep 'x)
-      'mouse-7))
-  "Alternative wheel right event to consider.")
+  "Event used for scrolling right, beside `wheel-right', if any.")
 
 (defun mouse-wheel--get-scroll-window (event)
   "Return window for mouse wheel event EVENT.
@@ -308,13 +284,11 @@ mouse-wheel--get-scroll-window
 (defmacro mwheel--is-dir-p (dir button)
   (declare (debug (sexp form)))
   (let ((custom-var (intern (format "mouse-wheel-%s-event" dir)))
-        (custom-var-alt (intern (format "mouse-wheel-%s-alternate-event" dir)))
         (event (intern (format "wheel-%s" dir))))
     (macroexp-let2 nil butsym button
       `(or (eq ,butsym ',event)
-           (eq ,butsym ,custom-var)
            ;; We presume here `button' is never nil.
-           (eq ,butsym ,custom-var-alt)))))
+           (eq ,butsym ,custom-var)))))
 
 (defun mwheel-scroll (event &optional arg)
   "Scroll up or down according to the EVENT.
@@ -504,16 +478,12 @@ mouse-wheel--setup-bindings
      ;; Bindings for changing font size.
      ((and (consp binding) (eq (cdr binding) 'text-scale))
       (dolist (event (list mouse-wheel-down-event mouse-wheel-up-event
-                           mouse-wheel-down-alternate-event
-                           mouse-wheel-up-alternate-event
                            'wheel-down 'wheel-up))
         (when event
           (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
-                           mouse-wheel-down-alternate-event
-                           mouse-wheel-up-alternate-event
                            'wheel-down 'wheel-up))
         (when event
           (mouse-wheel--add-binding `[,(append (car binding) (list event))]
@@ -522,10 +492,6 @@ mouse-wheel--setup-bindings
      (t
       (dolist (event (list mouse-wheel-down-event mouse-wheel-up-event
                            mouse-wheel-left-event mouse-wheel-right-event
-                           mouse-wheel-down-alternate-event
-                           mouse-wheel-up-alternate-event
-                           mouse-wheel-left-alternate-event
-                           mouse-wheel-right-alternate-event
                            'wheel-down 'wheel-up 'wheel-left 'wheel-right))
         (when event
           (dolist (key (mouse-wheel--create-scroll-keys binding event))
-- 
2.39.2


[-- Attachment #8: 0007-mwheel.el-Code-clean-to-reduce-duplication.patch --]
[-- Type: text/x-diff, Size: 5284 bytes --]

From f355557bb9e93f5d5e7c3fae17e4da398226efb4 Mon Sep 17 00:00:00 2001
From: Stefan Monnier <monnier@iro.umontreal.ca>
Date: Fri, 12 Jan 2024 19:05:24 -0500
Subject: [PATCH 7/7] mwheel.el: Code clean to reduce duplication

* lisp/mwheel.el (mouse-wheel-obey-old-style-wheel-buttons): New var,
extracted from `mouse-wheel-*-event` definitions.
(mouse-wheel-down-event, mouse-wheel-up-event)
(mouse-wheel-left-event, mouse-wheel-right-event): Use it.
---
 lisp/mwheel.el | 54 ++++++++++++++++++++------------------------------
 1 file changed, 21 insertions(+), 33 deletions(-)

diff --git a/lisp/mwheel.el b/lisp/mwheel.el
index 438ca5f84d5..fc1f8e8b6d6 100644
--- a/lisp/mwheel.el
+++ b/lisp/mwheel.el
@@ -56,33 +56,33 @@ mouse-wheel-change-button
              (bound-and-true-p mouse-wheel-mode))
     (mouse-wheel-mode 1)))
 
-(defcustom mouse-wheel-down-event
+(defvar mouse-wheel-obey-old-style-wheel-buttons
+  ;; FIXME: Yuck!
   (if (or (featurep 'w32-win) (featurep 'ns-win)
           (featurep 'haiku-win) (featurep 'pgtk-win)
           (featurep 'android-win))
       (if (featurep 'xinput2)
           nil
         (unless (featurep 'x)
-         'mouse-4))
-    'mouse-4)
+          t))
+    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 `xterm-mouse-mode' as well.")
+
+(defcustom mouse-wheel-down-event
+  (if mouse-wheel-obey-old-style-wheel-buttons 'mouse-4)
   "Event used for scrolling down, beside `wheel-down', if any."
   :group 'mouse
   :type 'symbol
-  :set 'mouse-wheel-change-button)
+  :set #'mouse-wheel-change-button)
 
 (defcustom mouse-wheel-up-event
-  (if (or (featurep 'w32-win) (featurep 'ns-win)
-          (featurep 'haiku-win) (featurep 'pgtk-win)
-          (featurep 'android-win))
-      (if (featurep 'xinput2)
-          nil
-        (unless (featurep 'x)
-          'mouse-5))
-    'mouse-5)
+  (if mouse-wheel-obey-old-style-wheel-buttons 'mouse-5)
   "Event used for scrolling up, beside `wheel-up', if any."
   :group 'mouse
   :type 'symbol
-  :set 'mouse-wheel-change-button)
+  :set #'mouse-wheel-change-button)
 
 (defcustom mouse-wheel-click-event 'mouse-2
   "Event that should be temporarily inhibited after mouse scrolling.
@@ -92,7 +92,7 @@ mouse-wheel-click-event
 set to the event sent when clicking on the mouse wheel button."
   :group 'mouse
   :type 'symbol
-  :set 'mouse-wheel-change-button)
+  :set #'mouse-wheel-change-button)
 
 (defcustom mouse-wheel-inhibit-click-time 0.35
   "Time in seconds to inhibit clicking on mouse wheel button after scroll."
@@ -149,7 +149,7 @@ mouse-wheel-scroll-amount
                     (const :tag "Scroll horizontally" :value hscroll)
                     (const :tag "Change buffer face size" :value text-scale)
                     (const :tag "Change global face size" :value global-text-scale)))))
-  :set 'mouse-wheel-change-button
+  :set #'mouse-wheel-change-button
   :version "28.1")
 
 (defcustom mouse-wheel-progressive-speed t
@@ -233,25 +233,11 @@ mwheel-scroll-right-function
   "Function that does the job of scrolling right.")
 
 (defvar mouse-wheel-left-event
-  (if (or (featurep 'w32-win) (featurep 'ns-win)
-          (featurep 'haiku-win) (featurep 'pgtk-win)
-          (featurep 'android-win))
-      (if (featurep 'xinput2)
-          nil
-        (unless (featurep 'x)
-          'mouse-6))
-    'mouse-6)
+  (if mouse-wheel-obey-old-style-wheel-buttons 'mouse-6)
   "Event used for scrolling left, beside `wheel-left', if any.")
 
 (defvar mouse-wheel-right-event
-  (if (or (featurep 'w32-win) (featurep 'ns-win)
-          (featurep 'haiku-win) (featurep 'pgtk-win)
-          (featurep 'android-win))
-      (if (featurep 'xinput2)
-          nil
-        (unless (featurep 'x)
-          'mouse-7))
-    'mouse-7)
+  (if mouse-wheel-obey-old-style-wheel-buttons 'mouse-7)
   "Event used for scrolling right, beside `wheel-right', if any.")
 
 (defun mouse-wheel--get-scroll-window (event)
@@ -335,7 +321,8 @@ mwheel-scroll
                                 mwheel-scroll-right-function)
                               mouse-wheel-scroll-amount-horizontal))
                     ((mwheel--is-dir-p down button)
-                     (condition-case nil (funcall mwheel-scroll-down-function amt)
+                     (condition-case nil
+                         (funcall mwheel-scroll-down-function amt)
                        ;; Make sure we do indeed scroll to the beginning of
                        ;; the buffer.
                        (beginning-of-buffer
@@ -359,7 +346,8 @@ mwheel-scroll
                     ((mwheel--is-dir-p up button)
                      (condition-case nil (funcall mwheel-scroll-up-function amt)
                        ;; Make sure we do indeed scroll to the end of the buffer.
-                       (end-of-buffer (while t (funcall mwheel-scroll-up-function)))))
+                       (end-of-buffer
+                        (while t (funcall mwheel-scroll-up-function)))))
                     ((mwheel--is-dir-p left button) ; for tilt scroll
                      (when mouse-wheel-tilt-scroll
                        (funcall (if mouse-wheel-flip-direction
-- 
2.39.2

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sat, 13 Jan 2024 01:17:33 -0600
> Cc: jm@pub.pink, 68213@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > You expect me to have an opinion by just reading the log messages?  Or
> > do "git diff" myself?  Or something else?
> 
> I've attached the patches below.

Thanks, I think these are fine to go on master.

bug#68213: 30.0.50; completion-preview-tests failure in --without-x build

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

> Thanks, I think these are fine to go on master.

Thanks, pushed (with a few tweaks), closing,


        Stefan