-
Notifications
You must be signed in to change notification settings - Fork 39
Compiles and run on Scala 2.12, latest version of Ensime (2.0.0-M4). Added settings to change server log level from VSCode #54
Conversation
fede0664
commented
Sep 11, 2017
- Scala 2.12: The challenge was upgrading scala-json-rpc to version 2.0.0, required a few changes on Commands.scala and JsonRpcUtils.scala
- Ensime (2.0.0-M4): implemented changes suggested at bump the server #50. The ActorInitializationException seems to be gone, but Ensime still has some irrecoverable fails, for example when getting the definition of a Manifest. Trouble showing local val tooltip #37 still not fixed.
- Settings change: Added a flag in VSCode settings that is sent to Coursier to change the default logging level for org.github.dragos.vscode on loogback.groovy. The goal of this is to limit the verbosity of the log on production, log files grow quickly to a several GBs in DEBUG level.
* Added support to scalariformFormat in package.json * Added support for scalariformFormat in sbt task handler
The extension is useless without it, and this avoids conflicts with other extensions like the Dotty Language Server (which will only start when a .dotty-ide.json file is present after scala/scala3#2777)
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.
Great work! I would ask you to try to follow the coding style in the project, in particular to have a space after :
in type ascription. I'll approve the PR and wait a bit to see if you have time to update it, otherwise I'll do it later.
I just noticed that Document Symbols stopped working. I see these errors in the log, perhaps related to the upgrade to scala-json-rpc 2.0?
it looks like the error is a Json validation error, I wonder why...
|
override def reads(json: JsValue) = Reads.of[B].reads(json).map(apply(_)) | ||
override def writes(o: A) = Writes.of[B].writes(unapply(o)) | ||
override def writes(o: A) = Writes.of[B].writes(unapply(o)).as[JsObject] |
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.
This seems to be the cause of the exception I reported earlier. In the case of DocumentSymbolResult
the underlying type is a sequence, not a Json object.
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.
If that's the case we'll need a similar workaround like the one I implemented for the Locations for "textDocument/definition". It seems that the latest play-json-rcp Message.MessageFormats only accepts JsObject, can't deal with JsValue. I'll take a look later today.
Can you add a test case to include this case?
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.
I pushed the changes to fix the issue with "textDocument/definition". I had to use the same workaround used for the Seq[Location]. Changed JsonRpcUtils.valueFormat to work only with JsObject.
Fantastic, thanks a lot! Merging this. |
Yay!! You cannot imagine the atrocities I've done to my 2.12 build to support the vscode integration for scalac-performance here: https://github.com/scalacenter/scalac-profiling. |
Hey. scala-json-rpc author here. I see now from looking over this (found it via https://twitter.com/jaguarul/status/907514621728260096) that this was a regression in scala-json-rpc 2.0.0. I've just fixed it via dhpiggott/scala-json-rpc#48 and released the fixed version as 2.0.1, so it should be possible now to update dragos-vscode-scala and remove these workarounds. |
Thank you @dhpiggott! Open-source at its finest! |