-
Notifications
You must be signed in to change notification settings - Fork 279
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
`evil-define-key' for minor mode does not take effect until a state transition #301
Comments
Original comment by Frank Fischer (Bitbucket: lyro, GitHub: lyro): I've been aware of this problem for some time now. I just do not know a solution. Evil uses special keymaps that hold the bindings, but in order to figure out which of these special keymaps to install Evil checks which other keymaps ( This check is done in I would be happy if some could tell me a good way to check fast when a certain minor-mode has been toggled. |
Original comment by Nick DeCoursin (Bitbucket: CapinCape, GitHub: Unknown): Here's a couple more issues that reference the same bug: |
Original comment by Frank Fischer (Bitbucket: lyro, GitHub: lyro): The problem is not what Note that I'm absolutely no fan of this whole But I think that a lot of people rely on and make heavy use of |
Original comment by Justin Burkett (Bitbucket: justbur, GitHub: justbur): Here's an idea that does things a little differently than
|
Original comment by Justin Burkett (Bitbucket: justbur, GitHub: justbur): I can explain the idea in the previous post in case it's not clear. The issue as I understand it is that The proposed solution is to use the |
Original comment by Frank Fischer (Bitbucket: lyro, GitHub: lyro): I see. I like the idea, but in the end it ties the bindings to modes, not to keymaps (as it has been done before). Although this should be sufficient in most cases, there are some modes where this does not work (IIRC undo-tree being one of them, although that might have changed in the meantime). Besides, I still find the the code hard to follow and very hard to check if it's correct. In particular, why do different buffers not influence each other? If one buffer has Evil and smartparens-mode enabled, and another one has only smartparens-mode enabled, then changing the state in the Evil-active buffer will influence the active keymaps in the other buffer, too. That's because Don't get me wrong, I really like the approach and appreciate your efforts to improve that really ugly part of Evil. However, it's a core feature and modifying this stuff will definitely break something for someone (that's for sure in my experience ;)). That's why I have to insist on an implementation and a documentation that I can follow and that (at least) gives me the feeling that everything works well :) |
Original comment by Justin Burkett (Bitbucket: justbur, GitHub: justbur): Good point on it being a global variable, and I agree with and understand the sentiment. Although I think from a user's perspective tying a key binding to a mode is probably the more natural way to think about things. Of course that means less freedom (since some modes have multiple keymaps), but I think a little more transparency. FWIW I put the code there to suggest a different conceptual approach to swapping maps in response to state and mode changes. I thought the code would communicate the idea better than I could in words. I never meant it to be a final product. If you would entertain changes to |
Original comment by Justin Burkett (Bitbucket: justbur, GitHub: justbur): Okay, here's another approach I thought of this morning that seems simpler and ties bindings to maps instead of modes. The idea is to insert state-specific maps as parents of the mode-maps (so we don't modify the mode-map, and also get the benefit of emacs taking care of when the maps are active). There could be a list keeping track of all the mode-maps that have a evil key defined for them, so you don't need to search all of the mode-maps for evil bindings. This is a clean way to do this imo and again only requires that we swap out maps on state changes. The drawback of this method is that since it uses parents, the evil bindings have lower priority than the mode map ones (you could automatically unbind those for people who wanted that though). I don't see this as a major issue, but it is a change in behavior. Here's a working example to illustrate: (defun init-example ()
(setq mode-map (make-sparse-keymap)
mode-map-parent (make-sparse-keymap)
state1-map (make-sparse-keymap)
state2-map (make-sparse-keymap))
(set-keymap-parent mode-map mode-map-parent)
(define-key mode-map "a" "A")
(define-key mode-map-parent "p" "P")
(define-key state1-map [evil-state] 'evil-state)
(define-key state1-map "b" "state1 b")
(define-key state2-map [evil-state] 'evil-state)
(define-key state2-map "b" "state2 b"))
(defun swap-state-map (mode-map new-state-map)
(let ((parent (keymap-parent mode-map)))
(cond
;; parent is a state-map => Replace parent with new-state-map
((and parent
(eq (lookup-key parent [evil-state]) 'evil-state))
(when (keymap-parent parent)
(set-keymap-parent new-state-map (keymap-parent parent)))
(set-keymap-parent mode-map new-state-map))
;; parent is not a state-map => Insert new parent
(parent
(set-keymap-parent new-state-map parent)
(set-keymap-parent mode-map new-state-map))
;; no parent => create one
(t
(set-keymap-parent mode-map new-state-map))))
mode-map)
(init-example)
(lookup-key mode-map "a")
(lookup-key mode-map "b")
(lookup-key mode-map "p")
(swap-state-map mode-map state1-map)
(lookup-key mode-map "a")
(lookup-key mode-map "b")
(lookup-key mode-map "p")
(swap-state-map mode-map state2-map)
(lookup-key mode-map "a")
(lookup-key mode-map "b")
(lookup-key mode-map "p")
(swap-state-map mode-map state1-map)
(lookup-key mode-map "a")
(lookup-key mode-map "b")
(lookup-key mode-map "p") |
Original comment by Frank Fischer (Bitbucket: lyro, GitHub: lyro): Well, I think it is a major issue. In fact, a lot of the machinery around keymaps is exactly for Evil keybindings taking priority over regular bindings. And the problem with different buffers using the same mode (and mode-map) and therefore sharing a common global variable remains. |
Original comment by Justin Burkett (Bitbucket: justbur, GitHub: justbur): Ok, one more, and then I will give up (I promise :-)). Here's some sample code I came up with today (defvar evil-minor-mode-maps
'((insert)
(normal)
;; remaining states
))
(defun evil-define-key-for-mode (state mode key def)
(let ((state-map-alist (assq state evil-minor-mode-maps))
map)
(cond ((and state-map-alist
(assq mode (cdr state-map-alist)))
(setq map (cdr (assq mode (cdr state-map-alist)))))
(state-map-alist
(setq map (make-sparse-keymap))
(setcdr state-map-alist
(append (list (cons mode map)) (cdr state-map-alist))))
(t
(setq map (make-sparse-keymap))
(push (cons state (list (cons mode map))) evil-minor-mode-maps)))
(define-key map key def)))
(evil-define-key-for-mode 'insert 'emacs-lisp-mode "\C-a" "a")
(evil-define-key-for-mode 'insert 'lisp-mode "\C-a" "a") The idea again is to use the mode instead of the mode-map, which at the very least can complement the current The idea is to create an evil This seems simple to me and avoids some of the issues above. You could even use this in a way that keeps the |
Is there any progress/workarounds on this bug? It just started being an issue for me after an upgrade |
Use |
I solved the issue by calling |
This can fix github:emacs-evil/evil#301.
This can fix github:emacs-evil/evil#301.
This can fix github:emacs-evil/evil#301.
I think I'm encountering this with tempel but hooking evil-normalize-keymaps after the relevent functions that update the minor mode map doesn't fix the issue. Any tips on why? So far I've tried. (advice-add 'tempel--insert :after #'evil-normalize-keymaps)
(advice-add 'tempel--done :after #'evil-normalize-keymaps) |
@mohkale when you say you think you are encountering this with tempel, what makes you think that? Does hitting the esc key fix the issue? |
Yes. I hit tempel expand, none of the bindings in tempel map are active. I hit escape and back to insert and they suddenly work again. |
Nice, ok. So you've tried advising some tempel fns. Why advising not hooking like others in this thread e.g. (untested):
or even using
|
Because tempel is a snippet library not a mode and tempel-map is only active when a snippet is expanded and being edited. Sorry if that wasn't clear in my original description. |
Ah sorry - no I'm not familiar with tempel. Can you share the code you're using to define the keybindings then? I will try to repro the problem and find a solution from that. |
BTW I fixed the issue with tempel by doing: (use-package tempel
:preface
(defun tempel--normalize-evil-keymaps+ (&rest _)
(evil-normalize-keymaps))
:init
(advice-add 'tempel--insert :after #'tempel--normalize-evil-keymaps+)
(advice-add 'tempel-done :after #'tempel--normalize-evil-keymaps+)) |
Originally reported by: Anonymous
Unit test. (you can change flyspell to any toggleble minor mode)
(require 'flyspell)
(evil-define-key 'normal flyspell-mode-map [f5]
(lambda () (interactive) (message "Hey")))
In scratch buffer
M-x: flyspell-mode
press F5, key is not bound.
press i then esc, key is now bound
This is a problem because evil is enabled from find-file-hook. before major mode specific hooks are called. If major mode specific hooks enable any minor modes, evil is already initialized, and it does not see those minor modes until a state transition is made, so any bindings for them are in-active
Tested with
GNU Emacs 24.3.1 (x86_64-suse-linux-gnu, X toolkit, Xaw scroll bars) of 2013-05-26 on momoland
Evil from git 8f07f5b0b1fc888428b913e14944e89a0341e7b5
The text was updated successfully, but these errors were encountered: