gabesoft / evil-mc

Multiple cursors implementation for evil-mode
MIT License
383 stars 35 forks source link

Use a keymap #109

Closed Silex closed 4 years ago

Silex commented 4 years ago

It took me a while but this is round two of #100

Given that I screwed up the first time I'd be nice if someone could confirm that this PR is ok.

I used https://github.com/noctuid/evil-guide#leader-key as the inspiration for the implementation. Maybe this can be rewritten more nicely? I'm still a bit confused about evil-define-key vs define-key. Also maybe I could use define-prefix-command instead of defvar for evil-mc-cursors-map?

@jojojames: the idea is that this is the first step in order to start modifying https://github.com/emacs-evil/evil-collection/issues/184. My goal is simply to then bind evil-mc-cursors-map to something appropriate and leave out the other invasive bindings (C-n, M-p, etc).

@duianto: thought you'd want to be noticed :smirk:

Silex commented 4 years ago

Damnit, I realise that the gr prefix keymap is added for all states (not only normal and visual).

Anyone knows how I'm supposed to map to a prefix keymap with evil-define-key?

Silex commented 4 years ago

@noctuid: maybe with your knowledge you could point me to the correct direction?

Basically my question is "how do I do a Mode Specific Keybindings to a prefix keymap?"

duianto commented 4 years ago

I haven't mapped to a prefix keymap, so I'm unable to provide any assistance.

noctuid commented 4 years ago

Do you just want evil-define-key* since map is a variable? What have you tried?

Silex commented 4 years ago

I want to go from https://github.com/gabesoft/evil-mc/blob/master/evil-mc.el#L79-L105 to https://github.com/Silex/evil-mc/blob/07b07504d3313d1e9922f7cca2cd294c225a671e/evil-mc.el#L79-L108, which means take all the bindings that start with gr and put them in a keymap on their own.

To start, the keymap extraction is easy. I think this part is right:

(defvar evil-mc-cursors-map
  (let ((map (make-sparse-keymap)))
    (define-key map (kbd "m") 'evil-mc-make-all-cursors)
    (define-key map (kbd "u") 'evil-mc-undo-last-added-cursor)
    (define-key map (kbd "q") 'evil-mc-undo-all-cursors)
    ;; snip
    map))

Then here is what I tried (that does not work):

(defvar evil-mc-key-map
  (let ((map (make-sparse-keymap)))
    (evil-define-key '(normal visual) map
      (kbd "gr") evil-mc-cursors-map
      (kbd "M-n") 'evil-mc-make-and-goto-next-cursor
      (kbd "M-p") 'evil-mc-make-and-goto-prev-cursor
      (kbd "C-n") 'evil-mc-make-and-goto-next-match
      ;; snip
    map))

Here is what seems to work, but the prefix map is defined for all modes and not only (normal visual):

(defvar evil-mc-key-map
  (let ((map (make-sparse-keymap)))
    (define-key map (kbd "gr") evil-mc-cursors-map)
    (evil-define-key '(normal visual) map
      (kbd "M-n") 'evil-mc-make-and-goto-next-cursor
      (kbd "M-p") 'evil-mc-make-and-goto-prev-cursor
      (kbd "C-n") 'evil-mc-make-and-goto-next-match
      ;; snip
    map))
noctuid commented 4 years ago

Yeah, using evil-define-key* seems to work fine for me.

Silex commented 4 years ago

Yeah, using evil-define-key* seems to work fine for me.

Nice! Thanks a lot. It looks like we'd be better off using evil-define-key* all the time no? I don't see any advantage of the macro form:

The use is nearly identical to ‘evil-define-key’ with the
exception that this is a function and not a macro (and so will
not be expanded when compiled which can have unintended
consequences). ‘evil-define-key*’ also does not defer any
bindings like ‘evil-define-key’ does using ‘evil-delay’. This
allows errors in the bindings to be caught immediately, and makes
its behavior more predictable.

Anyway, @gabesoft I think you can merge.

noctuid commented 4 years ago

I find the delaying behavior pretty useful personally for configuration. For use in packages, evil-define-key* is probably almost always sufficient.