general.el icon indicating copy to clipboard operation
general.el copied to clipboard

Display/specify which-key for general-predicate-dispatch

Open terlar opened this issue 8 years ago • 9 comments

Not sure if there is any way around this, but when I define a keybinding with general-predicate-dispatch I don't see that key appearing at all with which-key. Otherwise I really like how this one is working, my usage currently:

(general-define-key
 :prefix "C-c"
 "SPC" (general-predicate-dispatch #'projectile-switch-project
         :docstring "Find file in project or switch project"
         (projectile-project-p) #'projectile-find-file))

terlar avatar Nov 02 '17 18:11 terlar

Unlike general-key-dispatch, for example, general-predicate-dispatch does not return a function but a menu item. If you eval (key-binding (kbd "C-c SPC")), you will actually get either projectile-switch-project or projectile-find-file depending on (projectile-project-p). I'm guessing that which-key doesn't support displaying menu items, so I'd suggest making an issue there.

noctuid avatar Nov 02 '17 18:11 noctuid

Thanks for the info, I really like this feature especially since some plugins rely on the actual binding being to projectile-find-file and replaces that binding with it's own, which works with general-predicate-dispatch but doesn't seem to work with the other approaches.

terlar avatar Nov 02 '17 21:11 terlar

I am not familiar with the menu items, but found this related closed issue inside the which-key project: https://github.com/justbur/emacs-which-key/pull/147

Perhaps that tells you something about the state? Or is it not really related?

terlar avatar Nov 02 '17 21:11 terlar

It's not really related. #39 spawned that issue. The idea of that pull request was to use the menu-item ITEM-NAME to clue which-key in about what to display as a replacement (and was rejected). In that case the only reason menu-item was used was to give which-key a replacement. In this case, menu-item is used for a predicate. I'd make a new which-key issue. It seems you can't even manually add a replacement for keys bound to a menu-item.

noctuid avatar Nov 02 '17 22:11 noctuid

@terlar and @noctuid. which-key can easily support this if (describe-buffer-bindings (current-buffer) KEY t) showed menu-items but it doesn't (at least how they are used here). That last argument suggests that menu-items should be reported but they are not for me.

It seems to me that this is something that could be fixed in describe-buffer-bindings.

For reasons that I outlined in the other attempts to change the way which-key found bindings, I'm not interested in writing a custom describe-buffer-bindings function. It's too error prone. And this usage is too narrow in my opinion.

I provided another way to conditionally display a certain string for a bindings here, which is an alternative route.

justbur avatar Nov 06 '17 15:11 justbur

@noctuid This commit (https://github.com/justbur/emacs-which-key/commit/f516b84eab1e307d3ffaa181324dca12c3951936) takes a partial step in that direction. If it works well, I can extend it. The idea is to pre-process arguments to define-key instead of taking on the more complicated task of parsing keymaps on the fly without built-in functions.

justbur avatar Dec 13 '17 16:12 justbur

It would be nice if it was possible to dynamically retrieve the description from a (STRING . DEFN) keybinding, but this seems like the best alternative. I do think it would be nice to extend this to work for extended menu items (e.g. use the ITEM-NAME as the description).

noctuid avatar Jan 20 '18 07:01 noctuid

I was able to display keybindings to general-predicate-dispatch actions with the following workaround:

  1. Specify :docstring in general-predicate-dispatch forms.
  2. Override the definition of which-key--get-current-bindings with the following implementation (diff):
(defun which-key--get-current-bindings (&optional prefix)
  "Generate a list of current active bindings."
  (let* ((ignore-bindings '(self-insert-command
                            ignore
                            ignore-event
                            company-ignore))
         (buffer (current-buffer))
         (active-maps (with-current-buffer buffer
                        (current-active-maps))))
    (cl-loop for entry in (cl-remove-duplicates
                           (thread-last
                               (if prefix
                                   (mapcar (lambda (map) (lookup-key map prefix))
                                           active-maps)
                                 active-maps)
                             (cl-remove-if-not #'keymapp)
                             (mapcar (lambda (map)
                                       (when (eq map 'mode-specific-command-prefix)
                                         (setq map (buffer-local-value 'mode-specific-map
                                                                       buffer)))
                                       (cl-etypecase map
                                         (list (cdr map))
                                         (symbol (symbol-value map)))))
                             (apply #'append))
                           :from-end t
                           ;; Prevent an error (wrong-type-argument listp keymap)
                           :key #'car-safe
                           :test #'eql)
             with bindings = nil
             do (pcase entry
                  (`(,key menu-item . ,details)
                   (push (cons (key-description (cl-etypecase key
                                                  (number (vector key))
                                                  (vector key)))
                               (or (car-safe details)
                                   "menu-item"))
                         bindings))
                  ((and `(,key . ,def)
                        (guard (not (memq def ignore-bindings))))
                   (push (cons (key-description (cl-etypecase key
                                                  (vector key)
                                                  (otherwise (vector key))))
                               (cond
                                ((keymapp def) "Prefix Command")
                                ((symbolp def) (copy-sequence (symbol-name def)))
                                ((eq 'lambda (car-safe def)) "lambda")
                                ((eq 'menu-item (car-safe def)) "menu-item")
                                ((stringp def) def)
                                ((vectorp def) (key-description def))
                                (t "unknown")))
                         bindings)))
             finally return bindings)))

The problem was that which-key (in which-key--get-current-bindings) uses describe-buffer-bindings to obtain keybindings, and the function doesn't generate keybindings to menu items. It accepts a third argument MENUS, but even if the option is set to non-nil, menu items generated by general-predicate-dispatch aren't contained in the result. I seem to have resolved the issue by reimplementing which-key--get-current-bindings by looking up the given prefix key in all active maps.

I am not sure if this is the right solution. There may be issues I am unaware of, so I want you to test if it produces expected results.

You don't have to turn on which-key-enable-extended-define-key. general-predicate-dispatch doesn't seem to work when the option is turned on. This must be an issue with which-key itself, and not with your package.

akirak avatar Dec 28 '19 17:12 akirak

Sorry, it seems to need a lot of workarounds to make it work, so my solution is unrealistic. @terlar was right. Please ignore my comment above.

akirak avatar Jan 01 '20 05:01 akirak