Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simulated keys are not shown with which-key #78

Closed
terlar opened this issue Nov 1, 2017 · 12 comments
Closed

Simulated keys are not shown with which-key #78

terlar opened this issue Nov 1, 2017 · 12 comments

Comments

@terlar
Copy link

terlar commented Nov 1, 2017

Earlier when I tried, which-key displayed the mapped simulated key help when I pressed it. Currently nothing is shown by which-key when a simulated key is used.

For example (when using projectile):
(general-define-key :keymaps '(normal visual) "SPC" (general-simulate-keys "C-c"))

  1. Press SPC (which-key shows nothing)
  2. Press p (which-key shows just p was pressed and displays the bindings for C-cp)

If you don't press another key which-key won't display any bindings for C-c.

@noctuid
Copy link
Owner

noctuid commented Nov 1, 2017

Sorry, I hadn't added documentation to the readme for this change yet. There are two ways of getting this behavior now with general's default method for general--simulate-keys (which is look up the key to try to match and run a command or match and set a keymap).

You can set which-key-show-transient-maps to t. I'm not sure that this which-key feature works correctly in all cases. It's useful in other cases but annoying in some since it works for all transient maps.

The other (recommended) way is to set the no-lookup argument (general-simulate-keys "C-c" nil nil t). I will change the default so that it won't use transient maps. The reason for the current default is because simulating keys in a way that triggers which-key breaks evil repeat/macro recording. General now fixes the issue during macro/repeat playback, so using transient maps by default isn't necessary anymore.

I'll close this after adding documentation to the readme.

@terlar
Copy link
Author

terlar commented Nov 1, 2017

Thank you that solution is working. It was vaguely hinted in the documentation, but I couldn't figure out how to get that specific behavior. So a clarification there would be greatly appreciated. Once again thank you for your work.

@yatesco
Copy link

yatesco commented Nov 2, 2017

I ran into this.

Previously, this:

(general-nvmap :prefix "SPC"
...
 "p" (general-simulate-keys
       "C-c p" t
       "Projectile"
       general-SPC-simulates-C-c-p)
...)

Showed "Projectile" if I pressed SPC. After a refresh it now fails trying to interpret "Projectile" as a symbol.

The following works, but it sets the which-key text to "general-SPC-simulates-C-c-p":

"p"  (general-simulate-keys
          "C-c p" t
           nil
           "Projectile")

Any hints (other than 'yeah, learn elisp! ;-)')

@terlar
Copy link
Author

terlar commented Nov 2, 2017

If I understand correctly that function takes KEYS &optional STATE KEYMAP NO-LOOKUP DOCSTRING NAME.

So I guess you could try to use:

(general-simulate-keys "C-c p" nil nil t "Projectile" general-SPC-simulates-C-c-p)

@noctuid
Copy link
Owner

noctuid commented Nov 2, 2017

That's correct. All general functions/macros have docstrings, so if you're in doubt, you can always C-h f funtion-name (and there is also eldoc).

@yatesco
Copy link

yatesco commented Nov 2, 2017

Thanks both, but that still shows "general-SPC-simulates-C-c-p" in which-key. I can live with that for sure.

@noctuid
Copy link
Owner

noctuid commented Nov 2, 2017

If I remember correctly, C-c p is bound to projectile-command-map. This would be preferable: "p" '(:keymap projectile-command-map :wk "projectile"). In either case, you currently can't set the which key replacement directly from general-simulate-keys.

@yatesco
Copy link

yatesco commented Nov 2, 2017

That's perfect @noctuid - thanks!

@noctuid
Copy link
Owner

noctuid commented Nov 2, 2017

I've tried to avoid any serious breaking changes, but in this case, since there have already been breaing changes to general-simulate-keys, I'm consider changing it further. If I make changes, what I'd probably do is use keyword arguments everything after KEYMAP for consistency with the other helpers (the key/predicate dispatch macros) and to make it easier to add extra arguments in the future (e.g. I may add WHICH-KEY to specify a which-key replacement like with general-key-dispatch). Any feedback on this would be appreciated.

@yatesco
Copy link

yatesco commented Nov 2, 2017

Makes sense to me.

If you are worried about breaking changes then you could also use a new function and mark the old one as deprecated (assuming emacs’ lisp has such a notion).

My vote for the new name is ‘press-this-instead’ :-).

noctuid added a commit that referenced this issue Jan 14, 2018
- add back key simulation tests
- general-simulate-keys (obsolete) -> general-simulate-key
  - uses keyword arguments instead of positional arguments
  - lookup defaults to t but only works for matched commands by
    default (re #78)
  - fix bug where simulating a command and keys with a count would
    affect the next command
- general--parse-keymap (obsolete) -> general--get-keymap
  - keymap argument is now optional
  - now supports both symbols and keymaps for the keymap argument
- general--key-lookup -> general--key-binding
  - only use for getting match; don't use to get auxiliary map
@noctuid
Copy link
Owner

noctuid commented Jan 14, 2018

I've fixed this issue in the simulate branch. I've added tests but would appreciate if someone else could also try it before I merge. All optional arguments are replaced with keyword arguments. The new macro is called general-simulate-key and the old general-simulate-keys is marked as obsolete. I'm also planning to add a :which-key keyword argument and another similar macro that I'm going to recommend using instead of general-simulate-key whenever possible.

noctuid added a commit that referenced this issue Jan 20, 2018
- add back key simulation tests
- general-simulate-keys (obsolete) -> general-simulate-key
  - uses keyword arguments instead of positional arguments
  - lookup defaults to t but only works for matched commands by
    default (re #78)
  - fix bug where simulating a command and keys with a count would
    affect the next command
- general--parse-keymap (obsolete) -> general--get-keymap
  - keymap argument is now optional
  - now supports both symbols and keymaps for the keymap argument
- general--key-lookup -> general--key-binding
  - only use for getting match; don't use to get auxiliary map
@noctuid
Copy link
Owner

noctuid commented Jan 20, 2018

The branch is now merged. general-key and general-simulate-key have replaced general-simulate-keys. Please let me know if you have any issues.

@noctuid noctuid closed this as completed Jan 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants