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

Another which-key keymap based replacements implementation #482

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

leifhelm
Copy link

I started working on implementing which-key keymap based replacements at the same time as @jgrey4296 (#481) as suggested in #193.
As I am no expert in programming with emacs lisp I choose to use which-key-add-keymap-based-replacements.
I did not correct the which-key tests yet.

@noctuid
Copy link
Owner

noctuid commented Jul 2, 2021

I will look at both your PRs next week, but if they both work, I will probably go with yours once tests are working since it is simpler.

Copy link
Owner

@noctuid noctuid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think completely replacing the old mechanism should be okay since all the regexps were auto-generated and not complicated (no functionality should be lost).

The only issue is that this needs to handle :definer 'minor-mode (should check kargs to determine whether to pass t for the next argument to general--get-keymap).

If you want to fix the minor mode issue, go ahead. I can take care of the tests and documentation.

Left to be done:

  • fix handling for evil-define-minor-mode-key
  • rewrite all tests to inspect keymaps instead
  • rewrite documentation; mention old keywords are deprecated and date which-key implemented this functionality

general.el Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants