bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

[Jimmy Yuen Ho Wong]

After typing C-x b or C-x C-f, the minor mode maps from the buffer that launched
the minibuffer will override the keymap given to `read-from-minibuffer`,
this seems to be a regression in emacs 28 master.

Reproduction:

1. Turn on `ido-mode`
2. Turn on some minor mode that rebinds `C-k` with `(define-key
some-minor-mode-map (kbd "C-k") 'some-func)`
3. C-x b
4. Select a buffer and type C-k

Expectation:

C-k should be bound to `ido-kill-buffer-at-head` and calls it, but
instead `some-func` is called.

Since C-k is bound in `ido-completion-map`, and it is given to
`read-from-minibuffer`, all the bindings in `ido-completion-map` should
take precedence.

Additional info:

After some debugging with `enable-recursive-minibuffers`, I typed
Shift-Meta-; to open a recursive eval buffer inside the
`ido-switch-buffer` buffer, `(cl-loop for buf in (buffer-list) if
(minibufferp buf) do (with-current-buffer buf (key-binding (kbd
"C-k"))))` reveals that indeed C-k isn't bound inside any minibuffer
buffers.


In GNU Emacs 28.0.50 (build 1, x86_64-apple-darwin20.4.0, NS appkit-2022.44 Version 11.3 (Build 20E232))
 of 2021-05-02 built on mg559.lan
Repository revision: f1adb6b77a948e4f2f3223940edb15e56c30466d
Repository branch: HEAD
Windowing system distributor 'Apple', version 10.3.2022
System Description:  macOS 11.3

Configured using:
 'configure --prefix=/opt/local --disable-silent-rules --without-dbus
 --without-gconf --without-libotf --without-m17n-flt --with-gmp
 --with-gnutls --with-json --with-xml2 --with-modules --infodir
 /opt/local/share/info/emacs --with-ns --with-lcms2 --without-harfbuzz
 --without-imagemagick --without-xaw3d --with-native-compilation
 --with-rsvg 'CFLAGS=-pipe -Os
 -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX11.3.sdk -arch
 x86_64' 'CPPFLAGS=-I/opt/local/include
 -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX11.3.sdk'
 'LDFLAGS=-L/opt/local/lib -Wl,-headerpad_max_install_names -Wl,-no_pie
 -Wl,-syslibroot,/Library/Developer/CommandLineTools/SDKs/MacOSX11.3.sdk
 -arch x86_64''

Configured features:
ACL GIF GLIB GMP GNUTLS JPEG JSON LCMS2 LIBXML2 MODULES NATIVE_COMP
NOTIFY KQUEUE NS PDUMPER PNG RSVG THREADS TIFF TOOLKIT_SCROLL_BARS XIM
ZLIB

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

Major mode: ELisp/l

Minor modes in effect:
  projectile-rails-global-mode: t
  projectile-mode: t
  eldoc-box-hover-at-point-mode: t
  company-box-mode: t
  company-prescient-mode: t
  prescient-persist-mode: t
  rainbow-mode: t
  elisp-def-mode: t
  display-line-numbers-mode: t
  subword-mode: t
  form-feed-mode: t
  beginend-prog-mode: t
  imenu-list-minor-mode: t
  purpose-mode: t
  diff-hl-flydiff-mode: t
  crm-custom-mode: t
  override-global-mode: t
  dap-tooltip-mode: t
  dap-ui-many-windows-mode: t
  dap-ui-controls-mode: t
  dap-ui-mode: t
  treemacs-filewatch-mode: t
  treemacs-follow-mode: t
  treemacs-git-mode: deferred
  treemacs-fringe-indicator-mode: t
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  shell-dirtrack-mode: t
  winner-mode: t
  which-key-mode: t
  which-function-mode: t
  smooth-scrolling-mode: t
  show-smartparens-global-mode: t
  show-smartparens-mode: t
  smartparens-global-mode: t
  smartparens-mode: t
  show-paren-mode: t
  server-mode: t
  savehist-mode: t
  save-place-mode: t
  rxt-global-mode: t
  rxt-mode: t
  recentf-mode: t
  minibuffer-depth-indicate-mode: t
  ido-vertical-mode: t
  ido-ubiquitous-mode: t
  global-whitespace-cleanup-mode: t
  whitespace-cleanup-mode: t
  global-tree-sitter-mode: t
  global-so-long-mode: t
  global-origami-mode: t
  origami-mode: t
  global-move-dup-mode: t
  move-dup-mode: t
  global-flycheck-mode: t
  global-diff-hl-show-hunk-mouse-mode: t
  diff-hl-show-hunk-mouse-mode: t
  global-diff-hl-mode: t
  diff-hl-mode: t
  global-company-mode: t
  company-mode: t
  global-auto-revert-mode: t
  flx-ido-mode: t
  ido-everywhere: t
  icomplete-vertical-mode: t
  icomplete-mode: t
  fido-mode: t
  editorconfig-mode: t
  desktop-save-mode: t
  delete-selection-mode: t
  dap-auto-configure-mode: t
  dap-mode: t
  yas-minor-mode: t
  auto-compile-on-save-mode: t
  auto-compile-mode: t
  amx-mode: t
  abridge-diff-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  window-divider-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  temp-buffer-resize-mode: t
  buffer-read-only: t
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t
  visual-line-mode: t
  transient-mark-mode: t

