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.

Sign up for GitHub

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

[Fix #586] Fix emacs freezing when opening file containing "comment" #651

Merged
merged 3 commits into from
Jun 23, 2023
Merged

[Fix #586] Fix emacs freezing when opening file containing "comment" #651

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1a25e95
Fix infinite loop in clojure-sexp-starts-until-position
Jun 8, 2023
da46575
Add tests for clojure-sexp-starts-until-position
Jun 8, 2023
267afc9
Mention issue #586 in changelog
Jun 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@

* Font-lock Lein's `defproject` as a keyword.

### Bugs fixed

* [#586](https://github.com/clojure-emacs/clojure-mode/issues/586): Fix infinite loop when opening file containing `comment` with `clojure-toplevel-inside-comment-form` set to `t`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, is this specific to comment or to just having a single form in the buffer? Looking at the code it's the latter.

Copy link
Contributor Author

@OknoLombarda OknoLombarda Jun 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If function clojure-sexp-starts-until-position is called directly, then yes, it will loop forever for any single form in buffer. But this bug is triggered without user doing anything (except for opening the buffer) only when there's just comment in buffer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would seem wise to create an issue for that edge case - having a more robust building block would make bugs less likely, in a future.

Other than that it seems good to merge this PR in!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #652


## 5.16.0 (2022-12-14)

### Changes
Expand Down
7 changes: 5 additions & 2 deletions clojure-mode.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2264,8 +2264,11 @@ 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))
;; Needed to prevent infinite recursion when there's only 1 form in buffer.
(if (eq (point) (car sexp-positions))
OknoLombarda marked this conversation as resolved.
Show resolved Hide resolved
OknoLombarda marked this conversation as resolved.
Show resolved Hide resolved
(goto-char position)
(push (point) sexp-positions)
(clojure-forward-logical-sexp 1)))
(scan-error nil))
sexp-positions)))

Expand Down
18 changes: 18 additions & 0 deletions test/clojure-mode-sexp-test.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,24 @@
(goto-char (point-max))
(expect (clojure-find-ns) :to-equal expected)))))))

(describe "clojure-sexp-starts-until-position"
(it "should return starting points for forms after POINT until POSITION"
(with-clojure-buffer "(run 1) (def b 2) (slurp \"file\")\n"
(goto-char (point-min))
(expect (not (cl-set-difference '(19 9 1)
(clojure-sexp-starts-until-position (point-max)))))))

(it "should return starting point for a single form in buffer after POINT"
(with-clojure-buffer "comment\n"
OknoLombarda marked this conversation as resolved.
Show resolved Hide resolved
(goto-char (point-min))
(expect (not (cl-set-difference '(1)
(clojure-sexp-starts-until-position (point-max)))))))

(it "should return nil if POSITION is behind POINT"
(with-clojure-buffer "(run 1) (def b 2)\n"
(goto-char (point-max))
(expect (not (clojure-sexp-starts-until-position (- (point-max) 1)))))))

(provide 'clojure-mode-sexp-test)

;;; clojure-mode-sexp-test.el ends here