clojure-emacs/clojure-mode

Emacs is stuck when opening file containing `comment` when clojure-toplevel-inside-comment-form is t

yyoncho opened this issue · 7 comments

Expected behavior

Emacs working fine

Actual behavior

Emacs freeze

Steps to reproduce the problem

do (setq clojure-toplevel-inside-comment-form t) then open clj file containing comment.

Originally reported at emacs-lsp/lsp-mode#2698

Environment & Version information

clojure-mode version

Latest from melpa:

;; Package-Version: 20210307.824
;; Package-Commit: b5c913a

Emacs version

Emacs 27.1

Operating system

Linux

I've had that variable set to t for the longest time and never encountered anything like this.
Do you have a specific repro case? I couldn't reproduce it with a clj file containing only:

comment

nor on many other files that contain comment forms.

Perhaps it's a lsp-mode interaction after all? (I'm on Emacs 28.0.50, not using lsp)

Do emacs -q -l plain.el foo.clj where plain.el is:

(require 'package)

(setq debug-on-error t
      no-byte-compile t
      package-archives '(("melpa" . "https://melpa.org/packages/")
                         ("gnu" . "https://elpa.gnu.org/packages/"))
      package-user-dir (expand-file-name (make-temp-name "lsp-tmp-elpa")
                                         user-emacs-directory)
      custom-file (expand-file-name "custom.el" package-user-dir))
(toggle-debug-on-quit)
(setq clojure-toplevel-inside-comment-form t)
(let* ((pkg-list '(clojure-mode)))

  (package-initialize)
  (package-refresh-contents)

  (mapc (lambda (pkg)
          (unless (package-installed-p pkg)
            (package-install pkg))
          (require pkg))
        pkg-list))

and foo.clj is

comment

It reproduces on both emacs 27.2 and emacs 27.1

I Just got the same bug suddenly.

Do emacs -q -l plain.el foo.clj where plain.el is

Trying #586 (comment) procedure and Emacs stuck with high CPU charge on Emacs 28.1 like for @yyoncho.

vemv commented

In case it helps anyone, #624 (not directly related to this issue) has a nice example of how to unit-test for a given top-level form.

If you can follow that pattern, adding comment as a test case, I guess that would be a start.

I work-around this issue by disabling lsp-eldoc-enable-hover (which disables LSP hover features)

(setq lsp-eldoc-enable-hover nil)

I actually set this as a Spacemacs lsp layer variable in Practicalli/spacemacs-configconfiguration

See related discussion:
#639 (comment)

It happens because of infinite loop in the function that collects starting points for forms in buffer that are later used for font locking. Particularly in this case, that function jumps back and forth, infinitely adding position of comment to the list. I managed to fix this by making sure the head of list is not the same as point to be added:

(defun clojure-sexp-starts-until-position (position)
  "Return the starting points for forms before POSITION.
Positions are in descending order to aide in finding the first starting
position before the current position."
  (save-excursion
    (let (sexp-positions)
      (condition-case nil
          (while (< (point) position)
            (clojure-forward-logical-sexp 1)
            (clojure-backward-logical-sexp 1)
            (if (eq (point) (car sexp-positions))
                (goto-char position)
              (push (point) sexp-positions)
              (clojure-forward-logical-sexp 1)))
        (scan-error nil))
      sexp-positions)))
diff --git a/clojure-mode.el b/clojure-mode.el
index a5f7531..22aaffd 100644
--- a/clojure-mode.el
+++ b/clojure-mode.el
@@ -2264,8 +2264,10 @@ position before the current position."
           (while (< (point) position)
             (clojure-forward-logical-sexp 1)
             (clojure-backward-logical-sexp 1)
-            (push (point) sexp-positions)
-            (clojure-forward-logical-sexp 1))
+            (if (eq (point) (car sexp-positions))
+                (goto-char position)
+              (push (point) sexp-positions)
+              (clojure-forward-logical-sexp 1)))
         (scan-error nil))
       sexp-positions)))

I'll send PR later today or tomorrow if my fix is good enough