`evil-delay` doesn't admit user use `evil-define-key` in `dolist`.
- Dominant language
- Emacs Lisp
- Stars
- 3.9k
- Forks
- 307
- PR merge metrics
- No merged PRs in 30d
Description
Originally reported by: **Ken Okada (Bitbucket: [kenoss](https://bitbucket.org/kenoss), GitHub: [kenoss](https://github.com/kenoss))**
----------------------------------------
IMHO, using `eval` for forms in macros is definitely bad idea. It is used in `evil-define-keys`.
For example, see following configration and what happens.
``` emacs-lisp
(with-eval-after-load 'evil
(defvar magit-commit-mode-map-orig magit-commit-mode-map)
(defvar magit-status-mode-map-orig magit-status-mode-map)
(defvar magit-log-mode-map-orig magit-log-mode-map)
(defvar magit-mode-map-orig magit-mode-map)
(defvar git-rebase-mode-map-orig git-rebase-mode-map)
(dolist (mode '(git-commit-mode
magit-log-edit-mode))
(evil-set-initial-state mode 'insert))
(dolist (mode '(magit-mode
magit-status-mode
magit-commit-mode
magit-log-mode
magit-wassup-mode
git-rebase-mode))
(evil-set-initial-state mode 'normal)
(unless (memq mode '(magit-wassup-mode))
(let ((map (symbol-value (intern (concat (symbol-name mode) "-map"))))
(map-orig (symbol-value (intern (concat (symbol-name mode) "-map-orig")))))
(evil-make-overriding-map map 'normal)
(evil-define-key 'normal map
";" (lookup-key evil-motion-state-map ";")
"h" 'magit-goto-previous-section
"t" 'magit-goto-next-section
"H" 'magit-goto-previous-sibling-section
"T" 'magit-goto-next-sibling-section
(kbd "z") nil
(kbd "z z") 'magit-toggle-section
))))
)
```
This produce not expected result because `map` and `map-orig` remain in the original form.
This is an evil use of eval.
I guess Vegard had chosen it because "map variable" doesn't exist in the evaluation of this form,
without appropriate `eval-after-load`. I'm not positive that though. Because:
1. This contradicts minimal suprising principal.
2. I beleave that users should write appropriate `require` or `eval-after-load` themselves. Emacs Lisp compiler warns when undefined variables are used.
If user wants to use autoload, he should write `(eval-when-compile (require ...))` to make quiet compiler, and this solves the problem above.
Hence, I'd like you to fix this macro or `evil-define-key`.
Note:
1. You shouldn't generate intrinstic variables in macros. Macros should do only convert code.
2. I think we shouldn't use non-interned symbols for hooks. This makes tracing problems difficult. If you want to use volatile hook, use random numbers like gensym.
See also: https://github.com/kenoss/erfi/blob/85f38c0a08dd607ad4f0cb87e410ca594240447c/lisp/erfi-emacs.el#L32
(Do not fully trust this code. This is not effective for lots of call. But it was enough for the being.)
----------------------------------------
- Bitbucket: https://bitbucket.org/lyro/evil/issue/526
Contributor guide
Research direction
The relevant entry points are evil-delay, evil-define-key, and evil-define-keys; read their macro expansions first, especially how forms inside dolist are evaluated. Reproduce the supplied Magit configuration and confirm that the key bindings use each loop's map values rather than the original forms.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- emacs-lisp
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100