oantolin / embark

Emacs Mini-Buffer Actions Rooted in Keymaps
GNU General Public License v3.0
952 stars 57 forks source link

Lisp nesting exceeds ‘max-lisp-eval-depth’ error when using embark-dwim in clojure-mode #732

Open deejayem opened 2 months ago

deejayem commented 2 months ago

I get a Lisp nesting exceeds ‘max-lisp-eval-depth’ error when I call embark-dwim in clojure-mode, and nothing else happens. xref-find-definitions works fine, and embark-dwim used to work (I'm not sure how recently it broke, as something had gone wrong in my config, and M-. had reverted to xref-find-definitions).

Here's a minimal config that reproduces the error (but doesn't make xref-find-definitions work; either cider or lsp-mode/eglot are needed for that):

(require 'package)
(setq package-enable-at-startup nil)
(add-to-list 'package-archives '("melpa" . "https://melpa.org/packages/"))
(unless package--initialized (package-initialize))
(unless (package-installed-p 'use-package)
  (package-refresh-contents)
  (package-install 'use-package))
(eval-when-compile
  (require 'use-package))
(setq use-package-always-ensure t)

(use-package embark
  :demand t
  :bind  ([remap xref-find-definitions] . embark-dwim))

(use-package clojure-mode
  :demand t)

Backtraces: M-.: https://gist.github.com/deejayem/1e5d91f2bdfca48e758e30e0966a9898 M-x embark-dwim: https://gist.github.com/deejayem/b147966b8275f8fa19f36cbfaa11fed9

I don't understand why the backtrace is different, as it's calling the same command either way.

If it matters, I've seen this on both 29.1 (macos) and 29.4 (linux).

deejayem commented 2 months ago

I've figured out what's going on here.

https://github.com/oantolin/embark/commit/d369bb6131ed156a06168f57cb11a5faedf94c23 combined with using :bind ([remap xref-find-definitions] . embark-dwim) causes this to happen.

In my config, I've switched back to ("M-." . embark-dwim), and for whatever reason the issue I was having with it still being bound to xref-find-definitions has gone away, so things are working again for me.

Possibly this can just be closed now, if working with remap isn't a priority.

oantolin commented 2 months ago

Wow, thanks for figuring it out! I was scratching my head about this.

I'll try to make the code work under remapping, but no promises.