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

Add LSP-metals plugin #7758

Merged
merged 2 commits into from
Jan 3, 2020
Merged

Add LSP-metals plugin #7758

merged 2 commits into from
Jan 3, 2020

Conversation

ayoub-benali
Copy link
Contributor

@ayoub-benali ayoub-benali commented Nov 16, 2019

Metals support for Sublime's LSP plugin.
It is similar to other language handlers in https://github.com/sublimelsp (see #7566, #7568, #7569 ) but for Metals. So far it provides:

  • automatic installation for the server
  • handler custom server endpoints

More features are planned once the package published :)

cc @olafurpg

Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: WARNING

Repo link: LSP-metals
Results help

Packages added:
  - LSP-metals

Processing package "LSP-metals"
  - WARNING: '.no-sublime-package' is defined. Please verify that it is *really* necessary
  - WARNING: The package does not contain a top-level LICENSE file. A license helps users to contribute to the package.

@ayoub-benali
Copy link
Contributor Author

@packagecontrol-bot

Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: WARNING

Repo link: LSP-metals
Results help

Packages added:
  - LSP-metals

Processing package "LSP-metals"
  - WARNING: The package does not contain a top-level LICENSE file. A license helps users to contribute to the package.
  - WARNING: '.no-sublime-package' is defined. Please verify that it is *really* necessary

Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: WARNING

Repo link: LSP-metals
Results help

Packages added:
  - LSP-metals

Processing package "LSP-metals"
  - WARNING: The package does not contain a top-level LICENSE file. A license helps users to contribute to the package.
  - WARNING: '.no-sublime-package' is defined. Please verify that it is *really* necessary

@ayoub-benali
Copy link
Contributor Author

.no-sublime-package is required because the package embed and executable (coursier)

Copy link
Collaborator

@packagecontrol-bot packagecontrol-bot left a comment

Choose a reason for hiding this comment

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

Automated testing result: WARNING

Repo link: LSP-metals
Results help

Packages added:
  - LSP-metals

Processing package "LSP-metals"
  - WARNING: '.no-sublime-package' is defined. Please verify that it is *really* necessary

ayoub-benali pushed a commit to ayoub-benali/metals-sublime that referenced this pull request Nov 16, 2019
pasting the repo URL in sublime is probably easier for end user
and the package is always up to date.

This a temporary solution until wbond/package_control_channel#7758
is merged.
ayoub-benali added a commit to scalameta/metals-sublime that referenced this pull request Nov 17, 2019
* Update installation guide

pasting the repo URL in sublime is probably easier for end user
and the package is always up to date.

This a temporary solution until wbond/package_control_channel#7758
is merged.

* enable 'metals-sublime' by default

* put back stable version
@rwols
Copy link
Contributor

rwols commented Dec 27, 2019

Perhaps we can use os.path.join(sublime.cache_path(), "LSP-metals") to put the coursier binary there?

@ayoub-benali
Copy link
Contributor Author

@rwols I am not sure to follow, are you talking about this line ? https://github.com/scalameta/metals-sublime/blob/master/plugin.py#L9

@olafurpg
Copy link

If it simplifies things, the package could download the coursier binary on-demand from the URL https://git.io/coursier-cli

@FichteFoll
Copy link
Collaborator

@rwols how it currently stands, I believe this package to be mergeable, so I will go ahead and do that. It's by far not as problematic as the other LSP-* packages, because it ships a single cross-platform binary with it that is only 24KB. However, I will continue to request a review from you or another LSP developer/contributor to ensure what the package is doing aligns with LSP's interests.

@FichteFoll FichteFoll merged commit 4a96422 into wbond:master Jan 3, 2020
@ayoub-benali
Copy link
Contributor Author

Thanks @FichteFoll

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.

5 participants