-
-
Notifications
You must be signed in to change notification settings - Fork 922
Add: Support Scala Language #528
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
base: main
Are you sure you want to change the base?
Conversation
4a003d2
to
47431d5
Compare
cs_command_path, | ||
"bootstrap", | ||
"--java-opt", | ||
"-XX:+UseG1GC", | ||
"--java-opt", | ||
"-XX:+UseStringDeduplication", | ||
"--java-opt", | ||
"-Xss4m", | ||
"--java-opt", | ||
"-Xms100m", | ||
"--java-opt", | ||
"-Dmetals.client=Serena", | ||
artifact, | ||
"-o", | ||
metals_executable, | ||
"-f", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good, tests for finding references are missing before it can be merged. Thanks for adding Scala support to Serena!
yield ls | ||
|
||
|
||
def test_scala_document_symbols(scala_ls): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as mentioned above, pls add tests on finding references within the same file and across files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comment below
f25090f
to
3717e3c
Compare
You probably need to override and increase |
I merged main where a fix regarding this was recently pushed and bump the waiting time. Let's see if that worked |
I also passes locally for me on ubuntu |
I think that not only LSP but also build tool dependencies are necessary. |
fe14eed
to
f00392a
Compare
I think the state in b787b30 was good, maybe just more waiting needed. It did work locally for me without installing any build tools, just after installing coursier |
Thank you for your confirmation. I will revert the commit! |
4892843
to
1ddd8e3
Compare
... |
I think a 5 sec wait was enough, it's running through on Ubuntu and macOS. I'll have a look at what's wrong on windows, I suppose either some path issue (using wrong separator), or a fundamental problem in the LS itself |
5ad195a
to
56ac8cd
Compare
This reverts commit a1e66b7.
Implements the `request_references` method in `ScalaLanguageServer` to handle Metals specific behavior of returning incomplete results. The implementation waits for a signal that compilation is done and retries the request to get complete results.
611d759
to
86f3a5e
Compare
bb713ef
to
e628787
Compare
e628787
to
7669054
Compare
30d2632
to
596a6e6
Compare
@YuMuuu thanks! I will try running the tests locally by following the guide and if everything works out, we can merge this |
@YuMuuu I readded the test files to try the setup you recommend in the docs. I added the plugins.sbt as suggested there. With the manual setup,
Is that to be expected? I thought the test files were a valid scala project. Btw, I have a new laptop with windows and had to install the scala toolchain from scratch. The executables I get are all |
close: #522
This PR adds comprehensive Scala language support to serena.
I have implementation of solidlsp for scala(metals).
required dependency: coursier