bug#54074: 29.0.50; Feature request emacs keymap-set and minor-modes

bug#54074: 29.0.50; Feature request emacs keymap-set and minor-modes

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

Hi:

1)

Recently I have seen that many packages have a pattern where a global
minor mode adds a set of keybindings to another (like isearch or
minibuffer). The issue comes when they want to disable the mode, because
sometimes they need to iterate or loop over, and then emacs looses the
previous binding unless they also adds more code to remember it when set
and then bring it back when unset.

So far with nested keymaps this could be solved, but there is not any
clean way to do that; so my request is to extend the new keymap-set to
set keymap without a prefix in another...

The idea is more or less to enable the developers of packages to do
something like:

```
(defvar-keymap my-mode-isearch-map
     ...)

(define-minor-mode my-mode
    :global t
    (if my-mode
        (keymap-set isearch-mode-map <something> my-mode-isearch-map)
      (keymap-unset isearch-mode-map <something>)))
```

With that then could be trivial to implemented something like
use-packages :bind, so

(define-minor-mode my-mode
    :global t
    :keymap (my-mode-map
             :map isearch-mode-map my-mode-isearch-map))


or similar... I am wondering how nobody have requested this before, when
it is a very common issue I find in many packages around.

2)

Something similar happens to some variables that are modified in minor
modes and then should be restored... this forces sometimes to restart
emacs... (for example completion-styles in fido-mode or some hooks that
are always added and then need to be removed... all these is very
mechanic and increases code and sources of errors...).

The best approach for this one is to save the original value in an alist
and assign with a wrapper around setq...  and use a loop when disabling
the mode to restore the original values.... it is not perfect, but works
in many cases...

To restore the original value they check if the current vars's value is
the same that was set (to assert it was not changed latter).

This way if I try fido-mode and I don't like it, then not restart is
needed to assert that completion-styles is what I had before...

Maybe we could consider to add some api for these functionalities...

Does it makes sense??




In GNU Emacs 29.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.31, cairo version 1.17.4)
 of 2022-02-20 built on Ergus
Repository revision: e6f541f2383cf860c6a2d6c8d366c21a3b8de2d0
Repository branch: master
System Description: Arch Linux

Configured using:
 'configure --prefix=/home/ergo/.local/ --with-mailutils --with-json
 --with-x-toolkit=gtk3 --with-xft --with-wide-int --with-modules
 --with-cairo --with-harfbuzz --with-native-compilation --with-pgtk'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBSYSTEMD LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER
PGTK PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS
WEBP XIM GTK3 ZLIB

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

Major mode: Lisp Interaction

Minor modes in effect:
  electric-pair-mode: t
  global-auto-revert-mode: t
  xclip-mode: t
  flyspell-mode: t
  company-mode: t
  flycheck-mode: t
  diff-hl-margin-mode: t
  composable-mark-mode: t
  composable-mode: t
  repeat-mode: t
  xterm-mouse-mode: t
  my/consult-mode: t
  vertico-mouse-mode: t
  vertico-mode: t
  minibuffer-depth-indicate-mode: t
  winner-mode: t
  save-place-mode: t
  delete-selection-mode: t
  savehist-mode: t
  global-display-fill-column-indicator-mode: t
  display-fill-column-indicator-mode: t
  global-display-line-numbers-mode: t
  display-line-numbers-mode: t
  which-key-mode: t
  override-global-mode: t
  eldoc-mode: t
  show-paren-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  context-menu-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
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t

