search

emacs-evil / evil-collection

A set of keybindings for evil-mode
GNU General Public License v3.0
1.21k stars 275 forks source link

Corfu keybindings are not set up properly #766

Closed sebwelkberg closed 7 months ago

sebwelkberg commented 9 months ago

An error message appears in the echo area which states that evil-normalize-keymaps is called with a wrong number of arguments as the Corfu completion popup is shown.

The issues is created by setting evil-normalize-keymaps as an advice to the setup and teardown function. The problem is that the Corfu functions take a fixed set of arguments while evil-normalize-keymaps just has one optional argument. Since advices must have the same signature as the function, the apply call fails.

This is fixed by just using a lambda with &rest that consumes the arguments and calls evil-normalize-keymaps.

I have a branch ready that I could push. Otherwise this is the required change:

-  (advice-add 'corfu--setup :after 'evil-normalize-keymaps)
-  (advice-add 'corfu--teardown :after 'evil-normalize-keymaps))
+  (advice-add 'corfu--setup :after (lambda (&rest r) (evil-normalize-keymaps)))
+  (advice-add 'corfu--teardown :after  (lambda (&rest r) (evil-normalize-keymaps))))
AlexanderArvidsson commented 9 months ago

A temporary workaround until your fix is merged upstream is like this:

(use-package corfu
  :custom
  (corfu-auto t)

  :init
  (global-corfu-mode)

  ;; https://github.com/emacs-evil/evil-collection/issues/766
  (advice-remove 'corfu--setup 'evil-normalize-keymaps)
  (advice-remove 'corfu--teardown 'evil-normalize-keymaps)

  (advice-add 'corfu--setup :after (lambda (&rest r) (evil-normalize-keymaps)))
  (advice-add 'corfu--teardown :after  (lambda (&rest r) (evil-normalize-keymaps))))

Basically, remove the advices introduced by evil-collection and add the correct ones from @sebwelkberg. This has to be done right after global-corfu-mode because the incorrect code (in evil-collection-corfu-setup) runs as a hook on global-corfu-mode.

minad commented 9 months ago

This issue is resolved by #767 and #769.