search

scalameta / metals-sublime

Sublime Text package for Metals, a language server for Scala
https://packagecontrol.io/packages/LSP-metals
Apache License 2.0
16 stars 10 forks source link

Make the plugin LSP 1.1 compatible #28

Closed rwols closed 3 years ago

rwols commented 3 years ago

I have a draft pull request where I remove the LanguageConfig entirely and replace it by a selector. This is much more practical. See: https://github.com/sublimelsp/LSP/pull/1428

Because there are many other improvements for the ST4 version I decided to add all improvements as well. For instance it's fairly easy to write custom notifications and request handlers with the AbstractPlugin class. It's also trivial now to put a status message on all views attached to the session. And also registration of plugins is now explicit (in plugin_loaded, plugin_unloaded) which allows users to put the package in their "ignored_packages" setting and then LSP will pick up this change in state and stop the server subprocess (likewise for when the user enables the package).

I also moved the helper py files to their own directory to ensure that ST doesn't load these files as separate plugins (but merely, they are imported by a single plugin file called plugin.py).

The changes ensure compatibility with both ST3 and ST4. Though I haven't tested if everything still works on ST3 with these changes.

Don't merge/release this unless the above mentioned pull request is merged.

ayoub-benali commented 3 years ago

@rwols thanks for taking care of this. I will review it later today :)

rchl commented 3 years ago

I'm working on lsp_utils refactoring that would hopefully make it easier to rewrite this in a more concise matter. I should have it ready in a few days or so.

(Do note I haven't checked the requirements of this plugin so I'm not sure how suitable that will be).

rwols commented 3 years ago

@ayoub-benali did you find the time to test this? Can we merge this soonish?

rwols commented 3 years ago

It’s the other way around. These changes make sure that this plugin will work with the linked changes in the LSP repo. So this should be merged and released first.

rwols commented 3 years ago

I notice now that I said in my initial comment “don’t merge this until the above mentioned pull request is merged” but that is wrong.

ayoub-benali commented 3 years ago

@rwols new release is out. Hopefully this would unblock https://github.com/sublimelsp/LSP/pull/1428 and thanks again :)

rwols commented 3 years ago

Awesome, thanks!