Skip to content
This repository has been archived by the owner on Jun 25, 2024. It is now read-only.

Support displaying menu-items #177

Open
terlar opened this issue Nov 3, 2017 · 16 comments
Open

Support displaying menu-items #177

terlar opened this issue Nov 3, 2017 · 16 comments

Comments

@terlar
Copy link

terlar commented Nov 3, 2017

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):
noctuid/general.el#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.

@justbur
Copy link
Owner

justbur commented Dec 13, 2017

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

@ramsayleung
Copy link

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 ?

@yyoncho
Copy link

yyoncho commented Jan 15, 2020

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
Copy link

yyoncho commented Jan 15, 2020

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
Copy link

yyoncho commented Jan 15, 2020

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.

@noctuid
Copy link

noctuid commented Feb 1, 2020

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)))

@yyoncho
Copy link

yyoncho commented Feb 1, 2020

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

@noctuid
Copy link

noctuid commented Feb 1, 2020

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?

@minad
Copy link
Contributor

minad commented Dec 14, 2020

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)))

@justbur
Copy link
Owner

justbur commented Dec 15, 2020

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 #177 (comment), 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?

@minad
Copy link
Contributor

minad commented Dec 15, 2020

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 :)

@justbur
Copy link
Owner

justbur commented Dec 15, 2020

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.

@minad
Copy link
Contributor

minad commented Dec 15, 2020

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.

@justbur
Copy link
Owner

justbur commented Dec 15, 2020

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?

@minad
Copy link
Contributor

minad commented Dec 15, 2020

@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
Copy link
Contributor

minad commented Dec 15, 2020

@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...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants