Allow to report syntax errors using LSP error and/or "window/showMessage" notification#2
Merged
Isopod merged 6 commits intoIsopod:masterfrom Nov 17, 2022
Merged
Conversation
window/showMessage Also, fixed the fake completion item: needs isIncomplete for Emacs to not raise Lisp errors.
michaliskambi
added a commit
to michaliskambi/elisp
that referenced
this pull request
Nov 14, 2022
Isopod
requested changes
Nov 14, 2022
Owner
Isopod
left a comment
Isopod
left a comment
There was a problem hiding this comment.
Thanks for the merge request. I added a few comments. Other than that it looks good.
to make it first -- to have best looking docs about it
Isopod
approved these changes
Nov 16, 2022
Owner
Isopod
left a comment
Isopod
left a comment
There was a problem hiding this comment.
Yep, looks good. Is this ready to be merged?
Author
Yes, all done, including last docs (README) improvements :) |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Looking for a perfect way to report Pascal syntax errors (easily caused by missing Pascal units), this PR gives users 2 new ways to see error:
window/showMessage. This is supported by VS Code nicely, by Emacs too (but poorly). It is by defaulttrue(as it seems to do no harm, in the worst case clients that don't support it will just ignore it).by returning LSP error instead of a "fake completion item". The fake completion item works poorly in Emacs, the LSP error is visible nicer. It is by default
falseto not break previous behavior. Emacs users may likely prefer to turn this on.Both these can be controlled by new
syntaxErrorCausesLspError,syntaxErrorCausesShowMessageflags that clients can pass as LSP initialization options.The options are documented in README.md.
See michaliskambi/elisp#1 for various details about Emacs.
Codewise, I have introduced 2 new classes:
TRpcNotificationandTRpcMessageToClient(common ancestor forTRpcNotificationandTRpcResponse).window/showMessageis a "notification" in JSON-RPC sense (see https://www.jsonrpc.org/specification#notification ) and it seemed cleanest to express it as a new class (rather then overusingTRpcResponse). Following JSON-RPC ver 2 spec, "notifications" differ from "responses" in that:Notifications do not pass any id. (Trying to pass
idwithwindow/showMessagewas leading to weirdest errors from both VS Code and Emacs :) ).The other side (LSP client in this case) is not supposed to reply. This is nice, it means we don't have to handle anything extra (no need to dispatch some LSP client answer) to support
window/showMessage.BTW 2 fixes:
fake completion item contains
isIncomplete, as required by LSP spec. Without it, Emacs throws scary Lisp error.I noticed that
TRpcPeer.Receivehas accidentallyResult.Reader := Readerassignment 2 times.