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

Language Server Protocol for TLAPM #93

Merged
merged 145 commits into from
Aug 28, 2024
Merged

Language Server Protocol for TLAPM #93

merged 145 commits into from
Aug 28, 2024

Conversation

kape1395
Copy link
Collaborator

@kape1395 kape1395 commented Oct 10, 2023

The changes are over #83 and #91. Closes #90 and #94. The corresponding client part for the VSCode extension is already merged as tlaplus/vscode-tlaplus#307.

  • It invokes the parser directly (as a library function) to analyze the document structure and then invokes tlapm as a sub-process.
  • The changes to the tlapm are:
    • Accept a file to check from a stdin. This allows the LSP server to check unsaved documents.
    • Accept a file to check as a string when calling the TLAPM parser as a library. Used by the LSP server to locate proof steps.
    • The location tracking for the QED step was broken in the parser. Now that's fixed. Used in the LSP server to show the proof status.
  • The LSP server's part requires OCaml-5.1 because I use Eio for concurrency. I made it optional so the TLAPM can still be compiled in older OCaml versions.

From the functionality perspective:

  • Proof checking can be invoked by pressing ctrl+g+g or via the code action (yellow lamp along the cursor position).
  • Proof status is shown by marking document lines in different colors; the hover explanations are also present.
  • Failed proofs are shown as diagnostics.
  • Progress of the proof checking is shown (status bar). The operation can be canceled.
  • If a new proof-checking command is invoked while the old one is still running, the old invocation is canceled automatically.

@damiendoligez
Copy link
Contributor

Now that #83 and #91 are merged, could you rebase this PR? This will make reviewing easier.

Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
kape1395 and others added 12 commits June 8, 2024 11:12
Co-authored-by: Damien Doligez <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Co-authored-by: Damien Doligez <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Co-authored-by: Damien Doligez <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Co-authored-by: Damien Doligez <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Co-authored-by: Damien Doligez <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Co-authored-by: Damien Doligez <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Co-authored-by: Damien Doligez <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Co-authored-by: Damien Doligez <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Co-authored-by: Damien Doligez <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
Copy link
Contributor

@damiendoligez damiendoligez left a comment

Choose a reason for hiding this comment

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

This concludes my review. Good to merge after you look at these last few comments.

src/loader.ml Outdated Show resolved Hide resolved
src/tlapm_args.ml Outdated Show resolved Hide resolved
src/tlapm_lib.ml Outdated Show resolved Hide resolved
src/tlapm_args.ml Outdated Show resolved Hide resolved
Signed-off-by: Karolis Petrauskas <[email protected]>
Signed-off-by: Karolis Petrauskas <[email protected]>
@kape1395 kape1395 marked this pull request as ready for review August 27, 2024 22:18
@kape1395
Copy link
Collaborator Author

@damiendoligez, it seems I updated the PR according to all the comments. Thanks; they were all helpful.

@kape1395 kape1395 merged commit 6acb3cb into tlaplus:main Aug 28, 2024
7 checks passed
@lemmy
Copy link
Member

lemmy commented Sep 2, 2024

Minor regression: #149

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

Successfully merging this pull request may close these issues.

LSP support
5 participants