search

emacs-evil / evil-magit

Black magic or evil keys for magit
https://github.com/justbur/evil-magit
GNU General Public License v3.0
273 stars 16 forks source link

Symbol's value as variable is void: binding #9

Closed cohomology closed 8 years ago

cohomology commented 8 years ago

Bugus commit: d45808c1f8cd327be4464685a8256bc3f08e10d6

After updating to the above mentioned commit, I can not start emacs anymore. Even --debug-init does not help.

As this hits all melpa users, and it is unknown so far, why you are not affected by this, I suggest reverting melpa to a previous state.

The error message from emacs is given in the title of this issue report.

If you can't reproduce this with your .emacs.d, you can do:

git clone https://github.com/cohomology/.emacs.d .emacs.d
cd .emacs.d
git submodule update --init --recursive
cd third-party/evil-magit
git pull

(After the version from melpa doesn't seem to work anymore, I stopped using evil-magit from melpa and took an old version as a git submodule in my .emacs.d).

Best regards, Kilian.

justbur commented 8 years ago

It looks like a error with the compiler that didn't show up in my version of emacs. I'm pushing a fix now, but you can probably delete evil-magit.elc for a quick workaround

justbur commented 8 years ago

ok, I just pushed what I think should fix this. Sorry for the mistake and thanks for the report

jschaf commented 8 years ago

That commit just broke it for me. Reverting the commit fixes the issue.

Debugger entered--Lisp error: (void-function keymap)

I'm pretty sure the issue is you're not quoting STATE or DEF and you're passing an already expanded keymap. So the eval is seeing:

(evil-define-key normal [RAW KEYMAP DEFINITION] "k" evil-previous-visual-line)

instead of:

(evil-define-key 'normal magit-blame-mode-map "k" 'evil-previous-visual-line)

This code seems to work, I added a quote to (nth 0 binding) and (nth 3 binding) and removed the symbol-value call.

(dolist (binding evil-magit-mode-map-bindings)
    (eval
     `(evil-define-key ',(nth 0 binding) ,(nth 1 binding)
        ,(nth 2 binding) ',(nth 3 binding))))

Additionally, The selectively evaling and quoting forms seems very suspect. Are you sure the original version was causing the error?

cohomology commented 8 years ago

This bug is certainly no optimizer issue, as I do not compile evil-magit. Consequently, there are no .elc files. After your patch, I get the same error as @jschaf. I use emacs 24.5.1.

Please revert melpa to 7c9f63341012fc1c4c9d3b78802b1935f6898869, which is the last working commit.

justbur commented 8 years ago

@cohomology It's reverted

@jschaf

I'm pretty sure the issue is you're not quoting STATE or DEF and you're passing an already expanded keymap. So the eval is seeing:

Right, so much for a quick fix

Additionally, The selectively evaling and quoting forms seems very suspect. Are you sure the original version was causing the error?

Agreed. I'll set this up differently

Sorry for the trouble @jschaf and @cohomology. There must have been something wrong with my environment that wasn't showing the error on my side. I do compile the files. It was probably a stupid mistake somewhere.

justbur commented 8 years ago

@cohomology I rewrote the logic for setting the main keys and pushed to the new-define-key branch. Before I merge this, would you be able to test the changes on your side to see if it fixes the original issue? FYI this branch also includes all of the other stuff I have done recently (setting up a test environment for example).

cohomology commented 8 years ago

@justbur I now get a (Wrong-type-argument keymapp nil) inside evil-magit-define-key.

justbur commented 8 years ago

Is that on the new branch? On Wed, Dec 9, 2015 at 7:21 AM Kilian Kilger notifications@github.com wrote:

@justbur https://github.com/justbur I now get a (Wrong-type-argument keymapp nil) inside evil-magit-define-key.

— Reply to this email directly or view it on GitHub https://github.com/justbur/evil-magit/issues/9#issuecomment-163210103.

justbur commented 8 years ago

@cohomology oh right it must be. That is probably the root cause of the original issue, because it would have triggered evil-define-key to delay the setting of the key. Can you provide a backtrace from that error? Are you up to date with magit?

justbur commented 8 years ago

@cohomology I'm confident that the new branch is correct now. I added a check for an uninitialized keymap, which I believe is the cause of the problem you were having. When you use the new branch with the latest commit, I believe you will see a message about a keymap that is not bound. That should help track down the problem.

cohomology commented 8 years ago

As you suspected, I now get the following in my messages window:

evil-magit: Error the keymap nil is not bound. Note that evil-magit assumes 
you have the latest version of magit.

What to do now? There is no debugger opening via --debug-init. I have Magit 20151208.744 from Melpa unstable (from yesterday!). Kilian.

justbur commented 8 years ago

@cohomology well the nil was unexpected actually :-). Can you paste the value you get from C-h v evil-magit-mode-map-bindings RET?

justbur commented 8 years ago

Nevermind. I see the problem. You probably don't use C-u for scrolling. The latest commit should handle this. That was dumb on my part

cohomology commented 8 years ago 
((motion magit-mode-map "g")
 (motion magit-mode-map "\n" magit-section-forward "n")
 (motion magit-mode-map "gj" magit-section-forward-sibling "\356")
 (motion magit-mode-map "]" magit-section-forward-sibling "\356")
 (motion magit-mode-map "" magit-section-backward "p")
 (motion magit-mode-map "gk" magit-section-backward-sibling "\360")
 (motion magit-mode-map "[" magit-section-backward-sibling "\360")
 (motion magit-mode-map "gr" magit-refresh "g")
 (motion magit-mode-map "gR" magit-refresh-all "G")
 (motion magit-mode-map "x" magit-delete-thing "k")
 (motion magit-mode-map "X" magit-file-untrack "K")
 (motion magit-mode-map "o" magit-revert-no-commit "v")
 (motion magit-mode-map "O" magit-revert-popup "V")
 (motion magit-mode-map "" magit-reset "x")
 (motion magit-mode-map "|" magit-git-command ":")
 (motion magit-mode-map ">" magit-submodule-popup "o")
 (motion magit-mode-map "j" evil-next-visual-line)
 (motion magit-mode-map "k" evil-previous-visual-line)
 (motion magit-mode-map "v" set-mark-command)
 (motion magit-mode-map "V" set-mark-command)
 (motion magit-mode-map "gg" evil-goto-first-line)
 (motion magit-mode-map "G" evil-goto-line)
 (motion magit-mode-map "" evil-scroll-down)
 (motion magit-mode-map "" evil-scroll-page-down)
 (motion magit-mode-map "" evil-scroll-page-up)
 (motion magit-mode-map ":" evil-ex)
 (motion magit-mode-map "/" evil-search-forward)
 (motion magit-mode-map "n" evil-search-next)
 (motion magit-mode-map "N" evil-search-previous)
 nil
 (motion magit-mode-map "" evil-emacs-state)
 (motion magit-mode-map
         [escape]
evil-magit-maybe-deactivate-mark)
 (motion magit-status-mode-map "gz" magit-jump-to-stashes "jz")
 (motion magit-status-mode-map "gt" magit-jump-to-tracked "jt")
 (motion magit-status-mode-map "gn" magit-jump-to-untracked "jn")
 (motion magit-status-mode-map "gu" magit-jump-to-unstaged "ju")
 (motion magit-status-mode-map "gs" magit-jump-to-staged "js")
 (motion magit-status-mode-map "gfu" magit-jump-to-unpulled-from-upstream "jfu")
 (motion magit-status-mode-map "gfp" magit-jump-to-unpulled-from-pushremote "jfp")
 (motion magit-status-mode-map "gpu" magit-jump-to-unpushed-to-upstream "jpu")
 (motion magit-status-mode-map "gpp" magit-jump-to-unpushed-to-pushremote "jpp")
 (motion magit-blob-mode-map "gj" magit-blob-next "n")
 (motion magit-blob-mode-map "gk" magit-blob-previous "p")
 (motion magit-diff-mode-map "gj" magit-section-forward)
 (motion magit-diff-mode-map "gd" magit-jump-to-diffstat-or-diff "j")
 (normal magit-blame-mode-map "j" evil-next-visual-line)
 (normal magit-blame-mode-map "\n" magit-blame-next-chunk "n")
 (normal magit-blame-mode-map "gj" magit-blame-next-chunk "n")
 (normal magit-blame-mode-map "gJ" magit-blame-next-chunk-same-commit "N")
 (normal magit-blame-mode-map "k" evil-previous-visual-line)
 (normal magit-blame-mode-map "" magit-blame-previous-chunk "p")
 (normal magit-blame-mode-map "gk" magit-blame-previous-chunk "p")
 (normal magit-blame-mode-map "gK" magit-blame-previous-chunk-same-commit "P")
 (motion git-commit-mode-map "gk" git-commit-prev-message "\360")
 (motion git-commit-mode-map "gj" git-commit-next-message "\356"))
justbur commented 8 years ago

Yep, that's it. Everything should work for you now on that branch. I appreciate the quick responses. Thank you

cohomology commented 8 years ago

Yeah, it works now.

justbur commented 8 years ago

Great. I'm going to merge then.