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

Update Sublime settings regarding window/showMessage #1218

Merged
merged 4 commits into from Dec 26, 2019

Conversation

@ayoub-benali
Copy link
Contributor

ayoub-benali commented Dec 21, 2019

Previously LSP displayed a dialog box for window/showMessage, latest version (0.9.3 via PR sublimelsp/LSP#811) displays that in the status bar.

Additionally:

  • make metals/status go as window/showMessage
  • removed some outdated comments.
Previously LSP displayed a dialog box for `window/showMessage`, latest
version display that in the status bar.

I also removed some outdated comments.
Copy link
Collaborator

tgodzik left a comment

Thanks @ayoub-benali for updating the docs! Just a small comment and I think we can merge 🎉

docs/editors/new-editor.md Show resolved Hide resolved
@gabro
gabro approved these changes Dec 26, 2019
Copy link
Member

gabro left a comment

Pretty cool to see Sublime getting better and better support 😍

I've left a minor comment, but LGTM regardless

// Sublime text opens an invasive alert dialogue for window/showMessage
// and window/showMessageRequest.
showMessage = ShowMessageConfig.logMessage,
showMessage = ShowMessageConfig.on,

This comment has been minimized.

Copy link
@gabro

gabro Dec 26, 2019

Member

I would remove this line so that it uses the default as the other editors (the default is "on" unless overridden by a system property)

Actually, we can probably remove ShowMessageConfig and ShowMessageRequestConfig altogether since as far as I know they were only used by Sublime Text.

I doubt anyone uses those system properties directly, and if they do, I would be curious to know why and better handle this on the server directly.

This comment has been minimized.

Copy link
@gabro

gabro Dec 26, 2019

Member

(It's fine to just remove the line for this PR, I can follow up with a different PR cleaning up the config options)

This comment has been minimized.

Copy link
@gabro

gabro Dec 26, 2019

Member

actually, I'll merge as is and follow up directly.

@gabro gabro merged commit b4fc595 into scalameta:master Dec 26, 2019
11 of 12 checks passed
11 of 12 checks passed
windows-latest jdk-11 unit tests
Details
macOS-latest jdk-11 unit tests
Details
ubuntu-latest jdk-8 unit tests ubuntu-latest jdk-8 unit tests
Details
ubuntu-latest jdk-11 unit tests
Details
Sbt integration
Details
Maven integration
Details
Gradle integration
Details
Mill integration
Details
Pants integration
Details
LSP integration tests
Details
Scala cross tests
Details
Scalafmt/Scalafix/Docs
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.