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

Use the cider--display-interactive-eval-result for more cases of compilation errors #3495

Open
vemv opened this issue Oct 3, 2023 · 5 comments

Comments

@vemv
Copy link
Member

vemv commented Oct 3, 2023

An overlay is displayed through cider--display-interactive-eval-result when a compilation error happens, but only when doing interactive evaluation.

(Note that that is the intended design, and fully makes sense: we use something named cider--display-interactive-eval-result)

However, from a user point of view, it is desirable to show this overlay under more use cases, which may not go under the cider-interactive-eval-handler (the part responsible for calling cider--display-interactive-eval-result)

  • cider-load-buffer (done)
  • cider-refresh

For both cases, besides from the display of an overlay, the jumping to the right line, as introduced in #3492 should also be offered. It might be more tricky for cider-refresh, since the cause of the error may reside in another file (possibly without a corresponding open buffer).

@magnars
Copy link
Contributor

magnars commented Oct 5, 2023

I would be willing to give this a shot, but am not familiar enough with the inner workings of CIDER to know where to begin. If I could get a few pointers, where to start looking, what sorts of things to look out for, I could see if I can figure something out.

@vemv
Copy link
Member Author

vemv commented Oct 5, 2023

cider-eval.el features a series of 'handler' defuns, please grep for: defun.*cider.*handler in that file.

Then, it's a matter of locating the relevant handler for cider-load-buffer, and modify it such that cider--display-interactive-eval-result is invoked, when adequate.

As noted in the OP we also want to reliably jump to the relevant line. That is accomplished with the cider-handle-compilation-errors function as can be seen in https://github.com/clojure-emacs/cider/pull/3492/files

Probably we want to only jump if the line:col is not 0:0 (some compilation errors are reported like that).

Hope it helps!

@magnars
Copy link
Contributor

magnars commented Oct 6, 2023

Thanks for the info, @vemv! It turns out cider-load-buffer is handled by cider-load-file-handler which it turns out already has this feature via cider-handle-compilation-errors. Its docstring says:

"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'."

The question then becomes why it doesn't seem to work. Debugging reveals that it is unable to find the location from the error. It looks like the regex being used does not match the syntax error message. The pattern matches this:

Syntax error compiling at

but my error message is

Syntax error reading source at

Adding this to the cider-clojure-1.10-error pattern does fix this to a certain degree, even if it does not use the cool new overlay. Instead it will add a squiggly red line with the message visible when hovering over the squiggly.

Any thoughts on further steps from here?

@vemv
Copy link
Member Author

vemv commented Oct 7, 2023

I'd say that the problem is that cider-handle-compilation-errors takes care of the squiggly and the jumping, but the overlay is provided via this separate call:

(let ((cider-result-use-clojure-font-lock nil))
  (cider--display-interactive-eval-result
   err end 'cider-error-overlay-face))

You can see both in action here

magnars added a commit to magnars/cider that referenced this issue Oct 9, 2023
Loading a buffer sometimes fails silently, since `cider-handle-compilation-errors` currently does not properly parse all syntax errors.

The pattern matches:

> Syntax error compiling at
> Syntax error macroexpanding at

but needs to also match:

> Syntax error reading source at

This PR fixes this by adding the missing string to the regex.

There is a relevant issue, clojure-emacs#3495, but that is larger in scope. Since evaluating code and getting no feedback about syntax errors is a bad experience, I propose that this be fixed without waiting for the additional quality of life improvements in that issue.
magnars added a commit to magnars/cider that referenced this issue Oct 9, 2023
Loading a buffer sometimes fails silently, since `cider-handle-compilation-errors` currently does not properly parse all syntax errors.

The pattern matches:

> Syntax error compiling at
> Syntax error macroexpanding at

but needs to also match:

> Syntax error reading source at

This PR fixes this by adding the missing string to the regex.

There is a relevant issue, clojure-emacs#3495, but that is larger in scope. Since evaluating code and getting no feedback about syntax errors is a bad experience, I propose that this be fixed without waiting for the additional quality of life improvements in that issue.
magnars added a commit to magnars/cider that referenced this issue Oct 9, 2023
Loading a buffer sometimes fails silently, since `cider-handle-compilation-errors` currently does not properly parse all syntax errors.

The pattern matches:

> Syntax error compiling at
> Syntax error macroexpanding at

but needs to also match:

> Syntax error reading source at

This PR fixes this by adding the missing string to the regex.

There is a relevant issue, clojure-emacs#3495, but that is larger in scope. Since evaluating code and getting no feedback about syntax errors is a bad experience, I propose that this be fixed without waiting for the additional quality of life improvements in that issue.
magnars added a commit to magnars/cider that referenced this issue Oct 9, 2023
Loading a buffer sometimes fails silently, since `cider-handle-compilation-errors` currently does not properly parse all syntax errors.

The pattern matches:

> Syntax error compiling at
> Syntax error macroexpanding at

but needs to also match:

> Syntax error reading source at

This PR fixes this by adding the missing string to the regex.

There is a relevant issue, clojure-emacs#3495, but that is larger in scope. Since evaluating code and getting no feedback about syntax errors is a bad experience, I propose that this be fixed without waiting for the additional quality of life improvements in that issue.
@daveliepmann
Copy link
Contributor

daveliepmann commented Oct 15, 2023

I think cider-pprint-eval-last-sexp also qualifies. Current behavior is to open a blank *cider-result*, no message in minibuffer, no overlay.


(@vemv edit: fixed in cider 1.10)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants