noctuid / general.el

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

Troubles with binding keys in eshell-mode-map. #80

Closed ogdenwebb closed 2 years ago

ogdenwebb commented 6 years ago

At first, thank you for this amazing Emacs package.
Yesterday I met an issue with eshell-mode-map bindings.

I have these mappings for eshell-mode:

(general-define-key :keymaps 'eshell-mode-map
                    :states  '(insert)       
                    "RET" 'eshell-send-input)

While Emacs startup I get the message: Eager macro-expansion failure: (wrong-type-argument keymapp nil) If I comment eshell-mode-map bindings, the problem is gone.

As you can see at the following screenshot, eshell-mode-map has nil value unless directly call eshell through M-x. screenshot-2017-11-03_16-11-53

p.s. It's two different Emacs instance without and with eshell.

noctuid commented 6 years ago

This is an eshell bug (see #32 also). Eshell starts out defining its map as nil and then only sets it to a keymap locally later. Here's the relevant excerpt from eshell:

;; these are only set to nil initially for the sake of the
;; byte-compiler, when compiling other files which `require' this one
(defvar eshell-mode nil)
(defvar eshell-mode-map nil)
...
;; FIXME: What the hell!?
(setq-local eshell-mode-map (make-sparse-keymap))
...

This is code is very wrong. I don't know why eshell still makes its map buffer-local (I'm guessing this has just never been fixed). If there is some good reason, it should be using (defvar eshell-mode-map) to satisfy the byte compiler and not (defvar eshell-mode-map nil), which is what causes this problem. Maybe defvar used to require an init value.

I don't know if this has been previously reported or warrants a new M-x report-emacs-bug, but I'll look into it.

As a workaround, you'll need to use add-hook with eshell-mode-hook to set up your keybindings if you also want to avoid the issue mentioned in #32.

Deanseo commented 4 years ago

In case you're using use-package for eshell, this issue prevents :bind from property acting also. You should use :hook alternatively as described above.

noctuid commented 2 years ago

It looks like eshell has fixed this in Emacs 29 (maybe earlier). I'm going to close this as there is nothing I can do about it.