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

Switch to new prefix declaration using which-key #2876

Closed
wants to merge 2 commits into from
Closed

Switch to new prefix declaration using which-key #2876

wants to merge 2 commits into from

Conversation

justbur
Copy link
Contributor

@justbur justbur commented Sep 3, 2015

This supports both the old method and the new method for now. People will have to update their version of which-key to get access to the function which-key-declare-prefixes

Fixes #2839 among others. We still need to actually go through and declare the missing prefixes now

@justbur justbur mentioned this pull request Sep 3, 2015
This was referenced Sep 4, 2015
@syl20bnr
Copy link
Owner

syl20bnr commented Sep 7, 2015

Excellent job, this is a way better solution than what I expected when I opened the issue.
Thank you again for another great contribution 👍
Cherry-picked into develop branch, you can safely delete your branch.

@syl20bnr syl20bnr closed this Sep 7, 2015
#+begin_src emacs-lisp
(spacemacs/declare-prefix "]" "bracket-prefix")
(evil-leader/set-key "]]" 'double-bracket-command)
#+end_src
Copy link
Contributor

Choose a reason for hiding this comment

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

When I read this I found it a bit confusing. When it says "keymaps can be nested", I thought that I can have a keymap that maps a to something and b to something, and then I can take that keymap and nest it behind SPC whatever, and the result would be bindings for SPC whatever a and SPC whatever b.

Later you use "nesting bindings". Maybe that way of putting it should be used everywhere, or maybe we can just talk about common prefixes and how command execution doesn't happen until a complete key sequence has been input.

Anyway, it was a very good read!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think it's a little awkward as it is. Is this better?

Finally, one should be aware of prefix keys, which are just key bindings that point to another keymap. Essentially, all keymaps can be nested, and nested keymaps are used extensively in spacemacs (and in vanilla Emacs for that matter). For example, SPC a points to a set of key bindings for "applications", like SPC ac for =calc-dispatch=. Internally, SPC a is simply bound to another keymap, making it a prefix key. Creating prefix keys is easy.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm. Still doesn't feel quite right. It says "Creating prefix keys is easy.". I'd say it's so easy that you're always just doing that implicitly, and never explicitly. You're not going "I'm going to create a prefix key." and write some code. You just create a key binding for more than one key. The focus is still the function the key sequence is bound to. The prefix just works automatically.

Maybe the name spacemacs/declare-prefix makes it sound a little "I first need to declare this thing, then I can use it." – like a variable in certain programming languages. But in fact declare-prefix just sets a which-key name for the prefix that helps documentation. The prefix itself works automatically, and you don't have to declare it.

"nesting keymaps" or "nesting bindings" sounds like this to me:

(keymap/prefix "]" "brackety things"
  ("]" 'double-bracket-command
   ")" 'bracket-paren-command
   "-" 'bracket-dash-command))

But that's not what it is. Or am I missing something about prefixes?

Maybe something like this (if you use it, feel free to change at will up to the point where it's unrecognizable :)):

"You can also bind commands to sequences of keys. This is used extensively in spacemacs to group related key bindings together. You can define a name for any key sequence with spacemacs/declare-prefix. This name will be used in the popup window that shows possible continuations of the current key sequence."

I actually just looked at spacemacs/declare-prefix, and I think it does a lot more than setting a name for the prefix to be used by which-key. I'm new to emacs and elisp. It calls define-prefix-command. Hmm, does a key sequence binding work only if all prefixes in the binding have been defined with define-prefix-command? Maybe the problem here is that I don't really understand the thing that is being documented :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mbertheau Why don't you open up a pr for this, so we can discuss it there? I'm happy to provide more explanation in the docs about what's going on. I just wasn't sure where to draw the line so to speak.

You're right that declaring the prefix only serves to name the prefix and is unnecessary for the functionality. That's why I was saying it's easy.

spacemacs/declare-prefix was recently changed to support the new way of naming prefixes, which is entirely done through which-key, but it also supports the old way, which I hope to phase out eventually. The old way actually renamed the keymap's symbol (which is why the name had to be unique, which was causing problems, etc) and was more of a hack to be honest. The new way just tells which-key that this key binding is a prefix and declares a name for it. Again, I'm happy to provide more detail, and please feel free to rework the explanation in a new pr since this has been closed already.

Editted for typo

@justbur justbur deleted the declare-prefixes branch November 15, 2015 20:20
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