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

Unregister language server extension on LS shutdown #608

Merged

Conversation

AObuchow
Copy link
Contributor

@AObuchow AObuchow commented Feb 6, 2020

fix #605

Signed-off-by: Andrew Obuchowicz [email protected]

@AObuchow AObuchow force-pushed the dipose_extensions_on_shutdown branch 3 times, most recently from c715135 to 39908ec Compare February 6, 2020 19:55
@angelozerr
Copy link
Contributor

@AObuchow AObuchow force-pushed the dipose_extensions_on_shutdown branch from 912e1d3 to c4b7450 Compare February 6, 2020 21:30
@AObuchow
Copy link
Contributor Author

AObuchow commented Feb 6, 2020

Last thing to do, do try/cacth inside unregisterExtension like it's done in registerExtension to be sure to call stop for each extension even if there is an extension which fails.

https://github.com/angelozerr/lsp4xml/blob/1057f73f1d29c91b0bcb20a1c78ae72e03d8c21a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/services/extensions/XMLExtensionsRegistry.java#L90

I notice that in

https://github.com/angelozerr/lsp4xml/blob/1057f73f1d29c91b0bcb20a1c78ae72e03d8c21a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/services/extensions/XMLExtensionsRegistry.java#L90

no try catch is done, please do it.
Thanks!

Ok, it's done

@angelozerr angelozerr merged commit 8b086af into eclipse-lemminx:master Feb 7, 2020
@angelozerr
Copy link
Contributor

It's perfect @AObuchow, thanks!

@AObuchow
Copy link
Contributor Author

AObuchow commented Feb 7, 2020

@angelozerr thanks for the review! Now the Maven extension's test should be happy :)

@angelozerr
Copy link
Contributor

@angelozerr thanks for the review!

You are welcome!

Now the Maven extension's test should be happy :)

Nice :)

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.

Unregister language server extension on LS shutdown
2 participants