noctuid / general.el

More convenient key definitions in emacs
GNU General Public License v3.0
992 stars 43 forks source link

:prefix-command clears existing keymaps on first mention #166

Open guraltsev opened 5 years ago

guraltsev commented 5 years ago

The per-definition key :prefix-command seems to clear out the keymap if it is defined already.

In particular execute the following two commands in a clean session (that doesn't have test2-map defined)

(general-define-key :prefix-map 'test2-map
                    "t" '(lambda () "" (interactive) (message "test2-map key t")))
(general-define-key :keymaps 'global
                    "M-<up>" '(:prefix-command test2-map))

You can also see that after the first command test2-map has the correct definition in it while after the second command the value of test2-map is empty.

This is quite unexpected behavior since the documentation doesn't mention that :prefix-command resets the value of anything.

Using :keymap works well:

(general-define-key :prefix-map 'test2-map
                    "t" '(lambda () "" (interactive) (message "test2-map key t")))
(general-define-key :keymaps 'global
                    "M-<up>" '(:keymap test2-map))

but fails if test2-map was not already defined.

Expected behaviour:

(general-define-key :keymaps 'global
                    "M-<up>" '(:prefix-command test2-map))

Should create a keymap test2-map if it is not defined and just call it without modifying if it is already defined

noctuid commented 5 years ago

This is the expected behavior. Creating a prefix command creates a sparse keymap. General creates a new prefix command if one doesn't exist already. In your example, the prefix command doesn't exist, only a prefix keymap. You should use :prefix-command in your first general-define-key if you want to use a prefix command.

guraltsev commented 5 years ago

Thanks!

I guess now it is clear how to write things correctly for my use-case. I also figured out the option with "keymap" by a trial and error. However, I have repeatedly read the manual and I am still a little at loss for what are the specific differences between :prefix-command :keymap :prefix-map and :keymaps and where they can be used. The package works great for simplifying keybindings but for a person with medium/advanced-user level experience in emacs what exactly these guys do is a bit confusing. Do you think the manual should be updated to clear up some confusion? Lots is done via examples that I tend to just copy/paste/readapt, but I am always a bit anxious..

noctuid commented 5 years ago

Yeah, I will mention this specific case in the readme.

:keymap is not a top-level keyword and is only used in extended definitions for "autoloading" keymaps.

:keymaps is a top-level keyword to bind in existing keymaps.

:prefix-map and :prefix-command will create the keymap for you if it doesn't exist. If :prefix-map is used alone, it will just create a keymap with defvar. If :prefix-command is specified, define-prefix-command is used.

I think the documentation on these individually is detailed enough, but I can add an entry to the FAQ to quickly summarize the differences.

guraltsev commented 5 years ago

What do you mean by "autoloading"? For example, this code seems to work but is it correct or is the suggested way to do things different? This is an extract of init.el

  (straight-use-package 'projectile)
  (projectile-mode +1)

...more code.....

  (general-define-key :prefix "C-x p"
               "" '(:keymap projectile-command-map :wk "projectile")
               "B" '(projectile-ibuffer :wk "ibuffer")
               "p" '(helm-projectile :wk "helm"))

The projectile-command-map is defined by the projectile package and I would not want it overwritten

noctuid commented 5 years ago

projectile-command-map doesn't exist until projectile is loaded, so this "autoloads" projectile when using the keybinding. If the keymap doesn't exist, :keymap binds the key so that it will load the package when used, rebind the key to the keymap, and then immediately act as that keymap. See the readme for further details (extended definition section).