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

Don't start server when workspace is undefined. #33

Closed ayoub-benali closed 3 years ago

ayoub-benali commented 3 years ago

fixes https://github.com/scalameta/metals-sublime/issues/30

@rwols is checking workspace_folders alone is enough or rootPath and rootUri are also required ?

ckipp01 commented 3 years ago

@rwols is checking workspace_folders alone is enough or rootPath and rootUri are also required ?

Both rootPath and rootUri are technically deprecated, so the check on workspace_folders should be enough if the core sets it.

So, we recently had this very same discussion on the VS Code extension, since technically, Metals says it supports standalone Scala files, but in by adding this, we no longer allow it. I see two different routes:

  1. If no workspace_folders or rootPath or rootUri are found, then you show a warning and don't start Metals. (This is what VS Code does right now for the Metals extension)
  2. If none of those are round, you default the workspace_folder[0] to being the parent directory (This is what I chose to do in nvim-metals). Then you do get standalone Standalone Scala file support, but you may risk confusion about why your project isn't working when you open /src/main/scala/Main.scala directly.

I felt fine choosing 2 because the function that fines the root recurses down until it fines a build file or a git directory. Only if it doesn't it then sets the parent. So even if I directly open Main.scala located in /workspace/src/main/scala/Main.scala, it will find correctly find the root being /workspace. So I'd say it depends on whether sublime traverses and looks for a root pattern or if it just takes the root of what was opened.

ayoub-benali commented 3 years ago

LSP already picks workspace_folder[0] as the parent directory https://github.com/sublimelsp/LSP/blob/c41435fd6f50e479f0b6cf87d7323cbaf913f594/plugin/core/sessions.py#L119

If no workspace_folders or rootPath or rootUri are found, then you show a warning and don't start Metals. (This is what VS Code does right now for the Metals extension)

This PR attempts the same. Server isn't started so behavior is consistent with VS code as well

ayoub-benali commented 3 years ago

Remember you can use on_pre_start instead and mutate the workspace_folders

I would rather be explicit and let the user choose his workspace. Often in scala the root workspace is where the build tool file is, not the parent directory of the opened file.

ckipp01 commented 3 years ago

Remember you can use on_pre_start instead and mutate the workspace_folders

I would rather be explicit and let the user choose his workspace. Often in scala the root workspace is where the build tool file is, not the parent directory of the opened file.

I'd also just ensure that this works as expected with a standalone Ammonite script for example.

ayoub-benali commented 3 years ago

TBH I never used and tested that. Do you mean worksheets ? or using Metals in directory where there is only .sc file and no build file ?

ckipp01 commented 3 years ago

TBH I never used and tested that. Do you mean worksheets ? or using Metals in directory where there is only .sc file and no build file ?

I don't mean worksheets, but Ammonite. So if Metals opens for example myScripts/test.sc, test.sc may be a valid standalone Ammonite script that a user wants to edit. I'm curious if that would work with Sublime. If it hasn't been tested before, then it may be something to just look into after this.

ayoub-benali commented 3 years ago

I just tested and it works as long as the workspace is first opened myScripts and then test.sc is opened inside sublime. Opening the file directly yields the error messaged added in this PR.

Is this also consistent with the other Metals clients ?

ckipp01 commented 3 years ago

I just tested and it works as long as the workspace is first opened myScripts and then test.sc is opened inside sublime. Opening the file directly yields the error messaged added in this PR.

Is this also consistent with the other Metals clients ?

I believe with VS Code, but not the Vim clients.