clojure-mode icon indicating copy to clipboard operation
clojure-mode copied to clipboard

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

Open yyoncho opened this issue 3 years ago • 3 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 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

yyoncho avatar Mar 08 '21 18:03 yyoncho

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)

yuhan0 avatar May 02 '21 06:05 yuhan0

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 avatar May 02 '21 07:05 yyoncho

It reproduces on both emacs 27.2 and emacs 27.1

yyoncho avatar May 02 '21 07:05 yyoncho

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.

prestancedesign avatar Feb 20 '23 19:02 prestancedesign

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.

vemv avatar Feb 23 '23 14:02 vemv

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

practicalli-johnny avatar Mar 13 '23 16:03 practicalli-johnny

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

OknoLombarda avatar May 27 '23 08:05 OknoLombarda