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

Added clarification to evil-define-key for escaping modifier keys #1856

Merged
merged 2 commits into from
Jan 11, 2024

Conversation

Naokotani
Copy link
Contributor

It is useful to have clarification in the doc string for evil-define-key that it is necessary to escape modifiers keys, such as "C" and "M", for users that are accustomed to using functions like global-set-key or keymap-global-set where this is not necessary. This is also not made particularly clear in the define-key function itself, which is a legacy function.

It is useful to have clarification in the doc string for
evil-define-key that it is necessary to escape modifiers keys for
users that are accustomed to using functions like global-set-key or
keymap-global-set where this is not necessary. This is also not made
particularly clear in the define-key function itself, which is a
legacy function.
Copy link
Collaborator

@axelf4 axelf4 left a comment

Choose a reason for hiding this comment

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

This is a good idea, but there are some things that I think could be reworded:

  • Users should just use kbd instead of typing out the key sequence representations by hand, unless they already know what they are doing.

  • That the macro uses define-key is an implementation detail, could instead say roughly something like this:

    KEY is an internal Emacs representation of a key, as for define-key. To bind key sequences that use modifier keys such as ..., convert the key sequences using kbd.

  • As C/M/\ are not Lisp symbols they should be quoted with double quotes instead of `...'.

@tomdl89 tomdl89 merged commit 59774e3 into emacs-evil:master Jan 11, 2024
4 checks passed
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.

3 participants