Load-path shadows:
/opt/local/share/emacs/site-lisp/cmake-mode hides /Users/jimmyyuenhowong/.emacs.d/elpa/cmake-mode-20210104.1831/cmake-mode
/Users/jimmyyuenhowong/.emacs.d/elpa/transient-20210426.2141/transient hides /Applications/MacPorts/Emacs.app/Contents/Resources/lisp/transient

Features:
(shadow sort mail-extr emacsbug sendmail visual-regexp-steroids
visual-regexp misearch multi-isearch tabify cl-print jka-compr helpful
trace edebug info-look elisp-refs two-column vc-mtn vc-hg vc-bzr vc-src
vc-sccs vc-svn vc-cvs vc-rcs diff-hl-dired all-the-icons-dired
dired-collapse dired-hacks-utils projectile-rails rake inflections
inf-ruby smartparens-ruby ruby-mode smie autoinsert projectile vc-git
eldoc-box company-box company-box-doc frame-local company-box-icons
dash-functional company-prescient prescient company-keywords
company-etags etags fileloop generator company-gtags
company-dabbrev-code company-dabbrev company-yasnippet company-capf
company-files company-cmake company-clang company-semantic
company-template company-bbdb rainbow-mode elisp-def ert debug backtrace
display-line-numbers cap-words superword subword smartparens-config
smartparens-javascript smartparens-markdown smartparens-text
smartparens-c form-feed editorconfig-core editorconfig-core-handle
editorconfig-fnmatch beginend window-purpose-x shut-up imenu-list
hideshow window-purpose window-purpose-fixes
window-purpose-prefix-overload window-purpose-switch
window-purpose-layout window-purpose-core window-purpose-configuration
eieio-compat window-purpose-utils windmove dired-hide-dotfiles
shrink-path magit-lfs forge-list forge-commands forge-semi
forge-bitbucket buck forge-gogs gogs forge-gitea gtea forge-gitlab glab
forge-github ghub-graphql treepy gsexp ghub let-alist gnutls
forge-notify forge-revnote forge-pullreq forge-issue forge-topic
bug-reference forge-post forge-repo forge forge-core forge-db closql
magit-submodule magit-obsolete magit-blame magit-stash magit-reflog
magit-bisect magit-push magit-pull magit-fetch magit-clone magit-remote
magit-commit magit-sequence magit-notes magit-worktree magit-tag
magit-merge magit-branch magit-reset magit-files magit-refs magit-status
magit magit-repos magit-apply magit-wip magit-log diff-hl-flydiff
ediff-merg ediff-mult ediff-wind ediff-diff ediff-help ediff-init
ediff-util dumb-jump popup rg rg-info-hack rg-menu rg-ibuffer rg-result
wgrep-rg wgrep rg-history rg-header ibuf-ext ibuffer ibuffer-loaddefs
grep lsp-metals view lsp-metals-treeview lsp-metals-protocol yard-mode
typescript-mode json-mode json-reformat json-snatcher js cc-mode
cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars
cc-defs reformatter cl-lib-highlight eterm-256color term disp-table
ehelp xterm-color native-complete emacsql-sqlite emacsql
emacsql-compiler url-http url-auth url-gw lsp-sourcekit lsp lsp-pyright
lsp-jedi tree-sitter-langs tree-sitter-langs-build tar-mode arc-mode
archive-mode tree-sitter-hl iedit iedit-lib mc-hide-unmatched-lines-mode
mc-mark-more mc-cycle-cursors multiple-cursors-core rect
icomplete-vertical crm-custom pager-default-keybindings pager
browse-kill-ring paradox paradox-menu paradox-commit-list
paradox-execute paradox-github paradox-core parse-time iso8601
use-package-bind-key bind-key use-package-delight osx-trash
exec-path-from-shell all-the-icons all-the-icons-faces data-material
data-weathericons data-octicons data-fileicons data-faicons
data-alltheicons spaceline-config spaceline-segments spaceline powerline
powerline-separators powerline-themes solarized-dark-theme
solarized-theme solarized solarized-faces delight quelpa-use-package
use-package-core quelpa lisp-mnt help-fns radix-tree dap-mouse dap-ui
lsp-treemacs lsp-treemacs-themes treemacs treemacs-header-line
treemacs-compatibility treemacs-mode treemacs-interface
treemacs-extensions treemacs-mouse-interface treemacs-tags
treemacs-persistence treemacs-filewatch-mode treemacs-follow-mode
treemacs-rendering treemacs-async treemacs-workspaces treemacs-dom
treemacs-visuals treemacs-fringe-indicator treemacs-scope pulse
treemacs-faces treemacs-icons treemacs-themes treemacs-core-utils
pfuture hl-line treemacs-logging treemacs-customization treemacs-macros
gdb-mi gud bui bui-list bui-info bui-entry bui-core bui-history
bui-button bui-utils lsp-lens magit-diff smerge-mode diff git-commit
log-edit message rfc822 mml mml-sec epa derived gnus-util rmail
rmail-loaddefs time-date mm-decode mm-bodies mm-encode mail-parse
rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr mailabbrev
mail-utils gmm-utils mailheader add-log magit-core magit-autorevert
magit-margin magit-transient magit-process with-editor shell pcomplete
magit-mode transient format-spec magit-git magit-section magit-utils crm
winner which-key which-func smooth-scrolling smartparens paren server
savehist saveplace pcre2el rxt re-builder recentf mb-depth
ido-vertical-mode ido-completing-read+ memoize cus-edit pp minibuf-eldef
help-at-pt whitespace-cleanup-mode whitespace tree-sitter
tree-sitter-load tree-sitter-cli tsc tsc-dyn tsc-dyn-get dired-aux dired
dired-loaddefs tsc-obsolete so-long origami origami-parsers move-dup
flycheck find-func diff-hl-show-hunk diff-hl-inline-popup diff-hl
log-view pcvs-util vc-dir vc vc-dispatcher diff-mode company autorevert
flx-ido advice flx ido icomplete editorconfig desktop frameset delsel
dap-mode dap-launch posframe dap-overlays lsp-mode lsp-protocol
yasnippet-snippets yasnippet xref project tree-widget wid-edit spinner
network-stream puny nsm rmc markdown-mode color thingatpt noutline
outline easy-mmode lv inline imenu ht filenotify f ewoc epg epg-config
dash compile text-property-search comint ansi-color ring auto-compile
packed amx s abridge-diff cus-load edmacro kmacro cl info package
browse-url url url-proxy url-privacy url-expand url-methods url-history
url-cookie url-domsuf url-util mailcap url-handlers url-parse
auth-source eieio eieio-core eieio-loaddefs password-cache json map
url-vars comp comp-cstr warnings subr-x rx cl-seq cl-macs cl-extra
help-mode seq byte-opt bytecomp byte-compile cconv gv pcase cl-loaddefs
cl-lib iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/ns-win ns-win ucs-normalize mule-util
term/common-win 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 kqueue cocoa ns lcms2
multi-tty make-network-process nativecomp emacs)

