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

Typechecker diagnostic messages in language server #184

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

ehllie
Copy link
Contributor

@ehllie ehllie commented Jan 24, 2024

Further work towards #179

@ehllie
Copy link
Contributor Author

ehllie commented Jan 24, 2024

I've left some relevant comments. This should be good to merge as is, but I have one idea/ suggestion for a change to the compiler that would help with the language server. It would be something akin to passing a file handler to the compiler and cache, and rather than reading files by creating a File, it would ask the handler for the file contents. That way the lsp could provide access to files that first goes through the document_map, allowing the compiler to run even if the edits made to the files are not saved to the disk, and also possibly speed up compilation by skipping the need to do filesystem io each time.

Copy link
Owner

@jfecher jfecher left a comment

Choose a reason for hiding this comment

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

image
Amazing 🤩!
Next steps would be to remove the full file path and source line from the error message I think. If we have access to the original Diagnostic, the inner DiagnosticKind should give us just the error message itself.

I agree with your suggestions you gave as well, it'd be good if we could retrieve the file contents directly from the ls and didn't have to re-read them.

It'd also be nice if we were able to make use of the check method in ante/src/main.rs so we don't have to repeat logic for lexing, parsing, name resolution, and type checking in the language server.

ante-ls/src/main.rs Show resolved Hide resolved
ante-ls/src/main.rs Show resolved Hide resolved
@jfecher jfecher merged commit dbfee06 into jfecher:master Jan 24, 2024
1 check passed
@ehllie
Copy link
Contributor Author

ehllie commented Jan 25, 2024

image Amazing 🤩! Next steps would be to remove the full file path and source line from the error message I think. If we have access to the original Diagnostic, the inner DiagnosticKind should give us just the error message itself.

I agree with your suggestions you gave as well, it'd be good if we could retrieve the file contents directly from the ls and didn't have to re-read them.

It'd also be nice if we were able to make use of the check method in ante/src/main.rs so we don't have to repeat logic for lexing, parsing, name resolution, and type checking in the language server.

It's interesting how it's using absolute paths for you. It should be using relative paths to the root of the project, and at least it's doing that for me. But I guess I'm currently doing it by "lying" to the compiler about what path the file is at, and it could be done in a smarter way.

image

@ehllie ehllie deleted the language-server branch January 28, 2024 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants