evil-collection-translate-key doesn't seem to work with evil-define-minor-mode-key

[gagbo]

Hello,

(This may be related to #433 but really not sure)

Ever since this evil-snipe commit it seems that

(evil-collection-translate-key '(normal motion) '(evil-snipe-local-mode-map evil-snipe-override-local-mode-map)
  "s" "k"
  "S" "K"
  "K" "S"
  "k" "s")

doesn't work as intended : after this call is executed, I expect s to move the cursor up (evil-previous-line, I use an alternate keyboard layout), but instead it calls evil-snipe-s still. It used to work before this commit

Worse, when I try to C-h k s, I see evil-snipe-s, but no associated binding is showing (I use helpful.el, and the "Key bindings" section is empty

(Instead, I expected evil-previous-line, which is mapped to evil-motion-state-map s)

Digging a little bit, I noticed 2 things :

• the evil-snipe change means that now the initial binding is done to evil-get-minor-mode-keymap
  https://github.com/emacs-evil/evil/blob/f20d442ff006aa5a6dc48ac654906b48b95107fd/evil-core.el#L1154
  instead of evil-get-auxiliary-keymap
  https://github.com/emacs-evil/evil/blob/f20d442ff006aa5a6dc48ac654906b48b95107fd/evil-core.el#L1116

• And evil-collection--translate-key only fetches the lookup-map through evil-get-auxiliary-keymap
  

  evil-collection/evil-collection.el

  Lines 603 to 605 in a24168f

  	(if state
  	(evil-get-auxiliary-keymap keymap state t t)
  	keymap))))

So maybe evil-collection--translate-key needs to be modified to also be able to lookup in that particular map ? I don't really know if it's going to be the solution (nor how to do that properly).

I don't know if the get-minor-mode-keymap and get-auxiliary keymap functions of evil are actually fetching the same keymap or not; and I don't know how those keymaps affect the key resolution logic of Emacs, so sorry if this shot in the dark is totally wrong (and I'd be happy to learn a little bit more; seeing "No binding" in helpful.el after a C-h k is a troubling experience)

Thanks for creating that function in the first place though, it's really a time saver

[jojojames]

Hi @gagbo ,

Thank you for the amazing bug report! Sorry for the long overdue message. Thank to your bug report, I've looked into it and added a new function/macro to do the key swaps. It worked for me in my short testing.

Reproduced by going to this commit for evil-snipe:
1f3ba0d * master Use evil-define-minor-mode-key

Then used the below:

e.g. Either of these works for me:

  (evil-collection-swap-minor-mode-key '(normal motion)
    '(evil-snipe-local-mode evil-snipe-override-local-mode)
    "k" "s"
    ;; Set this to t to make this swap the keys everytime
    ;; this expression is evaluated.
    :destructive t)

  (evil-collection-translate-minor-mode-key
    '(normal motion)
    '(evil-snipe-local-mode evil-snipe-override-local-mode)
    "k" "s"
    "s" "k"
    ;; Set this to t to make this swap the keys everytime
    ;; this expression is evaluated.
    :destructive nil)

And key translation works as expected for me, please try and thanks again for the helpful bug report.

[jojojames]

Note to future:

evil-collection-translate-key will work with any keybindings defined by evil-define-key. That is because it reads from the auxillery-map (e.g. evil-get-auxiliary-keymap) that is created/set by evil-define-key. Generally this will work since most (not all unfortunately) of evil-collection binds its keys using evil-collection-define-key which routes to evil-define-key.

The mentioned commit here in evil-snipe changed to evil-define-minor-mode-key which means key translation won't work since the lookup keymap will be wrong. We should be able to translate any minor mode key then looking up the minor mode keymap using evil-get-minor-mode-keymap.