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

Use CompletableFuture to load DOMDocument #449

Merged
merged 1 commit into from
Jun 19, 2019
Merged

Use CompletableFuture to load DOMDocument #449

merged 1 commit into from
Jun 19, 2019

Conversation

angelozerr
Copy link
Contributor

Fix #439

This PR uses CompletableFuture to load DOMDocument. The basic idea is when a service is called (ex: foldingRanges) it load DOM document if it need and then execute foldingRanges compute.

It's more cleaner than using a synchronized keyword. This PR remove the language model cache (which stored the DOMDocument in a map) and now the ModelTextDocument (which extends TextDocument) provides a getModel which returns the completable future which load the DOM document.

This PR should improve the stop of parse of DOM document which is done when requets is canceled (by client) or when text document version changed.

Copy link
Contributor

@NikolasKomonen NikolasKomonen left a comment

Choose a reason for hiding this comment

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

I'm not completely sure about the validity code wise. But the PR works well for me, no issues found from a user level.

@angelozerr
Copy link
Contributor Author

I'm not completely sure about the validity code wise. But the PR works well for me, no issues found from a user level.

Thank's @NikolasKomonen for your feedback. @fbricon I merge this PR, if you don't like it, please remove my PR. Thanks!

@angelozerr angelozerr merged commit 231bf5d into master Jun 19, 2019
@angelozerr angelozerr deleted the async-dom branch June 19, 2019 16:44
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.

Use CompletableFuture to load DOMDocument
2 participants