emacs-which-key icon indicating copy to clipboard operation
emacs-which-key copied to clipboard

Support displaying menu-items

Open terlar opened this issue 7 years ago • 16 comments

It would be useful if which-key could support displaying menu-items. I am not very familiar with menu-items but it was used by general.el for example.

This came up in this issue (for reference): https://github.com/noctuid/general.el/issues/79

Sorry for the lack of detail, but I am not very familiar with the subject.

I have tried to work-around this by manually adding a replacement for the keys bound to a menu-item, but it doesn't work, the keys are currently omitted from the which-key display.

terlar avatar Nov 03 '17 14:11 terlar

This commit (f516b84eab1e307d3ffaa181324dca12c3951936) takes a partial step in that direction. If it works well, I can extend it.

justbur avatar Dec 13 '17 16:12 justbur

I have the same issue with @terlar , this is my general configuration:

	    (general-define-key
	     "C-c b" (general-predicate-dispatch 'helm-projectile-ag
		       (samray/does-use-ivy) 'samray/counsel-ag-symbol-at-point)
	     )

and this is which-key configuration:

(use-package which-key
  :ensure t
  :diminish which-key-mode
  :init (progn
	  (setq which-key-idle-delay 0.3)
	  (setq which-key-enable-extended-define-key t)
	  )
  :config(progn
	   (which-key-mode t)
	   ))

But I could not find C-c b in which-key menu, am I doing somethings wrong ?

ramsayleung avatar Sep 05 '18 02:09 ramsayleung

I am looking for this feature too: See https://endlessparentheses.com/define-context-aware-keys-in-emacs.html

The menu items allow creating conditional bindings. Also, it will be great if which-key extracts and shows the text from the menu item.

Here it is an example from lsp-mode codebase:

(defmacro lsp-define-conditional-key (keymap label key def
                                             &rest body)
  "In KEYMAP, define key sequence KEY as DEF conditionally.
This is like `define-key', except the definition
\"disappears\" whenever BODY evaluates to nil."
  (declare (indent 3)
           (debug (form form form &rest sexp)))
  `(define-key ,keymap ,key
     '(menu-item
       ,label
       nil
       :filter (lambda (&optional _)
                 (when ,(macroexp-progn body)
                   ,def)))))

