show keybinding instead of 'menu-item' in embark-bindings

[dwa]

A "works-for-me" pull request which aims to fix #715, and seems to be in line with what is hinted at in #724.

Seems to work in my limited testing, no harm if others test it as well.

[minad]

@dwa Do you have a simple use case for testing this, something which doesn't involve large packages like lsp-mode, ideally starting with bare emacs -Q only?

[oantolin]

I second @minad's request for an easy test case.

[dwa]

So I haven't tested w/ bare emacs, but I found context-menu-mode useful. With this enabled, you can see how embark prior to these changes show in emacs-lisp-mode:

menu-item Emacs-Lisp Prefix command [...]

While w/ the updated PR:

<menu-bar> <emacs-lisp> <keymap> Prefix command [...]

I've also tested in org-mode and can't see anything wrong.

If this is not what you're after in terms of tests, would mind being a bit more specific what you want to see?

[minad]

Thanks. I think this is already helpful.

[minad]

Okay, as far as I can tell, this PR does not improve things, at least in this test case:

Before:

After:

[dwa]

@minad Thanks for testing.

The main reason for the patch is to show keybindings from lsp-mode, so a no-change/no-worse on other entries should really be a good thing here.

That said, I'm surprised to see that your "After" screenshot still show "menu-item" entries. They don't show up for me w/the PR.

[minad]

The main reason for the patch is to show keybindings from lsp-mode, so a no-change/no-worse on other entries should really be a good thing here.

Sure. But it is not a "no-change/no-worse" here. The "after" screenshot is less meaningful than the one from "before". My point is that we should not accept a change to the current logic if it is not universally better than before. Otherwise we are simply replacing the current hack with another hack.

That said, I'm surprised to see that your "After" screenshot still show "menu-item" entries.

No, idea. But I think we have to dig a bit deeper on how to handle the menu-items properly.