Load-path shadows:
~/gits/emacs_clones/composable/composable-mark hides /home/ergo/.config/emacs/elpa/composable-20201024.1458/composable-mark
~/gits/emacs_clones/composable/composable hides /home/ergo/.config/emacs/elpa/composable-20201024.1458/composable
/home/ergo/.config/emacs/elpa/transient-20220216.2303/transient hides /home/ergo/.local/share/emacs/29.0.50/lisp/transient

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media rmc puny
rfc822 mml mml-sec password-cache epa derived epg rfc6068 epg-config
gnus-util text-property-search time-date mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils vertico-directory
elec-pair autorevert filenotify xclip flyspell-correct flyspell ispell
company-semantic company-template company-capf company flycheck
ansi-color json map find-func dash pcase diff-hl-margin diff-hl-dired
advice dired-x dired dired-loaddefs diff-hl log-view pcvs-util vc-dir
ewoc vc vc-dispatcher diff-mode vertico-repeat cape comp comp-cstr
warnings term/tmux term/xterm xterm init composable composable-mark
repeat xt-mouse simple-16-theme edmacro kmacro vertico-mouse vertico
mb-depth winner ring saveplace delsel savehist
display-fill-column-indicator display-line-numbers diminish which-key
cl-extra help-mode use-package use-package-ensure use-package-delight
use-package-diminish use-package-bind-key bind-key easy-mmode
use-package-core disp-table info ede/auto eieio-base cl-seq eieio seq
subr-x byte-opt bytecomp byte-compile cconv eieio-core cl-macs gv
eieio-loaddefs cl-loaddefs cl-lib tex-site rx slime-autoloads early-init
iso-transl tooltip eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/pgtk-win pgtk-win term/common-win
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-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 emoji-zwj 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 keymap hashtable-print-readable backquote threads
dbusbind inotify dynamic-setting system-font-setting font-render-setting
cairo gtk pgtk lcms2 multi-tty make-network-process native-compile
emacs)

Memory information:
((conses 16 198940 54047)
 (symbols 48 15486 0)
 (strings 32 59909 31324)
 (string-bytes 1 2103818)
 (vectors 16 30201)
 (vector-slots 8 452967 284466)
 (floats 8 109 1121)
 (intervals 56 726 0)
 (buffers 992 11))

bug#54074: 29.0.50; Feature request emacs keymap-set and minor-modes

[Lars Ingebrigtsen]

Ergus <spacibba@aol.com> writes:

> Recently I have seen that many packages have a pattern where a global
> minor mode adds a set of keybindings to another (like isearch or
> minibuffer). The issue comes when they want to disable the mode, because
> sometimes they need to iterate or loop over, and then emacs looses the
> previous binding unless they also adds more code to remember it when set
> and then bring it back when unset.

I'm not sure I understand the use case.  Isn't the way to do this just
to have my-mode add itself to isearch-mode-hook, and then have my-mode
have a minor mode keymap with the prefix it uses?

> Something similar happens to some variables that are modified in minor
> modes and then should be restored... this forces sometimes to restart
> emacs... (for example completion-styles in fido-mode or some hooks that
> are always added and then need to be removed... all these is very
> mechanic and increases code and sources of errors...).
>
> The best approach for this one is to save the original value in an alist
> and assign with a wrapper around setq...  and use a loop when disabling
> the mode to restore the original values.... it is not perfect, but works
> in many cases...

I don't think there's any general mechanism that's possible for this.
Any package may mogrify any variable (destructively or not), and only
the package can know what it has to do to bring things back to normal
when the package is switched off.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#54074: 29.0.50; Feature request emacs keymap-set and minor-modes

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

On February 20, 2022 8:39:16 PM GMT+01:00, Lars Ingebrigtsen <larsi@gnus.org> wrote:
>Ergus <spacibba@aol.com> writes:
>
>> Recently I have seen that many packages have a pattern where a global
>> minor mode adds a set of keybindings to another (like isearch or
>> minibuffer). The issue comes when they want to disable the mode, because
>> sometimes they need to iterate or loop over, and then emacs looses the
>> previous binding unless they also adds more code to remember it when set
>> and then bring it back when unset.
>
>I'm not sure I understand the use case.  Isn't the way to do this just
>to have my-mode add itself to isearch-mode-hook, and then have my-mode
>have a minor mode keymap with the prefix it uses?
>

Actually the mode does not need to be only for isearch-mode... It can be a global mode that adds multiple other stuff + some extra search commands... It only needs to be enabled once... And in general there is not any reason to force it's keys to add an extra prefix...

Adding 1 command to isearch-mode-map is fine... Suppose we need to add 5... Then it is easier to have a map and add it inline to isearch-mode-map...




