Add missing case for Clojure 1.10 syntax errors #3506
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
There are no more cases: so LGTM! Simply make sure to expand |
magnars
force-pushed
the
patch-2
branch
2 times, most recently
from
51e1db8 to
68ca5b4
Compare
|
I have added the test. There is one failing check, a timeout on Windows. I don't see any way to trigger just that test for a re-run, so I hope this should be enough to pass. Thanks for verifying that these are the only three cases, @vemv |
vemv
reviewed
Oct 9, 2023
test/cider-eval-tests.el
Outdated
| @@ -55,5 +55,7 @@ | |||
| :to-equal '("src/haystack/analyzer.clj" 18 1 cider-error-highlight-face "Syntax error compiling clojure.core/let at (src/haystack/analyzer.clj:18:1).\n[1] - failed: even-number-of-forms? at: [:bindings] spec: :clojure.core.specs.alpha/bindings\n")) | |||
| (expect (cider-extract-error-info cider-compilation-regexp "Syntax error macroexpanding clojure.core/let at (src/haystack/analyzer.clj:18:1).\n[1] - failed: even-number-of-forms? at: [:bindings] spec: :clojure.core.specs.alpha/bindings\n") | |||
| :to-equal '("src/haystack/analyzer.clj" 18 1 cider-error-highlight-face "Syntax error macroexpanding clojure.core/let at (src/haystack/analyzer.clj:18:1).\n[1] - failed: even-number-of-forms? at: [:bindings] spec: :clojure.core.specs.alpha/bindings\n")) | |||
| (expect (cider-extract-error-info cider-compilation-regexp "Syntax error reading source clojure.core/let at (src/haystack/analyzer.clj:18:1).\n[1] - failed: even-number-of-forms? at: [:bindings] spec: :clojure.core.specs.alpha/bindings\n") | |||
| :to-equal '("src/haystack/analyzer.clj" 18 1 cider-error-highlight-face "Syntax error reading source clojure.core/let at (src/haystack/analyzer.clj:18:1).\n[1] - failed: even-number-of-forms? at: [:bindings] spec: :clojure.core.specs.alpha/bindings\n")) | |||
There was a problem hiding this comment.
The exact message I get for e.g. ::::e is: "Syntax error reading source at (/Users/vemv/haystack/src/haystack/parser.cljc:13:0)."
Please let's reflect such a message here - the added test appears to be synthetic.
There was a problem hiding this comment.
Agreed, that's an improvement for sure. Done.
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.
|
Thanks! |
|
Thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Loading a buffer sometimes fails silently, since
cider-handle-compilation-errorscurrently does not properly parse all syntax errors.The pattern matches:
but needs to also match:
This PR fixes this by adding the missing string to the regex.
There is a relevant issue, #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.
I will start work on all the checkboxes below if this seems like a fair change.
Before submitting the PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):
eldev test)eldev lint) which is based onelisp-lintand includescheckdoc, check-declare, packaging metadata, indentation, and trailing whitespace checks.Thanks!
If you're just starting out to hack on CIDER you might find this section of its
manual extremely useful.