(defvar lsp-command-map
  (-doto (make-sparse-keymap)
    (lsp-define-conditional-key "Workspace Restart" "wr" #'lsp-workspace-restart (lsp-workspaces)))
  "Keymap for lsp log buffer mode.")

(define-key lsp-mode-map (kbd "C-c l") lsp-command-map)

yyoncho avatar Jan 15 '20 10:01 yyoncho

I did some research and it looks like the menu-item support works fine but the :filter support is not in place:

(define-key global-map (kbd "C-c C-l r l") '(menu-item "Label for foo-bar" :filter (lambda (&optional _)
                                                                       #'foo-bar)))

(define-key global-map (kbd "C-c C-l r r") '(menu-item "Label for foo-bar" foo-bar))

yyoncho avatar Jan 15 '20 10:01 yyoncho

Please ignore 2 previous comments, it was actually an issue with my code, the correct way to define menu-item is like that:

(define-key global-map (kbd "C-c C-l r X")
  `(menu-item "2" switch-to-buffer
              :filter ,(lambda (cmd) cmd)))

and which key works fine with which-key.

yyoncho avatar Jan 15 '20 17:01 yyoncho

It only works if you specify REAL-BINDING and return it. This won't appear, for example:

(define-key global-map (kbd "C-c C-l r X")
  `(menu-item "description" nil
              :filter ,(lambda (_) 'foo)))

noctuid avatar Feb 01 '20 17:02 noctuid

@noctuid I think that you are supposed to return a menu-item from the filter function

yyoncho avatar Feb 01 '20 18:02 yyoncho

Filter returns a binding to use instead of REAL-BINDING. Returning any command works. Not sure what you mean by returning a menu-item from the filter function; literally returning another menu-item gives an error. Could you elaborate?

noctuid avatar Feb 01 '20 21:02 noctuid

What is missing to implement the use case described by @noctuid? It would be great if that would work. Is it possible that which-key shows the description?

(define-key global-map (kbd "C-c C-l r X")
  `(menu-item "description" nil
              :filter ,(lambda (_) 'foo)))

minad avatar Dec 14 '20 18:12 minad

which-key depends on the internal function describe-buffer-bindings to find keys for the current prefix. This is fast and reliable but is not aware of menu-item filters. Exposing them to emacs would either require patching describe-buffer-bindings or writing a custom replacement for which-key. I haven't wanted to do the latter because it seems error prone and slower.

It's also worth mentioning that there are other more standard ways of giving context to a key binding. In the example in https://github.com/justbur/emacs-which-key/issues/177#issuecomment-418579944, it seems you only want to condition the binding on ivy being active. Is this not easier to accomplish by placing the binding in the ivy-mode-map?

justbur avatar Dec 15 '20 18:12 justbur

I understand that it is due to the restriction of describe-buffer-bindings. Would still be cool if there is additional support on top.

My use case is a bit special: I am generating a dynamic keymap on the fly, which shows some actions in the minibuffer. See here https://github.com/minad/consult/blob/ccbe9a91a264f8e7517e6096212874d66cc8e070/consult.el#L393. I am using menu-items for that and normal function bindings. The thing is that the function name is not meaningful, since multiple keys use the same function. If I could simply use menu-items for the descriptions, I would be done. I know that I can also configure which-key externally via the which key replacements, but in that case it would not work well because the descriptions I would like to show are dynamically computed when the keymap is being generated.

These days it seems that which-key is used very often to generate these dynamic menus automatically, e.g. hercules or embark uses which-key to show their bindings. These are similar to my use case and could probably be enhanced if which-key would allow them to show some descriptions additionally to the keys.

Maybe you know a better way or I am just expecting too much from which-key? Which-key is already doing a great job and works well in many situations from what I can tell. I think I said that before - but it is the most important discoverability/reminder tool for me to enhance the basic Emacs experience :)

minad avatar Dec 15 '20 18:12 minad

I think making describe-buffer-bindings aware of menu-item filters might be worth exploring. Which-key is not the only package that uses describe-buffer-bindings, so this may have beneficial effects for others as well.

justbur avatar Dec 15 '20 18:12 justbur

I think making describe-buffer-bindings aware of menu-item filters might be worth exploring. Which-key is not the only package that uses describe-buffer-bindings, so this may have beneficial effects for others as well.

I agree. In the long-term that is the best way forward - the downside is only that it will take a long time until the user see the benefits.

There is still the which-key keymap based replacement infrastructure, maybe that could be leveraged to provide better descriptions for dynamically generated keymaps? I tried that quickly but it did not work, maybe I made some mistakes.

minad avatar Dec 15 '20 19:12 minad

There is still the which-key keymap based replacement infrastructure, maybe that could be leveraged to provide better descriptions for dynamically generated keymaps? I tried that quickly but it did not work, maybe I made some mistakes.

Yes, which-key can handle dynamic replacements. Maybe general can handle inserting the proper entry to make that happen?

justbur avatar Dec 15 '20 19:12 justbur

@justbur

What I wrote before:

There is still the which-key keymap based replacement infrastructure, maybe that could be leveraged to provide better descriptions for dynamically generated keymaps? I tried that quickly but it did not work, maybe I made some mistakes.

I think I actually got this to work. I am experimenting right now.

minad avatar Dec 15 '20 19:12 minad

@justbur I got it to work by using the keymap based replacements! See https://github.com/minad/consult/blob/7cd2d40c131333d38787227cd62502cc6e991bea/consult.el#L393. I am pretty happy with this now and it works well. The only downside is that it is dependent on the internal which-key format and that it is not using a "more general" mechanism. If you have a better idea, please let me know!

Maybe one thing - regarding the internal keymap format used for the keymap based replacements, would it make sense to to change the keys to the following: (vconcat prefix-keys [which-key] (vector last-key)? I think that is a bit simpler than what you are doing right now with the key-description, formatting, interning...

minad avatar Dec 15 '20 19:12 minad