>> Something similar happens to some variables that are modified in minor
>> modes and then should be restored... this forces sometimes to restart
>> emacs... (for example completion-styles in fido-mode or some hooks that
>> are always added and then need to be removed... all these is very
>> mechanic and increases code and sources of errors...).
>>
>> The best approach for this one is to save the original value in an alist
>> and assign with a wrapper around setq...  and use a loop when disabling
>> the mode to restore the original values.... it is not perfect, but works
>> in many cases...
>
>I don't think there's any general mechanism that's possible for this.
>Any package may mogrify any variable (destructively or not), and only
>the package can know what it has to do to bring things back to normal
>when the package is switched off.
>

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

bug#54074: 29.0.50; Feature request emacs keymap-set and minor-modes

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

On Sun, Feb 20, 2022 at 08:39:16PM +0100, Lars Ingebrigtsen wrote:
>Ergus <spacibba@aol.com> writes:
>
>> Recently I have seen that many packages have a pattern where a global
>> minor mode adds a set of keybindings to another (like isearch or
>> minibuffer). The issue comes when they want to disable the mode, because
>> sometimes they need to iterate or loop over, and then emacs looses the
>> previous binding unless they also adds more code to remember it when set
>> and then bring it back when unset.

Hi Lars.. thanks for replying!

>
>I'm not sure I understand the use case.  Isn't the way to do this just
>to have my-mode add itself to isearch-mode-hook, and then have my-mode
>have a minor mode keymap with the prefix it uses?
>
Yes this is actually the common way to do it, but it is cumbersome with
no reason for simpler cases... and when more than one keymap can be
affected.

Basically I mean to have a way to add an entry: (keymap …) to another...

Because from the manual:

"If an element of a keymap is itself a keymap, it counts as if this inner
keymap were inlined in the outer keymap."

So, simpler cleaner and eventually not needing execute or start a mode
every time in a hook...

Now keymap-set allows to add a command or keymap to a prefix in another,
but not just append a keymap as is with no prefix and then remove it
completely in a clean way...


>> Something similar happens to some variables that are modified in minor
>> modes and then should be restored... this forces sometimes to restart
>> emacs... (for example completion-styles in fido-mode or some hooks that
>> are always added and then need to be removed... all these is very
>> mechanic and increases code and sources of errors...).
>>
>> The best approach for this one is to save the original value in an alist
>> and assign with a wrapper around setq...  and use a loop when disabling
>> the mode to restore the original values.... it is not perfect, but works
>> in many cases...
>
>I don't think there's any general mechanism that's possible for this.
>Any package may mogrify any variable (destructively or not), and only
>the package can know what it has to do to bring things back to normal
>when the package is switched off.
>

The common is just to save and the restore... isn't it the typical idea
behind a mode? Right now the modes cleanup is a source of issues and
code replication here and there for many mechanical thinks like
add-hook/remove-hook, set variable, unset them, and so on... when the
general idea is that all that may follow the same philosophy than
keybindings set-unset...

Best,
Ergus

bug#54074: [External] : bug#54074: 29.0.50; Feature request emacs keymap-set and minor-modes

[Drew Adams]

FWIW, in Icicles I save the bindings before
`icicle-mode' (minor mode) is turned on, and
restore them when it is later turned off.

(I do the same for some functions I redefine.
The same can be done with adding and removing
advice.)

However, bindings that something or someone
makes in the interim are of course not taken
into consideration.  That is, what's restored
is the state before `icicle-mode' was turned on.

I don't think there's a great general solution.
No way to know what something or someone really
might want, for example.

bug#54074: 29.0.50; Feature request emacs keymap-set and minor-modes

[Juri Linkov]

> 1)
>
> With that then could be trivial to implemented something like
> use-packages :bind, so
>
> (define-minor-mode my-mode
>     :global t
>     :keymap (my-mode-map
>              :map isearch-mode-map my-mode-isearch-map))
>
>
> or similar... I am wondering how nobody have requested this before, when
> it is a very common issue I find in many packages around.

You might be interested in how the composite keymaps are defined here:

