my-hjkl-rotation is not affecting magit.

[clonex10100]

I'm on commit 63a9dcb

my-hjkl-rotation is working fine in other modes but not in magit-mode.

[Ambrevar]

What is my-hjkl-rotation? Are you using evil-magit? If so, you should not use it in conjunction with a recent evil-collection which already includes the evil-magit bindings.

[clonex10100]

I'm not using evil-magit.

Sorry, my-hjkl-rotation wasn't very clear. I see now it's just a function name that was used in the documentation, but I don't really know what else to call it.

(defun my-hjkl-rotation (_mode mode-keymaps &rest _rest)
  (evil-collection-translate-key 'normal mode-keymaps
      "n" "h"
      "e" "j"
      "i" "k"
      "o" "l"

      "h" "i"
      "j" "n"
      "k" "o"
      "l" "e"))

(add-hook 'evil-collection-setup-hook #'my-hjkl-rotation)

I would expect this to make "e" #'evil-next-visual-line and "j" #'evil-search-next, but "j" stays as #'evil-next-visual-line and "e" as #'magit-ediff-dwim.

[Ambrevar]

I'm not very knowledgeable with key translations. @jojojames, any idea?

[jojojames]

Can you provide an emacs -Q?

[clonex10100]

(setq evil-want-integration t)
(setq evil-want-keybinding nil)
(require 'evil)
(evil-mode 1)

(require 'magit)

(require 'evil-collection)
(defun my-hjkl-rotation (_mode mode-keymaps &rest _rest)
  (evil-collection-translate-key 'normal mode-keymaps
    "j" "k"))
(add-hook 'evil-collection-setup-hook #'my-hjkl-rotation)
(evil-collection-init)

Should bind "j" #'evil-previous-visual-line in magit status.

[jojojames]

Added something for this, the translation function should be taking into account the mode map now. When I try swapping though, it doesn't seem to work as expected. @noctuid

Adding a log message:

    (message "Applying translation for keymap symbol: %S\n keymap: %S\n translations: %S\n maps: %S\n" keymap-symbol keymap translations maps)

Like this:

(defun evil-collection--translate-key (state keymap-symbol
                                             translations
                                             destructive)
  "Helper function for `evil-collection-translate-key'.
In the keymap corresponding to STATE and KEYMAP-SYMBOL, make the key
TRANSLATIONS. When DESTRUCTIVE is non-nil, make the TRANSLATIONS destructively
without creating/referencing a backup keymap."
  (let* ((backup-keymap-symbol (intern (format "evil-collection-%s%s-backup-map"
                                               keymap-symbol
                                               (if state
                                                   (format "-%s-state" state)
                                                 ""))))
         (keymap (symbol-value keymap-symbol))
         (lookup-keymap (if (and (not destructive)
                                 (boundp backup-keymap-symbol))
                            (symbol-value backup-keymap-symbol)
                          (copy-keymap
                           (if state
                               (evil-get-auxiliary-keymap keymap state t t)
                             keymap))))
         (maps (cl-loop for (key replacement) on translations by 'cddr
                        ;; :destructive can be in TRANSLATIONS
                        unless (keywordp key)
                        collect key
                        and collect (when replacement
                                      (lookup-key lookup-keymap replacement)))))
    (unless (or destructive
                (boundp backup-keymap-symbol))
      (set backup-keymap-symbol lookup-keymap))
    (message "Applying translation for keymap symbol: %S\n keymap: %S\n translations: %S\n maps: %S\n" keymap-symbol keymap translations maps)
    (apply #'evil-define-key* state keymap maps)))

For this configuration:

  (defun my-hjkl-rotation (_mode mode-keymaps &rest _rest)
    ;; (evil-collection-swap-key 'normal mode-keymaps
    ;;   "j" "k")
    (evil-collection-translate-key 'normal mode-keymaps
      "j" "k"
      "k" "j"))

  (add-hook 'evil-collection-setup-hook #'my-hjkl-rotation)

Results in this log message:

Applying translation for keymap symbol: magit-status-mode-map
 keymap: (keymap (override-state . all) (visual-state keymap (103 keymap (104 . magit-section-up) (112 keymap (112 . magit-jump-to-unpushed-to-pushremote) (117 . magit-jump-to-unpushed-to-upstream)) (102 keymap (112 . magit-jump-to-unpulled-from-pushremote) (117 . magit-jump-to-unpulled-from-upstream)) (115 . magit-jump-to-staged) (117 . magit-jump-to-unstaged) (110 . magit-jump-to-untracked) (116 . magit-jump-to-tracked) (122 . magit-jump-to-stashes)) "Auxiliary keymap for Visual state") (normal-state keymap (103 keymap (104 . magit-section-up) (112 keymap (112 . magit-jump-to-unpushed-to-pushremote) (117 . magit-jump-to-unpushed-to-upstream)) (102 keymap (112 . magit-jump-to-unpulled-from-pushremote) (117 . magit-jump-to-unpulled-from-upstream)) (115 . magit-jump-to-staged) (117 . magit-jump-to-unstaged) (110 . magit-jump-to-untracked) (116 . magit-jump-to-tracked) (122 . magit-jump-to-stashes)) "Auxiliary keymap for Normal state" (81 . delete-window) (36 . evil-end-of-line) (48 . evil-digit-argument-or-evil-beginning-of-line) (126 . magit-diff-default-context) (96 . magit-process-buffer) (113 . magit-mode-bury-buffer)) (remap keymap (dired-jump . magit-dired-jump)) (106 . magit-status-jump) keymap (override-state . all) (visual-state keymap "Auxiliary keymap for Visual state" (121 . evil-yank) (23 . evil-window-map) (86 . evil-visual-line) (118 . evil-visual-line) (67 keymap (45 keymap (117 . evil-scroll-up))) (108 . evil-forward-char) (104 . evil-backward-char) (12 . magit-log-refresh) (76 . magit-log) (72 . magit-dispatch) (78 . evil-search-previous) (110 . evil-search-next) (47 . evil-search-forward) (33554559 . magit-diff-show-or-scroll-down) (33554464 . magit-diff-show-or-scroll-up) (113 . magit-mode-bury-buffer) (58 . evil-ex) (2 . evil-scroll-page-up) (6 . evil-scroll-page-down) (4 . evil-scroll-down) (71 . evil-goto-line) (107 . evil-previous-line) (106 . evil-next-line) (64 . forge-dispatch) (61 . magit-diff-less-context) (34 . magit-subtree) (39 . magit-submodule) (124 . magit-git-command) (79 . magit-reset) (111 . magit-reset-quickly) (112 . magit-push) (95 . magit-revert) (45 . magit-revert-no-commit) (88 . magit-file-untrack) (120 . magit-delete-thing) (91 . magit-section-backward-sibling) (11 . magit-section-backward) (93 . magit-section-forward-sibling) (10 . magit-section-forward) (103 keymap (103 . evil-goto-first-line) (82 . magit-refresh-all) (114 . magit-refresh) (107 . magit-section-backward-sibling) (106 . magit-section-forward-sibling))) (normal-state keymap "Auxiliary keymap for Normal state" (92 . evil-collection-magit-toggle-text-mode) (20 . evil-collection-magit-toggle-text-mode) (121 keymap (98 . magit-copy-buffer-revision) (115 . magit-copy-section-value) (114 . magit-show-refs) (121 . evil-collection-magit-yank-whole-line)) (23 . evil-window-map) (86 . evil-visual-line) (118 . evil-visual-line) (108 . evil-forward-char) (104 . evil-backward-char) (12 . magit-log-refresh) (76 . magit-log) (72 . magit-dispatch) (78 . evil-search-previous) (110 . evil-search-next) (47 . evil-search-forward) (60 keymap (101 keymap (115 keymap (99 keymap (97 keymap (112 keymap (101 keymap (62 . magit-mode-bury-buffer)))))))) (67 keymap (45 keymap (117 . evil-scroll-up) (122 . evil-emacs-state))) (33554559 . magit-diff-show-or-scroll-down) (33554464 . magit-diff-show-or-scroll-up) (113 . magit-mode-bury-buffer) (58 . evil-ex) (2 . evil-scroll-page-up) (6 . evil-scroll-page-down) (4 . evil-scroll-down) (71 . evil-goto-line) (107 . evil-next-line) (106 . evil-previous-line) (64 . forge-dispatch) (61 . magit-diff-less-context) (34 . magit-subtree) (39 . magit-submodule) (124 . magit-git-command) (79 . magit-reset) (111 . magit-reset-quickly) (112 . magit-push) (95 . magit-revert) (45 . magit-revert-no-commit) (88 . magit-file-untrack) (120 . magit-delete-thing) (91 . magit-section-backward-sibling) (11 . magit-section-backward) (93 . magit-section-forward-sibling) (10 . magit-section-forward) (103 keymap (103 . evil-goto-first-line) (82 . magit-refresh-all) (114 . magit-refresh) (107 . magit-section-backward-sibling) (106 . magit-section-forward-sibling))) (menu-bar keymap (Magit menu-item "Magit" (keymap "Magit" (Refresh menu-item "Refresh" magit-refresh) (Refresh\ all menu-item "Refresh all" magit-refresh-all) (nil menu-item "---") (Stage menu-item "Stage" magit-stage) (Stage\ modified menu-item "Stage modified" magit-stage-modified) (Unstage menu-item "Unstage" magit-unstage) (Reset\ index menu-item "Reset index" magit-reset-index) (Commit menu-item "Commit" magit-commit) (Add\ log\ entry menu-item "Add log entry" magit-commit-add-log) (Tag menu-item "Tag" magit-tag-create) (nil-10 menu-item "---") (Diff\ working\ tree menu-item "Diff working tree" magit-diff-working-tree) (Diff menu-item "Diff" magit-diff) (Log menu-item "Log" (keymap "Log" (Log menu-item "Log" magit-log-other) (Reflog menu-item "Reflog" magit-reflog-other) (Extended\.\.\. menu-item "Extended..." magit-log))) (nil-14 menu-item "---") (Cherry\ pick menu-item "Cherry pick" magit-cherry-pick) (Revert\ commit menu-item "Revert commit" magit-revert) (nil-17 menu-item "---") (Ignore\ globally menu-item "Ignore globally" magit-gitignore-globally) (Ignore\ locally menu-item "Ignore locally" magit-gitignore-locally) (Discard menu-item "Discard" magit-discard) (Reset\ head\ and\ index menu-item "Reset head and index" magit-reset-mixed) (Stash menu-item "Stash" magit-stash-both) (Snapshot menu-item "Snapshot" magit-snapshot-both) (nil-24 menu-item "---") (Branch\.\.\. menu-item "Branch..." magit-checkout) (Merge menu-item "Merge" magit-merge) (Ediff\ resolve menu-item "Ediff resolve" magit-ediff-resolve) (Rebase\.\.\. menu-item "Rebase..." magit-rebase) (nil-29 menu-item "---") (Push menu-item "Push" magit-push) (Pull menu-item "Pull" magit-pull-branch) (Remote\ update menu-item "Remote update" magit-fetch-all) (Submodule menu-item "Submodule" (keymap "Submodule" (Submodule\ update menu-item "Submodule update" magit-submodule-update) (Submodule\ update\ and\ init menu-item "Submodule update and init" magit-submodule-setup) (Submodule\ init menu-item "Submodule init" magit-submodule-init) (Submodule\ sync menu-item "Submodule sync" magit-submodule-sync))) (nil-34 menu-item "---") (Extensions menu-item "Extensions" (keymap "Extensions")) (nil-36 menu-item "---") (Display\ Git\ output menu-item "Display Git output" magit-process-buffer) (Quit\ Magit menu-item "Quit Magit" magit-mode-bury-buffer)))) (remap keymap (evil-next-line . evil-next-visual-line) (evil-previous-line . evil-previous-visual-line) (next-line . magit-next-line) (previous-line . magit-previous-line)) (23 . magit-copy-section-value) (3 keymap (23 . magit-browse-thing) (15 . magit-browse-thing) (5 . magit-edit-thing) (3 . magit-dispatch)) (33 . magit-run) (58 . magit-git-command) (90 . magit-stash) (122 . magit-stash) (89 . magit-cherry) (121 . magit-show-refs) (88 . magit-reset) (120 . magit-reset-quickly) (87 . magit-patch) (119 . magit-am) (86 . magit-revert) (118 . magit-revert-no-commit) (85 . magit-unstage-all) (117 . magit-unstage-file) (84 . magit-notes) (116 . magit-tag) (83 . magit-stage-modified) (115 . magit-stage-file) (82 . magit-file-rename) (114 . magit-rebase) (113 . magit-mode-bury-buffer) (79 . magit-subtree) (111 . magit-submodule) (77 . magit-remote) (109 . magit-merge) (76 . magit-log-refresh) (108 . magit-log) (63 . magit-dispatch) (104 . magit-dispatch) (71 . magit-refresh-all) (103 . magit-refresh) (70 . magit-pull) (102 . magit-fetch) (69 . magit-ediff) (101 . magit-ediff-dwim) (68 . magit-diff-refresh) (100 . magit-diff) (67 . magit-clone) (99 . magit-commit) (66 . magit-bisect) (98 . magit-branch) (65 . magit-cherry-pick) (97 . magit-cherry-apply) (37 . magit-worktree) (36 . magit-process-buffer) (48 . magit-diff-default-context) (45 . magit-diff-less-context) (43 . magit-diff-more-context) (127 . magit-diff-show-or-scroll-down) (33554464 . magit-diff-show-or-scroll-down) (32 . magit-diff-show-or-scroll-up) (73 . magit-gitignore) (105 . magit-gitignore) (75 . magit-file-untrack) (107 . magit-delete-thing) (80 . magit-push) (M-tab . magit-section-cycle-diffs) (27 keymap (119 . magit-copy-buffer-revision) (9 . magit-dired-jump)) (13 . magit-visit-thing) (C-return . magit-visit-thing) keymap #^[nil nil keymap 
#^^[3 0 nil nil nil nil nil nil nil nil nil magit-section-toggle nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil (keymap (52 . magit-section-show-level-4-all) (51 . magit-section-show-level-3-all) (50 . magit-section-show-level-2-all) (49 . magit-section-show-level-1-all) (110 . magit-section-forward-sibling) (112 . magit-section-backward-sibling)) nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil magit-section-show-level-1 magit-section-show-level-2 magit-section-show-level-3 magit-section-show-level-4 nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil magit-section-up nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil magit-section-forward nil magit-section-backward nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] #^^[1 0 #^^[2 0 
#^^[3 0 nil nil nil nil nil nil nil nil nil magit-section-toggle nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil (keymap (52 . magit-section-show-level-4-all) (51 . magit-section-show-level-3-all) (50 . magit-section-show-level-2-all) (49 . magit-section-show-level-1-all) (110 . magit-section-forward-sibling) (112 . magit-section-backward-sibling)) nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil magit-section-show-level-1 magit-section-show-level-2 magit-section-show-level-3 magit-section-show-level-4 nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil magit-section-up nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil magit-section-forward nil magit-section-backward nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil nil] (backtab . magit-section-cycle-global) (M-tab . magit-section-cycle) (C-tab . magit-section-cycle) (remap keymap (self-insert-command . undefined)))
 translations: ("j" "k" "k" "j")
 maps: ("j" evil-next-line "k" evil-previous-line)

I tried something like this with general:

  (with-eval-after-load 'magit
    (general-swap-key 'normal '(magit-status-mode-map) "j" "k"))

and to my surprise, both keys were unbound instead of being swapped.

[noctuid]

Is this just an issue with the general.el version? Based on what evil-collection-translate-key prints, it looks like it should be working as long as j and k are actually bound in those keymaps. I never updated the general version to match the evil-collection version completely, so there may be a bug with the general version.

[jojojames]

Commenting from memory, both don't seem to work for me. General seems to unbind the keys whereas evil-collection's version seems to not do the swap.

 maps: ("j" evil-next-line "k" evil-previous-line)

From the previous log, this seems to imply the keys don't get swapped, right?

If I do something like this:

(evil-collection-translate-key 'normal mode-keymaps
      "j" "k")

e.g. no swap, but a reassignment, I believe j properly calls evil-previous-line.

[jojojames]

Taking another look at this, I found that:

  (evil-collection-translate-key 'normal '(magit-status-mode-map)
    "j" "k"
    "k" "j"
    :destructive t)

  (evil-collection-swap-key 'normal  '(magit-status-mode-map)
    "j" "k"
    :destructive t)

Does actually work and the swap is made properly.

But something like this will fail for magit:

  (defun my-hjkl-rotation (_mode mode-keymaps &rest _rest)
    (evil-collection-translate-key 'normal mode-keymaps
      "j" "k"
      "k" "j"
      :destructive t
      ))

  (add-hook 'evil-collection-setup-hook #'my-hjkl-rotation)

Because magit contains multiple keymaps where some keymaps inherit from others.

For:

(defconst evil-collection-magit-maps '(evil-collection-magit-toggle-text-minor-mode-map
                                       magit-blame-mode-map
                                       magit-blame-read-only-mode-map
                                       magit-blob-mode-map
                                       magit-log-mode-map
                                       magit-mode-map
                                       magit-repolist-mode-map
                                       magit-status-mode-map
                                       magit-submodule-list-mode-map))

We can look at magit-status-mode-map & magit-mode-map. Passing '(magit-mode-map magit-status-mode-map) to the translate function with flip "j" and "k" for magit-mode-map and then back for magit-status-mode-map. I'm assuming because the auxillery keymap pulls from the parent keymap?

If I define "j" and "k" at the magit-status-mode-map layer, e.g.

...
       (,states magit-mode-map "j"     evil-next-line)
       (,states magit-mode-map "k"     evil-previous-line)
...
       (,states magit-status-mode-map "j"     evil-next-line)
       (,states magit-status-mode-map "k"     evil-previous-line)

Key translation works properly. I'm assuming the auxillery-map now pulls the binding from magit-status-mode-map instead of magit-mode-map with this.

Leaving out :destructive t in these translate calls alters the behavior somewhat (bindings are niled out) but I'm assuming the cause is the same. (The solution here is the same, defining the key at the child keymap.)

What do you think @noctuid ? Is the only approach here to rebind all these extra keys at the child keymap or is there a smarter way to translate the function?

[noctuid]

What happens if you change the order so you swap in child maps first and parent maps last? Lookup in a child map will return the definition in the parent map for a key not bound directly in the child map. Possibly, the translate function could do nothing if the key was not bound directly in the child map.

Alternatively, maybe you could do something complicated like look at keymap-parent to see if you had already done a translation for the parent keymap, but that wouldn't be ideal. I'll have to take a deeper look to see what's going on here.

[jojojames]

If I swap the child map and parent map like this:

(defconst evil-collection-magit-maps
  '(evil-collection-magit-toggle-text-minor-mode-map
    magit-log-mode-map ; -> parent: `magit-mode-map'
    magit-status-mode-map ; -> parent: `magit-mode-map'
    magit-blame-mode-map
    magit-blame-read-only-mode-map
    magit-blob-mode-map
    magit-submodule-list-mode-map ; -> parent: `magit-repolist-mode-map'
    magit-repolist-mode-map
    magit-mode-map))

Then the auxillery map returned by this call:

(evil-get-auxiliary-keymap keymap state t t)
;; Result: (keymap "Auxiliary keymap for Normal state" (107) (106))

is almost empty and calling:

(lookup-key lookup-keymap replacement)

returns nil.