bug#50430: windmove bindings now override org-read-date

bug#50430: windmove bindings now override org-read-date

[Radon Rosborough]

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

Since commit b5416d971a720e1dfb9aedd8e7d23f24b1ea5325 of 2021-05-25
(Improve windmove-*-default-keybindings functions), windmove bindings are
established in a minor mode map. Since minor mode bindings take precedence
over minibuffer-local-map, this has the effect of overriding any bindings
that may be established for the minibuffer, which is often not what is
desired.

For example, in Org, the function `org-read-date' allows you to use
shift+arrows to select a date while in the minibuffer. This no longer works
in the most recent Git build of Emacs. To reproduce, simply run
`windmove-default-keybindings', (require 'org), and evaluate
(org-read-date).

In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.25,
cairo version 1.16.0)
 of 2021-09-05 built on wit
Repository revision: dd4be8ad3b0cbcd701a5d5f7221a7e4c2a69d6be
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Pop!_OS 21.04

Configured using:
 'configure --with-json --with-native-compilation --with-xwidgets
 --with-x'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES
NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF
TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM XWIDGETS GTK3 ZLIB

Important settings:
  value of $LC_MONETARY: en_US.UTF-8
  value of $LC_NUMERIC: en_US.UTF-8
  value of $LC_TIME: en_US.UTF-8
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  windmove-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml mml-sec epa derived epg rfc6068 epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map text-property-search seq byte-opt
gv bytecomp byte-compile cconv mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils mule-util cal-move parse-time
iso8601 windmove org ob ob-tangle ob-ref ob-lob ob-table ob-exp
org-macro org-footnote org-src ob-comint org-pcomplete pcomplete comint
ansi-color ring org-list org-faces org-entities time-date subr-x
noutline outline easy-mmode org-version ob-emacs-lisp ob-core ob-eval
org-table ol org-keys org-compat advice org-macs org-loaddefs
format-spec find-func cal-menu calendar cal-loaddefs cl-loaddefs cl-lib
iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer cl-generic
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 charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads
xwidget-internal dbusbind inotify lcms2 dynamic-setting
system-font-setting font-render-setting cairo move-toolbar gtk x-toolkit
x multi-tty make-network-process native-compile emacs)

Memory information:
((conses 16 123884 7960)
 (symbols 48 11399 0)
 (strings 32 38108 2316)
 (string-bytes 1 1442696)
 (vectors 16 20830)
 (vector-slots 8 400252 11467)
 (floats 8 79 109)
 (intervals 56 497 53)
 (buffers 992 12))

[-- Attachment #2: Type: text/html, Size: 4687 bytes --]

bug#50430: windmove bindings now override org-read-date

[Philip Kaludercic]

Radon Rosborough <radon.neon@gmail.com> writes:

> Since commit b5416d971a720e1dfb9aedd8e7d23f24b1ea5325 of 2021-05-25
> (Improve windmove-*-default-keybindings functions), windmove bindings are
> established in a minor mode map. Since minor mode bindings take precedence
> over minibuffer-local-map, this has the effect of overriding any bindings
> that may be established for the minibuffer, which is often not what is
> desired.
>
> For example, in Org, the function `org-read-date' allows you to use
> shift+arrows to select a date while in the minibuffer. This no longer works
> in the most recent Git build of Emacs. To reproduce, simply run
> `windmove-default-keybindings', (require 'org), and evaluate
> (org-read-date).

It is not a proper solution, but one can always change the default keys
that windmove uses (e.g. instead of shift+arrow use
super+arrow). Another alternative is to just manually bind the windmove
commands, as the minor mode has no functionality of its own.

The issue here is that a minor mode is being used, precisely to override
local keys. Otherwise you have the issue that you might switch to a
buffer using shift+right, but if that buffer overrides shift+left, the
intuitive reverse operation won't work.

Beyond just reverting the change, which I think is too radical, I am not
sure how this can be fixed.

-- 
	Philip Kaludercic

bug#50430: windmove bindings now override org-read-date

[Radon Rosborough]

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

> Another alternative is to just manually bind the windmove
> commands, as the minor mode has no functionality of its own.

Yes, this is what I did in my configuration.

> Beyond just reverting the change, which I think is too radical, I am not
> sure how this can be fixed.

It feels intuitively appropriate to me that minibuffer-local-map should
override non-buffer-local minor modes. Is that an option?

