Skip to content

Restore cider--display-interactive-eval-result overlays for compilation errors #3492

New issue

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

#

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

Already on GitHub? # to your account

Merged
merged 3 commits into from
Sep 30, 2023
Merged

Restore cider--display-interactive-eval-result overlays for compilation errors #3492

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
8b12a0f
Restore `cider--display-interactive-eval-result` overlays for compila…
vemv Sep 29, 2023
ce4ddf2
Refine behavior
vemv Sep 29, 2023
e4dc68e
Rename
vemv Sep 30, 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: 2 additions & 2 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@
- [#3331](https://github.com/clojure-emacs/cider/issues/3331): `cider-eval`: never jump to spurious locations, as sometimes conveyed by nREPL.
- [#3112](https://github.com/clojure-emacs/cider/issues/3112): Fix the CIDER `xref-find-references` backend to return correct filenames.
- [#3393](https://github.com/clojure-emacs/cider/issues/3393): recompute namespace info on each shadow-cljs recompilation or evaluation.
- [#3402](https://github.com/clojure-emacs/cider/issues/3402): fix `cider-format-connection-params` edge case for Emacs 29.
- [#3402](https://github.com/clojure-emacs/cider/issues/3402): Fix `cider-format-connection-params` edge case for Emacs 29.
- [#3393](https://github.com/clojure-emacs/cider/issues/3393): Recompute namespace info on each shadow-cljs recompilation or evaluation.
- Recompute namespace info on each fighweel-main recompilation.
- [#3250](https://github.com/clojure-emacs/cider/issues/3250): don't lose the CIDER session over TRAMP files.
- [#3250](https://github.com/clojure-emacs/cider/issues/3250): Don't lose the CIDER session over TRAMP files.
- [#3413](https://github.com/clojure-emacs/cider/issues/3413): Make jump-to-definition work in projects needing `cider-path-translations` (i.e. Dockerized projects).
- [#2436](https://github.com/clojure-emacs/cider/issues/2436): Prevent malformed `cider-repl-history-file`s from failing `cider-jack-in`.
- [#3456](https://github.com/clojure-emacs/cider/issues/3456): restore xref-based jump-to-definition in Babashka (and any nREPL clients not having cider-nrepl).
Expand Down
57 changes: 42 additions & 15 deletions cider-eval.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -678,19 +678,20 @@ If location could not be found, return nil."
(point))))
(list begin end buffer)))))))))))))

(defun cider-handle-compilation-errors (message eval-buffer)
"Highlight and jump to compilation error extracted from MESSAGE.
(defun cider-handle-compilation-errors (message eval-buffer &optional no-jump)
"Highlight and jump to compilation error extracted from MESSAGE, honor NO-JUMP.
EVAL-BUFFER is the buffer that was current during user's interactive
evaluation command. Honor `cider-auto-jump-to-error'."
(when-let* ((loc (cider--find-last-error-location message))
(overlay (make-overlay (nth 0 loc) (nth 1 loc) (nth 2 loc)))
(info (cider-extract-error-info cider-compilation-regexp message)))
(let* ((face (nth 3 info))
(note (nth 4 info))
(auto-jump (if (eq cider-auto-jump-to-error 'errors-only)
(not (or (eq face 'cider-warning-highlight-face)
(string-match-p "warning" note)))
cider-auto-jump-to-error)))
(auto-jump (unless no-jump
(if (eq cider-auto-jump-to-error 'errors-only)
(not (or (eq face 'cider-warning-highlight-face)
(string-match-p "warning" note)))
cider-auto-jump-to-error))))
(overlay-put overlay 'cider-note-p t)
(overlay-put overlay 'font-lock-face face)
(overlay-put overlay 'cider-note note)
Expand Down Expand Up @@ -774,6 +775,19 @@ REPL buffer. This is controlled via
(cider--make-fringe-overlay (point)))
(scan-error nil)))))

(defun cider--error-phase-of-last-exception (buffer)
"Returns the :phase of the latest exception associated to BUFFER, if any."
(when cider-clojure-compilation-error-phases
(when-let ((conn (with-current-buffer buffer
(cider-current-repl))))
(when (cider-nrepl-op-supported-p "analyze-last-stacktrace" conn)
(when-let* ((result (nrepl-send-sync-request (thread-last (map-merge 'list
Copy link
Member Author

Choose a reason for hiding this comment

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

There may be up to 2 analyze-last-stacktrace requests during the same interaction (one async in defun cider-default-err-op-handler, one sync in here).

About 1mo ago I had tried doing this sort of thing in just 1 async call, but was overly complex.

For avoiding any unnecessary costs, we can introduce some caching, cider-nrepl side. The cache would have a size of 1 (since one normally is only interested in analyzing the last exception - older exceptions are uninteresting / unlikely to be re-queried)

Copy link
Member Author

Choose a reason for hiding this comment

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

I checked this out and while our stacktrace analysis isn't blazing-fast, runtimes in the 20-40ms range don't seem worth caching.

'(("op" "analyze-last-stacktrace"))
(cider--nrepl-print-request-map fill-column))
(seq-mapcat #'identity))
conn)))
(nrepl-dict-get result "phase"))))))

(declare-function cider-inspect-last-result "cider-inspector")
(defun cider-interactive-eval-handler (&optional buffer place)
"Make an interactive eval handler for BUFFER.
Expand All @@ -796,17 +810,30 @@ when `cider-auto-inspect-after-eval' is non-nil."
(cider--display-interactive-eval-result res end))
(lambda (_buffer out)
(cider-emit-interactive-eval-output out))
(lambda (_buffer err)
(lambda (buffer err)
(cider-emit-interactive-eval-err-output err)

(when (or (not cider-show-error-buffer)
(not (cider-connection-has-capability-p 'jvm-compilation-errors)))

;; Display errors as temporary overlays
(let ((cider-result-use-clojure-font-lock nil))
(cider--display-interactive-eval-result
err end 'cider-error-overlay-face)))
(cider-handle-compilation-errors err eval-buffer))
(let ((phase (cider--error-phase-of-last-exception buffer)))
(when (or
;; if we won't show *cider-error*, because of configuration, the overlay is adequate because it compensates for the lack of info in a compact manner:
(not cider-show-error-buffer)
(not (cider-connection-has-capability-p 'jvm-compilation-errors))
;; if we won't show *cider-error*, because of an ignored phase, the overlay is adequate:
(and cider-show-error-buffer
(member phase cider-clojure-compilation-error-phases)))
;; Display errors as temporary overlays
(let ((cider-result-use-clojure-font-lock nil))
(cider--display-interactive-eval-result
err end 'cider-error-overlay-face)))

(cider-handle-compilation-errors err
eval-buffer
;; we prevent jumping behavior on compilation errors,
;; because lines tend to be spurious (e.g. 0:0)
;; and because on compilation errors, normally
;; the error is 'right there' in the current line
;; and needs no jumping:
phase)))
(lambda (buffer)
(if beg
(unless fringed
Expand Down