Memory information:
((conses 16 1144104 503139)
 (symbols 48 53507 0)
 (strings 32 202614 92506)
 (string-bytes 1 7453813)
 (vectors 16 153419)
 (vector-slots 8 3884557 589614)
 (floats 8 2540 3047)
 (intervals 56 8434 5150)
 (buffers 992 29))

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

[Gregory Heytings]

>
> After typing C-x b or C-x C-f, the minor mode maps from the buffer that 
> launched the minibuffer will override the keymap given to 
> `read-from-minibuffer`, this seems to be a regression in emacs 28 
> master.
>
> Reproduction:
>
> 1. Turn on `ido-mode`
> 2. Turn on some minor mode that rebinds `C-k` with `(define-key
> some-minor-mode-map (kbd "C-k") 'some-func)`
> 3. C-x b
> 4. Select a buffer and type C-k
>
> Expectation:
>
> C-k should be bound to `ido-kill-buffer-at-head` and calls it, but 
> instead `some-func` is called.
>
> Since C-k is bound in `ido-completion-map`, and it is given to 
> `read-from-minibuffer`, all the bindings in `ido-completion-map` should 
> take precedence.
>

Thanks for your bug report.  I'm unable to reproduce your recipe on recent 
versions of the trunk (today, a week ago).  I tried

