Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
yyoncho opened this issue Mar 8, 2021 · 7 comments · Fixed by #651
Labels

Comments

@yyoncho
Copy link

yyoncho commented Mar 8, 2021

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

@bbatsov bbatsov added the bug label Mar 8, 2021
@yuhan0
Copy link
Contributor

yuhan0 commented May 2, 2021

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
Copy link
Author

yyoncho commented May 2, 2021

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
Copy link
Author

yyoncho commented May 2, 2021

It reproduces on both emacs 27.2 and emacs 27.1

@prestancedesign
Copy link

prestancedesign commented Feb 20, 2023

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
Copy link
Member

vemv commented Feb 23, 2023

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.

@practicalli-johnny
Copy link

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)

@OknoLombarda
Copy link
Contributor

OknoLombarda commented May 27, 2023

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 pushed a commit to OknoLombarda/clojure-mode that referenced this issue May 28, 2023
OknoLombarda pushed a commit to OknoLombarda/clojure-mode that referenced this issue May 29, 2023
OknoLombarda pushed a commit to OknoLombarda/clojure-mode that referenced this issue Jun 8, 2023
OknoLombarda pushed a commit to OknoLombarda/clojure-mode that referenced this issue Jun 8, 2023
@vemv vemv closed this as completed in #651 Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants