Rework which-key--get-current-bindings #144
Conversation
Getting the bindings for a a prefix by parsing the buffer produced by `describe-buffer-bindings` seems a little convoluted. Especially when `map-keymap` seems to be just the job and has the added advantage of giving us the actual values of the bindings in the current keymap. This doesn't matter too much for a bound command, but when the binding is to a keymap it lets us attempt to get `(keymap-prompt binding)` as a potential name. I'd not be surprised if it's faster too since we're not mucking about with regular expressions.
|
Note that it might be better to move the use of |
|
I'm not totally opposed to this idea, but there are some issues.
Speed has never been an issue for me. It also happens to be the case that Anyway, if you find a nice solution to those issues I'll test some more. |
Allow things like
(define-key global-map (kbd "C-c .")
(lambda ()
"open .init.el"
(find-file-existing user-init-file)))
to Just Work without having to tell which which-key anything else
I was trying to be too clever when dealing with the the 'key' part of map-keymap. If we just do `(vector key)`, we get something compatible with key-description
|
Fixed the It's not The nice thing about |
Binding something like:
`(menu-map "" nil :filter
,(lambda (&optional _)
(if (some-predicate) 'command-name)))
is a cunning trick for setting up bindings that are only active if some
condition holds. This makes which-key show the correct 'final' binding
by evaluating the filter.
|
That last patch does possibly go a little far, but it does demonstrate what's possible by using |
|
I haven't been through everything you did, but you can see the multiple key bindings problem here
|
|
Could you attach the result of Which minor mode is it? EDITAh... no matter. Found it. |
Yay for free software! After taking a look at the implementation of `map-keymap` it's apparent that it respects the ordering of a keymap so the first entry we get for a given key will be the binding that would be triggered if we were to actually type that key sequence. So `-get-raw-current-bindings` now does an assoc look up on the key description and skips any that have already been seen. Fixes the issue with C-x u showing up twice while `global-undo-tree` is turned on (and other, similar cases).
Introduces a new `which-key--canonicalize-bindings` function and rewrites `which-key--get-raw-current-bindings` in terms of it. This will allow us to test our keymap munging by passing artificial keymaps into -canonicalize-bindings, which is a good deal easier than trying to set up a temporary buffer with the bindings we need (I tried. I couldn't do it).
Minor modes can end up creating keymaps analogous to:
'(keymap (?a . 'active-binding)
(keymap (?a . 'overriden-binding)))
We should ensure that the output from `which-key--get-current-bindings`
has eliminated any overridden bindings. This tests that.
Here are the thoughts of an unthinking hacker. "Right... the tests are running fine, but I've just realised that that parameter is poorly named. I'll just fix that and commit, no need to re-run the tests. "Hmm... the Travis build is failing... why's that?" *tapitty tappity* "Ah... yeah, that'd do it, I need to change that parameter name in the body of the function don't I?" We now return you to your regular programming.
|
Still not working. I tried this (define-key global-map (kbd "C-x M-x") (lambda () (interactive) (message "hi")))and nothing shows up for I don't see any real advantage to doing this way. The only (pretty minor) advantage I can see is to not have to parse a text buffer, but in it's place you now have to know and keep track of all the rules about how keys are represented and which keys have priority. I think you should give up on this to be honest. |
|
"Having to keep track of all the rules" isn't a bug, it's a feature. The thing is, if I do: (define-key help-map "M-x" (cons "description" (lambda () (interactive) "doc string" ...)))Then it seems reasonable to me that which-key would show I'll work up a full featured branch that shows what I mean and submit another pull request I think. |
|
It's definitely a bug right now, because all the bindings are not showing up (#144 (comment)). Maybe if you get the binding and event handling rules right it enables some additional features, but it also adds maintenance burden. |
|
Replaced with #147 |
Getting the bindings for a a prefix by parsing the buffer produced by
describe-buffer-bindingsseems a little convoluted. Especially whenmap-keymapseems to be just the job and has the added advantage ofgiving us the actual values of the bindings in the current keymap. This
doesn't matter too much for a bound command, but when the binding is to
a keymap it lets us attempt to get
(keymap-prompt binding)as apotential name.
I'd not be surprised if it's faster too since we're not mucking about
with regular expressions.