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 #605

Closed
AObuchow opened this issue Feb 5, 2020 · 5 comments · Fixed by #608
Closed

Unregister language server extension on LS shutdown #605

AObuchow opened this issue Feb 5, 2020 · 5 comments · Fixed by #608
Assignees
Milestone

Comments

@AObuchow
Copy link
Contributor

AObuchow commented Feb 5, 2020

For the LemMinX/lsp4xml maven extension testing suite, it's required that IXMLExtension.stop() be called when the LS gets shutdown. In our case, file locks for Maven index files get released when the extension is unregistered. However, currently the extensions aren't unregistered when the server is shutdown.

Unless there is a suitable workaround, having XML LS extensions get unregistered on LS shutdown would be greatly appreciated.

@AObuchow
Copy link
Contributor Author

AObuchow commented Feb 5, 2020

Also I can submit a seperate bug for this, but I just noticed that the ExtensionRegistry.unregister... methods seem to call List.add(). Is this intentional?

@fbricon
Copy link
Contributor

fbricon commented Feb 5, 2020

Also I can submit a seperate bug for this, but I just noticed that the ExtensionRegistry.unregister... methods seem to call List.add(). Is this intentional?

definitely a bug. All methods were copied/pasted, all unregister methods are wrong, so yes please open another ticket.

As for your initial question, I'll let @angelozerr respond

@angelozerr
Copy link
Contributor

definitely a bug. All methods were copied/pasted, all unregister methods are wrong, so yes please open another ticket.

Exactly, we don't use this feature @AObuchow please create a PR for fixing that.

Unless there is a suitable workaround, having XML LS extensions get unregistered on LS shutdown would be greatly appreciated.

Unregister extension is not supported (just coding some unregister method on the start of the project, but nothing else).

PLease create a PR for that, I suggest you that you add:

  • a dispose method in the XMLExtensionRegistry which loops for each extensions and call the existing unregisterExtension
  • call this dispose method on the XML Language server shutdown.

@AObuchow
Copy link
Contributor Author

AObuchow commented Feb 6, 2020

Also I can submit a seperate bug for this, but I just noticed that the ExtensionRegistry.unregister... methods seem to call List.add(). Is this intentional?

definitely a bug. All methods were copied/pasted, all unregister methods are wrong, so yes please open another ticket.

As for your initial question, I'll let @angelozerr respond

Sounds good, I'll open a ticket and submit a PR

AObuchow added a commit to AObuchow/lsp4xml that referenced this issue Feb 6, 2020
@AObuchow
Copy link
Contributor Author

AObuchow commented Feb 6, 2020

PLease create a PR for that, I suggest you that you add:

  • a dispose method in the XMLExtensionRegistry which loops for each extensions and call the existing unregisterExtension
  • call this dispose method on the XML Language server shutdown.

Sounds good, will submit a PR momentarily.

AObuchow added a commit to AObuchow/lsp4xml that referenced this issue Feb 6, 2020
AObuchow added a commit to AObuchow/lsp4xml that referenced this issue Feb 6, 2020
AObuchow added a commit to AObuchow/lsp4xml that referenced this issue Feb 6, 2020
AObuchow added a commit to AObuchow/lsp4xml that referenced this issue Feb 6, 2020
AObuchow added a commit to AObuchow/lsp4xml that referenced this issue Feb 6, 2020
angelozerr pushed a commit that referenced this issue Feb 7, 2020
@angelozerr angelozerr added this to the v0.9.2 milestone Feb 7, 2020
@angelozerr angelozerr modified the milestones: v0.9.2, 0.10.1 Mar 9, 2020
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 a pull request may close this issue.

3 participants