[-- Attachment #2: Type: text/html, Size: 526 bytes --]

bug#50430: windmove bindings now override org-read-date

[Juri Linkov]

>> To reproduce, simply run `windmove-default-keybindings', (require 'org),
>> and evaluate (org-read-date).
>
> It is not a proper solution, but one can always change the default keys
> that windmove uses (e.g. instead of shift+arrow use
> super+arrow). Another alternative is to just manually bind the windmove
> commands, as the minor mode has no functionality of its own.

Or to set a buffer-local value of windmove-mode to nil in minibuffer-setup-hook.

> The issue here is that a minor mode is being used, precisely to override
> local keys. Otherwise you have the issue that you might switch to a
> buffer using shift+right, but if that buffer overrides shift+left, the
> intuitive reverse operation won't work.

Indeed, the user might want to use shift+up to switch from the minibuffer
to the original buffer and shift+down to go back.  I do this all the time
(but I use super+arrow instead of shift+arrow).

bug#50430: [External] : bug#50430: windmove bindings now override org-read-date

[Drew Adams]

> It feels intuitively appropriate to me that
> minibuffer-local-map should override
> non-buffer-local minor modes. Is that an option?

Do you mean generally, or by default - or do you
mean that you would like such a _possibility_ as
an optional (opt-in) behavior?

The use of a global minor mode to override
minibuffer keymap bindings is 100% appropriate.
It's just as appropriate as a global minor mode
overriding any other major-mode bindings.

bug#50430: [External] : bug#50430: windmove bindings now override org-read-date

[Radon Rosborough]

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

> Do you mean generally, or by default

We don't have to change the existing behavior if there are concerns about
backwards compatibility. But it is a common situation where you want to
define bindings in a minor mode, but you don't want them to override
minibuffer bindings. This often comes up in completion packages such as
https://github.com/raxod502/selectrum, and there is no existing solution in
Emacs---all the options are more or less hacks. If there were a way to make
this happen, then it would be a great improvement, even if it weren't the
default.

I can indeed think of situations where you want to override a binding
globally, so it seems like it would be best if there were ways to declare
this behavior on a per-mode basis.

[-- Attachment #2: Type: text/html, Size: 896 bytes --]

bug#50430: [External] : bug#50430: windmove bindings now override org-read-date

[Drew Adams]

> a common situation where you want to define bindings
> in a minor mode, but you don't want them to override
> minibuffer bindings.

The global minor mode commands you bind could just
invoke a minibuffer key binding, if any, when there.

Or if you don't want the global minor mode to have
_any_ effect in the minibuffer then you could, in
effect, exclude minibuffers in some way.

But yes, those would be workarounds.

>I can indeed think of situations where you want to
> override a binding globally, so it seems like it
> would be best if there were ways to declare this
> behavior on a per-mode basis.

Sounds reasonable to me.  But is it only about key
bindings, or would you be asking (instead? also?)
for a general, easy way to exclude some buffers
from a global minor mode?  IOW, instead of it being
entirely global, have it be on (locally) everywhere
except in specified buffers?

bug#50430: [External] : bug#50430: windmove bindings now override org-read-date

[Radon Rosborough]

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

> The global minor mode commands you bind could just
> invoke a minibuffer key binding
How? What I'm trying to do is avoid overriding a minibuffer key binding
established by another package, which I don't control.

> if you don't want the global minor mode to have
> _any_ effect in the minibuffer

No, generally the desired behavior for these use cases is that the
keybinding will by default be active in the minibuffer, unless
minibuffer-local-map happens to override that key, in which case
minibuffer-local-map should be preferred.

> is it only about key bindings

That's the problem I face, yes. Per my above comment, just conditionally
disabling the minor mode doesn't actually solve the problem. Unless you
were to conditionally disable it by scanning minibuffer-local-map for a
specific set of keybindings, which feels somewhat horrifying.

More generally though, it would be really nice if there were a way to
specify the numeric priority of a keybinding. Currently, minor modes are
unconditionally preferred over major modes and minibuffer bindings, which
are unconditionally preferred over global bindings. Usually this is what
you want, but when it isn't, the workarounds feel pretty hacky.

[-- Attachment #2: Type: text/html, Size: 1420 bytes --]

bug#50430: [External] : bug#50430: windmove bindings now override org-read-date

[Philip Kaludercic]

Radon Rosborough <radon.neon@gmail.com> writes:

>> The global minor mode commands you bind could just
>> invoke a minibuffer key binding
> How? What I'm trying to do is avoid overriding a minibuffer key binding
> established by another package, which I don't control.

Might it make sense to add a windmove option to prevent windmove
commands from entering the minibuffer in general?

-- 
	Philip Kaludercic

bug#50430: windmove bindings now override org-read-date

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

> The issue here is that a minor mode is being used, precisely to override
> local keys. Otherwise you have the issue that you might switch to a
> buffer using shift+right, but if that buffer overrides shift+left, the
> intuitive reverse operation won't work.
>
> Beyond just reverting the change, which I think is too radical, I am not
> sure how this can be fixed.

(I'm going through old bug reports that unfortunately weren't resolved
at the time.)

I think the conclusion here was that we don't want to change how
windmove works, and I'm therefore closing this bug report.