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

LSP: Using a virtual filesystem #524

Merged
merged 2 commits into from
Dec 24, 2024
Merged

LSP: Using a virtual filesystem #524

merged 2 commits into from
Dec 24, 2024

Conversation

kengorab
Copy link
Owner

Add a primitive notion of a "virtual filesystem" (which is just a Map<String, String> for now) to represent abra files that require typechecking but which have not yet been saved to disk. For now, the contents of this map are wholesale replaced upon receipt of a textDocument/didChange notification from the LSP client, and removed upon textDocument/didSave (since at that point, the file can once again be read from disk to obtain the latest version).

In the future, there ought to be a more sophisticated way of representing the file contents in the virtual filesystem, which leverages the Incremental contentChange option for textDocument/didChange notifications, rather than having to wastefully parse and replace the entire contents of the document each time. In fact, the LSP currently falls over when trying to operate on large files, most likely for this reason.

Add a primitive notion of a "virtual filesystem" (which is just a
`Map<String, String>` for now) to represent abra files that require
typechecking but which have not yet been saved to disk. For now, the
contents of this map are wholesale replaced upon receipt of a
`textDocument/didChange` notification from the LSP client, and removed
upon `textDocument/didSave` (since at that point, the file can once
again be read from disk to obtain the latest version).

In the future, there ought to be a more sophisticated way of
representing the file contents in the virtual filesystem, which
leverages the Incremental `contentChange` option for
`textDocument/didChange` notifications, rather than having to wastefully
parse and replace the entire contents of the document each time. In
fact, the LSP currently falls over when trying to operate on large
files, most likely for this reason.
@kengorab kengorab added the lsp label Dec 20, 2024
Using this current approach, the entire contents of the changed file
will be sent in `textDocument/didChange` events (as well as
`textDocument/didOpen`, which is less relevant but still wasteful).
Until a more performant system is developed to leverage Incremental
didChange events, let's only run diagnostics on save and instruct the
client to not send didOpen or didChange events. Additionally, we
instruct the client to not send the file contents in the didSave event.
@kengorab kengorab merged commit 2fcbb08 into master Dec 24, 2024
1 check passed
@kengorab kengorab deleted the lsp-add-vfs branch December 24, 2024 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant