Closed lucasfigueiredo closed 6 years ago
Hi @lucasfigueiredo! Thanks for reporting this.
That tools.jar
is needed for ENSIME I see from the linked issue, but it seems that it's not really needed for the metals server. I'm going to double check it and if it's true we can just remove this check and move the jar mention to the ENSIME-specific part of the code. Do you want to submit a PR with this change?
Sorry, right now I am still learning how to use github and in this case I am not sure how I would test it (I mean, generating a new bundle and adding to atom).
If it helps, I had a look at your code (surprisingly small) and I did not find a place to move it because both Ensime and Scalameta ServerTypes
are case objects and do not seem to have a method for checking things.
One option would be to leave this check to ensime.
Another would be to force a check here, I guess:
global.atom.config.get("ide-scala.serverType").toString match {
case "scalameta" => Scalameta
case "ensime" =>
// insert some ensime-specific checks here
Ensime
}
Sorry, right now I am still learning how to use github
@lucasfigueiredo That's not a problem! Just let me know if you're willing to try and I will help you to get around 😉 You can start with checking the Contributing guideline in the repo. It also links to the Github help page about pull-requests.
and in this case I am not sure how I would test it (I mean, generating a new bundle and adding to atom).
And this is also covered by the Development guide. Check it out and if something is not detailed enough or confusing, tell me, I'll be glad to improve it.
both Ensime and Scalameta ServerTypes are case objects and do not seem to have a method for checking things.
I think we can do a simple thing here:
tools.jar
existence"--extra-jars", toolsJar
to the Ensime server type's coursierArgs
This will unblock the Scalameta server for now. Ensime may crash when you try to run it on Java 9, but a) this should be taken care of in Ensime (the issue you linked) and b) Ensime support in this plugin right now is purely experimental and related implementation will change anyway (see #13 and https://github.com/laughedelic/atom-ide-scala/pull/3#issuecomment-345897287).
@lucasfigueiredo it looks like @Jarlakxen has already nailed it in #32, so you can check out his implementation. I'm also going to make a release soon so you'll be able to try it out and see if it solves the original problem.
I want to test this too
Thanks guys. Just a newbie question... How do I test it? Inside Atom the available version is still the 0.5.0. How do I test the current master-branch / snapshot?
@lucasfigueiredo check the General setup part in the Development guide: you need to uninstall first the plugin you got from the store, then link the cloned repo with apm link --dev
, then run sbt package
, then apm install
and you're ready to go. Don't hesitate to ask for more details if you have any problems with it. It's probably more convenient to discuss in the chat.
@cquiroz I guess you already know how to test it locally, so if you could check master
before I make a release, it would be very helpful.
I get this: "Java Home is not found or is invalid. Try to set it explicitly in the plugin settings. Scala (Scalameta)" even after setting the Java Home in the plugin settings.
My guess: Java 9 no longer has a jar called tools.jar and the
class ScalaLanguageClient
is trying to find it:Fs.existsSync(Path.join(javaHome, "lib", "tools.jar"))
This seems to be related to a similar bug in ensime