clojure-emacs / clojure-mode

Emacs support for the Clojure(Script) programming language
914 stars 246 forks source link

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

Closed yyoncho closed 1 year ago

yyoncho commented 3 years ago

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 https://github.com/emacs-lsp/lsp-mode/issues/2698

Environment & Version information

clojure-mode version

Latest from melpa:

;; Package-Version: 20210307.824 ;; Package-Commit: b5c913af12ab3f55723fa3f57d373a4984655e8a

Emacs version

Emacs 27.1

Operating system

Linux

yuhan0 commented 3 years ago

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)

yyoncho commented 3 years ago

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
yyoncho commented 3 years ago

It reproduces on both emacs 27.2 and emacs 27.1

prestancedesign commented 1 year ago

I Just got the same bug suddenly.

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

Trying https://github.com/clojure-emacs/clojure-mode/issues/586#issuecomment-830762217 procedure and Emacs stuck with high CPU charge on Emacs 28.1 like for @yyoncho.

vemv commented 1 year ago

In case it helps anyone, https://github.com/clojure-emacs/clojure-mode/pull/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.

practicalli-johnny commented 1 year ago

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: https://github.com/clojure-emacs/clojure-mode/issues/639#issuecomment-1466500943

OknoLombarda commented 1 year ago

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