```
(define-minor-mode outline-minor-mode
  :keymap (easy-mmode-define-keymap
           `(([menu-bar] . ,outline-minor-mode-menu-bar-map)
             (,outline-minor-mode-prefix . ,outline-mode-prefix-map))
           :inherit outline-minor-mode-cycle-map)
```

Maybe such keymap definitions could be simplified.

> 2)
>
> The best approach for this one is to save the original value in an alist
> and assign with a wrapper around setq...  and use a loop when disabling
> the mode to restore the original values.... it is not perfect, but works
> in many cases...
> [...]
> Does it makes sense??

Yes, this makes sense.  We used the functions copied below
until you proposed to move keybindings to context-menu-mode-map:

```
(defvar context-menu--saved-bindings nil
  "Alist of bindings to restore when `context-menu-mode' is disabled.")

(defun context-menu--bind-mouse (click-sym down-sym)
  "Enable `context-menu-mode' mouse bindings.
CLICK-SYM and DOWN-SYM are the mouse click and down key symbols to use."
  (let ((click (vector click-sym))
        (down (vector down-sym)))
    (push (cons click-sym (global-key-binding click))
          context-menu--saved-bindings)
    (global-unset-key click)
    (push (cons down-sym (global-key-binding down))
          context-menu--saved-bindings)
    (global-set-key down context-menu-entry)))

