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

Make minibuffer keymap available #62

Closed
rgrinberg opened this issue Oct 16, 2020 · 6 comments
Closed

Make minibuffer keymap available #62

rgrinberg opened this issue Oct 16, 2020 · 6 comments

Comments

@rgrinberg
Copy link
Contributor

Hi,

I like using which-key to inspect keymaps. Is there a reason why the keymap itself has to be created on the fly? Can it be the global variable we get to customize instead of the list of bindings.

@raxod502
Copy link
Member

It's because I thought it was an elegant idea at the time. Since then, it has been pointed out by everyone else on the Internet that it's not a good idea -- but nobody has yet gone back and changed the variable to a regular keymap :)

@rgrinberg
Copy link
Contributor Author

I can send a PR if you agree that this is a good change. Do you think we can keep compat with the old variable? Or should we just delete it and replace it with a new map variable.

@raxod502
Copy link
Member

raxod502 commented Oct 17, 2020

Yes, definitely a good change, feel free. I'd like to keep backwards compatibility, but it's a little tricky in this case. Here's one way that I think could work nicely. When we are applying the keymap, we can check the value of ctrlf-mode-bindings against its default value (this is stored in the standard-value symbol property) to see if it's been customized. If so, we could use that instead of the new keymap variable, possibly printing a warning in the process.

@hrehfeld
Copy link

hrehfeld commented Jul 16, 2021

Awesome, thanks for the PR, but I have a few questions:

  • why is ctrlf-mode-bindings still present? (backwards-compat?)
  • why is there no map for minibuffer-bindings? (oh, I see the title now)
  • documentation in readme is outdated and doesn't mention the map.

@raxod502
Copy link
Member

why is ctrlf-mode-bindings still present? (backwards-compat?)

Yes, backwards compatibility.

why is there no map for minibuffer-bindings? (oh, I see the title now)

Looks like only one of the two keymaps was fixed in the linked PR, I didn't notice that before.

documentation in readme is outdated and doesn't mention the map

It's been updated in the linked PR.

@raxod502
Copy link
Member

Thanks to @dawranliou this has now been fixed in #97.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants