Add replacements for minor mode bindings

[erikarvstedt]

Currently, it's not possible to define key description replacements that are specific to minor modes.

A cursory look at the code seems to suggest that this should be rather easy to implement:

• Rename which-key-add-major-mode-key-based-replacements to which-key-add-mode-key-based-replacements. The function itself can be left unchanged.

• In which-key--get-current-bindings, store for each binding the mode (major or minor) that provided the binding. This info is readily provided by describe-buffer-bindings. (Store nil if the binding is global.)

• In which-key--maybe-replace, instead of using the current major-mode for mode-specific replacments, use the mode that provided the binding.

Do you think this approach is feasible or am I missing something?
This should probably be a breeze to implement. If you're low on time, I'd try an implementation by myself.

[justbur]

Hm, there are a couple of tricky parts I can think of off the top of my head:

1. Determining which minor-modes are active at any point in time is a little annoying and not quite foolproof
2. There might need to be some logic for determining priority of minor-mode replacements over other minor-mode replacements and over major-mode replacements.

What would the case be where this is valuable? I would think that in most cases the minor-mode activates and deactivates the bindings automatically and that all of the bindings are prefixed with the name of the minor mode, so that needing to know whether the mode is active or not is unnecessary.

[erikarvstedt]

1. describe-buffer-bindings already does that. For each active binding, unless it's defined globally, there's exactly one mode that provides it. This may be a major or a minor mode, but this distinction should be irrelevant for which-key. We can then just use that mode for the lookup. Am I missing something here?

2. I don't think we would need that. What I had in mind is this very simple mode of operation:
   We allow two kinds of replacements, global and mode-specific.
   When determining the replacement for a binding, we first lookup the mode-specific, then the global replacement.

What would the case be where this is valuable?

I run into that all the time. I have bindings where the same keys are bound to different commands, depending on the active minor mode. Currently, which-key doesn't allow to define description replacements for these bindings.
My proposal naturally extends the existing replacement mechanism to all modes, major and minor.

[erikarvstedt]

I failed to note that the proposed mechanism is not backwards compatible when which-key-add-major-mode-key-based-replacements was used to define replacements for bindings that are not actually provided by the major mode (but instead, e.g., by a minor mode that's activated by the major mode). So we'd have to keep that old function and use the old mechanism for repls that were defined this way.

[justbur]

Ok, I think I understand the problem better. You're right about describe-buffer-bindings. There is still some inherent guess work involved with minor modes but it's not a big deal.

The problem with the mode-specific binding part is that there is one major-mode and simultaneously any number of minor modes active at one time, so there are conflicts to resolve there.

I run into that all the time. I have bindings where the same keys are bound to different commands, depending on the active minor mode. Currently, which-key doesn't allow to define description replacements for these bindings.
My proposal naturally extends the existing replacement mechanism to all modes, major and minor.

There's a more straightforward way to handle this problem I think. You can replace based on key and/or command name, so if you defined your replacements based on command name you would have to worry about the times when that command is not available. I could add a helper function for this kind of replacement if you wanted.

There's also IMO a nicer way of specifying replacements that gets around this problem. You can do

(setq which-key-enable-extended-define-key t)
(define-key my-map "k" '("My Command Name" . command))

and it will show "My Command Name" for command whenever my-map is active. If my-map is a minor-mode map it will only show when the minor-mode is enabled.

[mohkale]

I like @justbur's solution, but I also think having which-key-add-mode-key-based-replacements would be more intuitive in the long run.

[erikarvstedt]

Sorry for the intermission, I was on an extended vacation.

… there is one major-mode and simultaneously any number of minor modes active at one time, so there are conflicts to resolve there.

Each active binding is provided by exactly one mode (or it's global). I don't see any potential conflicts when looking up the description.

You can replace based on key and/or command name …

I use which-key mainly for prefix keys, so this is not an option here.

which-key-enable-extended-define-key would indeed work, but it messes up describe-bindings with which-key's internal definitions.
The solution outlined in my initial post would solve this elegantly. The only downside is that it's not backwards-compatible, as existing descriptions of minor mode bindings need to associated with the actual minor mode that defines them. But we could keep the old mechanism around for a while.

Edit: Sorry, I accidentally hit the close and comment button.

[justbur]

@erikarvstedt I'd review (and probably accept) a PR for this if it doesn't get too messy

[JAremko]

We are sorting out bindings in Spacemacs and this feature will be very welcome.

[erikarvstedt]

Yep, will probably be helpful for doom-emacs, too. I'll work it out in the next few days.