(define-minor-mode test-mode "" :keymap (let ((map (make-sparse-keymap))) (define-key map (kbd "C-k") '(lambda () (interactive) (message "BAD !"))) map))

and C-k is indeed bound to 'ido-kill-buffer-at-head'.  I suspect this 
misbehavior is due to your configuration; if not, could you try to create 
a recipe starting with emacs -Q?

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

[Jimmy Yuen Ho Wong]

Sorry I missed a reproduction step. You have to turn on the minor mode for the minibuffers. In my config there actually is a global minor mode that turns the minor mode on even for the minibuffer.

In Emacs 27, turning the minor mode on inside the minibuffer would not override the key map given to read-from-minibuffer.

I was able to reproduce this on a pristine init.el with just these reproduction steps plus turning the minor mode on for all buffers with a global minor mode.

My emacs has native compilation configured if that helps.

> On 3 May 2021, at 4:15 pm, Gregory Heytings <gregory@heytings.org> wrote:
> 
> 
>> 
>> After typing C-x b or C-x C-f, the minor mode maps from the buffer that launched the minibuffer will override the keymap given to `read-from-minibuffer`, this seems to be a regression in emacs 28 master.
>> 
>> Reproduction:
>> 
>> 1. Turn on `ido-mode`
>> 2. Turn on some minor mode that rebinds `C-k` with `(define-key
>> some-minor-mode-map (kbd "C-k") 'some-func)`
>> 3. C-x b
>> 4. Select a buffer and type C-k
>> 
>> Expectation:
>> 
>> C-k should be bound to `ido-kill-buffer-at-head` and calls it, but instead `some-func` is called.
>> 
>> Since C-k is bound in `ido-completion-map`, and it is given to `read-from-minibuffer`, all the bindings in `ido-completion-map` should take precedence.
>> 
> 
> Thanks for your bug report.  I'm unable to reproduce your recipe on recent versions of the trunk (today, a week ago).  I tried
> 
> (define-minor-mode test-mode "" :keymap (let ((map (make-sparse-keymap))) (define-key map (kbd "C-k") '(lambda () (interactive) (message "BAD !"))) map))
> 
> and C-k is indeed bound to 'ido-kill-buffer-at-head'.  I suspect this misbehavior is due to your configuration; if not, could you try to create a recipe starting with emacs -Q?

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

[Gregory Heytings]

>
> Sorry I missed a reproduction step. You have to turn on the minor mode 
> for the minibuffers. In my config there actually is a global minor mode 
> that turns the minor mode on even for the minibuffer.
>
> In Emacs 27, turning the minor mode on inside the minibuffer would not 
> override the key map given to read-from-minibuffer.
>
> I was able to reproduce this on a pristine init.el with just these 
> reproduction steps plus turning the minor mode on for all buffers with a 
> global minor mode.
>

Thanks for your clarification.  Could your perhaps mention what that minor 
mode is?  Otherwise it's rather difficult to reproduce and debug your 
problem.

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

[Jimmy Yuen Ho Wong]

It's literally just a global minor mode defined for your test minor
mode, basically `(define-global-minor-mode test-global-mode test-mode
(lambda() t))`

Jimmy

On Tue, May 4, 2021 at 2:13 PM Gregory Heytings <gregory@heytings.org> wrote:
>
>
> >
> > Sorry I missed a reproduction step. You have to turn on the minor mode
> > for the minibuffers. In my config there actually is a global minor mode
> > that turns the minor mode on even for the minibuffer.
> >
> > In Emacs 27, turning the minor mode on inside the minibuffer would not
> > override the key map given to read-from-minibuffer.
> >
> > I was able to reproduce this on a pristine init.el with just these
> > reproduction steps plus turning the minor mode on for all buffers with a
> > global minor mode.
> >
>
> Thanks for your clarification.  Could your perhaps mention what that minor
> mode is?  Otherwise it's rather difficult to reproduce and debug your
> problem.

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

[Gregory Heytings]

>
> It's literally just a global minor mode defined for your test minor 
> mode, basically `(define-global-minor-mode test-global-mode test-mode 
> (lambda() t))`
>

In that case, you should disable (or rather, not enable) that minor mode 
for minibuffers, for example:

(define-global-minor-mode test-global-mode test-mode (lambda () (or (minibufferp) (test-mode 1))))

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

[Gregory Heytings]

>> It's literally just a global minor mode defined for your test minor 
>> mode, basically `(define-global-minor-mode test-global-mode test-mode 
>> (lambda() t))`
>
> In that case, you should disable (or rather, not enable) that minor mode 
> for minibuffers, for example:
>
> (define-global-minor-mode test-global-mode test-mode (lambda () (or 
> (minibufferp) (test-mode 1))))
>

(Note that this does not answer your question "In Emacs 27, turning the 
minor mode on inside the minibuffer would not override the key map given 
to read-from-minibuffer", with the above the effect is the same in Emacs 
27 and 28 AFAICS.  Without seeing the minor mode, I cannot help you more, 
I don't have a crystal ball.)

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

[Jimmy Yuen Ho Wong]

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

The minor mode is exactly the same as what you have in your previous email.
What actually happens in my regular setup is the key bindings in multiple
minor modes override the key map given to read-from-minibuffer. The problem
is I don't want to turn off those minor mode as they used to work in the
minibuffer in emacs 27 as intended, but not so in emacs 28.

One mode is smartparens and another is move-dup.

On Tue, 4 May 2021 at 3:29 pm Gregory Heytings <gregory@heytings.org> wrote:

>
> >> It's literally just a global minor mode defined for your test minor
> >> mode, basically `(define-global-minor-mode test-global-mode test-mode
> >> (lambda() t))`
> >
> > In that case, you should disable (or rather, not enable) that minor mode
> > for minibuffers, for example:
> >
> > (define-global-minor-mode test-global-mode test-mode (lambda () (or
> > (minibufferp) (test-mode 1))))
> >
>
> (Note that this does not answer your question "In Emacs 27, turning the
> minor mode on inside the minibuffer would not override the key map given
> to read-from-minibuffer", with the above the effect is the same in Emacs
> 27 and 28 AFAICS.  Without seeing the minor mode, I cannot help you more,
> I don't have a crystal ball.)
>
-- 
Jimmy Yuen Ho Wong

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

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

[Gregory Heytings]

>
> The minor mode is exactly the same as what you have in your previous 
> email. What actually happens in my regular setup is the key bindings in 
> multiple minor modes override the key map given to read-from-minibuffer. 
> The problem is I don't want to turn off those minor mode as they used to 
> work in the minibuffer in emacs 27 as intended, but not so in emacs 28.
>

The problem is that, with the test-mode in my previous mail, I cannot 
reproduce the issue.

>
> One mode is smartparens and another is move-dup.
>

But neither of these minor modes override C-k, which you mentioned in your 
original bug report?

I tried move-dup, and its four bindings do not override the corresponding 
bindings in the minibuffer, with or without ido-mode.

So I still cannot see the issue you see...

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

[Jimmy Yuen Ho Wong]

Sorry I'm really bad at giving instructions. Here's some additional information.

First of all, this is the entirety of the test init.el.

(defun bad ()
  (interactive)
  (message "BAD !"))

(define-minor-mode test-mode ""
  :keymap
  (let ((map (make-sparse-keymap)))
    (define-key map (kbd "C-k") 'bad)
    map))

(define-global-minor-mode test-global-mode test-mode (lambda () (test-mode 1)))

(test-global-mode 1)
(ido-mode 1)
(setf enable-recursive-minibuffers 1)

Once emacs has started, C-x b C-h b should show that C-k is bound to `bad`.

Second of all, in order to get the key map bindings to
read-from-minibuffer overridden on emacs 27, it requires quitting from
switch-to-buffer, and then C-x b C-h b again to see that C-k is bound
to `bad`. In other words, on emacs 27, one has to pop up the
minibuffer twice. On emacs 28, just opening the minibuffer the first
time will see the key bindings overridden. I guess this is why I say
this is a regression on emacs 28 but you said you couldn't reproduce
it. I think both versions are wrong, it's just on emacs 28, it's wrong
in a more obvious way. In any case, I'm not sure if the minor mode
maps should override the key bindings in the keymap given to
read-from-buffer.

Jimmy

On Tue, May 4, 2021 at 5:36 PM Gregory Heytings <gregory@heytings.org> wrote:
>
>
> >
> > The minor mode is exactly the same as what you have in your previous
> > email. What actually happens in my regular setup is the key bindings in
> > multiple minor modes override the key map given to read-from-minibuffer.
> > The problem is I don't want to turn off those minor mode as they used to
> > work in the minibuffer in emacs 27 as intended, but not so in emacs 28.
> >
>
> The problem is that, with the test-mode in my previous mail, I cannot
> reproduce the issue.
>
> >
> > One mode is smartparens and another is move-dup.
> >
>
> But neither of these minor modes override C-k, which you mentioned in your
> original bug report?
>
> I tried move-dup, and its four bindings do not override the corresponding
> bindings in the minibuffer, with or without ido-mode.
>
> So I still cannot see the issue you see...

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

[Gregory Heytings]

>
> Second of all, in order to get the key map bindings to 
> read-from-minibuffer overridden on emacs 27, it requires quitting from 
> switch-to-buffer, and then C-x b C-h b again to see that C-k is bound to 
> `bad`. In other words, on emacs 27, one has to pop up the minibuffer 
> twice. On emacs 28, just opening the minibuffer the first time will see 
> the key bindings overridden. I guess this is why I say this is a 
> regression on emacs 28 but you said you couldn't reproduce it. I think 
> both versions are wrong, it's just on emacs 28, it's wrong in a more 
> obvious way.
>

Okay, now I finally see what you mean (and I can reproduce it).  Indeed 
this is a recent change in the handling of minibuffers, the first time the 
minibuffer is entered is not anymore different from the other times.

>
> In any case, I'm not sure if the minor mode maps should override the key 
> bindings in the keymap given to read-from-buffer.
>

Why would they not?  The minibuffer behaves, in that respect, like any 
other buffer.  Note that they do so only when the minor mode is enabled in 
the minibuffer.  With the example init file, using

(define-global-minor-mode test-global-mode test-mode (lambda () (or (minibufferp) (test-mode 1))))

works as you want it with both Emacs 27 and 28.

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

[Jimmy Yuen Ho Wong]

>
> Why would they not?  The minibuffer behaves, in that respect, like any
> other buffer.  Note that they do so only when the minor mode is enabled in
> the minibuffer.
>

Because the minibuffer does not behave like any other buffers. Setting
`minor-mode-overriding-map-alist` has no effect, so I think either the
minibuffer really needs to behave like any other buffer, or
special-cased and documented the ways it differs from regular buffers.

I have this code in my init.el, and it doesn't seem to work.

(defun ido-resurrect-keybinding ()
  (pcase-dolist (`(,minor-mode . ,keymap)
                 (seq-filter
                  (lambda (entry) (symbol-value (car entry)))
                  minor-mode-map-alist))
    (cl-loop for buf in (buffer-list)
             if (and (minibufferp buf)
                     (with-current-buffer buf
                       (symbol-value minor-mode)))
             do
             (map-keymap
              (lambda (event def)
                (when-let (key (and (or (characterp event)
                                        (and (symbolp event)
                                             (not (eq event 'remap))
                                             (not (keymapp def))))
                                    (if (characterp event)
                                        (format "%c" event)
                                      (vector event))))
                  (when (lookup-key ido-completion-map key)
                    (let ((map (copy-keymap keymap)))
                      (define-key map key nil)
                      (with-current-buffer buf
                        (push (cons minor-mode map)
                              minor-mode-overriding-map-alist))))))
              keymap))))

(with-eval-after-load 'ido
  (add-hook 'ido-setup-hook 'ido-resurrect-keybinding))

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

[Eli Zaretskii]

> From: Jimmy Yuen Ho Wong <wyuenho@gmail.com>
> Date: Wed, 5 May 2021 10:10:23 +0100
> Cc: 48153@debbugs.gnu.org
> 
> > Why would they not?  The minibuffer behaves, in that respect, like any
> > other buffer.  Note that they do so only when the minor mode is enabled in
> > the minibuffer.
> 
> Because the minibuffer does not behave like any other buffers. Setting
> `minor-mode-overriding-map-alist` has no effect, so I think either the
> minibuffer really needs to behave like any other buffer, or
> special-cased and documented the ways it differs from regular buffers.

I admit that I've lost the context in this discussion.  I'm CC'ing
Stefan in the hope he could tell whether we do or don't have a problem
here; if Stefan is unable to do that, either, we will unfortunately
have to get back to the beginning and explain what kind of problems
the current behavior causes.  Because in general what Jimmy described
in the original report sounds the expected behavior to me.

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

[Gregory Heytings]

Okay, now ido enters into the loop again...

I added your code (as is) to the init file you gave upthread, and I see 
the exact same behavior in Emacs 27 and 28.  When the activation of 
test-mode is made conditional to (minibufferp), I again see the exact same 
behavior in Emacs 27 and 28.

And with both Emacs 27 and 28, minor-mode-overriding-map-alist is nil 
after ido-resurrect-keybinding.

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

[Gregory Heytings]

>>> Why would they not?  The minibuffer behaves, in that respect, like any 
>>> other buffer.  Note that they do so only when the minor mode is 
>>> enabled in the minibuffer.
>>
>> Because the minibuffer does not behave like any other buffers. Setting 
>> `minor-mode-overriding-map-alist` has no effect, so I think either the 
>> minibuffer really needs to behave like any other buffer, or 
>> special-cased and documented the ways it differs from regular buffers.
>
> I admit that I've lost the context in this discussion.  I'm CC'ing 
> Stefan in the hope he could tell whether we do or don't have a problem 
> here; if Stefan is unable to do that, either, we will unfortunately have 
> to get back to the beginning and explain what kind of problems the 
> current behavior causes.  Because in general what Jimmy described in the 
> original report sounds the expected behavior to me.
>

I admit I'm lost, too.  The description of the problem has changed several 
times, and what was described in the original report is the actual 
behavior.  Of course I may be missing something; I'm not fortunate enough 
to have a crystal ball, like Stefan ;-)

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

[Jimmy Yuen Ho Wong]

Ok let me summarize the issue here.

1. On emacs 27, minor mode keybindings will only override the key
bindings given to read-from-minibuffer after the second invocation of
the minibuffer, but doesn't on the first invocation.
2. On emacs 28, minor mode keybindings override the key bindings given
to read-from-minibuffer at all times.
3. But, `minor-mode-overriding-map-alist` does not override the
override in effect in the minibuffer like other buffers.

My expectation is when a keymap is explicitly given to
read-from-minibuffer, the key bindings in it should take precedence,
but it doesn't. As you can see from the snippet from my last email,
both ido-completion-map and test-mode bind to C-k, I expect the C-k
binding in ido-completion-map to take effect inside the minibuffer,
without being overridden by any minor modes in effect inside the
minibuffer. If this is not to be desired, I'd expect setting
minor-mode-overriding-map-alist or the usual key binding lookup search
algorithm to work inside the minibuffer.

Does it make sense?

Jimmy

On Wed, May 5, 2021 at 2:12 PM Gregory Heytings <gregory@heytings.org> wrote:
>
>
> >>> Why would they not?  The minibuffer behaves, in that respect, like any
> >>> other buffer.  Note that they do so only when the minor mode is
> >>> enabled in the minibuffer.
> >>
> >> Because the minibuffer does not behave like any other buffers. Setting
> >> `minor-mode-overriding-map-alist` has no effect, so I think either the
> >> minibuffer really needs to behave like any other buffer, or
> >> special-cased and documented the ways it differs from regular buffers.
> >
> > I admit that I've lost the context in this discussion.  I'm CC'ing
> > Stefan in the hope he could tell whether we do or don't have a problem
> > here; if Stefan is unable to do that, either, we will unfortunately have
> > to get back to the beginning and explain what kind of problems the
> > current behavior causes.  Because in general what Jimmy described in the
> > original report sounds the expected behavior to me.
> >
>
> I admit I'm lost, too.  The description of the problem has changed several
> times, and what was described in the original report is the actual
> behavior.  Of course I may be missing something; I'm not fortunate enough
> to have a crystal ball, like Stefan ;-)

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

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

> Sorry I missed a reproduction step. You have to turn on the minor mode for
> the minibuffers. In my config there actually is a global minor mode that
> turns the minor mode on even for the minibuffer.
>
> In Emacs 27, turning the minor mode on inside the minibuffer would not
> override the key map given to read-from-minibuffer.

It would have helped to provide a precise recipe to start with, but
indeed there's been a change in Emacs-28, in that the minibuffer now is
setup in its own major mode, which may have the side-effect of enabling the
buffer-local minor modes made global via `define-globalized-minor-mode`
in some cases where this didn't happen before.
[ I consider this change as a bug-fix.  ]

Note that `define-globalized-minor-mode` distinguishes between its MODE
argument and the TURN-ON function specifically so that the mode can be
enabled only in those buffers where it makes sense.

So if your globalized minor mode shouldn't be used in minibuffers, you
need to tweak its TURN-ON such that it tests `minibufferp` like in
Gregory's example.

> Because the minibuffer does not behave like any other buffers.  Setting
> `minor-mode-overriding-map-alist` has no effect, so I think either the

I can't think of any reason why that would be the case.  Do you have
a simple test case (your `ido-resurrect-keybinding` is too complex for
my little brain to understand what it's expected to do).


        Stefan

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

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

> 1. On emacs 27, minor mode keybindings will only override the key
> bindings given to read-from-minibuffer after the second invocation of
> the minibuffer, but doesn't on the first invocation.

Something like that, yes.
That's only for minor modes defined via `define-globalized-minor-mode`.
Not for global minor modes defined in the usual way via (define-minor-mode
... :global t ...).

This change makes for a more predictable behavior and is a side-effect
of the change described in etc/NEWS as:

    ** An active minibuffer now has major mode 'minibuffer-mode', not the
    erroneous 'minibuffer-inactive-mode' it formerly had.

More specifically, the difference is that in Emacs<28 sometimes those
globalized minor modes where not enabled (hence their keybindings are
not active).

> 2. On emacs 28, minor mode keybindings override the key bindings given
> to read-from-minibuffer at all times.

And this was also true in all previous versions of Emacs.

> 3. But, `minor-mode-overriding-map-alist` does not override the
> override in effect in the minibuffer like other buffers.

That's the part of your bug report I haven't understood yet.

> My expectation is when a keymap is explicitly given to
> read-from-minibuffer, the key bindings in it should take precedence,
> but it doesn't.

No, this keymap is the "local map" a.k.a "major mode map", so it has
lower precedence than minor keymaps.  Always had.

> As you can see from the snippet from my last email, both
> ido-completion-map and test-mode bind to C-k, I expect the C-k binding
> in ido-completion-map to take effect inside the minibuffer, without
> being overridden by any minor modes in effect inside the minibuffer.

This expectation is incorrect ;-)

> If this is not to be desired, I'd expect setting
> minor-mode-overriding-map-alist or the usual key binding lookup search
> algorithm to work inside the minibuffer.

This should be the case: minibuffers are treated exactly like all other
buffers by the keymap code, AFAIK.


        Stefan

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

[Jimmy Yuen Ho Wong]

Ok I think I've found a solution to my problem. There are a couple of
things that I didn't understand and had to attach a function to print
out which one is being called on 5 different hooks to find out the
invocation order.

This is call order I've found:

1. ido-setup-hook
2. minibuffer-mode-hook
3. minibuffer-setup-hook
4. minibuffer-exit-hook
5. minibuffer-inactive-mode-hook

Specifically, the override keymaps are cleared between step 1 and 2,
so if I loop thru the minibuffers in step 1 to assign to
minor-mode-override-map-alist inside them, they will be reset in step
2 I suppose. So if I needed to "unshadow" the shadowed keybindings
inside the minibuffer, I had to do these processing in
minibuffer-setup-hook because that is called every time the minibuffer
is entered.

Thanks for your help, you can close this issue now.

Jimmy

On Wed, May 5, 2021 at 3:18 PM Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> > 1. On emacs 27, minor mode keybindings will only override the key
> > bindings given to read-from-minibuffer after the second invocation of
> > the minibuffer, but doesn't on the first invocation.
>
> Something like that, yes.
> That's only for minor modes defined via `define-globalized-minor-mode`.
> Not for global minor modes defined in the usual way via (define-minor-mode
> ... :global t ...).
>
> This change makes for a more predictable behavior and is a side-effect
> of the change described in etc/NEWS as:
>
>     ** An active minibuffer now has major mode 'minibuffer-mode', not the
>     erroneous 'minibuffer-inactive-mode' it formerly had.
>
> More specifically, the difference is that in Emacs<28 sometimes those
> globalized minor modes where not enabled (hence their keybindings are
> not active).
>
> > 2. On emacs 28, minor mode keybindings override the key bindings given
> > to read-from-minibuffer at all times.
>
> And this was also true in all previous versions of Emacs.
>
> > 3. But, `minor-mode-overriding-map-alist` does not override the
> > override in effect in the minibuffer like other buffers.
>
> That's the part of your bug report I haven't understood yet.
>
> > My expectation is when a keymap is explicitly given to
> > read-from-minibuffer, the key bindings in it should take precedence,
> > but it doesn't.
>
> No, this keymap is the "local map" a.k.a "major mode map", so it has
> lower precedence than minor keymaps.  Always had.
>
> > As you can see from the snippet from my last email, both
> > ido-completion-map and test-mode bind to C-k, I expect the C-k binding
> > in ido-completion-map to take effect inside the minibuffer, without
> > being overridden by any minor modes in effect inside the minibuffer.
>
> This expectation is incorrect ;-)
>
> > If this is not to be desired, I'd expect setting
> > minor-mode-overriding-map-alist or the usual key binding lookup search
> > algorithm to work inside the minibuffer.
>
> This should be the case: minibuffers are treated exactly like all other
> buffers by the keymap code, AFAIK.
>
>
>         Stefan
>

bug#48153: 28.0.50; minor mode keymaps should not override keymap given to read-from-minibuffer

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

> Ok I think I've found a solution to my problem. There are a couple of
> things that I didn't understand and had to attach a function to print
> out which one is being called on 5 different hooks to find out the
> invocation order.
>
> This is call order I've found:
>
> 1. ido-setup-hook
> 2. minibuffer-mode-hook
> 3. minibuffer-setup-hook
> 4. minibuffer-exit-hook
> 5. minibuffer-inactive-mode-hook
>
> Specifically, the override keymaps are cleared between step 1 and 2,
> so if I loop thru the minibuffers in step 1 to assign to
> minor-mode-override-map-alist inside them, they will be reset in step
> 2 I suppose.

Indeed, `ido-setup-hook` is run after IDO has configured its own keymaps
and variables, but hasn't yet entered the minibuffer, so it's too early
to work on `minor-mode-override-map-alist`.

> So if I needed to "unshadow" the shadowed keybindings inside the
> minibuffer, I had to do these processing in minibuffer-setup-hook
> because that is called every time the minibuffer is entered.

That's right.

> Thanks for your help, you can close this issue now.

Closing, thanks,


        Stefan