Skip to content

Global keybinding C-c m prefix contravenes key binding conventions #26

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

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

Conversation

fuzzbomb
Copy link
Contributor

The changes in #21 brought some nice flexibility for the key bindings.

However, the ready-player-minor-mode-map-prefix default value of C-c m doesn't follow Emacs Key Binding Conventions. Sequences consisting of C-c and a letter are reserved for users.

I appreciate that ready-player-minor-mode-map-prefix can be customized, but it's probably a good idea to change it to something which follows the convention. C-c followed by punctuation is allocated for minor modes.

I looked at other well-known minor mode keymaps to see what they used. Some notable examples:

  • Flycheck uses C-c !. (Mnemonic: Warnings.)
  • Outline uses C-c @. (Mnemonic: at, as in location. The prefix has navigation and structural editing commands.)

My suggestion is C-c #. As a mnemonic: it resembles a sharp note in music notation, for playing audio.

@fuzzbomb fuzzbomb changed the title Change default value of ready-player-minor-mode-map-prefix Global keybinding C-c m prefix contravenes key binding conventions] Jan 15, 2025
@fuzzbomb fuzzbomb changed the title Global keybinding C-c m prefix contravenes key binding conventions] Global keybinding C-c m prefix contravenes key binding conventions Jan 15, 2025
@xenodium
Copy link
Owner

Hi @fuzzbomb. Thanks for the thoughtful PR!

The proposed # binding has a lovely mnemonic. Requiring an additional modifier, however, is less accessible than m. Try advancing a few tracks globally.

On a different note. ready-player has existing users now accustomed to the default binding. While not a huge deal to migrate, I'd prefer keeping the more accessible default as we already have a good compromise that enables customizing (via ready-player-minor-mode-map-prefix) without much difficulty.

@fuzzbomb
Copy link
Contributor Author

Right you are, then. I suppose it's a breaking change of sorts.

In which case, if you want to keep the current C-c m prefix, then perhaps the README could acknowledge that it doesn't follow the conventions?

It would be worth highlighting the ready-player-minor-mode-map-prefix option in the "Global key bindings" heading. As currently worded, it seems like it's C-c m, or disable it. The prefix option is doesn't get mentioned until much later.

I'll gladly make a PR for that, if you like.

Requiring an additional modifier, however, is less accessible

Heh, that GNU bolted years ago 😃 🦬. It depends on your keyboard, of course; several layouts have # unshifted.

@xenodium
Copy link
Owner

Wonderful. Thanks for that. If you'd like to send a PR for README that would be great.

@xenodium
Copy link
Owner

Lemme know when you'd like me to take another look at PR changes.

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