evil-collection icon indicating copy to clipboard operation
evil-collection copied to clipboard

Which-key binding descriptions

Open loafofpiecrust opened this issue 3 years ago • 10 comments

I like falling back to which-key when I'm learning a new mode or I've forgotten what exactly a particular command is bound to. However, I don't love always seeing the full command name. Especially in, say, proced when I hit o I see all the ordering options but they're all prefixed with proced- which creates visual noise for me to wade through. So I would love to have the option to enable plainer which-key descriptions that are synced/paired with the bindings defined here (i.e. Sort by PID instead of proced-sort-pid). I understand the value of both kinds of display, so I think it should be a toggle.

Would y'all be open to a PR that starts this for a few modes using which-key-add-keymap-based-replacements if which-key is loaded and a new defcustom is t?

loafofpiecrust avatar Jan 08 '21 04:01 loafofpiecrust

I tried using which-key a couple times in the past and I think I ran into similar issues like you, the full command name + the massive amount of potential commands in one mode (I think there was a way to filter it..) made it hard for me to use it and I always end up dropping it, despite how useful it is. I wound up wanting something like hydra where you declare your keys and run it off a leader key.

https://github.com/abo-abo/hydra

Which had a similar problem (the hydra-d keys/finished UI, when done in a programmatic fashion seemed unpolished, unclear, rough around the edges).

Something like https://github.com/jerrypnz/major-mode-hydra.el improved on the UI a ton (at least from the screenshots) and seemed worth trying until I tried transient.

https://github.com/magit/transient

I'm using transient right now for my needs and it works out pretty well I think (with a big downside being that it seems to force-load all your packages that's declared in the transient...)

Big problem with transient and others like hydra is the need to basically hand-write the configuration for every package. Otherwise, it'll always feel kind-of unpolished. (You run into a mode without the handwritten configuration, and it looks like more bikeshedding is needed).

You can see my efforts for declaring transients here: https://github.com/jojojames/matcha (if you look at the archives, you'll even see a trove of hydras declared: e.g. https://github.com/jojojames/matcha/blob/master/archive/matcha-evil-mc.el).

I think one problem evil-collection solves is that big 'writing a configuration/wrapper/layer for [insert mode #589] is too tedious for any one person, and at the same time, starting one for one of those packages might also be too big an undertaking. I do wish there was something similar for which-key/hydra/transient/etc (a collection of them) that anyone can just go and use. (e.g. if there was a solid collection of hydras that was just in a repo somewhere, I'd totally just switch back to hydra). I don't use doom or spacemacs but maybe those projects have a better story around these types of leader/which-key based UI due to the crowd-sourced nature of the projects.

As for which-key-add-keymap-based-replacements, it's been a while since I last used which-key, so you'll have to remind me on how it works. Is this something that we need to tie specifically to evil-collection's key definitions. E.g. If we created a separate repo/package for these which-key definitions, it wouldn't be feasible because they'd quickly get out of sync?

If it was more like "for this function, use this name, in this place" to set up a proper UI and it doesn't need to be bound to evil-collection, I'd imagine a separate package would work right?, at least in theory.

At that point I'd recommend even which-key take it up (I know @justbur is busy so maybe that's a no go :D)

I'm potentially open to it, with the big caveat being 1. it just might not belong in this repo and 2. the additional overhead (even doing with-eval-after-loads, etc to check against which-key) might be too much 3. potential maintenance ahead so it'd be interesting if there was more interest in this from more people before we do the go ahead.

jojojames avatar Jan 08 '21 10:01 jojojames

Thank you for the very detailed response! I generally like the idea of defining transients for different modes, and transient is really powerful when it's needed (like with magit), but it seems a bit overkill for most modes to redefine the key associations if we want to keep in sync with evil-collection for example. I imagine you could pass in a command => string association and at runtime find which key that matches up with in the given map, showing that in the transient string. As you also said, it seems to me that what we actually want is to associate a command with a description, not a key; and anyone should be able to switch the binding of a command and get the same description (if in the same keymap) in whatever interface: transient, which-key, hydra, menu bar.

So maybe this partial definition I'm pulling from matcha:

