makey icon indicating copy to clipboard operation
makey copied to clipboard

Not recognizing prefix keys

Open xiongtx opened this issue 8 years ago • 4 comments

Expected behavior

Calling (makey-key-mode-build-keymap 'clojure-mode) should return a keymap.

Actual behavior

Calling (makey-key-mode-build-keymap 'clojure-mode) gives the following error:

Debugger entered--Lisp error: (error "Key sequence C-c C-r C-a starts with non-prefix key C-c C-r")
  define-key((keymap (3 keymap (27 keymap (106 lambda nil (interactive) (makey-key-mode-command (quote cider-jack-in))) (99 lambda nil (interactive) (makey-key-mode-command (quote cider-connect))) (74 lambda nil (interactive) (makey-key-mode-command (quote cider-jack-in-clojurescript)))) (32 lambda nil (interactive) (makey-key-mode-command (quote clojure-align))) (18 lambda nil (interactive) (makey-key-mode-command (quote clojure-refactor-map)))) (67108922 lambda nil (interactive) (makey-key-mode-command (quote clojure-toggle-keyword-string))) (63 lambda nil (interactive) (makey-key-mode-help (quote clojure-mode))) (113 lambda nil (interactive) (makey-key-mode-command nil)) (7 lambda nil (interactive) (makey-key-mode-command nil)) (remap keymap (self-insert-command . undefined))) "" (lambda nil (interactive) (makey-key-mode-command (quote clojure-unwind-all))))
  #[(k action) "\303\304	@!\305\306\307\nF#\207" [map k action define-key kbd lambda nil (interactive)] 7](("C-c C-r C-a" "Fully unwind thread at point or above point." clojure-unwind-all) (makey-key-mode-command (quote clojure-unwind-all)))
  makey-key-mode-build-keymap(clojure-mode)
...

C-c C-r is bound to a keymap, so it should be considered a prefix key. makey-key-mode-options-for-clojure-mode even acknowledges it as such.

...
(\"C-c C-r\" \"Prefix command (definition is a keymap associating keystrokes with commands).\" clojure-refactor-map)
...

Steps to reproduce the problem

  1. Install clojure-mode
  2. Call (makey-key-mode-build-keymap 'clojure-mode)

Operating system

Darwin C02RW05YFVH6 15.6.0 Darwin Kernel Version 15.6.0

xiongtx avatar Jan 12 '17 01:01 xiongtx

The problem seems to be with handling a symbol whose function definition is a keymap. This is allowed, according to the Elisp manual.

A minimal working example:

(defvar ctl-x-ctl-t-map
  (let ((map (make-sparse-keymap)))
    (define-key map "b" #'helm-mini)
    (define-key map "f" #'helm-find-files)
    map))
(fset 'ctl-x-ctl-t-map ctl-x-ctl-t-map)

(defvar test-keymap-mode-map
  (let ((map (make-sparse-keymap)))
    (define-key map (kbd "C-x C-t") 'ctl-x-ctl-t-map)
    map))

(define-derived-mode test-keymap-mode prog-mode "TestKeymap")

xiongtx avatar Jan 14 '17 03:01 xiongtx

Nice one, @xiongtx -- I don't think I've ever seen this use case before. Can you recommend a fix?

mickeynp avatar Jan 14 '17 09:01 mickeynp

I'm not sure whether makey should be fixed at all, TBH. Excluding prefix keys in discover-my-major certainly seems easier.

What's the intended usage of makey? Should it handle prefix maps "actions"?

xiongtx avatar Jan 14 '17 19:01 xiongtx

It's a good question, really. Its main purpose originally was as a proof-of-concept to show that Magit's excellent popup system had value outside of Magit itself. Both hydra and now the magit project have their own versions, though I do not think either of them support let-binding variables in the interface like makey does.

I'm torn on whether it should support this and really want to leave it up to you: if it's easier to fix it in discover-my-major then that's fine with me -- if it's easier to do in makey then that is also fine if it preserves backwards compatibility.

mickeynp avatar Jan 27 '17 20:01 mickeynp