Why are 'parent' keymaps copied? about helm HOT 8 CLOSED

Jonas Bernoulli
[email protected]
writes:

Why is e.g. helm-map created by copying minibuffer-local-map
instead of using the latter as a parent keymap?
This can maybe modified in future, but I have to be careful on this, as
helm keymap code is quite complex since I have introduced kind of local
map per source and commands.

I have heavily modified the vanilla minibuffer-local maps
and this therefor means I have to duplicate all my customization.
So what is exactly the problem with your settings?

Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997

from helm.

uuh uhh. I already created a patch. I will push it to my fork but delay sending a pull request.

I doubt that properly using set-keymap-parent would cause any problems though.

The problem is that copying a keymap instead of using it as a parent is almost always wrong. One doesn't even need to go crazy and heavily modify the builtin minibuffer maps. Currently if one makes a change in helm-map this change does not propagate to any of the other maps. The only option is to restart Emacs. (Just loading helm-config.el again does not work because defvar does not change a variable which is already bound (which in turn is a good thing))

from helm.

Hi Jonas,

Jonas Bernoulli
[email protected]
writes:

uuh uhh. I already created a patch. I will push it to my fork but delay sending a pull request.

I doubt that properly using set-keymap-parent would cause any problems though.

The problem is that copying a keymap instead of using it as a parent
is wrong in 99% of the time. One doesn't even need to go crazy and
heavily modify the builtin minibuffer maps. Currently if one makes a
change in helm-map this change does not propagate to any of the
other maps. The only option is to restart Emacs. (Just loading
helm-config.elagain does not work becausedefvar` does not change a
variable which is already bound (which in turn is a good thing))

We don't want bindings in helm-map overhide bindings in
helm--map, but the contrary:

What is in helm--map should overhide what is in helm-map.

e.g
In helm-map ==> C-p' is bound to 'Dothis' In helm-<local>-map ==>C-p' is bound to 'Dothat'

What I want when I use the source that use helm--map is `C-p'
bound to 'Dothat'.

With your patch, I think defining a key in the inherited keymap
helm--map that is already existing in parent helm-map doesn't
overhide it; It is not what is expected.

But maybe I am wrong, need to verify.

Thanks.

Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997

from helm.

set-keymap-parent does what you want not what you think it does:

From elisp > 22.5 Inheritance and Keymaps:

A keymap can inherit the bindings of another keymap, which we call the
"parent keymap". Such a keymap looks like this:

 (keymap ELEMENTS... . PARENT-KEYMAP)

The effect is that this keymap inherits all the bindings of
PARENT-KEYMAP, whatever they may be at the time a key is looked up, but
can add to them or override them with ELEMENTS.

If you change the bindings in PARENT-KEYMAP using define-key or
other key-binding functions, these changed bindings are visible in the
inheriting keymap, unless shadowed by the bindings made by ELEMENTS.
The converse is not true: if you use define-key to change bindings in
the inheriting keymap, these changes are recorded in ELEMENTS, but have
no effect on PARENT-KEYMAP.

from helm.

Jonas Bernoulli
[email protected]
writes:

set-keymap-parent does what you want not what you think it does:

From elisp > 22.5 Inheritance and Keymaps:

A keymap can inherit the bindings of another keymap, which we call the
"parent keymap". Such a keymap looks like this:

 (keymap ELEMENTS... . PARENT-KEYMAP)

The effect is that this keymap inherits all the bindings of
PARENT-KEYMAP, whatever they may be at the time a key is looked up, but
can add to them or override them with ELEMENTS.

If you change the bindings in PARENT-KEYMAP using define-key or
other key-binding functions, these changed bindings are visible in the
inheriting keymap, unless shadowed by the bindings made by ELEMENTS.
The converse is not true: if you use define-key to change bindings in
the inheriting keymap, these changes are recorded in ELEMENTS, but have
no effect on PARENT-KEYMAP.
Indeed, and that is the problem IMHO.
See in helm-read-pattern-maybe' how local keymaps are modified. See e.ghelm-for-files' and see how the keymap is changed when you
change source.

Test your changes with two similar keys e.g C-n': Assign a Value in helm-map toC-n' say helm-next-line' Assign a value in helm-find-files-map toC-n' that is different from
the one you gave in help-map' sayhelm-previous-line'.
Now when you use helm-find-files' what isC-n' doing?
*next-line or *-previous-line?

This is what I want to verify before merging your changes.

Thanks.

Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997

from helm.

Okay I will look into it.

Anyways if it turns out that the existing code does not work as expected when using parent keymaps then it should be fixed (instead of not using parents). I will do that if it turns out that this is the case.

from helm.

Jonas Bernoulli
[email protected]
writes:

Okay I will look into it.

Anyways if it turns out that the existing code does not work as
expected when using parent keymaps then it should be fixed (instead of
not using parents). I will do that if it turns out that this is the
case.
After a quick test, your patch seems to work as expected.
I am going to merge it, we keep an eye on it, anyway we can revert the
changes later if some problem occur.
Thanks.

Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997

from helm.

This has been fixed. Closing.

from helm.