(define-transient-command matcha-rust-mode-cargo
  "Cargo"
  [
   :description (lambda () (format "Cargo: %s" (matcha-projectile-root)))
   ["Actions"
    ("x" "Run" cargo-process-run)
    ("X" "Run Example" cargo-process-run-example)
    ("v" "Check" cargo-process-check)]
   ["Manage"
    ("i" "Init" cargo-process-init)
    ("n" "New" cargo-process-new)]])

could instead be defined as something like so:

(define-transient-keymap-command matcha-rust-mode-cargo rustic-mode-map
  "Cargo"
  [
   :description (lambda () (format "Cargo: %s" (matcha-projectile-root)))
   ["Actions"
    ("Run" cargo-process-run)
    ("Run Example" cargo-process-run-example)
    ("Check" cargo-process-check)]
   ["Manage"
    ("Init" cargo-process-init)
    ("New" cargo-process-new)]])

and that'd give me personally something like b r -> Run and b i -> Init, because ultimately being able to define keymaps independent from these nice UIs to display them is valuable and it becomes clearer how I might modify bindings. I guess that leads me to think about modifications to transient to support pairing with an existing keymap and keeping the same description for a given command even if the key binding changes on the fly.

To start with, in terms of which-key, it seems to define replacements strictly in terms of key => string mapping rather than command => string. So either which-key supports a new kind of replacement based on function (ideal!) or as a stopgap I can try writing the function I described that takes a set of command => string mappings and finds the matching keys in the given keymap at runtime, to then apply to which-key-add-keymap-based-replacements.

Either way, I think you're right that this should be outside the scope of evil-collection and instead live in matcha (or something like it). At least I have somewhere to start based on this discussion, thank you!

loafofpiecrust avatar Jan 09 '21 03:01 loafofpiecrust

because ultimately being able to define keymaps independent from these nice UIs to display them is valuable and it becomes clearer how I might modify bindings.

Right at least for both hydra and transient, they serve two purposes (binding the key &and& displaying them), so in effect, the transient/hydra stuff is more close to the leader functionality in vim.

In a sense, if everyone agreed what the hydra/transient should look like, the problem you're asking for (the description of the binding itself), would be a solved problem for hydra/transient. So then, the problem after that is really getting that critical mass of crowdsourced hydras/transients in one repo everyone can consume.

Which-key, on the other hand is closer to solving the "the keys are already bound, how do we show them to the user".

strictly in terms of key => string mapping rather than command => string.

Yeah, for bigger modes like dired or something, it'll probably be tedious to write but like I mentioned before, the potential crowd sourced nature of it would make it a much easier problem to solve.

jojojames avatar Jan 09 '21 04:01 jojojames

So I just rigged up what I mentioned with which-key in my DOOM setup. Running the following at the end of my config adds descriptions to some otherwise plain bindings without knowing at the callsite what the key sequence is.