(defun context-menu--restore-bindings ()
  "Restore saved `context-menu-mode' bindings."
  (pcase-dolist (`(,sym . ,binding) context-menu--saved-bindings)
    (let ((key (vector sym)))
      (if binding
          (global-set-key key binding)
        (global-unset-key key)))))
```

bug#54074: [External] : bug#54074: 29.0.50; Feature request emacs keymap-set and minor-modes

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

> However, bindings that something or someone
> makes in the interim are of course not taken
> into consideration.

That's the problem with the naive approach, indeed (in addition to the
need to write the code).

> I don't think there's a great general solution.  No way to know what
> something or someone really might want, for example.

In his example, the minor mode just adds the keymap and then removes it,
so it doesn't interact unduly with other changes that might take place
in the mean time.


        Stefan

bug#54074: 29.0.50; Feature request emacs keymap-set and minor-modes

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

>> Recently I have seen that many packages have a pattern where a global
>> minor mode adds a set of keybindings to another (like isearch or
>> minibuffer). The issue comes when they want to disable the mode, because
>> sometimes they need to iterate or loop over, and then emacs looses the
>> previous binding unless they also adds more code to remember it when set
>> and then bring it back when unset.
> I'm not sure I understand the use case.  Isn't the way to do this just
> to have my-mode add itself to isearch-mode-hook, and then have my-mode
> have a minor mode keymap with the prefix it uses?

I think the use case is basically any minor mode which currently uses
`define-key` in its body to modify some other package's keymap.

> I don't think there's any general mechanism that's possible for this.
> Any package may mogrify any variable (destructively or not), and only
> the package can know what it has to do to bring things back to normal
> when the package is switched off.

I think we should provide an API to do that meaningfully.  I.e. instead
of having a minor mode just bluntly set a variable, associate with each
global var a pair (VAL . FUNS) of a base value and a list of functions,
where the effective value (the one that should be stored in the
`symbol-value`, barring intervention from code that disregards the new
API) is (FUN1 (FUN2 (... VAL))).

The minor mode can then add/remove a function from the list, and just as
is the case with `add-hook`, `add-function`, etc... it will correctly
handle the case where the additions/removals are not nested.


        Stefan

bug#54074: 29.0.50; Feature request emacs keymap-set and minor-modes

[Lars Ingebrigtsen]

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

>> I'm not sure I understand the use case.  Isn't the way to do this just
>> to have my-mode add itself to isearch-mode-hook, and then have my-mode
>> have a minor mode keymap with the prefix it uses?
>
> I think the use case is basically any minor mode which currently uses
> `define-key` in its body to modify some other package's keymap.

But why do that instead of using a minor mode?  I think many cases where
packages do this, it's because the idea didn't really occur to the
people that did the implementation.

> I think we should provide an API to do that meaningfully.  I.e. instead
> of having a minor mode just bluntly set a variable, associate with each
> global var a pair (VAL . FUNS) of a base value and a list of functions,
> where the effective value (the one that should be stored in the
> `symbol-value`, barring intervention from code that disregards the new
> API) is (FUN1 (FUN2 (... VAL))).

As Eli noted, isn't this kinda `add-variable-watcher'?  But:

> The minor mode can then add/remove a function from the list, and just as
> is the case with `add-hook`, `add-function`, etc... it will correctly
> handle the case where the additions/removals are not nested.

My feeling is that this would be both too complicated for most people
writing modes, and it would be really confusing for us people that have
to try to figure out what's actually going on, and finally, values in
Emacs Lisp are mutable, so it won't work reliably anyway.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#54074: 29.0.50; Feature request emacs keymap-set and minor-modes

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

Lars Ingebrigtsen [2022-02-22 14:22:49] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> I'm not sure I understand the use case.  Isn't the way to do this just
>>> to have my-mode add itself to isearch-mode-hook, and then have my-mode
>>> have a minor mode keymap with the prefix it uses?
>> I think the use case is basically any minor mode which currently uses
>> `define-key` in its body to modify some other package's keymap.
> But why do that instead of using a minor mode?

[ I assume you meant "using a minor mode MAP".  ]

E.g. because they don't want the priority that comes with minor mode maps?

> I think many cases where packages do this, it's because the idea
> didn't really occur to the people that did the implementation.

Many, maybe, but definitely not all.

BTW, we currently have related functions to add/remove a keymap from
another: `internal-push/pop-keymap` used by `set-transient-map`.

>> The minor mode can then add/remove a function from the list, and just as
>> is the case with `add-hook`, `add-function`, etc... it will correctly
>> handle the case where the additions/removals are not nested.
> My feeling is that this would be both too complicated for most people
> writing modes, and it would be really confusing for us people that have
> to try to figure out what's actually going on, and finally, values in
> Emacs Lisp are mutable, so it won't work reliably anyway.

W.r.t complexity, it's a worry, indeed.  Until someone implements it and
tries to provide convenient ways to use it (e.g. for minor modes), it's
hard to tell.

W.r.t it being unreliable, it shouldn't be any less reliable than what we
have today, and more importantly any misbehavior could then be fixed by
making the relevant code(s) use the new API, whereas currently there's
basically no way to fix those problems (short of ad-hoc hacks).


        Stefan

bug#54074: 29.0.50; Feature request emacs keymap-set and minor-modes

[Lars Ingebrigtsen]

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

> [ I assume you meant "using a minor mode MAP".  ]

Yes.

> E.g. because they don't want the priority that comes with minor mode maps?

In what way?

> BTW, we currently have related functions to add/remove a keymap from
> another: `internal-push/pop-keymap` used by `set-transient-map`.

Hm...  right.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#54074: 29.0.50; Feature request emacs keymap-set and minor-modes

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

Lars Ingebrigtsen [2022-02-22 15:31:45] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> [ I assume you meant "using a minor mode MAP".  ]
> Yes.
>> E.g. because they don't want the priority that comes with minor mode maps?
> In what way?

E.g. some (typically global) modes want to have lower keymap-priority
than the bindings from major and/or minor modes, so they basically
want to have a priority between the global map and the local map.
The easiest way to do that is to add their bindings to the global map.

>> BTW, we currently have related functions to add/remove a keymap from
>> another: `internal-push/pop-keymap` used by `set-transient-map`.
> Hm...  right.

Are you suggesting that I could occasionally be wrong?
What is this?  A mutiny?


        Stefan

bug#54074: 29.0.50; Feature request emacs keymap-set and minor-modes

[Lars Ingebrigtsen]

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

> E.g. some (typically global) modes want to have lower keymap-priority
> than the bindings from major and/or minor modes, so they basically
> want to have a priority between the global map and the local map.
> The easiest way to do that is to add their bindings to the global map.

Ah, yes, that seems to be a valid use case; hadn't thought of that.

>> Hm...  right.
>
> Are you suggesting that I could occasionally be wrong?
> What is this?  A mutiny?

I would never imply anything like that!

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no