(defun +which-key-add-keymap-based-replacements (keymap key replacement &rest more)
  (while key
    (let ((string (if (stringp replacement)
                      replacement
                    (car-safe replacement)))
          (command (cdr-safe replacement)))
      (define-key keymap (which-key--pseudo-key key)
        `(which-key ,(cons string command))))
    (setq key (pop more)
          replacement (pop more))))

(defun +which-key-add-command-based-replacements (keymap replacements)
  (seq-do (lambda (replacement)
            (seq-do (lambda (binding) (+which-key-add-keymap-based-replacements
                                  keymap
                                  binding
                                  (cdr replacement)))
                    (where-is-internal (car replacement) keymap)))
          replacements))

(+which-key-add-command-based-replacements
 doom-leader-map
 '((bitwarden-edit . "Edit Account")
   (bitwarden-getpass . "Copy Password")
   (bitwarden-generate-password . "Generate Password")
   (olivetti-mode . "Olivetti")
   (prettify-symbols-mode . "Prettify Symbols")
   (literate-calc-minor-mode . "Literate Calc")
   (calc . "Calculator")))

But, of course, this doesn't respond to new bindings in the keymap after that.

loafofpiecrust avatar Jan 09 '21 04:01 loafofpiecrust

So I feel a bit dumb for going through all that with which-key without realizing that they provide which-key-enable-extended-define-key which advises define-key to allow you to provide the command replacement directly in the keymap like so

(define-key some-map "f" '("foo" . command-foo))

Since that's a valid define-key call with or without this turned on, evil-collection could just start providing bindings like that!

(evil-collection-define-key 'normal 'proced-mode-map
  "o" '("order")
  "os" '("Order by start" . proced-sort-start))

Then, if which-key isn't installed or (null which-key-enable-extended-define-key) the binding will work as normal, showing proced-sort-start in the which-key buffer.

If that option is enabled before which-key and evil-collection are loaded, however, the custom titles are shown in the which-key buffer. This may not be a perfect solution but it feels like a decent place to start from within this package.

My feeling is that if we can define this much directly in the keymap, then we should theoretically be able to write hydras or transients that pull the binding and description straight from an existing keymap too, thus reducing duplication of work and bindings getting out of sync. This is basically hercules, but like you mention which-key doesn't look very polished with many bindings. So maybe building some kind of grouping into hercules plus descriptions living in the keymap would be just the right kind of magic?

loafofpiecrust avatar Jan 09 '21 19:01 loafofpiecrust

So I just rigged up what I mentioned with which-key in my DOOM setup.

I'm curious, does DOOM not support this use case?

If that option is enabled before which-key and evil-collection are loaded, however, the custom titles are shown in the which-key buffer. This may not be a perfect solution but it feels like a decent place to start from within this package.

I guess this is built into define-key? That's pretty nice and might be the right way to go. I'm curious how much extra latency parsing (if any?) happens when you pass strings into define-key. I wouldn't be surprised if it's near zero and also wouldn't be surprised if it's significant. :)

without realizing that they provide which-key-enable-extended-define-key

I did some extra reading and it seems general also provides something similar.

https://github.com/noctuid/general.el#which-key-integration

Tangential, but looking back, it probably would've been nice to use general as the base instead of evil-define-key, we would've been able to get freebies like key translation, key recording and a much more expressive API to go with it.

Not sure if I have the energy to look into switching though.

https://github.com/noctuid/general.el#which-key-integration

jojojames avatar Jan 10 '21 02:01 jojojames

Yeah so based on my reading, the general integration (keyword :wk/:which-key) uses which-key-replacement-alist, which is slower, since it uses regex matching, than embedding the replacement in the keymap itself as I've described (or using which-key-add-keymap-based-replacements which is what the define-key advice calls automatically) which seems to have minimal overhead. And it seems that the author of which-key wants to push users away from which-key-replacement-alist because it doesn't scale as well (see this). DOOM currently supports binding name replacements, but only using the regex method of which-key-replacement-alist, which is more technically powerful but means that replacements don't transfer when I use an existing keymap under a new prefix. I want to make a PR to doom switching it to the tighter, keymap-based method and I can make a PR here starting to add key descriptions for modes I use.

EDIT: Then again, maybe using which-key-add-keymap-based-replacements in a separate package with the snippet I posted is the way to go? Adding many command descriptions is technically independent of evil and if I detect which keys are bound to each command at runtime to add the description, all the vanilla bindings receive the same treatment. Not sure, what do you think?

loafofpiecrust avatar Jan 10 '21 04:01 loafofpiecrust

I want to make a PR to doom switching it to the tighter, keymap-based method and I can make a PR here starting to add key descriptions for modes I use.

I'm curious how hard it'd be to user-override the key definitions if we place them on the keymap? For example, I'm thinking all the replacements will be in english but I can see other users not using english for their definitions. Do they have easy recourse without remapping everything?

jojojames avatar Feb 02 '21 07:02 jojojames

That's a great point that I hadn't thought much about. A real multilingual solution might be to maintain a separate set of mappings that change the binding description based on your language selection. Unless there's an in-built Emacs solution, which I would expect for menu items at large. So not sure.

In terms of user replacements, I would provide a function that changes the description of whatever key a given command is bound to (I use this in my config). Then, at least, the user doesn't need to restate the key, just the command.

Also, see this PR on which-key to support vanilla menu items.

loafofpiecrust avatar Feb 02 '21 15:02 loafofpiecrust

I haven't had the chance to look at that PR but this topic does come to mind every so often, just thought I'd leave an update. If you had any -in the meanwhile- proposals, I'd be up to hear them.

jojojames avatar Mar 02 '21 01:03 jojojames

Closing for now, I haven't had much interest in this but will reopen if it comes up. Please feel free to reopen if something sparks your mind too.

jojojames avatar Mar 09 '23 19:03 jojojames